[PATCH] sata_nv,ahci: add the ahci legacy mode support to sata_nv

[PATCH] sata_nv,ahci: add the ahci legacy mode support to sata_nv

[Peer Chen]

Add the ahci controller legacy mode support to sata_nv.
Move the DIDs of legacy mode from ahci.c to sata_nv.c

The patch base on kernel 2.6.23-rc8

Signed-off-by: Peer Chen <peerchen@gmail.com>
---
diff -uprN -X linux-2.6.23-rc8-vanilla/Documentation/dontdiff linux-2.6.23-rc8-vanilla/drivers/ata/ahci.c linux-2.6.23-rc8/drivers/ata/ahci.c
--- linux-2.6.23-rc8-vanilla/drivers/ata/ahci.c	2007-09-25 11:53:40.000000000 -0400
+++ linux-2.6.23-rc8/drivers/ata/ahci.c	2007-09-25 11:53:25.000000000 -0400
@@ -434,14 +434,6 @@ static const struct pci_device_id ahci_p
 	{ PCI_VDEVICE(NVIDIA, 0x044d), board_ahci },		/* MCP65 */
 	{ PCI_VDEVICE(NVIDIA, 0x044e), board_ahci },		/* MCP65 */
 	{ PCI_VDEVICE(NVIDIA, 0x044f), board_ahci },		/* MCP65 */
-	{ PCI_VDEVICE(NVIDIA, 0x045c), board_ahci },		/* MCP65 */
-	{ PCI_VDEVICE(NVIDIA, 0x045d), board_ahci },		/* MCP65 */
-	{ PCI_VDEVICE(NVIDIA, 0x045e), board_ahci },		/* MCP65 */
-	{ PCI_VDEVICE(NVIDIA, 0x045f), board_ahci },		/* MCP65 */
-	{ PCI_VDEVICE(NVIDIA, 0x0550), board_ahci },		/* MCP67 */
-	{ PCI_VDEVICE(NVIDIA, 0x0551), board_ahci },		/* MCP67 */
-	{ PCI_VDEVICE(NVIDIA, 0x0552), board_ahci },		/* MCP67 */
-	{ PCI_VDEVICE(NVIDIA, 0x0553), board_ahci },		/* MCP67 */
 	{ PCI_VDEVICE(NVIDIA, 0x0554), board_ahci },		/* MCP67 */
 	{ PCI_VDEVICE(NVIDIA, 0x0555), board_ahci },		/* MCP67 */
 	{ PCI_VDEVICE(NVIDIA, 0x0556), board_ahci },		/* MCP67 */
@@ -450,10 +442,6 @@ static const struct pci_device_id ahci_p
 	{ PCI_VDEVICE(NVIDIA, 0x0559), board_ahci },		/* MCP67 */
 	{ PCI_VDEVICE(NVIDIA, 0x055a), board_ahci },		/* MCP67 */
 	{ PCI_VDEVICE(NVIDIA, 0x055b), board_ahci },		/* MCP67 */
-	{ PCI_VDEVICE(NVIDIA, 0x07f0), board_ahci },		/* MCP73 */
-	{ PCI_VDEVICE(NVIDIA, 0x07f1), board_ahci },		/* MCP73 */
-	{ PCI_VDEVICE(NVIDIA, 0x07f2), board_ahci },		/* MCP73 */
-	{ PCI_VDEVICE(NVIDIA, 0x07f3), board_ahci },		/* MCP73 */
 	{ PCI_VDEVICE(NVIDIA, 0x07f4), board_ahci },		/* MCP73 */
 	{ PCI_VDEVICE(NVIDIA, 0x07f5), board_ahci },		/* MCP73 */
 	{ PCI_VDEVICE(NVIDIA, 0x07f6), board_ahci },		/* MCP73 */
@@ -462,10 +450,6 @@ static const struct pci_device_id ahci_p
 	{ PCI_VDEVICE(NVIDIA, 0x07f9), board_ahci },		/* MCP73 */
 	{ PCI_VDEVICE(NVIDIA, 0x07fa), board_ahci },		/* MCP73 */
 	{ PCI_VDEVICE(NVIDIA, 0x07fb), board_ahci },		/* MCP73 */
-	{ PCI_VDEVICE(NVIDIA, 0x0ad0), board_ahci },		/* MCP77 */
-	{ PCI_VDEVICE(NVIDIA, 0x0ad1), board_ahci },		/* MCP77 */
-	{ PCI_VDEVICE(NVIDIA, 0x0ad2), board_ahci },		/* MCP77 */
-	{ PCI_VDEVICE(NVIDIA, 0x0ad3), board_ahci },		/* MCP77 */
 	{ PCI_VDEVICE(NVIDIA, 0x0ad4), board_ahci },		/* MCP77 */
 	{ PCI_VDEVICE(NVIDIA, 0x0ad5), board_ahci },		/* MCP77 */
 	{ PCI_VDEVICE(NVIDIA, 0x0ad6), board_ahci },		/* MCP77 */
