Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1308094 > unrolled thread

Unhappy ahci controller on Dell XPS 13 9350

Back to article view | Back to linux.kernel

Contents

  Unhappy ahci controller on Dell XPS 13 9350 Andy Lutomirski <luto@amacapital.net> - 2016-01-13 08:00 +0100
    Re: Unhappy ahci controller on Dell XPS 13 9350 Tejun Heo <tj@kernel.org> - 2016-01-14 23:20 +0100
    [PATCH] libata: disable forced PORTS_IMPL for >= v3.0 controllers Tejun Heo <tj@kernel.org> - 2016-01-14 23:40 +0100
      Re: [PATCH] libata: disable forced PORTS_IMPL for >= v3.0 controllers Andy Lutomirski <luto@amacapital.net> - 2016-01-15 02:10 +0100
        Re: [PATCH] libata: disable forced PORTS_IMPL for >= v3.0 controllers Tejun Heo <tj@kernel.org> - 2016-01-15 20:50 +0100
      [PATCH v2] libata: disable forced PORTS_IMPL for >= AHCI 1.3 Tejun Heo <tj@kernel.org> - 2016-01-15 21:20 +0100
        Re: [PATCH v2] libata: disable forced PORTS_IMPL for >= AHCI 1.3 Andy Lutomirski <luto@amacapital.net> - 2016-01-15 23:50 +0100
        Re: [PATCH v2] libata: disable forced PORTS_IMPL for >= AHCI 1.3 Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> - 2016-01-16 11:10 +0100
          Re: [PATCH v2] libata: disable forced PORTS_IMPL for >= AHCI 1.3 Tejun Heo <tj@kernel.org> - 2016-01-19 18:30 +0100
            Re: [PATCH v2] libata: disable forced PORTS_IMPL for >= AHCI 1.3 Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> - 2016-01-19 18:30 +0100
        [PATCH v3] libata: disable forced PORTS_IMPL for >= AHCI 1.3 Tejun Heo <tj@kernel.org> - 2016-01-25 21:50 +0100

#1308094 — Unhappy ahci controller on Dell XPS 13 9350

Hi-

My XPS 13 9350 laptop has an nvme drive.  It has on-chip ahci
capability, but the M.2 port isn't using it due to the aforementioned
nvme drive using the same signal pins.

It is very unhappy:

[    0.230455] libata version 3.00 loaded.
[    1.039360] ahci 0000:00:17.0: version 3.0
[    1.039386] ahci 0000:00:17.0: forcing PORTS_IMPL to 0xff
[    1.539388] ahci 0000:00:17.0: failed to stop engine (-5)
[    2.039392] ahci 0000:00:17.0: failed to stop engine (-5)
[    2.539402] ahci 0000:00:17.0: failed to stop engine (-5)
[    2.539437] ahci 0000:00:17.0: AHCI 0001.0301 32 slots 8 ports 6
Gbps 0xff impl SATA mode
[    2.539440] ahci 0000:00:17.0: flags: 64bit ncq pm led clo only pio
slum part deso sadm sds apst
[    2.540855] scsi host0: ahci
[    2.541455] scsi host1: ahci
[    2.541972] scsi host2: ahci
[    2.542504] scsi host3: ahci
[    2.543027] scsi host4: ahci
[    2.543550] scsi host5: ahci
[    2.544035] scsi host6: ahci
[    2.544565] scsi host7: ahci
[    2.544641] ata1: SATA max UDMA/133 abar m2048@0xdc333000 port
0xdc333100 irq 122
[    2.544645] ata2: SATA max UDMA/133 abar m2048@0xdc333000 port
0xdc333180 irq 122
[    2.544648] ata3: SATA max UDMA/133 abar m2048@0xdc333000 port
0xdc333200 irq 122
[    2.544652] ata4: SATA max UDMA/133 abar m2048@0xdc333000 port
0xdc333280 irq 122
[    2.544656] ata5: SATA max UDMA/133 abar m2048@0xdc333000 port
0xdc333300 irq 122
[    2.544659] ata6: SATA max UDMA/133 abar m2048@0xdc333000 port
0xdc333380 irq 122
[    2.544662] ata7: SATA max UDMA/133 abar m2048@0xdc333000 port
0xdc333400 irq 122
[    2.544666] ata8: SATA max UDMA/133 abar m2048@0xdc333000 port
0xdc333480 irq 122
[    3.552450] ata8: failed to resume link (SControl 0)
[    3.552471] ata8: SATA link down (SStatus 0 SControl 0)
[    3.552491] ata7: failed to resume link (SControl 0)
[    3.552508] ata7: SATA link down (SStatus 0 SControl 0)
[    3.552528] ata6: failed to resume link (SControl 0)
[    3.552548] ata6: SATA link down (SStatus 0 SControl 0)
[    3.552567] ata5: failed to resume link (SControl 0)
[    3.552585] ata5: SATA link down (SStatus 0 SControl 0)
[    3.552602] ata4: failed to resume link (SControl 0)
[    3.552616] ata4: SATA link down (SStatus 0 SControl 0)
[    4.051472] ata2: failed to resume link (SControl FFFFFFFF)
[    4.051493] ata2: SATA link down (SStatus FFFFFFFF SControl FFFFFFFF)
[    4.052349] ata1: failed to resume link (SControl FFFFFFFF)
[    4.052370] ata1: SATA link down (SStatus FFFFFFFF SControl FFFFFFFF)
[    4.052394] ata3: failed to resume link (SControl FFFFFFFF)
[    4.052414] ata3: SATA link down (SStatus FFFFFFFF SControl FFFFFFFF)
[    4.556598] Write protecting the kernel read-only data: 14336k
[   12.410784] EXT4-fs (dm-1): mounted filesystem with ordered data
mode. Opts: (null)
[   13.471239] EXT4-fs (nvme0n1p2): mounted filesystem with ordered
data mode. Opts: (null)

