[PATCH] sata_via: PATA support, resubmit

[PATCH] sata_via: PATA support, resubmit

[Alan]

This is a clean version of the PATA support for the sata_via hardware.
I'm resubmitting it since nothing has happened since the last submission
despite promises of libata core changes. Given users actually need to use
this stuff today and the code is clean it should get merged irrespective
of any longer term plans for per channel operations structs and the like.

Signed-off-by: Alan Cox <alan@redhat.com>

diff -u --new-file --recursive --exclude-from /usr/src/exclude linux.vanilla-2.6.20-rc3-mm1/drivers/ata/sata_via.c linux-2.6.20-rc3-mm1/drivers/ata/sata_via.c
--- linux.vanilla-2.6.20-rc3-mm1/drivers/ata/sata_via.c	2007-01-05 13:09:36.000000000 +0000
+++ linux-2.6.20-rc3-mm1/drivers/ata/sata_via.c	2007-01-05 14:11:36.000000000 +0000
@@ -59,11 +59,14 @@
 	SATA_INT_GATE		= 0x41, /* SATA interrupt gating */
 	SATA_NATIVE_MODE	= 0x42, /* Native mode enable */
 	SATA_PATA_SHARING	= 0x49, /* PATA/SATA sharing func ctrl */
-
+	PATA_UDMA_TIMING	= 0xB3, /* PATA timing for DMA/ cable detect */
+	PATA_PIO_TIMING		= 0xAB, /* PATA timing register */
+	
 	PORT0			= (1 << 1),
 	PORT1			= (1 << 0),
 	ALL_PORTS		= PORT0 | PORT1,
-	N_PORTS			= 2,
+	PATA_PORT		= 2,	/* PATA is port 2 */
+	N_PORTS			= 3,
 
 	NATIVE_MODE_ALL		= (1 << 7) | (1 << 6) | (1 << 5) | (1 << 4),
 
@@ -75,6 +78,10 @@
 static u32 svia_scr_read (struct ata_port *ap, unsigned int sc_reg);
 static void svia_scr_write (struct ata_port *ap, unsigned int sc_reg, u32 val);
 static void vt6420_error_handler(struct ata_port *ap);
+static void vt6421_error_handler(struct ata_port *ap);
+static void vt6421_set_pio_mode(struct ata_port *ap, struct ata_device *adev);
+static void vt6421_set_dma_mode(struct ata_port *ap, struct ata_device *adev);
+static unsigned long vt6421_mode_filter(const struct ata_port *ap, struct ata_device *adev, unsigned long modes);
 
 static const struct pci_device_id svia_pci_tbl[] = {
 	{ PCI_VDEVICE(VIA, 0x0591), vt6420 },
@@ -140,8 +147,12 @@
 	.host_stop		= ata_host_stop,
 };
 
-static const struct ata_port_operations vt6421_sata_ops = {
+static const struct ata_port_operations vt6421_ata_ops = {
 	.port_disable		= ata_port_disable,
+	
+	.set_piomode		= vt6421_set_pio_mode,
+	.set_dmamode		= vt6421_set_dma_mode,
+	.mode_filter		= vt6421_mode_filter,
 
 	.tf_load		= ata_tf_load,
 	.tf_read		= ata_tf_read,
@@ -160,7 +171,7 @@
 
 	.freeze			= ata_bmdma_freeze,
 	.thaw			= ata_bmdma_thaw,
-	.error_handler		= ata_bmdma_error_handler,
+	.error_handler		= vt6421_error_handler,
 	.post_internal_cmd	= ata_bmdma_post_internal_cmd,
 
 	.irq_handler		= ata_interrupt,
@@ -278,6 +289,55 @@
 				  NULL, ata_std_postreset);
 }
 
