| Message ID | 20241121-ahci-nonconsecutive-ports-v1-1-1a20f52816fb@solid-run.com |
|---|---|
| State | New |
| Headers | show |
| Series | [RFC] ata: libahci_platform: support non-consecutive port numbers | expand |
On 11/22/24 12:05 AM, Josua Mayer wrote: > So far ahci_platform relied on number of child nodes in firmware to > allocate arrays and expected port numbers to start from 0 without holes. > This number of ports is then set in private structure for use when > configuring phys and regulators. > > Some platforms may not use every port of an ahci controller. > E.g. SolidRUN CN9130 Clearfog uses only port 1 but not port 0, leading > to the following errors during boot: > [ 1.719476] ahci f2540000.sata: invalid port number 1 > [ 1.724562] ahci f2540000.sata: No port enabled > > Remove from ahci_host_priv the property nports which only makes sense > when enabled ports are consecutive. It is replaced with AHCI_MAX_PORTS > and checks for hpriv->mask_port_map, which indicates each port that is > enabled. > > Update ahci_host_priv properties target_pwrs and phys from dynamically > allocated arrays to statically allocated to size AHCI_MAX_PORTS. > > Update ahci_platform_get_resources to ignore holes in the port numbers > and enable ports defined in firmware by their reg property only. > > When firmware does not define children it is assumed that there is > exactly one port, using index 0. > > I marked this RFC because it was only tested with Linux v6.1, Marvell > fork, CN9130 Clearfog Pro which has only port number 1 in device-tree. > Further I am not completely sure if it has severe side-effects on > other platforms. > I plan to submit it again after testing on v6.13-rc1, but do welcome > feedback in the meantime, particularly whether this idea of supporting > non-consecutive ports is acceptable. > > Signed-off-by: Josua Mayer <josua@solid-run.com> [...] > @@ -539,41 +544,7 @@ struct ahci_host_priv *ahci_platform_get_resources(struct platform_device *pdev, > hpriv->f_rsts = flags & AHCI_PLATFORM_RST_TRIGGER; > } > > - /* > - * Too many sub-nodes most likely means having something wrong with > - * the firmware. > - */ > child_nodes = of_get_child_count(dev->of_node); > - if (child_nodes > AHCI_MAX_PORTS) { > - rc = -EINVAL; > - goto err_out; > - } Why remove this check ? Your platform may not need ti, but it is still valid for others. > - > - /* > - * If no sub-node was found, we still need to set nports to > - * one in order to be able to use the > - * ahci_platform_[en|dis]able_[phys|regulators] functions. > - */ > - if (child_nodes) > - hpriv->nports = child_nodes; > - else > - hpriv->nports = 1; Same here. > - > - hpriv->phys = devm_kcalloc(dev, hpriv->nports, sizeof(*hpriv->phys), GFP_KERNEL); > - if (!hpriv->phys) { > - rc = -ENOMEM; > - goto err_out; > - } > - /* > - * We cannot use devm_ here, since ahci_platform_put_resources() uses > - * target_pwrs after devm_ have freed memory > - */ > - hpriv->target_pwrs = kcalloc(hpriv->nports, sizeof(*hpriv->target_pwrs), GFP_KERNEL); > - if (!hpriv->target_pwrs) { > - rc = -ENOMEM; > - goto err_out; > - } And for platforms that actually have a valid nports with no ID holes, the above is OK and uses less memory... Why not simply adding code that checks the ID of the child nodes ? If there are no ID holes, then nothing need to change. If there are holes, then hpriv->nports can be set to the highest ID + 1 and you can set hpriv->mask_port_map as you go. With just that, you should get everything working with far less changes than you have here. > if (child_nodes) { > for_each_child_of_node_scoped(dev->of_node, child) { > u32 port; > @@ -587,7 +558,7 @@ struct ahci_host_priv *ahci_platform_get_resources(struct platform_device *pdev, > goto err_out; > } > > - if (port >= hpriv->nports) { > + if (port >= AHCI_MAX_PORTS) { > dev_warn(dev, "invalid port number %d\n", port); > continue; > } > @@ -625,6 +596,8 @@ struct ahci_host_priv *ahci_platform_get_resources(struct platform_device *pdev, > * If no sub-node was found, keep this for device tree > * compatibility > */ > + hpriv->mask_port_map |= BIT(0); > + > rc = ahci_platform_get_phy(hpriv, 0, dev, dev->of_node); > if (rc) > goto err_out; > > --- > base-commit: adc218676eef25575469234709c2d87185ca223a > change-id: 20241121-ahci-nonconsecutive-ports-a8911b3255a7 > > Best regards,
Am 25.11.24 um 02:12 schrieb Damien Le Moal: > On 11/22/24 12:05 AM, Josua Mayer wrote: >> So far ahci_platform relied on number of child nodes in firmware to >> allocate arrays and expected port numbers to start from 0 without holes. >> This number of ports is then set in private structure for use when >> configuring phys and regulators. >> >> Some platforms may not use every port of an ahci controller. >> E.g. SolidRUN CN9130 Clearfog uses only port 1 but not port 0, leading >> to the following errors during boot: >> [ 1.719476] ahci f2540000.sata: invalid port number 1 >> [ 1.724562] ahci f2540000.sata: No port enabled >> >> Remove from ahci_host_priv the property nports which only makes sense >> when enabled ports are consecutive. It is replaced with AHCI_MAX_PORTS >> and checks for hpriv->mask_port_map, which indicates each port that is >> enabled. >> >> Update ahci_host_priv properties target_pwrs and phys from dynamically >> allocated arrays to statically allocated to size AHCI_MAX_PORTS. >> >> Update ahci_platform_get_resources to ignore holes in the port numbers >> and enable ports defined in firmware by their reg property only. >> >> When firmware does not define children it is assumed that there is >> exactly one port, using index 0. >> >> I marked this RFC because it was only tested with Linux v6.1, Marvell >> fork, CN9130 Clearfog Pro which has only port number 1 in device-tree. >> Further I am not completely sure if it has severe side-effects on >> other platforms. >> I plan to submit it again after testing on v6.13-rc1, but do welcome >> feedback in the meantime, particularly whether this idea of supporting >> non-consecutive ports is acceptable. >> >> Signed-off-by: Josua Mayer <josua@solid-run.com> > [...] > > >> @@ -539,41 +544,7 @@ struct ahci_host_priv *ahci_platform_get_resources(struct platform_device *pdev, >> hpriv->f_rsts = flags & AHCI_PLATFORM_RST_TRIGGER; >> } >> >> - /* >> - * Too many sub-nodes most likely means having something wrong with >> - * the firmware. >> - */ >> child_nodes = of_get_child_count(dev->of_node); >> - if (child_nodes > AHCI_MAX_PORTS) { >> - rc = -EINVAL; >> - goto err_out; >> - } > Why remove this check ? Your platform may not need ti, but it is still valid > for others. The check is superfluous, since the following loop will print a warning and ignore any child with port number greater than AHCI_MAX_PORTS. The check merely protected against dynamically allocating greater than AHCI_MAX_PORTS. > >> - >> - /* >> - * If no sub-node was found, we still need to set nports to >> - * one in order to be able to use the >> - * ahci_platform_[en|dis]able_[phys|regulators] functions. >> - */ >> - if (child_nodes) >> - hpriv->nports = child_nodes; >> - else >> - hpriv->nports = 1; > Same here. This is already handled in else case of if (child_nodes) > >> - >> - hpriv->phys = devm_kcalloc(dev, hpriv->nports, sizeof(*hpriv->phys), GFP_KERNEL); >> - if (!hpriv->phys) { >> - rc = -ENOMEM; >> - goto err_out; >> - } >> - /* >> - * We cannot use devm_ here, since ahci_platform_put_resources() uses >> - * target_pwrs after devm_ have freed memory >> - */ >> - hpriv->target_pwrs = kcalloc(hpriv->nports, sizeof(*hpriv->target_pwrs), GFP_KERNEL); >> - if (!hpriv->target_pwrs) { >> - rc = -ENOMEM; >> - goto err_out; >> - } > And for platforms that actually have a valid nports with no ID holes, the above > is OK and uses less memory... The port number is being used as index into the target_pwrs and phys arrays, which is why those arrays must allocate at least to the highest port id. A better way to save memory is by cleaning out this semantic, e.g. by dynamically allocating a structure of id, phy and supply for each port. > > Why not simply adding code that checks the ID of the child nodes ? If there are > no ID holes, then nothing need to change. If there are holes, then > hpriv->nports can be set to the highest ID + 1 and you can set > hpriv->mask_port_map as you go. This would make the already complex function more complex and less readable. I prefer to reduce corner cases rather than adding extras. > With just that, you should get everything > working with far less changes than you have here. > >> if (child_nodes) { >> for_each_child_of_node_scoped(dev->of_node, child) { >> u32 port; >> @@ -587,7 +558,7 @@ struct ahci_host_priv *ahci_platform_get_resources(struct platform_device *pdev, >> goto err_out; >> } >> >> - if (port >= hpriv->nports) { >> + if (port >= AHCI_MAX_PORTS) { >> dev_warn(dev, "invalid port number %d\n", port); >> continue; >> } >> @@ -625,6 +596,8 @@ struct ahci_host_priv *ahci_platform_get_resources(struct platform_device *pdev, >> * If no sub-node was found, keep this for device tree >> * compatibility >> */ >> + hpriv->mask_port_map |= BIT(0); >> + >> rc = ahci_platform_get_phy(hpriv, 0, dev, dev->of_node); >> if (rc) >> goto err_out; >> >> --- >> base-commit: adc218676eef25575469234709c2d87185ca223a >> change-id: 20241121-ahci-nonconsecutive-ports-a8911b3255a7 >> >> Best regards, >
On 11/27/24 20:52, Josua Mayer wrote: > Am 25.11.24 um 02:12 schrieb Damien Le Moal: > >> On 11/22/24 12:05 AM, Josua Mayer wrote: >>> So far ahci_platform relied on number of child nodes in firmware to >>> allocate arrays and expected port numbers to start from 0 without holes. >>> This number of ports is then set in private structure for use when >>> configuring phys and regulators. >>> >>> Some platforms may not use every port of an ahci controller. >>> E.g. SolidRUN CN9130 Clearfog uses only port 1 but not port 0, leading >>> to the following errors during boot: >>> [ 1.719476] ahci f2540000.sata: invalid port number 1 >>> [ 1.724562] ahci f2540000.sata: No port enabled >>> >>> Remove from ahci_host_priv the property nports which only makes sense >>> when enabled ports are consecutive. It is replaced with AHCI_MAX_PORTS >>> and checks for hpriv->mask_port_map, which indicates each port that is >>> enabled. >>> >>> Update ahci_host_priv properties target_pwrs and phys from dynamically >>> allocated arrays to statically allocated to size AHCI_MAX_PORTS. >>> >>> Update ahci_platform_get_resources to ignore holes in the port numbers >>> and enable ports defined in firmware by their reg property only. >>> >>> When firmware does not define children it is assumed that there is >>> exactly one port, using index 0. >>> >>> I marked this RFC because it was only tested with Linux v6.1, Marvell >>> fork, CN9130 Clearfog Pro which has only port number 1 in device-tree. >>> Further I am not completely sure if it has severe side-effects on >>> other platforms. >>> I plan to submit it again after testing on v6.13-rc1, but do welcome >>> feedback in the meantime, particularly whether this idea of supporting >>> non-consecutive ports is acceptable. >>> >>> Signed-off-by: Josua Mayer <josua@solid-run.com> >> [...] >> >> >>> @@ -539,41 +544,7 @@ struct ahci_host_priv *ahci_platform_get_resources(struct platform_device *pdev, >>> hpriv->f_rsts = flags & AHCI_PLATFORM_RST_TRIGGER; >>> } >>> >>> - /* >>> - * Too many sub-nodes most likely means having something wrong with >>> - * the firmware. >>> - */ >>> child_nodes = of_get_child_count(dev->of_node); >>> - if (child_nodes > AHCI_MAX_PORTS) { >>> - rc = -EINVAL; >>> - goto err_out; >>> - } >> Why remove this check ? Your platform may not need ti, but it is still valid >> for others. > The check is superfluous, since the following loop will print a warning > and ignore any child with port number greater than AHCI_MAX_PORTS. > The check merely protected against dynamically allocating greater than > AHCI_MAX_PORTS. >> >>> - >>> - /* >>> - * If no sub-node was found, we still need to set nports to >>> - * one in order to be able to use the >>> - * ahci_platform_[en|dis]able_[phys|regulators] functions. >>> - */ >>> - if (child_nodes) >>> - hpriv->nports = child_nodes; >>> - else >>> - hpriv->nports = 1; >> Same here. > This is already handled in else case of if (child_nodes) >> >>> - >>> - hpriv->phys = devm_kcalloc(dev, hpriv->nports, sizeof(*hpriv->phys), GFP_KERNEL); >>> - if (!hpriv->phys) { >>> - rc = -ENOMEM; >>> - goto err_out; >>> - } >>> - /* >>> - * We cannot use devm_ here, since ahci_platform_put_resources() uses >>> - * target_pwrs after devm_ have freed memory >>> - */ >>> - hpriv->target_pwrs = kcalloc(hpriv->nports, sizeof(*hpriv->target_pwrs), GFP_KERNEL); >>> - if (!hpriv->target_pwrs) { >>> - rc = -ENOMEM; >>> - goto err_out; >>> - } >> And for platforms that actually have a valid nports with no ID holes, the above >> is OK and uses less memory... > The port number is being used as index into the target_pwrs and phys arrays, > which is why those arrays must allocate at least to the highest port id. > A better way to save memory is by cleaning out this semantic, > e.g. by dynamically allocating a structure of id, phy and supply for each port. >> >> Why not simply adding code that checks the ID of the child nodes ? If there are >> no ID holes, then nothing need to change. If there are holes, then >> hpriv->nports can be set to the highest ID + 1 and you can set >> hpriv->mask_port_map as you go. > This would make the already complex function more complex and less readable. > I prefer to reduce corner cases rather than adding extras. It would not. Simply implement a helper function that scans the OF IDs and return a mask of ports and max ID. With that, the modifications of ahci_platform_get_resources() would be very minimal. >> With just that, you should get everything >> working with far less changes than you have here. >> >>> if (child_nodes) { >>> for_each_child_of_node_scoped(dev->of_node, child) { >>> u32 port; >>> @@ -587,7 +558,7 @@ struct ahci_host_priv *ahci_platform_get_resources(struct platform_device *pdev, >>> goto err_out; >>> } >>> >>> - if (port >= hpriv->nports) { >>> + if (port >= AHCI_MAX_PORTS) { >>> dev_warn(dev, "invalid port number %d\n", port); >>> continue; >>> } >>> @@ -625,6 +596,8 @@ struct ahci_host_priv *ahci_platform_get_resources(struct platform_device *pdev, >>> * If no sub-node was found, keep this for device tree >>> * compatibility >>> */ >>> + hpriv->mask_port_map |= BIT(0); >>> + >>> rc = ahci_platform_get_phy(hpriv, 0, dev, dev->of_node); >>> if (rc) >>> goto err_out; >>> >>> --- >>> base-commit: adc218676eef25575469234709c2d87185ca223a >>> change-id: 20241121-ahci-nonconsecutive-ports-a8911b3255a7 >>> >>> Best regards, >>
diff --git a/drivers/ata/ahci.h b/drivers/ata/ahci.h index 8f40f75ba08cff4deca261b1162ee9a0fb91fed6..544b6242c11131f75c724d15244a74588410ee05 100644 --- a/drivers/ata/ahci.h +++ b/drivers/ata/ahci.h @@ -348,15 +348,14 @@ struct ahci_host_priv { struct clk_bulk_data *clks; /* Optional */ unsigned int f_rsts; struct reset_control *rsts; /* Optional */ - struct regulator **target_pwrs; /* Optional */ + struct regulator *target_pwrs[AHCI_MAX_PORTS]; /* Optional */ struct regulator *ahci_regulator;/* Optional */ struct regulator *phy_regulator;/* Optional */ /* * If platform uses PHYs. There is a 1:1 relation between the port number and * the PHY position in this array. */ - struct phy **phys; - unsigned nports; /* Number of ports */ + struct phy *phys[AHCI_MAX_PORTS]; void *plat_data; /* Other platform data */ unsigned int irq; /* interrupt line */ /* diff --git a/drivers/ata/ahci_brcm.c b/drivers/ata/ahci_brcm.c index 2f16524c252629cf89014144b33fec85d6bca137..169519c4cde37fe8aea6a48fbef16fd10d4d402c 100644 --- a/drivers/ata/ahci_brcm.c +++ b/drivers/ata/ahci_brcm.c @@ -287,7 +287,10 @@ static unsigned int brcm_ahci_read_id(struct ata_device *dev, brcm_sata_phy_enable(priv, ap->port_no); /* Re-initialize and calibrate the PHY */ - for (i = 0; i < hpriv->nports; i++) { + for (i = 0; i < AHCI_MAX_PORTS; i++) { + if (!(hpriv->mask_port_map & (1 << i))) + continue; + rc = phy_init(hpriv->phys[i]); if (rc) goto disable_phys; diff --git a/drivers/ata/ahci_ceva.c b/drivers/ata/ahci_ceva.c index 11a2c199a7c24628e858f2fc8e88e69a60c8b94b..4f0b5674f68e62d6871207e82de16c8422232285 100644 --- a/drivers/ata/ahci_ceva.c +++ b/drivers/ata/ahci_ceva.c @@ -205,20 +205,24 @@ static int ceva_ahci_platform_enable_resources(struct ahci_host_priv *hpriv) if (rc) goto disable_clks; - for (i = 0; i < hpriv->nports; i++) { - rc = phy_init(hpriv->phys[i]); - if (rc) - goto disable_rsts; + for (i = 0; i < AHCI_MAX_PORTS; i++) { + if (hpriv->mask_port_map & (1 << i)) { + rc = phy_init(hpriv->phys[i]); + if (rc) + goto disable_rsts; + } } /* De-assert the controller reset */ ahci_platform_deassert_rsts(hpriv); - for (i = 0; i < hpriv->nports; i++) { - rc = phy_power_on(hpriv->phys[i]); - if (rc) { - phy_exit(hpriv->phys[i]); - goto disable_phys; + for (i = 0; i < AHCI_MAX_PORTS; i++) { + if (hpriv->mask_port_map & (1 << i)) { + rc = phy_power_on(hpriv->phys[i]); + if (rc) { + phy_exit(hpriv->phys[i]); + goto disable_phys; + } } } diff --git a/drivers/ata/libahci_platform.c b/drivers/ata/libahci_platform.c index 7a8064520a35bd86a1fa82d05c1ecaa8a81b7010..e73dbdaa17604999561646f8d0b04f3e7314305c 100644 --- a/drivers/ata/libahci_platform.c +++ b/drivers/ata/libahci_platform.c @@ -48,7 +48,10 @@ int ahci_platform_enable_phys(struct ahci_host_priv *hpriv) { int rc, i; - for (i = 0; i < hpriv->nports; i++) { + for (i = 0; i < AHCI_MAX_PORTS; i++) { + if (!(hpriv->mask_port_map & (1 << i))) + continue; + rc = phy_init(hpriv->phys[i]); if (rc) goto disable_phys; @@ -70,8 +73,10 @@ int ahci_platform_enable_phys(struct ahci_host_priv *hpriv) disable_phys: while (--i >= 0) { - phy_power_off(hpriv->phys[i]); - phy_exit(hpriv->phys[i]); + if (hpriv->mask_port_map & (1 << i)) { + phy_power_off(hpriv->phys[i]); + phy_exit(hpriv->phys[i]); + } } return rc; } @@ -87,9 +92,11 @@ void ahci_platform_disable_phys(struct ahci_host_priv *hpriv) { int i; - for (i = 0; i < hpriv->nports; i++) { - phy_power_off(hpriv->phys[i]); - phy_exit(hpriv->phys[i]); + for (i = 0; i < AHCI_MAX_PORTS; i++) { + if (hpriv->mask_port_map & (1 << i)) { + phy_power_off(hpriv->phys[i]); + phy_exit(hpriv->phys[i]); + } } } EXPORT_SYMBOL_GPL(ahci_platform_disable_phys); @@ -209,13 +216,12 @@ int ahci_platform_enable_regulators(struct ahci_host_priv *hpriv) if (rc) goto disable_ahci_pwrs; - for (i = 0; i < hpriv->nports; i++) { - if (!hpriv->target_pwrs[i]) - continue; - - rc = regulator_enable(hpriv->target_pwrs[i]); - if (rc) - goto disable_target_pwrs; + for (i = 0; i < AHCI_MAX_PORTS; i++) { + if (hpriv->mask_port_map & (1 << i)) { + rc = regulator_enable(hpriv->target_pwrs[i]); + if (rc) + goto disable_target_pwrs; + } } return 0; @@ -243,10 +249,9 @@ void ahci_platform_disable_regulators(struct ahci_host_priv *hpriv) { int i; - for (i = 0; i < hpriv->nports; i++) { - if (!hpriv->target_pwrs[i]) - continue; - regulator_disable(hpriv->target_pwrs[i]); + for (i = 0; i < AHCI_MAX_PORTS; i++) { + if (hpriv->mask_port_map & (1 << i)) + regulator_disable(hpriv->target_pwrs[i]); } regulator_disable(hpriv->ahci_regulator); @@ -343,8 +348,8 @@ static void ahci_platform_put_resources(struct device *dev, void *res) * SATA device itself. So we can't use devm for automatically * releasing them. We have to do it manually here. */ - for (c = 0; c < hpriv->nports; c++) - if (hpriv->target_pwrs && hpriv->target_pwrs[c]) + for (c = 0; c < AHCI_MAX_PORTS; c++) + if ((hpriv->mask_port_map & (1 << c)) && hpriv->target_pwrs[c]) regulator_put(hpriv->target_pwrs[c]); kfree(hpriv->target_pwrs); @@ -539,41 +544,7 @@ struct ahci_host_priv *ahci_platform_get_resources(struct platform_device *pdev, hpriv->f_rsts = flags & AHCI_PLATFORM_RST_TRIGGER; } - /* - * Too many sub-nodes most likely means having something wrong with - * the firmware. - */ child_nodes = of_get_child_count(dev->of_node); - if (child_nodes > AHCI_MAX_PORTS) { - rc = -EINVAL; - goto err_out; - } - - /* - * If no sub-node was found, we still need to set nports to - * one in order to be able to use the - * ahci_platform_[en|dis]able_[phys|regulators] functions. - */ - if (child_nodes) - hpriv->nports = child_nodes; - else - hpriv->nports = 1; - - hpriv->phys = devm_kcalloc(dev, hpriv->nports, sizeof(*hpriv->phys), GFP_KERNEL); - if (!hpriv->phys) { - rc = -ENOMEM; - goto err_out; - } - /* - * We cannot use devm_ here, since ahci_platform_put_resources() uses - * target_pwrs after devm_ have freed memory - */ - hpriv->target_pwrs = kcalloc(hpriv->nports, sizeof(*hpriv->target_pwrs), GFP_KERNEL); - if (!hpriv->target_pwrs) { - rc = -ENOMEM; - goto err_out; - } - if (child_nodes) { for_each_child_of_node_scoped(dev->of_node, child) { u32 port; @@ -587,7 +558,7 @@ struct ahci_host_priv *ahci_platform_get_resources(struct platform_device *pdev, goto err_out; } - if (port >= hpriv->nports) { + if (port >= AHCI_MAX_PORTS) { dev_warn(dev, "invalid port number %d\n", port); continue; } @@ -625,6 +596,8 @@ struct ahci_host_priv *ahci_platform_get_resources(struct platform_device *pdev, * If no sub-node was found, keep this for device tree * compatibility */ + hpriv->mask_port_map |= BIT(0); + rc = ahci_platform_get_phy(hpriv, 0, dev, dev->of_node); if (rc) goto err_out;
So far ahci_platform relied on number of child nodes in firmware to allocate arrays and expected port numbers to start from 0 without holes. This number of ports is then set in private structure for use when configuring phys and regulators. Some platforms may not use every port of an ahci controller. E.g. SolidRUN CN9130 Clearfog uses only port 1 but not port 0, leading to the following errors during boot: [ 1.719476] ahci f2540000.sata: invalid port number 1 [ 1.724562] ahci f2540000.sata: No port enabled Remove from ahci_host_priv the property nports which only makes sense when enabled ports are consecutive. It is replaced with AHCI_MAX_PORTS and checks for hpriv->mask_port_map, which indicates each port that is enabled. Update ahci_host_priv properties target_pwrs and phys from dynamically allocated arrays to statically allocated to size AHCI_MAX_PORTS. Update ahci_platform_get_resources to ignore holes in the port numbers and enable ports defined in firmware by their reg property only. When firmware does not define children it is assumed that there is exactly one port, using index 0. I marked this RFC because it was only tested with Linux v6.1, Marvell fork, CN9130 Clearfog Pro which has only port number 1 in device-tree. Further I am not completely sure if it has severe side-effects on other platforms. I plan to submit it again after testing on v6.13-rc1, but do welcome feedback in the meantime, particularly whether this idea of supporting non-consecutive ports is acceptable. Signed-off-by: Josua Mayer <josua@solid-run.com> --- drivers/ata/ahci.h | 5 ++- drivers/ata/ahci_brcm.c | 5 ++- drivers/ata/ahci_ceva.c | 22 +++++++----- drivers/ata/libahci_platform.c | 81 ++++++++++++++---------------------------- 4 files changed, 46 insertions(+), 67 deletions(-) --- base-commit: adc218676eef25575469234709c2d87185ca223a change-id: 20241121-ahci-nonconsecutive-ports-a8911b3255a7 Best regards,