diff -uprN -X linux-2.6.23-rc8-vanilla/Documentation/dontdiff linux-2.6.23-rc8-vanilla/drivers/ata/sata_nv.c linux-2.6.23-rc8/drivers/ata/sata_nv.c
--- linux-2.6.23-rc8-vanilla/drivers/ata/sata_nv.c	2007-09-25 11:31:03.000000000 -0400
+++ linux-2.6.23-rc8/drivers/ata/sata_nv.c	2007-09-25 11:19:48.000000000 -0400
@@ -266,6 +266,7 @@ static void nv_adma_tf_read(struct ata_p
 enum nv_host_type
 {
 	GENERIC,
+	AHCI_LEG,
 	NFORCE2,
 	NFORCE3 = NFORCE2,	/* NF2 == NF3 as far as sata_nv is concerned */
 	CK804,
@@ -287,6 +288,29 @@ static const struct pci_device_id nv_pci
 	{ PCI_VDEVICE(NVIDIA, PCI_DEVICE_ID_NVIDIA_NFORCE_MCP61_SATA), GENERIC },
 	{ PCI_VDEVICE(NVIDIA, PCI_DEVICE_ID_NVIDIA_NFORCE_MCP61_SATA2), GENERIC },
 	{ PCI_VDEVICE(NVIDIA, PCI_DEVICE_ID_NVIDIA_NFORCE_MCP61_SATA3), GENERIC },
+	{ PCI_VDEVICE(NVIDIA, 0x45c), AHCI_LEG },	/* MCP65 */
+	{ PCI_VDEVICE(NVIDIA, 0x45d), AHCI_LEG },	/* MCP65 */
+	{ PCI_VDEVICE(NVIDIA, 0x45e), AHCI_LEG },	/* MCP65 */
+	{ PCI_VDEVICE(NVIDIA, 0x45f), AHCI_LEG },	/* MCP65 */
+	{ PCI_VDEVICE(NVIDIA, 0x550), AHCI_LEG },	/* MCP67 */
+	{ PCI_VDEVICE(NVIDIA, 0x551), AHCI_LEG },	/* MCP67 */
+	{ PCI_VDEVICE(NVIDIA, 0x552), AHCI_LEG },	/* MCP67 */
+	{ PCI_VDEVICE(NVIDIA, 0x553), AHCI_LEG },	/* MCP67 */
+	{ PCI_VDEVICE(NVIDIA, 0x7f0), AHCI_LEG },	/* MCP73 */
+	{ PCI_VDEVICE(NVIDIA, 0x7f1), AHCI_LEG },	/* MCP73 */
+	{ PCI_VDEVICE(NVIDIA, 0x7f2), AHCI_LEG },	/* MCP73 */
+	{ PCI_VDEVICE(NVIDIA, 0x7f3), AHCI_LEG },	/* MCP73 */
+	{ PCI_VDEVICE(NVIDIA, 0xad0), AHCI_LEG },	/* MCP77 */
+	{ PCI_VDEVICE(NVIDIA, 0xad1), AHCI_LEG },	/* MCP77 */
+	{ PCI_VDEVICE(NVIDIA, 0xad2), AHCI_LEG },	/* MCP77 */
+	{ PCI_VDEVICE(NVIDIA, 0xad3), AHCI_LEG },	/* MCP77 */
+	{ PCI_VDEVICE(NVIDIA, 0xab4), AHCI_LEG },	/* MCP79 */
+	{ PCI_VDEVICE(NVIDIA, 0xab5), AHCI_LEG },	/* MCP79 */
+	{ PCI_VDEVICE(NVIDIA, 0xab6), AHCI_LEG },	/* MCP79 */
+	{ PCI_VDEVICE(NVIDIA, 0xab7), AHCI_LEG },	/* MCP79 */	
+	{ PCI_VENDOR_ID_NVIDIA, PCI_ANY_ID,
+		PCI_ANY_ID, PCI_ANY_ID,
+		PCI_CLASS_STORAGE_IDE<<8, 0xffff00, AHCI_LEG },
 
 	{ } /* terminate list */
 };
@@ -365,6 +389,30 @@ static const struct ata_port_operations 
 	.port_start		= ata_port_start,
 };
 
