88SE9172 additional PCI ID: 1b4b:9172

88SE9172 additional PCI ID: 1b4b:9172

[George Spelvin]

On a Gigabyte X79-UP4 motherboard, which is documented as having
3x 88SE9172, lspci -nn gives me:

05:00.0 SATA controller [0106]: Marvell Technology Group Ltd. 88SE9172 SATA 6Gb/s Controller [1b4b:9172] (rev 11)
06:00.0 SATA controller [0106]: Marvell Technology Group Ltd. 88SE9172 SATA 6Gb/s Controller [1b4b:9172] (rev 11)
07:00.0 SATA controller [0106]: Marvell Technology Group Ltd. 88SE9172 SATA 6Gb/s Controller [1b4b:9172] (rev 11)
08:00.0 SATA controller [0106]: Marvell Technology Group Ltd. 88SE9123 PCIe SATA 6.0 Gb/s controller [1b4b:9123] (rev 11)

(The fourth is an expansion card I have pluggied into a PCIE x1 slot.)

drivers/ata/ahci.c lists:
	{ PCI_DEVICE(0x1b4b, 0x917a),
	  .driver_data = board_ahci_yes_fbs },	/* 88se9172 */
	{ PCI_DEVICE(0x1b4b, 0x9192),
	  .driver_data = board_ahci_yes_fbs },	/* 88se9172 on some Gigabyte */

Is that second one a typo, or do we need a third line for the 0x9172 PCI ID?
For now, I have added the third line and it's mostly working.