+static int vt6421_prereset(struct ata_port *ap)
+{
+	struct pci_dev *pdev = to_pci_dev(ap->host->dev);
+	u8 tmp;
+	
+	if (ap->port_no != PATA_PORT) {
+		ap->cbl = ATA_CBL_SATA;
+		return 0;
+	}
+	pci_read_config_byte(pdev, PATA_UDMA_TIMING, &tmp);
+	
+	if (tmp & 0x10)
+		ap->cbl = ATA_CBL_PATA40;
+	else
+		ap->cbl = ATA_CBL_PATA80;
+	return 0;
+}
+
+static void vt6421_error_handler(struct ata_port *ap)
+{
+	return ata_bmdma_drive_eh(ap, vt6421_prereset, ata_std_softreset,
+				  NULL, ata_std_postreset);
+}
+
+static void vt6421_set_pio_mode(struct ata_port *ap, struct ata_device *adev)
+{
+	struct pci_dev *pdev = to_pci_dev(ap->host->dev);
+	static const u8 pio_bits[] = { 0xA8, 0x65, 0x65, 0x31, 0x20 };
+	if (ap->port_no == PATA_PORT)
+		pci_write_config_byte(pdev, PATA_PIO_TIMING, pio_bits[adev->pio_mode - XFER_PIO_0]);
+}
+
+static void vt6421_set_dma_mode(struct ata_port *ap, struct ata_device *adev)
+{
+	struct pci_dev *pdev = to_pci_dev(ap->host->dev);
+	static const u8 udma_bits[] = { 0xEE, 0xE8, 0xE6, 0xE4, 0xE2, 0xE1, 0xE0, 0xE0 };
+	if (ap->port_no == PATA_PORT)
+		pci_write_config_byte(pdev, PATA_UDMA_TIMING, udma_bits[adev->pio_mode - XFER_UDMA_0]);
+}
+
+static unsigned long vt6421_mode_filter(const struct ata_port *ap, struct ata_device *adev, unsigned long modes)
+{
+	if(ap->port_no == PATA_PORT) {
+		modes &= ~ATA_MASK_MWDMA;	/* No MWDMA support */
+		modes &= ~ (0x40 << ATA_SHIFT_UDMA);  /* UDMA 133 limited */
+	}
+	return modes;
+}
+
 static const unsigned int svia_bar_sizes[] = {
 	8, 4, 8, 4, 16, 256
 };
@@ -348,7 +408,7 @@
 
 	probe_ent->sht		= &svia_sht;
 	probe_ent->port_flags	= ATA_FLAG_SATA | ATA_FLAG_NO_LEGACY;
-	probe_ent->port_ops	= &vt6421_sata_ops;
+	probe_ent->port_ops	= &vt6421_ata_ops;
 	probe_ent->n_ports	= N_PORTS;
 	probe_ent->irq		= pdev->irq;
 	probe_ent->irq_flags	= IRQF_SHARED;
@@ -500,4 +560,3 @@
 
 module_init(svia_init);
 module_exit(svia_exit);
-

Re: [PATCH] sata_via: PATA support, resubmit

[Jeff Garzik]

Alan wrote:
> This is a clean version of the PATA support for the sata_via hardware.
> I'm resubmitting it since nothing has happened since the last submission
> despite promises of libata core changes. Given users actually need to use
> this stuff today and the code is clean it should get merged irrespective
> of any longer term plans for per channel operations structs and the like.
> 
> Signed-off-by: Alan Cox <alan@redhat.com>

Re-explanation since you missed it the first time:

Per-channel operations structs work /today/.

The problem you need to fix or work around is ata_probe_ent, which 
doesn't properly fill in ata_port info for this situation.  Tejun has 
posted patches that kill ata_probe_ent, which you were pointed to. 
Mikael Pettersson just posted a sata_promise example that uses 
->port_start to work around this problem, setting the cable type and 
ata_port::ops properly at runtime, based on SATA or PATA.  See "[RFC] 
sata_promise: handle TX2plus PATA locally", I believe you were CC'd on 
my response.

So, working code for both the short term workaround and long term fix 
exist /today/.

If you get the setup right, you don't bloat each hook with "is this port 
PATA?" tests.  At present, your sata_via patch introduces these needless 
tests.

	Jeff