It has Alpine Ridge, but I don't think there's such thing as SATA over
USB Type C.  I'd guess that that what's going on is that the ahci
controller genuinely has zero ports enabled and the workaround for
port_map == 0 is firing incorrectly.

This is Linux 4.4.

--Andy

[toc] | [next] | [standalone]

#1309705

Hello, Andy.

On Tue, Jan 12, 2016 at 10:51:15PM -0800, Andy Lutomirski wrote:
> Hi-
> 
> My XPS 13 9350 laptop has an nvme drive.  It has on-chip ahci
> capability, but the M.2 port isn't using it due to the aforementioned
> nvme drive using the same signal pins.
> 
> It is very unhappy:
> 
> [    0.230455] libata version 3.00 loaded.
> [    1.039360] ahci 0000:00:17.0: version 3.0
> [    1.039386] ahci 0000:00:17.0: forcing PORTS_IMPL to 0xff
...
> It has Alpine Ridge, but I don't think there's such thing as SATA over
> USB Type C.  I'd guess that that what's going on is that the ahci
> controller genuinely has zero ports enabled and the workaround for
> port_map == 0 is firing incorrectly.

Yeah, that's a workaround for very early ahci controllers.  I'll think
of a way to disable it for newer controllers.  Filtering on ahci
controller version should work.

Thanks.

-- 
tejun

[toc] | [prev] | [next] | [standalone]

#1309725 — [PATCH] libata: disable forced PORTS_IMPL for >= v3.0 controllers

Some early controllers incorrectly reported zero ports in PORTS_IMPL
register and the ahci driver fabricates PORTS_IMPL from the number of
ports in those cases.  This hasn't mattered but with the new nvme
controllers there are cases where zero PORTS_IMPL is valid and should
be honored.

Disable the workaround for controllers >= v3.0.

Signed-off-by: Tejun Heo <tj@kernel.org>
Reported-by: Andy Lutomirski <luto@amacapital.net>
Link: http://lkml.kernel.org/g/CALCETrU7yMvXEDhjAUShoHEhDwifJGapdw--BKxsP0jmjKGmRw@mail.gmail.com
---
Hello, Andy.

Can you please test whether this fixes the issue?

Thanks.

 drivers/ata/libahci.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/ata/libahci.c b/drivers/ata/libahci.c
index d61740e..4931075 100644
--- a/drivers/ata/libahci.c
+++ b/drivers/ata/libahci.c
@@ -496,8 +496,8 @@ void ahci_save_initial_config(struct device *dev, struct ahci_host_priv *hpriv)
 		}
 	}
 
