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

[Claudiu Beznea]

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;



> +		rzv2h_num_total_lanes += num_lanes;
> +		host->num_lanes = num_lanes;
> +	}
> +
> +	return ret;

And return 0 here. Up to you.

w/ or w/o that addressed:

Reviewed-by: Claudiu Beznea <claudiu.beznea.uj@xxxxxxxxxxxxxx>

Tested it on RZ/G3S:

Tested-by: Claudiu Beznea <claudiu.beznea.uj@xxxxxxxxxxxxxx>

Thank you,
Claudiu