From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Add support for the RZ/V2H(P) SoC PCIe controller to the rzg3s-host
driver.

The RZ/V2H(P) SoC features two independent PCIe channels that share
physical lanes. The hardware supports two configuration modes: single
x4 mode where one controller uses all four lanes, or dual x2 mode
where both controllers use two lanes each.

Introduce configure_lanes() function pointer to configure the PCIe
lanes based on the number of channels enabled. Implement
rzv2h_pcie_configure_lanes() to detect the active PCIe channels at
boot time and program the lane mode via the system controller using
the new RZG3S_SYSC_FUNC_ID_LINK_MASTER function ID.

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
 drivers/pci/controller/pcie-rzg3s-host.c | 142 +++++++++++++++++++++++
 1 file changed, 142 insertions(+)

diff --git a/drivers/pci/controller/pcie-rzg3s-host.c b/drivers/pci/controller/pcie-rzg3s-host.c
index a629e861bbd0..d1bf1e750d9b 100644
--- a/drivers/pci/controller/pcie-rzg3s-host.c
+++ b/drivers/pci/controller/pcie-rzg3s-host.c
@@ -179,6 +179,16 @@
 /* Timeouts experimentally determined */
 #define RZG3S_REQ_ISSUE_TIMEOUT_US		2500
 
+/**
+ * enum rzg3s_sysc_link_mode - PCIe link configuration modes
+ * @RZG3S_SYSC_LINK_MODE_SINGLE_X4: Single port with x4 lanes
+ * @RZG3S_SYSC_LINK_MODE_DUAL_X2: Dual ports with x2 lanes each
+ */
+enum rzg3s_sysc_link_mode {
+	RZG3S_SYSC_LINK_MODE_SINGLE_X4 = 1,
+	RZG3S_SYSC_LINK_MODE_DUAL_X2 = 3,
+};
+
 /**
  * struct rzg3s_sysc_function - System Controller function descriptor
  * @offset: Register offset from the System Controller base address
@@ -194,12 +204,14 @@ struct rzg3s_sysc_function {
  * @RZG3S_SYSC_FUNC_ID_RST_RSM_B: RST_RSM_B SYSC function ID
  * @RZG3S_SYSC_FUNC_ID_L1_ALLOW: L1 allow SYSC function ID
  * @RZG3S_SYSC_FUNC_ID_MODE: Mode SYSC function ID
+ * @RZG3S_SYSC_FUNC_ID_LINK_MASTER: Link master SYSC function ID
  * @RZG3S_SYSC_FUNC_ID_MAX: Max SYSC function ID
  */
 enum rzg3s_sysc_func_id {
 	RZG3S_SYSC_FUNC_ID_RST_RSM_B,
 	RZG3S_SYSC_FUNC_ID_L1_ALLOW,
 	RZG3S_SYSC_FUNC_ID_MODE,
+	RZG3S_SYSC_FUNC_ID_LINK_MASTER,
 	RZG3S_SYSC_FUNC_ID_MAX,
 };
 
@@ -261,6 +273,7 @@ struct rzg3s_pcie_host;
  * @config_pre_init: Optional callback for SoC-specific pre-configuration
  * @config_post_init: Callback for SoC-specific post-configuration
  * @config_deinit: Callback for SoC-specific de-initialization
+ * @setup_lanes: Callback for setting up the number of lanes
  * @power_resets: array with the resets that need to be de-asserted after
  *                power-on
  * @cfg_resets: array with the resets that need to be de-asserted after
@@ -268,17 +281,20 @@ struct rzg3s_pcie_host;
  * @sysc_info: System Controller info for each PCIe channel
  * @num_power_resets: number of power resets
  * @num_cfg_resets: number of configuration resets
