| Message ID | 4CF45BC0.2030701@iki.fi |
|---|---|
| State | Not Applicable |
| Delegated to: | David Miller |
| Headers | show |
Hello, On 11/30/2010 03:04 AM, Anssi Hannula wrote: > On 24.07.2010 17:53, Tejun Heo wrote: >> 88SE9128 can do FBS and sets it in HOST_CAP but forgets to set FBSCP >> in PORT_CMD. Implement AHCI_HFLAG_YES_FBS and apply it to 88SE9128. >> >> Signed-off-by: Tejun Heo <tj@kernel.org> > [...] >> diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c >> index f252253..41fa0a3 100644 > [...] >> @@ -362,6 +371,8 @@ static const struct pci_device_id ahci_pci_tbl[] = { >> /* Marvell */ >> { PCI_VDEVICE(MARVELL, 0x6145), board_ahci_mv }, /* 6145 */ >> { PCI_VDEVICE(MARVELL, 0x6121), board_ahci_mv }, /* 6121 */ >> + { PCI_DEVICE(0x1b4b, 0x9123), >> + .driver_data = board_ahci_yes_fbs }, /* 88se9128 */ > > The device contains an IDE interface as well, and the above > pci_device_id matches them both: > 08:00.0 SATA controller [0106]: Device [1b4b:9123] (rev 10) > 08:00.1 IDE interface [0101]: Device [1b4b:9123] (rev 10) *GASP* Come on Marvell.... :-( > This results in some (apparently harmless) mess [1]. > > Attached is a patch to make the id entry class-specific. The patch is > untested (the issue happens in a production machine). Thanks a lot for the patch. It looks correct but it would be great if it can be veified. Also, there's a pending patch to add another PCI ID for similar marvell controller. I wonder whether similar workaround should be applied. Hmmm... ISTR the other one reporting IDE class even though it works in IDE mode. Can someone with marvell documentation access check what's going on? Thanks.
On 30.11.2010 15:31, Tejun Heo wrote: > Hello, > > On 11/30/2010 03:04 AM, Anssi Hannula wrote: >> On 24.07.2010 17:53, Tejun Heo wrote: >>> 88SE9128 can do FBS and sets it in HOST_CAP but forgets to set FBSCP >>> in PORT_CMD. Implement AHCI_HFLAG_YES_FBS and apply it to 88SE9128. >>> >>> Signed-off-by: Tejun Heo <tj@kernel.org> >> [...] >>> diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c >>> index f252253..41fa0a3 100644 >> [...] >>> @@ -362,6 +371,8 @@ static const struct pci_device_id ahci_pci_tbl[] = { >>> /* Marvell */ >>> { PCI_VDEVICE(MARVELL, 0x6145), board_ahci_mv }, /* 6145 */ >>> { PCI_VDEVICE(MARVELL, 0x6121), board_ahci_mv }, /* 6121 */ >>> + { PCI_DEVICE(0x1b4b, 0x9123), >>> + .driver_data = board_ahci_yes_fbs }, /* 88se9128 */ >> >> The device contains an IDE interface as well, and the above >> pci_device_id matches them both: >> 08:00.0 SATA controller [0106]: Device [1b4b:9123] (rev 10) >> 08:00.1 IDE interface [0101]: Device [1b4b:9123] (rev 10) > > *GASP* Come on Marvell.... :-( > >> This results in some (apparently harmless) mess [1]. >> >> Attached is a patch to make the id entry class-specific. The patch is >> untested (the issue happens in a production machine). > > Thanks a lot for the patch. It looks correct but it would be great if > it can be veified. I think I can do that later, but probably not this week or so. > Also, there's a pending patch to add another PCI > ID for similar marvell controller. I wonder whether similar > workaround should be applied. Hmmm... ISTR the other one reporting > IDE class even though it works in IDE mode. IDE class in IDE mode? Isn't that kind of expected? :) Also, just to avoid any misunderstanding: I see both the SATA and IDE interfaces at the same time. > Can someone with marvell > documentation access check what's going on? > > Thanks. >
On 30.11.2010 22:10, Anssi Hannula wrote: > On 30.11.2010 15:31, Tejun Heo wrote: >> Hello, Hi! >> On 11/30/2010 03:04 AM, Anssi Hannula wrote: >>> On 24.07.2010 17:53, Tejun Heo wrote: >>>> 88SE9128 can do FBS and sets it in HOST_CAP but forgets to set FBSCP >>>> in PORT_CMD. Implement AHCI_HFLAG_YES_FBS and apply it to 88SE9128. >>>> >>>> Signed-off-by: Tejun Heo <tj@kernel.org> >>> [...] >>>> diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c >>>> index f252253..41fa0a3 100644 >>> [...] >>>> @@ -362,6 +371,8 @@ static const struct pci_device_id ahci_pci_tbl[] = { >>>> /* Marvell */ >>>> { PCI_VDEVICE(MARVELL, 0x6145), board_ahci_mv }, /* 6145 */ >>>> { PCI_VDEVICE(MARVELL, 0x6121), board_ahci_mv }, /* 6121 */ >>>> + { PCI_DEVICE(0x1b4b, 0x9123), >>>> + .driver_data = board_ahci_yes_fbs }, /* 88se9128 */ >>> >>> The device contains an IDE interface as well, and the above >>> pci_device_id matches them both: >>> 08:00.0 SATA controller [0106]: Device [1b4b:9123] (rev 10) >>> 08:00.1 IDE interface [0101]: Device [1b4b:9123] (rev 10) >> >> *GASP* Come on Marvell.... :-( >> >>> This results in some (apparently harmless) mess [1]. >>> >>> Attached is a patch to make the id entry class-specific. The patch is >>> untested (the issue happens in a production machine). >> >> Thanks a lot for the patch. It looks correct but it would be great if >> it can be veified. > > I think I can do that later, but probably not this week or so. I've now verified that the patch works on a system with 1b4b:9123 controller. >> Also, there's a pending patch to add another PCI >> ID for similar marvell controller. I wonder whether similar >> workaround should be applied. Hmmm... ISTR the other one reporting >> IDE class even though it works in IDE mode. > > IDE class in IDE mode? Isn't that kind of expected? :) > > Also, just to avoid any misunderstanding: I see both the SATA and IDE > interfaces at the same time. > >> Can someone with marvell >> documentation access check what's going on? >> >> Thanks. >> >
Hello, On 12/07/2010 08:45 PM, Anssi Hannula wrote: >>>> Attached is a patch to make the id entry class-specific. The patch is >>>> untested (the issue happens in a production machine). >>> >>> Thanks a lot for the patch. It looks correct but it would be great if >>> it can be veified. >> >> I think I can do that later, but probably not this week or so. > > I've now verified that the patch works on a system with 1b4b:9123 > controller. Thanks for verifying. Can you please post the kernel log? Also, do all ports seem to work? >>> Also, there's a pending patch to add another PCI >>> ID for similar marvell controller. I wonder whether similar >>> workaround should be applied. Hmmm... ISTR the other one reporting >>> IDE class even though it works in IDE mode. >> >> IDE class in IDE mode? Isn't that kind of expected? :) Oh, the latter should have been ahci. I'm not completely comfortable with going ahead with the patch tho. It probably is okay but it's still a shot in the dark. Is there anyone with marvell contact? Thanks.
On 12/08/2010 06:10 AM, Tejun Heo wrote: > Hello, > > On 12/07/2010 08:45 PM, Anssi Hannula wrote: >>>>> Attached is a patch to make the id entry class-specific. The patch is >>>>> untested (the issue happens in a production machine). >>>> >>>> Thanks a lot for the patch. It looks correct but it would be great if >>>> it can be veified. >>> >>> I think I can do that later, but probably not this week or so. >> >> I've now verified that the patch works on a system with 1b4b:9123 >> controller. > > Thanks for verifying. Can you please post the kernel log? Also, do > all ports seem to work? > >>>> Also, there's a pending patch to add another PCI >>>> ID for similar marvell controller. I wonder whether similar >>>> workaround should be applied. Hmmm... ISTR the other one reporting >>>> IDE class even though it works in IDE mode. >>> >>> IDE class in IDE mode? Isn't that kind of expected? :) > > Oh, the latter should have been ahci. > > I'm not completely comfortable with going ahead with the patch tho. > It probably is okay but it's still a shot in the dark. Is there > anyone with marvell contact? I have docs for most Marvell models, but not this one; and no contacts... Presumably it is some variant of their existing Marvell AHCI line of chips, but beyond that, you and I are both just guessing. Jeff -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hello, On 12/08/2010 07:09 PM, Jeff Garzik wrote: >> I'm not completely comfortable with going ahead with the patch tho. >> It probably is okay but it's still a shot in the dark. Is there >> anyone with marvell contact? > > I have docs for most Marvell models, but not this one; and no > contacts... Presumably it is some variant of their existing Marvell > AHCI line of chips, but beyond that, you and I are both just > guessing. Eh, well, in that case as it's a fix one way or the other, Acked-by: Tejun Heo <tj@kernel.org> Thanks.
On 08.12.2010 13:10, Tejun Heo wrote: > Hello, Hello! Sorry for the delay (again). > On 12/07/2010 08:45 PM, Anssi Hannula wrote: >>>>> Attached is a patch to make the id entry class-specific. The patch is >>>>> untested (the issue happens in a production machine). >>>> >>>> Thanks a lot for the patch. It looks correct but it would be great if >>>> it can be veified. >>> >>> I think I can do that later, but probably not this week or so. >> >> I've now verified that the patch works on a system with 1b4b:9123 >> controller. > > Thanks for verifying. Can you please post the kernel log? Also, do > all ports seem to work? Kernel log at boot time is attached. The 1b4b:9123 controllers are at 01:00.x, 06:00.x, 08:00.x, no disks attached to them. Both of the ports on the controllers seem to work.
On 12/08/2010 01:13 PM, Tejun Heo wrote: > Hello, > > On 12/08/2010 07:09 PM, Jeff Garzik wrote: >>> I'm not completely comfortable with going ahead with the patch tho. >>> It probably is okay but it's still a shot in the dark. Is there >>> anyone with marvell contact? >> >> I have docs for most Marvell models, but not this one; and no >> contacts... Presumably it is some variant of their existing Marvell >> AHCI line of chips, but beyond that, you and I are both just >> guessing. > > Eh, well, in that case as it's a fix one way or the other, > > Acked-by: Tejun Heo<tj@kernel.org> It's ugly, and maybe it will go away in the future. But I don't think it will harm existing installations to add the additional class match, so I'm applying it. Jeff -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From 5e4e904eb78d69528bc3b3ddd39a55579642bfbd Mon Sep 17 00:00:00 2001 From: Anssi Hannula <anssi.hannula@iki.fi> Date: Tue, 30 Nov 2010 03:43:15 +0200 Subject: [PATCH] ahci: do not match the IDE interface of 88SE9128 Commit 5f173107ecad83a50 added HFLAG_YES_FBS workaround for 88SE9128 (1b4b:9123). However, that change inadvertently caused the legacy IDE interface of the controller (with the same pci id) to become associated with the AHCI driver as well, causing the driver to try to bring the interface up in vain. Fix that by matching against class as well. Signed-off-by: Anssi Hannula <anssi.hannula@iki.fi> --- drivers/ata/ahci.c | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-) diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c index 3288263..791cf9b 100644 --- a/drivers/ata/ahci.c +++ b/drivers/ata/ahci.c @@ -379,6 +379,8 @@ static const struct pci_device_id ahci_pci_tbl[] = { { PCI_VDEVICE(MARVELL, 0x6145), board_ahci_mv }, /* 6145 */ { PCI_VDEVICE(MARVELL, 0x6121), board_ahci_mv }, /* 6121 */ { PCI_DEVICE(0x1b4b, 0x9123), + .class = PCI_CLASS_STORAGE_SATA_AHCI, + .class_mask = 0xffffff, .driver_data = board_ahci_yes_fbs }, /* 88se9128 */ /* Promise */ -- 1.7.3