Re: sata_sil 3112 activity LED patch

[Aric Cyr <acyr@alumni.uwaterloo.ca>]

On Thu, Jul 07, 2005 at 02:47:03PM +0200, Jens Axboe wrote:
> On Wed, Jul 06 2005, Jeff Garzik wrote:
> > I don't think its ugly, necessarily.  I do worry about the flash memory 
> > stuff, though, which is why I don't want to merge this upstream for now.
> > 
> > For your patch specifically, it would be nice to follow the coding style 
> > that is found in the rest of the driver (single-tab indents, etc., read 
> > CodingStyle in kernel source tree).

I cleaned it up... it got quite a bit larger since I am attempting to
check for valid usage.  Unfortunately I do not know if the 3114 has a
similar GPIO mechanism, nor do I have an add on card with a 3112 to
verify with.  My new/improved patch is attached, tested on my system
with 2.6.12.

If anyone with either such devices could test (with caution!) I'd like
to hear the results.  Alternatively, I'd be interested in the value of
the MMIO register at BASE_ADDRESS_5 + 0x54 before the driver is
loaded.

> There's also an existing variant of this in the block layer, the
> activity_fn, that we use on the ibook/powerbook to use the sleep led as
> an activity light. Just in case you prefer that to overloading the bmdma
> start/stop handlers.

You suggestion at first looked to be incredibly nice... until I looked
at how much implementation was required.  I am considering trying it,
but I cannot find a place for an sata driver to call the
blk_queue_activity_fn() with meaningful parameters during init.

On a second look, I guess I would have to override
ata_scsi_slave_config() in the driver and hook up the activity light
there.  This would be fine I guess.  Unless I am interpreting this
incorrectly, however, I would need to use a timer or something to turn
the light back off?  I'm probably missing something, so is there a
simpler way to do this?

-- 
Aric Cyr <acyr at alumni dot uwaterloo dot ca>    (http://acyr.net)
gpg fingerprint: 943A 1549 47AC D766 B7F8  D551 6703 7142 C282 D542

--- drivers/scsi/sata_sil.c.orig	2005-07-07 10:07:19.000000000 +0900
+++ drivers/scsi/sata_sil.c	2005-07-07 23:19:36.000000000 +0900
@@ -54,6 +54,7 @@
 	SIL_FIFO_W3		= 0x245,
 
 	SIL_SYSCFG		= 0x48,
+	SIL_GPIO                = 0x54,
 	SIL_MASK_IDE0_INT	= (1 << 22),
 	SIL_MASK_IDE1_INT	= (1 << 23),
 	SIL_MASK_IDE2_INT	= (1 << 24),
@@ -74,6 +75,9 @@
 static u32 sil_scr_read (struct ata_port *ap, unsigned int sc_reg);
 static void sil_scr_write (struct ata_port *ap, unsigned int sc_reg, u32 val);
 static void sil_post_set_mode (struct ata_port *ap);
+static void sil_bmdma_start(struct ata_queued_cmd *qc);
+static void sil_bmdma_stop(struct ata_port *ap);
+static void sil_host_stop (struct ata_host_set *host_set);
 
 static struct pci_device_id sil_pci_tbl[] = {
 	{ 0x1095, 0x3112, PCI_ANY_ID, PCI_ANY_ID, 0, 0, sil_3112 },
@@ -149,8 +152,8 @@
 	.phy_reset		= sata_phy_reset,
 	.post_set_mode		= sil_post_set_mode,
 	.bmdma_setup            = ata_bmdma_setup,
-	.bmdma_start            = ata_bmdma_start,
-	.bmdma_stop		= ata_bmdma_stop,
+	.bmdma_start            = sil_bmdma_start,
+	.bmdma_stop		= sil_bmdma_stop,
 	.bmdma_status		= ata_bmdma_status,
 	.qc_prep		= ata_qc_prep,
 	.qc_issue		= ata_qc_issue_prot,
@@ -161,7 +164,7 @@
 	.scr_write		= sil_scr_write,
 	.port_start		= ata_port_start,
 	.port_stop		= ata_port_stop,
-	.host_stop		= ata_host_stop,
+	.host_stop		= sil_host_stop,
 };
 
 static struct ata_port_info sil_port_info[] = {
@@ -204,6 +207,10 @@
 	/* ... port 3 */
 };
 
+struct sil_host_priv {
+	u8			use_gpio;
+};
+
 MODULE_AUTHOR("Jeff Garzik");
 MODULE_DESCRIPTION("low-level driver for Silicon Image SATA controller");
 MODULE_LICENSE("GPL");
@@ -217,6 +224,48 @@
 	return cache_line;
 }
 