Re: [PATCH] sata_via: PATA support, resubmit

[Alan]

> The problem you need to fix or work around is ata_probe_ent, which 
> doesn't properly fill in ata_port info for this situation.  Tejun has 
> posted patches that kill ata_probe_ent, which you were pointed to. 

And which are not yet in the main tree leaving many users unable to
install Linux. This isn't the way to get stuff done. When you've got the
new patches in the driver can use them if its worth it (which, see below,
I question).

> If you get the setup right, you don't bloat each hook with "is this port 
> PATA?" tests.  At present, your sata_via patch introduces these needless 
> tests.

Which I might note are actually smaller than the extra structs and on a
code path usually executed twice per boot. So the needless tests are more
efficient and not on a hot path and are shorter than the elegant
not-present solution and let users actually install Linux on current VIA
systems, which right now the generally cannot do.

Out of boredom I'll also note that clock timings say that if the extra
port info stuff causes a single extra L1 cache miss its also faster to do
the tests.

Alan

Re: [PATCH] sata_via: PATA support, resubmit

[Jeff Garzik]

Alan wrote:
>> The problem you need to fix or work around is ata_probe_ent, which 
>> doesn't properly fill in ata_port info for this situation.  Tejun has 
>> posted patches that kill ata_probe_ent, which you were pointed to. 
> 
> And which are not yet in the main tree leaving many users unable to
> install Linux.

The controllers affected by your patch have been around for well over a 
year.  I doubt a huge amount of suffering will be caused by pausing to 
get it right...  especially when you have been pointed at two working 
code examples that already get it right.


> This isn't the way to get stuff done. When you've got the
> new patches in the driver can use them if its worth it (which, see below,
> I question).

In Linux, we work /with/ the subsystem, not around it.

Your current approach is fundamentally flawed.  You can see this because 
e.g. a call to vt6421_ops::scr_read() will immediately oops, after your 
patch.

Just separate PATA and SATA operations.  That way everything works as 
expected, and you don't unintentionally add lovely oopses all over the 
place.

	Jeff

Re: [PATCH] sata_via: PATA support, resubmit

[Alan]

> Just separate PATA and SATA operations.  That way everything works as 
> expected, and you don't unintentionally add lovely oopses all over the 
> place.

Ok - based on your pointers to [ab]using port_start you want something
like this for now, with the port_start evaporating when Tejun's probe
changes go in ?

Signed-off-by: Alan Cox <alan@redhat.com>

diff -u --new-file --recursive --exclude-from /usr/src/exclude linux.vanilla-2.6.20-rc3-mm1/drivers/ata/sata_via.c linux-2.6.20-rc3-mm1/drivers/ata/sata_via.c
--- linux.vanilla-2.6.20-rc3-mm1/drivers/ata/sata_via.c	2007-01-05 13:09:36.000000000 +0000
+++ linux-2.6.20-rc3-mm1/drivers/ata/sata_via.c	2007-01-08 16:06:12.000000000 +0000
@@ -59,11 +59,14 @@
 	SATA_INT_GATE		= 0x41, /* SATA interrupt gating */
 	SATA_NATIVE_MODE	= 0x42, /* Native mode enable */
 	SATA_PATA_SHARING	= 0x49, /* PATA/SATA sharing func ctrl */
-
+	PATA_UDMA_TIMING	= 0xB3, /* PATA timing for DMA/ cable detect */
+	PATA_PIO_TIMING		= 0xAB, /* PATA timing register */
+	
 	PORT0			= (1 << 1),
 	PORT1			= (1 << 0),
 	ALL_PORTS		= PORT0 | PORT1,
-	N_PORTS			= 2,
+	PATA_PORT		= 2,	/* PATA is port 2 */
+	N_PORTS			= 3,
 
 	NATIVE_MODE_ALL		= (1 << 7) | (1 << 6) | (1 << 5) | (1 << 4),
 
