sata_via: PATA support, resubmit

PATCH Applied Alan 2 participants 8 messages Jan 8, 2007 – Jan 9, 2007 libata

Share & tools

Alan

author

2007-01-08 12:16 UTC #0 lore raw

PATCH 1/1 sata_via: PATA support, resubmit

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 <[email protected]>

drivers/ata/sata_via.c +65 -6

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
@@ -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);
-

Jeff Garzik

2007-01-08 13:04 UTC #1 lore raw

Quoted 7 lines · 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 <[email protected]>

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

Alan

author

2007-01-08 15:32 UTC #2 lore raw

Quoted 3 lines

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).

Quoted 3 lines

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

Jeff Garzik

2007-01-08 15:51 UTC #3 lore raw

Quoted 4 lines · Alan

Quoted 1 line
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.

Quoted 3 lines

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

Alan

author

2007-01-08 16:30 UTC #4 lore raw

Quoted 3 lines

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 <[email protected]>

drivers/ata/sata_via.c +104 -6

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
@@ -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);
-

Jeff Garzik

2007-01-08 16:34 UTC #5 lore raw

Quoted 7 lines · Alan

Quoted 1 line
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 <[email protected]>

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

Quoted 5 lines

drivers/ata/sata_via.c +5 -0

+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)

drivers/ata/sata_via.c line 368

Quoted 10 lines

drivers/ata/sata_via.c +10 -0

+		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

Alan

author

2007-01-08 17:00 UTC #6 lore raw

Quoted 3 lines

Quoted 1 line
Signed-off-by Alan Cox <[email protected]>
Looks good to me, modulo minor comments below...

New revision below

Signed-off-by Alan Cox <[email protected]>

drivers/ata/sata_via.c +102 -6

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
@@ -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);
-

Jeff Garzik

Applied

2007-01-09 10:39 UTC #7 lore raw

Quoted 6 lines · Alan

Quoted 2 lines
Quoted 1 line
Signed-off-by Alan Cox <[email protected]>
Looks good to me, modulo minor comments below...
New revision below
Signed-off-by Alan Cox <[email protected]>

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