+static const struct ata_port_operations nv_ahci_leg_ops = {
+	.port_disable		= ata_port_disable,
+	.tf_load		= ata_tf_load,
+	.tf_read		= ata_tf_read,
+	.exec_command		= ata_exec_command,
+	.check_status		= ata_check_status,
+	.dev_select		= ata_std_dev_select,
+	.bmdma_setup		= ata_bmdma_setup,
+	.bmdma_start		= ata_bmdma_start,
+	.bmdma_stop		= ata_bmdma_stop,
+	.bmdma_status		= ata_bmdma_status,
+	.qc_prep		= ata_qc_prep,
+	.qc_issue		= ata_qc_issue_prot,
+	.freeze			= ata_bmdma_freeze,
+	.thaw			= ata_bmdma_thaw,
+	.error_handler		= nv_error_handler,
+	.post_internal_cmd	= ata_bmdma_post_internal_cmd,
+	.data_xfer		= ata_data_xfer,
+	.irq_clear		= ata_bmdma_irq_clear,
+	.irq_on			= ata_irq_on,
+	.irq_ack		= ata_irq_ack,
+	.port_start		= ata_port_start,
+};
+
 static const struct ata_port_operations nv_nf2_ops = {
 	.port_disable		= ata_port_disable,
 	.tf_load		= ata_tf_load,
@@ -463,6 +511,17 @@ static const struct ata_port_info nv_por
 		.port_ops	= &nv_generic_ops,
 		.irq_handler	= nv_generic_interrupt,
 	},
+	/* ahci legacy */
+	{
+		.sht		= &nv_sht,
+		.flags		= ATA_FLAG_SATA | ATA_FLAG_SLAVE_POSS |
+				  ATA_FLAG_HRST_TO_RESUME,
+		.pio_mask	= NV_PIO_MASK,
+		.mwdma_mask	= NV_MWDMA_MASK,
+		.udma_mask	= NV_UDMA_MASK,
+		.port_ops	= &nv_ahci_leg_ops,
+		.irq_handler	= nv_generic_interrupt,
+	},
 	/* nforce2/3 */
 	{
 		.sht		= &nv_sht,
-
 				
--------------
Peer Chen
2007-09-25

Re: [PATCH] sata_nv,ahci: add the ahci legacy mode support to sata_nv

[Jeff Garzik]

Peer Chen wrote:
> Add the ahci controller legacy mode support to sata_nv.
> Move the DIDs of legacy mode from ahci.c to sata_nv.c
> 
> The patch base on kernel 2.6.23-rc8
> 
> Signed-off-by: Peer Chen <peerchen@gmail.com>

I don't understand why these are being moved?

If an interface can be driven via the AHCI driver, that is greatly 
preferred over the sata_nv driver.

	Jeff

Re: Re: [PATCH] sata_nv,ahci: add the ahci legacy mode support to sata_nv

[Peer Chen]

We have three mode for one controller - IDE/RAID/AHCI, we want sata_nv being load when user select the IDE mode in BIOS, load ahci driver if RAID/AHCI being selected, which will verify if our legacy mode work well and have additional option if there is any
bug for the ahci mode.

----------------				 
Peer Chen
2007-09-25

-------------------------------------------------------------
·¢¼þÈË£ºJeff Garzik
·¢ËÍÈÕÆÚ£º2007-09-25 15:08:45
ÊÕ¼þÈË£ºPeer Chen
³­ËÍ£ºlinux-kernel; linux-ide; akpm
Ö÷Ì⣺Re: [PATCH] sata_nv,ahci: add the ahci legacy mode support to sata_nv

Peer Chen wrote:
> Add the ahci controller legacy mode support to sata_nv.
> Move the DIDs of legacy mode from ahci.c to sata_nv.c
> 
> The patch base on kernel 2.6.23-rc8
> 
> Signed-off-by: Peer Chen <peerchen@gmail.com>

I don't understand why these are being moved?

If an interface can be driven via the AHCI driver, that is greatly 
preferred over the sata_nv driver.

	Jeff

Re: [PATCH] sata_nv,ahci: add the ahci legacy mode support to sata_nv

[Jeff Garzik]

Peer Chen wrote:
> We have three mode for one controller - IDE/RAID/AHCI, we want sata_nv being load when user select the IDE mode in BIOS, load ahci driver if RAID/AHCI being selected, which will verify if our legacy mode work well and have additional option if there is any
> bug for the ahci mode.

I understand that logic, but look at what happens in practice:

1) User installs new OS in AHCI mode.  Distro updates initramfs (loaded 
at kernel boot time, with boot drivers) to include ahci driver.
2) User reboots into BIOS setup, and switches from AHCI mode to IDE mode.
3) BIOS setup reboots computer.
4) OS kernel and initramfs image are loaded.  ahci driver load fails.
5) User is left without a bootable system.

The same situation happens in reverse, if you install in IDE mode 
(sata_nv in initramfs), and then switch to AHCI/RAID mode.

Additionally, AHCI provides better performance and more direct exposure 
to the SATA frames.  This is key for supporting many modern SATA 
features that cannot be accessed via IDE legacy mode.  AHCI lacks 
in-silicon simulation of an IDE interface, which time has shown is a 
less stable, edge-case-prone approach to SATA.

