Skip to contentSkip to conversation

Resurrect IT8172 IDE controller driver

PATCH Superseded Shane McDonald 2 participants 4 messages Dec 31, 2008 – Jan 2, 2009

Subsystems (2)
PCI libata

v4 of 5 Superseded Latest is v5 →

lore
Share & tools
lore

Shane McDonald

author
#0 lore raw

PATCH v4 1/1 Resurrect IT8172 IDE controller driver

Support for the IT8172 IDE controller was removed from the kernel sometime after 2.6.18. Support for the only boards that used the IT8172 was removed from the kernel after 2.6.18, as they had never compiled since 2.6.0. However, there are a couple of platforms that use this
chip: the PMC-Sierra Xiao Hu thin-client computer, which is no longer in production, and the Linksys NSS4000 Network Attached Storage box, which is based on the Xiao Hu board. I am attempting to add support for the Xiao Hu to the kernel, and this IT8172 IDE controller is the first bit of code in this effort.

This patch resurrects the IT8172 IDE controller code. I began with the 2.6.18 version of the it8172.c file, and have moved it forward so that it works with the latest version of the kernel. I have run this driver on a PMC-Sierra Xiao Hu board with the 2.6.28 kernel, and I have had no problems with it in my configuration. The attached patch applies cleanly against 2.6.28.

Signed-off-by Shane McDonald <[email protected]>
---
 drivers/ide/Kconfig     |    7 +
 drivers/ide/Makefile    |    1
 drivers/ide/it8172.c    |  173 ++++++++++++++++++++++++++++++++++++++
 include/linux/pci_ids.h |    1
 4 files changed, 182 insertions(+)

Changes since version 3 of the patch:
- corrected settings of RTx and PWx for PIO mode 1
- removed code duplication for different drives when setting PIO modes
- simplified test for IORDY support
- simplified calculation of bit settings for UDMA modes
- removed remnants of support for SWDMA
- corrected a coding style issue

drivers/ide/Kconfig +7 -0
diff -uprN a/drivers/ide/Kconfig b/drivers/ide/Kconfig
@@ -524,6 +524,13 @@ config BLK_DEV_PIIX
 	  This allows the kernel to change PIO, DMA and UDMA speeds and to
 	  configure the chip to optimum performance.
 
+config BLK_DEV_IT8172
+	tristate "IT8172 IDE support"
+	select BLK_DEV_IDEDMA_PCI
+	help
+	  This driver adds support for the IDE controller on the
+	  IT8172 System Controller.
+
 config BLK_DEV_IT8213
 	tristate "IT8213 IDE support"
 	select BLK_DEV_IDEDMA_PCI
drivers/ide/Makefile +1 -0
diff -uprN a/drivers/ide/Makefile b/drivers/ide/Makefile
@@ -46,6 +46,7 @@ obj-$(CONFIG_BLK_DEV_SC1200)		+= sc1200.
 obj-$(CONFIG_BLK_DEV_CY82C693)		+= cy82c693.o
 obj-$(CONFIG_BLK_DEV_DELKIN)		+= delkin_cb.o
 obj-$(CONFIG_BLK_DEV_HPT366)		+= hpt366.o
+obj-$(CONFIG_BLK_DEV_IT8172)		+= it8172.o
 obj-$(CONFIG_BLK_DEV_IT8213)		+= it8213.o
 obj-$(CONFIG_BLK_DEV_IT821X)		+= it821x.o
 obj-$(CONFIG_BLK_DEV_JMICRON)		+= jmicron.o
drivers/ide/it8172.c +173 -0

173 lines not on this page · raw message

include/linux/pci_ids.h +1 -0
diff -uprN a/include/linux/pci_ids.h b/include/linux/pci_ids.h
@@ -1658,6 +1658,7 @@
 #define PCI_VENDOR_ID_ROCKWELL		0x127A
 
 #define PCI_VENDOR_ID_ITE		0x1283
+#define PCI_DEVICE_ID_ITE_8172		0x8172
 #define PCI_DEVICE_ID_ITE_8211		0x8211
 #define PCI_DEVICE_ID_ITE_8212		0x8212
 #define PCI_DEVICE_ID_ITE_8213		0x8213
Acked-by on v4 1/1
#1 lore raw

Hello.

Quoted 10 lines · Shane McDonald

Support for the IT8172 IDE controller was removed from the kernel sometime after 2.6.18. Support for the only boards that used the IT8172 was removed from the kernel after 2.6.18, as they had never compiled since 2.6.0. However, there are a couple of platforms that use this
chip: the PMC-Sierra Xiao Hu thin-client computer, which is no longer in production, and the Linksys NSS4000 Network Attached Storage box, which is based on the Xiao Hu board. I am attempting to add support for the Xiao Hu to the kernel, and this IT8172 IDE controller is the first bit of code in this effort.

Quoted 9 lines

This patch resurrects the IT8172 IDE controller code. I began with the 2.6.18 version of the it8172.c file, and have moved it forward so that it works with the latest version of the kernel. I have run this driver on a PMC-Sierra Xiao Hu board with the 2.6.28 kernel, and I have had no problems with it in my configuration. The attached patch applies cleanly against 2.6.28.

Signed-off-by Shane McDonald <[email protected]>
Acked-by Sergei Shtylyov <[email protected]>
Quoted 5 lines

diff -uprN a/drivers/ide/it8172.c b/drivers/ide/it8172.c

drivers/ide/it8172.c line 10

