All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] ata: ahci: preserve preset port masks
@ 2026-10-15  3:06 zhoumin
  2026-10-15  5:44 ` Damien Le Moal
  2026-10-15  7:59 ` Niklas Cassel
  0 siblings, 2 replies; 5+ messages in thread
From: zhoumin @ 2026-10-15  3:06 UTC (permalink / raw)
  To: dlemoal, cassel; +Cc: linux-ide, linux-kernel, teczm

ahci_get_port_mask() returns zero for absent or unmatched parameters,
overwriting the mask set by the Marvell PATA quirk.

Update the mask in place only when parsing a global value or a matching
device entry. Preserve existing numeric parsing and override behavior.

Fixes: 70c1595c181c ("ata: ahci: Allow ignoring the external/hotplug capability of ports")
Signed-off-by: zhoumin <teczm@foxmail.com>
---
 drivers/ata/ahci.c | 21 +++++++++------------
 1 file changed, 9 insertions(+), 12 deletions(-)

diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
index 9b8c0935001c..324e4918a055 100644
--- a/drivers/ata/ahci.c
+++ b/drivers/ata/ahci.c
@@ -1654,22 +1654,21 @@ static u32 ahci_port_mask(struct device *dev, char *mask_s)
 	return mask;
 }
 
-static u32 ahci_get_port_mask(struct device *dev, char *mask_p)
+static void ahci_get_port_mask(struct device *dev, char *mask_p, u32 *mask)
 {
 	char *param, *end, *str, *mask_s;
 	char *name;
-	u32 mask = 0;
 
 	if (!mask_p || !strlen(mask_p))
-		return 0;
+		return;
 
 	str = kstrdup(mask_p, GFP_KERNEL);
 	if (!str)
-		return 0;
+		return;
 
 	/* Handle single mask case */
 	if (!strchr(str, '=')) {
-		mask = ahci_port_mask(dev, str);
+		*mask = ahci_port_mask(dev, str);
 		goto free;
 	}
 
@@ -1704,13 +1703,11 @@ static u32 ahci_get_port_mask(struct device *dev, char *mask_p)
 			param++;
 		}
 
-		mask = ahci_port_mask(dev, mask_s);
+		*mask = ahci_port_mask(dev, mask_s);
 	}
 
 free:
 	kfree(str);
-
-	return mask;
 }
 
 static void ahci_pci_save_initial_config(struct pci_dev *pdev,
@@ -1736,10 +1733,10 @@ static void ahci_pci_save_initial_config(struct pci_dev *pdev,
 	}
 
 	/* Handle port map masks passed as module parameter. */
-	hpriv->mask_port_map =
-		ahci_get_port_mask(&pdev->dev, ahci_mask_port_map);
-	hpriv->mask_port_ext =
-		ahci_get_port_mask(&pdev->dev, ahci_mask_port_ext);
+	ahci_get_port_mask(&pdev->dev, ahci_mask_port_map,
+			   &hpriv->mask_port_map);
+	ahci_get_port_mask(&pdev->dev, ahci_mask_port_ext,
+			   &hpriv->mask_port_ext);
 
 	ahci_save_initial_config(&pdev->dev, hpriv);
 }
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] ata: ahci: preserve preset port masks
  2026-10-15  3:06 [PATCH] ata: ahci: preserve preset port masks zhoumin
@ 2026-10-15  5:44 ` Damien Le Moal
  2026-10-15  7:59 ` Niklas Cassel
  1 sibling, 0 replies; 5+ messages in thread
From: Damien Le Moal @ 2026-10-15  5:44 UTC (permalink / raw)
  To: zhoumin, cassel; +Cc: linux-ide, linux-kernel

On 2026/10/15 5:06, zhoumin wrote:
> ahci_get_port_mask() returns zero for absent or unmatched parameters,
> overwriting the mask set by the Marvell PATA quirk.
> 
> Update the mask in place only when parsing a global value or a matching
> device entry. Preserve existing numeric parsing and override behavior.
> 
> Fixes: 70c1595c181c ("ata: ahci: Allow ignoring the external/hotplug capability of ports")
> Signed-off-by: zhoumin <teczm@foxmail.com>

Looks OK.

Reviewed-by: Damien Le Moal <dlemoal@kernel.org>

