* Re: [PATCH v6 1/1] PCI: of: Remove max-link-speed generation validation
2025-12-18 13:20 [PATCH v6 1/1] PCI: of: Remove max-link-speed generation validation Hans Zhang
@ 2026-01-18 13:45 ` Hans Zhang
2026-03-06 15:47 ` Hans Zhang
2026-03-06 17:08 ` Bjorn Helgaas
2 siblings, 0 replies; 5+ messages in thread
From: Hans Zhang @ 2026-01-18 13:45 UTC (permalink / raw)
To: bhelgaas, helgaas; +Cc: mani, ilpo.jarvinen, linux-pci, linux-kernel
Hi,
Gentle ping.
Best regards,
Hans
On 2025/12/18 21:20, Hans Zhang wrote:
> The current implementation of of_pci_get_max_link_speed() validates
> max-link-speed property values to be in the range 1~4 (Gen1~Gen4).
> However, this creates maintenance overhead as each new PCIe generation
> requires updating this validation logic.
>
> Since device tree binding validation already enforces the allowed
> values through the schema, and the callers of this function perform
> their own validation checks, this intermediate validation becomes
> redundant.
>
> Furthermore, with upcoming SOCs using Synopsys/Cadence IP requiring
> Gen5/Gen6 support, removing this hardcoded check enables seamless
> support for future PCIe generations without requiring kernel updates
> for each new speed grade.
>
> Remove the max-link-speed > 4 validation check while retaining the
> property existence and non-zero check. This simplifies maintenance
> and aligns with the existing validation architecture where DT binding
> and driver-level checks provide sufficient validation.
>
> Signed-off-by: Hans Zhang <18255117159@163.com>
> Acked-by: Manivannan Sadhasivam <mani@kernel.org>
> ---
> Changes for v6:
> - It'd be good to return the actual errno as of_property_read_u32() can return
> -EINVAL, -ENODATA and -EOVERFLOW. (Mani)
>
> Changes for v5:
> https://patchwork.kernel.org/project/linux-pci/patch/20251218125909.305300-1-18255117159@163.com/
>
> - Delete the check for speed. (Mani)
>
> Changes for v4:
> https://patchwork.kernel.org/project/linux-pci/patch/20251105134701.182795-1-18255117159@163.com/
>
> - Add pcie_max_supported_link_speed.(Ilpo)
>
> Changes for v3:
> https://patchwork.kernel.org/project/linux-pci/patch/20251101164132.14145-1-18255117159@163.com/
>
> - Modify the commit message.
> - Add Reviewed-by tag.
>
> Changes for v2:
> https://patchwork.kernel.org/project/linux-pci/cover/20250529021026.475861-1-18255117159@163.com/
> - The following files have been deleted:
> Documentation/devicetree/bindings/pci/pci.txt
>
> Update to this file again:
> dtschema/schemas/pci/pci-bus-common.yaml
> ---
> drivers/pci/of.c | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/pci/of.c b/drivers/pci/of.c
> index 3579265f1198..b56fdbcb3d72 100644
> --- a/drivers/pci/of.c
> +++ b/drivers/pci/of.c
> @@ -888,10 +888,11 @@ bool of_pci_supply_present(struct device_node *np)
> int of_pci_get_max_link_speed(struct device_node *node)
> {
> u32 max_link_speed;
> + int ret;
>
> - if (of_property_read_u32(node, "max-link-speed", &max_link_speed) ||
> - max_link_speed == 0 || max_link_speed > 4)
> - return -EINVAL;
> + ret = of_property_read_u32(node, "max-link-speed", &max_link_speed);
> + if (ret)
> + return ret;
>
> return max_link_speed;
> }
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH v6 1/1] PCI: of: Remove max-link-speed generation validation
2025-12-18 13:20 [PATCH v6 1/1] PCI: of: Remove max-link-speed generation validation Hans Zhang
2026-01-18 13:45 ` Hans Zhang
@ 2026-03-06 15:47 ` Hans Zhang
2026-03-06 17:08 ` Bjorn Helgaas
2 siblings, 0 replies; 5+ messages in thread
From: Hans Zhang @ 2026-03-06 15:47 UTC (permalink / raw)
To: bhelgaas, helgaas; +Cc: mani, ilpo.jarvinen, linux-pci, linux-kernel
Hi,
Gentle ping.
Best regards,
Hans
On 2025/12/18 21:20, Hans Zhang wrote:
> The current implementation of of_pci_get_max_link_speed() validates
> max-link-speed property values to be in the range 1~4 (Gen1~Gen4).
> However, this creates maintenance overhead as each new PCIe generation
> requires updating this validation logic.
>
> Since device tree binding validation already enforces the allowed
> values through the schema, and the callers of this function perform
> their own validation checks, this intermediate validation becomes
> redundant.
>
> Furthermore, with upcoming SOCs using Synopsys/Cadence IP requiring
> Gen5/Gen6 support, removing this hardcoded check enables seamless
> support for future PCIe generations without requiring kernel updates
> for each new speed grade.
>
> Remove the max-link-speed > 4 validation check while retaining the
> property existence and non-zero check. This simplifies maintenance
> and aligns with the existing validation architecture where DT binding
> and driver-level checks provide sufficient validation.
>
> Signed-off-by: Hans Zhang <18255117159@163.com>
> Acked-by: Manivannan Sadhasivam <mani@kernel.org>
> ---
> Changes for v6:
> - It'd be good to return the actual errno as of_property_read_u32() can return
> -EINVAL, -ENODATA and -EOVERFLOW. (Mani)
>
> Changes for v5:
> https://patchwork.kernel.org/project/linux-pci/patch/20251218125909.305300-1-18255117159@163.com/
>
> - Delete the check for speed. (Mani)
>
> Changes for v4:
> https://patchwork.kernel.org/project/linux-pci/patch/20251105134701.182795-1-18255117159@163.com/
>
> - Add pcie_max_supported_link_speed.(Ilpo)
>
> Changes for v3:
> https://patchwork.kernel.org/project/linux-pci/patch/20251101164132.14145-1-18255117159@163.com/
>
> - Modify the commit message.
> - Add Reviewed-by tag.
>
> Changes for v2:
> https://patchwork.kernel.org/project/linux-pci/cover/20250529021026.475861-1-18255117159@163.com/
> - The following files have been deleted:
> Documentation/devicetree/bindings/pci/pci.txt
>
> Update to this file again:
> dtschema/schemas/pci/pci-bus-common.yaml
> ---
> drivers/pci/of.c | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/pci/of.c b/drivers/pci/of.c
> index 3579265f1198..b56fdbcb3d72 100644
> --- a/drivers/pci/of.c
> +++ b/drivers/pci/of.c
> @@ -888,10 +888,11 @@ bool of_pci_supply_present(struct device_node *np)
> int of_pci_get_max_link_speed(struct device_node *node)
> {
> u32 max_link_speed;
> + int ret;
>
> - if (of_property_read_u32(node, "max-link-speed", &max_link_speed) ||
> - max_link_speed == 0 || max_link_speed > 4)
> - return -EINVAL;
> + ret = of_property_read_u32(node, "max-link-speed", &max_link_speed);
> + if (ret)
> + return ret;
>
> return max_link_speed;
> }
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH v6 1/1] PCI: of: Remove max-link-speed generation validation
2025-12-18 13:20 [PATCH v6 1/1] PCI: of: Remove max-link-speed generation validation Hans Zhang
2026-01-18 13:45 ` Hans Zhang
2026-03-06 15:47 ` Hans Zhang
@ 2026-03-06 17:08 ` Bjorn Helgaas
2026-03-08 14:00 ` Hans Zhang
2 siblings, 1 reply; 5+ messages in thread
From: Bjorn Helgaas @ 2026-03-06 17:08 UTC (permalink / raw)
To: Hans Zhang
Cc: bhelgaas, mani, ilpo.jarvinen, linux-pci, linux-kernel,
Rob Herring
[+cc Rob]
On Thu, Dec 18, 2025 at 09:20:36PM +0800, Hans Zhang wrote:
> The current implementation of of_pci_get_max_link_speed() validates
> max-link-speed property values to be in the range 1~4 (Gen1~Gen4).
> However, this creates maintenance overhead as each new PCIe generation
> requires updating this validation logic.
>
> Since device tree binding validation already enforces the allowed
> values through the schema, and the callers of this function perform
> their own validation checks, this intermediate validation becomes
> redundant.
>
> Furthermore, with upcoming SOCs using Synopsys/Cadence IP requiring
> Gen5/Gen6 support, removing this hardcoded check enables seamless
> support for future PCIe generations without requiring kernel updates
> for each new speed grade.
The upcoming SoC info seems like too much detail for the commit log.
> Remove the max-link-speed > 4 validation check while retaining the
> property existence and non-zero check. This simplifies maintenance
> and aligns with the existing validation architecture where DT binding
> and driver-level checks provide sufficient validation.
I don't think it's legit to rely on DT binding checks. The code
should protect itself regardless of what offline binding checkers
find.
I'm not convinced that callers do their own validation. For example,
dw_pcie_get_resources() assigns of_pci_get_max_link_speed() to
pci->max_link_speed with no validation at all.
That's in the DWC core, so it's probably OK to expect the actual
controller drivers to validate pci->max_link_speed when they use it,
but dw_pcie_config_presets() uses pcie_link_speed[pci->max_link_speed]
and I don't see an obvious check that prevents an out-of-bounds
reference there.
I think this patch is the right direction -- I don't think there's
anything in drivers/pci/of.c that *uses* the value of
"max-link-speed", so it would be nice if it didn't have to be updated
for every generation.
> Signed-off-by: Hans Zhang <18255117159@163.com>
> Acked-by: Manivannan Sadhasivam <mani@kernel.org>
> ---
> Changes for v6:
> - It'd be good to return the actual errno as of_property_read_u32() can return
> -EINVAL, -ENODATA and -EOVERFLOW. (Mani)
>
> Changes for v5:
> https://patchwork.kernel.org/project/linux-pci/patch/20251218125909.305300-1-18255117159@163.com/
>
> - Delete the check for speed. (Mani)
>
> Changes for v4:
> https://patchwork.kernel.org/project/linux-pci/patch/20251105134701.182795-1-18255117159@163.com/
>
> - Add pcie_max_supported_link_speed.(Ilpo)
>
> Changes for v3:
> https://patchwork.kernel.org/project/linux-pci/patch/20251101164132.14145-1-18255117159@163.com/
>
> - Modify the commit message.
> - Add Reviewed-by tag.
>
> Changes for v2:
> https://patchwork.kernel.org/project/linux-pci/cover/20250529021026.475861-1-18255117159@163.com/
> - The following files have been deleted:
> Documentation/devicetree/bindings/pci/pci.txt
>
> Update to this file again:
> dtschema/schemas/pci/pci-bus-common.yaml
> ---
> drivers/pci/of.c | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/pci/of.c b/drivers/pci/of.c
> index 3579265f1198..b56fdbcb3d72 100644
> --- a/drivers/pci/of.c
> +++ b/drivers/pci/of.c
> @@ -888,10 +888,11 @@ bool of_pci_supply_present(struct device_node *np)
> int of_pci_get_max_link_speed(struct device_node *node)
> {
> u32 max_link_speed;
> + int ret;
>
> - if (of_property_read_u32(node, "max-link-speed", &max_link_speed) ||
> - max_link_speed == 0 || max_link_speed > 4)
> - return -EINVAL;
> + ret = of_property_read_u32(node, "max-link-speed", &max_link_speed);
> + if (ret)
> + return ret;
>
> return max_link_speed;
> }
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH v6 1/1] PCI: of: Remove max-link-speed generation validation
2026-03-06 17:08 ` Bjorn Helgaas
@ 2026-03-08 14:00 ` Hans Zhang
0 siblings, 0 replies; 5+ messages in thread
From: Hans Zhang @ 2026-03-08 14:00 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: bhelgaas, mani, ilpo.jarvinen, linux-pci, linux-kernel,
Rob Herring
On 2026/3/7 01:08, Bjorn Helgaas wrote:
> [+cc Rob]
>
> On Thu, Dec 18, 2025 at 09:20:36PM +0800, Hans Zhang wrote:
>> The current implementation of of_pci_get_max_link_speed() validates
>> max-link-speed property values to be in the range 1~4 (Gen1~Gen4).
>> However, this creates maintenance overhead as each new PCIe generation
>> requires updating this validation logic.
>>
>> Since device tree binding validation already enforces the allowed
>> values through the schema, and the callers of this function perform
>> their own validation checks, this intermediate validation becomes
>> redundant.
>>
>> Furthermore, with upcoming SOCs using Synopsys/Cadence IP requiring
>> Gen5/Gen6 support, removing this hardcoded check enables seamless
>> support for future PCIe generations without requiring kernel updates
>> for each new speed grade.
>
> The upcoming SoC info seems like too much detail for the commit log.
>
>> Remove the max-link-speed > 4 validation check while retaining the
>> property existence and non-zero check. This simplifies maintenance
>> and aligns with the existing validation architecture where DT binding
>> and driver-level checks provide sufficient validation.
>
> I don't think it's legit to rely on DT binding checks. The code
> should protect itself regardless of what offline binding checkers
> find.
>
> I'm not convinced that callers do their own validation. For example,
> dw_pcie_get_resources() assigns of_pci_get_max_link_speed() to
> pci->max_link_speed with no validation at all.
>
> That's in the DWC core, so it's probably OK to expect the actual
> controller drivers to validate pci->max_link_speed when they use it,
> but dw_pcie_config_presets() uses pcie_link_speed[pci->max_link_speed]
> and I don't see an obvious check that prevents an out-of-bounds
> reference there.
>
> I think this patch is the right direction -- I don't think there's
> anything in drivers/pci/of.c that *uses* the value of
> "max-link-speed", so it would be nice if it didn't have to be updated
> for every generation.
Hi Bjorn,
Thank you very much for your reply.
I will resend the v7 series patch and check in the dwc code whether
there is an array out-of-bounds issue with the maximum link speed.
Best regards,
Hans
>
>> Signed-off-by: Hans Zhang <18255117159@163.com>
>> Acked-by: Manivannan Sadhasivam <mani@kernel.org>
>> ---
>> Changes for v6:
>> - It'd be good to return the actual errno as of_property_read_u32() can return
>> -EINVAL, -ENODATA and -EOVERFLOW. (Mani)
>>
>> Changes for v5:
>> https://patchwork.kernel.org/project/linux-pci/patch/20251218125909.305300-1-18255117159@163.com/
>>
>> - Delete the check for speed. (Mani)
>>
>> Changes for v4:
>> https://patchwork.kernel.org/project/linux-pci/patch/20251105134701.182795-1-18255117159@163.com/
>>
>> - Add pcie_max_supported_link_speed.(Ilpo)
>>
>> Changes for v3:
>> https://patchwork.kernel.org/project/linux-pci/patch/20251101164132.14145-1-18255117159@163.com/
>>
>> - Modify the commit message.
>> - Add Reviewed-by tag.
>>
>> Changes for v2:
>> https://patchwork.kernel.org/project/linux-pci/cover/20250529021026.475861-1-18255117159@163.com/
>> - The following files have been deleted:
>> Documentation/devicetree/bindings/pci/pci.txt
>>
>> Update to this file again:
>> dtschema/schemas/pci/pci-bus-common.yaml
>> ---
>> drivers/pci/of.c | 7 ++++---
>> 1 file changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/pci/of.c b/drivers/pci/of.c
>> index 3579265f1198..b56fdbcb3d72 100644
>> --- a/drivers/pci/of.c
>> +++ b/drivers/pci/of.c
>> @@ -888,10 +888,11 @@ bool of_pci_supply_present(struct device_node *np)
>> int of_pci_get_max_link_speed(struct device_node *node)
>> {
>> u32 max_link_speed;
>> + int ret;
>>
>> - if (of_property_read_u32(node, "max-link-speed", &max_link_speed) ||
>> - max_link_speed == 0 || max_link_speed > 4)
>> - return -EINVAL;
>> + ret = of_property_read_u32(node, "max-link-speed", &max_link_speed);
>> + if (ret)
>> + return ret;
>>
>> return max_link_speed;
>> }
>> --
>> 2.34.1
>>
^ permalink raw reply [flat|nested] 5+ messages in thread