@@ -75,6 +78,11 @@
 static u32 svia_scr_read (struct ata_port *ap, unsigned int sc_reg);
 static void svia_scr_write (struct ata_port *ap, unsigned int sc_reg, u32 val);
 static void vt6420_error_handler(struct ata_port *ap);
+static void vt6421_sata_error_handler(struct ata_port *ap);
+static void vt6421_pata_error_handler(struct ata_port *ap);
+static void vt6421_set_pio_mode(struct ata_port *ap, struct ata_device *adev);
+static void vt6421_set_dma_mode(struct ata_port *ap, struct ata_device *adev);
+static int vt6421_port_start(struct ata_port *ap);
 
 static const struct pci_device_id svia_pci_tbl[] = {
 	{ PCI_VDEVICE(VIA, 0x0591), vt6420 },
@@ -140,9 +148,43 @@
 	.host_stop		= ata_host_stop,
 };
 
-static const struct ata_port_operations vt6421_sata_ops = {
+static const struct ata_port_operations vt6421_pata_ops = {
 	.port_disable		= ata_port_disable,
+	
+	.set_piomode		= vt6421_set_pio_mode,
+	.set_dmamode		= vt6421_set_dma_mode,
+
+	.tf_load		= ata_tf_load,
+	.tf_read		= ata_tf_read,
+	.check_status		= ata_check_status,
+	.exec_command		= ata_exec_command,
+	.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,
+	.data_xfer		= ata_pio_data_xfer,
+
+	.freeze			= ata_bmdma_freeze,
+	.thaw			= ata_bmdma_thaw,
+	.error_handler		= vt6421_pata_error_handler,
+	.post_internal_cmd	= ata_bmdma_post_internal_cmd,
 
+	.irq_handler		= ata_interrupt,
+	.irq_clear		= ata_bmdma_irq_clear,
+
+	.port_start		= vt6421_port_start,
+	.port_stop		= ata_port_stop,
+	.host_stop		= ata_host_stop,
+};
+
+static const struct ata_port_operations vt6421_sata_ops = {
+	.port_disable		= ata_port_disable,
+	
 	.tf_load		= ata_tf_load,
 	.tf_read		= ata_tf_read,
 	.check_status		= ata_check_status,
@@ -160,7 +202,7 @@
 
 	.freeze			= ata_bmdma_freeze,
 	.thaw			= ata_bmdma_thaw,
-	.error_handler		= ata_bmdma_error_handler,
+	.error_handler		= vt6421_sata_error_handler,
 	.post_internal_cmd	= ata_bmdma_post_internal_cmd,
 
 	.irq_handler		= ata_interrupt,
@@ -169,7 +211,7 @@
 	.scr_read		= svia_scr_read,
 	.scr_write		= svia_scr_write,
 
-	.port_start		= ata_port_start,
+	.port_start		= vt6421_port_start,
 	.port_stop		= ata_port_stop,
 	.host_stop		= ata_host_stop,
 };
@@ -278,6 +320,63 @@
 				  NULL, ata_std_postreset);
 }
 