I do not find the "verify nvidia's legacy mode works" argument 
compelling; that is not the kernel's job, nor the user's.  And if there 
is an AHCI silicon bug, let us deal with that when such a bug appears.

Overall, AFAICS this patch -introduces- new ways for the user to easily 
render their systems unbootable.

	Jeff

Re: Re: [PATCH] sata_nv,ahci: add the ahci legacy mode support to sata_nv

[Peer Chen]

Yes,I hear what you are saying but user should know what they are setting in BIOS,there are lots of ways to change the BIOS setting result in unbootable system not only change AHCI/IDE mode. If they encounter booting failure after changing the BIOS setting,they should restore it.
Using legacy driver for legacy mode won't affect user to enjoy the feature of AHCI,just select AHCI/RAID mode will ok.
As I know, Intel did it in the same way,and I think it's reasonable.


------------------				 
Peer Chen
2007-09-25

-------------------------------------------------------------
·¢¼þÈË£ºJeff Garzik
·¢ËÍÈÕÆÚ£º2007-09-25 16:13:52
ÊÕ¼þÈË£ºPeer Chen
³­ËÍ£ºlinux-kernel; linux-ide; akpm
Ö÷Ì⣺Re: [PATCH] sata_nv,ahci: add the ahci legacy mode support to sata_nv

Peer Chen wrote:
> We have three mode for one controller - IDE/RAID/AHCI, we want sata_nv being load when user select the IDE mode in BIOS, load ahci driver if RAID/AHCI being selected, which will verify if our legacy mode work well and have additional option if there is any
> bug for the ahci mode.

I understand that logic, but look at what happens in practice:

1) User installs new OS in AHCI mode.  Distro updates initramfs (loaded 
at kernel boot time, with boot drivers) to include ahci driver.
2) User reboots into BIOS setup, and switches from AHCI mode to IDE mode.
3) BIOS setup reboots computer.
4) OS kernel and initramfs image are loaded.  ahci driver load fails.
5) User is left without a bootable system.

The same situation happens in reverse, if you install in IDE mode 
(sata_nv in initramfs), and then switch to AHCI/RAID mode.

Additionally, AHCI provides better performance and more direct exposure 
to the SATA frames.  This is key for supporting many modern SATA 
features that cannot be accessed via IDE legacy mode.  AHCI lacks 
in-silicon simulation of an IDE interface, which time has shown is a 
less stable, edge-case-prone approach to SATA.

I do not find the "verify nvidia's legacy mode works" argument 
compelling; that is not the kernel's job, nor the user's.  And if there 
is an AHCI silicon bug, let us deal with that when such a bug appears.

Overall, AFAICS this patch -introduces- new ways for the user to easily 
render their systems unbootable.

	Jeff

Re: [PATCH] sata_nv,ahci: add the ahci legacy mode support to sata_nv

[Jeff Garzik]

Peer Chen wrote:
> Add the ahci controller legacy mode support to sata_nv.
> Move the DIDs of legacy mode from ahci.c to sata_nv.c
> 
> The patch base on kernel 2.6.23-rc8
> 
> Signed-off-by: Peer Chen <peerchen@gmail.com>

Would you mind checking libata-dev.git#upstream, and make sure it has 
all the NV patches?

I'm thinking I should go ahead and push the 'nv-swncq' branch, which 
contains the sata_nv updates for swncq.  They have been in -mm for a 
while.  I am leaning towards leaving the default to 'off' for 2.6.24 though.

Comments welcome...

	Jeff

Re: [PATCH] sata_nv,ahci: add the ahci legacy mode support to sata_nv

[peer chen]

I hope one of the following patches can be merged to 2.6.24.
==========================
http://lkml.org/lkml/2007/10/8/93
http://lkml.org/lkml/2007/9/25/20

Yes, I agree to set the 'swncq' as default for 2.6.24, after all, for
our server customers, stability is far more important than the new
feature no matter the problem is caused by drive or controller.


2007/10/13, Jeff Garzik <jeff@garzik.org>:
> Peer Chen wrote:
> > Add the ahci controller legacy mode support to sata_nv.
> > Move the DIDs of legacy mode from ahci.c to sata_nv.c
> >
> > The patch base on kernel 2.6.23-rc8
> >
> > Signed-off-by: Peer Chen <peerchen@gmail.com>
>
> Would you mind checking libata-dev.git#upstream, and make sure it has
> all the NV patches?
>
> I'm thinking I should go ahead and push the 'nv-swncq' branch, which
> contains the sata_nv updates for swncq.  They have been in -mm for a
> while.  I am leaning towards leaving the default to 'off' for 2.6.24 though.
>
> Comments welcome...
>
>        Jeff
>
>
>
>

Re: [PATCH] sata_nv,ahci: add the ahci legacy mode support to sata_nv

[Jeff Garzik]

