Skip to contentSkip to conversation

sata_sil misc updates

PATCH New Tejun Heo 2 participants 13 messages Mar 5, 2006

Subsystems (2)
lore
Share & tools
lore

Tejun Heo

author
#0 lore raw

PATCH 1/1 sata_sil misc updates

Hello, Jeff and Carlos.

This patchset contains misc fixes/updates for sata_sil. The first two fix pci id tables and board ids as reported by Carlos. The rest are cosmetic changes without any functional change.

Thanks.

--
tejun

author
#1 lore raw

PATCH 1/6 sata_sil: replace sil_3112_m15w board id with sil_3112

All 3112's have m15w. Replace sil_3112_m15w with sil_3112 and flag sil_3112 with SIL_FLAG_MOD15WRITE.

Cc Carlos Pardo <[email protected]>

---

 drivers/scsi/sata_sil.c |   25 +++++++------------------
 1 files changed, 7 insertions(+), 18 deletions(-)

8ba88e0e2e3356e2364f1defdac5c269bc03d93c

drivers/scsi/sata_sil.c +7 -18
index cdf800e..8cb662d 100644
@@ -53,9 +53,8 @@ enum {
 	SIL_FLAG_MOD15WRITE	= (1 << 30),
 
 	sil_3112		= 0,
-	sil_3112_m15w		= 1,
-	sil_3512		= 2,
-	sil_3114		= 3,
+	sil_3512		= 1,
+	sil_3114		= 2,
 
 	SIL_FIFO_R0		= 0x40,
 	SIL_FIFO_W0		= 0x41,
@@ -90,13 +89,13 @@ static void sil_post_set_mode (struct at
 
 
 static const struct pci_device_id sil_pci_tbl[] = {
-	{ 0x1095, 0x3112, PCI_ANY_ID, PCI_ANY_ID, 0, 0, sil_3112_m15w },
-	{ 0x1095, 0x0240, PCI_ANY_ID, PCI_ANY_ID, 0, 0, sil_3112_m15w },
+	{ 0x1095, 0x3112, PCI_ANY_ID, PCI_ANY_ID, 0, 0, sil_3112 },
+	{ 0x1095, 0x0240, PCI_ANY_ID, PCI_ANY_ID, 0, 0, sil_3112 },
 	{ 0x1095, 0x3512, PCI_ANY_ID, PCI_ANY_ID, 0, 0, sil_3512 },
 	{ 0x1095, 0x3114, PCI_ANY_ID, PCI_ANY_ID, 0, 0, sil_3114 },
-	{ 0x1002, 0x436e, PCI_ANY_ID, PCI_ANY_ID, 0, 0, sil_3112_m15w },
-	{ 0x1002, 0x4379, PCI_ANY_ID, PCI_ANY_ID, 0, 0, sil_3112_m15w },
-	{ 0x1002, 0x437a, PCI_ANY_ID, PCI_ANY_ID, 0, 0, sil_3112_m15w },
+	{ 0x1002, 0x436e, PCI_ANY_ID, PCI_ANY_ID, 0, 0, sil_3112 },
+	{ 0x1002, 0x4379, PCI_ANY_ID, PCI_ANY_ID, 0, 0, sil_3112 },
+	{ 0x1002, 0x437a, PCI_ANY_ID, PCI_ANY_ID, 0, 0, sil_3112 },
 	{ }	/* terminate list */
 };
 
@@ -182,16 +181,6 @@ static const struct ata_port_info sil_po
 	{
 		.sht		= &sil_sht,
 		.host_flags	= ATA_FLAG_SATA | ATA_FLAG_NO_LEGACY |
-				  ATA_FLAG_MMIO,
-		.pio_mask	= 0x1f,			/* pio0-4 */
-		.mwdma_mask	= 0x07,			/* mwdma0-2 */
-		.udma_mask	= 0x3f,			/* udma0-5 */
-		.port_ops	= &sil_ops,
-	},
-	/* sil_3112_15w - keep it sync'd w/ sil_3112 */
-	{
-		.sht		= &sil_sht,
-		.host_flags	= ATA_FLAG_SATA | ATA_FLAG_NO_LEGACY |
 				  ATA_FLAG_MMIO | SIL_FLAG_MOD15WRITE,
 		.pio_mask	= 0x1f,			/* pio0-4 */
 		.mwdma_mask	= 0x07,			/* mwdma0-2 */
Signature · 1 line
1.2.1
#2 lore raw

Re: [PATCH 1/6] sata_sil: replace sil_3112_m15w board id with sil_3112

Quoted 5 lines · Tejun Heo

All 3112's have m15w. Replace sil_3112_m15w with sil_3112 and flag sil_3112 with SIL_FLAG_MOD15WRITE.

Cc Carlos Pardo <[email protected]>

applied 1, 3, 4, 6

author
#3 lore raw

PATCH 2/6 sata_sil: kill bogus PCI ID 0x1095/0x0240

In reply to Tejun Heo

Silicon Image confirmed that 0x1095/0x0240 has never been used for 3112. Kill it.

Cc Carlos Pardo <[email protected]>

---

 drivers/scsi/sata_sil.c |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

4c960f8b73294d01b9307ea6775304fc8556e550

drivers/scsi/sata_sil.c +0 -1
index 8cb662d..0d962a4 100644
@@ -90,7 +90,6 @@ static void sil_post_set_mode (struct at
 
 static const struct pci_device_id sil_pci_tbl[] = {
 	{ 0x1095, 0x3112, PCI_ANY_ID, PCI_ANY_ID, 0, 0, sil_3112 },
-	{ 0x1095, 0x0240, PCI_ANY_ID, PCI_ANY_ID, 0, 0, sil_3112 },
 	{ 0x1095, 0x3512, PCI_ANY_ID, PCI_ANY_ID, 0, 0, sil_3512 },
 	{ 0x1095, 0x3114, PCI_ANY_ID, PCI_ANY_ID, 0, 0, sil_3114 },
 	{ 0x1002, 0x436e, PCI_ANY_ID, PCI_ANY_ID, 0, 0, sil_3112 },
Signature · 1 line
1.2.1
#4 lore raw

Re: [PATCH 2/6] sata_sil: kill bogus PCI ID 0x1095/0x0240

Quoted 17 lines · Tejun Heo

Silicon Image confirmed that 0x1095/0x0240 has never been used for 3112. Kill it.

Cc Carlos Pardo <[email protected]>

---

 drivers/scsi/sata_sil.c |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

4c960f8b73294d01b9307ea6775304fc8556e550

drivers/scsi/sata_sil.c +0 -0
index 8cb662d..0d962a4 100644

drivers/scsi/sata_sil.c static void sil_post_set_mode (struct at · line 95

Quoted 7 lines
drivers/scsi/sata_sil.c +0 -1
@@ -90,7 +90,6 @@ static void sil_post_set_mode (struct at
 
 static const struct pci_device_id sil_pci_tbl[] = {
 	{ 0x1095, 0x3112, PCI_ANY_ID, PCI_ANY_ID, 0, 0, sil_3112 },
-	{ 0x1095, 0x0240, PCI_ANY_ID, PCI_ANY_ID, 0, 0, sil_3112 },
 	{ 0x1095, 0x3512, PCI_ANY_ID, PCI_ANY_ID, 0, 0, sil_3512 },
 	{ 0x1095, 0x3114, PCI_ANY_ID, PCI_ANY_ID, 0, 0, sil_3114 },
 	{ 0x1002, 0x436e, PCI_ANY_ID, PCI_ANY_ID, 0, 0, sil_3112 },

These things just don't get made up out of thin air. I'm wonder if it applies to a Silicon Image product running under a proprietary driver, or OEM product with a bunch of SiI chips on it. In any case, I'm reluctant to remove it.

Jeff

author
#5 lore raw

Re: [PATCH 2/6] sata_sil: kill bogus PCI ID 0x1095/0x0240

Quoted 20 lines · Jeff Garzik
Quoted 18 lines · Tejun Heo

Silicon Image confirmed that 0x1095/0x0240 has never been used for 3112. Kill it.

Cc Carlos Pardo <[email protected]>

---

 drivers/scsi/sata_sil.c |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

4c960f8b73294d01b9307ea6775304fc8556e550

drivers/scsi/sata_sil.c +0 -0
index 8cb662d..0d962a4 100644
@@ -90,7 +90,6 @@ static void sil_post_set_mode (struct at
 
Quoted 11 lines

drivers/scsi/sata_sil.c static void sil_post_set_mode (struct at · line 95

Quoted 6 lines
drivers/scsi/sata_sil.c +0 -1
 static const struct pci_device_id sil_pci_tbl[] = {
     { 0x1095, 0x3112, PCI_ANY_ID, PCI_ANY_ID, 0, 0, sil_3112 },
-    { 0x1095, 0x0240, PCI_ANY_ID, PCI_ANY_ID, 0, 0, sil_3112 },
     { 0x1095, 0x3512, PCI_ANY_ID, PCI_ANY_ID, 0, 0, sil_3512 },
     { 0x1095, 0x3114, PCI_ANY_ID, PCI_ANY_ID, 0, 0, sil_3114 },
     { 0x1002, 0x436e, PCI_ANY_ID, PCI_ANY_ID, 0, 0, sil_3112 },

These things just don't get made up out of thin air. I'm wonder if it applies to a Silicon Image product running under a proprietary driver, or OEM product with a bunch of SiI chips on it. In any case, I'm reluctant to remove it.

Agreed.

Signature · 1 line
tejun
author
#6 lore raw

PATCH 3/6 sata_sil: use kzalloc

In reply to Tejun Heo

Use kzalloc instead of kmalloc/memset.

---

 drivers/scsi/sata_sil.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

329b9143f3e901977aed81c1593b06dda52bb090

drivers/scsi/sata_sil.c +1 -2
index 0d962a4..b5401fa 100644
@@ -406,13 +406,12 @@ static int sil_init_one (struct pci_dev 
 	if (rc)
 		goto err_out_regions;
 
-	probe_ent = kmalloc(sizeof(*probe_ent), GFP_KERNEL);
+	probe_ent = kzalloc(sizeof(*probe_ent), GFP_KERNEL);
 	if (probe_ent == NULL) {
 		rc = -ENOMEM;
 		goto err_out_regions;
 	}
 
-	memset(probe_ent, 0, sizeof(*probe_ent));
 	INIT_LIST_HEAD(&probe_ent->node);
 	probe_ent->dev = pci_dev_to_dev(pdev);
 	probe_ent->port_ops = sil_port_info[ent->driver_data].port_ops;
Signature · 1 line
1.2.1
author
#7 lore raw

PATCH 4/6 sata_sil: replace register address constants with sil_port[] entry

In reply to Tejun Heo

Kill SIL_FIFO_* and SIL_IDE2_BMDMA and replace them with proper sil_port[] entry.

---

 drivers/scsi/sata_sil.c |   39 +++++++++++----------------------------
 1 files changed, 11 insertions(+), 28 deletions(-)

a63a8c77b1ed35894f0d0627ec3942ede40b2290

drivers/scsi/sata_sil.c +11 -28
index b5401fa..f096cc9 100644
@@ -56,15 +56,6 @@ enum {
 	sil_3512		= 1,
 	sil_3114		= 2,
 
-	SIL_FIFO_R0		= 0x40,
-	SIL_FIFO_W0		= 0x41,
-	SIL_FIFO_R1		= 0x44,
-	SIL_FIFO_W1		= 0x45,
-	SIL_FIFO_R2		= 0x240,
-	SIL_FIFO_W2		= 0x241,
-	SIL_FIFO_R3		= 0x244,
-	SIL_FIFO_W3		= 0x245,
-
 	SIL_SYSCFG		= 0x48,
 	SIL_MASK_IDE0_INT	= (1 << 22),
 	SIL_MASK_IDE1_INT	= (1 << 23),
@@ -74,8 +65,6 @@ enum {
 	SIL_MASK_4PORT		= SIL_MASK_2PORT |
 				  SIL_MASK_IDE2_INT | SIL_MASK_IDE3_INT,
 
-	SIL_IDE2_BMDMA		= 0x200,
-
 	SIL_INTR_STEERING	= (1 << 1),
 	SIL_QUIRK_MOD15WRITE	= (1 << 0),
 	SIL_QUIRK_UDMA5MAX	= (1 << 1),
@@ -216,16 +205,17 @@ static const struct {
 	unsigned long tf;	/* ATA taskfile register block */
 	unsigned long ctl;	/* ATA control/altstatus register block */
 	unsigned long bmdma;	/* DMA register block */
+	unsigned long fifo_cfg;	/* FIFO Valid Byte Count and Control */
 	unsigned long scr;	/* SATA control register block */
 	unsigned long sien;	/* SATA Interrupt Enable register */
 	unsigned long xfer_mode;/* data transfer mode register */
 	unsigned long sfis_cfg;	/* SATA FIS reception config register */
 } sil_port[] = {
 	/* port 0 ... */
-	{ 0x80, 0x8A, 0x00, 0x100, 0x148, 0xb4, 0x14c },
-	{ 0xC0, 0xCA, 0x08, 0x180, 0x1c8, 0xf4, 0x1cc },
-	{ 0x280, 0x28A, 0x200, 0x300, 0x348, 0x2b4, 0x34c },
-	{ 0x2C0, 0x2CA, 0x208, 0x380, 0x3c8, 0x2f4, 0x3cc },
+	{ 0x80, 0x8A, 0x00, 0x40, 0x100, 0x148, 0xb4, 0x14c },
+	{ 0xC0, 0xCA, 0x08, 0x44, 0x180, 0x1c8, 0xf4, 0x1cc },
+	{ 0x280, 0x28A, 0x200, 0x240, 0x300, 0x348, 0x2b4, 0x34c },
+	{ 0x2C0, 0x2CA, 0x208, 0x244, 0x380, 0x3c8, 0x2f4, 0x3cc },
 	/* ... port 3 */
 };
 
@@ -448,19 +438,12 @@ static int sil_init_one (struct pci_dev 
 	if (cls) {
 		cls >>= 3;
 		cls++;  /* cls = (line_size/8)+1 */
-		writeb(cls, mmio_base + SIL_FIFO_R0);
-		writeb(cls, mmio_base + SIL_FIFO_W0);
-		writeb(cls, mmio_base + SIL_FIFO_R1);
-		writeb(cls, mmio_base + SIL_FIFO_W1);
-		if (ent->driver_data == sil_3114) {
-			writeb(cls, mmio_base + SIL_FIFO_R2);
-			writeb(cls, mmio_base + SIL_FIFO_W2);
-			writeb(cls, mmio_base + SIL_FIFO_R3);
-			writeb(cls, mmio_base + SIL_FIFO_W3);
-		}
+		for (i = 0; i < probe_ent->n_ports; i++)
+			writew(cls << 8 | cls,
+			       mmio_base + sil_port[i].fifo_cfg);
 	} else
 		dev_printk(KERN_WARNING, &pdev->dev,
-			 "cache line size not set.  Driver may not function\n");
+			   "cache line size not set.  Driver may not function\n");
 
 	/* Apply R_ERR on DMA activate FIS errata workaround */
 	if (probe_ent->host_flags & SIL_FLAG_RERR_ON_DMA_ACT) {
@@ -483,10 +466,10 @@ static int sil_init_one (struct pci_dev 
 		irq_mask = SIL_MASK_4PORT;
 
 		/* flip the magic "make 4 ports work" bit */
-		tmp = readl(mmio_base + SIL_IDE2_BMDMA);
+		tmp = readl(mmio_base + sil_port[2].bmdma);
 		if ((tmp & SIL_INTR_STEERING) == 0)
 			writel(tmp | SIL_INTR_STEERING,
-			       mmio_base + SIL_IDE2_BMDMA);
+			       mmio_base + sil_port[2].bmdma);
 
 	} else {
 		irq_mask = SIL_MASK_2PORT;
Signature · 1 line
1.2.1
author
#8 lore raw

PATCH 5/6 sata_sil: convert sil_port[] to u16 from unsigned long

In reply to Tejun Heo

unsigned long is way too big for sil_port[] offsets (currently the table is 256 bytes on 64bit machines). Convert sil_port[] to u16.

---

 drivers/scsi/sata_sil.c |   20 ++++++++++----------
 1 files changed, 10 insertions(+), 10 deletions(-)

21111e9d5d332ba90051d6821ca7abf058ee5e99

drivers/scsi/sata_sil.c +10 -10
index f096cc9..fb86e36 100644
@@ -202,18 +202,18 @@ static const struct ata_port_info sil_po
 /* per-port register offsets */
 /* TODO: we can probably calculate rather than use a table */
 static const struct {
-	unsigned long tf;	/* ATA taskfile register block */
-	unsigned long ctl;	/* ATA control/altstatus register block */
-	unsigned long bmdma;	/* DMA register block */
-	unsigned long fifo_cfg;	/* FIFO Valid Byte Count and Control */
-	unsigned long scr;	/* SATA control register block */
-	unsigned long sien;	/* SATA Interrupt Enable register */
-	unsigned long xfer_mode;/* data transfer mode register */
-	unsigned long sfis_cfg;	/* SATA FIS reception config register */
+	u16 tf;		/* ATA taskfile register block */
+	u16 ctl;	/* ATA control/altstatus register block */
+	u16 bmdma;	/* DMA register block */
+	u16 fifo_cfg;	/* FIFO Valid Byte Count and Control */
+	u16 scr;	/* SATA control register block */
+	u16 sien;	/* SATA Interrupt Enable register */
+	u16 xfer_mode;	/* data transfer mode register */
+	u16 sfis_cfg;	/* SATA FIS reception config register */
 } sil_port[] = {
 	/* port 0 ... */
-	{ 0x80, 0x8A, 0x00, 0x40, 0x100, 0x148, 0xb4, 0x14c },
-	{ 0xC0, 0xCA, 0x08, 0x44, 0x180, 0x1c8, 0xf4, 0x1cc },
+	{ 0x080, 0x08A, 0x000, 0x040, 0x100, 0x148, 0x0b4, 0x14c },
+	{ 0x0C0, 0x0CA, 0x008, 0x044, 0x180, 0x1c8, 0x0f4, 0x1cc },
 	{ 0x280, 0x28A, 0x200, 0x240, 0x300, 0x348, 0x2b4, 0x34c },
 	{ 0x2C0, 0x2CA, 0x208, 0x244, 0x380, 0x3c8, 0x2f4, 0x3cc },
 	/* ... port 3 */
Signature · 1 line
1.2.1
#9 lore raw

Re: [PATCH 5/6] sata_sil: convert sil_port[] to u16 from unsigned long

Quoted 4 lines · Tejun Heo

unsigned long is way too big for sil_port[] offsets (currently the table is 256 bytes on 64bit machines). Convert sil_port[] to u16.

NAK. This just forces the compiler to promote these from u16 to machine int (i.e. unsigned long) at addition time. Table size is a very trivial matter.

Jeff

author
#10 lore raw

Re: [PATCH 5/6] sata_sil: convert sil_port[] to u16 from unsigned long

Quoted 10 lines · Jeff Garzik
Quoted 4 lines · Tejun Heo

unsigned long is way too big for sil_port[] offsets (currently the table is 256 bytes on 64bit machines). Convert sil_port[] to u16.

NAK. This just forces the compiler to promote these from u16 to machine int (i.e. unsigned long) at addition time. Table size is a very trivial matter.

Wouldn't that be cheaper than extra cache lines? I'm pretty sure it would be on x86/64.

Signature · 1 line
tejun
#11 lore raw

Re: [PATCH 5/6] sata_sil: convert sil_port[] to u16 from unsigned long

Quoted 7 lines · Tejun Heo
Quoted 5 lines · Jeff Garzik
Quoted 3 lines · Tejun Heo

unsigned long is way too big for sil_port[] offsets (currently the table is 256 bytes on 64bit machines). Convert sil_port[] to u16.

Quoted 8 lines
Quoted 6 lines
Quoted 1 line

NAK. This just forces the compiler to promote these from u16 to machine int (i.e. unsigned long) at addition time. Table size is a very trivial matter.

Wouldn't that be cheaper than extra cache lines? I'm pretty sure it would be on x86/64.

Actually, when I got this reply, I looked at it again. sil_port[] is only used at startup, therefore performance is not an issue, and we don't care (within reason) how big the table is. So, I would rather leave it in native format (machine int -> unsigned long).

Jeff

author
#12 lore raw

PATCH 6/6 sata_sil: cosmetic flag/constant changes

In reply to Tejun Heo

Collect common host flags into SIL_DFL_HOST_FLAGS and add comments to constants.

---

 drivers/scsi/sata_sil.c |   32 ++++++++++++++++++++++++--------
 1 files changed, 24 insertions(+), 8 deletions(-)

792d3b8f3bb747b2c359aed4c004c2e42ef18db2

drivers/scsi/sata_sil.c +24 -8
index fb86e36..56a92a9 100644
@@ -49,14 +49,30 @@
 #define DRV_VERSION	"0.9"
 
 enum {
+	/*
+	 * host flags
+	 */
 	SIL_FLAG_RERR_ON_DMA_ACT = (1 << 29),
 	SIL_FLAG_MOD15WRITE	= (1 << 30),
+	SIL_DFL_HOST_FLAGS	= ATA_FLAG_SATA | ATA_FLAG_NO_LEGACY |
+				  ATA_FLAG_MMIO,
 
+	/*
+	 * Controller IDs
+	 */
 	sil_3112		= 0,
 	sil_3512		= 1,
 	sil_3114		= 2,
 
+	/*
+	 * Register offsets
+	 */
 	SIL_SYSCFG		= 0x48,
+
+	/*
+	 * Register bits
+	 */
+	/* SYSCFG */
 	SIL_MASK_IDE0_INT	= (1 << 22),
 	SIL_MASK_IDE1_INT	= (1 << 23),
 	SIL_MASK_IDE2_INT	= (1 << 24),
@@ -65,7 +81,12 @@ enum {
 	SIL_MASK_4PORT		= SIL_MASK_2PORT |
 				  SIL_MASK_IDE2_INT | SIL_MASK_IDE3_INT,
 
+	/* BMDMA/BMDMA2 */
 	SIL_INTR_STEERING	= (1 << 1),
+
+	/*
+	 * Others
+	 */
 	SIL_QUIRK_MOD15WRITE	= (1 << 0),
 	SIL_QUIRK_UDMA5MAX	= (1 << 1),
 };
@@ -168,8 +189,7 @@ static const struct ata_port_info sil_po
 	/* sil_3112 */
 	{
 		.sht		= &sil_sht,
-		.host_flags	= ATA_FLAG_SATA | ATA_FLAG_NO_LEGACY |
-				  ATA_FLAG_MMIO | SIL_FLAG_MOD15WRITE,
+		.host_flags	= SIL_DFL_HOST_FLAGS | SIL_FLAG_MOD15WRITE,
 		.pio_mask	= 0x1f,			/* pio0-4 */
 		.mwdma_mask	= 0x07,			/* mwdma0-2 */
 		.udma_mask	= 0x3f,			/* udma0-5 */
@@ -178,9 +198,7 @@ static const struct ata_port_info sil_po
 	/* sil_3512 */
 	{
 		.sht		= &sil_sht,
-		.host_flags	= ATA_FLAG_SATA | ATA_FLAG_NO_LEGACY |
-				  ATA_FLAG_MMIO |
-				  SIL_FLAG_RERR_ON_DMA_ACT,
+		.host_flags	= SIL_DFL_HOST_FLAGS | SIL_FLAG_RERR_ON_DMA_ACT,
 		.pio_mask	= 0x1f,			/* pio0-4 */
 		.mwdma_mask	= 0x07,			/* mwdma0-2 */
 		.udma_mask	= 0x3f,			/* udma0-5 */
@@ -189,9 +207,7 @@ static const struct ata_port_info sil_po
 	/* sil_3114 */
 	{
 		.sht		= &sil_sht,
-		.host_flags	= ATA_FLAG_SATA | ATA_FLAG_NO_LEGACY |
-				  ATA_FLAG_MMIO |
-				  SIL_FLAG_RERR_ON_DMA_ACT,
+		.host_flags	= SIL_DFL_HOST_FLAGS | SIL_FLAG_RERR_ON_DMA_ACT,
 		.pio_mask	= 0x1f,			/* pio0-4 */
 		.mwdma_mask	= 0x07,			/* mwdma0-2 */
 		.udma_mask	= 0x3f,			/* udma0-5 */
Signature · 1 line
1.2.1