Quoted 10 lines
drivers/ide/it8172.c +10 -0
@@ -0,0 +1,173 @@
+static void it8172_set_pio_mode(ide_drive_t *drive, const u8 pio)
+{
+	ide_hwif_t *hwif	= HWIF(drive);
+	struct pci_dev *dev	= to_pci_dev(hwif->dev);
+	u16 drive_enables;
+	u32 drive_timing;
+
+				     /* RTx  PWx */
+	static const u8 timings[][2] = {
+					{ 7, 7 },

By a "happy concidence" these are also an optimal MWDMA0 timings
(PIO0 is considerably slower).

drivers/ide/it8172.c line 59

Quoted 11 lines
drivers/ide/it8172.c +10 -0
+					{ 7, 4 },
+					{ 3, 3 },
+					{ 2, 2 },
+					{ 1, 2 }, };
+	/*
+	 * The highest value of DIOR/DIOW pulse width and recovery time
+	 * that can be set in the IT8172 is 8 PCI clock cycles.  As a result,
+	 * it cannot be configured for PIO mode 0.  This table sets these
+	 * parameters to the maximum supported by the IT8172.
+	 */

This comment seems somewhat mispalced...

Quoted 12 lines
drivers/ide/it8172.c +11 -0
+	pci_read_config_word(dev, 0x40, &drive_enables);
+	pci_read_config_dword(dev, 0x44, &drive_timing);
+
+	/*
+	 * Enable port 0x44. The IT8172 spec is confused; it calls
+	 * this register the "Slave IDE Timing Register", but in fact,
+	 * it controls timing for both master and slave drives.
+	 */
+	drive_enables |= 0x4000;
+
+	drive_enables &= drive->dn ? 0xc006 : 0xc060;

drivers/ide/it8172.c line 81

Quoted 10 lines
drivers/ide/it8172.c +10 -0
+	if (drive->media == ide_disk)
+		/* enable prefetch */
+		drive_enables |= 0x0004 << (drive->dn * 4);
+	if (ata_id_has_iordy(drive->id))
+		/* enable IORDY sample-point */
+		drive_enables |= 0x0002 << (drive->dn * 4);
+
+	drive_timing &= drive->dn ? 0x00003f00 : 0x000fc000;
+	drive_timing |= ((timings[pio][0] << 11) | (timings[pio][1] << 8))
+			<< (drive->dn * 6);

This can be somewhat shortened:

	drive_timing |= ((timings[pio][0] << 3) | timings[pio][1]) <<
			(drive->dn * 6 + 8);

Could've be shortened even more if timings[] was single-dimensional...

MBR, Sergei

author
#2 lore raw

Sergei,

Quoted 3 lines · Sergei Shtylyov
Quoted 1 line · Shane McDonald
Signed-off-by Shane McDonald <[email protected]>
Acked-by Sergei Shtylyov <[email protected]>

Please forgive my ignorance -- this is my first non-trivial patch... I have made a new patch incorporating your suggestions as described below. Do I include your Acked-by line in my re-spun patch, or do I need you to Ack it again?
I will post the new patch when I receive guidance on this.

Quoted 8 lines
Quoted 8 lines

diff -uprN a/drivers/ide/it8172.c b/drivers/ide/it8172.c

drivers/ide/it8172.c 2008-12-30 20:55:29.000000000 -0600 renamed from drivers/ide/it8172.c 1969-12-31 18:00:00.000000000 -0600 +5 -0
+                                       { 7, 4 },
+                                       { 3, 3 },
+                                       { 2, 2 },
+                                       { 1, 2 }, };
+       /*
Quoted 8 lines

drivers/ide/it8172.c 2008-12-30 20:55:29.000000000 -0600

Quoted 6 lines
drivers/ide/it8172.c 2008-12-30 20:55:29.000000000 -0600 renamed from drivers/ide/it8172.c 1969-12-31 18:00:00.000000000 -0600 +2 -0
+        * The highest value of DIOR/DIOW pulse width and recovery time
+        * that can be set in the IT8172 is 8 PCI clock cycles.  As a

result,

drivers/ide/it8172.c 2008-12-30 20:55:29.000000000 -0600 renamed from drivers/ide/it8172.c 1969-12-31 18:00:00.000000000 -0600 +3 -0
+        * it cannot be configured for PIO mode 0.  This table sets these
+        * parameters to the maximum supported by the IT8172.
+        */

This comment seems somewhat mispalced...

I will move the comment before the timings array, where it belongs.

Quoted 11 lines

drivers/ide/it8172.c 2008-12-30 20:55:29.000000000 -0600

Quoted 3 lines
drivers/ide/it8172.c 2008-12-30 20:55:29.000000000 -0600 renamed from drivers/ide/it8172.c 1969-12-31 18:00:00.000000000 -0600 +3 -0
+       drive_timing &= drive->dn ? 0x00003f00 : 0x000fc000;
+       drive_timing |= ((timings[pio][0] << 11) | (timings[pio][1] << 8))
+                       << (drive->dn * 6);

This can be somewhat shortened:

       drive_timing |= ((timings[pio][0] << 3) | timings[pio][1]) <<
                       (drive->dn * 6 + 8);

Could've be shortened even more if timings[] was single-dimensional...

Good point! I have made the array one-dimensional and shortened the statement accordingly.

Quoted 1 line

MBR, Sergei

Shane

#3 lore raw

Hello.

Quoted 9 lines · Shane McDonald
Quoted 4 lines
Quoted 2 lines
Signed-off-by Shane McDonald <[email protected]>
Acked-by Sergei Shtylyov <[email protected]>

Please forgive my ignorance -- this is my first non-trivial patch... I have made a new patch incorporating your suggestions as described below. Do I include your Acked-by line in my re-spun patch, or do I need you

Yes, you can. I can always take my AXK back. :-)

Quoted 3 lines

to Ack it again?
I will post the new patch when I receive guidance on this.

Oh, that's really a minor matter -- don't let it delay you. :-)

MBR, Sergei