+static int vt6421_pata_prereset(struct ata_port *ap)
+{
+	struct pci_dev *pdev = to_pci_dev(ap->host->dev);
+	u8 tmp;
+
+	pci_read_config_byte(pdev, PATA_UDMA_TIMING, &tmp);
+	if (tmp & 0x10)
+		ap->cbl = ATA_CBL_PATA40;
+	else
+		ap->cbl = ATA_CBL_PATA80;
+	return 0;
+}
+
+static void vt6421_pata_error_handler(struct ata_port *ap)
+{
+	return ata_bmdma_drive_eh(ap, vt6421_pata_prereset, ata_std_softreset,
+				  NULL, ata_std_postreset);
+}
+
+static int vt6421_sata_prereset(struct ata_port *ap)
+{
+	ap->cbl = ATA_CBL_SATA;
+	return 0;
+}
+
+static void vt6421_sata_error_handler(struct ata_port *ap)
+{
+	return ata_bmdma_drive_eh(ap, vt6421_sata_prereset, ata_std_softreset,
+				  NULL, ata_std_postreset);
+}
+
+static void vt6421_set_pio_mode(struct ata_port *ap, struct ata_device *adev)
+{
+	struct pci_dev *pdev = to_pci_dev(ap->host->dev);
+	static const u8 pio_bits[] = { 0xA8, 0x65, 0x65, 0x31, 0x20 };
+	if (ap->port_no == PATA_PORT)
+		pci_write_config_byte(pdev, PATA_PIO_TIMING, pio_bits[adev->pio_mode - XFER_PIO_0]);
+}
+
+static void vt6421_set_dma_mode(struct ata_port *ap, struct ata_device *adev)
+{
+	struct pci_dev *pdev = to_pci_dev(ap->host->dev);
+	static const u8 udma_bits[] = { 0xEE, 0xE8, 0xE6, 0xE4, 0xE2, 0xE1, 0xE0, 0xE0 };
+	if (ap->port_no == PATA_PORT)
+		pci_write_config_byte(pdev, PATA_UDMA_TIMING, udma_bits[adev->pio_mode - XFER_UDMA_0]);
+}
+
+static int vt6421_port_start(struct ata_port *ap)
+{
+	if (ap->port_no == PATA_PORT) {
+		ap->ops = &vt6421_pata_ops;
+		ap->mwdma_mask = 0;
+		ap->flags = ATA_FLAG_SLAVE_POSS | ATA_FLAG_NO_LEGACY | ATA_FLAG_SRST;
+	}
+	return ata_port_start(ap);
+}
+
 static const unsigned int svia_bar_sizes[] = {
 	8, 4, 8, 4, 16, 256
 };
@@ -500,4 +599,3 @@
 
 module_init(svia_init);
 module_exit(svia_exit);
-

Re: [PATCH] sata_via: PATA support, resubmit

[Jeff Garzik]

Alan wrote:
>> Just separate PATA and SATA operations.  That way everything works as 
>> expected, and you don't unintentionally add lovely oopses all over the 
>> place.
> 
> Ok - based on your pointers to [ab]using port_start you want something
> like this for now, with the port_start evaporating when Tejun's probe
> changes go in ?
> 
> Signed-off-by: Alan Cox <alan@redhat.com>

Looks good to me, modulo minor comments below...



> +static void vt6421_set_pio_mode(struct ata_port *ap, struct ata_device *adev)
> +{
> +	struct pci_dev *pdev = to_pci_dev(ap->host->dev);
> +	static const u8 pio_bits[] = { 0xA8, 0x65, 0x65, 0x31, 0x20 };
> +	if (ap->port_no == PATA_PORT)
> +		pci_write_config_byte(pdev, PATA_PIO_TIMING, pio_bits[adev->pio_mode - XFER_PIO_0]);
> +}
> +
> +static void vt6421_set_dma_mode(struct ata_port *ap, struct ata_device *adev)
> +{
> +	struct pci_dev *pdev = to_pci_dev(ap->host->dev);
> +	static const u8 udma_bits[] = { 0xEE, 0xE8, 0xE6, 0xE4, 0xE2, 0xE1, 0xE0, 0xE0 };
> +	if (ap->port_no == PATA_PORT)
> +		pci_write_config_byte(pdev, PATA_UDMA_TIMING, udma_bits[adev->pio_mode - XFER_UDMA_0]);
> +}

You can kill the '== PATA_PORT' tests now.

	Jeff

Re: [PATCH] sata_via: PATA support, resubmit

[Alan]

> > Signed-off-by: Alan Cox <alan@redhat.com>
> 
> Looks good to me, modulo minor comments below...

New revision below

Signed-off-by: Alan Cox <alan@redhat.com>