+ * @num_channels: number of PCIe channels
  */
 struct rzg3s_pcie_soc_data {
 	int (*init_phy)(struct rzg3s_pcie_host *host);
 	void (*config_pre_init)(struct rzg3s_pcie_host *host);
 	int (*config_post_init)(struct rzg3s_pcie_host *host);
 	int (*config_deinit)(struct rzg3s_pcie_host *host);
+	int (*setup_lanes)(struct rzg3s_pcie_host *host);
 	const char * const *power_resets;
 	const char * const *cfg_resets;
 	struct rzg3s_sysc_info sysc_info[RZG3S_PCIE_CHANNEL_ID_MAX];
 	u8 num_power_resets;
 	u8 num_cfg_resets;
+	u8 num_channels;
 };
 
 /**
@@ -309,6 +325,7 @@ struct rzg3s_pcie_port {
  * @intx_irqs: INTx interrupts
  * @max_link_speed: maximum supported link speed
  * @channel_id: PCIe channel identifier, used for System Controller access
+ * @num_lanes: The number of lanes
  */
 struct rzg3s_pcie_host {
 	void __iomem *axi;
@@ -325,6 +342,7 @@ struct rzg3s_pcie_host {
 	int intx_irqs[PCI_NUM_INTX];
 	int max_link_speed;
 	enum rzg3s_pcie_channel_id channel_id;
+	u8 num_lanes;
 };
 
 #define rzg3s_msi_to_host(_msi)	container_of(_msi, struct rzg3s_pcie_host, msi)
@@ -1155,6 +1173,13 @@ static int rzg3s_pcie_config_init(struct rzg3s_pcie_host *host)
 	rzg3s_pcie_update_bits(host->pcie, PCI_CLASS_REVISION, mask,
 			       field_prep(mask, PCI_CLASS_BRIDGE_PCI_NORMAL));
 
+	if (host->num_lanes) {
+		rzg3s_pcie_update_bits(host->pcie + RZG3S_PCI_CFG_PCIEC,
+				       PCI_EXP_LNKCAP, PCI_EXP_LNKCAP_MLW,
+				       FIELD_PREP(PCI_EXP_LNKCAP_MLW,
+						  host->num_lanes));
+	}
+
 	/* Disable access control to the CFGU */
 	writel_relaxed(0, host->axi + RZG3S_PCI_PERM);
 
@@ -1687,6 +1712,63 @@ rzg3s_pcie_host_setup(struct rzg3s_pcie_host *host,
 	return ret;
 }
 
+static int rzg3s_pcie_get_controller_id(struct rzg3s_pcie_host *host)
+{
+	struct device_node *np = host->dev->of_node;
+	u32 domain;
+	int ret;
+
+	if (host->data->num_channels == 1)
+		return 0;
+
+	ret = of_property_read_u32(np, "linux,pci-domain", &domain);
+	if (ret)
+		return ret;
+
+	if (domain >= host->data->num_channels)
+		return -EINVAL;
+
+	host->channel_id = domain;
+
+	return 0;
+}
+
+static int rzv2h_pcie_setup_lanes(struct rzg3s_pcie_host *host)
+{
+	struct device_node *np = host->dev->of_node;
+	static u8 rzv2h_num_total_lanes;
+	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.
+	 * Dual x2 mode is only supported with 2 total lanes. Validate
+	 * the configuration to avoid conflicts with other host, if any.
+	 */
+	if (num_lanes != 4 && num_lanes != 2)
+		return -EINVAL;
+
+	if (rzv2h_num_total_lanes == 2 && num_lanes != 2)
+		return -EINVAL;
+
+	if (rzv2h_num_total_lanes == 4)
+		return -EINVAL;
+
+	rzv2h_num_total_lanes += num_lanes;
+
+	host->num_lanes = num_lanes;
+
+	return 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);
+}
+
 static int rzg3s_pcie_probe(struct platform_device *pdev)
 {
 	struct pci_host_bridge *bridge;
@@ -1711,6 +1793,10 @@ static int rzg3s_pcie_probe(struct platform_device *pdev)
 	if (!host->sysc)
 		return -ENOMEM;
 
+	ret = rzg3s_pcie_get_controller_id(host);
+	if (ret)
+		return ret;
+
 	sysc = host->sysc;
 	sysc->info = &host->data->sysc_info[host->channel_id];
 
@@ -1740,6 +1826,12 @@ static int rzg3s_pcie_probe(struct platform_device *pdev)
 	if (ret)
 		goto port_refclk_put;
 
+	if (host->data->setup_lanes) {
+		ret = host->data->setup_lanes(host);
+		if (ret)
+			goto sysc_signal_restore;
+	}
+
 	ret = rzg3s_pcie_resets_prepare_and_get(host);
 	if (ret)
 		goto sysc_signal_restore;
@@ -1901,6 +1993,7 @@ static const struct rzg3s_pcie_soc_data rzg3s_soc_data = {
 	.num_power_resets = ARRAY_SIZE(rzg3s_soc_power_resets),
 	.cfg_resets = rzg3s_soc_cfg_resets,
 	.num_cfg_resets = ARRAY_SIZE(rzg3s_soc_cfg_resets),
+	.num_channels = 1,
 	.config_post_init = rzg3s_pcie_config_post_init,
 	.config_deinit = rzg3s_pcie_config_deinit,
 	.init_phy = rzg3s_soc_pcie_init_phy,
@@ -1921,6 +2014,7 @@ static const char * const rzg3e_soc_power_resets[] = { "aresetn" };
 static const struct rzg3s_pcie_soc_data rzg3e_soc_data = {
 	.power_resets = rzg3e_soc_power_resets,
 	.num_power_resets = ARRAY_SIZE(rzg3e_soc_power_resets),
+	.num_channels = 1,
 	.config_pre_init = rzg3e_pcie_config_pre_init,
 	.config_post_init = rzg3e_pcie_config_post_init,
 	.config_deinit = rzg3e_pcie_config_deinit,
@@ -1940,6 +2034,50 @@ static const struct rzg3s_pcie_soc_data rzg3e_soc_data = {
 	},
 };
 
+static const struct rzg3s_pcie_soc_data rzv2h_soc_data = {
+	.power_resets = rzg3e_soc_power_resets,
+	.num_power_resets = ARRAY_SIZE(rzg3e_soc_power_resets),
+	.num_channels = 2,
+	.config_pre_init = rzg3e_pcie_config_pre_init,
+	.config_post_init = rzg3e_pcie_config_post_init,
+	.config_deinit = rzg3e_pcie_config_deinit,
+	.setup_lanes = rzv2h_pcie_setup_lanes,
+	.sysc_info = {
+		[RZG3S_PCIE_CHANNEL_ID_0] = {
+			.functions = {
+				[RZG3S_SYSC_FUNC_ID_L1_ALLOW] = {
+					.offset = 0x1020,
+					.mask = BIT(0),
+				},
+				[RZG3S_SYSC_FUNC_ID_MODE] = {
+					.offset = 0x1024,
+					.mask = BIT(0),
+				},
+				[RZG3S_SYSC_FUNC_ID_LINK_MASTER] = {
+					.offset = 0x1060,
+					.mask = GENMASK(9, 8),
+				},
+			},
+		},
+		[RZG3S_PCIE_CHANNEL_ID_1] = {
+			.functions = {
+				[RZG3S_SYSC_FUNC_ID_L1_ALLOW] = {
+					.offset = 0x1050,
+					.mask = BIT(0),
+				},
+				[RZG3S_SYSC_FUNC_ID_MODE] = {
+					.offset = 0x1054,
+					.mask = BIT(0),
+				},
+				[RZG3S_SYSC_FUNC_ID_LINK_MASTER] = {
+					.offset = 0x1060,
+					.mask = GENMASK(9, 8),
+				},
+			},
+		},
+	},
+};
+
 static const struct of_device_id rzg3s_pcie_of_match[] = {
 	{
 		.compatible = "renesas,r9a08g045-pcie",
@@ -1949,6 +2087,10 @@ static const struct of_device_id rzg3s_pcie_of_match[] = {
 		.compatible = "renesas,r9a09g047-pcie",
 		.data = &rzg3e_soc_data,
 	},
+	{
+		.compatible = "renesas,r9a09g057-pcie",
+		.data = &rzv2h_soc_data,
+	},
 	{}
 };
 
-- 
2.53.0

Hi, Prabhakar,

On 3/18/26 14:44, Prabhakar wrote:
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> 
> Add support for the RZ/V2H(P) SoC PCIe controller to the rzg3s-host
> driver.
> 
> The RZ/V2H(P) SoC features two independent PCIe channels that share
> physical lanes. The hardware supports two configuration modes: single
> x4 mode where one controller uses all four lanes, or dual x2 mode
> where both controllers use two lanes each.
> 
> Introduce configure_lanes() function pointer to configure the PCIe
> lanes based on the number of channels enabled. Implement
> rzv2h_pcie_configure_lanes() to detect the active PCIe channels at
> boot time and program the lane mode via the system controller using
> the new RZG3S_SYSC_FUNC_ID_LINK_MASTER function ID.
> 
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> ---
>   drivers/pci/controller/pcie-rzg3s-host.c | 142 +++++++++++++++++++++++
>   1 file changed, 142 insertions(+)
> 
> diff --git a/drivers/pci/controller/pcie-rzg3s-host.c b/drivers/pci/controller/pcie-rzg3s-host.c
> index a629e861bbd0..d1bf1e750d9b 100644
> --- a/drivers/pci/controller/pcie-rzg3s-host.c
> +++ b/drivers/pci/controller/pcie-rzg3s-host.c
> @@ -179,6 +179,16 @@
>   /* Timeouts experimentally determined */
>   #define RZG3S_REQ_ISSUE_TIMEOUT_US		2500
>   
> +/**
> + * enum rzg3s_sysc_link_mode - PCIe link configuration modes
> + * @RZG3S_SYSC_LINK_MODE_SINGLE_X4: Single port with x4 lanes
> + * @RZG3S_SYSC_LINK_MODE_DUAL_X2: Dual ports with x2 lanes each
> + */
> +enum rzg3s_sysc_link_mode {
> +	RZG3S_SYSC_LINK_MODE_SINGLE_X4 = 1,
> +	RZG3S_SYSC_LINK_MODE_DUAL_X2 = 3,
> +};
> +
>   /**
>    * struct rzg3s_sysc_function - System Controller function descriptor
>    * @offset: Register offset from the System Controller base address
> @@ -194,12 +204,14 @@ struct rzg3s_sysc_function {
>    * @RZG3S_SYSC_FUNC_ID_RST_RSM_B: RST_RSM_B SYSC function ID
>    * @RZG3S_SYSC_FUNC_ID_L1_ALLOW: L1 allow SYSC function ID
>    * @RZG3S_SYSC_FUNC_ID_MODE: Mode SYSC function ID
> + * @RZG3S_SYSC_FUNC_ID_LINK_MASTER: Link master SYSC function ID
>    * @RZG3S_SYSC_FUNC_ID_MAX: Max SYSC function ID
>    */
>   enum rzg3s_sysc_func_id {
>   	RZG3S_SYSC_FUNC_ID_RST_RSM_B,
>   	RZG3S_SYSC_FUNC_ID_L1_ALLOW,
>   	RZG3S_SYSC_FUNC_ID_MODE,
> +	RZG3S_SYSC_FUNC_ID_LINK_MASTER,
>   	RZG3S_SYSC_FUNC_ID_MAX,
>   };
>   
> @@ -261,6 +273,7 @@ struct rzg3s_pcie_host;
>    * @config_pre_init: Optional callback for SoC-specific pre-configuration
>    * @config_post_init: Callback for SoC-specific post-configuration
>    * @config_deinit: Callback for SoC-specific de-initialization
> + * @setup_lanes: Callback for setting up the number of lanes
>    * @power_resets: array with the resets that need to be de-asserted after
>    *                power-on
>    * @cfg_resets: array with the resets that need to be de-asserted after
> @@ -268,17 +281,20 @@ struct rzg3s_pcie_host;
>    * @sysc_info: System Controller info for each PCIe channel
>    * @num_power_resets: number of power resets
>    * @num_cfg_resets: number of configuration resets
> + * @num_channels: number of PCIe channels
>    */
>   struct rzg3s_pcie_soc_data {
>   	int (*init_phy)(struct rzg3s_pcie_host *host);
>   	void (*config_pre_init)(struct rzg3s_pcie_host *host);
>   	int (*config_post_init)(struct rzg3s_pcie_host *host);
>   	int (*config_deinit)(struct rzg3s_pcie_host *host);
> +	int (*setup_lanes)(struct rzg3s_pcie_host *host);
>   	const char * const *power_resets;
>   	const char * const *cfg_resets;
>   	struct rzg3s_sysc_info sysc_info[RZG3S_PCIE_CHANNEL_ID_MAX];
>   	u8 num_power_resets;
>   	u8 num_cfg_resets;
> +	u8 num_channels;
>   };
>   
>   /**
> @@ -309,6 +325,7 @@ struct rzg3s_pcie_port {
>    * @intx_irqs: INTx interrupts
>    * @max_link_speed: maximum supported link speed
>    * @channel_id: PCIe channel identifier, used for System Controller access
> + * @num_lanes: The number of lanes
>    */
>   struct rzg3s_pcie_host {
>   	void __iomem *axi;
> @@ -325,6 +342,7 @@ struct rzg3s_pcie_host {
>   	int intx_irqs[PCI_NUM_INTX];
>   	int max_link_speed;
>   	enum rzg3s_pcie_channel_id channel_id;
> +	u8 num_lanes;
>   };
>   
>   #define rzg3s_msi_to_host(_msi)	container_of(_msi, struct rzg3s_pcie_host, msi)
> @@ -1155,6 +1173,13 @@ static int rzg3s_pcie_config_init(struct rzg3s_pcie_host *host)
>   	rzg3s_pcie_update_bits(host->pcie, PCI_CLASS_REVISION, mask,
>   			       field_prep(mask, PCI_CLASS_BRIDGE_PCI_NORMAL));
>   
> +	if (host->num_lanes) {
> +		rzg3s_pcie_update_bits(host->pcie + RZG3S_PCI_CFG_PCIEC,
> +				       PCI_EXP_LNKCAP, PCI_EXP_LNKCAP_MLW,
> +				       FIELD_PREP(PCI_EXP_LNKCAP_MLW,
> +						  host->num_lanes));
> +	}
> +
>   	/* Disable access control to the CFGU */
>   	writel_relaxed(0, host->axi + RZG3S_PCI_PERM);
>   
> @@ -1687,6 +1712,63 @@ rzg3s_pcie_host_setup(struct rzg3s_pcie_host *host,
>   	return ret;
>   }
>   
> +static int rzg3s_pcie_get_controller_id(struct rzg3s_pcie_host *host)
> +{
> +	struct device_node *np = host->dev->of_node;
> +	u32 domain;
> +	int ret;
> +
> +	if (host->data->num_channels == 1)
> +		return 0;
> +
> +	ret = of_property_read_u32(np, "linux,pci-domain", &domain);

This introduces some limits in the systems with RZ/V2H(P) SoCs with regards to 
the usage of linux,pci-domain. I would like the PCIe maintainers take on this.

As this is necessary to index in the system controller driver specific data (as 
there are different SYSC offsets for different PCIe controllers) I see the 
following alternatives, if any:

1/ add a dedicated DT property for this, e.g. renesas,pcie-controller-id
2/ Add dedicated DT bindings for RZ/V2H(P) SoC that would be used to specify the
    system controller register offset and mask for different functionalities.

    E.g.:
    renesas,sysc-l1-allow = <&sysc 0x1020 0x1>;
    renesas,sysc-mode = <&sysc 0x1024 0x1>;
    renesas,sysc-link-master = <&sysc 0x1060 0x300>;

    And use them in each controller DT node. E.g.:

    pcie0: pcie@add1 {
        // ...

        renesas,sysc-l1-allow = <&sysc 0x1020 0x1>;
        renesas,sysc-mode = <&sysc 0x1024 0x1>;
        renesas,sysc-link-master = <&sysc 0x1060 0x300>;

        // ...
    };

    pcie0: pcie@add1 {
        // ...

        renesas,sysc-l1-allow = <&sysc 0x1050 0x1>;
        renesas,sysc-mode = <&sysc 0x1054 0x1>;
        renesas,sysc-link-master = <&sysc 0x1060 0x300>;

        // ...
    };

3/ as sashiko.dev mentions [1], using aliases for the PCIe nodes should also be
    what you need here.

[1] 
https://sashiko.dev/#/patchset/20260318124450.163471-1-prabhakar.mahadev-lad.rj%40bp.renesas.com

> +	if (ret)
> +		return ret;
> +
> +	if (domain >= host->data->num_channels)
> +		return -EINVAL;
> +
> +	host->channel_id = domain;
> +
> +	return 0;
> +}
> +
> +static int rzv2h_pcie_setup_lanes(struct rzg3s_pcie_host *host)
> +{
> +	struct device_node *np = host->dev->of_node;
> +	static u8 rzv2h_num_total_lanes;
> +	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.
> +	 * Dual x2 mode is only supported with 2 total lanes. Validate
> +	 * the configuration to avoid conflicts with other host, if any.
> +	 */
> +	if (num_lanes != 4 && num_lanes != 2)
> +		return -EINVAL;
> +
> +	if (rzv2h_num_total_lanes == 2 && num_lanes != 2)
> +		return -EINVAL;
> +
> +	if (rzv2h_num_total_lanes == 4)
> +		return -EINVAL;
> +
> +	rzv2h_num_total_lanes += num_lanes;

There is a a valid concern raised by sashiko.dev [1] with regards to 
incrementing this if later the probe fails:

from [1]:
"For example, if rzg3s_pcie_resets_prepare_and_get() returns -EPROBE_DEFER,
the static variable is never decremented. On subsequent probe retries,
the variable will be artificially inflated, eventually causing the bounds
check to fail and returning a permanent -EINVAL. This would also prevent
driver unbind and rebind from working correctly."

also:

"Additionally, since the driver sets .probe_type = PROBE_PREFER_ASYNCHRONOUS,
could multiple PCIe controllers probing concurrently cause a data race when
reading and modifying this static variable without locking?"

> +
> +	host->num_lanes = num_lanes;
> +
> +	return 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);

I think this one should also be configured on resume (to have the same 
configuration sequence as in probe) even though RZ/V2H(P) don't currently 
support s2ram. E.g. so something like:

if (host->num_lanes) {
	ret = rzg3s_sysc_config_func(host->sysc,
				     RZG3S_SYSC_FUNC_ID_LINK_MASTER,
				     host->num_lanes == 2  ?
				     RZG3S_SYSC_LINK_MODE_DUAL_X2 :
				     RZG3S_SYSC_LINK_MODE_SINGLE_X4);
	if (ret)
		goto assert_rst_rsm_b;
}

after ret = rzg3s_sysc_config_func(sysc, RZG3S_SYSC_FUNC_ID_RST_RSM_B, 1);

Thank you,
Claudiu

Hi Claudiu,

Thank you for the review.

On Wed, Mar 25, 2026 at 10:18 AM Claudiu Beznea
<claudiu.beznea@tuxon.dev> wrote:
>
> Hi, Prabhakar,
>
> On 3/18/26 14:44, Prabhakar wrote:
> > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> >
> > Add support for the RZ/V2H(P) SoC PCIe controller to the rzg3s-host
> > driver.
> >
> > The RZ/V2H(P) SoC features two independent PCIe channels that share
> > physical lanes. The hardware supports two configuration modes: single
> > x4 mode where one controller uses all four lanes, or dual x2 mode
> > where both controllers use two lanes each.
> >
> > Introduce configure_lanes() function pointer to configure the PCIe
> > lanes based on the number of channels enabled. Implement
> > rzv2h_pcie_configure_lanes() to detect the active PCIe channels at
> > boot time and program the lane mode via the system controller using
> > the new RZG3S_SYSC_FUNC_ID_LINK_MASTER function ID.
> >
> > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > ---
> >   drivers/pci/controller/pcie-rzg3s-host.c | 142 +++++++++++++++++++++++
> >   1 file changed, 142 insertions(+)
> >
> > diff --git a/drivers/pci/controller/pcie-rzg3s-host.c b/drivers/pci/controller/pcie-rzg3s-host.c
> > index a629e861bbd0..d1bf1e750d9b 100644
> > --- a/drivers/pci/controller/pcie-rzg3s-host.c
> > +++ b/drivers/pci/controller/pcie-rzg3s-host.c
> > @@ -179,6 +179,16 @@
> >   /* Timeouts experimentally determined */
> >   #define RZG3S_REQ_ISSUE_TIMEOUT_US          2500
> >
> > +/**
> > + * enum rzg3s_sysc_link_mode - PCIe link configuration modes
> > + * @RZG3S_SYSC_LINK_MODE_SINGLE_X4: Single port with x4 lanes
> > + * @RZG3S_SYSC_LINK_MODE_DUAL_X2: Dual ports with x2 lanes each
> > + */
> > +enum rzg3s_sysc_link_mode {
> > +     RZG3S_SYSC_LINK_MODE_SINGLE_X4 = 1,
> > +     RZG3S_SYSC_LINK_MODE_DUAL_X2 = 3,
> > +};
> > +
> >   /**
> >    * struct rzg3s_sysc_function - System Controller function descriptor
> >    * @offset: Register offset from the System Controller base address
> > @@ -194,12 +204,14 @@ struct rzg3s_sysc_function {
> >    * @RZG3S_SYSC_FUNC_ID_RST_RSM_B: RST_RSM_B SYSC function ID
> >    * @RZG3S_SYSC_FUNC_ID_L1_ALLOW: L1 allow SYSC function ID
> >    * @RZG3S_SYSC_FUNC_ID_MODE: Mode SYSC function ID
> > + * @RZG3S_SYSC_FUNC_ID_LINK_MASTER: Link master SYSC function ID
> >    * @RZG3S_SYSC_FUNC_ID_MAX: Max SYSC function ID
> >    */
> >   enum rzg3s_sysc_func_id {
> >       RZG3S_SYSC_FUNC_ID_RST_RSM_B,
> >       RZG3S_SYSC_FUNC_ID_L1_ALLOW,
> >       RZG3S_SYSC_FUNC_ID_MODE,
> > +     RZG3S_SYSC_FUNC_ID_LINK_MASTER,
> >       RZG3S_SYSC_FUNC_ID_MAX,
> >   };
> >
> > @@ -261,6 +273,7 @@ struct rzg3s_pcie_host;
> >    * @config_pre_init: Optional callback for SoC-specific pre-configuration
> >    * @config_post_init: Callback for SoC-specific post-configuration
> >    * @config_deinit: Callback for SoC-specific de-initialization
> > + * @setup_lanes: Callback for setting up the number of lanes
> >    * @power_resets: array with the resets that need to be de-asserted after
> >    *                power-on
> >    * @cfg_resets: array with the resets that need to be de-asserted after
> > @@ -268,17 +281,20 @@ struct rzg3s_pcie_host;
> >    * @sysc_info: System Controller info for each PCIe channel
> >    * @num_power_resets: number of power resets
> >    * @num_cfg_resets: number of configuration resets
> > + * @num_channels: number of PCIe channels
> >    */
> >   struct rzg3s_pcie_soc_data {
> >       int (*init_phy)(struct rzg3s_pcie_host *host);
> >       void (*config_pre_init)(struct rzg3s_pcie_host *host);
> >       int (*config_post_init)(struct rzg3s_pcie_host *host);
> >       int (*config_deinit)(struct rzg3s_pcie_host *host);
> > +     int (*setup_lanes)(struct rzg3s_pcie_host *host);
> >       const char * const *power_resets;
> >       const char * const *cfg_resets;
> >       struct rzg3s_sysc_info sysc_info[RZG3S_PCIE_CHANNEL_ID_MAX];
> >       u8 num_power_resets;
> >       u8 num_cfg_resets;
> > +     u8 num_channels;
> >   };
> >
> >   /**
> > @@ -309,6 +325,7 @@ struct rzg3s_pcie_port {
> >    * @intx_irqs: INTx interrupts
> >    * @max_link_speed: maximum supported link speed
> >    * @channel_id: PCIe channel identifier, used for System Controller access
> > + * @num_lanes: The number of lanes
> >    */
> >   struct rzg3s_pcie_host {
> >       void __iomem *axi;
> > @@ -325,6 +342,7 @@ struct rzg3s_pcie_host {
> >       int intx_irqs[PCI_NUM_INTX];
> >       int max_link_speed;
> >       enum rzg3s_pcie_channel_id channel_id;
> > +     u8 num_lanes;
> >   };
> >
> >   #define rzg3s_msi_to_host(_msi)     container_of(_msi, struct rzg3s_pcie_host, msi)
> > @@ -1155,6 +1173,13 @@ static int rzg3s_pcie_config_init(struct rzg3s_pcie_host *host)
> >       rzg3s_pcie_update_bits(host->pcie, PCI_CLASS_REVISION, mask,
> >                              field_prep(mask, PCI_CLASS_BRIDGE_PCI_NORMAL));
> >
> > +     if (host->num_lanes) {
> > +             rzg3s_pcie_update_bits(host->pcie + RZG3S_PCI_CFG_PCIEC,
> > +                                    PCI_EXP_LNKCAP, PCI_EXP_LNKCAP_MLW,
> > +                                    FIELD_PREP(PCI_EXP_LNKCAP_MLW,
> > +                                               host->num_lanes));
> > +     }
> > +
> >       /* Disable access control to the CFGU */
> >       writel_relaxed(0, host->axi + RZG3S_PCI_PERM);
> >
> > @@ -1687,6 +1712,63 @@ rzg3s_pcie_host_setup(struct rzg3s_pcie_host *host,
> >       return ret;
> >   }
> >
> > +static int rzg3s_pcie_get_controller_id(struct rzg3s_pcie_host *host)
> > +{
> > +     struct device_node *np = host->dev->of_node;
> > +     u32 domain;
> > +     int ret;
> > +
> > +     if (host->data->num_channels == 1)
> > +             return 0;
> > +
> > +     ret = of_property_read_u32(np, "linux,pci-domain", &domain);
>
> This introduces some limits in the systems with RZ/V2H(P) SoCs with regards to
> the usage of linux,pci-domain. I would like the PCIe maintainers take on this.
>
+ DT maintainers too.

> As this is necessary to index in the system controller driver specific data (as
> there are different SYSC offsets for different PCIe controllers) I see the
> following alternatives, if any:
>
> 1/ add a dedicated DT property for this, e.g. renesas,pcie-controller-id
> 2/ Add dedicated DT bindings for RZ/V2H(P) SoC that would be used to specify the
>     system controller register offset and mask for different functionalities.
>
>     E.g.:
>     renesas,sysc-l1-allow = <&sysc 0x1020 0x1>;
>     renesas,sysc-mode = <&sysc 0x1024 0x1>;
>     renesas,sysc-link-master = <&sysc 0x1060 0x300>;
>
>     And use them in each controller DT node. E.g.:
>
>     pcie0: pcie@add1 {
>         // ...
>
>         renesas,sysc-l1-allow = <&sysc 0x1020 0x1>;
>         renesas,sysc-mode = <&sysc 0x1024 0x1>;
>         renesas,sysc-link-master = <&sysc 0x1060 0x300>;
>
>         // ...
>     };
>
>     pcie0: pcie@add1 {
>         // ...
>
>         renesas,sysc-l1-allow = <&sysc 0x1050 0x1>;
>         renesas,sysc-mode = <&sysc 0x1054 0x1>;
>         renesas,sysc-link-master = <&sysc 0x1060 0x300>;
>
>         // ...
>     };
>
The current approach is being used to align with the existing driver
design and to reuse the already available DT property, rather than
introducing a new one.

Regarding option #2, I don’t see this as a scalable solution. For
every new register, we would need to introduce a separate DT property,
which would quickly become unwieldy and harder to maintain.

Im ok, with option #1 or any other suggestion based on feedback of
PCIe and DT maintainers.

> 3/ as sashiko.dev mentions [1], using aliases for the PCIe nodes should also be
>     what you need here.
>
> [1]
> https://sashiko.dev/#/patchset/20260318124450.163471-1-prabhakar.mahadev-lad.rj%40bp.renesas.com
>
> > +     if (ret)
> > +             return ret;
> > +
> > +     if (domain >= host->data->num_channels)
> > +             return -EINVAL;
> > +
> > +     host->channel_id = domain;
> > +
> > +     return 0;
> > +}
> > +
> > +static int rzv2h_pcie_setup_lanes(struct rzg3s_pcie_host *host)
> > +{
> > +     struct device_node *np = host->dev->of_node;
> > +     static u8 rzv2h_num_total_lanes;
> > +     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.
> > +      * Dual x2 mode is only supported with 2 total lanes. Validate
> > +      * the configuration to avoid conflicts with other host, if any.
> > +      */
> > +     if (num_lanes != 4 && num_lanes != 2)
> > +             return -EINVAL;
> > +
> > +     if (rzv2h_num_total_lanes == 2 && num_lanes != 2)
> > +             return -EINVAL;
> > +
> > +     if (rzv2h_num_total_lanes == 4)
> > +             return -EINVAL;
> > +
> > +     rzv2h_num_total_lanes += num_lanes;
>
> There is a a valid concern raised by sashiko.dev [1] with regards to
> incrementing this if later the probe fails:
>
> from [1]:
> "For example, if rzg3s_pcie_resets_prepare_and_get() returns -EPROBE_DEFER,
> the static variable is never decremented. On subsequent probe retries,
> the variable will be artificially inflated, eventually causing the bounds
> check to fail and returning a permanent -EINVAL. This would also prevent
> driver unbind and rebind from working correctly."
>
The other alternative would be the below, where we wouldn't need to
use the num-lanes property but would need a comparison with the DT
compatible,

+       for_each_compatible_node(np, NULL, "renesas,r9a09g057-pcie") {
+               if (of_device_is_available(np))
+                       count++;
+       }
+       if (!count)
+               return 0;
+
+       /* If both PCIe channels are enabled configure the LINK_MASTER
in x2 lane mode.
+        * If only one channel is enabled check the port index and if
port1 is enabled
+        * configure the LINK_MASTER in x2 lane mode, otherwise keep
it in x4 lane mode.
+        */
+       if (count == RZV2H_MAX_PCIE_PORTS ||
+           (count == 1 && host->channel == 1))
+               host->link_mode = RZV2H_PCIE_MODE_DUAL_X2;
+       else
+               host->link_mode = RZV2H_PCIE_MODE_SINGLE_X4;

> also:
>
> "Additionally, since the driver sets .probe_type = PROBE_PREFER_ASYNCHRONOUS,
> could multiple PCIe controllers probing concurrently cause a data race when
> reading and modifying this static variable without locking?"
>
> > +
> > +     host->num_lanes = num_lanes;
> > +
> > +     return 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);
>
> I think this one should also be configured on resume (to have the same
> configuration sequence as in probe) even though RZ/V2H(P) don't currently
> support s2ram. E.g. so something like:
>
> if (host->num_lanes) {
>         ret = rzg3s_sysc_config_func(host->sysc,
>                                      RZG3S_SYSC_FUNC_ID_LINK_MASTER,
>                                      host->num_lanes == 2  ?
>                                      RZG3S_SYSC_LINK_MODE_DUAL_X2 :
>                                      RZG3S_SYSC_LINK_MODE_SINGLE_X4);
>         if (ret)
>                 goto assert_rst_rsm_b;
> }
>
> after ret = rzg3s_sysc_config_func(sysc, RZG3S_SYSC_FUNC_ID_RST_RSM_B, 1);
>
Ok.

Cheers,
Prabhakar

Hi, Prabhakar,

On 3/25/26 13:53, Lad, Prabhakar wrote:
>> from [1]:
>> "For example, if rzg3s_pcie_resets_prepare_and_get() returns -EPROBE_DEFER,
>> the static variable is never decremented. On subsequent probe retries,
>> the variable will be artificially inflated, eventually causing the bounds
>> check to fail and returning a permanent -EINVAL. This would also prevent
>> driver unbind and rebind from working correctly."
>>
> The other alternative would be the below, where we wouldn't need to
> use the num-lanes property but would need a comparison with the DT
> compatible,

Or move rzv2h_num_total_lanes outside of rzv2h_pcie_setup_lanes() and reset it 
on failure path.

> 
> +       for_each_compatible_node(np, NULL, "renesas,r9a09g057-pcie") {

If it's possible I would avoid spreading compatibles though the file but instead 
use driver data where possible.

Thank you,
Claudiu

> +               if (of_device_is_available(np))
> +                       count++;
> +       }
> +       if (!count)
> +               return 0;
> +
> +       /* If both PCIe channels are enabled configure the LINK_MASTER
> in x2 lane mode.
> +        * If only one channel is enabled check the port index and if
> port1 is enabled
> +        * configure the LINK_MASTER in x2 lane mode, otherwise keep
> it in x4 lane mode.
> +        */
> +       if (count == RZV2H_MAX_PCIE_PORTS ||
> +           (count == 1 && host->channel == 1))
> +               host->link_mode = RZV2H_PCIE_MODE_DUAL_X2;
> +       else
> +               host->link_mode = RZV2H_PCIE_MODE_SINGLE_X4;