Hi, On 1-Jan-25 1:13 PM, 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 > > Update all accessesors of ahci_host_priv phys and target_pwrs arrays to > support holes. Access is gated by hpriv->mask_port_map which has a bit > set for each enabled port. > > 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. > > Signed-off-by: Josua Mayer <josua@xxxxxxxxxxxxx> > --- > Changes in v2: > - reverted back to dynamically allocated arrays > (Reported-by: Damien Le Moal <dlemoal@xxxxxxxxxx>) > - added helper function to find maximum port id > (Reported-by: Damien Le Moal <dlemoal@xxxxxxxxxx>) > - reduced size of changes > - rebased on 6.13-rc1 > - tested on 6.13-rc1 with CN9130 Clearfog Pro > - Link to v1: https://lore.kernel.org/r/20241121-ahci-nonconsecutive-ports-v1-1-1a20f52816fb@xxxxxxxxxxxxx Thanks, patch looks good to me: Reviewed-by: Hans de Goede <hdegoede@xxxxxxxxxx> Regards, Hans > --- > drivers/ata/ahci_brcm.c | 3 +++ > drivers/ata/ahci_ceva.c | 6 ++++++ > drivers/ata/libahci_platform.c | 40 ++++++++++++++++++++++++++++++++++------ > 3 files changed, 43 insertions(+), 6 deletions(-) > > diff --git a/drivers/ata/ahci_brcm.c b/drivers/ata/ahci_brcm.c > index ef569eae4ce4625e92c24c7dd54e8704b9aff2c4..24c471b485ab8b43eca21909ea16cb47a2a95ee1 100644 > --- a/drivers/ata/ahci_brcm.c > +++ b/drivers/ata/ahci_brcm.c > @@ -288,6 +288,9 @@ static unsigned int brcm_ahci_read_id(struct ata_device *dev, > > /* Re-initialize and calibrate the PHY */ > for (i = 0; i < hpriv->nports; 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 1ec35778903ddc28aebdab7d72676a31e757e56f..f2e20ed11ec70f48cb5f2c12812996bb99872aa5 100644 > --- a/drivers/ata/ahci_ceva.c > +++ b/drivers/ata/ahci_ceva.c > @@ -206,6 +206,9 @@ static int ceva_ahci_platform_enable_resources(struct ahci_host_priv *hpriv) > goto disable_clks; > > for (i = 0; i < hpriv->nports; i++) { > + if (!(hpriv->mask_port_map & (1 << i))) > + continue; > + > rc = phy_init(hpriv->phys[i]); > if (rc) > goto disable_rsts; > @@ -215,6 +218,9 @@ static int ceva_ahci_platform_enable_resources(struct ahci_host_priv *hpriv) > ahci_platform_deassert_rsts(hpriv); > > for (i = 0; i < hpriv->nports; i++) { > + if (!(hpriv->mask_port_map & (1 << i))) > + continue; > + > rc = phy_power_on(hpriv->phys[i]); > if (rc) { > phy_exit(hpriv->phys[i]); > diff --git a/drivers/ata/libahci_platform.c b/drivers/ata/libahci_platform.c > index 7a8064520a35bd86a1fa82d05c1ecaa8a81b7010..b68777841f7a544b755a16a633b1a2a47b90da08 100644 > --- a/drivers/ata/libahci_platform.c > +++ b/drivers/ata/libahci_platform.c > @@ -49,6 +49,9 @@ int ahci_platform_enable_phys(struct ahci_host_priv *hpriv) > int rc, i; > > for (i = 0; i < hpriv->nports; i++) { > + if (!(hpriv->mask_port_map & (1 << i))) > + continue; > + > rc = phy_init(hpriv->phys[i]); > if (rc) > goto disable_phys; > @@ -70,6 +73,9 @@ int ahci_platform_enable_phys(struct ahci_host_priv *hpriv) > > disable_phys: > while (--i >= 0) { > + if (!(hpriv->mask_port_map & (1 << i))) > + continue; > + > phy_power_off(hpriv->phys[i]); > phy_exit(hpriv->phys[i]); > } > @@ -88,6 +94,9 @@ void ahci_platform_disable_phys(struct ahci_host_priv *hpriv) > int i; > > for (i = 0; i < hpriv->nports; i++) { > + if (!(hpriv->mask_port_map & (1 << i))) > + continue; > + > phy_power_off(hpriv->phys[i]); > phy_exit(hpriv->phys[i]); > } > @@ -432,6 +441,20 @@ static int ahci_platform_get_firmware(struct ahci_host_priv *hpriv, > return 0; > } > > +static u32 ahci_platform_find_max_port_id(struct device *dev) > +{ > + u32 max_port = 0; > + > + for_each_child_of_node_scoped(dev->of_node, child) { > + u32 port; > + > + if (!of_property_read_u32(child, "reg", &port)) > + max_port = max(max_port, port); > + } > + > + return max_port; > +} > + > /** > * ahci_platform_get_resources - Get platform resources > * @pdev: platform device to get resources for > @@ -458,6 +481,7 @@ struct ahci_host_priv *ahci_platform_get_resources(struct platform_device *pdev, > struct device *dev = &pdev->dev; > struct ahci_host_priv *hpriv; > u32 mask_port_map = 0; > + u32 max_port; > > if (!devres_open_group(dev, NULL, GFP_KERNEL)) > return ERR_PTR(-ENOMEM); > @@ -549,15 +573,17 @@ struct ahci_host_priv *ahci_platform_get_resources(struct platform_device *pdev, > goto err_out; > } > > + /* find maximum port id for allocating structures */ > + max_port = ahci_platform_find_max_port_id(dev); > /* > - * If no sub-node was found, we still need to set nports to > - * one in order to be able to use the > + * Set nports according to maximum port id. Clamp at > + * AHCI_MAX_PORTS, warning message for invalid port id > + * is generated later. > + * When DT has no sub-nodes max_port is 0, nports is 1, > + * 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->nports = min(AHCI_MAX_PORTS, max_port + 1); > > hpriv->phys = devm_kcalloc(dev, hpriv->nports, sizeof(*hpriv->phys), GFP_KERNEL); > if (!hpriv->phys) { > @@ -625,6 +651,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: 40384c840ea1944d7c5a392e8975ed088ecf0b37 > change-id: 20241121-ahci-nonconsecutive-ports-a8911b3255a7 > > Best regards,