diff -u --new-file --recursive --exclude-from /usr/src/exclude linux.vanilla-2.6.20-rc3-mm1/drivers/ata/sata_via.c linux-2.6.20-rc3-mm1/drivers/ata/sata_via.c
--- linux.vanilla-2.6.20-rc3-mm1/drivers/ata/sata_via.c	2007-01-05 13:09:36.000000000 +0000
+++ linux-2.6.20-rc3-mm1/drivers/ata/sata_via.c	2007-01-08 16:42:44.629970272 +0000
@@ -59,11 +59,14 @@
 	SATA_INT_GATE		= 0x41, /* SATA interrupt gating */
 	SATA_NATIVE_MODE	= 0x42, /* Native mode enable */
 	SATA_PATA_SHARING	= 0x49, /* PATA/SATA sharing func ctrl */
-
+	PATA_UDMA_TIMING	= 0xB3, /* PATA timing for DMA/ cable detect */
+	PATA_PIO_TIMING		= 0xAB, /* PATA timing register */
+	
 	PORT0			= (1 << 1),
 	PORT1			= (1 << 0),
 	ALL_PORTS		= PORT0 | PORT1,
-	N_PORTS			= 2,
+	PATA_PORT		= 2,	/* PATA is port 2 */
+	N_PORTS			= 3,
 
 	NATIVE_MODE_ALL		= (1 << 7) | (1 << 6) | (1 << 5) | (1 << 4),
 
@@ -75,6 +78,11 @@
 static u32 svia_scr_read (struct ata_port *ap, unsigned int sc_reg);
 static void svia_scr_write (struct ata_port *ap, unsigned int sc_reg, u32 val);
 static void vt6420_error_handler(struct ata_port *ap);
+static void vt6421_sata_error_handler(struct ata_port *ap);
+static void vt6421_pata_error_handler(struct ata_port *ap);
+static void vt6421_set_pio_mode(struct ata_port *ap, struct ata_device *adev);
+static void vt6421_set_dma_mode(struct ata_port *ap, struct ata_device *adev);
+static int vt6421_port_start(struct ata_port *ap);
 
 static const struct pci_device_id svia_pci_tbl[] = {
 	{ PCI_VDEVICE(VIA, 0x0591), vt6420 },
@@ -140,9 +148,43 @@
 	.host_stop		= ata_host_stop,
 };
 
-static const struct ata_port_operations vt6421_sata_ops = {
+static const struct ata_port_operations vt6421_pata_ops = {
 	.port_disable		= ata_port_disable,
+	
+	.set_piomode		= vt6421_set_pio_mode,
+	.set_dmamode		= vt6421_set_dma_mode,
+
+	.tf_load		= ata_tf_load,
+	.tf_read		= ata_tf_read,
+	.check_status		= ata_check_status,
+	.exec_command		= ata_exec_command,
+	.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,
+	.data_xfer		= ata_pio_data_xfer,
+
+	.freeze			= ata_bmdma_freeze,
+	.thaw			= ata_bmdma_thaw,
+	.error_handler		= vt6421_pata_error_handler,
+	.post_internal_cmd	= ata_bmdma_post_internal_cmd,
 
+	.irq_handler		= ata_interrupt,
+	.irq_clear		= ata_bmdma_irq_clear,
+
+	.port_start		= vt6421_port_start,
+	.port_stop		= ata_port_stop,
+	.host_stop		= ata_host_stop,
+};
+
+static const struct ata_port_operations vt6421_sata_ops = {
+	.port_disable		= ata_port_disable,
+	
 	.tf_load		= ata_tf_load,
 	.tf_read		= ata_tf_read,
 	.check_status		= ata_check_status,
@@ -160,7 +202,7 @@
 
 	.freeze			= ata_bmdma_freeze,
 	.thaw			= ata_bmdma_thaw,
-	.error_handler		= ata_bmdma_error_handler,
+	.error_handler		= vt6421_sata_error_handler,
 	.post_internal_cmd	= ata_bmdma_post_internal_cmd,
 
 	.irq_handler		= ata_interrupt,
@@ -169,7 +211,7 @@
 	.scr_read		= svia_scr_read,
 	.scr_write		= svia_scr_write,
 
-	.port_start		= ata_port_start,
+	.port_start		= vt6421_port_start,
 	.port_stop		= ata_port_stop,
 	.host_stop		= ata_host_stop,
 };
@@ -278,6 +320,61 @@
 				  NULL, ata_std_postreset);
 }
 
