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 --- 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, -- Josua Mayer <josua@xxxxxxxxxxxxx>
