[PATCH] ata: libahci_platform: Do not set mask_port_map when not needed

[PATCH] ata: libahci_platform: Do not set mask_port_map when not needed

[Damien Le Moal]

Commit 8c87215dd3a2 ("ata: libahci_platform: support non-consecutive
port numbers") modified ahci_platform_get_resources() to allow
identifying the ports of a controller that are defined as child nodes of
the controller node in order to support non-consecutive port numbers (as
defined by the platform device tree).

However, this commit also erroneously sets bits 0 of
hpriv->mask_port_map when the platform devices tree does not define port
child nodes, to match the fact that the temporary default number of
ports used in that case is 1. Doing so causes ahci_platform_init_host()
to initialize and probe only the first port, even if the controller has
multiple ports (that are not defined through the platform DT).

Fix this by removing setting bit 0 of hpriv->mask_port_map when the
platform devices tree does not define port child nodes.

Reported-by: Klaus Kudielka <klaus.kudielka@gmail.com>
Fixes: 8c87215dd3a2 ("ata: libahci_platform: support non-consecutive port numbers")
Cc: stable@vger.kernel.org
Tested-by: Klaus Kudielka <klaus.kudielka@gmail.com>
Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
---
 drivers/ata/libahci_platform.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/ata/libahci_platform.c b/drivers/ata/libahci_platform.c
index 53b2c7719dc5..91d44302eac9 100644
--- a/drivers/ata/libahci_platform.c
+++ b/drivers/ata/libahci_platform.c
@@ -651,8 +651,6 @@ 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;
-- 
2.48.1

Re: [PATCH] ata: libahci_platform: Do not set mask_port_map when not needed

[Niklas Cassel]

On Fri, Feb 07, 2025 at 04:48:10PM +0900, Damien Le Moal wrote:
> Commit 8c87215dd3a2 ("ata: libahci_platform: support non-consecutive
> port numbers") modified ahci_platform_get_resources() to allow
> identifying the ports of a controller that are defined as child nodes of
> the controller node in order to support non-consecutive port numbers (as
> defined by the platform device tree).
> 
> However, this commit also erroneously sets bits 0 of

s/bits/bit/


> hpriv->mask_port_map when the platform devices tree does not define port
> child nodes, to match the fact that the temporary default number of
> ports used in that case is 1. Doing so causes ahci_platform_init_host()
> to initialize and probe only the first port, even if the controller has
> multiple ports (that are not defined through the platform DT).
> 
> Fix this by removing setting bit 0 of hpriv->mask_port_map when the
> platform devices tree does not define port child nodes.

s/devices/device/


> 
> Reported-by: Klaus Kudielka <klaus.kudielka@gmail.com>
> Fixes: 8c87215dd3a2 ("ata: libahci_platform: support non-consecutive port numbers")
> Cc: stable@vger.kernel.org

Cc: stable is used then the fix should be backported.
The commit referenced in Fixes: is only in v6.14-rc1, so no need to backport,
and thus no need to Cc: stable.



Kind regards,
Niklas

Re: [PATCH] ata: libahci_platform: Do not set mask_port_map when not needed

[Josua Mayer]

Hi Damien,

Am 07.02.25 um 08:48 schrieb Damien Le Moal:
> Commit 8c87215dd3a2 ("ata: libahci_platform: support non-consecutive
> port numbers") modified ahci_platform_get_resources() to allow
> identifying the ports of a controller that are defined as child nodes of
> the controller node in order to support non-consecutive port numbers (as
> defined by the platform device tree).
>
> However, this commit also erroneously sets bits 0 of
> hpriv->mask_port_map when the platform devices tree does not define port
> child nodes, to match the fact that the temporary default number of
> ports used in that case is 1.
And to match the fact that only index 0 of hpriv->phys[0] was initialized
from calling into ahci_platform_get_phy.
> Doing so causes ahci_platform_init_host()
> to initialize and probe only the first port, even if the controller has
> multiple ports (that are not defined through the platform DT).
>
> Fix this by removing setting bit 0 of hpriv->mask_port_map when the
> platform devices tree does not define port child nodes.
>
> Reported-by: Klaus Kudielka <klaus.kudielka@gmail.com>
> Fixes: 8c87215dd3a2 ("ata: libahci_platform: support non-consecutive port numbers")
> Cc: stable@vger.kernel.org
> Tested-by: Klaus Kudielka <klaus.kudielka@gmail.com>
> Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
> ---
>  drivers/ata/libahci_platform.c | 2 --
>  1 file changed, 2 deletions(-)
>
> diff --git a/drivers/ata/libahci_platform.c b/drivers/ata/libahci_platform.c
> index 53b2c7719dc5..91d44302eac9 100644
> --- a/drivers/ata/libahci_platform.c
> +++ b/drivers/ata/libahci_platform.c
> @@ -651,8 +651,6 @@ 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);

This function initializes hpriv->phys[0], and only at index 0.

I am not aware whether or not this causes any issue,
therefore no strong objection to restoring original behaviour.

>  		if (rc)
>  			goto err_out;
sincerely
Josua Mayer