peer chen wrote:
> I hope one of the following patches can be merged to 2.6.24.
> ==========================
> http://lkml.org/lkml/2007/10/8/93
> http://lkml.org/lkml/2007/9/25/20

Unfortunately I do not feel like this is the right course of action.

Experience from Intel platforms tells us that our users get very unhappy 
when their silicon supports AHCI mode, but they are forced into using a 
less-performant mode.  A popular example is an <unnamed> OEM whose BIOS 
had no method whatsoever for enabling AHCI -- didn't even program the 
PCI BAR -- even though tests showed the AHCI mode worked just fine when 
manually programmed.

AHCI is more likely to provide a /stable/ Serial ATA experience, because 
the silicon deals primarily with sending and receiving FIS's, and not 
much else.  In constrast, experience has shown the legacy IDE interface 
to be a less reliable method of SATA support.  And certainly AHCI is 
much, much faster with less per-command overhead.

Given that AHCI is both faster and more stable, I feel it is the best 
policy to enable AHCI when the hardware supports it, regardless of PCI 
class code (IDE, SATA, or RAID).


> Yes, I agree to set the 'swncq' as default for 2.6.24, after all, for
> our server customers, stability is far more important than the new
> feature no matter the problem is caused by drive or controller.

Agreed.  Done!

	Jeff

Re: [PATCH] sata_nv,ahci: add the ahci legacy mode support to sata_nv

[peer chen]

Ok,I agree to use AHCI driver for our AHCI controllers no matter their
class codes are IDE/RAID/AHCI. But for those new or upcoming AHCI
controller which DIDs are not included in ahci.c and also IDE/RAID
mode being set in BIOS, no driver will be loaded currently, so I hope
the first patch http://lkml.org/lkml/2007/10/8/93 can be appied for
this case. Any comments?

2007/10/19, Jeff Garzik <jeff@garzik.org>:
> peer chen wrote:
> > I hope one of the following patches can be merged to 2.6.24.
> > ==========================
> > http://lkml.org/lkml/2007/10/8/93
> > http://lkml.org/lkml/2007/9/25/20
>
> Unfortunately I do not feel like this is the right course of action.
>
> Experience from Intel platforms tells us that our users get very unhappy
> when their silicon supports AHCI mode, but they are forced into using a
> less-performant mode.  A popular example is an <unnamed> OEM whose BIOS
> had no method whatsoever for enabling AHCI -- didn't even program the
> PCI BAR -- even though tests showed the AHCI mode worked just fine when
> manually programmed.
>
> AHCI is more likely to provide a /stable/ Serial ATA experience, because
> the silicon deals primarily with sending and receiving FIS's, and not
> much else.  In constrast, experience has shown the legacy IDE interface
> to be a less reliable method of SATA support.  And certainly AHCI is
> much, much faster with less per-command overhead.
>
> Given that AHCI is both faster and more stable, I feel it is the best
> policy to enable AHCI when the hardware supports it, regardless of PCI
> class code (IDE, SATA, or RAID).
>
>
> > Yes, I agree to set the 'swncq' as default for 2.6.24, after all, for
> > our server customers, stability is far more important than the new
> > feature no matter the problem is caused by drive or controller.
>
> Agreed.  Done!
>
>        Jeff
>
>
>
>
>

Re: [PATCH] sata_nv,ahci: add the ahci legacy mode support to sata_nv

[Jeff Garzik]

peer chen wrote:
> Ok,I agree to use AHCI driver for our AHCI controllers no matter their
> class codes are IDE/RAID/AHCI. But for those new or upcoming AHCI
> controller which DIDs are not included in ahci.c and also IDE/RAID
> mode being set in BIOS, no driver will be loaded currently, so I hope
> the first patch http://lkml.org/lkml/2007/10/8/93 can be appied for
> this case. Any comments?

hmmm is that the correct URL?  That one updates sata_nv to support AHCI 
controllers?.

I would think you would want to add the RAID class code to ahci.c 
instead?  And just some regular PCI IDs?

	Jeff

Re: [PATCH] sata_nv,ahci: add the ahci legacy mode support to sata_nv

[peer chen]

Yes, link - http://lkml.org/lkml/2007/10/8/93 add the AHCI legacy
support to sata_nv when IDE/RAID mode been set in SBIOS and Device IDs
are not in ahci.c at this moment. To do so, when a new chipset come
out and DIDs haven't been submited to LKML,user still can use ahci
driver to handle it when setting AHCI mode in BIOS, using sata_nv when
setting RAID/IDE in BIOS.
For example, ahci driver in Fedora 7 doesn't include the DIDs of
MCP78, so if you set the IDE/RAID mode in BIOS, os installation onto
SATA drive will fail. But if Fedora 7 include this patch, installation
will be successful.