-	/* fabricate port_map from cap.nr_ports */
-	if (!port_map) {
+	/* fabricate port_map from cap.nr_ports for older controllers */
+	if (!port_map && (vers >> 16) <= 2) {
 		port_map = (1 << ahci_nr_ports(cap)) - 1;
 		dev_warn(dev, "forcing PORTS_IMPL to 0x%x\n", port_map);
 

[toc] | [prev] | [next] | [standalone]

#1309795 — Re: [PATCH] libata: disable forced PORTS_IMPL for >= v3.0 controllers

On Thu, Jan 14, 2016 at 2:37 PM, Tejun Heo <tj@kernel.org> wrote:
> Some early controllers incorrectly reported zero ports in PORTS_IMPL
> register and the ahci driver fabricates PORTS_IMPL from the number of
> ports in those cases.  This hasn't mattered but with the new nvme
> controllers there are cases where zero PORTS_IMPL is valid and should
> be honored.
>
> Disable the workaround for controllers >= v3.0.

I still see:

[  +0.001350] ahci 0000:00:17.0: version 3.0
[  +0.000034] ahci 0000:00:17.0: forcing PORTS_IMPL to 0xff

My "vers" is 0x10301.

--Andy

[toc] | [prev] | [next] | [standalone]

#1310414 — Re: [PATCH] libata: disable forced PORTS_IMPL for >= v3.0 controllers

Hello,

On Thu, Jan 14, 2016 at 05:00:57PM -0800, Andy Lutomirski wrote:
> [  +0.001350] ahci 0000:00:17.0: version 3.0
> [  +0.000034] ahci 0000:00:17.0: forcing PORTS_IMPL to 0xff
> 
> My "vers" is 0x10301.

Ah, right, of course.  I was looking at the driver version, which I'm
not even sure why we have.  Will update the patch.

Thanks.

-- 
tejun

[toc] | [prev] | [next] | [standalone]

#1310446 — [PATCH v2] libata: disable forced PORTS_IMPL for >= AHCI 1.3

Some early controllers incorrectly reported zero ports in PORTS_IMPL
register and the ahci driver fabricates PORTS_IMPL from the number of
ports in those cases.  This hasn't mattered but with the new nvme
controllers there are cases where zero PORTS_IMPL is valid and should
be honored.

Disable the workaround for >= AHCI 1.3.

Signed-off-by: Tejun Heo <tj@kernel.org>
Reported-by: Andy Lutomirski <luto@amacapital.net>
Link: http://lkml.kernel.org/g/CALCETrU7yMvXEDhjAUShoHEhDwifJGapdw--BKxsP0jmjKGmRw@mail.gmail.com
---
Hello, Andy.

Can you please see whether this one works?

Thanks.

 drivers/ata/libahci.c |    5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/ata/libahci.c b/drivers/ata/libahci.c
index d61740e..a91432a 100644
--- a/drivers/ata/libahci.c
+++ b/drivers/ata/libahci.c
@@ -496,8 +496,9 @@ void ahci_save_initial_config(struct device *dev, struct ahci_host_priv *hpriv)
 		}
 	}
 
