Re: [PATCH v3 4/4] PCI: rzg3s-host: Add support for RZ/V2H(P) SoC

[Lad, Prabhakar]

Hi Claudiu,

Thank you for the review.

On Tue, Jun 2, 2026 at 9:00 AM Claudiu Beznea <claudiu.beznea@xxxxxxxxx> wrote:
>
> Hi, Prabhakar,
>
> On 5/20/26 19:48, Prabhakar wrote:
> > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx>
> >
> > Add support for the RZ/V2H(P) SoC PCIe controllers to the rzg3s-host
> > driver.
> >
> > The RZ/V2H(P) SoC features two independent PCIe controllers that share
> > four physical lanes. The hardware supports two configuration modes:
> > single x4 mode where the first controller uses all four lanes, or dual
> > x2 mode where both controllers use two lanes each.
> >
> > Introduce a setup_lanes() function pointer to configure the PCIe lanes
> > based on the hardware instance. Implement rzv2h_pcie_setup_lanes() to
> > detect the configuration at boot time and program the lane mode via the
> > system controller.
> >
> > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx>
> > ---
> > v2->v3:
> > - Parsed controller-id from the "renesas,sysc" property instead of
> >    using linux,pci-domain.
> >
> > v1->v2:
> > - Updated commit message.
> > - Added locks to protect shared lane configuration state and
> >    prevent concurrent access issues during probe.
> > - Added cleanup action to release lanes on driver removal.
> > - Reconfigured RZG3S_SYSC_FUNC_ID_LINK_MASTER in resume path.
> > - Renamed num_channels to num_pcie_controllers for clarity.
> > ---
> >   drivers/pci/controller/pcie-rzg3s-host.c | 180 +++++++++++++++++++++++
> >   1 file changed, 180 insertions(+)
> >
> > diff --git a/drivers/pci/controller/pcie-rzg3s-host.c b/drivers/pci/controller/pcie-rzg3s-host.c
> > index edb49af7429a..15879b2c0bd2 100644
>
> [ ...]
>
> > +static int rzv2h_pcie_setup_lanes(struct rzg3s_pcie_host *host)
> > +{
> > +     struct device_node *np = host->dev->of_node;
> > +     u32 num_lanes;
> > +     int ret;
> > +
> > +     ret = of_property_read_u32(np, "num-lanes", &num_lanes);
> > +     if (ret)
> > +             return ret;
> > +
> > +     /*
> > +      * RZ/V2H(P) supports up to 4 lanes, but only in single x4 mode
> > +      * for the first controller. Dual x2 mode is supported with 2
> > +      * lanes for both controllers.
> > +      */
> > +     if (num_lanes != 4 && num_lanes != 2)
> > +             return -EINVAL;
> > +
> > +     if (host->controller_id == RZG3S_PCIE_CONTROLLER_ID_1 && num_lanes > 2)
> > +             return -EINVAL;
> > +
> > +     guard(spinlock)(&rzv2h_lane_lock);
> > +     if (rzv2h_num_total_lanes + num_lanes > RZV2H_PCIE_MAX_LANES)
> > +             return -EINVAL;
> > +
> > +     ret = rzg3s_sysc_config_func(host->sysc, RZG3S_SYSC_FUNC_ID_LINK_MASTER,
> > +                                  num_lanes == 2 ?
> > +                                  RZG3S_SYSC_LINK_MODE_DUAL_X2 :
> > +                                  RZG3S_SYSC_LINK_MODE_SINGLE_X4);
> > +     if (!ret) {
>
> I personally would prefer:
>
>         if (ret)
>                 return ret;
>
Sure, I'll update it as above and send a new version.

Cheers,
Prabhakar