2007/10/19, Jeff Garzik <jeff@garzik.org>:
> peer chen wrote:
> > Ok,I agree to use AHCI driver for our AHCI controllers no matter their
> > class codes are IDE/RAID/AHCI. But for those new or upcoming AHCI
> > controller which DIDs are not included in ahci.c and also IDE/RAID
> > mode being set in BIOS, no driver will be loaded currently, so I hope
> > the first patch http://lkml.org/lkml/2007/10/8/93 can be appied for
> > this case. Any comments?
>
> hmmm is that the correct URL?  That one updates sata_nv to support AHCI
> controllers?.
>
> I would think you would want to add the RAID class code to ahci.c
> instead?  And just some regular PCI IDs?
>
>        Jeff
>
>
>

Re: [PATCH] sata_nv,ahci: add the ahci legacy mode support to sata_nv

[Jeff Garzik]

peer chen wrote:
> Yes, link - http://lkml.org/lkml/2007/10/8/93 add the AHCI legacy
> support to sata_nv when IDE/RAID mode been set in SBIOS and Device IDs
> are not in ahci.c at this moment. To do so, when a new chipset come
> out and DIDs haven't been submited to LKML,user still can use ahci
> driver to handle it when setting AHCI mode in BIOS, using sata_nv when
> setting RAID/IDE in BIOS.
> For example, ahci driver in Fedora 7 doesn't include the DIDs of
> MCP78, so if you set the IDE/RAID mode in BIOS, os installation onto
> SATA drive will fail. But if Fedora 7 include this patch, installation
> will be successful.

The problem with this change is that sata_nv picks it up through the 
default entry, and then later on, ahci picks it up as well.

Rather than the confusion of "which driver is the user running on this 
hardware?" it is easier to take steps to ensure the user is always 
running in AHCI mode.

Therefore, I think the best idea is to follow the same policy as 
drivers/net/forcedeth.c (Ayaz @ NVIDIA) and Intel AHCI, and simply 
provide the individual PCI IDs in advance of product release.

If you wish to avoid this, then take steps to make sure 
drivers/ata/ahci.c has the appropriate wildcard entry in its 
pci_device_id table.

Really, look at the sata_nv bug reports that keep popping up.  We want 
to move away from that driver long term, and always drive the SATA 
interface in AHCI mode.

The proposed sata_nv patch does the opposite -- guarantees we must 
support the continually problematic legacy IDE interface ad infinitum. 
Such patches are OK for the test lab, but in this specific case users 
/suffer/ when not running AHCI mode.

	Jeff

Re: [PATCH] sata_nv,ahci: add the ahci legacy mode support to sata_nv

[Jeff Garzik]

Jeff Garzik wrote:
> The proposed sata_nv patch does the opposite -- guarantees we must 
> support the continually problematic legacy IDE interface ad infinitum. 
> Such patches are OK for the test lab, but in this specific case users 
> /suffer/ when not running AHCI mode.

Just to reinforce...

sata_nv support and bug fixes are primarily done right now through the 
valiant efforts of Robert Hancock (with assists from Alan, Tejun, and 
others).

Robert's job is difficult, because he has no hardware documentation[1], 
and NVIDIA does not seem to be helping out much with driver bug reports 
on the lists or in bugzillas.

As far as I know, I am the only one in the universe outside of NVIDIA 
with any SATA docs at all, and those docs _only_ cover ADMA registers 
and DMA structures, no PCI config info, no errata, nothing on SWNCQ or 
legacy IDE (well, half a page).

NVIDIA has indeed become more engaged in sata_nv in recent times, and 
that's a positive sign.  You, Kuon and Ayaz have all been noticeably 
more responsive in email.  Thanks.  Users have definitely benefited, 
particularly from your help addressing a couple SWNCQ issues.

But at this point in time, being asked to choose between sata_nv and 
ahci is no choice at all.  One has public documentation, wide industry 
support and little-or-no bugs.  The other has several open issues, no 
documentation, and support obstacles.

	Jeff


[1] Robert, please correct me if I'm wrong...

Re: [PATCH] sata_nv,ahci: add the ahci legacy mode support to sata_nv

[Robert Hancock]

Jeff Garzik wrote:
> Jeff Garzik wrote:
>> The proposed sata_nv patch does the opposite -- guarantees we must 
>> support the continually problematic legacy IDE interface ad infinitum. 
>> Such patches are OK for the test lab, but in this specific case users 
>> /suffer/ when not running AHCI mode.
> 
> Just to reinforce...
> 
> sata_nv support and bug fixes are primarily done right now through the 
> valiant efforts of Robert Hancock (with assists from Alan, Tejun, and 
> others).
> 
> Robert's job is difficult, because he has no hardware documentation[1], 
> and NVIDIA does not seem to be helping out much with driver bug reports 
> on the lists or in bugzillas.