+static void sil_bmdma_start(struct ata_queued_cmd *qc)
+{
+	struct sil_host_priv* hpriv = qc->ap->host_set->private_data;
+	if (hpriv->use_gpio) {
+		void* mmio_base = qc->ap->host_set->mmio_base;
+		u32 gpio = readl(mmio_base + SIL_GPIO);
+
+		/* set the lower 8 bits to activate the LED */
+		gpio |= 0xff;
+		writel(gpio, mmio_base + SIL_GPIO);
+		readl(mmio_base + SIL_GPIO);	/* flush */
+	}
+
+	ata_bmdma_start(qc);
+}
+
+static void sil_bmdma_stop(struct ata_port *ap)
+{
+	struct sil_host_priv* hpriv = ap->host_set->private_data;
+
+	ata_bmdma_stop(ap);
+
+	if (hpriv->use_gpio) {
+		void* mmio_base = ap->host_set->mmio_base;
+		u32 gpio = readl(mmio_base + SIL_GPIO);
+
+		/* set bits [15:8] to disable the LED */
+		gpio |= 0xff00;
+		writel(gpio, mmio_base + SIL_GPIO);
+		readl(mmio_base + SIL_GPIO);	/* flush */
+	}
+}
+
+static void sil_host_stop (struct ata_host_set *host_set)
+{
+	if (host_set->private_data)
+		kfree(host_set->private_data);
+
+	ata_host_stop(host_set);
+}
+
+
 static void sil_post_set_mode (struct ata_port *ap)
 {
 	struct ata_host_set *host_set = ap->host_set;
@@ -353,6 +402,7 @@
 {
 	static int printed_version;
 	struct ata_probe_ent *probe_ent = NULL;
+	struct sil_host_priv *hpriv = NULL;
 	unsigned long base;
 	void *mmio_base;
 	int rc;
@@ -391,6 +441,13 @@
 		goto err_out_regions;
 	}
 
+	hpriv = kmalloc(sizeof(*hpriv), GFP_KERNEL);
+	if (!hpriv) {
+		rc = -ENOMEM;
+		goto err_out_free_ent;
+	}
+	memset(hpriv, 0, sizeof(*hpriv));
+
 	memset(probe_ent, 0, sizeof(*probe_ent));
 	INIT_LIST_HEAD(&probe_ent->node);
 	probe_ent->dev = pci_dev_to_dev(pdev);
@@ -408,10 +465,11 @@
 		            pci_resource_len(pdev, 5));
 	if (mmio_base == NULL) {
 		rc = -ENOMEM;
-		goto err_out_free_ent;
+		goto err_out_free_hpriv;
 	}
 
 	probe_ent->mmio_base = mmio_base;
+	probe_ent->private_data = hpriv;
 
 	base = (unsigned long) mmio_base;
 
@@ -456,6 +514,12 @@
 		irq_mask = SIL_MASK_2PORT;
 	}
 
+	/* check for LED GPIO on 3112 parts */
+	tmp = readl(mmio_base + SIL_GPIO);
+	if ((ent->driver_data == sil_3112) && ((tmp & 0xff) == 0xff)) {
+		hpriv->use_gpio = 1;
+	}
+
 	/* make sure IDE0/1/2/3 interrupts are not masked */
 	tmp = readl(mmio_base + SIL_SYSCFG);
 	if (tmp & irq_mask) {
@@ -477,6 +541,8 @@
 
 	return 0;
 
+err_out_free_hpriv:
+	kfree(hpriv);
 err_out_free_ent:
 	kfree(probe_ent);
 err_out_regions:

Attachment: pgp9igR7h9PFT.pgp
Description: PGP signature