-	/* fabricate port_map from cap.nr_ports */
-	if (!port_map) {
+	/* fabricate port_map from cap.nr_ports for < AHCI 1.3 */
+	if (!port_map && (!(vers >> 16) ||
+			  ((vers >> 16) == 1 && (vers & 0xFFFF) < 0x300))) {
 		port_map = (1 << ahci_nr_ports(cap)) - 1;
 		dev_warn(dev, "forcing PORTS_IMPL to 0x%x\n", port_map);
 

[toc] | [prev] | [next] | [standalone]

#1310515 — Re: [PATCH v2] libata: disable forced PORTS_IMPL for >= AHCI 1.3

On Fri, Jan 15, 2016 at 12:13 PM, Tejun Heo <tj@kernel.org> wrote:
> Some early controllers incorrectly reported zero ports in PORTS_IMPL
> register and the ahci driver fabricates PORTS_IMPL from the number of
> ports in those cases.  This hasn't mattered but with the new nvme
> controllers there are cases where zero PORTS_IMPL is valid and should
> be honored.
>
> Disable the workaround for >= AHCI 1.3.

Tested-by: Andy Lutomirski <luto@kernel.org>

--Andy

[toc] | [prev] | [next] | [standalone]

#1310917 — Re: [PATCH v2] libata: disable forced PORTS_IMPL for >= AHCI 1.3

Hello.

On 1/15/2016 11:13 PM, Tejun Heo wrote:

> Some early controllers incorrectly reported zero ports in PORTS_IMPL
> register and the ahci driver fabricates PORTS_IMPL from the number of
> ports in those cases.  This hasn't mattered but with the new nvme
> controllers there are cases where zero PORTS_IMPL is valid and should
> be honored.
>
> Disable the workaround for >= AHCI 1.3.
>
> Signed-off-by: Tejun Heo <tj@kernel.org>
> Reported-by: Andy Lutomirski <luto@amacapital.net>
> Link: http://lkml.kernel.org/g/CALCETrU7yMvXEDhjAUShoHEhDwifJGapdw--BKxsP0jmjKGmRw@mail.gmail.com
> ---
> Hello, Andy.
>
> Can you please see whether this one works?
>
> Thanks.
>
>   drivers/ata/libahci.c |    5 +++--
>   1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/ata/libahci.c b/drivers/ata/libahci.c
> index d61740e..a91432a 100644
> --- a/drivers/ata/libahci.c
> +++ b/drivers/ata/libahci.c
> @@ -496,8 +496,9 @@ void ahci_save_initial_config(struct device *dev, struct ahci_host_priv *hpriv)
>   		}
>   	}
>
> -	/* fabricate port_map from cap.nr_ports */
> -	if (!port_map) {
> +	/* fabricate port_map from cap.nr_ports for < AHCI 1.3 */
> +	if (!port_map && (!(vers >> 16) ||
> +			  ((vers >> 16) == 1 && (vers & 0xFFFF) < 0x300))) {

     Hm, won't just (vers < 0x1300) just work?

MBR, Sergei

[toc] | [prev] | [next] | [standalone]

#1312396 — Re: [PATCH v2] libata: disable forced PORTS_IMPL for >= AHCI 1.3

On Sat, Jan 16, 2016 at 01:09:37PM +0300, Sergei Shtylyov wrote:
> >+	/* fabricate port_map from cap.nr_ports for < AHCI 1.3 */
> >+	if (!port_map && (!(vers >> 16) ||
> >+			  ((vers >> 16) == 1 && (vers & 0xFFFF) < 0x300))) {
> 
>     Hm, won't just (vers < 0x1300) just work?

lol, indeed.  Lemme update the patch.

Thanks.

-- 
tejun

[toc] | [prev] | [next] | [standalone]

#1312398 — Re: [PATCH v2] libata: disable forced PORTS_IMPL for >= AHCI 1.3

Hello.

On 01/19/2016 08:22 PM, Tejun Heo wrote:

>>> +	/* fabricate port_map from cap.nr_ports for < AHCI 1.3 */
>>> +	if (!port_map && (!(vers >> 16) ||
>>> +			  ((vers >> 16) == 1 && (vers & 0xFFFF) < 0x300))) {
>>
>>      Hm, won't just (vers < 0x1300) just work?
>
> lol, indeed.  Lemme update the patch.

    Oops. 0x10300, of course.

> Thanks.

MBR, Sergei

[toc] | [prev] | [next] | [standalone]

#1317291 — [PATCH v3] libata: disable forced PORTS_IMPL for >= AHCI 1.3

Hello,

Applied the following to libata/for-4.5-fixes.

Thanks.

------ 8< ------
From 566d1827df2ef0cbe921d3d6946ac3007b1a6938 Mon Sep 17 00:00:00 2001
From: Tejun Heo <tj@kernel.org>
Date: Fri, 15 Jan 2016 15:13:05 -0500
Subject: [PATCH] libata: disable forced PORTS_IMPL for >= AHCI 1.3

Some early controllers incorrectly reported zero ports in PORTS_IMPL
register and the ahci driver fabricates PORTS_IMPL from the number of
ports in those cases.  This hasn't mattered but with the new nvme
controllers there are cases where zero PORTS_IMPL is valid and should
be honored.

Disable the workaround for >= AHCI 1.3.

Signed-off-by: Tejun Heo <tj@kernel.org>
Reported-by: Andy Lutomirski <luto@amacapital.net>
Link: http://lkml.kernel.org/g/CALCETrU7yMvXEDhjAUShoHEhDwifJGapdw--BKxsP0jmjKGmRw@mail.gmail.com
Cc: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
Cc: stable@vger.kernel.org
---
 drivers/ata/libahci.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/ata/libahci.c b/drivers/ata/libahci.c
index 284a176..4029679 100644
--- a/drivers/ata/libahci.c
+++ b/drivers/ata/libahci.c
@@ -496,8 +496,8 @@ void ahci_save_initial_config(struct device *dev, struct ahci_host_priv *hpriv)
 		}
 	}
 
-	/* fabricate port_map from cap.nr_ports */
-	if (!port_map) {
+	/* fabricate port_map from cap.nr_ports for < AHCI 1.3 */
+	if (!port_map && vers < 0x10300) {
 		port_map = (1 << ahci_nr_ports(cap)) - 1;
 		dev_warn(dev, "forcing PORTS_IMPL to 0x%x\n", port_map);
 
-- 
2.5.0

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web