Right, I don't have anything. Unless the original incomplete ADMA driver 
release from NVIDIA counts as documentation, lol.

And yes, I've CC'ed NVIDIA people about a few ADMA-related issues and 
been met with silence. It would be nice if they were as responsive about 
ADMA issues as I must say Kuan and Peer have been on the SWNCQ side of 
things..

> 
> As far as I know, I am the only one in the universe outside of NVIDIA 
> with any SATA docs at all, and those docs _only_ cover ADMA registers 
> and DMA structures, no PCI config info, no errata, nothing on SWNCQ or 
> legacy IDE (well, half a page).
> 
> NVIDIA has indeed become more engaged in sata_nv in recent times, and 
> that's a positive sign.  You, Kuon and Ayaz have all been noticeably 
> more responsive in email.  Thanks.  Users have definitely benefited, 
> particularly from your help addressing a couple SWNCQ issues.
> 
> But at this point in time, being asked to choose between sata_nv and 
> ahci is no choice at all.  One has public documentation, wide industry 
> support and little-or-no bugs.  The other has several open issues, no 
> documentation, and support obstacles.

They're not even equivalent interfaces in this case, in the proposed 
AHCI legacy mode patch these controllers are supported in the default 
SFF mode only, no ADMA or SWNCQ, so you don't get any NCQ support..

-- 
Robert Hancock      Saskatoon, SK, Canada
To email, remove "nospam" from hancockr@nospamshaw.ca
Home Page: http://www.roberthancock.com/

RE: [PATCH] sata_nv,adma: fix error when rmmod sata_nv

[Kuan Luo]

hi,
The below error happens when i rmmod sata_nv in adma mode on ck804
chipset with 2.6.24 kernel.
I traced the code and found that the driver attempts to write device mem
that has been unmapped.

Only simply removing the code" writew(0, mmio + NV_ADMA_CTL);" in the
nv_adma_port_stop function or remove .port_stop field in nv_adma_ops,
rmmod is ok.

static void nv_adma_port_stop(struct ata_port *ap)
{
	struct nv_adma_port_priv *pp = ap->private_data;
	void __iomem *mmio = pp->ctl_block;

	VPRINTK("ENTER\n");
-	writew(0, mmio + NV_ADMA_CTL);
}

Or
 Place pcim_iomap_regions before ata_pci_prepare_native_host in
nv_init_one function.
This can guarantee that the code "writew(0, mmio + NV_ADMA_CTL) " write
device mem before the device mem is unmapped.