(If I enable VT-d and the IOMMU, the devices are recognized but not the
disks plugged into them, but I'm still working on figuring that out.)

Re: 88SE9172 additional PCI ID: 1b4b:9172

[Tejun Heo]

On Mon, May 27, 2013 at 07:31:17PM -0400, George Spelvin wrote:
> On a Gigabyte X79-UP4 motherboard, which is documented as having
> 3x 88SE9172, lspci -nn gives me:
> 
> 05:00.0 SATA controller [0106]: Marvell Technology Group Ltd. 88SE9172 SATA 6Gb/s Controller [1b4b:9172] (rev 11)
> 06:00.0 SATA controller [0106]: Marvell Technology Group Ltd. 88SE9172 SATA 6Gb/s Controller [1b4b:9172] (rev 11)
> 07:00.0 SATA controller [0106]: Marvell Technology Group Ltd. 88SE9172 SATA 6Gb/s Controller [1b4b:9172] (rev 11)
> 08:00.0 SATA controller [0106]: Marvell Technology Group Ltd. 88SE9123 PCIe SATA 6.0 Gb/s controller [1b4b:9123] (rev 11)
> 
> (The fourth is an expansion card I have pluggied into a PCIE x1 slot.)
> 
> drivers/ata/ahci.c lists:
> 	{ PCI_DEVICE(0x1b4b, 0x917a),
> 	  .driver_data = board_ahci_yes_fbs },	/* 88se9172 */
> 	{ PCI_DEVICE(0x1b4b, 0x9192),
> 	  .driver_data = board_ahci_yes_fbs },	/* 88se9172 on some Gigabyte */

I don't think it's a typo.

> Is that second one a typo, or do we need a third line for the 0x9172 PCI ID?
> For now, I have added the third line and it's mostly working.

And we'll need a separate entry

> (If I enable VT-d and the IOMMU, the devices are recognized but not the
> disks plugged into them, but I'm still working on figuring that out.)

when the device gets working.

Thanks!

-- 
tejun

Re: 88SE9172 additional PCI ID: 1b4b:9172

[George Spelvin]

> I don't think it's a typo.

Indeed, pci.ids has three entries.

> And we'll need a separate entry
>
>> (If I enable VT-d and the IOMMU, the devices are recognized but not the
>> disks plugged into them, but I'm still working on figuring that out.)
>
> when the device gets working.

I don't quite understand this.  With VT-d disabled in the BIOS,
the devices *are* working.  I even have a port multiplier plugged
into one.  (Which gets all confused by hdparm -m16, but whatever.)

When I get time, I'm going to play with various BIOS and kernel
options to narrow down the problem.  But that's for later.

Can you explain what you think is not working now?

Re: 88SE9172 additional PCI ID: 1b4b:9172

[Tejun Heo]

Hello,

On Tue, May 28, 2013 at 03:12:20AM -0400, George Spelvin wrote:
> > when the device gets working.
> 
> I don't quite understand this.  With VT-d disabled in the BIOS,
> the devices *are* working.  I even have a port multiplier plugged
> into one.  (Which gets all confused by hdparm -m16, but whatever.)
> 
> When I get time, I'm going to play with various BIOS and kernel
> options to narrow down the problem.  But that's for later.
> 
> Can you explain what you think is not working now?

I just misunderstood what you were saying in the previous email.  Can
you please post a proper patch with SOB?

Thanks.

-- 
tejun

[PATCH] Use observed PCI ID for Marvell 88se9172 SATA controller.

[George Spelvin]

A third possible PCI ID, as personally observed, and found in the
pci.ids list.

Signed-off-by: George Spelvin <linux@horizon.com>
---
 drivers/ata/ahci.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
index 6a67b07..d41437a 100644
--- a/drivers/ata/ahci.c
+++ b/drivers/ata/ahci.c
@@ -421,6 +421,8 @@ static const struct pci_device_id ahci_pci_tbl[] = {
 	  .driver_data = board_ahci_yes_fbs },			/* 88se9128 */
 	{ PCI_DEVICE(0x1b4b, 0x9125),
 	  .driver_data = board_ahci_yes_fbs },			/* 88se9125 */
+	{ PCI_DEVICE(0x1b4b, 0x9172),
+	  .driver_data = board_ahci_yes_fbs },			/* 88se9172 */
 	{ PCI_DEVICE(0x1b4b, 0x917a),
 	  .driver_data = board_ahci_yes_fbs },			/* 88se9172 */
 	{ PCI_DEVICE(0x1b4b, 0x9192),
-- 
1.8.3.rc3

[PATCH] ahci: add an observed PCI ID for Marvell 88se9172 SATA controller

[Tejun Heo]

Applied to libata/for-3.10-fixes.  In the future, please base patches
on top of git://git.kernel.org/pub/scm/linux/kernel/git/tj/libata.git
for-XXX branches.

Thanks.

>From fcce9a35f8faaa1f52236c554ef1b15d99a7537e Mon Sep 17 00:00:00 2001
From: George Spelvin <linux@horizon.com>
Date: Wed, 29 May 2013 10:20:35 +0900

A third possible PCI ID, as personally observed, and found in the
pci.ids list.

Signed-off-by: George Spelvin <linux@horizon.com>
Signed-off-by: Tejun Heo <tj@kernel.org>
---
 drivers/ata/ahci.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
index 2180876..2b50dfd 100644
--- a/drivers/ata/ahci.c
+++ b/drivers/ata/ahci.c
@@ -423,6 +423,8 @@ static const struct pci_device_id ahci_pci_tbl[] = {
 	  .driver_data = board_ahci_yes_fbs },			/* 88se9125 */
 	{ PCI_DEVICE(PCI_VENDOR_ID_MARVELL_EXT, 0x917a),
 	  .driver_data = board_ahci_yes_fbs },			/* 88se9172 */
+	{ PCI_DEVICE(PCI_VENDOR_ID_MARVELL_EXT, 0x9172),
+	  .driver_data = board_ahci_yes_fbs },			/* 88se9172 */
 	{ PCI_DEVICE(PCI_VENDOR_ID_MARVELL_EXT, 0x9192),
 	  .driver_data = board_ahci_yes_fbs },			/* 88se9172 on some Gigabyte */
 	{ PCI_DEVICE(PCI_VENDOR_ID_MARVELL_EXT, 0x91a3),
-- 
1.8.1.4

Re: [PATCH] ahci: add an observed PCI ID for Marvell 88se9172 SATA controller

[George Spelvin]

> Applied to libata/for-3.10-fixes.  In the future, please base patches
> on top of git://git.kernel.org/pub/scm/linux/kernel/git/tj/libata.git
> for-XXX branches.

As you saw, it didn't initially occur to me to prepare a patch at all;
it seemed such a trivial copy/paste/one-character-edit exercise.
After diagnosis, the most difficult part was writing a meaningful
commit message.

Which I presumed would be easier for you; a lot of the difficulty for me
was trying to figure out one line that would fit the ongoing narrative
that is the kernel changelog.

I was actually surprised when you asked for a full patch, so I just sent
you a copy of my private commit, with an improved commit message and S-o-b.
(Should I have added "Cc: stable" to it?)

But since I now know you prefer even trivial changes like this in formal
patch form, I'll try to do it right in future.

Re: 88SE9172 additional PCI ID: 1b4b:9172

[Robert Hancock]

On 05/28/2013 01:12 AM, George Spelvin wrote:
>> I don't think it's a typo.
>
> Indeed, pci.ids has three entries.
>
>> And we'll need a separate entry
>>
>>> (If I enable VT-d and the IOMMU, the devices are recognized but not the
>>> disks plugged into them, but I'm still working on figuring that out.)
>>
>> when the device gets working.
>
> I don't quite understand this.  With VT-d disabled in the BIOS,
> the devices *are* working.  I even have a port multiplier plugged
> into one.  (Which gets all confused by hdparm -m16, but whatever.)
>
> When I get time, I'm going to play with various BIOS and kernel
> options to narrow down the problem.  But that's for later.
>
> Can you explain what you think is not working now?

There are some known problems with some Marvell controllers and VT-d, 
apparently they issue PCIe requests containing the wrong device function 
causing IOMMU faults to occur. I believe a PCI quirk to work around the 
problem was either being worked on or finished - if it's in already, it 
might need this device ID added to it as well.

Re: [PATCH] ahci: add an observed PCI ID for Marvell 88se9172 SATA controller

[Tejun Heo]

Hello,

On Wed, May 29, 2013 at 08:36:27AM -0400, George Spelvin wrote:
> As you saw, it didn't initially occur to me to prepare a patch at all;
> it seemed such a trivial copy/paste/one-character-edit exercise.
> After diagnosis, the most difficult part was writing a meaningful
> commit message.
> 
> Which I presumed would be easier for you; a lot of the difficulty for me
> was trying to figure out one line that would fit the ongoing narrative
> that is the kernel changelog.
> 
> I was actually surprised when you asked for a full patch, so I just sent
> you a copy of my private commit, with an improved commit message and S-o-b.
> (Should I have added "Cc: stable" to it?)
> 
> But since I now know you prefer even trivial changes like this in formal
> patch form, I'll try to do it right in future.

It's partly for convenience on my part but more to attribute changes
correctly.  This is something you did so if at all possible I'd like
you to take the credit and (however minute it may be) the accompanying
responsibility.  It doesn't have to be perfect and I'm happy to edit
the commit messages / patches as necessary and adding cc to stable.
Well, at least unless you're gonna be submitting large volume of
patches.

Thanks.

-- 
tejun