-- 
Damien Le Moal
Western Digital Research

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] ata: ahci: preserve preset port masks
  2026-10-15  3:06 [PATCH] ata: ahci: preserve preset port masks zhoumin
  2026-10-15  5:44 ` Damien Le Moal
@ 2026-10-15  7:59 ` Niklas Cassel
  2026-10-15  9:41   ` [PATCH v2] " zhoumin
  1 sibling, 1 reply; 5+ messages in thread
From: Niklas Cassel @ 2026-10-15  7:59 UTC (permalink / raw)
  To: zhoumin; +Cc: dlemoal, linux-ide, linux-kernel

On Thu, Oct 15, 2026 at 11:06:55AM +0800, zhoumin wrote:
> ahci_get_port_mask() returns zero for absent or unmatched parameters,
> overwriting the mask set by the Marvell PATA quirk.
> 
> Update the mask in place only when parsing a global value or a matching
> device entry. Preserve existing numeric parsing and override behavior.
> 
> Fixes: 70c1595c181c ("ata: ahci: Allow ignoring the external/hotplug capability of ports")
> Signed-off-by: zhoumin <teczm@foxmail.com>
> ---
>  drivers/ata/ahci.c | 21 +++++++++------------
>  1 file changed, 9 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
> index 9b8c0935001c..324e4918a055 100644
> --- a/drivers/ata/ahci.c
> +++ b/drivers/ata/ahci.c
> @@ -1654,22 +1654,21 @@ static u32 ahci_port_mask(struct device *dev, char *mask_s)
>  	return mask;
>  }
>  
> -static u32 ahci_get_port_mask(struct device *dev, char *mask_p)
> +static void ahci_get_port_mask(struct device *dev, char *mask_p, u32 *mask)
>  {
>  	char *param, *end, *str, *mask_s;
>  	char *name;
> -	u32 mask = 0;
>  
>  	if (!mask_p || !strlen(mask_p))
> -		return 0;
> +		return;
>  
>  	str = kstrdup(mask_p, GFP_KERNEL);
>  	if (!str)
> -		return 0;
> +		return;
>  
>  	/* Handle single mask case */
>  	if (!strchr(str, '=')) {
> -		mask = ahci_port_mask(dev, str);
> +		*mask = ahci_port_mask(dev, str);
>  		goto free;
>  	}
>  
> @@ -1704,13 +1703,11 @@ static u32 ahci_get_port_mask(struct device *dev, char *mask_p)
>  			param++;
>  		}
>  
> -		mask = ahci_port_mask(dev, mask_s);
> +		*mask = ahci_port_mask(dev, mask_s);
>  	}
>  
>  free:
>  	kfree(str);
> -
> -	return mask;

Invalid values still wipe out the Marvell mask.
ahci_port_mask()  returns 0 when parsing fails, and the patch writes that
0 into the mask.

Before 70c1595c181c a bad value left the mask alone.

I suggest the following change on top of your patch:

diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
index 324e4918a055..fdce365969b4 100644
--- a/drivers/ata/ahci.c
+++ b/drivers/ata/ahci.c
@@ -1642,16 +1642,10 @@ MODULE_PARM_DESC(mask_port_ext,
 		 "where <pci_dev> is the PCI ID of an AHCI controller in the "
 		 "form \"domain:bus:dev.func\"");
 
-static u32 ahci_port_mask(struct device *dev, char *mask_s)
+static void ahci_port_mask(struct device *dev, char *mask_s, u32 *mask)
 {
-	unsigned int mask;
-
-	if (kstrtouint(mask_s, 0, &mask)) {
+	if (kstrtou32(mask_s, 0, mask))
 		dev_err(dev, "Invalid port map mask\n");
-		return 0;
-	}
-
-	return mask;
 }
 
 static void ahci_get_port_mask(struct device *dev, char *mask_p, u32 *mask)
@@ -1668,7 +1662,7 @@ static void ahci_get_port_mask(struct device *dev, char *mask_p, u32 *mask)
 
 	/* Handle single mask case */
 	if (!strchr(str, '=')) {
-		*mask = ahci_port_mask(dev, str);
+		ahci_port_mask(dev, str, mask);
 		goto free;
 	}
 
@@ -1703,7 +1697,7 @@ static void ahci_get_port_mask(struct device *dev, char *mask_p, u32 *mask)
 			param++;
 		}
 
-		*mask = ahci_port_mask(dev, mask_s);
+		ahci_port_mask(dev, mask_s, mask);
 	}
 
 free:







 This relies on  kstrtou32()  writing its result only on success.
 That is documented ("@res: Where to write the result of the conversion
 on success") and matches the code in  lib/kstrtox.c.
 So a bad value now just logs the error and leaves  the Marvell mask alone.

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH v2] ata: ahci: preserve preset port masks
  2026-10-15  7:59 ` Niklas Cassel
@ 2026-10-15  9:41   ` zhoumin
  2026-10-15 10:55     ` Niklas Cassel
  0 siblings, 1 reply; 5+ messages in thread
From: zhoumin @ 2026-10-15  9:41 UTC (permalink / raw)
  To: cassel; +Cc: dlemoal, linux-ide, linux-kernel, teczm