messages:
Dec 19 02:23:24 localhost kernel: ata16.00: disabled
Dec 19 02:23:24 localhost kernel: sd 15:0:0:0: [sda] Synchronizing SCSI
cache
Dec 19 02:23:24 localhost kernel: sd 15:0:0:0: [sda] Result:
hostbyte=0x04 driverbyte=0x00
Dec 19 02:23:24 localhost kernel: sd 15:0:0:0: [sda] Stopping disk
Dec 19 02:23:24 localhost kernel: sd 15:0:0:0: [sda] START_STOP FAILED
Dec 19 02:23:24 localhost kernel: sd 15:0:0:0: [sda] Result:
hostbyte=0x04 driverbyte=0x00
Dec 19 02:23:24 localhost kernel: BUG: unable to handle kernel paging
request at virtual address e881a4c0
Dec 19 02:23:24 localhost kernel: printing eip: e88224e9 *pde = 0180c067
*pte = 00000000 
Dec 19 02:23:24 localhost kernel: Oops: 0002 [#1] SMP 
Dec 19 02:23:24 localhost kernel: Modules linked in: sata_nv libata
Dec 19 02:23:24 localhost kernel: 
Dec 19 02:23:24 localhost kernel: Pid: 4915, comm: rmmod Not tainted
(2.6.24-rc1-ge2e031eb #2)
Dec 19 02:23:24 localhost kernel: EIP: 0060:[<e88224e9>] EFLAGS:
00210292 CPU: 1
Dec 19 02:23:24 localhost kernel: EIP is at nv_adma_port_stop+0x3d/0x53
[sata_nv]
Dec 19 02:23:24 localhost kernel: EAX: 00000015 EBX: e881a480 ECX:
00200046 EDX: 00200046
Dec 19 02:23:24 localhost kernel: ESI: c220b50c EDI: c1979154 EBP:
c197904c ESP: c2429ecc
Dec 19 02:23:24 localhost kernel:  DS: 007b ES: 007b FS: 00d8 GS: 0033
SS: 0068
Dec 19 02:23:24 localhost kernel: Process rmmod (pid: 4915, ti=c2428000
task=c25e5a40 task.ti=c2428000)
Dec 19 02:23:24 localhost kernel: Stack: e8823e1b e881a480 00000000
e884398e c220b500 c18a3ec0 c1979154 00000000 
Dec 19 02:23:24 localhost kernel:        c02818be c1979154 c197904c
0000000d c18a3ec0 c18a3a40 c197904c e88259d0 
Dec 19 02:23:24 localhost kernel:        e88259d0 c2428000 c0281987
00200286 c197904c c027f8bb c1834600 c197904c 
Dec 19 02:23:24 localhost kernel: Call Trace:
Dec 19 02:23:24 localhost kernel:  [<e884398e>]
ata_host_release+0x2f/0x92 [libata]
Dec 19 02:23:24 localhost kernel:  [<c02818be>]
release_nodes+0x10f/0x12f
Dec 19 02:23:24 localhost kernel:  [<c0281987>]
devres_release_all+0x27/0x2a
Dec 19 02:23:24 localhost kernel:  [<c027f8bb>]
__device_release_driver+0x72/0x88
Dec 19 02:23:24 localhost kernel:  [<c027fccb>] driver_detach+0x76/0xb3
Dec 19 02:23:24 localhost kernel:  [<c027f49f>]
bus_remove_driver+0x63/0x81
Dec 19 02:23:24 localhost kernel:  [<c0228b5a>]
pci_unregister_driver+0xc/0x57
Dec 19 02:23:24 localhost kernel:  [<c0143792>]
sys_delete_module+0x197/0x1bd
Dec 19 02:23:24 localhost kernel:  [<c0417673>]
do_page_fault+0x202/0x581
Dec 19 02:23:24 localhost kernel:  [<c01570ec>] do_munmap+0x193/0x1ac
Dec 19 02:23:24 localhost kernel:  [<c0104e4e>]
sysenter_past_esp+0x5f/0x85
Dec 19 02:23:24 localhost kernel:  =======================
Dec 19 02:23:24 localhost kernel: Code: 3e 82 e8 e8 b8 30 90 d7 89 5c 24
04 c7 04 24 0f 3e 82 e8 e8 a8 30 90 d7 8b 5b 10 c7 04 24 1b 3e 82 e8 89
5c 24 04 e8 95 30 90 d7 <66> c7 43 40 00 00 c7 04 24 29 3e 82 e8 e8 83
30 90 d7 5b 58 5b 
Dec 19 02:23:24 localhost kernel: EIP: [<e88224e9>]
nv_adma_port_stop+0x3d/0x53 [sata_nv] SS:ESP 0068:c2429ecc
-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain
confidential information.  Any unauthorized review, use, disclosure or distribution
is prohibited.  If you are not the intended recipient, please contact the sender by
reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------

Re: [PATCH] sata_nv,adma: fix error when rmmod sata_nv

[Tejun Heo]

Kuan Luo wrote:
> hi,
> The below error happens when i rmmod sata_nv in adma mode on ck804
> chipset with 2.6.24 kernel.
> I traced the code and found that the driver attempts to write device mem
> that has been unmapped.
> 
> Only simply removing the code" writew(0, mmio + NV_ADMA_CTL);" in the
> nv_adma_port_stop function or remove .port_stop field in nv_adma_ops,
> rmmod is ok.
> 
> static void nv_adma_port_stop(struct ata_port *ap)
> {
> 	struct nv_adma_port_priv *pp = ap->private_data;
> 	void __iomem *mmio = pp->ctl_block;
> 
> 	VPRINTK("ENTER\n");
> -	writew(0, mmio + NV_ADMA_CTL);
> }
> 
> Or
>  Place pcim_iomap_regions before ata_pci_prepare_native_host in
> nv_init_one function.
> This can guarantee that the code "writew(0, mmio + NV_ADMA_CTL) " write
> device mem before the device mem is unmapped.

Which kernel version are you using?  The following commit should have
fixed the problem.  Please give a shot at 2.6.24-rc5.  Thanks.

commit 32ebbc0c0d5d18c0135b55d1eb0029f48c54aff0
Author: Tejun Heo <htejun@gmail.com>
Date:   Thu Nov 8 13:09:00 2007 +0900

libata: port and host should be stopped before hardware resources are
released

-- 
tejun

RE: [PATCH] sata_nv,adma: fix error when rmmod sata_nv

[Kuan Luo]

Tejun Heo  wrote:
> Which kernel version are you using?  The following commit should have
> fixed the problem.  Please give a shot at 2.6.24-rc5.  Thanks.
> 
> commit 32ebbc0c0d5d18c0135b55d1eb0029f48c54aff0
> Author: Tejun Heo <htejun@gmail.com>
> Date:   Thu Nov 8 13:09:00 2007 +0900
> 
> libata: port and host should be stopped before hardware resources are
> released
> 
> -- 
> tejun
> 
I have seen your commit. And i apologize for not updating the newest
kernel.
-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain
confidential information.  Any unauthorized review, use, disclosure or distribution
is prohibited.  If you are not the intended recipient, please contact the sender by
reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------