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

[Josua Mayer <josua@solid-run.com>]

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