ahci_get_port_mask() returns zero for absent or unmatched parameters,
overwriting the mask set by the Marvell PATA quirk.

Update the mask in place only when parsing a global value or a matching
device entry. Leave the mask unchanged on parsing errors.

Fixes: 70c1595c181c ("ata: ahci: Allow ignoring the external/hotplug capability of ports")
Suggested-by: Niklas Cassel <cassel@kernel.org>
Signed-off-by: zhoumin <teczm@foxmail.com>
---
Changes in v2:
- Leave the mask unchanged on parsing errors, as suggested by Niklas.

 drivers/ata/ahci.c | 31 +++++++++++--------------------
 1 file changed, 11 insertions(+), 20 deletions(-)

diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
index 9b8c0935001c..fdce365969b4 100644
--- a/drivers/ata/ahci.c
+++ b/drivers/ata/ahci.c
@@ -1642,34 +1642,27 @@ MODULE_PARM_DESC(mask_port_ext,
 		 "where <pci_dev> is the PCI ID of an AHCI controller in the "
 		 "form \"domain:bus:dev.func\"");
 
-static u32 ahci_port_mask(struct device *dev, char *mask_s)
+static void ahci_port_mask(struct device *dev, char *mask_s, u32 *mask)
 {
-	unsigned int mask;
-
-	if (kstrtouint(mask_s, 0, &mask)) {
+	if (kstrtou32(mask_s, 0, mask))
 		dev_err(dev, "Invalid port map mask\n");
-		return 0;
-	}
-
-	return mask;
 }
 
-static u32 ahci_get_port_mask(struct device *dev, char *mask_p)
+static void ahci_get_port_mask(struct device *dev, char *mask_p, u32 *mask)
 {
 	char *param, *end, *str, *mask_s;
 	char *name;
-	u32 mask = 0;
 
 	if (!mask_p || !strlen(mask_p))
-		return 0;
+		return;
 
 	str = kstrdup(mask_p, GFP_KERNEL);
 	if (!str)
-		return 0;
+		return;
 
 	/* Handle single mask case */
 	if (!strchr(str, '=')) {
-		mask = ahci_port_mask(dev, str);
+		ahci_port_mask(dev, str, mask);
 		goto free;
 	}
 
@@ -1704,13 +1697,11 @@ static u32 ahci_get_port_mask(struct device *dev, char *mask_p)
 			param++;
 		}
 
-		mask = ahci_port_mask(dev, mask_s);
+		ahci_port_mask(dev, mask_s, mask);
 	}
 
 free:
 	kfree(str);
-
-	return mask;
 }
 
 static void ahci_pci_save_initial_config(struct pci_dev *pdev,
@@ -1736,10 +1727,10 @@ static void ahci_pci_save_initial_config(struct pci_dev *pdev,
 	}
 
 	/* Handle port map masks passed as module parameter. */
-	hpriv->mask_port_map =
-		ahci_get_port_mask(&pdev->dev, ahci_mask_port_map);
-	hpriv->mask_port_ext =
-		ahci_get_port_mask(&pdev->dev, ahci_mask_port_ext);
+	ahci_get_port_mask(&pdev->dev, ahci_mask_port_map,
+			   &hpriv->mask_port_map);
+	ahci_get_port_mask(&pdev->dev, ahci_mask_port_ext,
+			   &hpriv->mask_port_ext);
 
 	ahci_save_initial_config(&pdev->dev, hpriv);
 }
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] ata: ahci: preserve preset port masks
  2026-10-15  9:41   ` [PATCH v2] " zhoumin
@ 2026-10-15 10:55     ` Niklas Cassel
  0 siblings, 0 replies; 5+ messages in thread
From: Niklas Cassel @ 2026-10-15 10:55 UTC (permalink / raw)
  To: zhoumin; +Cc: dlemoal, linux-ide, linux-kernel

On Thu, 15 Oct 2026 17:41:02 +0800, zhoumin wrote:
> ahci_get_port_mask() returns zero for absent or unmatched parameters,
> overwriting the mask set by the Marvell PATA quirk.
> 
> Update the mask in place only when parsing a global value or a matching
> device entry. Leave the mask unchanged on parsing errors.
> 
> 
> [...]

Applied to libata/linux.git (for-7.4), thanks!

[1/1] ata: ahci: preserve preset port masks
      https://git.kernel.org/libata/linux/c/014c29a8

Kind regards,
Niklas


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-10-15 10:55 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-15  3:06 [PATCH] ata: ahci: preserve preset port masks zhoumin
2026-10-15  5:44 ` Damien Le Moal
2026-10-15  7:59 ` Niklas Cassel
2026-10-15  9:41   ` [PATCH v2] " zhoumin
2026-10-15 10:55     ` Niklas Cassel

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.