ahci: asm1064: asm1166: don't limit reported ports

[PATCH] ahci: asm1064: asm1166: don't limit reported ports

Posted by Conrad Kostecki 1 year, 9 months ago

Previously, patches have been added to limit the reported count of SATA
ports for asm1064 and asm1166 SATA controllers, as those controllers do
report more ports than physical having.

Unfortunately, this causes trouble for users, which are using SATA
controllers, which provide more ports through SATA PMP
(Port-MultiPlier) and are now not any more recognized.

This happens, as asm1064 and 1166 are handling SATA PMP transparently,
so all non-physical ports needs to be enabled to use that feature.

This patch reverts both patches for asm1064 and asm1166, so old
behavior is restored and SATA PMP will work again, so all physical and
non-physical ports will work again.

Fixes: 0077a504e1a4 ("ahci: asm1166: correct count of reported ports")
Fixes: 9815e3961754 ("ahci: asm1064: correct count of reported ports")
Cc: stable@vger.kernel.org
Reported-by: Matt <cryptearth@googlemail.com>
Signed-off-by: Conrad Kostecki <conikost@gentoo.org>
---
 drivers/ata/ahci.c | 13 -------------
 1 file changed, 13 deletions(-)

diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
index 78570684ff68..562302e2e57c 100644
--- a/drivers/ata/ahci.c
+++ b/drivers/ata/ahci.c
@@ -669,19 +669,6 @@ MODULE_PARM_DESC(mobile_lpm_policy, "Default LPM policy for mobile chipsets");
 static void ahci_pci_save_initial_config(struct pci_dev *pdev,
 					 struct ahci_host_priv *hpriv)
 {
-	if (pdev->vendor == PCI_VENDOR_ID_ASMEDIA) {
-		switch (pdev->device) {
-		case 0x1166:
-			dev_info(&pdev->dev, "ASM1166 has only six ports\n");
-			hpriv->saved_port_map = 0x3f;
-			break;
-		case 0x1064:
-			dev_info(&pdev->dev, "ASM1064 has only four ports\n");
-			hpriv->saved_port_map = 0xf;
-			break;
-		}
-	}
-
 	if (pdev->vendor == PCI_VENDOR_ID_JMICRON && pdev->device == 0x2361) {
 		dev_info(&pdev->dev, "JMB361 has only one port\n");
 		hpriv->saved_port_map = 1;
-- 
2.44.0

Re: [PATCH] ahci: asm1064: asm1166: don't limit reported ports

Posted by Niklas Cassel 1 year, 9 months ago

On Wed, Mar 13, 2024 at 10:46:50PM +0100, Conrad Kostecki wrote:
> Previously, patches have been added to limit the reported count of SATA
> ports for asm1064 and asm1166 SATA controllers, as those controllers do
> report more ports than physical having.
> 
> Unfortunately, this causes trouble for users, which are using SATA
> controllers, which provide more ports through SATA PMP
> (Port-MultiPlier) and are now not any more recognized.
> 
> This happens, as asm1064 and 1166 are handling SATA PMP transparently,
> so all non-physical ports needs to be enabled to use that feature.
> 
> This patch reverts both patches for asm1064 and asm1166, so old
> behavior is restored and SATA PMP will work again, so all physical and
> non-physical ports will work again.
> 
> Fixes: 0077a504e1a4 ("ahci: asm1166: correct count of reported ports")
> Fixes: 9815e3961754 ("ahci: asm1064: correct count of reported ports")
> Cc: stable@vger.kernel.org
> Reported-by: Matt <cryptearth@googlemail.com>
> Signed-off-by: Conrad Kostecki <conikost@gentoo.org>
> ---
>  drivers/ata/ahci.c | 13 -------------
>  1 file changed, 13 deletions(-)
> 
> diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
> index 78570684ff68..562302e2e57c 100644
> --- a/drivers/ata/ahci.c
> +++ b/drivers/ata/ahci.c
> @@ -669,19 +669,6 @@ MODULE_PARM_DESC(mobile_lpm_policy, "Default LPM policy for mobile chipsets");
>  static void ahci_pci_save_initial_config(struct pci_dev *pdev,
>  					 struct ahci_host_priv *hpriv)
>  {
> -	if (pdev->vendor == PCI_VENDOR_ID_ASMEDIA) {
> -		switch (pdev->device) {
> -		case 0x1166:
> -			dev_info(&pdev->dev, "ASM1166 has only six ports\n");
> -			hpriv->saved_port_map = 0x3f;
> -			break;
> -		case 0x1064:
> -			dev_info(&pdev->dev, "ASM1064 has only four ports\n");
> -			hpriv->saved_port_map = 0xf;
> -			break;
> -		}
> -	}
> -
>  	if (pdev->vendor == PCI_VENDOR_ID_JMICRON && pdev->device == 0x2361) {
>  		dev_info(&pdev->dev, "JMB361 has only one port\n");
>  		hpriv->saved_port_map = 1;
> -- 
> 2.44.0
> 

I took the liberty to add additional information in the commit message.

Applied:
https://git.kernel.org/pub/scm/linux/kernel/git/libata/linux.git/commit/?id=6cd8adc3e18960f6e59d797285ed34ef473cc896

...and already sent to Linus:
https://lore.kernel.org/linux-ide/20240319113758.197709-1-cassel@kernel.org/T/#u


Kind regards,
Niklas

Re: [PATCH] ahci: asm1064: asm1166: don't limit reported ports

Posted by Hans de Goede 1 year, 9 months ago

Hi,

On 3/13/24 10:46 PM, Conrad Kostecki wrote:
> Previously, patches have been added to limit the reported count of SATA
> ports for asm1064 and asm1166 SATA controllers, as those controllers do
> report more ports than physical having.
> 
> Unfortunately, this causes trouble for users, which are using SATA
> controllers, which provide more ports through SATA PMP
> (Port-MultiPlier) and are now not any more recognized.
> 
> This happens, as asm1064 and 1166 are handling SATA PMP transparently,
> so all non-physical ports needs to be enabled to use that feature.
> 
> This patch reverts both patches for asm1064 and asm1166, so old
> behavior is restored and SATA PMP will work again, so all physical and
> non-physical ports will work again.
> 
> Fixes: 0077a504e1a4 ("ahci: asm1166: correct count of reported ports")
> Fixes: 9815e3961754 ("ahci: asm1064: correct count of reported ports")
> Cc: stable@vger.kernel.org
> Reported-by: Matt <cryptearth@googlemail.com>
> Signed-off-by: Conrad Kostecki <conikost@gentoo.org>

Thank you for the quick patch, patch looks good to me:

Reviewed-by: Hans de Goede <hdegoede@redhat.com>

Regards,

Hans







> ---
>  drivers/ata/ahci.c | 13 -------------
>  1 file changed, 13 deletions(-)
> 
> diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
> index 78570684ff68..562302e2e57c 100644
> --- a/drivers/ata/ahci.c
> +++ b/drivers/ata/ahci.c
> @@ -669,19 +669,6 @@ MODULE_PARM_DESC(mobile_lpm_policy, "Default LPM policy for mobile chipsets");
>  static void ahci_pci_save_initial_config(struct pci_dev *pdev,
>  					 struct ahci_host_priv *hpriv)
>  {
> -	if (pdev->vendor == PCI_VENDOR_ID_ASMEDIA) {
> -		switch (pdev->device) {
> -		case 0x1166:
> -			dev_info(&pdev->dev, "ASM1166 has only six ports\n");
> -			hpriv->saved_port_map = 0x3f;
> -			break;
> -		case 0x1064:
> -			dev_info(&pdev->dev, "ASM1064 has only four ports\n");
> -			hpriv->saved_port_map = 0xf;
> -			break;
> -		}
> -	}
> -
>  	if (pdev->vendor == PCI_VENDOR_ID_JMICRON && pdev->device == 0x2361) {
>  		dev_info(&pdev->dev, "JMB361 has only one port\n");
>  		hpriv->saved_port_map = 1;