+static int vt6421_pata_prereset(struct ata_port *ap)
+{
+	struct pci_dev *pdev = to_pci_dev(ap->host->dev);
+	u8 tmp;
+
+	pci_read_config_byte(pdev, PATA_UDMA_TIMING, &tmp);
+	if (tmp & 0x10)
+		ap->cbl = ATA_CBL_PATA40;
+	else
+		ap->cbl = ATA_CBL_PATA80;
+	return 0;
+}
+
+static void vt6421_pata_error_handler(struct ata_port *ap)
+{
+	return ata_bmdma_drive_eh(ap, vt6421_pata_prereset, ata_std_softreset,
+				  NULL, ata_std_postreset);
+}
+
+static int vt6421_sata_prereset(struct ata_port *ap)
+{
+	ap->cbl = ATA_CBL_SATA;
+	return 0;
+}
+
+static void vt6421_sata_error_handler(struct ata_port *ap)
+{
+	return ata_bmdma_drive_eh(ap, vt6421_sata_prereset, ata_std_softreset,
+				  NULL, ata_std_postreset);
+}
+
+static void vt6421_set_pio_mode(struct ata_port *ap, struct ata_device *adev)
+{
+	struct pci_dev *pdev = to_pci_dev(ap->host->dev);
+	static const u8 pio_bits[] = { 0xA8, 0x65, 0x65, 0x31, 0x20 };
+	pci_write_config_byte(pdev, PATA_PIO_TIMING, pio_bits[adev->pio_mode - XFER_PIO_0]);
+}
+
+static void vt6421_set_dma_mode(struct ata_port *ap, struct ata_device *adev)
+{
+	struct pci_dev *pdev = to_pci_dev(ap->host->dev);
+	static const u8 udma_bits[] = { 0xEE, 0xE8, 0xE6, 0xE4, 0xE2, 0xE1, 0xE0, 0xE0 };
+	pci_write_config_byte(pdev, PATA_UDMA_TIMING, udma_bits[adev->pio_mode - XFER_UDMA_0]);
+}
+
+static int vt6421_port_start(struct ata_port *ap)
+{
+	if (ap->port_no == PATA_PORT) {
+		ap->ops = &vt6421_pata_ops;
+		ap->mwdma_mask = 0;
+		ap->flags = ATA_FLAG_SLAVE_POSS | ATA_FLAG_NO_LEGACY | ATA_FLAG_SRST;
+	}
+	return ata_port_start(ap);
+}
+
 static const unsigned int svia_bar_sizes[] = {
 	8, 4, 8, 4, 16, 256
 };
@@ -500,4 +597,3 @@
 
 module_init(svia_init);
 module_exit(svia_exit);
-

Re: [PATCH] sata_via: PATA support, resubmit

[Jeff Garzik]

Alan wrote:
>>> Signed-off-by: Alan Cox <alan@redhat.com>
>> Looks good to me, modulo minor comments below...
> 
> New revision below
> 
> Signed-off-by: Alan Cox <alan@redhat.com>

applied, thanks for your patience :)

Two procedural notes:

* you should include the patch description in each resend, so that 
scripts can automatically suck it into git changelog.

* yet again you added whitespace:


[jgarzik@pretzel libata-dev]$ git-am --signoff --utf8 /g/tmp/mbox

Applying 'sata_via: PATA support'

Adds trailing whitespace.
.dotest/patch:11:
Adds trailing whitespace.
.dotest/patch:40:
Adds trailing whitespace.
.dotest/patch:74:
warning: 3 lines add trailing whitespaces.
Wrote tree 76aff322aa7259d077e20f61b7405e3b16423a2e
Committed: 2041f6a9e6c927512023621413c6333ee104e6d5