All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] SCSI: Add the SGPIO support for sata_nv.c
       [not found] <15F501D1A78BD343BE8F4D8DB854566B059FE0B3@hkemmail01.nvidia.com>
@ 2006-10-31  8:43   ` Peer Chen
  0 siblings, 0 replies; 27+ messages in thread
From: Peer Chen @ 2006-10-31  8:43 UTC (permalink / raw)
  To: jeff, linux-kernel, linux-ide; +Cc: Kuan Luo

The following SGPIO patch for sata_nv.c is based on 2.6.18 kernel.
Signed-off by: Kuan Luo <kluo@nvidia.com>


--- linux-2.6.18/drivers/scsi/sata_nv.c.orig	2006-10-08
17:20:07.000000000 +0800
+++ linux-2.6.18/drivers/scsi/sata_nv.c	2006-10-13 11:12:01.000000000
+0800
@@ -80,6 +80,175 @@
 	NV_MCP_SATA_CFG_20_SATA_SPACE_EN = 0x04,
 };
 
+//sgpio
+// Sgpio defines
+// SGPIO state defines
+#define NV_SGPIO_STATE_RESET		0
+#define NV_SGPIO_STATE_OPERATIONAL	1
+#define NV_SGPIO_STATE_ERROR		2
+
+// SGPIO command opcodes
+#define NV_SGPIO_CMD_RESET		0
+#define NV_SGPIO_CMD_READ_PARAMS	1
+#define NV_SGPIO_CMD_READ_DATA		2
+#define NV_SGPIO_CMD_WRITE_DATA		3
+
+// SGPIO command status defines
+#define NV_SGPIO_CMD_OK			0
+#define NV_SGPIO_CMD_ACTIVE		1
+#define NV_SGPIO_CMD_ERR		2
+
+#define NV_SGPIO_UPDATE_TICK		90
+#define NV_SGPIO_MIN_UPDATE_DELTA	33
+#define NV_CNTRLR_SHARE_INIT		2
+#define NV_SGPIO_MAX_ACTIVITY_ON	20
+#define NV_SGPIO_MIN_FORCE_OFF		5
+#define NV_SGPIO_PCI_CSR_OFFSET		0x58
+#define NV_SGPIO_PCI_CB_OFFSET		0x5C
+#define NV_SGPIO_DFLT_CB_SIZE		256
+#define NV_ON 1
+#define NV_OFF 0
+#ifndef bool
+#define bool u8
+#endif
+
+
+#define BF_EXTRACT(v, off, bc)	\
+	((((u8)(v)) >> (off)) & ((1 << (bc)) - 1))
+
+#define BF_INS(v, ins, off, bc)				\
+	(((v) & ~((((1 << (bc)) - 1)) << (off))) |	\
+	(((u8)(ins)) << (off)))
+
+#define BF_EXTRACT_U32(v, off, bc)	\
+	((((u32)(v)) >> (off)) & ((1 << (bc)) - 1))
+
+#define BF_INS_U32(v, ins, off, bc)			\
+	(((v) & ~((((1 << (bc)) - 1)) << (off))) |	\
+	(((u32)(ins)) << (off)))
+
+#define GET_SGPIO_STATUS(v)	BF_EXTRACT(v, 0, 2)
+#define GET_CMD_STATUS(v)	BF_EXTRACT(v, 3, 2)
+#define GET_CMD(v)		BF_EXTRACT(v, 5, 3)
+#define SET_CMD(v, cmd)		BF_INS(v, cmd, 5, 3) 
+
+#define GET_ENABLE(v)		BF_EXTRACT_U32(v, 23, 1)
+#define SET_ENABLE(v)		BF_INS_U32(v, 1, 23, 1)
+
+// Needs to have a u8 bit-field insert.
+#define GET_ACTIVITY(v)		BF_EXTRACT(v, 5, 3)
+#define SET_ACTIVITY(v, on_off)	BF_INS(v, on_off, 5, 3)
+
+union nv_sgpio_nvcr 
+{
+	struct {
+		u8	init_cnt;
+		u8	cb_size;
+		u8	cbver;
+		u8	rsvd;
+	} bit;
+	u32	all;
+};
+
+union nv_sgpio_tx 
+{
+	u8	tx_port[4];
+	u32 	all;
+};
+
+struct nv_sgpio_cb 
+{
+	u64			scratch_space;
+	union nv_sgpio_nvcr	nvcr;
+	u32			cr0;
+	u32                     rsvd[4];
+	union nv_sgpio_tx       tx[2];
+};
+
+struct nv_sgpio_host_share
+{
+	spinlock_t	*plock;
+	unsigned long   *ptstamp;
+};
+
+struct nv_sgpio_host_flags
+{
+	u8	sgpio_enabled:1;
+	u8	need_update:1;
+	u8	rsvd:6;
+};
+	
+struct nv_host_sgpio
+{
+	struct nv_sgpio_host_flags	flags;
+	u8				*pcsr;
+	struct nv_sgpio_cb		*pcb;	
+	struct nv_sgpio_host_share	share;
+	struct timer_list		sgpio_timer;
+};
+
+struct nv_sgpio_port_flags
+{
+	u8	last_state:1;
+	u8	recent_activity:1;
+	u8	rsvd:6;
+};
+
+struct nv_sgpio_led 
+{
+	struct nv_sgpio_port_flags	flags;
+	u8				force_off;
+	u8      			last_cons_active;
+};
+
+struct nv_port_sgpio
+{
+	struct nv_sgpio_led	activity;
+};
+
+static spinlock_t	nv_sgpio_lock;
+static unsigned long	nv_sgpio_tstamp;
+
+static inline void nv_sgpio_set_csr(u8 csr, unsigned long pcsr)
+{
+	outb(csr, pcsr);
+}
+
+static inline u8 nv_sgpio_get_csr(unsigned long pcsr)
+{
+	return inb(pcsr);
+}
+
+static inline u8 nv_sgpio_get_func(struct ata_host_set *host_set)
+{
+	u8 devfn = (to_pci_dev(host_set->dev))->devfn;
+	return (PCI_FUNC(devfn));
+}
+
+static inline u8 nv_sgpio_tx_host_offset(struct ata_host_set *host_set)
+{
+	return (nv_sgpio_get_func(host_set)/NV_CNTRLR_SHARE_INIT);
+}
+
+static inline u8 nv_sgpio_calc_tx_offset(u8 cntrlr, u8 channel)
+{
+	return (sizeof(union nv_sgpio_tx) - (NV_CNTRLR_SHARE_INIT *
+		(cntrlr % NV_CNTRLR_SHARE_INIT)) - channel - 1);
+}
+
+static inline u8 nv_sgpio_tx_port_offset(struct ata_port *ap)
+{
+	u8 cntrlr = nv_sgpio_get_func(ap->host_set);
+	return (nv_sgpio_calc_tx_offset(cntrlr, ap->port_no));
+}
+
+static inline bool nv_sgpio_capable(const struct pci_device_id *ent)
+{
+	if (ent->device == PCI_DEVICE_ID_NVIDIA_NFORCE_MCP55_SATA2)
+		return 1;
+	else
+		return 0;
+}
 static int nv_init_one (struct pci_dev *pdev, const struct
pci_device_id *ent);
 static void nv_ck804_host_stop(struct ata_host_set *host_set);
 static irqreturn_t nv_generic_interrupt(int irq, void *dev_instance,
@@ -90,7 +259,10 @@
 				      struct pt_regs *regs);
 static u32 nv_scr_read (struct ata_port *ap, unsigned int sc_reg);
 static void nv_scr_write (struct ata_port *ap, unsigned int sc_reg, u32
val);
-
+static void nv_host_stop (struct ata_host_set *host_set);
+static int nv_port_start(struct ata_port *ap);
+static void nv_port_stop(struct ata_port *ap);
+static int nv_qc_issue(struct ata_queued_cmd *qc);
 static void nv_nf2_freeze(struct ata_port *ap);
 static void nv_nf2_thaw(struct ata_port *ap);
 static void nv_ck804_freeze(struct ata_port *ap);
@@ -147,6 +319,27 @@
 	{ 0, } /* terminate list */
 };
 
+struct nv_host
+{
+	unsigned long		host_flags;
+	struct nv_host_sgpio	host_sgpio;
+};
+
+struct nv_port
+{
+	struct nv_port_sgpio	port_sgpio;
+};
+
+// SGPIO function prototypes
+static void nv_sgpio_init(struct pci_dev *pdev, struct nv_host *phost);
+static void nv_sgpio_reset(u8 *pcsr);
+static void nv_sgpio_set_timer(struct timer_list *ptimer, 
+				unsigned int timeout_msec);
+static void nv_sgpio_timer_handler(unsigned long ptr);
+static void nv_sgpio_host_cleanup(struct nv_host *host);
+static bool nv_sgpio_update_led(struct nv_sgpio_led *led, bool
*on_off);
+static void nv_sgpio_clear_all_leds(struct ata_port *ap);
+static bool nv_sgpio_send_cmd(struct nv_host *host, u8 cmd);
 static struct pci_driver nv_pci_driver = {
 	.name			= DRV_NAME,
 	.id_table		= nv_pci_tbl,
@@ -184,7 +377,7 @@
 	.bmdma_stop		= ata_bmdma_stop,
 	.bmdma_status		= ata_bmdma_status,
 	.qc_prep		= ata_qc_prep,
-	.qc_issue		= ata_qc_issue_prot,
+	.qc_issue		= nv_qc_issue,
 	.freeze			= ata_bmdma_freeze,
 	.thaw			= ata_bmdma_thaw,
 	.error_handler		= nv_error_handler,
@@ -194,9 +387,9 @@
 	.irq_clear		= ata_bmdma_irq_clear,
 	.scr_read		= nv_scr_read,
 	.scr_write		= nv_scr_write,
-	.port_start		= ata_port_start,
-	.port_stop		= ata_port_stop,
-	.host_stop		= ata_pci_host_stop,
+	.port_start		= nv_port_start,
+	.port_stop		= nv_port_stop,
+	.host_stop		= nv_host_stop,
 };
 
 static const struct ata_port_operations nv_nf2_ops = {
@@ -211,7 +404,7 @@
 	.bmdma_stop		= ata_bmdma_stop,
 	.bmdma_status		= ata_bmdma_status,
 	.qc_prep		= ata_qc_prep,
-	.qc_issue		= ata_qc_issue_prot,
+	.qc_issue		= nv_qc_issue,
 	.freeze			= nv_nf2_freeze,
 	.thaw			= nv_nf2_thaw,
 	.error_handler		= nv_error_handler,
@@ -221,9 +414,9 @@
 	.irq_clear		= ata_bmdma_irq_clear,
 	.scr_read		= nv_scr_read,
 	.scr_write		= nv_scr_write,
-	.port_start		= ata_port_start,
-	.port_stop		= ata_port_stop,
-	.host_stop		= ata_pci_host_stop,
+	.port_start		= nv_port_start,
+	.port_stop		= nv_port_stop,
+	.host_stop		= nv_host_stop,
 };
 
 static const struct ata_port_operations nv_ck804_ops = {
@@ -238,7 +431,7 @@
 	.bmdma_stop		= ata_bmdma_stop,
 	.bmdma_status		= ata_bmdma_status,
 	.qc_prep		= ata_qc_prep,
-	.qc_issue		= ata_qc_issue_prot,
+	.qc_issue		= nv_qc_issue,
 	.freeze			= nv_ck804_freeze,
 	.thaw			= nv_ck804_thaw,
 	.error_handler		= nv_error_handler,
@@ -248,8 +441,8 @@
 	.irq_clear		= ata_bmdma_irq_clear,
 	.scr_read		= nv_scr_read,
 	.scr_write		= nv_scr_write,
-	.port_start		= ata_port_start,
-	.port_stop		= ata_port_stop,
+	.port_start		= nv_port_start,
+	.port_stop		= nv_port_stop,
 	.host_stop		= nv_ck804_host_stop,
 };
 
@@ -480,10 +673,10 @@
 	ata_bmdma_drive_eh(ap, ata_std_prereset, ata_std_softreset,
 			   nv_hardreset, ata_std_postreset);
 }
-
 static int nv_init_one (struct pci_dev *pdev, const struct
pci_device_id *ent)
 {
 	static int printed_version = 0;
+	struct nv_host *host;
 	struct ata_port_info *ppi;
 	struct ata_probe_ent *probe_ent;
 	int pci_dev_busy = 0;
@@ -525,6 +718,13 @@
 	if (!probe_ent)
 		goto err_out_regions;
 
+	host = kmalloc(sizeof(struct nv_host), GFP_KERNEL);
+	if (!host)
+		goto err_out_free_ent;
+
+	memset(host, 0, sizeof(struct nv_host));
+
+	probe_ent->private_data = host;
 	probe_ent->mmio_base = pci_iomap(pdev, 5, 0);
 	if (!probe_ent->mmio_base) {
 		rc = -EIO;
@@ -550,6 +750,8 @@
 	rc = ata_device_add(probe_ent);
 	if (rc != NV_PORTS)
 		goto err_out_iounmap;
+	if (nv_sgpio_capable(ent))
+		nv_sgpio_init(pdev, host);
 
 	kfree(probe_ent);
 
@@ -568,12 +770,56 @@
 	return rc;
 }
 
+static int nv_port_start(struct ata_port *ap)
+{
+	int stat;
+	struct nv_port *port;
+
+	stat = ata_port_start(ap);
+	if (stat) {
+		return stat;
+	}
+
+	port = kmalloc(sizeof(struct nv_port), GFP_KERNEL);
+	if (!port) 
+		goto err_out_no_free;
+
+	memset(port, 0, sizeof(struct nv_port));
+
+	ap->private_data = port;
+	return 0;
+
+err_out_no_free:
+	return 1;
+}
+
+static void nv_port_stop(struct ata_port *ap)
+{
+	nv_sgpio_clear_all_leds(ap);
+
+	if (ap->private_data) {
+		kfree(ap->private_data);
+		ap->private_data = NULL;
+	}
+	ata_port_stop(ap);
+}
+
+static int nv_qc_issue(struct ata_queued_cmd *qc)
+{
+	struct nv_port *port = qc->ap->private_data;
+
+	if (port) 
+		port->port_sgpio.activity.flags.recent_activity = 1;
+	return (ata_qc_issue_prot(qc));
+}
 static void nv_ck804_host_stop(struct ata_host_set *host_set)
 {
+	struct nv_host *host = host_set->private_data;
 	struct pci_dev *pdev = to_pci_dev(host_set->dev);
 	u8 regval;
 
 	/* disable SATA space for CK804 */
+	nv_sgpio_host_cleanup(host);
 	pci_read_config_byte(pdev, NV_MCP_SATA_CFG_20, &regval);
 	regval &= ~NV_MCP_SATA_CFG_20_SATA_SPACE_EN;
 	pci_write_config_byte(pdev, NV_MCP_SATA_CFG_20, regval);
@@ -581,6 +827,277 @@
 	ata_pci_host_stop(host_set);
 }
 
+static void nv_host_stop (struct ata_host_set *host_set)
+{
+	struct nv_host *host = host_set->private_data;
+
+
+	nv_sgpio_host_cleanup(host);
+	kfree(host);
+	ata_pci_host_stop(host_set);
+
+}
+
+static void nv_sgpio_init(struct pci_dev *pdev, struct nv_host *phost)
+{
+	u16 csr_add; 
+	u32 cb_add, temp32;
+	struct device *dev = pci_dev_to_dev(pdev);
+	struct ata_host_set *host_set = dev_get_drvdata(dev);
+	u8 pro=0;
+	
+	pci_read_config_word(pdev, NV_SGPIO_PCI_CSR_OFFSET, &csr_add);
+	pci_read_config_dword(pdev, NV_SGPIO_PCI_CB_OFFSET, &cb_add);
+	pci_read_config_byte(pdev, 0xA4, &pro);
+	
+	if (csr_add == 0 || cb_add == 0)
+		return;
+	
+	if (!(pro&0x40))
+		return;	
+		
+	temp32 = csr_add;
+	phost->host_sgpio.pcsr = (void *)temp32;
+	phost->host_sgpio.pcb = phys_to_virt(cb_add);
+
+	if (phost->host_sgpio.pcb->nvcr.bit.init_cnt!=0x2 ||
phost->host_sgpio.pcb->nvcr.bit.cbver!=0x0)
+		return;
+		
+	if (temp32 <=0x200 || temp32 >=0xFFFE )
+		return;
+		
+	if (cb_add<=0x80000 || cb_add>=0x9FC00)
+		return;
+	
+	if (phost->host_sgpio.pcb->scratch_space == 0) {
+		spin_lock_init(&nv_sgpio_lock);
+		phost->host_sgpio.share.plock = &nv_sgpio_lock;
+		phost->host_sgpio.share.ptstamp = &nv_sgpio_tstamp;
+		phost->host_sgpio.pcb->scratch_space = 
+			(unsigned long)&phost->host_sgpio.share;
+		spin_lock(phost->host_sgpio.share.plock);
+		nv_sgpio_reset(phost->host_sgpio.pcsr);
+		phost->host_sgpio.pcb->cr0 = 
+			SET_ENABLE(phost->host_sgpio.pcb->cr0);
+
+		spin_unlock(phost->host_sgpio.share.plock);
+	}
+
+	phost->host_sgpio.share = 
+		*(struct nv_sgpio_host_share *)(unsigned long)
+		phost->host_sgpio.pcb->scratch_space;
+	phost->host_sgpio.flags.sgpio_enabled = 1;
+
+	init_timer(&phost->host_sgpio.sgpio_timer);
+	phost->host_sgpio.sgpio_timer.data = (unsigned long)host_set;
+	nv_sgpio_set_timer(&phost->host_sgpio.sgpio_timer, 
+				NV_SGPIO_UPDATE_TICK);
+}
+
+static void nv_sgpio_set_timer(struct timer_list *ptimer, unsigned int
timeout_msec)
+{
+	if (!ptimer)
+		return;
+	ptimer->function = nv_sgpio_timer_handler;
+	ptimer->expires = msecs_to_jiffies(timeout_msec) + jiffies;
+	add_timer(ptimer);
+}
+
+static void nv_sgpio_timer_handler(unsigned long context)
+{
+
+	struct ata_host_set *host_set = (struct ata_host_set *)context;
+	struct nv_host *host;
+	u8 count, host_offset, port_offset;
+	union nv_sgpio_tx tx;
+	bool on_off;
+	unsigned long mask = 0xFFFF;
+	struct nv_port *port;
+
+	if (!host_set)
+		goto err_out;
+	else 
+		host = (struct nv_host *)host_set->private_data;
+
+	if (!host->host_sgpio.flags.sgpio_enabled)
+		goto err_out;
+
+	host_offset = nv_sgpio_tx_host_offset(host_set);
+
+	spin_lock(host->host_sgpio.share.plock);
+	tx = host->host_sgpio.pcb->tx[host_offset];
+	spin_unlock(host->host_sgpio.share.plock);
+
+	for (count = 0; count < host_set->n_ports; count++) {
+		struct ata_port *ap; 
+
+		ap = host_set->ports[count];
+	
+		if (!(ap && !(ap->flags & ATA_FLAG_DISABLED)))
+			continue;
+
+		port = (struct nv_port *)ap->private_data;
+		if (!port)
+			continue;            		
+		port_offset = nv_sgpio_tx_port_offset(ap);
+		on_off = GET_ACTIVITY(tx.tx_port[port_offset]);
+		if (nv_sgpio_update_led(&port->port_sgpio.activity,
&on_off)) {
+			tx.tx_port[port_offset] = 
+				SET_ACTIVITY(tx.tx_port[port_offset],
on_off);
+			host->host_sgpio.flags.need_update = 1;
+	       }
+	}
+
+
+	if (host->host_sgpio.flags.need_update) {
+		spin_lock(host->host_sgpio.share.plock);    
+		if (nv_sgpio_get_func(host_set) 
+			% NV_CNTRLR_SHARE_INIT == 0) {
+			host->host_sgpio.pcb->tx[host_offset].all &=
mask;
+			mask = mask << 16;
+			tx.all &= mask;
+		} else {
+			tx.all &= mask;
+			mask = mask << 16;
+			host->host_sgpio.pcb->tx[host_offset].all &=
mask;
+		}
+		host->host_sgpio.pcb->tx[host_offset].all |= tx.all;
+		spin_unlock(host->host_sgpio.share.plock);     
+ 
+		if (nv_sgpio_send_cmd(host, NV_SGPIO_CMD_WRITE_DATA)) { 
+			host->host_sgpio.flags.need_update = 0;
+			return;
+		}
+	} else {
+		nv_sgpio_set_timer(&host->host_sgpio.sgpio_timer, 
+				NV_SGPIO_UPDATE_TICK);
+	}
+err_out:
+	return;
+}
+
+static bool nv_sgpio_send_cmd(struct nv_host *host, u8 cmd)
+{
+	u8 csr;
+	unsigned long *ptstamp;
+
+	spin_lock(host->host_sgpio.share.plock);    
+	ptstamp = host->host_sgpio.share.ptstamp;
+	if (jiffies_to_msecs(jiffies - *ptstamp) >=
NV_SGPIO_MIN_UPDATE_DELTA) {
+		csr = nv_sgpio_get_csr((unsigned
long)host->host_sgpio.pcsr);
+		if ((GET_SGPIO_STATUS(csr) !=
NV_SGPIO_STATE_OPERATIONAL) ||
+			(GET_CMD_STATUS(csr) == NV_SGPIO_CMD_ACTIVE)) {
+			
+		} else {
+			host->host_sgpio.pcb->cr0 = 
+				SET_ENABLE(host->host_sgpio.pcb->cr0);
+			csr = 0;
+			csr = SET_CMD(csr, cmd);
+			nv_sgpio_set_csr(csr, 
+				(unsigned long)host->host_sgpio.pcsr);
+			*ptstamp = jiffies;
+		}
+		spin_unlock(host->host_sgpio.share.plock);
+		nv_sgpio_set_timer(&host->host_sgpio.sgpio_timer, 
+			NV_SGPIO_UPDATE_TICK);
+		return 1;
+	} else {
+		spin_unlock(host->host_sgpio.share.plock);
+		nv_sgpio_set_timer(&host->host_sgpio.sgpio_timer, 
+				(NV_SGPIO_MIN_UPDATE_DELTA - 
+				jiffies_to_msecs(jiffies - *ptstamp)));
+		return 0;
+	}
+}
+
+static bool nv_sgpio_update_led(struct nv_sgpio_led *led, bool *on_off)
+{
+	bool need_update = 0;
+
+	if (led->force_off > 0) {
+		led->force_off--;
+	} else if (led->flags.recent_activity ^ led->flags.last_state) {
+		*on_off = led->flags.recent_activity;
+		led->flags.last_state = led->flags.recent_activity;
+		need_update = 1;
+	} else if ((led->flags.recent_activity & led->flags.last_state)
&&
+		(led->last_cons_active >= NV_SGPIO_MAX_ACTIVITY_ON)) {
+		*on_off = NV_OFF;
+		led->flags.last_state = NV_OFF;
+		led->force_off = NV_SGPIO_MIN_FORCE_OFF;
+		need_update = 1;
+	}
+
+	if (*on_off) 
+		led->last_cons_active++;	
+	else
+		led->last_cons_active = 0;
+
+	led->flags.recent_activity = 0;
+	return need_update;
+}
+
+static void nv_sgpio_reset(u8  *pcsr)
+{
+	u8 csr;
+
+	csr = nv_sgpio_get_csr((unsigned long)pcsr);
+	if (GET_SGPIO_STATUS(csr) == NV_SGPIO_STATE_RESET) {
+		csr = 0;
+		csr = SET_CMD(csr, NV_SGPIO_CMD_RESET);
+		nv_sgpio_set_csr(csr, (unsigned long)pcsr);
+	}
+	csr = 0;
+	csr = SET_CMD(csr, NV_SGPIO_CMD_READ_PARAMS);
+	nv_sgpio_set_csr(csr, (unsigned long)pcsr);
+}
+
+static void nv_sgpio_host_cleanup(struct nv_host *host)
+{
+	u8 csr;
+
+	if (!host)
+		return;
+
+	if (host->host_sgpio.flags.sgpio_enabled) {
+		spin_lock(host->host_sgpio.share.plock);
+		host->host_sgpio.pcb->cr0 =
SET_ENABLE(host->host_sgpio.pcb->cr0);
+		csr = 0;
+		csr = SET_CMD(csr, NV_SGPIO_CMD_WRITE_DATA);
+		nv_sgpio_set_csr(csr,(unsigned
long)host->host_sgpio.pcsr);
+		spin_unlock(host->host_sgpio.share.plock);
+
+		if (timer_pending(&host->host_sgpio.sgpio_timer))
+			del_timer(&host->host_sgpio.sgpio_timer);
+		host->host_sgpio.flags.sgpio_enabled = 0;
+		host->host_sgpio.pcb->scratch_space = 0;
+	}
+}
+
+static void nv_sgpio_clear_all_leds(struct ata_port *ap)
+{
+	struct nv_port *port = ap->private_data;
+	struct nv_host *host;
+	u8 host_offset, port_offset;
+
+	if (!port || !ap->host_set)
+		return;
+	if (!ap->host_set->private_data)
+		return;
+
+	host = ap->host_set->private_data;
+	if (!host->host_sgpio.flags.sgpio_enabled)
+		return;
+
+	host_offset = nv_sgpio_tx_host_offset(ap->host_set);
+	port_offset = nv_sgpio_tx_port_offset(ap);
+
+	spin_lock(host->host_sgpio.share.plock);
+	host->host_sgpio.pcb->tx[host_offset].tx_port[port_offset] = 0;
+	host->host_sgpio.flags.need_update = 1;
+	spin_unlock(host->host_sgpio.share.plock);
+}
+
 static int __init nv_init(void)
 {
 	return pci_module_init(&nv_pci_driver);

Best Regards,

Kuan luo
MCP Driver - Shanghai ZPK
Tel: 86 21 61041936
kluo@nvidia.com

-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain
confidential information.  Any unauthorized review, use, disclosure or distribution
is prohibited.  If you are not the intended recipient, please contact the sender by
reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------

^ permalink raw reply	[flat|nested] 27+ messages in thread

* [PATCH] SCSI: Add the SGPIO support for sata_nv.c
@ 2006-10-31  8:43   ` Peer Chen
  0 siblings, 0 replies; 27+ messages in thread
From: Peer Chen @ 2006-10-31  8:43 UTC (permalink / raw)
  To: jeff, linux-kernel, linux-ide; +Cc: Kuan Luo

The following SGPIO patch for sata_nv.c is based on 2.6.18 kernel.
Signed-off by: Kuan Luo <kluo@nvidia.com>


--- linux-2.6.18/drivers/scsi/sata_nv.c.orig	2006-10-08
17:20:07.000000000 +0800
+++ linux-2.6.18/drivers/scsi/sata_nv.c	2006-10-13 11:12:01.000000000
+0800
@@ -80,6 +80,175 @@
 	NV_MCP_SATA_CFG_20_SATA_SPACE_EN = 0x04,
 };
 
+//sgpio
+// Sgpio defines
+// SGPIO state defines
+#define NV_SGPIO_STATE_RESET		0
+#define NV_SGPIO_STATE_OPERATIONAL	1
+#define NV_SGPIO_STATE_ERROR		2
+
+// SGPIO command opcodes
+#define NV_SGPIO_CMD_RESET		0
+#define NV_SGPIO_CMD_READ_PARAMS	1
+#define NV_SGPIO_CMD_READ_DATA		2
+#define NV_SGPIO_CMD_WRITE_DATA		3
+
+// SGPIO command status defines
+#define NV_SGPIO_CMD_OK			0
+#define NV_SGPIO_CMD_ACTIVE		1
+#define NV_SGPIO_CMD_ERR		2
+
+#define NV_SGPIO_UPDATE_TICK		90
+#define NV_SGPIO_MIN_UPDATE_DELTA	33
+#define NV_CNTRLR_SHARE_INIT		2
+#define NV_SGPIO_MAX_ACTIVITY_ON	20
+#define NV_SGPIO_MIN_FORCE_OFF		5
+#define NV_SGPIO_PCI_CSR_OFFSET		0x58
+#define NV_SGPIO_PCI_CB_OFFSET		0x5C
+#define NV_SGPIO_DFLT_CB_SIZE		256
+#define NV_ON 1
+#define NV_OFF 0
+#ifndef bool
+#define bool u8
+#endif
+
+
+#define BF_EXTRACT(v, off, bc)	\
+	((((u8)(v)) >> (off)) & ((1 << (bc)) - 1))
+
+#define BF_INS(v, ins, off, bc)				\
+	(((v) & ~((((1 << (bc)) - 1)) << (off))) |	\
+	(((u8)(ins)) << (off)))
+
+#define BF_EXTRACT_U32(v, off, bc)	\
+	((((u32)(v)) >> (off)) & ((1 << (bc)) - 1))
+
+#define BF_INS_U32(v, ins, off, bc)			\
+	(((v) & ~((((1 << (bc)) - 1)) << (off))) |	\
+	(((u32)(ins)) << (off)))
+
+#define GET_SGPIO_STATUS(v)	BF_EXTRACT(v, 0, 2)
+#define GET_CMD_STATUS(v)	BF_EXTRACT(v, 3, 2)
+#define GET_CMD(v)		BF_EXTRACT(v, 5, 3)
+#define SET_CMD(v, cmd)		BF_INS(v, cmd, 5, 3) 
+
+#define GET_ENABLE(v)		BF_EXTRACT_U32(v, 23, 1)
+#define SET_ENABLE(v)		BF_INS_U32(v, 1, 23, 1)
+
+// Needs to have a u8 bit-field insert.
+#define GET_ACTIVITY(v)		BF_EXTRACT(v, 5, 3)
+#define SET_ACTIVITY(v, on_off)	BF_INS(v, on_off, 5, 3)
+
+union nv_sgpio_nvcr 
+{
+	struct {
+		u8	init_cnt;
+		u8	cb_size;
+		u8	cbver;
+		u8	rsvd;
+	} bit;
+	u32	all;
+};
+
+union nv_sgpio_tx 
+{
+	u8	tx_port[4];
+	u32 	all;
+};
+
+struct nv_sgpio_cb 
+{
+	u64			scratch_space;
+	union nv_sgpio_nvcr	nvcr;
+	u32			cr0;
+	u32                     rsvd[4];
+	union nv_sgpio_tx       tx[2];
+};
+
+struct nv_sgpio_host_share
+{
+	spinlock_t	*plock;
+	unsigned long   *ptstamp;
+};
+
+struct nv_sgpio_host_flags
+{
+	u8	sgpio_enabled:1;
+	u8	need_update:1;
+	u8	rsvd:6;
+};
+	
+struct nv_host_sgpio
+{
+	struct nv_sgpio_host_flags	flags;
+	u8				*pcsr;
+	struct nv_sgpio_cb		*pcb;	
+	struct nv_sgpio_host_share	share;
+	struct timer_list		sgpio_timer;
+};
+
+struct nv_sgpio_port_flags
+{
+	u8	last_state:1;
+	u8	recent_activity:1;
+	u8	rsvd:6;
+};
+
+struct nv_sgpio_led 
+{
+	struct nv_sgpio_port_flags	flags;
+	u8				force_off;
+	u8      			last_cons_active;
+};
+
+struct nv_port_sgpio
+{
+	struct nv_sgpio_led	activity;
+};
+
+static spinlock_t	nv_sgpio_lock;
+static unsigned long	nv_sgpio_tstamp;
+
+static inline void nv_sgpio_set_csr(u8 csr, unsigned long pcsr)
+{
+	outb(csr, pcsr);
+}
+
+static inline u8 nv_sgpio_get_csr(unsigned long pcsr)
+{
+	return inb(pcsr);
+}
+
+static inline u8 nv_sgpio_get_func(struct ata_host_set *host_set)
+{
+	u8 devfn = (to_pci_dev(host_set->dev))->devfn;
+	return (PCI_FUNC(devfn));
+}
+
+static inline u8 nv_sgpio_tx_host_offset(struct ata_host_set *host_set)
+{
+	return (nv_sgpio_get_func(host_set)/NV_CNTRLR_SHARE_INIT);
+}
+
+static inline u8 nv_sgpio_calc_tx_offset(u8 cntrlr, u8 channel)
+{
+	return (sizeof(union nv_sgpio_tx) - (NV_CNTRLR_SHARE_INIT *
+		(cntrlr % NV_CNTRLR_SHARE_INIT)) - channel - 1);
+}
+
+static inline u8 nv_sgpio_tx_port_offset(struct ata_port *ap)
+{
+	u8 cntrlr = nv_sgpio_get_func(ap->host_set);
+	return (nv_sgpio_calc_tx_offset(cntrlr, ap->port_no));
+}
+
+static inline bool nv_sgpio_capable(const struct pci_device_id *ent)
+{
+	if (ent->device == PCI_DEVICE_ID_NVIDIA_NFORCE_MCP55_SATA2)
+		return 1;
+	else
+		return 0;
+}
 static int nv_init_one (struct pci_dev *pdev, const struct
pci_device_id *ent);
 static void nv_ck804_host_stop(struct ata_host_set *host_set);
 static irqreturn_t nv_generic_interrupt(int irq, void *dev_instance,
@@ -90,7 +259,10 @@
 				      struct pt_regs *regs);
 static u32 nv_scr_read (struct ata_port *ap, unsigned int sc_reg);
 static void nv_scr_write (struct ata_port *ap, unsigned int sc_reg, u32
val);
-
+static void nv_host_stop (struct ata_host_set *host_set);
+static int nv_port_start(struct ata_port *ap);
+static void nv_port_stop(struct ata_port *ap);
+static int nv_qc_issue(struct ata_queued_cmd *qc);
 static void nv_nf2_freeze(struct ata_port *ap);
 static void nv_nf2_thaw(struct ata_port *ap);
 static void nv_ck804_freeze(struct ata_port *ap);
@@ -147,6 +319,27 @@
 	{ 0, } /* terminate list */
 };
 
+struct nv_host
+{
+	unsigned long		host_flags;
+	struct nv_host_sgpio	host_sgpio;
+};
+
+struct nv_port
+{
+	struct nv_port_sgpio	port_sgpio;
+};
+
+// SGPIO function prototypes
+static void nv_sgpio_init(struct pci_dev *pdev, struct nv_host *phost);
+static void nv_sgpio_reset(u8 *pcsr);
+static void nv_sgpio_set_timer(struct timer_list *ptimer, 
+				unsigned int timeout_msec);
+static void nv_sgpio_timer_handler(unsigned long ptr);
+static void nv_sgpio_host_cleanup(struct nv_host *host);
+static bool nv_sgpio_update_led(struct nv_sgpio_led *led, bool
*on_off);
+static void nv_sgpio_clear_all_leds(struct ata_port *ap);
+static bool nv_sgpio_send_cmd(struct nv_host *host, u8 cmd);
 static struct pci_driver nv_pci_driver = {
 	.name			= DRV_NAME,
 	.id_table		= nv_pci_tbl,
@@ -184,7 +377,7 @@
 	.bmdma_stop		= ata_bmdma_stop,
 	.bmdma_status		= ata_bmdma_status,
 	.qc_prep		= ata_qc_prep,
-	.qc_issue		= ata_qc_issue_prot,
+	.qc_issue		= nv_qc_issue,
 	.freeze			= ata_bmdma_freeze,
 	.thaw			= ata_bmdma_thaw,
 	.error_handler		= nv_error_handler,
@@ -194,9 +387,9 @@
 	.irq_clear		= ata_bmdma_irq_clear,
 	.scr_read		= nv_scr_read,
 	.scr_write		= nv_scr_write,
-	.port_start		= ata_port_start,
-	.port_stop		= ata_port_stop,
-	.host_stop		= ata_pci_host_stop,
+	.port_start		= nv_port_start,
+	.port_stop		= nv_port_stop,
+	.host_stop		= nv_host_stop,
 };
 
 static const struct ata_port_operations nv_nf2_ops = {
@@ -211,7 +404,7 @@
 	.bmdma_stop		= ata_bmdma_stop,
 	.bmdma_status		= ata_bmdma_status,
 	.qc_prep		= ata_qc_prep,
-	.qc_issue		= ata_qc_issue_prot,
+	.qc_issue		= nv_qc_issue,
 	.freeze			= nv_nf2_freeze,
 	.thaw			= nv_nf2_thaw,
 	.error_handler		= nv_error_handler,
@@ -221,9 +414,9 @@
 	.irq_clear		= ata_bmdma_irq_clear,
 	.scr_read		= nv_scr_read,
 	.scr_write		= nv_scr_write,
-	.port_start		= ata_port_start,
-	.port_stop		= ata_port_stop,
-	.host_stop		= ata_pci_host_stop,
+	.port_start		= nv_port_start,
+	.port_stop		= nv_port_stop,
+	.host_stop		= nv_host_stop,
 };
 
 static const struct ata_port_operations nv_ck804_ops = {
@@ -238,7 +431,7 @@
 	.bmdma_stop		= ata_bmdma_stop,
 	.bmdma_status		= ata_bmdma_status,
 	.qc_prep		= ata_qc_prep,
-	.qc_issue		= ata_qc_issue_prot,
+	.qc_issue		= nv_qc_issue,
 	.freeze			= nv_ck804_freeze,
 	.thaw			= nv_ck804_thaw,
 	.error_handler		= nv_error_handler,
@@ -248,8 +441,8 @@
 	.irq_clear		= ata_bmdma_irq_clear,
 	.scr_read		= nv_scr_read,
 	.scr_write		= nv_scr_write,
-	.port_start		= ata_port_start,
-	.port_stop		= ata_port_stop,
+	.port_start		= nv_port_start,
+	.port_stop		= nv_port_stop,
 	.host_stop		= nv_ck804_host_stop,
 };
 
@@ -480,10 +673,10 @@
 	ata_bmdma_drive_eh(ap, ata_std_prereset, ata_std_softreset,
 			   nv_hardreset, ata_std_postreset);
 }
-
 static int nv_init_one (struct pci_dev *pdev, const struct
pci_device_id *ent)
 {
 	static int printed_version = 0;
+	struct nv_host *host;
 	struct ata_port_info *ppi;
 	struct ata_probe_ent *probe_ent;
 	int pci_dev_busy = 0;
@@ -525,6 +718,13 @@
 	if (!probe_ent)
 		goto err_out_regions;
 
+	host = kmalloc(sizeof(struct nv_host), GFP_KERNEL);
+	if (!host)
+		goto err_out_free_ent;
+
+	memset(host, 0, sizeof(struct nv_host));
+
+	probe_ent->private_data = host;
 	probe_ent->mmio_base = pci_iomap(pdev, 5, 0);
 	if (!probe_ent->mmio_base) {
 		rc = -EIO;
@@ -550,6 +750,8 @@
 	rc = ata_device_add(probe_ent);
 	if (rc != NV_PORTS)
 		goto err_out_iounmap;
+	if (nv_sgpio_capable(ent))
+		nv_sgpio_init(pdev, host);
 
 	kfree(probe_ent);
 
@@ -568,12 +770,56 @@
 	return rc;
 }
 
+static int nv_port_start(struct ata_port *ap)
+{
+	int stat;
+	struct nv_port *port;
+
+	stat = ata_port_start(ap);
+	if (stat) {
+		return stat;
+	}
+
+	port = kmalloc(sizeof(struct nv_port), GFP_KERNEL);
+	if (!port) 
+		goto err_out_no_free;
+
+	memset(port, 0, sizeof(struct nv_port));
+
+	ap->private_data = port;
+	return 0;
+
+err_out_no_free:
+	return 1;
+}
+
+static void nv_port_stop(struct ata_port *ap)
+{
+	nv_sgpio_clear_all_leds(ap);
+
+	if (ap->private_data) {
+		kfree(ap->private_data);
+		ap->private_data = NULL;
+	}
+	ata_port_stop(ap);
+}
+
+static int nv_qc_issue(struct ata_queued_cmd *qc)
+{
+	struct nv_port *port = qc->ap->private_data;
+
+	if (port) 
+		port->port_sgpio.activity.flags.recent_activity = 1;
+	return (ata_qc_issue_prot(qc));
+}
 static void nv_ck804_host_stop(struct ata_host_set *host_set)
 {
+	struct nv_host *host = host_set->private_data;
 	struct pci_dev *pdev = to_pci_dev(host_set->dev);
 	u8 regval;
 
 	/* disable SATA space for CK804 */
+	nv_sgpio_host_cleanup(host);
 	pci_read_config_byte(pdev, NV_MCP_SATA_CFG_20, &regval);
 	regval &= ~NV_MCP_SATA_CFG_20_SATA_SPACE_EN;
 	pci_write_config_byte(pdev, NV_MCP_SATA_CFG_20, regval);
@@ -581,6 +827,277 @@
 	ata_pci_host_stop(host_set);
 }
 
+static void nv_host_stop (struct ata_host_set *host_set)
+{
+	struct nv_host *host = host_set->private_data;
+
+
+	nv_sgpio_host_cleanup(host);
+	kfree(host);
+	ata_pci_host_stop(host_set);
+
+}
+
+static void nv_sgpio_init(struct pci_dev *pdev, struct nv_host *phost)
+{
+	u16 csr_add; 
+	u32 cb_add, temp32;
+	struct device *dev = pci_dev_to_dev(pdev);
+	struct ata_host_set *host_set = dev_get_drvdata(dev);
+	u8 pro=0;
+	
+	pci_read_config_word(pdev, NV_SGPIO_PCI_CSR_OFFSET, &csr_add);
+	pci_read_config_dword(pdev, NV_SGPIO_PCI_CB_OFFSET, &cb_add);
+	pci_read_config_byte(pdev, 0xA4, &pro);
+	
+	if (csr_add == 0 || cb_add == 0)
+		return;
+	
+	if (!(pro&0x40))
+		return;	
+		
+	temp32 = csr_add;
+	phost->host_sgpio.pcsr = (void *)temp32;
+	phost->host_sgpio.pcb = phys_to_virt(cb_add);
+
+	if (phost->host_sgpio.pcb->nvcr.bit.init_cnt!=0x2 ||
phost->host_sgpio.pcb->nvcr.bit.cbver!=0x0)
+		return;
+		
+	if (temp32 <=0x200 || temp32 >=0xFFFE )
+		return;
+		
+	if (cb_add<=0x80000 || cb_add>=0x9FC00)
+		return;
+	
+	if (phost->host_sgpio.pcb->scratch_space == 0) {
+		spin_lock_init(&nv_sgpio_lock);
+		phost->host_sgpio.share.plock = &nv_sgpio_lock;
+		phost->host_sgpio.share.ptstamp = &nv_sgpio_tstamp;
+		phost->host_sgpio.pcb->scratch_space = 
+			(unsigned long)&phost->host_sgpio.share;
+		spin_lock(phost->host_sgpio.share.plock);
+		nv_sgpio_reset(phost->host_sgpio.pcsr);
+		phost->host_sgpio.pcb->cr0 = 
+			SET_ENABLE(phost->host_sgpio.pcb->cr0);
+
+		spin_unlock(phost->host_sgpio.share.plock);
+	}
+
+	phost->host_sgpio.share = 
+		*(struct nv_sgpio_host_share *)(unsigned long)
+		phost->host_sgpio.pcb->scratch_space;
+	phost->host_sgpio.flags.sgpio_enabled = 1;
+
+	init_timer(&phost->host_sgpio.sgpio_timer);
+	phost->host_sgpio.sgpio_timer.data = (unsigned long)host_set;
+	nv_sgpio_set_timer(&phost->host_sgpio.sgpio_timer, 
+				NV_SGPIO_UPDATE_TICK);
+}
+
+static void nv_sgpio_set_timer(struct timer_list *ptimer, unsigned int
timeout_msec)
+{
+	if (!ptimer)
+		return;
+	ptimer->function = nv_sgpio_timer_handler;
+	ptimer->expires = msecs_to_jiffies(timeout_msec) + jiffies;
+	add_timer(ptimer);
+}
+
+static void nv_sgpio_timer_handler(unsigned long context)
+{
+
+	struct ata_host_set *host_set = (struct ata_host_set *)context;
+	struct nv_host *host;
+	u8 count, host_offset, port_offset;
+	union nv_sgpio_tx tx;
+	bool on_off;
+	unsigned long mask = 0xFFFF;
+	struct nv_port *port;
+
+	if (!host_set)
+		goto err_out;
+	else 
+		host = (struct nv_host *)host_set->private_data;
+
+	if (!host->host_sgpio.flags.sgpio_enabled)
+		goto err_out;
+
+	host_offset = nv_sgpio_tx_host_offset(host_set);
+
+	spin_lock(host->host_sgpio.share.plock);
+	tx = host->host_sgpio.pcb->tx[host_offset];
+	spin_unlock(host->host_sgpio.share.plock);
+
+	for (count = 0; count < host_set->n_ports; count++) {
+		struct ata_port *ap; 
+
+		ap = host_set->ports[count];
+	
+		if (!(ap && !(ap->flags & ATA_FLAG_DISABLED)))
+			continue;
+
+		port = (struct nv_port *)ap->private_data;
+		if (!port)
+			continue;            		
+		port_offset = nv_sgpio_tx_port_offset(ap);
+		on_off = GET_ACTIVITY(tx.tx_port[port_offset]);
+		if (nv_sgpio_update_led(&port->port_sgpio.activity,
&on_off)) {
+			tx.tx_port[port_offset] = 
+				SET_ACTIVITY(tx.tx_port[port_offset],
on_off);
+			host->host_sgpio.flags.need_update = 1;
+	       }
+	}
+
+
+	if (host->host_sgpio.flags.need_update) {
+		spin_lock(host->host_sgpio.share.plock);    
+		if (nv_sgpio_get_func(host_set) 
+			% NV_CNTRLR_SHARE_INIT == 0) {
+			host->host_sgpio.pcb->tx[host_offset].all &=
mask;
+			mask = mask << 16;
+			tx.all &= mask;
+		} else {
+			tx.all &= mask;
+			mask = mask << 16;
+			host->host_sgpio.pcb->tx[host_offset].all &=
mask;
+		}
+		host->host_sgpio.pcb->tx[host_offset].all |= tx.all;
+		spin_unlock(host->host_sgpio.share.plock);     
+ 
+		if (nv_sgpio_send_cmd(host, NV_SGPIO_CMD_WRITE_DATA)) { 
+			host->host_sgpio.flags.need_update = 0;
+			return;
+		}
+	} else {
+		nv_sgpio_set_timer(&host->host_sgpio.sgpio_timer, 
+				NV_SGPIO_UPDATE_TICK);
+	}
+err_out:
+	return;
+}
+
+static bool nv_sgpio_send_cmd(struct nv_host *host, u8 cmd)
+{
+	u8 csr;
+	unsigned long *ptstamp;
+
+	spin_lock(host->host_sgpio.share.plock);    
+	ptstamp = host->host_sgpio.share.ptstamp;
+	if (jiffies_to_msecs(jiffies - *ptstamp) >=
NV_SGPIO_MIN_UPDATE_DELTA) {
+		csr = nv_sgpio_get_csr((unsigned
long)host->host_sgpio.pcsr);
+		if ((GET_SGPIO_STATUS(csr) !=
NV_SGPIO_STATE_OPERATIONAL) ||
+			(GET_CMD_STATUS(csr) == NV_SGPIO_CMD_ACTIVE)) {
+			
+		} else {
+			host->host_sgpio.pcb->cr0 = 
+				SET_ENABLE(host->host_sgpio.pcb->cr0);
+			csr = 0;
+			csr = SET_CMD(csr, cmd);
+			nv_sgpio_set_csr(csr, 
+				(unsigned long)host->host_sgpio.pcsr);
+			*ptstamp = jiffies;
+		}
+		spin_unlock(host->host_sgpio.share.plock);
+		nv_sgpio_set_timer(&host->host_sgpio.sgpio_timer, 
+			NV_SGPIO_UPDATE_TICK);
+		return 1;
+	} else {
+		spin_unlock(host->host_sgpio.share.plock);
+		nv_sgpio_set_timer(&host->host_sgpio.sgpio_timer, 
+				(NV_SGPIO_MIN_UPDATE_DELTA - 
+				jiffies_to_msecs(jiffies - *ptstamp)));
+		return 0;
+	}
+}
+
+static bool nv_sgpio_update_led(struct nv_sgpio_led *led, bool *on_off)
+{
+	bool need_update = 0;
+
+	if (led->force_off > 0) {
+		led->force_off--;
+	} else if (led->flags.recent_activity ^ led->flags.last_state) {
+		*on_off = led->flags.recent_activity;
+		led->flags.last_state = led->flags.recent_activity;
+		need_update = 1;
+	} else if ((led->flags.recent_activity & led->flags.last_state)
&&
+		(led->last_cons_active >= NV_SGPIO_MAX_ACTIVITY_ON)) {
+		*on_off = NV_OFF;
+		led->flags.last_state = NV_OFF;
+		led->force_off = NV_SGPIO_MIN_FORCE_OFF;
+		need_update = 1;
+	}
+
+	if (*on_off) 
+		led->last_cons_active++;	
+	else
+		led->last_cons_active = 0;
+
+	led->flags.recent_activity = 0;
+	return need_update;
+}
+
+static void nv_sgpio_reset(u8  *pcsr)
+{
+	u8 csr;
+
+	csr = nv_sgpio_get_csr((unsigned long)pcsr);
+	if (GET_SGPIO_STATUS(csr) == NV_SGPIO_STATE_RESET) {
+		csr = 0;
+		csr = SET_CMD(csr, NV_SGPIO_CMD_RESET);
+		nv_sgpio_set_csr(csr, (unsigned long)pcsr);
+	}
+	csr = 0;
+	csr = SET_CMD(csr, NV_SGPIO_CMD_READ_PARAMS);
+	nv_sgpio_set_csr(csr, (unsigned long)pcsr);
+}
+
+static void nv_sgpio_host_cleanup(struct nv_host *host)
+{
+	u8 csr;
+
+	if (!host)
+		return;
+
+	if (host->host_sgpio.flags.sgpio_enabled) {
+		spin_lock(host->host_sgpio.share.plock);
+		host->host_sgpio.pcb->cr0 =
SET_ENABLE(host->host_sgpio.pcb->cr0);
+		csr = 0;
+		csr = SET_CMD(csr, NV_SGPIO_CMD_WRITE_DATA);
+		nv_sgpio_set_csr(csr,(unsigned
long)host->host_sgpio.pcsr);
+		spin_unlock(host->host_sgpio.share.plock);
+
+		if (timer_pending(&host->host_sgpio.sgpio_timer))
+			del_timer(&host->host_sgpio.sgpio_timer);
+		host->host_sgpio.flags.sgpio_enabled = 0;
+		host->host_sgpio.pcb->scratch_space = 0;
+	}
+}
+
+static void nv_sgpio_clear_all_leds(struct ata_port *ap)
+{
+	struct nv_port *port = ap->private_data;
+	struct nv_host *host;
+	u8 host_offset, port_offset;
+
+	if (!port || !ap->host_set)
+		return;
+	if (!ap->host_set->private_data)
+		return;
+
+	host = ap->host_set->private_data;
+	if (!host->host_sgpio.flags.sgpio_enabled)
+		return;
+
+	host_offset = nv_sgpio_tx_host_offset(ap->host_set);
+	port_offset = nv_sgpio_tx_port_offset(ap);
+
+	spin_lock(host->host_sgpio.share.plock);
+	host->host_sgpio.pcb->tx[host_offset].tx_port[port_offset] = 0;
+	host->host_sgpio.flags.need_update = 1;
+	spin_unlock(host->host_sgpio.share.plock);
+}
+
 static int __init nv_init(void)
 {
 	return pci_module_init(&nv_pci_driver);

Best Regards,

Kuan luo
MCP Driver - Shanghai ZPK
Tel: 86 21 61041936
kluo@nvidia.com

-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain
confidential information.  Any unauthorized review, use, disclosure or distribution
is prohibited.  If you are not the intended recipient, please contact the sender by
reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH] SCSI: Add the SGPIO support for sata_nv.c
  2006-10-31  8:43   ` Peer Chen
  (?)
@ 2006-10-31 10:40   ` Christoph Hellwig
  2006-10-31 12:37     ` Prakash Punnoor
  2006-11-07  9:55       ` Peer Chen
  -1 siblings, 2 replies; 27+ messages in thread
From: Christoph Hellwig @ 2006-10-31 10:40 UTC (permalink / raw)
  To: Peer Chen; +Cc: jeff, linux-kernel, linux-ide, Kuan Luo

On Tue, Oct 31, 2006 at 04:43:19PM +0800, Peer Chen wrote:
> The following SGPIO patch for sata_nv.c is based on 2.6.18 kernel.
> Signed-off by: Kuan Luo <kluo@nvidia.com>

First I'd like to say the patch subject isn't very informative.  For
a sata_nv patch it should read:

[PATCH] sata_nv: foooblah

and then the SGPIO in both the subject and description doesn't say anything
at all, what functionality or improvement does this patch actually add.

And in general send patches against latest git or -mm tree.  I don't know
how much the sata_nv driver changed since 2.6.18 but in general sending
patches against old kernel means they often can't be applied anymore.

> +//sgpio
> +// Sgpio defines
> +// SGPIO state defines

please use /* */ style comments.  Also these aren't exactly useful

> +#define NV_ON 1
> +#define NV_OFF 0
> +#ifndef bool
> +#define bool u8
> +#endif

please don't use your own bool types.

> +#define BF_EXTRACT(v, off, bc)	\
> +	((((u8)(v)) >> (off)) & ((1 << (bc)) - 1))
> +
> +#define BF_INS(v, ins, off, bc)				\
> +	(((v) & ~((((1 << (bc)) - 1)) << (off))) |	\
> +	(((u8)(ins)) << (off)))
> +
> +#define BF_EXTRACT_U32(v, off, bc)	\
> +	((((u32)(v)) >> (off)) & ((1 << (bc)) - 1))
> +
> +#define BF_INS_U32(v, ins, off, bc)			\
> +	(((v) & ~((((1 << (bc)) - 1)) << (off))) |	\
> +	(((u32)(ins)) << (off)))

please make such things inline functions.  Also they could have
more descriptive names.

> +union nv_sgpio_nvcr 
> +{
> +	struct {
> +		u8	init_cnt;
> +		u8	cb_size;
> +		u8	cbver;
> +		u8	rsvd;
> +	} bit;
> +	u32	all;
> +};
> +
> +union nv_sgpio_tx 
> +{
> +	u8	tx_port[4];
> +	u32 	all;
> +};
> +
> +struct nv_sgpio_cb 
> +{
> +	u64			scratch_space;
> +	union nv_sgpio_nvcr	nvcr;
> +	u32			cr0;
> +	u32                     rsvd[4];
> +	union nv_sgpio_tx       tx[2];
> +};
> +
> +struct nv_sgpio_host_share
> +{
> +	spinlock_t	*plock;
> +	unsigned long   *ptstamp;
> +};
> +
> +struct nv_sgpio_host_flags
> +{
> +	u8	sgpio_enabled:1;
> +	u8	need_update:1;
> +	u8	rsvd:6;
> +};
> +	
> +struct nv_host_sgpio
> +{
> +	struct nv_sgpio_host_flags	flags;
> +	u8				*pcsr;
> +	struct nv_sgpio_cb		*pcb;	
> +	struct nv_sgpio_host_share	share;
> +	struct timer_list		sgpio_timer;
> +};
> +
> +struct nv_sgpio_port_flags
> +{
> +	u8	last_state:1;
> +	u8	recent_activity:1;
> +	u8	rsvd:6;
> +};
> +
> +struct nv_sgpio_led 
> +{
> +	struct nv_sgpio_port_flags	flags;
> +	u8				force_off;
> +	u8      			last_cons_active;
> +};
> +
> +struct nv_port_sgpio
> +{
> +	struct nv_sgpio_led	activity;
> +};
> +
> +static spinlock_t	nv_sgpio_lock;

please use DEFINE_SPINLOCK to initialize the lock statically.

> +static unsigned long	nv_sgpio_tstamp;

> +static inline void nv_sgpio_set_csr(u8 csr, unsigned long pcsr)
> +{
> +	outb(csr, pcsr);
> +}
> +
> +static inline u8 nv_sgpio_get_csr(unsigned long pcsr)
> +{
> +	return inb(pcsr);
> +}

these helpers aren't useful at all, please remove them.

> +static inline u8 nv_sgpio_get_func(struct ata_host_set *host_set)
> +{
> +	u8 devfn = (to_pci_dev(host_set->dev))->devfn;
> +	return (PCI_FUNC(devfn));
> +}
> +
> +static inline u8 nv_sgpio_tx_host_offset(struct ata_host_set *host_set)
> +{
> +	return (nv_sgpio_get_func(host_set)/NV_CNTRLR_SHARE_INIT);
> +}
> +
> +static inline u8 nv_sgpio_calc_tx_offset(u8 cntrlr, u8 channel)
> +{
> +	return (sizeof(union nv_sgpio_tx) - (NV_CNTRLR_SHARE_INIT *
> +		(cntrlr % NV_CNTRLR_SHARE_INIT)) - channel - 1);
> +}
> +
> +static inline u8 nv_sgpio_tx_port_offset(struct ata_port *ap)
> +{
> +	u8 cntrlr = nv_sgpio_get_func(ap->host_set);
> +	return (nv_sgpio_calc_tx_offset(cntrlr, ap->port_no));
> +}
> +
> +static inline bool nv_sgpio_capable(const struct pci_device_id *ent)
> +{
> +	if (ent->device == PCI_DEVICE_ID_NVIDIA_NFORCE_MCP55_SATA2)
> +		return 1;
> +	else
> +		return 0;
> +}
>  static int nv_init_one (struct pci_dev *pdev, const struct
> pci_device_id *ent);
>  static void nv_ck804_host_stop(struct ata_host_set *host_set);
>  static irqreturn_t nv_generic_interrupt(int irq, void *dev_instance,
> @@ -90,7 +259,10 @@
>  				      struct pt_regs *regs);
>  static u32 nv_scr_read (struct ata_port *ap, unsigned int sc_reg);
>  static void nv_scr_write (struct ata_port *ap, unsigned int sc_reg, u32
> val);
> -
> +static void nv_host_stop (struct ata_host_set *host_set);
> +static int nv_port_start(struct ata_port *ap);
> +static void nv_port_stop(struct ata_port *ap);
> +static int nv_qc_issue(struct ata_queued_cmd *qc);
>  static void nv_nf2_freeze(struct ata_port *ap);
>  static void nv_nf2_thaw(struct ata_port *ap);
>  static void nv_ck804_freeze(struct ata_port *ap);
> @@ -147,6 +319,27 @@
>  	{ 0, } /* terminate list */
>  };
>  
> +struct nv_host
> +{
> +	unsigned long		host_flags;
> +	struct nv_host_sgpio	host_sgpio;
> +};
> +
> +struct nv_port
> +{
> +	struct nv_port_sgpio	port_sgpio;
> +};
> +
> +// SGPIO function prototypes
> +static void nv_sgpio_init(struct pci_dev *pdev, struct nv_host *phost);
> +static void nv_sgpio_reset(u8 *pcsr);
> +static void nv_sgpio_set_timer(struct timer_list *ptimer, 
> +				unsigned int timeout_msec);
> +static void nv_sgpio_timer_handler(unsigned long ptr);
> +static void nv_sgpio_host_cleanup(struct nv_host *host);
> +static bool nv_sgpio_update_led(struct nv_sgpio_led *led, bool
> *on_off);
> +static void nv_sgpio_clear_all_leds(struct ata_port *ap);
> +static bool nv_sgpio_send_cmd(struct nv_host *host, u8 cmd);
>  static struct pci_driver nv_pci_driver = {
>  	.name			= DRV_NAME,
>  	.id_table		= nv_pci_tbl,
> @@ -184,7 +377,7 @@
>  	.bmdma_stop		= ata_bmdma_stop,
>  	.bmdma_status		= ata_bmdma_status,
>  	.qc_prep		= ata_qc_prep,
> -	.qc_issue		= ata_qc_issue_prot,
> +	.qc_issue		= nv_qc_issue,
>  	.freeze			= ata_bmdma_freeze,
>  	.thaw			= ata_bmdma_thaw,
>  	.error_handler		= nv_error_handler,
> @@ -194,9 +387,9 @@
>  	.irq_clear		= ata_bmdma_irq_clear,
>  	.scr_read		= nv_scr_read,
>  	.scr_write		= nv_scr_write,
> -	.port_start		= ata_port_start,
> -	.port_stop		= ata_port_stop,
> -	.host_stop		= ata_pci_host_stop,
> +	.port_start		= nv_port_start,
> +	.port_stop		= nv_port_stop,
> +	.host_stop		= nv_host_stop,
>  };
>  
>  static const struct ata_port_operations nv_nf2_ops = {
> @@ -211,7 +404,7 @@
>  	.bmdma_stop		= ata_bmdma_stop,
>  	.bmdma_status		= ata_bmdma_status,
>  	.qc_prep		= ata_qc_prep,
> -	.qc_issue		= ata_qc_issue_prot,
> +	.qc_issue		= nv_qc_issue,
>  	.freeze			= nv_nf2_freeze,
>  	.thaw			= nv_nf2_thaw,
>  	.error_handler		= nv_error_handler,
> @@ -221,9 +414,9 @@
>  	.irq_clear		= ata_bmdma_irq_clear,
>  	.scr_read		= nv_scr_read,
>  	.scr_write		= nv_scr_write,
> -	.port_start		= ata_port_start,
> -	.port_stop		= ata_port_stop,
> -	.host_stop		= ata_pci_host_stop,
> +	.port_start		= nv_port_start,
> +	.port_stop		= nv_port_stop,
> +	.host_stop		= nv_host_stop,
>  };
>  
>  static const struct ata_port_operations nv_ck804_ops = {
> @@ -238,7 +431,7 @@
>  	.bmdma_stop		= ata_bmdma_stop,
>  	.bmdma_status		= ata_bmdma_status,
>  	.qc_prep		= ata_qc_prep,
> -	.qc_issue		= ata_qc_issue_prot,
> +	.qc_issue		= nv_qc_issue,
>  	.freeze			= nv_ck804_freeze,
>  	.thaw			= nv_ck804_thaw,
>  	.error_handler		= nv_error_handler,
> @@ -248,8 +441,8 @@
>  	.irq_clear		= ata_bmdma_irq_clear,
>  	.scr_read		= nv_scr_read,
>  	.scr_write		= nv_scr_write,
> -	.port_start		= ata_port_start,
> -	.port_stop		= ata_port_stop,
> +	.port_start		= nv_port_start,
> +	.port_stop		= nv_port_stop,
>  	.host_stop		= nv_ck804_host_stop,
>  };
>  
> @@ -480,10 +673,10 @@
>  	ata_bmdma_drive_eh(ap, ata_std_prereset, ata_std_softreset,
>  			   nv_hardreset, ata_std_postreset);
>  }
> -
>  static int nv_init_one (struct pci_dev *pdev, const struct
> pci_device_id *ent)
>  {
>  	static int printed_version = 0;
> +	struct nv_host *host;
>  	struct ata_port_info *ppi;
>  	struct ata_probe_ent *probe_ent;
>  	int pci_dev_busy = 0;
> @@ -525,6 +718,13 @@
>  	if (!probe_ent)
>  		goto err_out_regions;
>  
> +	host = kmalloc(sizeof(struct nv_host), GFP_KERNEL);
> +	if (!host)
> +		goto err_out_free_ent;
> +
> +	memset(host, 0, sizeof(struct nv_host));

Please use kzalloc().

> +static int nv_port_start(struct ata_port *ap)
> +{
> +	int stat;
> +	struct nv_port *port;
> +
> +	stat = ata_port_start(ap);
> +	if (stat) {
> +		return stat;
> +	}

useless braces.

> +
> +	port = kmalloc(sizeof(struct nv_port), GFP_KERNEL);
> +	if (!port) 
> +		goto err_out_no_free;
> +
> +	memset(port, 0, sizeof(struct nv_port));

Please use kzalloc again.

> +	return (ata_qc_issue_prot(qc));

no need for braces around the return value.

> +static void nv_sgpio_init(struct pci_dev *pdev, struct nv_host *phost)
> +{
> +	u16 csr_add; 
> +	u32 cb_add, temp32;
> +	struct device *dev = pci_dev_to_dev(pdev);
> +	struct ata_host_set *host_set = dev_get_drvdata(dev);
> +	u8 pro=0;

missing spacezs around the =.

> +	if (!(pro&0x40))

Again missing spaces.  Please take another look at Documentation/CodingStyle
for the preferred linux style.

> +		return;	
> +		
> +	temp32 = csr_add;
> +	phost->host_sgpio.pcsr = (void *)temp32;
> +	phost->host_sgpio.pcb = phys_to_virt(cb_add);

Use of phys_to_virt is generally a bug.  What are you trying to do here?

> +
> +	if (phost->host_sgpio.pcb->nvcr.bit.init_cnt!=0x2 ||
> phost->host_sgpio.pcb->nvcr.bit.cbver!=0x0)

in addition to the whitespace damage this line is far too long,
please break it up.


<skipping the rest of the file for now until it's made a little more
 readable>

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH] SCSI: Add the SGPIO support for sata_nv.c
  2006-10-31 10:40   ` Christoph Hellwig
@ 2006-10-31 12:37     ` Prakash Punnoor
  2006-11-07  9:55       ` Peer Chen
  1 sibling, 0 replies; 27+ messages in thread
From: Prakash Punnoor @ 2006-10-31 12:37 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: Peer Chen, jeff, linux-kernel, linux-ide, Kuan Luo

[-- Attachment #1: Type: text/plain, Size: 933 bytes --]

Am Dienstag 31 Oktober 2006 11:40 schrieb Christoph Hellwig:
> On Tue, Oct 31, 2006 at 04:43:19PM +0800, Peer Chen wrote:
> > +	u32 cb_add, temp32;
> > +	struct device *dev = pci_dev_to_dev(pdev);
> > +	struct ata_host_set *host_set = dev_get_drvdata(dev);
> > +	u8 pro=0;
> > +	if (!(pro&0x40))
> > +		return;
> > +
> > +	temp32 = csr_add;
> > +	phost->host_sgpio.pcsr = (void *)temp32;
> > +	phost->host_sgpio.pcb = phys_to_virt(cb_add);
>
> Use of phys_to_virt is generally a bug.  What are you trying to do here?

I am also wondering whether casting of temp32 to a pointer is very 64bit 
friendly? At least my compiler on x86_64 gives a warning...

I also found 

http://lkml.org/lkml/2006/8/21/324

which looks alike at first glance and it seems NVidia didn't really fix what 
Andrew Morton told them to do so...

Cheers,
-- 
(°=                 =°)
//\ Prakash Punnoor /\\
V_/                 \_V

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH] SCSI: Add the SGPIO support for sata_nv.c
  2006-10-31  8:43   ` Peer Chen
  (?)
  (?)
@ 2006-11-01  1:39   ` Jeff Garzik
  -1 siblings, 0 replies; 27+ messages in thread
From: Jeff Garzik @ 2006-11-01  1:39 UTC (permalink / raw)
  To: Peer Chen; +Cc: linux-kernel, linux-ide, Kuan Luo

Peer Chen wrote:
> The following SGPIO patch for sata_nv.c is based on 2.6.18 kernel.
> Signed-off by: Kuan Luo <kluo@nvidia.com>

ACK what Christoph Hellwig said:  please describe in detail what sgpio 
is, and why it is needed.

This is a non-trivial patch, with basically no explanation at all.

	Jeff

^ permalink raw reply	[flat|nested] 27+ messages in thread

* RE: [PATCH] SCSI: Add the SGPIO support for sata_nv.c
  2006-10-31 10:40   ` Christoph Hellwig
@ 2006-11-07  9:55       ` Peer Chen
  2006-11-07  9:55       ` Peer Chen
  1 sibling, 0 replies; 27+ messages in thread
From: Peer Chen @ 2006-11-07  9:55 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: jeff, linux-kernel, linux-ide, Kuan Luo

[-- Attachment #1: Type: text/plain, Size: 11352 bytes --]

Modified and resent out the patch as attachment.
Description about the patch:
Add SGPIO support in sata_nv.c.
SGPIO (Serial General Purpose Input Output) is a sideband serial 4-wire
interface that a storage controller uses to communicate with a storage
enclosure management controller, primarily to control activity and
status LEDs that are located within drive bays or on a storage
backplane. SGPIO is defined by [SFF8485].
In this patch,we drive the LEDs to blink when read/write operation
happen on SATA drives connect the corresponding ports on MCP55 board.
==========
The patch will be applied to kernel 2.6.19-rc4-git9.

Singed-off-by: Kuan Luo <kluo@nvidia.com>
Singed-off-by: Peer Chen <pchen@nvidia.com>


BRs
Peer Chen

-----Original Message-----
From: Christoph Hellwig [mailto:hch@infradead.org] 
Sent: Tuesday, October 31, 2006 6:41 PM
To: Peer Chen
Cc: jeff@garzik.org; linux-kernel@vger.kernel.org;
linux-ide@vger.kernel.org; Kuan Luo
Subject: Re: [PATCH] SCSI: Add the SGPIO support for sata_nv.c

On Tue, Oct 31, 2006 at 04:43:19PM +0800, Peer Chen wrote:
> The following SGPIO patch for sata_nv.c is based on 2.6.18 kernel.
> Signed-off by: Kuan Luo <kluo@nvidia.com>

First I'd like to say the patch subject isn't very informative.  For
a sata_nv patch it should read:

[PATCH] sata_nv: foooblah

and then the SGPIO in both the subject and description doesn't say
anything
at all, what functionality or improvement does this patch actually add.

And in general send patches against latest git or -mm tree.  I don't
know
how much the sata_nv driver changed since 2.6.18 but in general sending
patches against old kernel means they often can't be applied anymore.

> +//sgpio
> +// Sgpio defines
> +// SGPIO state defines

please use /* */ style comments.  Also these aren't exactly useful

> +#define NV_ON 1
> +#define NV_OFF 0
> +#ifndef bool
> +#define bool u8
> +#endif

please don't use your own bool types.

> +#define BF_EXTRACT(v, off, bc)	\
> +	((((u8)(v)) >> (off)) & ((1 << (bc)) - 1))
> +
> +#define BF_INS(v, ins, off, bc)				\
> +	(((v) & ~((((1 << (bc)) - 1)) << (off))) |	\
> +	(((u8)(ins)) << (off)))
> +
> +#define BF_EXTRACT_U32(v, off, bc)	\
> +	((((u32)(v)) >> (off)) & ((1 << (bc)) - 1))
> +
> +#define BF_INS_U32(v, ins, off, bc)			\
> +	(((v) & ~((((1 << (bc)) - 1)) << (off))) |	\
> +	(((u32)(ins)) << (off)))

please make such things inline functions.  Also they could have
more descriptive names.

> +union nv_sgpio_nvcr 
> +{
> +	struct {
> +		u8	init_cnt;
> +		u8	cb_size;
> +		u8	cbver;
> +		u8	rsvd;
> +	} bit;
> +	u32	all;
> +};
> +
> +union nv_sgpio_tx 
> +{
> +	u8	tx_port[4];
> +	u32 	all;
> +};
> +
> +struct nv_sgpio_cb 
> +{
> +	u64			scratch_space;
> +	union nv_sgpio_nvcr	nvcr;
> +	u32			cr0;
> +	u32                     rsvd[4];
> +	union nv_sgpio_tx       tx[2];
> +};
> +
> +struct nv_sgpio_host_share
> +{
> +	spinlock_t	*plock;
> +	unsigned long   *ptstamp;
> +};
> +
> +struct nv_sgpio_host_flags
> +{
> +	u8	sgpio_enabled:1;
> +	u8	need_update:1;
> +	u8	rsvd:6;
> +};
> +	
> +struct nv_host_sgpio
> +{
> +	struct nv_sgpio_host_flags	flags;
> +	u8				*pcsr;
> +	struct nv_sgpio_cb		*pcb;	
> +	struct nv_sgpio_host_share	share;
> +	struct timer_list		sgpio_timer;
> +};
> +
> +struct nv_sgpio_port_flags
> +{
> +	u8	last_state:1;
> +	u8	recent_activity:1;
> +	u8	rsvd:6;
> +};
> +
> +struct nv_sgpio_led 
> +{
> +	struct nv_sgpio_port_flags	flags;
> +	u8				force_off;
> +	u8      			last_cons_active;
> +};
> +
> +struct nv_port_sgpio
> +{
> +	struct nv_sgpio_led	activity;
> +};
> +
> +static spinlock_t	nv_sgpio_lock;

please use DEFINE_SPINLOCK to initialize the lock statically.

> +static unsigned long	nv_sgpio_tstamp;

> +static inline void nv_sgpio_set_csr(u8 csr, unsigned long pcsr)
> +{
> +	outb(csr, pcsr);
> +}
> +
> +static inline u8 nv_sgpio_get_csr(unsigned long pcsr)
> +{
> +	return inb(pcsr);
> +}

these helpers aren't useful at all, please remove them.

> +static inline u8 nv_sgpio_get_func(struct ata_host_set *host_set)
> +{
> +	u8 devfn = (to_pci_dev(host_set->dev))->devfn;
> +	return (PCI_FUNC(devfn));
> +}
> +
> +static inline u8 nv_sgpio_tx_host_offset(struct ata_host_set
*host_set)
> +{
> +	return (nv_sgpio_get_func(host_set)/NV_CNTRLR_SHARE_INIT);
> +}
> +
> +static inline u8 nv_sgpio_calc_tx_offset(u8 cntrlr, u8 channel)
> +{
> +	return (sizeof(union nv_sgpio_tx) - (NV_CNTRLR_SHARE_INIT *
> +		(cntrlr % NV_CNTRLR_SHARE_INIT)) - channel - 1);
> +}
> +
> +static inline u8 nv_sgpio_tx_port_offset(struct ata_port *ap)
> +{
> +	u8 cntrlr = nv_sgpio_get_func(ap->host_set);
> +	return (nv_sgpio_calc_tx_offset(cntrlr, ap->port_no));
> +}
> +
> +static inline bool nv_sgpio_capable(const struct pci_device_id *ent)
> +{
> +	if (ent->device == PCI_DEVICE_ID_NVIDIA_NFORCE_MCP55_SATA2)
> +		return 1;
> +	else
> +		return 0;
> +}
>  static int nv_init_one (struct pci_dev *pdev, const struct
> pci_device_id *ent);
>  static void nv_ck804_host_stop(struct ata_host_set *host_set);
>  static irqreturn_t nv_generic_interrupt(int irq, void *dev_instance,
> @@ -90,7 +259,10 @@
>  				      struct pt_regs *regs);
>  static u32 nv_scr_read (struct ata_port *ap, unsigned int sc_reg);
>  static void nv_scr_write (struct ata_port *ap, unsigned int sc_reg,
u32
> val);
> -
> +static void nv_host_stop (struct ata_host_set *host_set);
> +static int nv_port_start(struct ata_port *ap);
> +static void nv_port_stop(struct ata_port *ap);
> +static int nv_qc_issue(struct ata_queued_cmd *qc);
>  static void nv_nf2_freeze(struct ata_port *ap);
>  static void nv_nf2_thaw(struct ata_port *ap);
>  static void nv_ck804_freeze(struct ata_port *ap);
> @@ -147,6 +319,27 @@
>  	{ 0, } /* terminate list */
>  };
>  
> +struct nv_host
> +{
> +	unsigned long		host_flags;
> +	struct nv_host_sgpio	host_sgpio;
> +};
> +
> +struct nv_port
> +{
> +	struct nv_port_sgpio	port_sgpio;
> +};
> +
> +// SGPIO function prototypes
> +static void nv_sgpio_init(struct pci_dev *pdev, struct nv_host
*phost);
> +static void nv_sgpio_reset(u8 *pcsr);
> +static void nv_sgpio_set_timer(struct timer_list *ptimer, 
> +				unsigned int timeout_msec);
> +static void nv_sgpio_timer_handler(unsigned long ptr);
> +static void nv_sgpio_host_cleanup(struct nv_host *host);
> +static bool nv_sgpio_update_led(struct nv_sgpio_led *led, bool
> *on_off);
> +static void nv_sgpio_clear_all_leds(struct ata_port *ap);
> +static bool nv_sgpio_send_cmd(struct nv_host *host, u8 cmd);
>  static struct pci_driver nv_pci_driver = {
>  	.name			= DRV_NAME,
>  	.id_table		= nv_pci_tbl,
> @@ -184,7 +377,7 @@
>  	.bmdma_stop		= ata_bmdma_stop,
>  	.bmdma_status		= ata_bmdma_status,
>  	.qc_prep		= ata_qc_prep,
> -	.qc_issue		= ata_qc_issue_prot,
> +	.qc_issue		= nv_qc_issue,
>  	.freeze			= ata_bmdma_freeze,
>  	.thaw			= ata_bmdma_thaw,
>  	.error_handler		= nv_error_handler,
> @@ -194,9 +387,9 @@
>  	.irq_clear		= ata_bmdma_irq_clear,
>  	.scr_read		= nv_scr_read,
>  	.scr_write		= nv_scr_write,
> -	.port_start		= ata_port_start,
> -	.port_stop		= ata_port_stop,
> -	.host_stop		= ata_pci_host_stop,
> +	.port_start		= nv_port_start,
> +	.port_stop		= nv_port_stop,
> +	.host_stop		= nv_host_stop,
>  };
>  
>  static const struct ata_port_operations nv_nf2_ops = {
> @@ -211,7 +404,7 @@
>  	.bmdma_stop		= ata_bmdma_stop,
>  	.bmdma_status		= ata_bmdma_status,
>  	.qc_prep		= ata_qc_prep,
> -	.qc_issue		= ata_qc_issue_prot,
> +	.qc_issue		= nv_qc_issue,
>  	.freeze			= nv_nf2_freeze,
>  	.thaw			= nv_nf2_thaw,
>  	.error_handler		= nv_error_handler,
> @@ -221,9 +414,9 @@
>  	.irq_clear		= ata_bmdma_irq_clear,
>  	.scr_read		= nv_scr_read,
>  	.scr_write		= nv_scr_write,
> -	.port_start		= ata_port_start,
> -	.port_stop		= ata_port_stop,
> -	.host_stop		= ata_pci_host_stop,
> +	.port_start		= nv_port_start,
> +	.port_stop		= nv_port_stop,
> +	.host_stop		= nv_host_stop,
>  };
>  
>  static const struct ata_port_operations nv_ck804_ops = {
> @@ -238,7 +431,7 @@
>  	.bmdma_stop		= ata_bmdma_stop,
>  	.bmdma_status		= ata_bmdma_status,
>  	.qc_prep		= ata_qc_prep,
> -	.qc_issue		= ata_qc_issue_prot,
> +	.qc_issue		= nv_qc_issue,
>  	.freeze			= nv_ck804_freeze,
>  	.thaw			= nv_ck804_thaw,
>  	.error_handler		= nv_error_handler,
> @@ -248,8 +441,8 @@
>  	.irq_clear		= ata_bmdma_irq_clear,
>  	.scr_read		= nv_scr_read,
>  	.scr_write		= nv_scr_write,
> -	.port_start		= ata_port_start,
> -	.port_stop		= ata_port_stop,
> +	.port_start		= nv_port_start,
> +	.port_stop		= nv_port_stop,
>  	.host_stop		= nv_ck804_host_stop,
>  };
>  
> @@ -480,10 +673,10 @@
>  	ata_bmdma_drive_eh(ap, ata_std_prereset, ata_std_softreset,
>  			   nv_hardreset, ata_std_postreset);
>  }
> -
>  static int nv_init_one (struct pci_dev *pdev, const struct
> pci_device_id *ent)
>  {
>  	static int printed_version = 0;
> +	struct nv_host *host;
>  	struct ata_port_info *ppi;
>  	struct ata_probe_ent *probe_ent;
>  	int pci_dev_busy = 0;
> @@ -525,6 +718,13 @@
>  	if (!probe_ent)
>  		goto err_out_regions;
>  
> +	host = kmalloc(sizeof(struct nv_host), GFP_KERNEL);
> +	if (!host)
> +		goto err_out_free_ent;
> +
> +	memset(host, 0, sizeof(struct nv_host));

Please use kzalloc().

> +static int nv_port_start(struct ata_port *ap)
> +{
> +	int stat;
> +	struct nv_port *port;
> +
> +	stat = ata_port_start(ap);
> +	if (stat) {
> +		return stat;
> +	}

useless braces.

> +
> +	port = kmalloc(sizeof(struct nv_port), GFP_KERNEL);
> +	if (!port) 
> +		goto err_out_no_free;
> +
> +	memset(port, 0, sizeof(struct nv_port));

Please use kzalloc again.

> +	return (ata_qc_issue_prot(qc));

no need for braces around the return value.

> +static void nv_sgpio_init(struct pci_dev *pdev, struct nv_host
*phost)
> +{
> +	u16 csr_add; 
> +	u32 cb_add, temp32;
> +	struct device *dev = pci_dev_to_dev(pdev);
> +	struct ata_host_set *host_set = dev_get_drvdata(dev);
> +	u8 pro=0;

missing spacezs around the =.

> +	if (!(pro&0x40))

Again missing spaces.  Please take another look at
Documentation/CodingStyle
for the preferred linux style.

> +		return;	
> +		
> +	temp32 = csr_add;
> +	phost->host_sgpio.pcsr = (void *)temp32;
> +	phost->host_sgpio.pcb = phys_to_virt(cb_add);

Use of phys_to_virt is generally a bug.  What are you trying to do here?

> +
> +	if (phost->host_sgpio.pcb->nvcr.bit.init_cnt!=0x2 ||
> phost->host_sgpio.pcb->nvcr.bit.cbver!=0x0)

in addition to the whitespace damage this line is far too long,
please break it up.


<skipping the rest of the file for now until it's made a little more
 readable>

-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain
confidential information.  Any unauthorized review, use, disclosure or distribution
is prohibited.  If you are not the intended recipient, please contact the sender by
reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------

[-- Attachment #2: sgpio-patch-2.6.19-rc4-git9 --]
[-- Type: application/octet-stream, Size: 15478 bytes --]

--- linux-2.6.19-rc4-git9/drivers/ata/sata_nv.c.orig	2006-11-06 08:47:49.000000000 +0800
+++ linux-2.6.19-rc4-git9/drivers/ata/sata_nv.c	2006-11-07 08:36:54.000000000 +0800
@@ -80,6 +80,176 @@
 	NV_MCP_SATA_CFG_20_SATA_SPACE_EN = 0x04,
 };
 
+/* sgpio
+* Sgpio defines
+* SGPIO state defines
+*/
+#define NV_SGPIO_STATE_RESET		0
+#define NV_SGPIO_STATE_OPERATIONAL	1
+#define NV_SGPIO_STATE_ERROR		2
+
+/* SGPIO command opcodes */
+#define NV_SGPIO_CMD_RESET		0
+#define NV_SGPIO_CMD_READ_PARAMS	1
+#define NV_SGPIO_CMD_READ_DATA		2
+#define NV_SGPIO_CMD_WRITE_DATA		3
+
+/* SGPIO command status defines */
+#define NV_SGPIO_CMD_OK			0
+#define NV_SGPIO_CMD_ACTIVE		1
+#define NV_SGPIO_CMD_ERR		2
+
+#define NV_SGPIO_UPDATE_TICK		90
+#define NV_SGPIO_MIN_UPDATE_DELTA	33
+#define NV_CNTRLR_SHARE_INIT		2
+#define NV_SGPIO_MAX_ACTIVITY_ON	20
+#define NV_SGPIO_MIN_FORCE_OFF		5
+#define NV_SGPIO_PCI_CSR_OFFSET		0x58
+#define NV_SGPIO_PCI_CB_OFFSET		0x5C
+#define NV_SGPIO_DFLT_CB_SIZE		256
+#define NV_ON 1
+#define NV_OFF 0
+
+
+
+static inline u8 bf_extract(u8 value, u8 offset, u8 bit_count)
+{
+	return ((((u8)(value)) >> (offset)) & ((1 << (bit_count)) - 1));
+}
+
+static inline u8 bf_ins(u8 value, u8 ins, u8 offset, u8 bit_count)
+{
+	return 	(((value) & ~((((1 << (bit_count)) - 1)) << (offset))) |
+						(((u8)(ins)) << (offset)));
+}
+
+static inline u32 bf_extract_u32(u32 value, u8 offset, u8 bit_count)
+{
+	return ((((u32)(value)) >> (offset)) & ((1 << (bit_count)) - 1));
+
+}
+static inline u32 bf_ins_u32(u32 value, u32 ins, u8 offset, u8 bit_count)
+{
+	return 	(((value) & ~((((1 << (bit_count)) - 1)) << (offset))) |
+						(((u32)(ins)) << (offset)));
+}
+
+#define GET_SGPIO_STATUS(v)	bf_extract(v, 0, 2)
+#define GET_CMD_STATUS(v)	bf_extract(v, 3, 2)
+#define GET_CMD(v)		bf_extract(v, 5, 3)
+#define SET_CMD(v, cmd)		bf_ins(v, cmd, 5, 3) 
+
+#define GET_ENABLE(v)		bf_extract(v, 23, 1)
+#define SET_ENABLE(v)		bf_ins_u32(v, 1, 23, 1)
+
+/* Needs to have a u8 bit-field insert. */
+#define GET_ACTIVITY(v)		bf_extract(v, 5, 3)
+#define SET_ACTIVITY(v, on_off)	bf_ins(v, on_off, 5, 3)
+
+
+
+union nv_sgpio_nvcr 
+{
+	struct {
+		u8	init_cnt;
+		u8	cb_size;
+		u8	cbver;
+		u8	rsvd;
+	} bit;
+	u32	all;
+};
+
+union nv_sgpio_tx 
+{
+	u8	tx_port[4];
+	u32 	all;
+};
+
+struct nv_sgpio_cb 
+{
+	u64			scratch_space;
+	union nv_sgpio_nvcr	nvcr;
+	u32			cr0;
+	u32                     rsvd[4];
+	union nv_sgpio_tx       tx[2];
+};
+
+struct nv_sgpio_host_share
+{
+	spinlock_t	*plock;
+	unsigned long   *ptstamp;
+};
+
+struct nv_sgpio_host_flags
+{
+	u8	sgpio_enabled:1;
+	u8	need_update:1;
+	u8	rsvd:6;
+};
+	
+struct nv_host_sgpio
+{
+	struct nv_sgpio_host_flags	flags;
+	u8				*pcsr;
+	struct nv_sgpio_cb		*pcb;	
+	struct nv_sgpio_host_share	share;
+	struct timer_list		sgpio_timer;
+};
+
+struct nv_sgpio_port_flags
+{
+	u8	last_state:1;
+	u8	recent_activity:1;
+	u8	rsvd:6;
+};
+
+struct nv_sgpio_led 
+{
+	struct nv_sgpio_port_flags	flags;
+	u8				force_off;
+	u8      			last_cons_active;
+};
+
+struct nv_port_sgpio
+{
+	struct nv_sgpio_led	activity;
+};
+
+static DEFINE_SPINLOCK(nv_sgpio_lock);
+
+static unsigned long	nv_sgpio_tstamp;
+
+
+static inline u8 nv_sgpio_get_func(struct ata_host *host)
+{
+	u8 devfn = (to_pci_dev(host->dev))->devfn;
+	return (PCI_FUNC(devfn));
+}
+
+static inline u8 nv_sgpio_tx_host_offset(struct ata_host *host)
+{
+	return (nv_sgpio_get_func(host)/NV_CNTRLR_SHARE_INIT);
+}
+
+static inline u8 nv_sgpio_calc_tx_offset(u8 cntrlr, u8 channel)
+{
+	return (sizeof(union nv_sgpio_tx) - (NV_CNTRLR_SHARE_INIT *
+		(cntrlr % NV_CNTRLR_SHARE_INIT)) - channel - 1);
+}
+
+static inline u8 nv_sgpio_tx_port_offset(struct ata_port *ap)
+{
+	u8 cntrlr = nv_sgpio_get_func(ap->host);
+	return (nv_sgpio_calc_tx_offset(cntrlr, ap->port_no));
+}
+
+static inline u8 nv_sgpio_capable(const struct pci_device_id *ent)
+{
+	if (ent->device == PCI_DEVICE_ID_NVIDIA_NFORCE_MCP55_SATA2)
+		return 1;
+	else
+		return 0;
+}
 static int nv_init_one (struct pci_dev *pdev, const struct pci_device_id *ent);
 static void nv_ck804_host_stop(struct ata_host *host);
 static irqreturn_t nv_generic_interrupt(int irq, void *dev_instance);
@@ -87,7 +257,10 @@
 static irqreturn_t nv_ck804_interrupt(int irq, void *dev_instance);
 static u32 nv_scr_read (struct ata_port *ap, unsigned int sc_reg);
 static void nv_scr_write (struct ata_port *ap, unsigned int sc_reg, u32 val);
-
+static void nv_host_stop (struct ata_host *host);
+static int nv_port_start(struct ata_port *ap);
+static void nv_port_stop(struct ata_port *ap);
+static unsigned int nv_qc_issue(struct ata_queued_cmd *qc);
 static void nv_nf2_freeze(struct ata_port *ap);
 static void nv_nf2_thaw(struct ata_port *ap);
 static void nv_ck804_freeze(struct ata_port *ap);
@@ -135,6 +308,27 @@
 	{ } /* terminate list */
 };
 
+struct nv_host
+{
+	unsigned long		host_flags;
+	struct nv_host_sgpio	host_sgpio;
+};
+
+struct nv_port
+{
+	struct nv_port_sgpio	port_sgpio;
+};
+
+/* SGPIO function prototypes */
+static void nv_sgpio_init(struct pci_dev *pdev, struct nv_host *phost);
+static void nv_sgpio_reset(u8 *pcsr);
+static void nv_sgpio_set_timer(struct timer_list *ptimer, 
+				unsigned int timeout_msec);
+static void nv_sgpio_timer_handler(unsigned long ptr);
+static void nv_sgpio_host_cleanup(struct nv_host *host);
+static u8 nv_sgpio_update_led(struct nv_sgpio_led *led, u8 *on_off);
+static void nv_sgpio_clear_all_leds(struct ata_port *ap);
+static u8 nv_sgpio_send_cmd(struct nv_host *host, u8 cmd);
 static struct pci_driver nv_pci_driver = {
 	.name			= DRV_NAME,
 	.id_table		= nv_pci_tbl,
@@ -172,7 +366,7 @@
 	.bmdma_stop		= ata_bmdma_stop,
 	.bmdma_status		= ata_bmdma_status,
 	.qc_prep		= ata_qc_prep,
-	.qc_issue		= ata_qc_issue_prot,
+	.qc_issue		= nv_qc_issue,
 	.freeze			= ata_bmdma_freeze,
 	.thaw			= ata_bmdma_thaw,
 	.error_handler		= nv_error_handler,
@@ -182,9 +376,9 @@
 	.irq_clear		= ata_bmdma_irq_clear,
 	.scr_read		= nv_scr_read,
 	.scr_write		= nv_scr_write,
-	.port_start		= ata_port_start,
-	.port_stop		= ata_port_stop,
-	.host_stop		= ata_pci_host_stop,
+	.port_start		= nv_port_start,
+	.port_stop		= nv_port_stop,
+	.host_stop		= nv_host_stop,
 };
 
 static const struct ata_port_operations nv_nf2_ops = {
@@ -465,10 +659,10 @@
 	ata_bmdma_drive_eh(ap, ata_std_prereset, ata_std_softreset,
 			   nv_hardreset, ata_std_postreset);
 }
-
 static int nv_init_one (struct pci_dev *pdev, const struct pci_device_id *ent)
 {
 	static int printed_version = 0;
+	struct nv_host *host;
 	struct ata_port_info *ppi[2];
 	struct ata_probe_ent *probe_ent;
 	int pci_dev_busy = 0;
@@ -510,6 +704,11 @@
 	if (!probe_ent)
 		goto err_out_regions;
 
+	host = kzalloc(sizeof(struct nv_host), GFP_KERNEL);
+	if (!host)
+		goto err_out_free_ent;
+
+	probe_ent->private_data = host;
 	probe_ent->mmio_base = pci_iomap(pdev, 5, 0);
 	if (!probe_ent->mmio_base) {
 		rc = -EIO;
@@ -535,6 +734,8 @@
 	rc = ata_device_add(probe_ent);
 	if (rc != NV_PORTS)
 		goto err_out_iounmap;
+	if (nv_sgpio_capable(ent))
+		nv_sgpio_init(pdev, host);
 
 	kfree(probe_ent);
 
@@ -553,6 +754,45 @@
 	return rc;
 }
 
+static int nv_port_start(struct ata_port *ap)
+{
+	int stat;
+	struct nv_port *port;
+
+	stat = ata_port_start(ap);
+	if (stat)
+		return stat;
+	
+	port = kzalloc(sizeof(struct nv_port), GFP_KERNEL);
+	if (!port) 
+		goto err_out_no_free;
+
+	ap->private_data = port;
+	return 0;
+
+err_out_no_free:
+	return 1;
+}
+
+static void nv_port_stop(struct ata_port *ap)
+{
+	nv_sgpio_clear_all_leds(ap);
+
+	if (ap->private_data) {
+		kfree(ap->private_data);
+		ap->private_data = NULL;
+	}
+	ata_port_stop(ap);
+}
+
+static unsigned int nv_qc_issue(struct ata_queued_cmd *qc)
+{
+	struct nv_port *port = qc->ap->private_data;
+
+	if (port) 
+		port->port_sgpio.activity.flags.recent_activity = 1;
+	return ata_qc_issue_prot(qc);
+}
 static void nv_ck804_host_stop(struct ata_host *host)
 {
 	struct pci_dev *pdev = to_pci_dev(host->dev);
@@ -566,6 +806,277 @@
 	ata_pci_host_stop(host);
 }
 
+static void nv_host_stop (struct ata_host *host)
+{
+	struct nv_host *phost = host->private_data;
+
+
+	nv_sgpio_host_cleanup(phost);
+	kfree(phost);
+	ata_pci_host_stop(host);
+
+}
+
+static void nv_sgpio_init(struct pci_dev *pdev, struct nv_host *phost)
+{
+	u16 csr_add; 
+	u32 cb_add, temp32;
+	struct device *dev = pci_dev_to_dev(pdev);
+	struct ata_host *host = dev_get_drvdata(dev);
+	u8 pro = 0;
+	
+	pci_read_config_word(pdev, NV_SGPIO_PCI_CSR_OFFSET, &csr_add);
+	pci_read_config_dword(pdev, NV_SGPIO_PCI_CB_OFFSET, &cb_add);
+	pci_read_config_byte(pdev, 0xA4, &pro);
+	
+	if (csr_add == 0 || cb_add == 0)
+		return;
+	
+	if (!(pro & 0x40))
+		return;	
+		
+	temp32 = csr_add;
+	phost->host_sgpio.pcsr = (void*)(unsigned long)temp32;
+	phost->host_sgpio.pcb = ioremap(cb_add, 256);
+
+	if (phost->host_sgpio.pcb->nvcr.bit.init_cnt != 0x2 ||
+			phost->host_sgpio.pcb->nvcr.bit.cbver != 0x0)
+		return;
+		
+	if (temp32 <= 0x200 || temp32 >= 0xFFFE )
+		return;
+		
+	if (cb_add <= 0x80000 || cb_add >= 0x9FC00)
+		return;
+	
+	if (phost->host_sgpio.pcb->scratch_space == 0) {
+		spin_lock_init(&nv_sgpio_lock);
+		phost->host_sgpio.share.plock = &nv_sgpio_lock;
+		phost->host_sgpio.share.ptstamp = &nv_sgpio_tstamp;
+		phost->host_sgpio.pcb->scratch_space = 
+			(unsigned long)&phost->host_sgpio.share;
+		spin_lock(phost->host_sgpio.share.plock);
+		nv_sgpio_reset(phost->host_sgpio.pcsr);
+		phost->host_sgpio.pcb->cr0 = 
+			SET_ENABLE(phost->host_sgpio.pcb->cr0);
+
+		spin_unlock(phost->host_sgpio.share.plock);
+	}
+
+	phost->host_sgpio.share = 
+		*(struct nv_sgpio_host_share *)(unsigned long)
+		phost->host_sgpio.pcb->scratch_space;
+	phost->host_sgpio.flags.sgpio_enabled = 1;
+
+	init_timer(&phost->host_sgpio.sgpio_timer);
+	phost->host_sgpio.sgpio_timer.data = (unsigned long)host;
+	nv_sgpio_set_timer(&phost->host_sgpio.sgpio_timer, 
+				NV_SGPIO_UPDATE_TICK);
+}
+
+static void nv_sgpio_set_timer(struct timer_list *ptimer, unsigned int timeout_msec)
+{
+	if (!ptimer)
+		return;
+	ptimer->function = nv_sgpio_timer_handler;
+	ptimer->expires = msecs_to_jiffies(timeout_msec) + jiffies;
+	add_timer(ptimer);
+}
+
+static void nv_sgpio_timer_handler(unsigned long context)
+{
+
+	struct ata_host *host = (struct ata_host *)context;
+	struct nv_host *phost;
+	u8 count, host_offset, port_offset;
+	union nv_sgpio_tx tx;
+	u8 on_off;
+	unsigned long mask = 0xFFFF;
+	struct nv_port *port;
+
+	if (!host)
+		goto err_out;
+	else 
+		phost = (struct nv_host *)host->private_data;
+
+	if (!phost->host_sgpio.flags.sgpio_enabled)
+		goto err_out;
+
+	host_offset = nv_sgpio_tx_host_offset(host);
+
+	spin_lock(phost->host_sgpio.share.plock);
+	tx = phost->host_sgpio.pcb->tx[host_offset];
+	spin_unlock(phost->host_sgpio.share.plock);
+
+	for (count = 0; count < host->n_ports; count++) {
+		struct ata_port *ap; 
+
+		ap = host->ports[count];
+	
+		if (!(ap && !(ap->flags & ATA_FLAG_DISABLED)))
+			continue;
+
+		port = (struct nv_port *)ap->private_data;
+		if (!port)
+			continue;            		
+		port_offset = nv_sgpio_tx_port_offset(ap);
+		on_off = GET_ACTIVITY(tx.tx_port[port_offset]);
+		if (nv_sgpio_update_led(&port->port_sgpio.activity, &on_off)) {
+			tx.tx_port[port_offset] = 
+				SET_ACTIVITY(tx.tx_port[port_offset], on_off);
+			phost->host_sgpio.flags.need_update = 1;
+	       }
+	}
+
+
+	if (phost->host_sgpio.flags.need_update) {
+		spin_lock(phost->host_sgpio.share.plock);    
+		if (nv_sgpio_get_func(host) 
+			% NV_CNTRLR_SHARE_INIT == 0) {
+			phost->host_sgpio.pcb->tx[host_offset].all &= mask;
+			mask = mask << 16;
+			tx.all &= mask;
+		} else {
+			tx.all &= mask;
+			mask = mask << 16;
+			phost->host_sgpio.pcb->tx[host_offset].all &= mask;
+		}
+		phost->host_sgpio.pcb->tx[host_offset].all |= tx.all;
+		spin_unlock(phost->host_sgpio.share.plock);     
+ 
+		if (nv_sgpio_send_cmd(phost, NV_SGPIO_CMD_WRITE_DATA)) { 
+			phost->host_sgpio.flags.need_update = 0;
+			return;
+		}
+	} else {
+		nv_sgpio_set_timer(&phost->host_sgpio.sgpio_timer, 
+				NV_SGPIO_UPDATE_TICK);
+	}
+err_out:
+	return;
+}
+
+static u8 nv_sgpio_send_cmd(struct nv_host *host, u8 cmd)
+{
+	u8 csr;
+	unsigned long *ptstamp;
+
+	spin_lock(host->host_sgpio.share.plock);    
+	ptstamp = host->host_sgpio.share.ptstamp;
+	if (jiffies_to_msecs(jiffies - *ptstamp) >= NV_SGPIO_MIN_UPDATE_DELTA) {
+		csr = inb((unsigned long)host->host_sgpio.pcsr);
+		if ((GET_SGPIO_STATUS(csr) != NV_SGPIO_STATE_OPERATIONAL) ||
+			(GET_CMD_STATUS(csr) == NV_SGPIO_CMD_ACTIVE)) {
+			
+		} else {
+			host->host_sgpio.pcb->cr0 = 
+				SET_ENABLE(host->host_sgpio.pcb->cr0);
+			csr = 0;
+			csr = SET_CMD(csr, cmd);
+			outb(csr, (unsigned long)host->host_sgpio.pcsr);
+			*ptstamp = jiffies;
+		}
+		spin_unlock(host->host_sgpio.share.plock);
+		nv_sgpio_set_timer(&host->host_sgpio.sgpio_timer, 
+			NV_SGPIO_UPDATE_TICK);
+		return 1;
+	} else {
+		spin_unlock(host->host_sgpio.share.plock);
+		nv_sgpio_set_timer(&host->host_sgpio.sgpio_timer, 
+				(NV_SGPIO_MIN_UPDATE_DELTA - 
+				jiffies_to_msecs(jiffies - *ptstamp)));
+		return 0;
+	}
+}
+
+static u8 nv_sgpio_update_led(struct nv_sgpio_led *led, u8 *on_off)
+{
+	u8 need_update = 0;
+
+	if (led->force_off > 0) {
+		led->force_off--;
+	} else if (led->flags.recent_activity ^ led->flags.last_state) {
+		*on_off = led->flags.recent_activity;
+		led->flags.last_state = led->flags.recent_activity;
+		need_update = 1;
+	} else if ((led->flags.recent_activity & led->flags.last_state) &&
+		(led->last_cons_active >= NV_SGPIO_MAX_ACTIVITY_ON)) {
+		*on_off = NV_OFF;
+		led->flags.last_state = NV_OFF;
+		led->force_off = NV_SGPIO_MIN_FORCE_OFF;
+		need_update = 1;
+	}
+
+	if (*on_off) 
+		led->last_cons_active++;	
+	else
+		led->last_cons_active = 0;
+
+	led->flags.recent_activity = 0;
+	return need_update;
+}
+
+static void nv_sgpio_reset(u8  *pcsr)
+{
+	u8 csr;
+
+	csr = inb((unsigned long)pcsr);
+	if (GET_SGPIO_STATUS(csr) == NV_SGPIO_STATE_RESET) {
+		csr = 0;
+		csr = SET_CMD(csr, NV_SGPIO_CMD_RESET);
+		outb(csr, (unsigned long)pcsr);
+	}
+	csr = 0;
+	csr = SET_CMD(csr, NV_SGPIO_CMD_READ_PARAMS);
+	outb(csr, (unsigned long)pcsr);
+}
+
+static void nv_sgpio_host_cleanup(struct nv_host *host)
+{
+	u8 csr;
+
+	if (!host)
+		return;
+
+	if (host->host_sgpio.flags.sgpio_enabled) {
+		spin_lock(host->host_sgpio.share.plock);
+		host->host_sgpio.pcb->cr0 = SET_ENABLE(host->host_sgpio.pcb->cr0);
+		csr = 0;
+		csr = SET_CMD(csr, NV_SGPIO_CMD_WRITE_DATA);
+		outb(csr, (unsigned long)host->host_sgpio.pcsr);
+		spin_unlock(host->host_sgpio.share.plock);
+
+		if (timer_pending(&host->host_sgpio.sgpio_timer))
+			del_timer(&host->host_sgpio.sgpio_timer);
+		host->host_sgpio.flags.sgpio_enabled = 0;
+		host->host_sgpio.pcb->scratch_space = 0;
+	}
+}
+
+static void nv_sgpio_clear_all_leds(struct ata_port *ap)
+{
+	struct nv_port *port = ap->private_data;
+	struct nv_host *host;
+	u8 host_offset, port_offset;
+
+	if (!port || !ap->host)
+		return;
+	if (!ap->host->private_data)
+		return;
+
+	host = ap->host->private_data;
+	if (!host->host_sgpio.flags.sgpio_enabled)
+		return;
+
+	host_offset = nv_sgpio_tx_host_offset(ap->host);
+	port_offset = nv_sgpio_tx_port_offset(ap);
+
+	spin_lock(host->host_sgpio.share.plock);
+	host->host_sgpio.pcb->tx[host_offset].tx_port[port_offset] = 0;
+	host->host_sgpio.flags.need_update = 1;
+	spin_unlock(host->host_sgpio.share.plock);
+}
+
 static int __init nv_init(void)
 {
 	return pci_register_driver(&nv_pci_driver);

^ permalink raw reply	[flat|nested] 27+ messages in thread

* RE: [PATCH] SCSI: Add the SGPIO support for sata_nv.c
@ 2006-11-07  9:55       ` Peer Chen
  0 siblings, 0 replies; 27+ messages in thread
From: Peer Chen @ 2006-11-07  9:55 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: jeff, linux-kernel, linux-ide, Kuan Luo

[-- Attachment #1: Type: text/plain, Size: 11352 bytes --]

Modified and resent out the patch as attachment.
Description about the patch:
Add SGPIO support in sata_nv.c.
SGPIO (Serial General Purpose Input Output) is a sideband serial 4-wire
interface that a storage controller uses to communicate with a storage
enclosure management controller, primarily to control activity and
status LEDs that are located within drive bays or on a storage
backplane. SGPIO is defined by [SFF8485].
In this patch,we drive the LEDs to blink when read/write operation
happen on SATA drives connect the corresponding ports on MCP55 board.
==========
The patch will be applied to kernel 2.6.19-rc4-git9.

Singed-off-by: Kuan Luo <kluo@nvidia.com>
Singed-off-by: Peer Chen <pchen@nvidia.com>


BRs
Peer Chen

-----Original Message-----
From: Christoph Hellwig [mailto:hch@infradead.org] 
Sent: Tuesday, October 31, 2006 6:41 PM
To: Peer Chen
Cc: jeff@garzik.org; linux-kernel@vger.kernel.org;
linux-ide@vger.kernel.org; Kuan Luo
Subject: Re: [PATCH] SCSI: Add the SGPIO support for sata_nv.c

On Tue, Oct 31, 2006 at 04:43:19PM +0800, Peer Chen wrote:
> The following SGPIO patch for sata_nv.c is based on 2.6.18 kernel.
> Signed-off by: Kuan Luo <kluo@nvidia.com>

First I'd like to say the patch subject isn't very informative.  For
a sata_nv patch it should read:

[PATCH] sata_nv: foooblah

and then the SGPIO in both the subject and description doesn't say
anything
at all, what functionality or improvement does this patch actually add.

And in general send patches against latest git or -mm tree.  I don't
know
how much the sata_nv driver changed since 2.6.18 but in general sending
patches against old kernel means they often can't be applied anymore.

> +//sgpio
> +// Sgpio defines
> +// SGPIO state defines

please use /* */ style comments.  Also these aren't exactly useful

> +#define NV_ON 1
> +#define NV_OFF 0
> +#ifndef bool
> +#define bool u8
> +#endif

please don't use your own bool types.

> +#define BF_EXTRACT(v, off, bc)	\
> +	((((u8)(v)) >> (off)) & ((1 << (bc)) - 1))
> +
> +#define BF_INS(v, ins, off, bc)				\
> +	(((v) & ~((((1 << (bc)) - 1)) << (off))) |	\
> +	(((u8)(ins)) << (off)))
> +
> +#define BF_EXTRACT_U32(v, off, bc)	\
> +	((((u32)(v)) >> (off)) & ((1 << (bc)) - 1))
> +
> +#define BF_INS_U32(v, ins, off, bc)			\
> +	(((v) & ~((((1 << (bc)) - 1)) << (off))) |	\
> +	(((u32)(ins)) << (off)))

please make such things inline functions.  Also they could have
more descriptive names.

> +union nv_sgpio_nvcr 
> +{
> +	struct {
> +		u8	init_cnt;
> +		u8	cb_size;
> +		u8	cbver;
> +		u8	rsvd;
> +	} bit;
> +	u32	all;
> +};
> +
> +union nv_sgpio_tx 
> +{
> +	u8	tx_port[4];
> +	u32 	all;
> +};
> +
> +struct nv_sgpio_cb 
> +{
> +	u64			scratch_space;
> +	union nv_sgpio_nvcr	nvcr;
> +	u32			cr0;
> +	u32                     rsvd[4];
> +	union nv_sgpio_tx       tx[2];
> +};
> +
> +struct nv_sgpio_host_share
> +{
> +	spinlock_t	*plock;
> +	unsigned long   *ptstamp;
> +};
> +
> +struct nv_sgpio_host_flags
> +{
> +	u8	sgpio_enabled:1;
> +	u8	need_update:1;
> +	u8	rsvd:6;
> +};
> +	
> +struct nv_host_sgpio
> +{
> +	struct nv_sgpio_host_flags	flags;
> +	u8				*pcsr;
> +	struct nv_sgpio_cb		*pcb;	
> +	struct nv_sgpio_host_share	share;
> +	struct timer_list		sgpio_timer;
> +};
> +
> +struct nv_sgpio_port_flags
> +{
> +	u8	last_state:1;
> +	u8	recent_activity:1;
> +	u8	rsvd:6;
> +};
> +
> +struct nv_sgpio_led 
> +{
> +	struct nv_sgpio_port_flags	flags;
> +	u8				force_off;
> +	u8      			last_cons_active;
> +};
> +
> +struct nv_port_sgpio
> +{
> +	struct nv_sgpio_led	activity;
> +};
> +
> +static spinlock_t	nv_sgpio_lock;

please use DEFINE_SPINLOCK to initialize the lock statically.

> +static unsigned long	nv_sgpio_tstamp;

> +static inline void nv_sgpio_set_csr(u8 csr, unsigned long pcsr)
> +{
> +	outb(csr, pcsr);
> +}
> +
> +static inline u8 nv_sgpio_get_csr(unsigned long pcsr)
> +{
> +	return inb(pcsr);
> +}

these helpers aren't useful at all, please remove them.

> +static inline u8 nv_sgpio_get_func(struct ata_host_set *host_set)
> +{
> +	u8 devfn = (to_pci_dev(host_set->dev))->devfn;
> +	return (PCI_FUNC(devfn));
> +}
> +
> +static inline u8 nv_sgpio_tx_host_offset(struct ata_host_set
*host_set)
> +{
> +	return (nv_sgpio_get_func(host_set)/NV_CNTRLR_SHARE_INIT);
> +}
> +
> +static inline u8 nv_sgpio_calc_tx_offset(u8 cntrlr, u8 channel)
> +{
> +	return (sizeof(union nv_sgpio_tx) - (NV_CNTRLR_SHARE_INIT *
> +		(cntrlr % NV_CNTRLR_SHARE_INIT)) - channel - 1);
> +}
> +
> +static inline u8 nv_sgpio_tx_port_offset(struct ata_port *ap)
> +{
> +	u8 cntrlr = nv_sgpio_get_func(ap->host_set);
> +	return (nv_sgpio_calc_tx_offset(cntrlr, ap->port_no));
> +}
> +
> +static inline bool nv_sgpio_capable(const struct pci_device_id *ent)
> +{
> +	if (ent->device == PCI_DEVICE_ID_NVIDIA_NFORCE_MCP55_SATA2)
> +		return 1;
> +	else
> +		return 0;
> +}
>  static int nv_init_one (struct pci_dev *pdev, const struct
> pci_device_id *ent);
>  static void nv_ck804_host_stop(struct ata_host_set *host_set);
>  static irqreturn_t nv_generic_interrupt(int irq, void *dev_instance,
> @@ -90,7 +259,10 @@
>  				      struct pt_regs *regs);
>  static u32 nv_scr_read (struct ata_port *ap, unsigned int sc_reg);
>  static void nv_scr_write (struct ata_port *ap, unsigned int sc_reg,
u32
> val);
> -
> +static void nv_host_stop (struct ata_host_set *host_set);
> +static int nv_port_start(struct ata_port *ap);
> +static void nv_port_stop(struct ata_port *ap);
> +static int nv_qc_issue(struct ata_queued_cmd *qc);
>  static void nv_nf2_freeze(struct ata_port *ap);
>  static void nv_nf2_thaw(struct ata_port *ap);
>  static void nv_ck804_freeze(struct ata_port *ap);
> @@ -147,6 +319,27 @@
>  	{ 0, } /* terminate list */
>  };
>  
> +struct nv_host
> +{
> +	unsigned long		host_flags;
> +	struct nv_host_sgpio	host_sgpio;
> +};
> +
> +struct nv_port
> +{
> +	struct nv_port_sgpio	port_sgpio;
> +};
> +
> +// SGPIO function prototypes
> +static void nv_sgpio_init(struct pci_dev *pdev, struct nv_host
*phost);
> +static void nv_sgpio_reset(u8 *pcsr);
> +static void nv_sgpio_set_timer(struct timer_list *ptimer, 
> +				unsigned int timeout_msec);
> +static void nv_sgpio_timer_handler(unsigned long ptr);
> +static void nv_sgpio_host_cleanup(struct nv_host *host);
> +static bool nv_sgpio_update_led(struct nv_sgpio_led *led, bool
> *on_off);
> +static void nv_sgpio_clear_all_leds(struct ata_port *ap);
> +static bool nv_sgpio_send_cmd(struct nv_host *host, u8 cmd);
>  static struct pci_driver nv_pci_driver = {
>  	.name			= DRV_NAME,
>  	.id_table		= nv_pci_tbl,
> @@ -184,7 +377,7 @@
>  	.bmdma_stop		= ata_bmdma_stop,
>  	.bmdma_status		= ata_bmdma_status,
>  	.qc_prep		= ata_qc_prep,
> -	.qc_issue		= ata_qc_issue_prot,
> +	.qc_issue		= nv_qc_issue,
>  	.freeze			= ata_bmdma_freeze,
>  	.thaw			= ata_bmdma_thaw,
>  	.error_handler		= nv_error_handler,
> @@ -194,9 +387,9 @@
>  	.irq_clear		= ata_bmdma_irq_clear,
>  	.scr_read		= nv_scr_read,
>  	.scr_write		= nv_scr_write,
> -	.port_start		= ata_port_start,
> -	.port_stop		= ata_port_stop,
> -	.host_stop		= ata_pci_host_stop,
> +	.port_start		= nv_port_start,
> +	.port_stop		= nv_port_stop,
> +	.host_stop		= nv_host_stop,
>  };
>  
>  static const struct ata_port_operations nv_nf2_ops = {
> @@ -211,7 +404,7 @@
>  	.bmdma_stop		= ata_bmdma_stop,
>  	.bmdma_status		= ata_bmdma_status,
>  	.qc_prep		= ata_qc_prep,
> -	.qc_issue		= ata_qc_issue_prot,
> +	.qc_issue		= nv_qc_issue,
>  	.freeze			= nv_nf2_freeze,
>  	.thaw			= nv_nf2_thaw,
>  	.error_handler		= nv_error_handler,
> @@ -221,9 +414,9 @@
>  	.irq_clear		= ata_bmdma_irq_clear,
>  	.scr_read		= nv_scr_read,
>  	.scr_write		= nv_scr_write,
> -	.port_start		= ata_port_start,
> -	.port_stop		= ata_port_stop,
> -	.host_stop		= ata_pci_host_stop,
> +	.port_start		= nv_port_start,
> +	.port_stop		= nv_port_stop,
> +	.host_stop		= nv_host_stop,
>  };
>  
>  static const struct ata_port_operations nv_ck804_ops = {
> @@ -238,7 +431,7 @@
>  	.bmdma_stop		= ata_bmdma_stop,
>  	.bmdma_status		= ata_bmdma_status,
>  	.qc_prep		= ata_qc_prep,
> -	.qc_issue		= ata_qc_issue_prot,
> +	.qc_issue		= nv_qc_issue,
>  	.freeze			= nv_ck804_freeze,
>  	.thaw			= nv_ck804_thaw,
>  	.error_handler		= nv_error_handler,
> @@ -248,8 +441,8 @@
>  	.irq_clear		= ata_bmdma_irq_clear,
>  	.scr_read		= nv_scr_read,
>  	.scr_write		= nv_scr_write,
> -	.port_start		= ata_port_start,
> -	.port_stop		= ata_port_stop,
> +	.port_start		= nv_port_start,
> +	.port_stop		= nv_port_stop,
>  	.host_stop		= nv_ck804_host_stop,
>  };
>  
> @@ -480,10 +673,10 @@
>  	ata_bmdma_drive_eh(ap, ata_std_prereset, ata_std_softreset,
>  			   nv_hardreset, ata_std_postreset);
>  }
> -
>  static int nv_init_one (struct pci_dev *pdev, const struct
> pci_device_id *ent)
>  {
>  	static int printed_version = 0;
> +	struct nv_host *host;
>  	struct ata_port_info *ppi;
>  	struct ata_probe_ent *probe_ent;
>  	int pci_dev_busy = 0;
> @@ -525,6 +718,13 @@
>  	if (!probe_ent)
>  		goto err_out_regions;
>  
> +	host = kmalloc(sizeof(struct nv_host), GFP_KERNEL);
> +	if (!host)
> +		goto err_out_free_ent;
> +
> +	memset(host, 0, sizeof(struct nv_host));

Please use kzalloc().

> +static int nv_port_start(struct ata_port *ap)
> +{
> +	int stat;
> +	struct nv_port *port;
> +
> +	stat = ata_port_start(ap);
> +	if (stat) {
> +		return stat;
> +	}

useless braces.

> +
> +	port = kmalloc(sizeof(struct nv_port), GFP_KERNEL);
> +	if (!port) 
> +		goto err_out_no_free;
> +
> +	memset(port, 0, sizeof(struct nv_port));

Please use kzalloc again.

> +	return (ata_qc_issue_prot(qc));

no need for braces around the return value.

> +static void nv_sgpio_init(struct pci_dev *pdev, struct nv_host
*phost)
> +{
> +	u16 csr_add; 
> +	u32 cb_add, temp32;
> +	struct device *dev = pci_dev_to_dev(pdev);
> +	struct ata_host_set *host_set = dev_get_drvdata(dev);
> +	u8 pro=0;

missing spacezs around the =.

> +	if (!(pro&0x40))

Again missing spaces.  Please take another look at
Documentation/CodingStyle
for the preferred linux style.

> +		return;	
> +		
> +	temp32 = csr_add;
> +	phost->host_sgpio.pcsr = (void *)temp32;
> +	phost->host_sgpio.pcb = phys_to_virt(cb_add);

Use of phys_to_virt is generally a bug.  What are you trying to do here?

> +
> +	if (phost->host_sgpio.pcb->nvcr.bit.init_cnt!=0x2 ||
> phost->host_sgpio.pcb->nvcr.bit.cbver!=0x0)

in addition to the whitespace damage this line is far too long,
please break it up.


<skipping the rest of the file for now until it's made a little more
 readable>

-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain
confidential information.  Any unauthorized review, use, disclosure or distribution
is prohibited.  If you are not the intended recipient, please contact the sender by
reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------

[-- Attachment #2: sgpio-patch-2.6.19-rc4-git9 --]
[-- Type: application/octet-stream, Size: 15478 bytes --]

--- linux-2.6.19-rc4-git9/drivers/ata/sata_nv.c.orig	2006-11-06 08:47:49.000000000 +0800
+++ linux-2.6.19-rc4-git9/drivers/ata/sata_nv.c	2006-11-07 08:36:54.000000000 +0800
@@ -80,6 +80,176 @@
 	NV_MCP_SATA_CFG_20_SATA_SPACE_EN = 0x04,
 };
 
+/* sgpio
+* Sgpio defines
+* SGPIO state defines
+*/
+#define NV_SGPIO_STATE_RESET		0
+#define NV_SGPIO_STATE_OPERATIONAL	1
+#define NV_SGPIO_STATE_ERROR		2
+
+/* SGPIO command opcodes */
+#define NV_SGPIO_CMD_RESET		0
+#define NV_SGPIO_CMD_READ_PARAMS	1
+#define NV_SGPIO_CMD_READ_DATA		2
+#define NV_SGPIO_CMD_WRITE_DATA		3
+
+/* SGPIO command status defines */
+#define NV_SGPIO_CMD_OK			0
+#define NV_SGPIO_CMD_ACTIVE		1
+#define NV_SGPIO_CMD_ERR		2
+
+#define NV_SGPIO_UPDATE_TICK		90
+#define NV_SGPIO_MIN_UPDATE_DELTA	33
+#define NV_CNTRLR_SHARE_INIT		2
+#define NV_SGPIO_MAX_ACTIVITY_ON	20
+#define NV_SGPIO_MIN_FORCE_OFF		5
+#define NV_SGPIO_PCI_CSR_OFFSET		0x58
+#define NV_SGPIO_PCI_CB_OFFSET		0x5C
+#define NV_SGPIO_DFLT_CB_SIZE		256
+#define NV_ON 1
+#define NV_OFF 0
+
+
+
+static inline u8 bf_extract(u8 value, u8 offset, u8 bit_count)
+{
+	return ((((u8)(value)) >> (offset)) & ((1 << (bit_count)) - 1));
+}
+
+static inline u8 bf_ins(u8 value, u8 ins, u8 offset, u8 bit_count)
+{
+	return 	(((value) & ~((((1 << (bit_count)) - 1)) << (offset))) |
+						(((u8)(ins)) << (offset)));
+}
+
+static inline u32 bf_extract_u32(u32 value, u8 offset, u8 bit_count)
+{
+	return ((((u32)(value)) >> (offset)) & ((1 << (bit_count)) - 1));
+
+}
+static inline u32 bf_ins_u32(u32 value, u32 ins, u8 offset, u8 bit_count)
+{
+	return 	(((value) & ~((((1 << (bit_count)) - 1)) << (offset))) |
+						(((u32)(ins)) << (offset)));
+}
+
+#define GET_SGPIO_STATUS(v)	bf_extract(v, 0, 2)
+#define GET_CMD_STATUS(v)	bf_extract(v, 3, 2)
+#define GET_CMD(v)		bf_extract(v, 5, 3)
+#define SET_CMD(v, cmd)		bf_ins(v, cmd, 5, 3) 
+
+#define GET_ENABLE(v)		bf_extract(v, 23, 1)
+#define SET_ENABLE(v)		bf_ins_u32(v, 1, 23, 1)
+
+/* Needs to have a u8 bit-field insert. */
+#define GET_ACTIVITY(v)		bf_extract(v, 5, 3)
+#define SET_ACTIVITY(v, on_off)	bf_ins(v, on_off, 5, 3)
+
+
+
+union nv_sgpio_nvcr 
+{
+	struct {
+		u8	init_cnt;
+		u8	cb_size;
+		u8	cbver;
+		u8	rsvd;
+	} bit;
+	u32	all;
+};
+
+union nv_sgpio_tx 
+{
+	u8	tx_port[4];
+	u32 	all;
+};
+
+struct nv_sgpio_cb 
+{
+	u64			scratch_space;
+	union nv_sgpio_nvcr	nvcr;
+	u32			cr0;
+	u32                     rsvd[4];
+	union nv_sgpio_tx       tx[2];
+};
+
+struct nv_sgpio_host_share
+{
+	spinlock_t	*plock;
+	unsigned long   *ptstamp;
+};
+
+struct nv_sgpio_host_flags
+{
+	u8	sgpio_enabled:1;
+	u8	need_update:1;
+	u8	rsvd:6;
+};
+	
+struct nv_host_sgpio
+{
+	struct nv_sgpio_host_flags	flags;
+	u8				*pcsr;
+	struct nv_sgpio_cb		*pcb;	
+	struct nv_sgpio_host_share	share;
+	struct timer_list		sgpio_timer;
+};
+
+struct nv_sgpio_port_flags
+{
+	u8	last_state:1;
+	u8	recent_activity:1;
+	u8	rsvd:6;
+};
+
+struct nv_sgpio_led 
+{
+	struct nv_sgpio_port_flags	flags;
+	u8				force_off;
+	u8      			last_cons_active;
+};
+
+struct nv_port_sgpio
+{
+	struct nv_sgpio_led	activity;
+};
+
+static DEFINE_SPINLOCK(nv_sgpio_lock);
+
+static unsigned long	nv_sgpio_tstamp;
+
+
+static inline u8 nv_sgpio_get_func(struct ata_host *host)
+{
+	u8 devfn = (to_pci_dev(host->dev))->devfn;
+	return (PCI_FUNC(devfn));
+}
+
+static inline u8 nv_sgpio_tx_host_offset(struct ata_host *host)
+{
+	return (nv_sgpio_get_func(host)/NV_CNTRLR_SHARE_INIT);
+}
+
+static inline u8 nv_sgpio_calc_tx_offset(u8 cntrlr, u8 channel)
+{
+	return (sizeof(union nv_sgpio_tx) - (NV_CNTRLR_SHARE_INIT *
+		(cntrlr % NV_CNTRLR_SHARE_INIT)) - channel - 1);
+}
+
+static inline u8 nv_sgpio_tx_port_offset(struct ata_port *ap)
+{
+	u8 cntrlr = nv_sgpio_get_func(ap->host);
+	return (nv_sgpio_calc_tx_offset(cntrlr, ap->port_no));
+}
+
+static inline u8 nv_sgpio_capable(const struct pci_device_id *ent)
+{
+	if (ent->device == PCI_DEVICE_ID_NVIDIA_NFORCE_MCP55_SATA2)
+		return 1;
+	else
+		return 0;
+}
 static int nv_init_one (struct pci_dev *pdev, const struct pci_device_id *ent);
 static void nv_ck804_host_stop(struct ata_host *host);
 static irqreturn_t nv_generic_interrupt(int irq, void *dev_instance);
@@ -87,7 +257,10 @@
 static irqreturn_t nv_ck804_interrupt(int irq, void *dev_instance);
 static u32 nv_scr_read (struct ata_port *ap, unsigned int sc_reg);
 static void nv_scr_write (struct ata_port *ap, unsigned int sc_reg, u32 val);
-
+static void nv_host_stop (struct ata_host *host);
+static int nv_port_start(struct ata_port *ap);
+static void nv_port_stop(struct ata_port *ap);
+static unsigned int nv_qc_issue(struct ata_queued_cmd *qc);
 static void nv_nf2_freeze(struct ata_port *ap);
 static void nv_nf2_thaw(struct ata_port *ap);
 static void nv_ck804_freeze(struct ata_port *ap);
@@ -135,6 +308,27 @@
 	{ } /* terminate list */
 };
 
+struct nv_host
+{
+	unsigned long		host_flags;
+	struct nv_host_sgpio	host_sgpio;
+};
+
+struct nv_port
+{
+	struct nv_port_sgpio	port_sgpio;
+};
+
+/* SGPIO function prototypes */
+static void nv_sgpio_init(struct pci_dev *pdev, struct nv_host *phost);
+static void nv_sgpio_reset(u8 *pcsr);
+static void nv_sgpio_set_timer(struct timer_list *ptimer, 
+				unsigned int timeout_msec);
+static void nv_sgpio_timer_handler(unsigned long ptr);
+static void nv_sgpio_host_cleanup(struct nv_host *host);
+static u8 nv_sgpio_update_led(struct nv_sgpio_led *led, u8 *on_off);
+static void nv_sgpio_clear_all_leds(struct ata_port *ap);
+static u8 nv_sgpio_send_cmd(struct nv_host *host, u8 cmd);
 static struct pci_driver nv_pci_driver = {
 	.name			= DRV_NAME,
 	.id_table		= nv_pci_tbl,
@@ -172,7 +366,7 @@
 	.bmdma_stop		= ata_bmdma_stop,
 	.bmdma_status		= ata_bmdma_status,
 	.qc_prep		= ata_qc_prep,
-	.qc_issue		= ata_qc_issue_prot,
+	.qc_issue		= nv_qc_issue,
 	.freeze			= ata_bmdma_freeze,
 	.thaw			= ata_bmdma_thaw,
 	.error_handler		= nv_error_handler,
@@ -182,9 +376,9 @@
 	.irq_clear		= ata_bmdma_irq_clear,
 	.scr_read		= nv_scr_read,
 	.scr_write		= nv_scr_write,
-	.port_start		= ata_port_start,
-	.port_stop		= ata_port_stop,
-	.host_stop		= ata_pci_host_stop,
+	.port_start		= nv_port_start,
+	.port_stop		= nv_port_stop,
+	.host_stop		= nv_host_stop,
 };
 
 static const struct ata_port_operations nv_nf2_ops = {
@@ -465,10 +659,10 @@
 	ata_bmdma_drive_eh(ap, ata_std_prereset, ata_std_softreset,
 			   nv_hardreset, ata_std_postreset);
 }
-
 static int nv_init_one (struct pci_dev *pdev, const struct pci_device_id *ent)
 {
 	static int printed_version = 0;
+	struct nv_host *host;
 	struct ata_port_info *ppi[2];
 	struct ata_probe_ent *probe_ent;
 	int pci_dev_busy = 0;
@@ -510,6 +704,11 @@
 	if (!probe_ent)
 		goto err_out_regions;
 
+	host = kzalloc(sizeof(struct nv_host), GFP_KERNEL);
+	if (!host)
+		goto err_out_free_ent;
+
+	probe_ent->private_data = host;
 	probe_ent->mmio_base = pci_iomap(pdev, 5, 0);
 	if (!probe_ent->mmio_base) {
 		rc = -EIO;
@@ -535,6 +734,8 @@
 	rc = ata_device_add(probe_ent);
 	if (rc != NV_PORTS)
 		goto err_out_iounmap;
+	if (nv_sgpio_capable(ent))
+		nv_sgpio_init(pdev, host);
 
 	kfree(probe_ent);
 
@@ -553,6 +754,45 @@
 	return rc;
 }
 
+static int nv_port_start(struct ata_port *ap)
+{
+	int stat;
+	struct nv_port *port;
+
+	stat = ata_port_start(ap);
+	if (stat)
+		return stat;
+	
+	port = kzalloc(sizeof(struct nv_port), GFP_KERNEL);
+	if (!port) 
+		goto err_out_no_free;
+
+	ap->private_data = port;
+	return 0;
+
+err_out_no_free:
+	return 1;
+}
+
+static void nv_port_stop(struct ata_port *ap)
+{
+	nv_sgpio_clear_all_leds(ap);
+
+	if (ap->private_data) {
+		kfree(ap->private_data);
+		ap->private_data = NULL;
+	}
+	ata_port_stop(ap);
+}
+
+static unsigned int nv_qc_issue(struct ata_queued_cmd *qc)
+{
+	struct nv_port *port = qc->ap->private_data;
+
+	if (port) 
+		port->port_sgpio.activity.flags.recent_activity = 1;
+	return ata_qc_issue_prot(qc);
+}
 static void nv_ck804_host_stop(struct ata_host *host)
 {
 	struct pci_dev *pdev = to_pci_dev(host->dev);
@@ -566,6 +806,277 @@
 	ata_pci_host_stop(host);
 }
 
+static void nv_host_stop (struct ata_host *host)
+{
+	struct nv_host *phost = host->private_data;
+
+
+	nv_sgpio_host_cleanup(phost);
+	kfree(phost);
+	ata_pci_host_stop(host);
+
+}
+
+static void nv_sgpio_init(struct pci_dev *pdev, struct nv_host *phost)
+{
+	u16 csr_add; 
+	u32 cb_add, temp32;
+	struct device *dev = pci_dev_to_dev(pdev);
+	struct ata_host *host = dev_get_drvdata(dev);
+	u8 pro = 0;
+	
+	pci_read_config_word(pdev, NV_SGPIO_PCI_CSR_OFFSET, &csr_add);
+	pci_read_config_dword(pdev, NV_SGPIO_PCI_CB_OFFSET, &cb_add);
+	pci_read_config_byte(pdev, 0xA4, &pro);
+	
+	if (csr_add == 0 || cb_add == 0)
+		return;
+	
+	if (!(pro & 0x40))
+		return;	
+		
+	temp32 = csr_add;
+	phost->host_sgpio.pcsr = (void*)(unsigned long)temp32;
+	phost->host_sgpio.pcb = ioremap(cb_add, 256);
+
+	if (phost->host_sgpio.pcb->nvcr.bit.init_cnt != 0x2 ||
+			phost->host_sgpio.pcb->nvcr.bit.cbver != 0x0)
+		return;
+		
+	if (temp32 <= 0x200 || temp32 >= 0xFFFE )
+		return;
+		
+	if (cb_add <= 0x80000 || cb_add >= 0x9FC00)
+		return;
+	
+	if (phost->host_sgpio.pcb->scratch_space == 0) {
+		spin_lock_init(&nv_sgpio_lock);
+		phost->host_sgpio.share.plock = &nv_sgpio_lock;
+		phost->host_sgpio.share.ptstamp = &nv_sgpio_tstamp;
+		phost->host_sgpio.pcb->scratch_space = 
+			(unsigned long)&phost->host_sgpio.share;
+		spin_lock(phost->host_sgpio.share.plock);
+		nv_sgpio_reset(phost->host_sgpio.pcsr);
+		phost->host_sgpio.pcb->cr0 = 
+			SET_ENABLE(phost->host_sgpio.pcb->cr0);
+
+		spin_unlock(phost->host_sgpio.share.plock);
+	}
+
+	phost->host_sgpio.share = 
+		*(struct nv_sgpio_host_share *)(unsigned long)
+		phost->host_sgpio.pcb->scratch_space;
+	phost->host_sgpio.flags.sgpio_enabled = 1;
+
+	init_timer(&phost->host_sgpio.sgpio_timer);
+	phost->host_sgpio.sgpio_timer.data = (unsigned long)host;
+	nv_sgpio_set_timer(&phost->host_sgpio.sgpio_timer, 
+				NV_SGPIO_UPDATE_TICK);
+}
+
+static void nv_sgpio_set_timer(struct timer_list *ptimer, unsigned int timeout_msec)
+{
+	if (!ptimer)
+		return;
+	ptimer->function = nv_sgpio_timer_handler;
+	ptimer->expires = msecs_to_jiffies(timeout_msec) + jiffies;
+	add_timer(ptimer);
+}
+
+static void nv_sgpio_timer_handler(unsigned long context)
+{
+
+	struct ata_host *host = (struct ata_host *)context;
+	struct nv_host *phost;
+	u8 count, host_offset, port_offset;
+	union nv_sgpio_tx tx;
+	u8 on_off;
+	unsigned long mask = 0xFFFF;
+	struct nv_port *port;
+
+	if (!host)
+		goto err_out;
+	else 
+		phost = (struct nv_host *)host->private_data;
+
+	if (!phost->host_sgpio.flags.sgpio_enabled)
+		goto err_out;
+
+	host_offset = nv_sgpio_tx_host_offset(host);
+
+	spin_lock(phost->host_sgpio.share.plock);
+	tx = phost->host_sgpio.pcb->tx[host_offset];
+	spin_unlock(phost->host_sgpio.share.plock);
+
+	for (count = 0; count < host->n_ports; count++) {
+		struct ata_port *ap; 
+
+		ap = host->ports[count];
+	
+		if (!(ap && !(ap->flags & ATA_FLAG_DISABLED)))
+			continue;
+
+		port = (struct nv_port *)ap->private_data;
+		if (!port)
+			continue;            		
+		port_offset = nv_sgpio_tx_port_offset(ap);
+		on_off = GET_ACTIVITY(tx.tx_port[port_offset]);
+		if (nv_sgpio_update_led(&port->port_sgpio.activity, &on_off)) {
+			tx.tx_port[port_offset] = 
+				SET_ACTIVITY(tx.tx_port[port_offset], on_off);
+			phost->host_sgpio.flags.need_update = 1;
+	       }
+	}
+
+
+	if (phost->host_sgpio.flags.need_update) {
+		spin_lock(phost->host_sgpio.share.plock);    
+		if (nv_sgpio_get_func(host) 
+			% NV_CNTRLR_SHARE_INIT == 0) {
+			phost->host_sgpio.pcb->tx[host_offset].all &= mask;
+			mask = mask << 16;
+			tx.all &= mask;
+		} else {
+			tx.all &= mask;
+			mask = mask << 16;
+			phost->host_sgpio.pcb->tx[host_offset].all &= mask;
+		}
+		phost->host_sgpio.pcb->tx[host_offset].all |= tx.all;
+		spin_unlock(phost->host_sgpio.share.plock);     
+ 
+		if (nv_sgpio_send_cmd(phost, NV_SGPIO_CMD_WRITE_DATA)) { 
+			phost->host_sgpio.flags.need_update = 0;
+			return;
+		}
+	} else {
+		nv_sgpio_set_timer(&phost->host_sgpio.sgpio_timer, 
+				NV_SGPIO_UPDATE_TICK);
+	}
+err_out:
+	return;
+}
+
+static u8 nv_sgpio_send_cmd(struct nv_host *host, u8 cmd)
+{
+	u8 csr;
+	unsigned long *ptstamp;
+
+	spin_lock(host->host_sgpio.share.plock);    
+	ptstamp = host->host_sgpio.share.ptstamp;
+	if (jiffies_to_msecs(jiffies - *ptstamp) >= NV_SGPIO_MIN_UPDATE_DELTA) {
+		csr = inb((unsigned long)host->host_sgpio.pcsr);
+		if ((GET_SGPIO_STATUS(csr) != NV_SGPIO_STATE_OPERATIONAL) ||
+			(GET_CMD_STATUS(csr) == NV_SGPIO_CMD_ACTIVE)) {
+			
+		} else {
+			host->host_sgpio.pcb->cr0 = 
+				SET_ENABLE(host->host_sgpio.pcb->cr0);
+			csr = 0;
+			csr = SET_CMD(csr, cmd);
+			outb(csr, (unsigned long)host->host_sgpio.pcsr);
+			*ptstamp = jiffies;
+		}
+		spin_unlock(host->host_sgpio.share.plock);
+		nv_sgpio_set_timer(&host->host_sgpio.sgpio_timer, 
+			NV_SGPIO_UPDATE_TICK);
+		return 1;
+	} else {
+		spin_unlock(host->host_sgpio.share.plock);
+		nv_sgpio_set_timer(&host->host_sgpio.sgpio_timer, 
+				(NV_SGPIO_MIN_UPDATE_DELTA - 
+				jiffies_to_msecs(jiffies - *ptstamp)));
+		return 0;
+	}
+}
+
+static u8 nv_sgpio_update_led(struct nv_sgpio_led *led, u8 *on_off)
+{
+	u8 need_update = 0;
+
+	if (led->force_off > 0) {
+		led->force_off--;
+	} else if (led->flags.recent_activity ^ led->flags.last_state) {
+		*on_off = led->flags.recent_activity;
+		led->flags.last_state = led->flags.recent_activity;
+		need_update = 1;
+	} else if ((led->flags.recent_activity & led->flags.last_state) &&
+		(led->last_cons_active >= NV_SGPIO_MAX_ACTIVITY_ON)) {
+		*on_off = NV_OFF;
+		led->flags.last_state = NV_OFF;
+		led->force_off = NV_SGPIO_MIN_FORCE_OFF;
+		need_update = 1;
+	}
+
+	if (*on_off) 
+		led->last_cons_active++;	
+	else
+		led->last_cons_active = 0;
+
+	led->flags.recent_activity = 0;
+	return need_update;
+}
+
+static void nv_sgpio_reset(u8  *pcsr)
+{
+	u8 csr;
+
+	csr = inb((unsigned long)pcsr);
+	if (GET_SGPIO_STATUS(csr) == NV_SGPIO_STATE_RESET) {
+		csr = 0;
+		csr = SET_CMD(csr, NV_SGPIO_CMD_RESET);
+		outb(csr, (unsigned long)pcsr);
+	}
+	csr = 0;
+	csr = SET_CMD(csr, NV_SGPIO_CMD_READ_PARAMS);
+	outb(csr, (unsigned long)pcsr);
+}
+
+static void nv_sgpio_host_cleanup(struct nv_host *host)
+{
+	u8 csr;
+
+	if (!host)
+		return;
+
+	if (host->host_sgpio.flags.sgpio_enabled) {
+		spin_lock(host->host_sgpio.share.plock);
+		host->host_sgpio.pcb->cr0 = SET_ENABLE(host->host_sgpio.pcb->cr0);
+		csr = 0;
+		csr = SET_CMD(csr, NV_SGPIO_CMD_WRITE_DATA);
+		outb(csr, (unsigned long)host->host_sgpio.pcsr);
+		spin_unlock(host->host_sgpio.share.plock);
+
+		if (timer_pending(&host->host_sgpio.sgpio_timer))
+			del_timer(&host->host_sgpio.sgpio_timer);
+		host->host_sgpio.flags.sgpio_enabled = 0;
+		host->host_sgpio.pcb->scratch_space = 0;
+	}
+}
+
+static void nv_sgpio_clear_all_leds(struct ata_port *ap)
+{
+	struct nv_port *port = ap->private_data;
+	struct nv_host *host;
+	u8 host_offset, port_offset;
+
+	if (!port || !ap->host)
+		return;
+	if (!ap->host->private_data)
+		return;
+
+	host = ap->host->private_data;
+	if (!host->host_sgpio.flags.sgpio_enabled)
+		return;
+
+	host_offset = nv_sgpio_tx_host_offset(ap->host);
+	port_offset = nv_sgpio_tx_port_offset(ap);
+
+	spin_lock(host->host_sgpio.share.plock);
+	host->host_sgpio.pcb->tx[host_offset].tx_port[port_offset] = 0;
+	host->host_sgpio.flags.need_update = 1;
+	spin_unlock(host->host_sgpio.share.plock);
+}
+
 static int __init nv_init(void)
 {
 	return pci_register_driver(&nv_pci_driver);

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH] SCSI: Add the SGPIO support for sata_nv.c
  2006-11-07  9:55       ` Peer Chen
  (?)
@ 2007-11-07  4:09       ` Yinghai Lu
  2009-01-18  9:20         ` Yinghai Lu
  -1 siblings, 1 reply; 27+ messages in thread
From: Yinghai Lu @ 2007-11-07  4:09 UTC (permalink / raw)
  To: Peer Chen; +Cc: Christoph Hellwig, jeff, linux-kernel, linux-ide, Kuan Luo

On Nov 7, 2006 1:55 AM, Peer Chen <pchen@nvidia.com> wrote:
> Modified and resent out the patch as attachment.
> Description about the patch:
> Add SGPIO support in sata_nv.c.
> SGPIO (Serial General Purpose Input Output) is a sideband serial 4-wire
> interface that a storage controller uses to communicate with a storage
> enclosure management controller, primarily to control activity and
> status LEDs that are located within drive bays or on a storage
> backplane. SGPIO is defined by [SFF8485].
> In this patch,we drive the LEDs to blink when read/write operation
> happen on SATA drives connect the corresponding ports on MCP55 board.
> ==========
> The patch will be applied to kernel 2.6.19-rc4-git9.

do you have one that can apply to 2.6.24-rc2 or current linus git tree.

YH

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH] SCSI: Add the SGPIO support for sata_nv.c
  2007-11-07  4:09       ` Yinghai Lu
@ 2009-01-18  9:20         ` Yinghai Lu
  2009-01-18  9:22           ` [PATCH] sata_nv: sgpio for nvidia mcp55 Yinghai Lu
  2009-01-18 13:49           ` [PATCH] SCSI: Add the SGPIO support for sata_nv.c Yan Seiner
  0 siblings, 2 replies; 27+ messages in thread
From: Yinghai Lu @ 2009-01-18  9:20 UTC (permalink / raw)
  To: Peer Chen, Yinghai Lu
  Cc: Christoph Hellwig, jeff, linux-kernel, linux-ide, Kuan Luo

On Tue, Nov 6, 2007 at 8:09 PM, Yinghai Lu <yhlu.kernel@gmail.com> wrote:
> On Nov 7, 2006 1:55 AM, Peer Chen <pchen@nvidia.com> wrote:
>> Modified and resent out the patch as attachment.
>> Description about the patch:
>> Add SGPIO support in sata_nv.c.
>> SGPIO (Serial General Purpose Input Output) is a sideband serial 4-wire
>> interface that a storage controller uses to communicate with a storage
>> enclosure management controller, primarily to control activity and
>> status LEDs that are located within drive bays or on a storage
>> backplane. SGPIO is defined by [SFF8485].
>> In this patch,we drive the LEDs to blink when read/write operation
>> happen on SATA drives connect the corresponding ports on MCP55 board.
>> ==========
>> The patch will be applied to kernel 2.6.19-rc4-git9.
>
> do you have one that can apply to 2.6.24-rc2 or current linus git tree.
>

will send out update to current patch.

YH

^ permalink raw reply	[flat|nested] 27+ messages in thread

* [PATCH] sata_nv: sgpio for nvidia mcp55
  2009-01-18  9:20         ` Yinghai Lu
@ 2009-01-18  9:22           ` Yinghai Lu
  2009-01-21  2:07             ` [PATCH] sata_nv: sgpio for nvidia mcp55 -v2 Yinghai Lu
  2009-01-18 13:49           ` [PATCH] SCSI: Add the SGPIO support for sata_nv.c Yan Seiner
  1 sibling, 1 reply; 27+ messages in thread
From: Yinghai Lu @ 2009-01-18  9:22 UTC (permalink / raw)
  To: Peer Chen, Yinghai Lu, jeff, Kuan Luo
  Cc: Christoph Hellwig, linux-kernel, linux-ide, Ingo Molnar, Andrew Morton


Impact: new features

based on patch on
    http://marc.info/?l=linux-ide&m=116289338705418&w=2

1. update the patch for 2.6.19 to latest upstream ( 2.6.28?)
2. fix shared sgpio to support several mcp55 + io55, so every mcp55 have
   seperate spinlock
3. use scratch_source as numbering of sgpio instead of address of struct,
   so could go through kexec/kdump

Signed-off-by: Yinghai Lu <yinghai@kernel.org>

---
 drivers/ata/sata_nv.c |  534 ++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 534 insertions(+)

Index: linux-2.6/drivers/ata/sata_nv.c
===================================================================
--- linux-2.6.orig/drivers/ata/sata_nv.c
+++ linux-2.6/drivers/ata/sata_nv.c
@@ -200,6 +200,167 @@ enum {
 
 };
 
+/* sgpio
+ * Sgpio defines
+ * SGPIO state defines
+ */
+#define NV_SGPIO_STATE_RESET		0
+#define NV_SGPIO_STATE_OPERATIONAL	1
+#define NV_SGPIO_STATE_ERROR		2
+
+/* SGPIO command opcodes */
+#define NV_SGPIO_CMD_RESET		0
+#define NV_SGPIO_CMD_READ_PARAMS	1
+#define NV_SGPIO_CMD_READ_DATA		2
+#define NV_SGPIO_CMD_WRITE_DATA	3
+
+/* SGPIO command status defines */
+#define NV_SGPIO_CMD_OK		0
+#define NV_SGPIO_CMD_ACTIVE		1
+#define NV_SGPIO_CMD_ERR		2
+
+#define NV_SGPIO_UPDATE_TICK		90
+#define NV_SGPIO_MIN_UPDATE_DELTA	33
+#define NV_CNTRLR_SHARE_INIT		2
+#define NV_SGPIO_MAX_ACTIVITY_ON	20
+#define NV_SGPIO_MIN_FORCE_OFF		5
+#define NV_SGPIO_PCI_CSR_OFFSET	0x58
+#define NV_SGPIO_PCI_CB_OFFSET		0x5C
+#define NV_SGPIO_DFLT_CB_SIZE		256
+#define NV_ON 1
+#define NV_OFF 0
+
+static inline u8 bf_extract(u8 value, u8 offset, u8 bit_count)
+{
+	return (((u8)(value)) >> (offset)) & ((1 << (bit_count)) - 1);
+}
+
+static inline u8 bf_ins(u8 value, u8 ins, u8 offset, u8 bit_count)
+{
+	return	((value) & ~((((1 << (bit_count)) - 1)) << (offset))) |
+						(((u8)(ins)) << (offset));
+}
+
+static inline u32 bf_extract_u32(u32 value, u8 offset, u8 bit_count)
+{
+	return (((u32)(value)) >> (offset)) & ((1 << (bit_count)) - 1);
+}
+static inline u32 bf_ins_u32(u32 value, u32 ins, u8 offset, u8 bit_count)
+{
+	return	((value) & ~((((1 << (bit_count)) - 1)) << (offset))) |
+						(((u32)(ins)) << (offset));
+}
+
+#define GET_SGPIO_STATUS(v)	bf_extract(v, 0, 2)
+#define GET_CMD_STATUS(v)	bf_extract(v, 3, 2)
+#define GET_CMD(v)		bf_extract(v, 5, 3)
+#define SET_CMD(v, cmd)	bf_ins(v, cmd, 5, 3)
+
+#define GET_ENABLE(v)		bf_extract(v, 23, 1)
+#define SET_ENABLE(v)		bf_ins_u32(v, 1, 23, 1)
+
+/* Needs to have a u8 bit-field insert. */
+#define GET_ACTIVITY(v)		bf_extract(v, 5, 3)
+#define SET_ACTIVITY(v, on_off)	bf_ins(v, on_off, 5, 3)
+
+union nv_sgpio_nvcr {
+	struct {
+		u8	init_cnt;
+		u8	cb_size;
+		u8	cbver;
+		u8	rsvd;
+	} bit;
+	u32	all;
+};
+
+union nv_sgpio_tx {
+	u8	tx_port[4];
+	u32	all;
+};
+
+struct nv_sgpio_cb {
+	u64			scratch_space;
+	union nv_sgpio_nvcr	nvcr;
+	u32			cr0;
+	u32                     rsvd[4];
+	union nv_sgpio_tx       tx[2];
+};
+
+struct nv_sgpio_host_share {
+	spinlock_t	*plock;
+	unsigned long   *ptstamp;
+};
+
+struct nv_sgpio_host_flags {
+	u8	sgpio_enabled:1;
+	u8	need_update:1;
+	u8	rsvd:6;
+};
+
+struct nv_host_sgpio {
+	struct nv_sgpio_host_flags	flags;
+	u8				*pcsr;
+	struct nv_sgpio_cb		*pcb;
+	struct nv_sgpio_host_share	share;
+	struct timer_list		sgpio_timer;
+};
+
+struct nv_sgpio_port_flags {
+	u8	last_state:1;
+	u8	recent_activity:1;
+	u8	rsvd:6;
+};
+
+struct nv_sgpio_led {
+	struct nv_sgpio_port_flags	flags;
+	u8				force_off;
+	u8				last_cons_active;
+};
+
+struct nv_port_sgpio {
+	struct nv_sgpio_led	activity;
+};
+
+/* 1 mcp55 and 3 io55, 0 is not used */
+#define NR_SGPIO 5
+static spinlock_t nv_sgpio_lock[NR_SGPIO];
+static unsigned long nv_sgpio_tstamp[NR_SGPIO];
+static unsigned long nv_sgpio_share[NR_SGPIO] = {
+	[0 ... NR_SGPIO-1] = 0
+};
+
+static inline u8 nv_sgpio_get_func(struct ata_host *host)
+{
+	u8 devfn = (to_pci_dev(host->dev))->devfn;
+
+	return PCI_FUNC(devfn);
+}
+
+static inline u8 nv_sgpio_tx_host_offset(struct ata_host *host)
+{
+	return nv_sgpio_get_func(host)/NV_CNTRLR_SHARE_INIT;
+}
+
+static inline u8 nv_sgpio_calc_tx_offset(u8 cntrlr, u8 channel)
+{
+	return sizeof(union nv_sgpio_tx) - (NV_CNTRLR_SHARE_INIT *
+		(cntrlr % NV_CNTRLR_SHARE_INIT)) - channel - 1;
+}
+
+static inline u8 nv_sgpio_tx_port_offset(struct ata_port *ap)
+{
+	u8 cntrlr = nv_sgpio_get_func(ap->host);
+	return nv_sgpio_calc_tx_offset(cntrlr, ap->port_no);
+}
+
+static inline u8 nv_sgpio_capable(const struct pci_device_id *ent)
+{
+	if (ent->device == PCI_DEVICE_ID_NVIDIA_NFORCE_MCP55_SATA2)
+		return 1;
+	else
+		return 0;
+}
+
 /* ADMA Physical Region Descriptor - one SG segment */
 struct nv_adma_prd {
 	__le64			addr;
@@ -240,6 +401,7 @@ struct nv_adma_cpb {
 
 
 struct nv_adma_port_priv {
+	struct nv_port_sgpio	port_sgpio;
 	struct nv_adma_cpb	*cpb;
 	dma_addr_t		cpb_dma;
 	struct nv_adma_prd	*aprd;
@@ -254,6 +416,12 @@ struct nv_adma_port_priv {
 
 struct nv_host_priv {
 	unsigned long		type;
+	unsigned long		host_flags;
+	struct nv_host_sgpio	host_sgpio;
+};
+
+struct nv_port_priv {
+	struct nv_port_sgpio	port_sgpio;
 };
 
 struct defer_queue {
@@ -271,6 +439,8 @@ enum ncq_saw_flag_list {
 };
 
 struct nv_swncq_port_priv {
+	struct nv_port_sgpio	port_sgpio;
+
 	struct ata_prd	*prd;	 /* our SG list */
 	dma_addr_t	prd_dma; /* and its DMA mapping */
 	void __iomem	*sactive_block;
@@ -311,6 +481,10 @@ static int nv_nf2_hardreset(struct ata_l
 			    unsigned long deadline);
 static void nv_ck804_freeze(struct ata_port *ap);
 static void nv_ck804_thaw(struct ata_port *ap);
+static int nv_port_start(struct ata_port *ap);
+static void nv_port_stop(struct ata_port *ap);
+static void nv_host_stop(struct ata_host *host);
+static unsigned int nv_qc_issue(struct ata_queued_cmd *qc);
 static int nv_adma_slave_config(struct scsi_device *sdev);
 static int nv_adma_check_atapi_dma(struct ata_queued_cmd *qc);
 static void nv_adma_qc_prep(struct ata_queued_cmd *qc);
@@ -374,6 +548,17 @@ static const struct pci_device_id nv_pci
 	{ } /* terminate list */
 };
 
+/* SGPIO function prototypes */
+static void nv_sgpio_init(struct pci_dev *pdev, struct ata_host *host);
+static void nv_sgpio_reset(u8 *pcsr);
+static void nv_sgpio_set_timer(struct timer_list *ptimer,
+				unsigned int timeout_msec);
+static void nv_sgpio_timer_handler(unsigned long ptr);
+static void nv_sgpio_host_cleanup(struct nv_host_priv *host);
+static u8 nv_sgpio_update_led(struct nv_sgpio_led *led, u8 *on_off);
+static void nv_sgpio_clear_all_leds(struct ata_port *ap);
+static u8 nv_sgpio_send_cmd(struct nv_host_priv *host, u8 cmd);
+
 static struct pci_driver nv_pci_driver = {
 	.name			= DRV_NAME,
 	.id_table		= nv_pci_tbl,
@@ -409,6 +594,10 @@ static struct ata_port_operations nv_com
 	.inherits		= &ata_bmdma_port_ops,
 	.scr_read		= nv_scr_read,
 	.scr_write		= nv_scr_write,
+	.qc_issue               = nv_qc_issue,
+	.port_start		= nv_port_start,
+	.port_stop		= nv_port_stop,
+	.host_stop		= nv_host_stop,
 };
 
 /* OSDL bz11195 reports that link doesn't come online after hardreset
@@ -1395,6 +1584,8 @@ static unsigned int nv_adma_qc_issue(str
 	void __iomem *mmio = pp->ctl_block;
 	int curr_ncq = (qc->tf.protocol == ATA_PROT_NCQ);
 
+	pp->port_sgpio.activity.flags.recent_activity = NV_ON;
+
 	VPRINTK("ENTER\n");
 
 	/* We can't handle result taskfile with NCQ commands, since
@@ -1673,6 +1864,34 @@ static void nv_adma_error_handler(struct
 	ata_sff_error_handler(ap);
 }
 
+static int nv_port_start(struct ata_port *ap)
+{
+	struct device *dev = ap->host->dev;
+	struct nv_port_priv *pp;
+	int rc;
+
+	rc = ata_sff_port_start(ap);
+	if (rc)
+		return rc;
+
+	pp = devm_kzalloc(dev, sizeof(*pp), GFP_KERNEL);
+	if (!pp)
+		return -ENOMEM;
+
+	ap->private_data = pp;
+
+	return 0;
+}
+
+static unsigned int nv_qc_issue(struct ata_queued_cmd *qc)
+{
+       struct nv_port_priv *port = qc->ap->private_data;
+
+       port->port_sgpio.activity.flags.recent_activity = NV_ON;
+
+       return ata_sff_qc_issue(qc);
+}
+
 static void nv_swncq_qc_to_dq(struct ata_port *ap, struct ata_queued_cmd *qc)
 {
 	struct nv_swncq_port_priv *pp = ap->private_data;
@@ -2017,6 +2236,8 @@ static unsigned int nv_swncq_qc_issue(st
 	struct ata_port *ap = qc->ap;
 	struct nv_swncq_port_priv *pp = ap->private_data;
 
+	pp->port_sgpio.activity.flags.recent_activity = NV_ON;
+
 	if (qc->tf.protocol != ATA_PROT_NCQ)
 		return ata_sff_qc_issue(qc);
 
@@ -2404,6 +2625,9 @@ static int nv_init_one(struct pci_dev *p
 	} else if (type == SWNCQ)
 		nv_swncq_host_init(host);
 
+	if (nv_sgpio_capable(ent))
+		nv_sgpio_init(pdev, host);
+
 	pci_set_master(pdev);
 	return ata_host_activate(host, pdev->irq, ipriv->irq_handler,
 				 IRQF_SHARED, ipriv->sht);
@@ -2459,11 +2683,19 @@ static int nv_pci_device_resume(struct p
 }
 #endif
 
+static void nv_port_stop(struct ata_port *ap)
+{
+	nv_sgpio_clear_all_leds(ap);
+}
+
 static void nv_ck804_host_stop(struct ata_host *host)
 {
 	struct pci_dev *pdev = to_pci_dev(host->dev);
+	struct nv_host_priv *phost = host->private_data;
 	u8 regval;
 
+	nv_sgpio_host_cleanup(phost);
+
 	/* disable SATA space for CK804 */
 	pci_read_config_byte(pdev, NV_MCP_SATA_CFG_20, &regval);
 	regval &= ~NV_MCP_SATA_CFG_20_SATA_SPACE_EN;
@@ -2487,6 +2719,308 @@ static void nv_adma_host_stop(struct ata
 	nv_ck804_host_stop(host);
 }
 
+static void nv_host_stop(struct ata_host *host)
+{
+	struct nv_host_priv *phost = host->private_data;
+
+	nv_sgpio_host_cleanup(phost);
+}
+
+static void nv_sgpio_init(struct pci_dev *pdev, struct ata_host *host)
+{
+	u16 csr_add;
+	u32 cb_add, temp32;
+	struct nv_host_priv *phost = host->private_data;
+	struct nv_host_sgpio *host_sgpio;
+	u8 pro = 0;
+	unsigned int share_sgpio;
+	int i;
+
+	pci_read_config_word(pdev, NV_SGPIO_PCI_CSR_OFFSET, &csr_add);
+	pci_read_config_dword(pdev, NV_SGPIO_PCI_CB_OFFSET, &cb_add);
+
+	if (csr_add == 0 || cb_add == 0)
+		return;
+
+	if (cb_add <= 0x80000 || cb_add >= 0x9FC00)
+		return;
+
+	pci_read_config_byte(pdev, 0xA4, &pro);
+	if (!(pro & 0x40))
+		return;
+
+	temp32 = csr_add;
+	if (temp32 <= 0x200 || temp32 >= 0xFFFE)
+		return;
+
+	dev_printk(KERN_DEBUG, &pdev->dev, "CSR 0x%x CB 0x%x\n",
+			 csr_add, cb_add);
+
+	host_sgpio = &phost->host_sgpio;
+	host_sgpio->pcsr = (void *)(unsigned long)temp32;
+	host_sgpio->pcb = ioremap(cb_add, 256);
+
+	if (host_sgpio->pcb->nvcr.bit.init_cnt != 0x2 ||
+			host_sgpio->pcb->nvcr.bit.cbver != 0x0)
+		return;
+
+	share_sgpio = host_sgpio->pcb->scratch_space;
+	if (share_sgpio == 0 || share_sgpio > (NR_SGPIO - 1)) {
+		/* find one good position */
+		for (i = 1; i < NR_SGPIO; i++) {
+			if (!nv_sgpio_share[i]) {
+				share_sgpio = i;
+				break;
+			}
+		}
+		if (share_sgpio == 0 || share_sgpio > (NR_SGPIO - 1)) {
+			printk(KERN_WARNING "NR_SGPIO %d is too small\n",
+				 NR_SGPIO);
+			return;
+		}
+	}
+
+	if (!nv_sgpio_share[share_sgpio]) {
+		/* also handle kexec path:
+		 * scratch_space is set, but not get nv_sgpio_share yet
+		 */
+		spin_lock_init(&nv_sgpio_lock[share_sgpio]);
+		host_sgpio->share.plock = &nv_sgpio_lock[share_sgpio];
+		host_sgpio->share.ptstamp = &nv_sgpio_tstamp[share_sgpio];
+		nv_sgpio_share[share_sgpio] =
+			(unsigned long)&host_sgpio->share;
+		host_sgpio->pcb->scratch_space = share_sgpio;
+		spin_lock(host_sgpio->share.plock);
+		nv_sgpio_reset(host_sgpio->pcsr);
+		host_sgpio->pcb->cr0 =
+			SET_ENABLE(host_sgpio->pcb->cr0);
+		spin_unlock(host_sgpio->share.plock);
+	} else {
+		host_sgpio->share = *(struct nv_sgpio_host_share *)
+				nv_sgpio_share[share_sgpio];
+	}
+
+	dev_printk(KERN_DEBUG, &pdev->dev, "using sgpio %d\n", share_sgpio);
+	host_sgpio->flags.sgpio_enabled = 1;
+
+	init_timer(&host_sgpio->sgpio_timer);
+	host_sgpio->sgpio_timer.data = (unsigned long)host;
+	nv_sgpio_set_timer(&host_sgpio->sgpio_timer,
+				NV_SGPIO_UPDATE_TICK);
+}
+
+static void nv_sgpio_set_timer(struct timer_list *ptimer,
+			       unsigned int timeout_msec)
+{
+	if (!ptimer)
+		return;
+	ptimer->function = nv_sgpio_timer_handler;
+	ptimer->expires = msecs_to_jiffies(timeout_msec) + jiffies;
+	add_timer(ptimer);
+}
+
+static void nv_sgpio_timer_handler(unsigned long context)
+{
+
+	struct ata_host *host = (struct ata_host *)context;
+	struct nv_host_priv *phost;
+	u8 count, host_offset, port_offset;
+	union nv_sgpio_tx tx;
+	u8 on_off;
+	unsigned long mask = 0xFFFF;
+	struct nv_port_priv *port;
+	struct nv_host_sgpio *host_sgpio;
+
+	if (!host)
+		goto err_out;
+
+	phost = (struct nv_host_priv *)host->private_data;
+
+	host_sgpio = &phost->host_sgpio;
+	if (!host_sgpio->flags.sgpio_enabled)
+		goto err_out;
+
+	host_offset = nv_sgpio_tx_host_offset(host);
+
+	spin_lock(host_sgpio->share.plock);
+	tx = host_sgpio->pcb->tx[host_offset];
+	spin_unlock(host_sgpio->share.plock);
+
+	for (count = 0; count < host->n_ports; count++) {
+		struct ata_port *ap;
+
+		ap = host->ports[count];
+
+		if (!(ap && !(ap->flags & ATA_FLAG_DISABLED)))
+			continue;
+
+		port = (struct nv_port_priv *)ap->private_data;
+		if (!port)
+			continue;
+		port_offset = nv_sgpio_tx_port_offset(ap);
+		on_off = GET_ACTIVITY(tx.tx_port[port_offset]);
+		if (nv_sgpio_update_led(&port->port_sgpio.activity, &on_off)) {
+			tx.tx_port[port_offset] =
+				SET_ACTIVITY(tx.tx_port[port_offset], on_off);
+			host_sgpio->flags.need_update = 1;
+	       }
+	}
+
+	if (host_sgpio->flags.need_update) {
+		spin_lock(host_sgpio->share.plock);
+		if (nv_sgpio_get_func(host)
+			% NV_CNTRLR_SHARE_INIT == 0) {
+			host_sgpio->pcb->tx[host_offset].all &= mask;
+			mask = mask << 16;
+			tx.all &= mask;
+		} else {
+			tx.all &= mask;
+			mask = mask << 16;
+			host_sgpio->pcb->tx[host_offset].all &= mask;
+		}
+		host_sgpio->pcb->tx[host_offset].all |= tx.all;
+		spin_unlock(host_sgpio->share.plock);
+
+		if (nv_sgpio_send_cmd(phost, NV_SGPIO_CMD_WRITE_DATA)) {
+			host_sgpio->flags.need_update = 0;
+			return;
+		}
+	} else {
+		nv_sgpio_set_timer(&host_sgpio->sgpio_timer,
+				NV_SGPIO_UPDATE_TICK);
+	}
+err_out:
+	return;
+}
+
+static u8 nv_sgpio_send_cmd(struct nv_host_priv *host, u8 cmd)
+{
+	u8 csr;
+	unsigned long *ptstamp;
+	struct nv_host_sgpio *host_sgpio;
+
+	host_sgpio = &host->host_sgpio;
+	spin_lock(host_sgpio->share.plock);
+	ptstamp = host_sgpio->share.ptstamp;
+	if (jiffies_to_msecs(jiffies - *ptstamp) >= NV_SGPIO_MIN_UPDATE_DELTA) {
+		csr = inb((unsigned long)host_sgpio->pcsr);
+		if ((GET_SGPIO_STATUS(csr) != NV_SGPIO_STATE_OPERATIONAL) ||
+		    (GET_CMD_STATUS(csr) == NV_SGPIO_CMD_ACTIVE)) {
+			;
+		} else {
+			host_sgpio->pcb->cr0 =
+				SET_ENABLE(host_sgpio->pcb->cr0);
+			csr = 0;
+			csr = SET_CMD(csr, cmd);
+			outb(csr, (unsigned long)host_sgpio->pcsr);
+			*ptstamp = jiffies;
+		}
+		spin_unlock(host_sgpio->share.plock);
+		nv_sgpio_set_timer(&host_sgpio->sgpio_timer,
+			NV_SGPIO_UPDATE_TICK);
+		return 1;
+	} else {
+		spin_unlock(host_sgpio->share.plock);
+		nv_sgpio_set_timer(&host_sgpio->sgpio_timer,
+				(NV_SGPIO_MIN_UPDATE_DELTA -
+				jiffies_to_msecs(jiffies - *ptstamp)));
+		return 0;
+	}
+}
+
+static u8 nv_sgpio_update_led(struct nv_sgpio_led *led, u8 *on_off)
+{
+	u8 need_update = 0;
+
+	if (led->force_off > 0) {
+		led->force_off--;
+	} else if (led->flags.recent_activity ^ led->flags.last_state) {
+		*on_off = led->flags.recent_activity;
+		led->flags.last_state = led->flags.recent_activity;
+		need_update = 1;
+	} else if ((led->flags.recent_activity == NV_ON) &&
+		   (led->flags.last_state == NV_ON) &&
+		   (led->last_cons_active >= NV_SGPIO_MAX_ACTIVITY_ON)) {
+		*on_off = NV_OFF;
+		led->flags.last_state = NV_OFF;
+		led->force_off = NV_SGPIO_MIN_FORCE_OFF;
+		need_update = 1;
+	}
+
+	if (*on_off == NV_ON)
+		led->last_cons_active++;
+	else
+		led->last_cons_active = 0;
+
+	led->flags.recent_activity = NV_OFF;
+	return need_update;
+}
+
+static void nv_sgpio_reset(u8  *pcsr)
+{
+	u8 csr;
+
+	csr = inb((unsigned long)pcsr);
+	if (GET_SGPIO_STATUS(csr) == NV_SGPIO_STATE_RESET) {
+		csr = 0;
+		csr = SET_CMD(csr, NV_SGPIO_CMD_RESET);
+		outb(csr, (unsigned long)pcsr);
+	}
+	csr = 0;
+	csr = SET_CMD(csr, NV_SGPIO_CMD_READ_PARAMS);
+	outb(csr, (unsigned long)pcsr);
+}
+
+static void nv_sgpio_host_cleanup(struct nv_host_priv *host)
+{
+	u8 csr;
+	struct nv_host_sgpio *host_sgpio;
+
+	if (!host)
+		return;
+
+	host_sgpio = &host->host_sgpio;
+	if (host_sgpio->flags.sgpio_enabled) {
+		spin_lock(host_sgpio->share.plock);
+		host_sgpio->pcb->cr0 = SET_ENABLE(host_sgpio->pcb->cr0);
+		csr = 0;
+		csr = SET_CMD(csr, NV_SGPIO_CMD_WRITE_DATA);
+		outb(csr, (unsigned long)host_sgpio->pcsr);
+		spin_unlock(host_sgpio->share.plock);
+
+		if (timer_pending(&host_sgpio->sgpio_timer))
+			del_timer(&host_sgpio->sgpio_timer);
+		host_sgpio->flags.sgpio_enabled = 0;
+		host_sgpio->pcb->scratch_space = 0;
+	}
+}
+
+static void nv_sgpio_clear_all_leds(struct ata_port *ap)
+{
+	struct nv_port_priv *port = ap->private_data;
+	struct nv_host_priv *host;
+	u8 host_offset, port_offset;
+	struct nv_host_sgpio *host_sgpio;
+
+	if (!port || !ap->host)
+		return;
+	if (!ap->host->private_data)
+		return;
+
+	host = ap->host->private_data;
+	host_sgpio = &host->host_sgpio;
+	if (!host_sgpio->flags.sgpio_enabled)
+		return;
+
+	host_offset = nv_sgpio_tx_host_offset(ap->host);
+	port_offset = nv_sgpio_tx_port_offset(ap);
+
+	spin_lock(host_sgpio->share.plock);
+	host_sgpio->pcb->tx[host_offset].tx_port[port_offset] = 0;
+	host_sgpio->flags.need_update = 1;
+	spin_unlock(host_sgpio->share.plock);
+}
+
 static int __init nv_init(void)
 {
 	return pci_register_driver(&nv_pci_driver);

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH] SCSI: Add the SGPIO support for sata_nv.c
  2009-01-18  9:20         ` Yinghai Lu
  2009-01-18  9:22           ` [PATCH] sata_nv: sgpio for nvidia mcp55 Yinghai Lu
@ 2009-01-18 13:49           ` Yan Seiner
  2009-01-20  7:36             ` Peer Chen
  1 sibling, 1 reply; 27+ messages in thread
From: Yan Seiner @ 2009-01-18 13:49 UTC (permalink / raw)
  To: Peer Chen; +Cc: linux-ide

Yinghai Lu wrote:
> On Tue, Nov 6, 2007 at 8:09 PM, Yinghai Lu <yhlu.kernel@gmail.com> wrote:
>   
>> On Nov 7, 2006 1:55 AM, Peer Chen <pchen@nvidia.com> wrote:
>>     
>>> Modified and resent out the patch as attachment.
>>> Description about the patch:
>>> Add SGPIO support in sata_nv.c.
>>> SGPIO (Serial General Purpose Input Output) is a sideband serial 4-wire
>>> interface that a storage controller uses to communicate with a storage
>>> enclosure management controller, primarily to control activity and
>>> status LEDs that are located within drive bays or on a storage
>>> backplane. SGPIO is defined by [SFF8485].
>>> In this patch,we drive the LEDs to blink when read/write operation
>>> happen on SATA drives connect the corresponding ports on MCP55 board.
>>>       
I missed the start of  this thread....  Is it possible to blink the 
lights from user space?  This is very useful when trying to locate a 
dead or dying drive.   for example, /dev/sdc dies, but I have no way of 
knowing which one it is without unplugging all the cables one at a 
time.  A userspace utility that would allow me to blink a drive activity 
light would be very useful.

--Yan

^ permalink raw reply	[flat|nested] 27+ messages in thread

* RE: [PATCH] SCSI: Add the SGPIO support for sata_nv.c
  2009-01-18 13:49           ` [PATCH] SCSI: Add the SGPIO support for sata_nv.c Yan Seiner
@ 2009-01-20  7:36             ` Peer Chen
  0 siblings, 0 replies; 27+ messages in thread
From: Peer Chen @ 2009-01-20  7:36 UTC (permalink / raw)
  To: Yan Seiner; +Cc: linux-ide

Yes, I think it's possible to drive the LED from user space utility.

BRs
Peer Chen
 

> -----Original Message-----
> From: Yan Seiner [mailto:yan@seiner.com] 
> Sent: Sunday, January 18, 2009 9:50 PM
> To: Peer Chen
> Cc: linux-ide@vger.kernel.org
> Subject: Re: [PATCH] SCSI: Add the SGPIO support for sata_nv.c
> 
> Yinghai Lu wrote:
> > On Tue, Nov 6, 2007 at 8:09 PM, Yinghai Lu 
> <yhlu.kernel@gmail.com> wrote:
> >   
> >> On Nov 7, 2006 1:55 AM, Peer Chen <pchen@nvidia.com> wrote:
> >>     
> >>> Modified and resent out the patch as attachment.
> >>> Description about the patch:
> >>> Add SGPIO support in sata_nv.c.
> >>> SGPIO (Serial General Purpose Input Output) is a sideband serial 
> >>> 4-wire interface that a storage controller uses to 
> communicate with 
> >>> a storage enclosure management controller, primarily to control 
> >>> activity and status LEDs that are located within drive 
> bays or on a 
> >>> storage backplane. SGPIO is defined by [SFF8485].
> >>> In this patch,we drive the LEDs to blink when read/write 
> operation 
> >>> happen on SATA drives connect the corresponding ports on 
> MCP55 board.
> >>>       
> I missed the start of  this thread....  Is it possible to 
> blink the lights from user space?  This is very useful when 
> trying to locate a 
> dead or dying drive.   for example, /dev/sdc dies, but I have 
> no way of 
> knowing which one it is without unplugging all the cables one 
> at a time.  A userspace utility that would allow me to blink 
> a drive activity light would be very useful.
> 
> --Yan
> 
-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain
confidential information.  Any unauthorized review, use, disclosure or distribution
is prohibited.  If you are not the intended recipient, please contact the sender by
reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------

^ permalink raw reply	[flat|nested] 27+ messages in thread

* [PATCH] sata_nv: sgpio for nvidia mcp55 -v2
  2009-01-18  9:22           ` [PATCH] sata_nv: sgpio for nvidia mcp55 Yinghai Lu
@ 2009-01-21  2:07             ` Yinghai Lu
  2009-01-21 14:30               ` Yan Seiner
  2009-01-22  2:32               ` [PATCH] sata_nv: sgpio for nvidia mcp55 -v3 Yinghai Lu
  0 siblings, 2 replies; 27+ messages in thread
From: Yinghai Lu @ 2009-01-21  2:07 UTC (permalink / raw)
  To: Peer Chen, jeff, Kuan Luo
  Cc: Christoph Hellwig, linux-kernel, linux-ide, Ingo Molnar, Andrew Morton


Impact: new features

based on patch on
    http://marc.info/?l=linux-ide&m=116289338705418&w=2

1. update the patch for 2.6.19 to latest upstream ( 2.6.28?)
2. fix shared sgpio to support several mcp55 + io55, so every mcp55 have
   seperate spinlock
3. use scratch_source as numbering of sgpio instead of address of struct,
   so could go through kexec/kdump

v2: revert NV_ON and NV_OFF, so turn on Activity LED all the time when disk is idle.

Signed-off-by: Yinghai Lu <yinghai@kernel.org>

---
 drivers/ata/sata_nv.c |  536 ++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 536 insertions(+)

Index: linux-2.6/drivers/ata/sata_nv.c
===================================================================
--- linux-2.6.orig/drivers/ata/sata_nv.c
+++ linux-2.6/drivers/ata/sata_nv.c
@@ -200,6 +200,167 @@ enum {
 
 };
 
+/* sgpio
+ * Sgpio defines
+ * SGPIO state defines
+ */
+#define NV_SGPIO_STATE_RESET		0
+#define NV_SGPIO_STATE_OPERATIONAL	1
+#define NV_SGPIO_STATE_ERROR		2
+
+/* SGPIO command opcodes */
+#define NV_SGPIO_CMD_RESET		0
+#define NV_SGPIO_CMD_READ_PARAMS	1
+#define NV_SGPIO_CMD_READ_DATA		2
+#define NV_SGPIO_CMD_WRITE_DATA		3
+
+/* SGPIO command status defines */
+#define NV_SGPIO_CMD_OK			0
+#define NV_SGPIO_CMD_ACTIVE		1
+#define NV_SGPIO_CMD_ERR		2
+
+#define NV_SGPIO_UPDATE_TICK		90
+#define NV_SGPIO_MIN_UPDATE_DELTA	33
+#define NV_CNTRLR_SHARE_INIT		2
+#define NV_SGPIO_MAX_ACTIVITY_ON	10
+#define NV_SGPIO_MIN_FORCE_OFF		5
+#define NV_SGPIO_PCI_CSR_OFFSET		0x58
+#define NV_SGPIO_PCI_CB_OFFSET		0x5C
+#define NV_SGPIO_DFLT_CB_SIZE		256
+#define NV_ON				0
+#define NV_OFF				1
+
+static inline u8 bf_extract(u8 value, u8 offset, u8 bit_count)
+{
+	return (((u8)(value)) >> (offset)) & ((1 << (bit_count)) - 1);
+}
+
+static inline u8 bf_ins(u8 value, u8 ins, u8 offset, u8 bit_count)
+{
+	return	((value) & ~((((1 << (bit_count)) - 1)) << (offset))) |
+						(((u8)(ins)) << (offset));
+}
+
+static inline u32 bf_extract_u32(u32 value, u8 offset, u8 bit_count)
+{
+	return (((u32)(value)) >> (offset)) & ((1 << (bit_count)) - 1);
+}
+static inline u32 bf_ins_u32(u32 value, u32 ins, u8 offset, u8 bit_count)
+{
+	return	((value) & ~((((1 << (bit_count)) - 1)) << (offset))) |
+						(((u32)(ins)) << (offset));
+}
+
+#define GET_SGPIO_STATUS(v)	bf_extract(v, 0, 2)
+#define GET_CMD_STATUS(v)	bf_extract(v, 3, 2)
+#define GET_CMD(v)		bf_extract(v, 5, 3)
+#define SET_CMD(v, cmd)	bf_ins(v, cmd, 5, 3)
+
+#define GET_ENABLE(v)		bf_extract(v, 23, 1)
+#define SET_ENABLE(v)		bf_ins_u32(v, 1, 23, 1)
+
+/* Needs to have a u8 bit-field insert. */
+#define GET_ACTIVITY(v)		bf_extract(v, 5, 3)
+#define SET_ACTIVITY(v, on_off)	bf_ins(v, on_off, 5, 3)
+
+union nv_sgpio_nvcr {
+	struct {
+		u8	init_cnt;
+		u8	cb_size;
+		u8	cbver;
+		u8	rsvd;
+	} bit;
+	u32	all;
+};
+
+union nv_sgpio_tx {
+	u8	tx_port[4];
+	u32	all;
+};
+
+struct nv_sgpio_cb {
+	u64			scratch_space;
+	union nv_sgpio_nvcr	nvcr;
+	u32			cr0;
+	u32                     rsvd[4];
+	union nv_sgpio_tx       tx[2];
+};
+
+struct nv_sgpio_host_share {
+	spinlock_t	*plock;
+	unsigned long   *ptstamp;
+};
+
+struct nv_sgpio_host_flags {
+	u8	sgpio_enabled:1;
+	u8	need_update:1;
+	u8	rsvd:6;
+};
+
+struct nv_host_sgpio {
+	struct nv_sgpio_host_flags	flags;
+	u8				*pcsr;
+	struct nv_sgpio_cb		*pcb;
+	struct nv_sgpio_host_share	share;
+	struct timer_list		sgpio_timer;
+};
+
+struct nv_sgpio_port_flags {
+	u8	last_state:1;
+	u8	recent_activity:1;
+	u8	rsvd:6;
+};
+
+struct nv_sgpio_led {
+	struct nv_sgpio_port_flags	flags;
+	u8				force_off;
+	u8				last_cons_active;
+};
+
+struct nv_port_sgpio {
+	struct nv_sgpio_led	activity;
+};
+
+/* 1 mcp55 and 3 io55, 0 is not used */
+#define NR_SGPIO 5
+static spinlock_t nv_sgpio_lock[NR_SGPIO];
+static unsigned long nv_sgpio_tstamp[NR_SGPIO];
+static unsigned long nv_sgpio_share[NR_SGPIO] = {
+	[0 ... NR_SGPIO-1] = 0
+};
+
+static inline u8 nv_sgpio_get_func(struct ata_host *host)
+{
+	u8 devfn = (to_pci_dev(host->dev))->devfn;
+
+	return PCI_FUNC(devfn);
+}
+
+static inline u8 nv_sgpio_tx_host_offset(struct ata_host *host)
+{
+	return nv_sgpio_get_func(host)/NV_CNTRLR_SHARE_INIT;
+}
+
+static inline u8 nv_sgpio_calc_tx_offset(u8 cntrlr, u8 channel)
+{
+	return sizeof(union nv_sgpio_tx) - (NV_CNTRLR_SHARE_INIT *
+		(cntrlr % NV_CNTRLR_SHARE_INIT)) - channel - 1;
+}
+
+static inline u8 nv_sgpio_tx_port_offset(struct ata_port *ap)
+{
+	u8 cntrlr = nv_sgpio_get_func(ap->host);
+	return nv_sgpio_calc_tx_offset(cntrlr, ap->port_no);
+}
+
+static inline u8 nv_sgpio_capable(const struct pci_device_id *ent)
+{
+	if (ent->device == PCI_DEVICE_ID_NVIDIA_NFORCE_MCP55_SATA2)
+		return 1;
+	else
+		return 0;
+}
+
 /* ADMA Physical Region Descriptor - one SG segment */
 struct nv_adma_prd {
 	__le64			addr;
@@ -240,6 +401,7 @@ struct nv_adma_cpb {
 
 
 struct nv_adma_port_priv {
+	struct nv_port_sgpio	port_sgpio;
 	struct nv_adma_cpb	*cpb;
 	dma_addr_t		cpb_dma;
 	struct nv_adma_prd	*aprd;
@@ -254,6 +416,12 @@ struct nv_adma_port_priv {
 
 struct nv_host_priv {
 	unsigned long		type;
+	unsigned long		host_flags;
+	struct nv_host_sgpio	host_sgpio;
+};
+
+struct nv_port_priv {
+	struct nv_port_sgpio	port_sgpio;
 };
 
 struct defer_queue {
@@ -271,6 +439,8 @@ enum ncq_saw_flag_list {
 };
 
 struct nv_swncq_port_priv {
+	struct nv_port_sgpio	port_sgpio;
+
 	struct ata_prd	*prd;	 /* our SG list */
 	dma_addr_t	prd_dma; /* and its DMA mapping */
 	void __iomem	*sactive_block;
@@ -311,6 +481,10 @@ static int nv_nf2_hardreset(struct ata_l
 			    unsigned long deadline);
 static void nv_ck804_freeze(struct ata_port *ap);
 static void nv_ck804_thaw(struct ata_port *ap);
+static int nv_port_start(struct ata_port *ap);
+static void nv_port_stop(struct ata_port *ap);
+static void nv_host_stop(struct ata_host *host);
+static unsigned int nv_qc_issue(struct ata_queued_cmd *qc);
 static int nv_adma_slave_config(struct scsi_device *sdev);
 static int nv_adma_check_atapi_dma(struct ata_queued_cmd *qc);
 static void nv_adma_qc_prep(struct ata_queued_cmd *qc);
@@ -374,6 +548,17 @@ static const struct pci_device_id nv_pci
 	{ } /* terminate list */
 };
 
+/* SGPIO function prototypes */
+static void nv_sgpio_init(struct pci_dev *pdev, struct ata_host *host);
+static void nv_sgpio_reset(u8 *pcsr);
+static void nv_sgpio_set_timer(struct timer_list *ptimer,
+				unsigned int timeout_msec);
+static void nv_sgpio_timer_handler(unsigned long ptr);
+static void nv_sgpio_host_cleanup(struct nv_host_priv *host);
+static u8 nv_sgpio_update_led(struct nv_sgpio_led *led, u8 *on_off);
+static void nv_sgpio_clear_all_leds(struct ata_port *ap);
+static u8 nv_sgpio_send_cmd(struct nv_host_priv *host, u8 cmd);
+
 static struct pci_driver nv_pci_driver = {
 	.name			= DRV_NAME,
 	.id_table		= nv_pci_tbl,
@@ -409,6 +594,10 @@ static struct ata_port_operations nv_com
 	.inherits		= &ata_bmdma_port_ops,
 	.scr_read		= nv_scr_read,
 	.scr_write		= nv_scr_write,
+	.qc_issue               = nv_qc_issue,
+	.port_start		= nv_port_start,
+	.port_stop		= nv_port_stop,
+	.host_stop		= nv_host_stop,
 };
 
 /* OSDL bz11195 reports that link doesn't come online after hardreset
@@ -1395,6 +1584,8 @@ static unsigned int nv_adma_qc_issue(str
 	void __iomem *mmio = pp->ctl_block;
 	int curr_ncq = (qc->tf.protocol == ATA_PROT_NCQ);
 
+	pp->port_sgpio.activity.flags.recent_activity = NV_ON;
+
 	VPRINTK("ENTER\n");
 
 	/* We can't handle result taskfile with NCQ commands, since
@@ -1673,6 +1864,34 @@ static void nv_adma_error_handler(struct
 	ata_sff_error_handler(ap);
 }
 
+static int nv_port_start(struct ata_port *ap)
+{
+	struct device *dev = ap->host->dev;
+	struct nv_port_priv *pp;
+	int rc;
+
+	rc = ata_sff_port_start(ap);
+	if (rc)
+		return rc;
+
+	pp = devm_kzalloc(dev, sizeof(*pp), GFP_KERNEL);
+	if (!pp)
+		return -ENOMEM;
+
+	ap->private_data = pp;
+
+	return 0;
+}
+
+static unsigned int nv_qc_issue(struct ata_queued_cmd *qc)
+{
+       struct nv_port_priv *port = qc->ap->private_data;
+
+       port->port_sgpio.activity.flags.recent_activity = NV_ON;
+
+       return ata_sff_qc_issue(qc);
+}
+
 static void nv_swncq_qc_to_dq(struct ata_port *ap, struct ata_queued_cmd *qc)
 {
 	struct nv_swncq_port_priv *pp = ap->private_data;
@@ -2017,6 +2236,8 @@ static unsigned int nv_swncq_qc_issue(st
 	struct ata_port *ap = qc->ap;
 	struct nv_swncq_port_priv *pp = ap->private_data;
 
+	pp->port_sgpio.activity.flags.recent_activity = NV_ON;
+
 	if (qc->tf.protocol != ATA_PROT_NCQ)
 		return ata_sff_qc_issue(qc);
 
@@ -2404,6 +2625,9 @@ static int nv_init_one(struct pci_dev *p
 	} else if (type == SWNCQ)
 		nv_swncq_host_init(host);
 
+	if (nv_sgpio_capable(ent))
+		nv_sgpio_init(pdev, host);
+
 	pci_set_master(pdev);
 	return ata_host_activate(host, pdev->irq, ipriv->irq_handler,
 				 IRQF_SHARED, ipriv->sht);
@@ -2459,11 +2683,19 @@ static int nv_pci_device_resume(struct p
 }
 #endif
 
+static void nv_port_stop(struct ata_port *ap)
+{
+	nv_sgpio_clear_all_leds(ap);
+}
+
 static void nv_ck804_host_stop(struct ata_host *host)
 {
 	struct pci_dev *pdev = to_pci_dev(host->dev);
+	struct nv_host_priv *phost = host->private_data;
 	u8 regval;
 
+	nv_sgpio_host_cleanup(phost);
+
 	/* disable SATA space for CK804 */
 	pci_read_config_byte(pdev, NV_MCP_SATA_CFG_20, &regval);
 	regval &= ~NV_MCP_SATA_CFG_20_SATA_SPACE_EN;
@@ -2487,6 +2719,310 @@ static void nv_adma_host_stop(struct ata
 	nv_ck804_host_stop(host);
 }
 
+static void nv_host_stop(struct ata_host *host)
+{
+	struct nv_host_priv *phost = host->private_data;
+
+	nv_sgpio_host_cleanup(phost);
+}
+
+static void nv_sgpio_init(struct pci_dev *pdev, struct ata_host *host)
+{
+	u16 csr_add;
+	u32 cb_add, temp32;
+	struct nv_host_priv *phost = host->private_data;
+	struct nv_host_sgpio *host_sgpio;
+	struct nv_sgpio_cb *pcb;
+	u8 pro = 0;
+	unsigned int share_sgpio;
+	int i;
+
+	pci_read_config_word(pdev, NV_SGPIO_PCI_CSR_OFFSET, &csr_add);
+	pci_read_config_dword(pdev, NV_SGPIO_PCI_CB_OFFSET, &cb_add);
+
+	if (csr_add == 0 || cb_add == 0)
+		return;
+
+	if (cb_add <= 0x80000 || cb_add >= 0x9FC00)
+		return;
+
+	pci_read_config_byte(pdev, 0xA4, &pro);
+	if (!(pro & 0x40))
+		return;
+
+	temp32 = csr_add;
+	if (temp32 <= 0x200 || temp32 >= 0xFFFE)
+		return;
+
+	dev_printk(KERN_DEBUG, &pdev->dev, "CSR 0x%x CB 0x%x\n",
+			 csr_add, cb_add);
+
+	host_sgpio = &phost->host_sgpio;
+	pcb = ioremap(cb_add, 256);
+
+	if (pcb->nvcr.bit.init_cnt != 0x2 || pcb->nvcr.bit.cbver != 0x0)
+		return;
+
+	share_sgpio = pcb->scratch_space;
+	if (share_sgpio == 0 || share_sgpio > (NR_SGPIO - 1)) {
+		/* find one good position */
+		for (i = 1; i < NR_SGPIO; i++) {
+			if (!nv_sgpio_share[i]) {
+				share_sgpio = i;
+				break;
+			}
+		}
+		if (share_sgpio == 0 || share_sgpio > (NR_SGPIO - 1)) {
+			printk(KERN_WARNING "NR_SGPIO %d is too small\n",
+				 NR_SGPIO);
+			return;
+		}
+	}
+
+	host_sgpio->pcsr = (void *)(unsigned long)temp32;
+	host_sgpio->pcb = pcb;
+	if (!nv_sgpio_share[share_sgpio]) {
+		/* also handle kexec path:
+		 * scratch_space is set, but not get nv_sgpio_share yet
+		 */
+		spin_lock_init(&nv_sgpio_lock[share_sgpio]);
+		host_sgpio->share.plock = &nv_sgpio_lock[share_sgpio];
+		host_sgpio->share.ptstamp = &nv_sgpio_tstamp[share_sgpio];
+		nv_sgpio_share[share_sgpio] =
+			(unsigned long)&host_sgpio->share;
+		pcb->scratch_space = share_sgpio;
+		spin_lock(host_sgpio->share.plock);
+		nv_sgpio_reset(host_sgpio->pcsr);
+		pcb->cr0 = SET_ENABLE(pcb->cr0);
+		spin_unlock(host_sgpio->share.plock);
+	} else {
+		host_sgpio->share = *(struct nv_sgpio_host_share *)
+				nv_sgpio_share[share_sgpio];
+	}
+
+	dev_printk(KERN_DEBUG, &pdev->dev, "using sgpio %d\n", share_sgpio);
+	host_sgpio->flags.sgpio_enabled = 1;
+
+	init_timer(&host_sgpio->sgpio_timer);
+	host_sgpio->sgpio_timer.data = (unsigned long)host;
+	nv_sgpio_set_timer(&host_sgpio->sgpio_timer,
+				NV_SGPIO_UPDATE_TICK);
+}
+
+static void nv_sgpio_set_timer(struct timer_list *ptimer,
+			       unsigned int timeout_msec)
+{
+	if (!ptimer)
+		return;
+	ptimer->function = nv_sgpio_timer_handler;
+	ptimer->expires = msecs_to_jiffies(timeout_msec) + jiffies;
+	add_timer(ptimer);
+}
+
+static void nv_sgpio_timer_handler(unsigned long context)
+{
+
+	struct ata_host *host = (struct ata_host *)context;
+	struct nv_host_priv *phost;
+	u8 count, host_offset, port_offset;
+	union nv_sgpio_tx tx;
+	u8 on_off;
+	unsigned long mask = 0xFFFF;
+	struct nv_port_priv *port;
+	struct nv_host_sgpio *host_sgpio;
+	struct nv_sgpio_cb *pcb;
+
+	if (!host)
+		goto err_out;
+
+	phost = (struct nv_host_priv *)host->private_data;
+
+	host_sgpio = &phost->host_sgpio;
+	if (!host_sgpio->flags.sgpio_enabled)
+		goto err_out;
+
+	host_offset = nv_sgpio_tx_host_offset(host);
+	pcb = host_sgpio->pcb;
+
+	spin_lock(host_sgpio->share.plock);
+	tx = pcb->tx[host_offset];
+	spin_unlock(host_sgpio->share.plock);
+
+	for (count = 0; count < host->n_ports; count++) {
+		struct ata_port *ap;
+
+		ap = host->ports[count];
+
+		if (!(ap && !(ap->flags & ATA_FLAG_DISABLED)))
+			continue;
+
+		port = (struct nv_port_priv *)ap->private_data;
+		if (!port)
+			continue;
+		port_offset = nv_sgpio_tx_port_offset(ap);
+		on_off = GET_ACTIVITY(tx.tx_port[port_offset]);
+		if (nv_sgpio_update_led(&port->port_sgpio.activity, &on_off)) {
+			tx.tx_port[port_offset] =
+				SET_ACTIVITY(tx.tx_port[port_offset], on_off);
+			host_sgpio->flags.need_update = 1;
+	       }
+	}
+
+	if (host_sgpio->flags.need_update) {
+		spin_lock(host_sgpio->share.plock);
+		if (nv_sgpio_get_func(host)
+			% NV_CNTRLR_SHARE_INIT == 0) {
+			pcb->tx[host_offset].all &= mask;
+			mask = mask << 16;
+			tx.all &= mask;
+		} else {
+			tx.all &= mask;
+			mask = mask << 16;
+			pcb->tx[host_offset].all &= mask;
+		}
+		pcb->tx[host_offset].all |= tx.all;
+		spin_unlock(host_sgpio->share.plock);
+
+		if (nv_sgpio_send_cmd(phost, NV_SGPIO_CMD_WRITE_DATA)) {
+			host_sgpio->flags.need_update = 0;
+			return;
+		}
+	} else {
+		nv_sgpio_set_timer(&host_sgpio->sgpio_timer,
+				NV_SGPIO_UPDATE_TICK);
+	}
+err_out:
+	return;
+}
+
+static u8 nv_sgpio_send_cmd(struct nv_host_priv *host, u8 cmd)
+{
+	u8 csr;
+	unsigned long *ptstamp;
+	struct nv_host_sgpio *host_sgpio;
+
+	host_sgpio = &host->host_sgpio;
+	spin_lock(host_sgpio->share.plock);
+	ptstamp = host_sgpio->share.ptstamp;
+	if (jiffies_to_msecs(jiffies - *ptstamp) >= NV_SGPIO_MIN_UPDATE_DELTA) {
+		csr = inb((unsigned long)host_sgpio->pcsr);
+		if ((GET_SGPIO_STATUS(csr) != NV_SGPIO_STATE_OPERATIONAL) ||
+		    (GET_CMD_STATUS(csr) == NV_SGPIO_CMD_ACTIVE)) {
+			;
+		} else {
+			host_sgpio->pcb->cr0 =
+				SET_ENABLE(host_sgpio->pcb->cr0);
+			csr = 0;
+			csr = SET_CMD(csr, cmd);
+			outb(csr, (unsigned long)host_sgpio->pcsr);
+			*ptstamp = jiffies;
+		}
+		spin_unlock(host_sgpio->share.plock);
+		nv_sgpio_set_timer(&host_sgpio->sgpio_timer,
+			NV_SGPIO_UPDATE_TICK);
+		return 1;
+	} else {
+		spin_unlock(host_sgpio->share.plock);
+		nv_sgpio_set_timer(&host_sgpio->sgpio_timer,
+				(NV_SGPIO_MIN_UPDATE_DELTA -
+				jiffies_to_msecs(jiffies - *ptstamp)));
+		return 0;
+	}
+}
+
+static u8 nv_sgpio_update_led(struct nv_sgpio_led *led, u8 *on_off)
+{
+	u8 need_update = 0;
+
+	if (led->force_off > 0) {
+		led->force_off--;
+	} else if (led->flags.recent_activity ^ led->flags.last_state) {
+		*on_off = led->flags.recent_activity;
+		led->flags.last_state = led->flags.recent_activity;
+		need_update = 1;
+	} else if ((led->flags.recent_activity == NV_ON) &&
+		   (led->flags.last_state == NV_ON) &&
+		   (led->last_cons_active >= NV_SGPIO_MAX_ACTIVITY_ON)) {
+		*on_off = NV_OFF;
+		led->flags.last_state = NV_OFF;
+		led->force_off = NV_SGPIO_MIN_FORCE_OFF;
+		need_update = 1;
+	}
+
+	if (*on_off == NV_ON)
+		led->last_cons_active++;
+	else
+		led->last_cons_active = 0;
+
+	led->flags.recent_activity = NV_OFF;
+	return need_update;
+}
+
+static void nv_sgpio_reset(u8  *pcsr)
+{
+	u8 csr;
+
+	csr = inb((unsigned long)pcsr);
+	if (GET_SGPIO_STATUS(csr) == NV_SGPIO_STATE_RESET) {
+		csr = 0;
+		csr = SET_CMD(csr, NV_SGPIO_CMD_RESET);
+		outb(csr, (unsigned long)pcsr);
+	}
+	csr = 0;
+	csr = SET_CMD(csr, NV_SGPIO_CMD_READ_PARAMS);
+	outb(csr, (unsigned long)pcsr);
+}
+
+static void nv_sgpio_host_cleanup(struct nv_host_priv *host)
+{
+	u8 csr;
+	struct nv_host_sgpio *host_sgpio;
+
+	if (!host)
+		return;
+
+	host_sgpio = &host->host_sgpio;
+	if (host_sgpio->flags.sgpio_enabled) {
+		spin_lock(host_sgpio->share.plock);
+		host_sgpio->pcb->cr0 = SET_ENABLE(host_sgpio->pcb->cr0);
+		csr = 0;
+		csr = SET_CMD(csr, NV_SGPIO_CMD_WRITE_DATA);
+		outb(csr, (unsigned long)host_sgpio->pcsr);
+		spin_unlock(host_sgpio->share.plock);
+
+		if (timer_pending(&host_sgpio->sgpio_timer))
+			del_timer(&host_sgpio->sgpio_timer);
+		host_sgpio->flags.sgpio_enabled = 0;
+		host_sgpio->pcb->scratch_space = 0;
+	}
+}
+
+static void nv_sgpio_clear_all_leds(struct ata_port *ap)
+{
+	struct nv_port_priv *port = ap->private_data;
+	struct nv_host_priv *host;
+	u8 host_offset, port_offset;
+	struct nv_host_sgpio *host_sgpio;
+
+	if (!port || !ap->host)
+		return;
+	if (!ap->host->private_data)
+		return;
+
+	host = ap->host->private_data;
+	host_sgpio = &host->host_sgpio;
+	if (!host_sgpio->flags.sgpio_enabled)
+		return;
+
+	host_offset = nv_sgpio_tx_host_offset(ap->host);
+	port_offset = nv_sgpio_tx_port_offset(ap);
+
+	spin_lock(host_sgpio->share.plock);
+	host_sgpio->pcb->tx[host_offset].tx_port[port_offset] = 0;
+	host_sgpio->flags.need_update = 1;
+	spin_unlock(host_sgpio->share.plock);
+}
+
 static int __init nv_init(void)
 {
 	return pci_register_driver(&nv_pci_driver);

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH] sata_nv: sgpio for nvidia mcp55 -v2
  2009-01-21  2:07             ` [PATCH] sata_nv: sgpio for nvidia mcp55 -v2 Yinghai Lu
@ 2009-01-21 14:30               ` Yan Seiner
  2009-01-21 18:48                 ` Yinghai Lu
  2009-01-22  2:32               ` [PATCH] sata_nv: sgpio for nvidia mcp55 -v3 Yinghai Lu
  1 sibling, 1 reply; 27+ messages in thread
From: Yan Seiner @ 2009-01-21 14:30 UTC (permalink / raw)
  To: Yinghai Lu; +Cc: Peer Chen, linux-ide

Yinghai Lu wrote:
> Impact: new features
>
> based on patch on
>     http://marc.info/?l=linux-ide&m=116289338705418&w=2
>
> 1. update the patch for 2.6.19 to latest upstream ( 2.6.28?)
> 2. fix shared sgpio to support several mcp55 + io55, so every mcp55 have
>    seperate spinlock
> 3. use scratch_source as numbering of sgpio instead of address of struct,
>    so could go through kexec/kdump
>
> v2: revert NV_ON and NV_OFF, so turn on Activity LED all the time when disk is idle.
>   
I've tried both this patch and the previous version; neither does 
anything for me.  I am patching a vanilla 2.6.28.1 kernel.  :-(

I am running an Asus M2N-SLI Deluxe:

00:04.0 IDE interface: nVidia Corporation MCP55 IDE (rev a1)
00:05.0 IDE interface: nVidia Corporation MCP55 SATA Controller (rev a2)
00:05.1 IDE interface: nVidia Corporation MCP55 SATA Controller (rev a2)
00:05.2 IDE interface: nVidia Corporation MCP55 SATA Controller (rev a2)

All 6 channels are in use.  All of the drive activity lights are lit all 
the time.

Anything I can test?

--Yan

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH] sata_nv: sgpio for nvidia mcp55 -v2
  2009-01-21 14:30               ` Yan Seiner
@ 2009-01-21 18:48                 ` Yinghai Lu
  2009-01-22  4:46                   ` Yan Seiner
  0 siblings, 1 reply; 27+ messages in thread
From: Yinghai Lu @ 2009-01-21 18:48 UTC (permalink / raw)
  To: Yan Seiner; +Cc: Peer Chen, linux-ide

Yan Seiner wrote:
> Yinghai Lu wrote:
>> Impact: new features
>>
>> based on patch on
>>     http://marc.info/?l=linux-ide&m=116289338705418&w=2
>>
>> 1. update the patch for 2.6.19 to latest upstream ( 2.6.28?)
>> 2. fix shared sgpio to support several mcp55 + io55, so every mcp55 have
>>    seperate spinlock
>> 3. use scratch_source as numbering of sgpio instead of address of struct,
>>    so could go through kexec/kdump
>>
>> v2: revert NV_ON and NV_OFF, so turn on Activity LED all the time when
>> disk is idle.
>>   
> I've tried both this patch and the previous version; neither does
> anything for me.  I am patching a vanilla 2.6.28.1 kernel.  :-(
> 
> I am running an Asus M2N-SLI Deluxe:
> 
> 00:04.0 IDE interface: nVidia Corporation MCP55 IDE (rev a1)
> 00:05.0 IDE interface: nVidia Corporation MCP55 SATA Controller (rev a2)
> 00:05.1 IDE interface: nVidia Corporation MCP55 SATA Controller (rev a2)
> 00:05.2 IDE interface: nVidia Corporation MCP55 SATA Controller (rev a2)
> 
> All 6 channels are in use.  All of the drive activity lights are lit all
> the time.

can you boot with "debug" in command line?

wonder if your system has SGPIO etc backplane.

YH

^ permalink raw reply	[flat|nested] 27+ messages in thread

* [PATCH] sata_nv: sgpio for nvidia mcp55 -v3
  2009-01-21  2:07             ` [PATCH] sata_nv: sgpio for nvidia mcp55 -v2 Yinghai Lu
  2009-01-21 14:30               ` Yan Seiner
@ 2009-01-22  2:32               ` Yinghai Lu
  2009-01-27  4:57                 ` Andrew Morton
  2009-04-13  8:18                 ` Jeff Garzik
  1 sibling, 2 replies; 27+ messages in thread
From: Yinghai Lu @ 2009-01-22  2:32 UTC (permalink / raw)
  To: Peer Chen, jeff, Kuan Luo
  Cc: Christoph Hellwig, linux-kernel, linux-ide, Ingo Molnar, Andrew Morton


Impact: new features

based on patch on
    http://marc.info/?l=linux-ide&m=116289338705418&w=2

1. update the patch for 2.6.19 to latest upstream ( 2.6.28?)
2. fix shared sgpio to support several mcp55 + io55, so every mcp55 have
   seperate spinlock
3. use scratch_source as numbering of sgpio instead of address of struct,
   so could go through kexec/kdump

v2: revert NV_ON and NV_OFF, so turn on Activity LED all the time when disk is idle.
v3: use one timer per sgpio instead of per nv_host, so for mcp55+io55 system will use 2 timers
    instead of 6 timers.

Signed-off-by: Yinghai Lu <yinghai@kernel.org>

---
 drivers/ata/sata_nv.c |  562 ++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 562 insertions(+)

Index: linux-2.6/drivers/ata/sata_nv.c
===================================================================
--- linux-2.6.orig/drivers/ata/sata_nv.c
+++ linux-2.6/drivers/ata/sata_nv.c
@@ -200,6 +200,164 @@ enum {
 
 };
 
+/* sgpio
+ * Sgpio defines
+ * SGPIO state defines
+ */
+#define NV_SGPIO_STATE_RESET		0
+#define NV_SGPIO_STATE_OPERATIONAL	1
+#define NV_SGPIO_STATE_ERROR		2
+
+/* SGPIO command opcodes */
+#define NV_SGPIO_CMD_RESET		0
+#define NV_SGPIO_CMD_READ_PARAMS	1
+#define NV_SGPIO_CMD_READ_DATA		2
+#define NV_SGPIO_CMD_WRITE_DATA		3
+
+/* SGPIO command status defines */
+#define NV_SGPIO_CMD_OK			0
+#define NV_SGPIO_CMD_ACTIVE		1
+#define NV_SGPIO_CMD_ERR		2
+
+#define NV_SGPIO_UPDATE_TICK		90
+#define NV_CNTRLR_SHARE_INIT		2
+#define NV_SGPIO_MAX_ACTIVITY_ON	10
+#define NV_SGPIO_MIN_FORCE_OFF		5
+#define NV_SGPIO_PCI_CSR_OFFSET		0x58
+#define NV_SGPIO_PCI_CB_OFFSET		0x5C
+#define NV_SGPIO_DFLT_CB_SIZE		256
+#define NV_ON				0
+#define NV_OFF				1
+
+static inline u8 bf_extract(u8 value, u8 offset, u8 bit_count)
+{
+	return (((u8)(value)) >> (offset)) & ((1 << (bit_count)) - 1);
+}
+
+static inline u8 bf_ins(u8 value, u8 ins, u8 offset, u8 bit_count)
+{
+	return	((value) & ~((((1 << (bit_count)) - 1)) << (offset))) |
+						(((u8)(ins)) << (offset));
+}
+
+static inline u32 bf_extract_u32(u32 value, u8 offset, u8 bit_count)
+{
+	return (((u32)(value)) >> (offset)) & ((1 << (bit_count)) - 1);
+}
+static inline u32 bf_ins_u32(u32 value, u32 ins, u8 offset, u8 bit_count)
+{
+	return	((value) & ~((((1 << (bit_count)) - 1)) << (offset))) |
+						(((u32)(ins)) << (offset));
+}
+
+#define GET_SGPIO_STATUS(v)	bf_extract(v, 0, 2)
+#define GET_CMD_STATUS(v)	bf_extract(v, 3, 2)
+#define GET_CMD(v)		bf_extract(v, 5, 3)
+#define SET_CMD(v, cmd)	bf_ins(v, cmd, 5, 3)
+
+#define GET_ENABLE(v)		bf_extract(v, 23, 1)
+#define SET_ENABLE(v)		bf_ins_u32(v, 1, 23, 1)
+
+/* Needs to have a u8 bit-field insert. */
+#define GET_ACTIVITY(v)		bf_extract(v, 5, 3)
+#define SET_ACTIVITY(v, on_off)	bf_ins(v, on_off, 5, 3)
+
+union nv_sgpio_nvcr {
+	struct {
+		u8	init_cnt;
+		u8	cb_size;
+		u8	cbver;
+		u8	rsvd;
+	} bit;
+	u32	all;
+};
+
+union nv_sgpio_tx {
+	u8	tx_port[4];
+	u32	all;
+};
+
+struct nv_sgpio_cb {
+	u64			scratch_space;
+	union nv_sgpio_nvcr	nvcr;
+	u32			cr0;
+	u32                     rsvd[4];
+	union nv_sgpio_tx       tx[2];
+};
+
+#define NR_SGPIO_SHARE_HOST 4
+struct nv_sgpio_host_share {
+	spinlock_t		lock;
+	struct timer_list	timer;
+	unsigned int		need_update;
+	struct ata_host		*host[NR_SGPIO_SHARE_HOST];
+};
+
+struct nv_sgpio_host_flags {
+	u8	sgpio_enabled:1;
+	u8	need_update:1;
+	u8	rsvd:6;
+};
+
+struct nv_host_sgpio {
+	struct nv_sgpio_host_flags	flags;
+	u8				*pcsr;
+	struct nv_sgpio_cb		*pcb;
+	unsigned  int			share_index;
+};
+
+struct nv_sgpio_port_flags {
+	u8	last_state:1;
+	u8	recent_activity:1;
+	u8	rsvd:6;
+};
+
+struct nv_sgpio_led {
+	struct nv_sgpio_port_flags	flags;
+	u8				force_off;
+	u8				last_cons_active;
+};
+
+struct nv_port_sgpio {
+	struct nv_sgpio_led	activity;
+};
+
+/* 1 mcp55 and 3 io55, 0 is not used */
+#define NR_SGPIO 5
+static struct nv_sgpio_host_share nv_sgpio_share[NR_SGPIO];
+
+static inline u8 nv_sgpio_get_func(struct ata_host *host)
+{
+	u8 devfn = (to_pci_dev(host->dev))->devfn;
+
+	return PCI_FUNC(devfn);
+}
+
+static inline u8 nv_sgpio_tx_host_offset(struct ata_host *host)
+{
+	return nv_sgpio_get_func(host)/NV_CNTRLR_SHARE_INIT;
+}
+
+static inline u8 nv_sgpio_calc_tx_offset(u8 cntrlr, u8 channel)
+{
+	return sizeof(union nv_sgpio_tx) - (NV_CNTRLR_SHARE_INIT *
+		(cntrlr % NV_CNTRLR_SHARE_INIT)) - channel - 1;
+}
+
+static inline u8 nv_sgpio_tx_port_offset(struct ata_port *ap)
+{
+	u8 cntrlr = nv_sgpio_get_func(ap->host);
+	return nv_sgpio_calc_tx_offset(cntrlr, ap->port_no);
+}
+
+static inline u8 nv_sgpio_capable(const struct pci_device_id *ent)
+{
+	if (ent->device == PCI_DEVICE_ID_NVIDIA_NFORCE_MCP55_SATA2)
+		return 1;
+	else
+		return 0;
+}
+
 /* ADMA Physical Region Descriptor - one SG segment */
 struct nv_adma_prd {
 	__le64			addr;
@@ -240,6 +398,7 @@ struct nv_adma_cpb {
 
 
 struct nv_adma_port_priv {
+	struct nv_port_sgpio	port_sgpio;
 	struct nv_adma_cpb	*cpb;
 	dma_addr_t		cpb_dma;
 	struct nv_adma_prd	*aprd;
@@ -254,6 +413,12 @@ struct nv_adma_port_priv {
 
 struct nv_host_priv {
 	unsigned long		type;
+	unsigned long		host_flags;
+	struct nv_host_sgpio	host_sgpio;
+};
+
+struct nv_port_priv {
+	struct nv_port_sgpio	port_sgpio;
 };
 
 struct defer_queue {
@@ -271,6 +436,8 @@ enum ncq_saw_flag_list {
 };
 
 struct nv_swncq_port_priv {
+	struct nv_port_sgpio	port_sgpio;
+
 	struct ata_prd	*prd;	 /* our SG list */
 	dma_addr_t	prd_dma; /* and its DMA mapping */
 	void __iomem	*sactive_block;
@@ -311,6 +478,10 @@ static int nv_nf2_hardreset(struct ata_l
 			    unsigned long deadline);
 static void nv_ck804_freeze(struct ata_port *ap);
 static void nv_ck804_thaw(struct ata_port *ap);
+static int nv_port_start(struct ata_port *ap);
+static void nv_port_stop(struct ata_port *ap);
+static void nv_host_stop(struct ata_host *host);
+static unsigned int nv_qc_issue(struct ata_queued_cmd *qc);
 static int nv_adma_slave_config(struct scsi_device *sdev);
 static int nv_adma_check_atapi_dma(struct ata_queued_cmd *qc);
 static void nv_adma_qc_prep(struct ata_queued_cmd *qc);
@@ -374,6 +545,17 @@ static const struct pci_device_id nv_pci
 	{ } /* terminate list */
 };
 
+/* SGPIO function prototypes */
+static void nv_sgpio_init(struct pci_dev *pdev, struct ata_host *host);
+static void nv_sgpio_reset(u8 *pcsr);
+static void nv_sgpio_set_timer(struct timer_list *ptimer,
+				unsigned int timeout_msec);
+static void nv_sgpio_timer_handler(unsigned long ptr);
+static void nv_sgpio_host_cleanup(struct nv_host_priv *host);
+static u8 nv_sgpio_update_led(struct nv_sgpio_led *led, u8 *on_off);
+static void nv_sgpio_clear_all_leds(struct ata_port *ap);
+static u8 nv_sgpio_send_cmd(struct nv_sgpio_host_share *sgpio_share, u8 cmd);
+
 static struct pci_driver nv_pci_driver = {
 	.name			= DRV_NAME,
 	.id_table		= nv_pci_tbl,
@@ -409,6 +591,10 @@ static struct ata_port_operations nv_com
 	.inherits		= &ata_bmdma_port_ops,
 	.scr_read		= nv_scr_read,
 	.scr_write		= nv_scr_write,
+	.qc_issue               = nv_qc_issue,
+	.port_start		= nv_port_start,
+	.port_stop		= nv_port_stop,
+	.host_stop		= nv_host_stop,
 };
 
 /* OSDL bz11195 reports that link doesn't come online after hardreset
@@ -1395,6 +1581,8 @@ static unsigned int nv_adma_qc_issue(str
 	void __iomem *mmio = pp->ctl_block;
 	int curr_ncq = (qc->tf.protocol == ATA_PROT_NCQ);
 
+	pp->port_sgpio.activity.flags.recent_activity = NV_ON;
+
 	VPRINTK("ENTER\n");
 
 	/* We can't handle result taskfile with NCQ commands, since
@@ -1673,6 +1861,34 @@ static void nv_adma_error_handler(struct
 	ata_sff_error_handler(ap);
 }
 
+static int nv_port_start(struct ata_port *ap)
+{
+	struct device *dev = ap->host->dev;
+	struct nv_port_priv *pp;
+	int rc;
+
+	rc = ata_sff_port_start(ap);
+	if (rc)
+		return rc;
+
+	pp = devm_kzalloc(dev, sizeof(*pp), GFP_KERNEL);
+	if (!pp)
+		return -ENOMEM;
+
+	ap->private_data = pp;
+
+	return 0;
+}
+
+static unsigned int nv_qc_issue(struct ata_queued_cmd *qc)
+{
+       struct nv_port_priv *port = qc->ap->private_data;
+
+       port->port_sgpio.activity.flags.recent_activity = NV_ON;
+
+       return ata_sff_qc_issue(qc);
+}
+
 static void nv_swncq_qc_to_dq(struct ata_port *ap, struct ata_queued_cmd *qc)
 {
 	struct nv_swncq_port_priv *pp = ap->private_data;
@@ -2017,6 +2233,8 @@ static unsigned int nv_swncq_qc_issue(st
 	struct ata_port *ap = qc->ap;
 	struct nv_swncq_port_priv *pp = ap->private_data;
 
+	pp->port_sgpio.activity.flags.recent_activity = NV_ON;
+
 	if (qc->tf.protocol != ATA_PROT_NCQ)
 		return ata_sff_qc_issue(qc);
 
@@ -2404,6 +2622,9 @@ static int nv_init_one(struct pci_dev *p
 	} else if (type == SWNCQ)
 		nv_swncq_host_init(host);
 
+	if (nv_sgpio_capable(ent))
+		nv_sgpio_init(pdev, host);
+
 	pci_set_master(pdev);
 	return ata_host_activate(host, pdev->irq, ipriv->irq_handler,
 				 IRQF_SHARED, ipriv->sht);
@@ -2459,11 +2680,19 @@ static int nv_pci_device_resume(struct p
 }
 #endif
 
+static void nv_port_stop(struct ata_port *ap)
+{
+	nv_sgpio_clear_all_leds(ap);
+}
+
 static void nv_ck804_host_stop(struct ata_host *host)
 {
 	struct pci_dev *pdev = to_pci_dev(host->dev);
+	struct nv_host_priv *phost = host->private_data;
 	u8 regval;
 
+	nv_sgpio_host_cleanup(phost);
+
 	/* disable SATA space for CK804 */
 	pci_read_config_byte(pdev, NV_MCP_SATA_CFG_20, &regval);
 	regval &= ~NV_MCP_SATA_CFG_20_SATA_SPACE_EN;
@@ -2487,6 +2716,339 @@ static void nv_adma_host_stop(struct ata
 	nv_ck804_host_stop(host);
 }
 
+static void nv_host_stop(struct ata_host *host)
+{
+	struct nv_host_priv *phost = host->private_data;
+
+	nv_sgpio_host_cleanup(phost);
+}
+
+static void nv_sgpio_init(struct pci_dev *pdev, struct ata_host *host)
+{
+	u16 csr_add;
+	u32 cb_add, temp32;
+	struct nv_host_priv *phost = host->private_data;
+	struct nv_host_sgpio *host_sgpio;
+	struct nv_sgpio_host_share *sgpio_share;
+	struct nv_sgpio_cb *pcb;
+	u8 pro = 0;
+	unsigned int share_index;
+	int i;
+
+	pci_read_config_word(pdev, NV_SGPIO_PCI_CSR_OFFSET, &csr_add);
+	pci_read_config_dword(pdev, NV_SGPIO_PCI_CB_OFFSET, &cb_add);
+
+	if (csr_add == 0 || cb_add == 0)
+		return;
+
+	if (cb_add <= 0x80000 || cb_add >= 0x9FC00)
+		return;
+
+	pci_read_config_byte(pdev, 0xA4, &pro);
+	if (!(pro & 0x40))
+		return;
+
+	temp32 = csr_add;
+	if (temp32 <= 0x200 || temp32 >= 0xFFFE)
+		return;
+
+	dev_printk(KERN_DEBUG, &pdev->dev, "CSR 0x%x CB 0x%x\n",
+			 csr_add, cb_add);
+
+	host_sgpio = &phost->host_sgpio;
+	pcb = ioremap(cb_add, 256);
+
+	if (pcb->nvcr.bit.init_cnt != 0x2 || pcb->nvcr.bit.cbver != 0x0)
+		return;
+
+	share_index = pcb->scratch_space;
+	if (share_index == 0 || share_index > (NR_SGPIO - 1)) {
+		/* find one good position */
+		for (i = 1; i < NR_SGPIO; i++) {
+			if (!nv_sgpio_share[i].host[0]) {
+				share_index = i;
+				break;
+			}
+		}
+		if (share_index == 0 || share_index > (NR_SGPIO - 1)) {
+			printk(KERN_WARNING "NR_SGPIO %d is too small\n",
+				 NR_SGPIO);
+			return;
+		}
+	}
+
+	host_sgpio->pcsr = (void *)(unsigned long)temp32;
+	host_sgpio->pcb = pcb;
+	sgpio_share = &nv_sgpio_share[share_index];
+	if (!sgpio_share->host[0]) {
+		/* also handle kexec path:
+		 * scratch_space is set, but not get nv_sgpio_share yet
+		 */
+		spin_lock_init(&sgpio_share->lock);
+		spin_lock(&sgpio_share->lock);
+		sgpio_share->host[0] = host;
+		host_sgpio->share_index = share_index;
+		pcb->scratch_space = share_index;
+		nv_sgpio_reset(host_sgpio->pcsr);
+		pcb->cr0 = SET_ENABLE(pcb->cr0);
+		init_timer(&sgpio_share->timer);
+		sgpio_share->timer.data = (unsigned long)sgpio_share;
+		nv_sgpio_set_timer(&sgpio_share->timer, NV_SGPIO_UPDATE_TICK);
+		spin_unlock(&sgpio_share->lock);
+	} else {
+		spin_lock(&sgpio_share->lock);
+		for (i = 1; i < NR_SGPIO_SHARE_HOST; i++) {
+			if (!sgpio_share->host[i]) {
+				sgpio_share->host[i] = host;
+				host_sgpio->share_index = share_index;
+				break;
+			}
+		}
+		if (i == NR_SGPIO_SHARE_HOST)
+			printk(KERN_WARNING "NR_SGPIO_SHARE_HOST %d is too small\n",
+					 NR_SGPIO_SHARE_HOST);
+		spin_unlock(&sgpio_share->lock);
+	}
+
+	dev_printk(KERN_DEBUG, &pdev->dev, "using sgpio %d\n", share_index);
+	host_sgpio->flags.sgpio_enabled = 1;
+
+}
+
+static void nv_sgpio_set_timer(struct timer_list *ptimer,
+			       unsigned int timeout_msec)
+{
+	if (!ptimer)
+		return;
+	ptimer->function = nv_sgpio_timer_handler;
+	ptimer->expires = msecs_to_jiffies(timeout_msec) + jiffies;
+	add_timer(ptimer);
+}
+
+static void nv_sgpio_timer_handler(unsigned long context)
+{
+
+	struct nv_sgpio_host_share *sgpio_share = (struct nv_sgpio_host_share *)context;
+	struct ata_host *host;
+	struct nv_host_priv *phost;
+	u8 count, host_offset, port_offset;
+	union nv_sgpio_tx tx;
+	u8 on_off;
+	unsigned long mask;
+	struct nv_port_priv *port;
+	struct nv_host_sgpio *host_sgpio;
+	struct nv_sgpio_cb *pcb;
+	int i;
+
+	for (i = 0; i < NR_SGPIO_SHARE_HOST; i++) {
+		host = sgpio_share->host[i];
+		/* not blank slot */
+		if (!host)
+			break;
+
+		phost = (struct nv_host_priv *)host->private_data;
+
+		host_sgpio = &phost->host_sgpio;
+		if (!host_sgpio->flags.sgpio_enabled)
+			continue;
+
+		mask = 0xFFFF;
+		host_offset = nv_sgpio_tx_host_offset(host);
+		pcb = host_sgpio->pcb;
+
+		spin_lock(&sgpio_share->lock);
+		tx = pcb->tx[host_offset];
+		spin_unlock(&sgpio_share->lock);
+
+		for (count = 0; count < host->n_ports; count++) {
+			struct ata_port *ap;
+
+			ap = host->ports[count];
+
+			if (!(ap && !(ap->flags & ATA_FLAG_DISABLED)))
+				continue;
+
+			port = (struct nv_port_priv *)ap->private_data;
+			if (!port)
+				continue;
+			port_offset = nv_sgpio_tx_port_offset(ap);
+			on_off = GET_ACTIVITY(tx.tx_port[port_offset]);
+			if (nv_sgpio_update_led(&port->port_sgpio.activity, &on_off)) {
+				tx.tx_port[port_offset] =
+					SET_ACTIVITY(tx.tx_port[port_offset], on_off);
+				host_sgpio->flags.need_update = 1;
+		       }
+		}
+
+		if (host_sgpio->flags.need_update) {
+			sgpio_share->need_update = 1;
+			spin_lock(&sgpio_share->lock);
+			if (nv_sgpio_get_func(host)
+				% NV_CNTRLR_SHARE_INIT == 0) {
+				pcb->tx[host_offset].all &= mask;
+				mask = mask << 16;
+				tx.all &= mask;
+			} else {
+				tx.all &= mask;
+				mask = mask << 16;
+				pcb->tx[host_offset].all &= mask;
+			}
+			pcb->tx[host_offset].all |= tx.all;
+			spin_unlock(&sgpio_share->lock);
+		}
+	}
+
+	if (sgpio_share->need_update) {
+		if (nv_sgpio_send_cmd(sgpio_share, NV_SGPIO_CMD_WRITE_DATA)) {
+			sgpio_share->need_update = 0;
+			for (i = 0; i < NR_SGPIO_SHARE_HOST; i++) {
+				host = sgpio_share->host[i];
+				/* not blank slot */
+				if (!host)
+					break;
+
+				phost = (struct nv_host_priv *)host->private_data;
+
+				host_sgpio = &phost->host_sgpio;
+				if (!host_sgpio->flags.sgpio_enabled)
+					continue;
+
+				host_sgpio->flags.need_update = 0;
+			}
+		}
+	}
+
+	nv_sgpio_set_timer(&sgpio_share->timer, NV_SGPIO_UPDATE_TICK);
+	return;
+}
+
+static u8 nv_sgpio_send_cmd(struct nv_sgpio_host_share *sgpio_share, u8 cmd)
+{
+	u8 csr;
+	struct nv_host_sgpio *host_sgpio;
+	struct nv_host_priv *phost;
+
+	phost = (struct nv_host_priv *)sgpio_share->host[0]->private_data;
+	host_sgpio = &phost->host_sgpio;
+	spin_lock(&sgpio_share->lock);
+	csr = inb((unsigned long)host_sgpio->pcsr);
+	if ((GET_SGPIO_STATUS(csr) != NV_SGPIO_STATE_OPERATIONAL) ||
+	    (GET_CMD_STATUS(csr) == NV_SGPIO_CMD_ACTIVE)) {
+		;
+	} else {
+		host_sgpio->pcb->cr0 =
+			SET_ENABLE(host_sgpio->pcb->cr0);
+		csr = 0;
+		csr = SET_CMD(csr, cmd);
+		outb(csr, (unsigned long)host_sgpio->pcsr);
+	}
+	spin_unlock(&sgpio_share->lock);
+	return 1;
+}
+
+static u8 nv_sgpio_update_led(struct nv_sgpio_led *led, u8 *on_off)
+{
+	u8 need_update = 0;
+
+	if (led->force_off > 0) {
+		led->force_off--;
+	} else if (led->flags.recent_activity ^ led->flags.last_state) {
+		*on_off = led->flags.recent_activity;
+		led->flags.last_state = led->flags.recent_activity;
+		need_update = 1;
+	} else if ((led->flags.recent_activity == NV_ON) &&
+		   (led->flags.last_state == NV_ON) &&
+		   (led->last_cons_active >= NV_SGPIO_MAX_ACTIVITY_ON)) {
+		*on_off = NV_OFF;
+		led->flags.last_state = NV_OFF;
+		led->force_off = NV_SGPIO_MIN_FORCE_OFF;
+		need_update = 1;
+	}
+
+	if (*on_off == NV_ON)
+		led->last_cons_active++;
+	else
+		led->last_cons_active = 0;
+
+	led->flags.recent_activity = NV_OFF;
+	return need_update;
+}
+
+static void nv_sgpio_reset(u8  *pcsr)
+{
+	u8 csr;
+
+	csr = inb((unsigned long)pcsr);
+	if (GET_SGPIO_STATUS(csr) == NV_SGPIO_STATE_RESET) {
+		csr = 0;
+		csr = SET_CMD(csr, NV_SGPIO_CMD_RESET);
+		outb(csr, (unsigned long)pcsr);
+	}
+	csr = 0;
+	csr = SET_CMD(csr, NV_SGPIO_CMD_READ_PARAMS);
+	outb(csr, (unsigned long)pcsr);
+}
+
+static void nv_sgpio_host_cleanup(struct nv_host_priv *host)
+{
+	u8 csr;
+	struct nv_host_sgpio *host_sgpio;
+	struct nv_sgpio_host_share *sgpio_share;
+
+	if (!host)
+		return;
+
+	host_sgpio = &host->host_sgpio;
+	if (!host_sgpio->share_index)
+		return;
+
+	sgpio_share = &nv_sgpio_share[host_sgpio->share_index];
+	if (host_sgpio->flags.sgpio_enabled) {
+		spin_lock(&sgpio_share->lock);
+		host_sgpio->pcb->cr0 = SET_ENABLE(host_sgpio->pcb->cr0);
+		csr = 0;
+		csr = SET_CMD(csr, NV_SGPIO_CMD_WRITE_DATA);
+		outb(csr, (unsigned long)host_sgpio->pcsr);
+		spin_unlock(&sgpio_share->lock);
+
+		if (timer_pending(&sgpio_share->timer))
+			del_timer(&sgpio_share->timer);
+		host_sgpio->flags.sgpio_enabled = 0;
+		host_sgpio->pcb->scratch_space = 0;
+	}
+}
+
+static void nv_sgpio_clear_all_leds(struct ata_port *ap)
+{
+	struct nv_port_priv *port = ap->private_data;
+	struct nv_host_priv *host;
+	u8 host_offset, port_offset;
+	struct nv_host_sgpio *host_sgpio;
+	struct nv_sgpio_host_share *sgpio_share;
+
+	if (!port || !ap->host)
+		return;
+	if (!ap->host->private_data)
+		return;
+
+	host = ap->host->private_data;
+	host_sgpio = &host->host_sgpio;
+	if (!host_sgpio->share_index)
+		return;
+
+	sgpio_share = &nv_sgpio_share[host_sgpio->share_index];
+	if (!host_sgpio->flags.sgpio_enabled)
+		return;
+
+	host_offset = nv_sgpio_tx_host_offset(ap->host);
+	port_offset = nv_sgpio_tx_port_offset(ap);
+
+	spin_lock(&sgpio_share->lock);
+	host_sgpio->pcb->tx[host_offset].tx_port[port_offset] = 0;
+	host_sgpio->flags.need_update = 1;
+	spin_unlock(&sgpio_share->lock);
+}
+
 static int __init nv_init(void)
 {
 	return pci_register_driver(&nv_pci_driver);

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH] sata_nv: sgpio for nvidia mcp55 -v2
  2009-01-21 18:48                 ` Yinghai Lu
@ 2009-01-22  4:46                   ` Yan Seiner
  2009-01-22  5:07                     ` Yinghai Lu
  0 siblings, 1 reply; 27+ messages in thread
From: Yan Seiner @ 2009-01-22  4:46 UTC (permalink / raw)
  To: Yinghai Lu; +Cc: Peer Chen, linux-ide

Yinghai Lu wrote:
> Yan Seiner wrote:
>   
>> Yinghai Lu wrote:
>>     
>>> Impact: new features
>>>
>>> based on patch on
>>>     http://marc.info/?l=linux-ide&m=116289338705418&w=2
>>>
>>> 1. update the patch for 2.6.19 to latest upstream ( 2.6.28?)
>>> 2. fix shared sgpio to support several mcp55 + io55, so every mcp55 have
>>>    seperate spinlock
>>> 3. use scratch_source as numbering of sgpio instead of address of struct,
>>>    so could go through kexec/kdump
>>>
>>> v2: revert NV_ON and NV_OFF, so turn on Activity LED all the time when
>>> disk is idle.
>>>   
>>>       
>> I've tried both this patch and the previous version; neither does
>> anything for me.  I am patching a vanilla 2.6.28.1 kernel.  :-(
>>
>> I am running an Asus M2N-SLI Deluxe:
>>
>> 00:04.0 IDE interface: nVidia Corporation MCP55 IDE (rev a1)
>> 00:05.0 IDE interface: nVidia Corporation MCP55 SATA Controller (rev a2)
>> 00:05.1 IDE interface: nVidia Corporation MCP55 SATA Controller (rev a2)
>> 00:05.2 IDE interface: nVidia Corporation MCP55 SATA Controller (rev a2)
>>
>> All 6 channels are in use.  All of the drive activity lights are lit all
>> the time.
>>     
>
> can you boot with "debug" in command line?
>
> wonder if your system has SGPIO etc backplane.
>   
It has a Norco LIB-700 backplane.  The backplane claims to have drive 
activity LEDs as well as power LEDs, and two LEDs do light up for each 
drive....

http://www.ipcdirect.net/servlet/Detail?no=75
http://www.norco.net.cn/UpLoadFile/Datasheet/DS-600S-EN.pdf

Is there any way to set and read the GPIO from userspace?

I haven't tried v3 of the patch and I can't shut the system down ATM to 
reboot with the debug parameter.

--Yan



> YH
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ide" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
> !DSPAM:49776e26281921804284693!
>
>   


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH] sata_nv: sgpio for nvidia mcp55 -v2
  2009-01-22  4:46                   ` Yan Seiner
@ 2009-01-22  5:07                     ` Yinghai Lu
  2009-01-22 16:22                       ` Yan Seiner
  0 siblings, 1 reply; 27+ messages in thread
From: Yinghai Lu @ 2009-01-22  5:07 UTC (permalink / raw)
  To: Yan Seiner; +Cc: Peer Chen, linux-ide

Yan Seiner wrote:
> Yinghai Lu wrote:
>> Yan Seiner wrote:
>>  
>>> Yinghai Lu wrote:
>>>    
>>>> Impact: new features
>>>>
>>>> based on patch on
>>>>     http://marc.info/?l=linux-ide&m=116289338705418&w=2
>>>>
>>>> 1. update the patch for 2.6.19 to latest upstream ( 2.6.28?)
>>>> 2. fix shared sgpio to support several mcp55 + io55, so every mcp55
>>>> have
>>>>    seperate spinlock
>>>> 3. use scratch_source as numbering of sgpio instead of address of
>>>> struct,
>>>>    so could go through kexec/kdump
>>>>
>>>> v2: revert NV_ON and NV_OFF, so turn on Activity LED all the time when
>>>> disk is idle.
>>>>         
>>> I've tried both this patch and the previous version; neither does
>>> anything for me.  I am patching a vanilla 2.6.28.1 kernel.  :-(
>>>
>>> I am running an Asus M2N-SLI Deluxe:
>>>
>>> 00:04.0 IDE interface: nVidia Corporation MCP55 IDE (rev a1)
>>> 00:05.0 IDE interface: nVidia Corporation MCP55 SATA Controller (rev a2)
>>> 00:05.1 IDE interface: nVidia Corporation MCP55 SATA Controller (rev a2)
>>> 00:05.2 IDE interface: nVidia Corporation MCP55 SATA Controller (rev a2)
>>>
>>> All 6 channels are in use.  All of the drive activity lights are lit all
>>> the time.
>>>     
>>
>> can you boot with "debug" in command line?
>>
>> wonder if your system has SGPIO etc backplane.
>>   
> It has a Norco LIB-700 backplane.  The backplane claims to have drive
> activity LEDs as well as power LEDs, and two LEDs do light up for each
> drive....
> 
> http://www.ipcdirect.net/servlet/Detail?no=75
> http://www.norco.net.cn/UpLoadFile/Datasheet/DS-600S-EN.pdf
> 
> Is there any way to set and read the GPIO from userspace?
> 
> I haven't tried v3 of the patch and I can't shut the system down ATM to
> reboot with the debug parameter.

please try -v3 and boot with debug in command line, then post bootlog.

YH

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH] sata_nv: sgpio for nvidia mcp55 -v2
  2009-01-22  5:07                     ` Yinghai Lu
@ 2009-01-22 16:22                       ` Yan Seiner
  2009-01-22 17:26                         ` Yinghai Lu
  0 siblings, 1 reply; 27+ messages in thread
From: Yan Seiner @ 2009-01-22 16:22 UTC (permalink / raw)
  To: Yinghai Lu; +Cc: Yan Seiner, Peer Chen, linux-ide


On Wed, January 21, 2009 9:07 pm, Yinghai Lu wrote:
>
> please try -v3 and boot with debug in command line, then post bootlog.

Dumb question:

Where does the debug parameter go?  Kernel command line?

sata_nv doesn't have a debug option.

--Yan

-- 
  o__
  ,>/'_          o__
  (_)\(_)        ,>/'_        o__
Yan Seiner      (_)\(_)       ,>/'_     o__
       Personal Trainer      (_)\(_)    ,>/'_        o__
             Professional Engineer     (_)\(_)       ,>/'_
Who says engineers have to be pencil necked geeks?  (_)\(_)

You are an adult when you realize that everyone's an idiot sometimes. You
are wise when you include yourself.



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH] sata_nv: sgpio for nvidia mcp55 -v2
  2009-01-22 16:22                       ` Yan Seiner
@ 2009-01-22 17:26                         ` Yinghai Lu
  2009-01-23  1:44                           ` Yan Seiner
  0 siblings, 1 reply; 27+ messages in thread
From: Yinghai Lu @ 2009-01-22 17:26 UTC (permalink / raw)
  To: Yan Seiner; +Cc: Peer Chen, linux-ide

Yan Seiner wrote:
> On Wed, January 21, 2009 9:07 pm, Yinghai Lu wrote:
>> please try -v3 and boot with debug in command line, then post bootlog.
> 
> Dumb question:
> 
> Where does the debug parameter go?  Kernel command line?

in your /boot/grub/grub.conf
every boot entry will have append line to let you add some boot parameter. add "debug" in those line, and remove quiet etc.

YH

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH] sata_nv: sgpio for nvidia mcp55 -v2
  2009-01-22 17:26                         ` Yinghai Lu
@ 2009-01-23  1:44                           ` Yan Seiner
  2009-01-23  1:58                             ` Yinghai Lu
  0 siblings, 1 reply; 27+ messages in thread
From: Yan Seiner @ 2009-01-23  1:44 UTC (permalink / raw)
  To: Yinghai Lu; +Cc: Peer Chen, linux-ide

Yinghai Lu wrote:
> Yan Seiner wrote:
>   
>> On Wed, January 21, 2009 9:07 pm, Yinghai Lu wrote:
>>     
>>> please try -v3 and boot with debug in command line, then post bootlog.
>>>       
>> Dumb question:
>>
>> Where does the debug parameter go?  Kernel command line?
>>     
>
> in your /boot/grub/grub.conf
> every boot entry will have append line to let you add some boot parameter. add "debug" in those line, and remove quiet etc.
>
> YH
>
> !DSPAM:4978ac48230791804284693!
>
>   
To recap, I've applied the latest patch (-v3) that's supposed to make 
the SGPIO work.  I am using a LIB-700 backplane from Norco:

http://www.ipcdirect.net/servlet/Detail?no=75
http://www.norco.net.cn/UpLoadFile/Datasheet/DS-600S-EN.pdf

The disk activity LEDs stay lit all the time.

I am running a vanilla kernel, not a debian kernel,. although it was 
built using debian's kbuild.

title           Debian GNU/Linux, kernel 2.6.28.1
root            (hd0,1)
kernel          /vmlinuz-2.6.28.1 root=/dev/md0 ro debug
initrd          /initrd.img-2.6.28.1

log files are at

http://seiner.com/sata_nv/syslog
http://seiner.com/sata_nv/dmesg.log

--Yan

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH] sata_nv: sgpio for nvidia mcp55 -v2
  2009-01-23  1:44                           ` Yan Seiner
@ 2009-01-23  1:58                             ` Yinghai Lu
  2009-01-23  3:18                               ` Yan Seiner
  0 siblings, 1 reply; 27+ messages in thread
From: Yinghai Lu @ 2009-01-23  1:58 UTC (permalink / raw)
  To: Yan Seiner; +Cc: Peer Chen, linux-ide

Yan Seiner wrote:
> Yinghai Lu wrote:
>> Yan Seiner wrote:
>>  
>>> On Wed, January 21, 2009 9:07 pm, Yinghai Lu wrote:
>>>    
>>>> please try -v3 and boot with debug in command line, then post bootlog.
>>>>       
>>> Dumb question:
>>>
>>> Where does the debug parameter go?  Kernel command line?
>>>     
>>
>> in your /boot/grub/grub.conf
>> every boot entry will have append line to let you add some boot
>> parameter. add "debug" in those line, and remove quiet etc.
>>
>> YH
>>
>> !DSPAM:4978ac48230791804284693!
>>
>>   
> To recap, I've applied the latest patch (-v3) that's supposed to make
> the SGPIO work.  I am using a LIB-700 backplane from Norco:
> 
> http://www.ipcdirect.net/servlet/Detail?no=75
> http://www.norco.net.cn/UpLoadFile/Datasheet/DS-600S-EN.pdf
> 
> The disk activity LEDs stay lit all the time.
> 
> I am running a vanilla kernel, not a debian kernel,. although it was
> built using debian's kbuild.
> 
> title           Debian GNU/Linux, kernel 2.6.28.1
> root            (hd0,1)
> kernel          /vmlinuz-2.6.28.1 root=/dev/md0 ro debug
> initrd          /initrd.img-2.6.28.1
> 
> log files are at
> 
> http://seiner.com/sata_nv/syslog
> http://seiner.com/sata_nv/dmesg.log

[   19.306367] sata_nv 0000:00:05.0: PCI INT A -> Link[APSI] -> GSI 23 (level, low) -> IRQ 23
[   19.306472] sata_nv 0000:00:05.0: Using SWNCQ mode
[   19.306895] sata_nv 0000:00:05.0: setting latency timer to 64
[   19.311020] scsi5 : sata_nv


it seems that your system HW is not using SGPIO...
my setup has more print out...

YH

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH] sata_nv: sgpio for nvidia mcp55 -v2
  2009-01-23  1:58                             ` Yinghai Lu
@ 2009-01-23  3:18                               ` Yan Seiner
  2009-01-23  7:30                                 ` Yinghai Lu
  0 siblings, 1 reply; 27+ messages in thread
From: Yan Seiner @ 2009-01-23  3:18 UTC (permalink / raw)
  To: Yinghai Lu; +Cc: Peer Chen, linux-ide

Yinghai Lu wrote:
> Yan Seiner wrote:
>   
>> To recap, I've applied the latest patch (-v3) that's supposed to make
>> the SGPIO work.  I am using a LIB-700 backplane from Norco:
>>
>> http://www.ipcdirect.net/servlet/Detail?no=75
>> http://www.norco.net.cn/UpLoadFile/Datasheet/DS-600S-EN.pdf
>>
>> The disk activity LEDs stay lit all the time.
>>
>> I am running a vanilla kernel, not a debian kernel,. although it was
>> built using debian's kbuild.
>>
>> title           Debian GNU/Linux, kernel 2.6.28.1
>> root            (hd0,1)
>> kernel          /vmlinuz-2.6.28.1 root=/dev/md0 ro debug
>> initrd          /initrd.img-2.6.28.1
>>
>> log files are at
>>
>> http://seiner.com/sata_nv/syslog
>> http://seiner.com/sata_nv/dmesg.log
>>     
>
> [   19.306367] sata_nv 0000:00:05.0: PCI INT A -> Link[APSI] -> GSI 23 (level, low) -> IRQ 23
> [   19.306472] sata_nv 0000:00:05.0: Using SWNCQ mode
> [   19.306895] sata_nv 0000:00:05.0: setting latency timer to 64
> [   19.311020] scsi5 : sata_nv
>
>
> it seems that your system HW is not using SGPIO...
> my setup has more print out...
>   
selene:/home/www/seiner/html/sata_nv# modinfo sata_nv
filename:       /lib/modules/2.6.28.1/kernel/drivers/ata/sata_nv.ko
version:        3.5
license:        GPL
description:    low-level driver for NVIDIA nForce SATA controller
author:         NVIDIA
srcversion:     4D85D19ABB1359D28F97C6E
alias:          pci:v000010DEd000003F7sv*sd*bc*sc*i*
...
depends:        libata
vermagic:       2.6.28.1 SMP mod_unload modversions
parm:           adma:Enable use of ADMA (Default: true) (bool)
parm:           swncq:Enable use of SWNCQ (Default: true) (bool)

Do I have the right version?  Should I be seeing a parameter for sgpio?

--Yan


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH] sata_nv: sgpio for nvidia mcp55 -v2
  2009-01-23  3:18                               ` Yan Seiner
@ 2009-01-23  7:30                                 ` Yinghai Lu
  0 siblings, 0 replies; 27+ messages in thread
From: Yinghai Lu @ 2009-01-23  7:30 UTC (permalink / raw)
  To: Yan Seiner; +Cc: Peer Chen, linux-ide

Yan Seiner wrote:
> Yinghai Lu wrote:
>> Yan Seiner wrote:
>>  
>>> To recap, I've applied the latest patch (-v3) that's supposed to make
>>> the SGPIO work.  I am using a LIB-700 backplane from Norco:
>>>
>>> http://www.ipcdirect.net/servlet/Detail?no=75
>>> http://www.norco.net.cn/UpLoadFile/Datasheet/DS-600S-EN.pdf
>>>
>>> The disk activity LEDs stay lit all the time.
>>>
>>> I am running a vanilla kernel, not a debian kernel,. although it was
>>> built using debian's kbuild.
>>>
>>> title           Debian GNU/Linux, kernel 2.6.28.1
>>> root            (hd0,1)
>>> kernel          /vmlinuz-2.6.28.1 root=/dev/md0 ro debug
>>> initrd          /initrd.img-2.6.28.1
>>>
>>> log files are at
>>>
>>> http://seiner.com/sata_nv/syslog
>>> http://seiner.com/sata_nv/dmesg.log
>>>     
>>
>> [   19.306367] sata_nv 0000:00:05.0: PCI INT A -> Link[APSI] -> GSI 23
>> (level, low) -> IRQ 23
>> [   19.306472] sata_nv 0000:00:05.0: Using SWNCQ mode
>> [   19.306895] sata_nv 0000:00:05.0: setting latency timer to 64
>> [   19.311020] scsi5 : sata_nv
>>
>>
>> it seems that your system HW is not using SGPIO...
>> my setup has more print out...
>>   
> selene:/home/www/seiner/html/sata_nv# modinfo sata_nv
> filename:       /lib/modules/2.6.28.1/kernel/drivers/ata/sata_nv.ko
> version:        3.5
> license:        GPL
> description:    low-level driver for NVIDIA nForce SATA controller
> author:         NVIDIA
> srcversion:     4D85D19ABB1359D28F97C6E
> alias:          pci:v000010DEd000003F7sv*sd*bc*sc*i*
> ...
> depends:        libata
> vermagic:       2.6.28.1 SMP mod_unload modversions
> parm:           adma:Enable use of ADMA (Default: true) (bool)
> parm:           swncq:Enable use of SWNCQ (Default: true) (bool)
> 
> Do I have the right version?  Should I be seeing a parameter for sgpio?
> 

boot log should like

[   13.410117] calling  nv_init+0x0/0x1b @ 1
[   13.414129] sata_nv 0000:00:05.0: version 3.5
[   13.418488] sata_nv 0000:00:05.0: PCI INT A -> Link[LSA0] -> GSI 20 (level, low) -> IRQ 20
[   13.426740] sata_nv 0000:00:05.0: Using SWNCQ mode
[   13.431539] sata_nv 0000:00:05.0: using 32bit DMA mask
[   13.436671] sata_nv 0000:00:05.0: using 32bit consistent DMA mask
[   13.442771] sata_nv 0000:00:05.0: CSR 0xeb00 CB 0x9cc00
[   13.447995] sata_nv 0000:00:05.0: using sgpio 1
[   13.452522] sata_nv 0000:00:05.0: setting latency timer to 64
[   13.458389] scsi0 : sata_nv
[   13.461428] scsi1 : sata_nv
[   13.464526] ata1: SATA max UDMA/133 cmd 0xa480 ctl 0xa400 bmdma 0x9c00 irq 20
[   13.471651] ata2: SATA max UDMA/133 cmd 0xa080 ctl 0xa000 bmdma 0x9c08 irq 20

is will say "using sgpio 1"...

YH


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH] sata_nv: sgpio for nvidia mcp55 -v3
  2009-01-22  2:32               ` [PATCH] sata_nv: sgpio for nvidia mcp55 -v3 Yinghai Lu
@ 2009-01-27  4:57                 ` Andrew Morton
  2009-04-13  8:18                 ` Jeff Garzik
  1 sibling, 0 replies; 27+ messages in thread
From: Andrew Morton @ 2009-01-27  4:57 UTC (permalink / raw)
  To: Yinghai Lu
  Cc: Peer Chen, jeff, Kuan Luo, Christoph Hellwig, linux-kernel,
	linux-ide, Ingo Molnar

On Wed, 21 Jan 2009 18:32:15 -0800 Yinghai Lu <yinghai@kernel.org> wrote:

> 
> Impact: new features
> 
> based on patch on
>     http://marc.info/?l=linux-ide&m=116289338705418&w=2
> 
> 1. update the patch for 2.6.19 to latest upstream ( 2.6.28?)
> 2. fix shared sgpio to support several mcp55 + io55, so every mcp55 have
>    seperate spinlock
> 3. use scratch_source as numbering of sgpio instead of address of struct,
>    so could go through kexec/kdump
> 
> v2: revert NV_ON and NV_OFF, so turn on Activity LED all the time when disk is idle.
> v3: use one timer per sgpio instead of per nv_host, so for mcp55+io55 system will use 2 timers
>     instead of 6 timers.

None of this is really usable as a changelog.  The original text from
the 2006 patch was better.

> Signed-off-by: Yinghai Lu <yinghai@kernel.org>

Signoffs from Kuan Luo and Peer Chen would be appropriate here.

Please include a full changelog with the next version incorporating the
above comments.  Information about how this version of the patch has
changed from the previous version _is_ useful for reviewers, but is not
appropriate for the permanent changelog.  Hence many people put this
material below the ^--- line in the changelog.

> --- linux-2.6.orig/drivers/ata/sata_nv.c
> +++ linux-2.6/drivers/ata/sata_nv.c
> @@ -200,6 +200,164 @@ enum {
>  
>  };
>  
> +/* sgpio
> + * Sgpio defines
> + * SGPIO state defines
> + */
> +#define NV_SGPIO_STATE_RESET		0
> +#define NV_SGPIO_STATE_OPERATIONAL	1
> +#define NV_SGPIO_STATE_ERROR		2
> +
> +/* SGPIO command opcodes */
> +#define NV_SGPIO_CMD_RESET		0
> +#define NV_SGPIO_CMD_READ_PARAMS	1
> +#define NV_SGPIO_CMD_READ_DATA		2
> +#define NV_SGPIO_CMD_WRITE_DATA		3
> +
> +/* SGPIO command status defines */
> +#define NV_SGPIO_CMD_OK			0
> +#define NV_SGPIO_CMD_ACTIVE		1
> +#define NV_SGPIO_CMD_ERR		2
> +
> +#define NV_SGPIO_UPDATE_TICK		90
> +#define NV_CNTRLR_SHARE_INIT		2
> +#define NV_SGPIO_MAX_ACTIVITY_ON	10
> +#define NV_SGPIO_MIN_FORCE_OFF		5
> +#define NV_SGPIO_PCI_CSR_OFFSET		0x58
> +#define NV_SGPIO_PCI_CB_OFFSET		0x5C
> +#define NV_SGPIO_DFLT_CB_SIZE		256
> +#define NV_ON				0
> +#define NV_OFF				1
> +
> +static inline u8 bf_extract(u8 value, u8 offset, u8 bit_count)
> +{
> +	return (((u8)(value)) >> (offset)) & ((1 << (bit_count)) - 1);
> +}
> +
> +static inline u8 bf_ins(u8 value, u8 ins, u8 offset, u8 bit_count)
> +{
> +	return	((value) & ~((((1 << (bit_count)) - 1)) << (offset))) |
> +						(((u8)(ins)) << (offset));
> +}
> +
> +static inline u32 bf_extract_u32(u32 value, u8 offset, u8 bit_count)
> +{
> +	return (((u32)(value)) >> (offset)) & ((1 << (bit_count)) - 1);
> +}
> +static inline u32 bf_ins_u32(u32 value, u32 ins, u8 offset, u8 bit_count)
> +{
> +	return	((value) & ~((((1 << (bit_count)) - 1)) << (offset))) |
> +						(((u32)(ins)) << (offset));
> +}

These helpers are generic and I would suggest that it would be better
to propose generic kernel-wide implementations and merge that
separately.

A suitable starting point might be drivers/net/3c59x.c's BFEXT() and
BFINS() macros.  With a bit of thought and testing, those macros could
be made to work with any size of input args, so we would then not need
separate u8, u16, u32 and u64 implementations.

> +#define GET_SGPIO_STATUS(v)	bf_extract(v, 0, 2)
> +#define GET_CMD_STATUS(v)	bf_extract(v, 3, 2)
> +#define GET_CMD(v)		bf_extract(v, 5, 3)
> +#define SET_CMD(v, cmd)	bf_ins(v, cmd, 5, 3)
> +
> +#define GET_ENABLE(v)		bf_extract(v, 23, 1)
> +#define SET_ENABLE(v)		bf_ins_u32(v, 1, 23, 1)
> +
> +/* Needs to have a u8 bit-field insert. */
> +#define GET_ACTIVITY(v)		bf_extract(v, 5, 3)
> +#define SET_ACTIVITY(v, on_off)	bf_ins(v, on_off, 5, 3)
> +
> +union nv_sgpio_nvcr {
> +	struct {
> +		u8	init_cnt;
> +		u8	cb_size;
> +		u8	cbver;
> +		u8	rsvd;
> +	} bit;
> +	u32	all;
> +};

What are the endianness considerations here?

> +union nv_sgpio_tx {
> +	u8	tx_port[4];
> +	u32	all;
> +};
> +
> +struct nv_sgpio_cb {
> +	u64			scratch_space;
> +	union nv_sgpio_nvcr	nvcr;
> +	u32			cr0;
> +	u32                     rsvd[4];
> +	union nv_sgpio_tx       tx[2];
> +};
> +
> +#define NR_SGPIO_SHARE_HOST 4
> +struct nv_sgpio_host_share {
> +	spinlock_t		lock;
> +	struct timer_list	timer;
> +	unsigned int		need_update;
> +	struct ata_host		*host[NR_SGPIO_SHARE_HOST];
> +};
> +
> +struct nv_sgpio_host_flags {
> +	u8	sgpio_enabled:1;
> +	u8	need_update:1;
> +	u8	rsvd:6;
> +};

The individual fields share a byte.  Is locking needed to prevent races
when updating them?  If not, why not?  If so, what is that locking
protocol?  A comment should be added here describing this.

> +struct nv_host_sgpio {
> +	struct nv_sgpio_host_flags	flags;
> +	u8				*pcsr;
> +	struct nv_sgpio_cb		*pcb;
> +	unsigned  int			share_index;
> +};
> +
> +struct nv_sgpio_port_flags {
> +	u8	last_state:1;
> +	u8	recent_activity:1;
> +	u8	rsvd:6;
> +};
> +
> +struct nv_sgpio_led {
> +	struct nv_sgpio_port_flags	flags;
> +	u8				force_off;
> +	u8				last_cons_active;
> +};
> +
> +struct nv_port_sgpio {
> +	struct nv_sgpio_led	activity;
> +};
> +
> +/* 1 mcp55 and 3 io55, 0 is not used */
> +#define NR_SGPIO 5
> +static struct nv_sgpio_host_share nv_sgpio_share[NR_SGPIO];
> +
> +static inline u8 nv_sgpio_get_func(struct ata_host *host)
> +{
> +	u8 devfn = (to_pci_dev(host->dev))->devfn;
> +
> +	return PCI_FUNC(devfn);
> +}
> +
> +static inline u8 nv_sgpio_tx_host_offset(struct ata_host *host)
> +{
> +	return nv_sgpio_get_func(host)/NV_CNTRLR_SHARE_INIT;
> +}
> +
> +static inline u8 nv_sgpio_calc_tx_offset(u8 cntrlr, u8 channel)
> +{
> +	return sizeof(union nv_sgpio_tx) - (NV_CNTRLR_SHARE_INIT *
> +		(cntrlr % NV_CNTRLR_SHARE_INIT)) - channel - 1;
> +}
> +
> +static inline u8 nv_sgpio_tx_port_offset(struct ata_port *ap)
> +{
> +	u8 cntrlr = nv_sgpio_get_func(ap->host);
> +	return nv_sgpio_calc_tx_offset(cntrlr, ap->port_no);
> +}
> +
> +static inline u8 nv_sgpio_capable(const struct pci_device_id *ent)
> +{
> +	if (ent->device == PCI_DEVICE_ID_NVIDIA_NFORCE_MCP55_SATA2)
> +		return 1;
> +	else
> +		return 0;
> +}
> +
>  /* ADMA Physical Region Descriptor - one SG segment */
>  struct nv_adma_prd {
>  	__le64			addr;
> @@ -240,6 +398,7 @@ struct nv_adma_cpb {
>  
>  
>  struct nv_adma_port_priv {
> +	struct nv_port_sgpio	port_sgpio;
>  	struct nv_adma_cpb	*cpb;
>  	dma_addr_t		cpb_dma;
>  	struct nv_adma_prd	*aprd;
> @@ -254,6 +413,12 @@ struct nv_adma_port_priv {
>  
>  struct nv_host_priv {
>  	unsigned long		type;
> +	unsigned long		host_flags;
> +	struct nv_host_sgpio	host_sgpio;
> +};
> +
> +struct nv_port_priv {
> +	struct nv_port_sgpio	port_sgpio;
>  };
>  
>  struct defer_queue {
> @@ -271,6 +436,8 @@ enum ncq_saw_flag_list {
>  };
>  
>  struct nv_swncq_port_priv {
> +	struct nv_port_sgpio	port_sgpio;
> +
>  	struct ata_prd	*prd;	 /* our SG list */
>  	dma_addr_t	prd_dma; /* and its DMA mapping */
>  	void __iomem	*sactive_block;
> @@ -311,6 +478,10 @@ static int nv_nf2_hardreset(struct ata_l
>  			    unsigned long deadline);
>  static void nv_ck804_freeze(struct ata_port *ap);
>  static void nv_ck804_thaw(struct ata_port *ap);
> +static int nv_port_start(struct ata_port *ap);
> +static void nv_port_stop(struct ata_port *ap);
> +static void nv_host_stop(struct ata_host *host);
> +static unsigned int nv_qc_issue(struct ata_queued_cmd *qc);
>  static int nv_adma_slave_config(struct scsi_device *sdev);
>  static int nv_adma_check_atapi_dma(struct ata_queued_cmd *qc);
>  static void nv_adma_qc_prep(struct ata_queued_cmd *qc);
> @@ -374,6 +545,17 @@ static const struct pci_device_id nv_pci
>  	{ } /* terminate list */
>  };
>  
> +/* SGPIO function prototypes */
> +static void nv_sgpio_init(struct pci_dev *pdev, struct ata_host *host);
> +static void nv_sgpio_reset(u8 *pcsr);
> +static void nv_sgpio_set_timer(struct timer_list *ptimer,
> +				unsigned int timeout_msec);
> +static void nv_sgpio_timer_handler(unsigned long ptr);
> +static void nv_sgpio_host_cleanup(struct nv_host_priv *host);
> +static u8 nv_sgpio_update_led(struct nv_sgpio_led *led, u8 *on_off);
> +static void nv_sgpio_clear_all_leds(struct ata_port *ap);
> +static u8 nv_sgpio_send_cmd(struct nv_sgpio_host_share *sgpio_share, u8 cmd);
> +
>  static struct pci_driver nv_pci_driver = {
>  	.name			= DRV_NAME,
>  	.id_table		= nv_pci_tbl,
> @@ -409,6 +591,10 @@ static struct ata_port_operations nv_com
>  	.inherits		= &ata_bmdma_port_ops,
>  	.scr_read		= nv_scr_read,
>  	.scr_write		= nv_scr_write,
> +	.qc_issue               = nv_qc_issue,
> +	.port_start		= nv_port_start,
> +	.port_stop		= nv_port_stop,
> +	.host_stop		= nv_host_stop,
>  };
>  
>  /* OSDL bz11195 reports that link doesn't come online after hardreset
> @@ -1395,6 +1581,8 @@ static unsigned int nv_adma_qc_issue(str
>  	void __iomem *mmio = pp->ctl_block;
>  	int curr_ncq = (qc->tf.protocol == ATA_PROT_NCQ);
>  
> +	pp->port_sgpio.activity.flags.recent_activity = NV_ON;
> +
>  	VPRINTK("ENTER\n");
>  
>  	/* We can't handle result taskfile with NCQ commands, since
> @@ -1673,6 +1861,34 @@ static void nv_adma_error_handler(struct
>  	ata_sff_error_handler(ap);
>  }
>  
> +static int nv_port_start(struct ata_port *ap)
> +{
> +	struct device *dev = ap->host->dev;
> +	struct nv_port_priv *pp;
> +	int rc;
> +
> +	rc = ata_sff_port_start(ap);
> +	if (rc)
> +		return rc;
> +
> +	pp = devm_kzalloc(dev, sizeof(*pp), GFP_KERNEL);
> +	if (!pp)
> +		return -ENOMEM;

Did we just leak the resources allocated by ata_sff_port_start()?

> +	ap->private_data = pp;
> +
> +	return 0;
> +}
> +
> +static unsigned int nv_qc_issue(struct ata_queued_cmd *qc)
> +{
> +       struct nv_port_priv *port = qc->ap->private_data;
> +
> +       port->port_sgpio.activity.flags.recent_activity = NV_ON;
> +
> +       return ata_sff_qc_issue(qc);
> +}
> +
>  static void nv_swncq_qc_to_dq(struct ata_port *ap, struct ata_queued_cmd *qc)
>  {
>  	struct nv_swncq_port_priv *pp = ap->private_data;
> @@ -2017,6 +2233,8 @@ static unsigned int nv_swncq_qc_issue(st
>  	struct ata_port *ap = qc->ap;
>  	struct nv_swncq_port_priv *pp = ap->private_data;
>  
> +	pp->port_sgpio.activity.flags.recent_activity = NV_ON;
> +
>  	if (qc->tf.protocol != ATA_PROT_NCQ)
>  		return ata_sff_qc_issue(qc);
>  
> @@ -2404,6 +2622,9 @@ static int nv_init_one(struct pci_dev *p
>  	} else if (type == SWNCQ)
>  		nv_swncq_host_init(host);
>  
> +	if (nv_sgpio_capable(ent))
> +		nv_sgpio_init(pdev, host);
> +
>  	pci_set_master(pdev);
>  	return ata_host_activate(host, pdev->irq, ipriv->irq_handler,
>  				 IRQF_SHARED, ipriv->sht);
> @@ -2459,11 +2680,19 @@ static int nv_pci_device_resume(struct p
>  }
>  #endif
>  
> +static void nv_port_stop(struct ata_port *ap)
> +{
> +	nv_sgpio_clear_all_leds(ap);
> +}

I'd have expected a function called nv_port_stop() to release all the
resources which were allocated by a function called nv_port_start().

I guess the dmam_*() crap^Whelpers do that.

>  static void nv_ck804_host_stop(struct ata_host *host)
>  {
>  	struct pci_dev *pdev = to_pci_dev(host->dev);
> +	struct nv_host_priv *phost = host->private_data;
>  	u8 regval;
>  
> +	nv_sgpio_host_cleanup(phost);
> +
>  	/* disable SATA space for CK804 */
>  	pci_read_config_byte(pdev, NV_MCP_SATA_CFG_20, &regval);
>  	regval &= ~NV_MCP_SATA_CFG_20_SATA_SPACE_EN;
> @@ -2487,6 +2716,339 @@ static void nv_adma_host_stop(struct ata
>  	nv_ck804_host_stop(host);
>  }
>  
> +static void nv_host_stop(struct ata_host *host)
> +{
> +	struct nv_host_priv *phost = host->private_data;
> +
> +	nv_sgpio_host_cleanup(phost);
> +}
> +
> +static void nv_sgpio_init(struct pci_dev *pdev, struct ata_host *host)
> +{
> +	u16 csr_add;
> +	u32 cb_add, temp32;
> +	struct nv_host_priv *phost = host->private_data;
> +	struct nv_host_sgpio *host_sgpio;
> +	struct nv_sgpio_host_share *sgpio_share;
> +	struct nv_sgpio_cb *pcb;
> +	u8 pro = 0;
> +	unsigned int share_index;
> +	int i;
> +
> +	pci_read_config_word(pdev, NV_SGPIO_PCI_CSR_OFFSET, &csr_add);
> +	pci_read_config_dword(pdev, NV_SGPIO_PCI_CB_OFFSET, &cb_add);
> +

> +		return;
> +
> +	if (cb_add <= 0x80000 || cb_add >= 0x9FC00)
> +		return;

The magic numbers at least need an explanatory comment?

> +	pci_read_config_byte(pdev, 0xA4, &pro);
> +	if (!(pro & 0x40))
> +		return;
> +
> +	temp32 = csr_add;
> +	if (temp32 <= 0x200 || temp32 >= 0xFFFE)

??

> +		return;
> +
> +	dev_printk(KERN_DEBUG, &pdev->dev, "CSR 0x%x CB 0x%x\n",
> +			 csr_add, cb_add);
> +
> +	host_sgpio = &phost->host_sgpio;
> +	pcb = ioremap(cb_add, 256);
> +
> +	if (pcb->nvcr.bit.init_cnt != 0x2 || pcb->nvcr.bit.cbver != 0x0)
> +		return;
> +
> +	share_index = pcb->scratch_space;
> +	if (share_index == 0 || share_index > (NR_SGPIO - 1)) {
> +		/* find one good position */
> +		for (i = 1; i < NR_SGPIO; i++) {
> +			if (!nv_sgpio_share[i].host[0]) {
> +				share_index = i;
> +				break;
> +			}
> +		}
> +		if (share_index == 0 || share_index > (NR_SGPIO - 1)) {
> +			printk(KERN_WARNING "NR_SGPIO %d is too small\n",
> +				 NR_SGPIO);

dev_warn()?

It's good to make the printks identify which driver (and sometimes
device) they are referring to.

> +			return;
> +		}
> +	}
> +
> +	host_sgpio->pcsr = (void *)(unsigned long)temp32;
> +	host_sgpio->pcb = pcb;
> +	sgpio_share = &nv_sgpio_share[share_index];
> +	if (!sgpio_share->host[0]) {
> +		/* also handle kexec path:
> +		 * scratch_space is set, but not get nv_sgpio_share yet
> +		 */

hm.  What has this to do with kexec?

> +		spin_lock_init(&sgpio_share->lock);
> +		spin_lock(&sgpio_share->lock);
> +		sgpio_share->host[0] = host;
> +		host_sgpio->share_index = share_index;
> +		pcb->scratch_space = share_index;
> +		nv_sgpio_reset(host_sgpio->pcsr);
> +		pcb->cr0 = SET_ENABLE(pcb->cr0);
> +		init_timer(&sgpio_share->timer);
> +		sgpio_share->timer.data = (unsigned long)sgpio_share;
> +		nv_sgpio_set_timer(&sgpio_share->timer, NV_SGPIO_UPDATE_TICK);
> +		spin_unlock(&sgpio_share->lock);
> +	} else {
> +		spin_lock(&sgpio_share->lock);
> +		for (i = 1; i < NR_SGPIO_SHARE_HOST; i++) {
> +			if (!sgpio_share->host[i]) {
> +				sgpio_share->host[i] = host;
> +				host_sgpio->share_index = share_index;
> +				break;
> +			}
> +		}
> +		if (i == NR_SGPIO_SHARE_HOST)
> +			printk(KERN_WARNING "NR_SGPIO_SHARE_HOST %d is too small\n",
> +					 NR_SGPIO_SHARE_HOST);
> +		spin_unlock(&sgpio_share->lock);
> +	}

The handling of sgpio_share->timer is confusing.

We can return from this function without having run init_timer() at
all.  Why is this not buggy?

Looking at the (obscure, undercommented) tricks which nv_sgpio_init()
is doing I can kinda sorta see how it works, but I think it should be
double-checked and simplified and clarified if poss.

> +	dev_printk(KERN_DEBUG, &pdev->dev, "using sgpio %d\n", share_index);
> +	host_sgpio->flags.sgpio_enabled = 1;
> +
> +}
> +
> +static void nv_sgpio_set_timer(struct timer_list *ptimer,
> +			       unsigned int timeout_msec)
> +{
> +	if (!ptimer)
> +		return;
> +	ptimer->function = nv_sgpio_timer_handler;
> +	ptimer->expires = msecs_to_jiffies(timeout_msec) + jiffies;
> +	add_timer(ptimer);
> +}

It would be better (I think) if this function were to also be passed
the timer.data argument, and if it were to also run init_timer().  Then
it could use the setup_timer() helper also.

> +static void nv_sgpio_timer_handler(unsigned long context)
> +{
> +
> +	struct nv_sgpio_host_share *sgpio_share = (struct nv_sgpio_host_share *)context;
> +	struct ata_host *host;
> +	struct nv_host_priv *phost;
> +	u8 count, host_offset, port_offset;
> +	union nv_sgpio_tx tx;
> +	u8 on_off;
> +	unsigned long mask;
> +	struct nv_port_priv *port;
> +	struct nv_host_sgpio *host_sgpio;
> +	struct nv_sgpio_cb *pcb;
> +	int i;
> +
> +	for (i = 0; i < NR_SGPIO_SHARE_HOST; i++) {
> +		host = sgpio_share->host[i];
> +		/* not blank slot */
> +		if (!host)
> +			break;
> +
> +		phost = (struct nv_host_priv *)host->private_data;

Unneeded, undesirable cast of void*.  Please check the whole patch for this.

> +		host_sgpio = &phost->host_sgpio;
> +		if (!host_sgpio->flags.sgpio_enabled)
> +			continue;
> +
> +		mask = 0xFFFF;
> +		host_offset = nv_sgpio_tx_host_offset(host);
> +		pcb = host_sgpio->pcb;
> +
> +		spin_lock(&sgpio_share->lock);
> +		tx = pcb->tx[host_offset];
> +		spin_unlock(&sgpio_share->lock);

This is interrupt-context code and it takes a spinlock.  But other,
non-interrupt-context code takes the same lock with a bare spin_lock(),
not with spin_lock_bh() or spin_lock_irq() or whatever.

Why is this not deadlocky?  Did it pass lockdep testing?

> +		for (count = 0; count < host->n_ports; count++) {
> +			struct ata_port *ap;
> +
> +			ap = host->ports[count];
> +
> +			if (!(ap && !(ap->flags & ATA_FLAG_DISABLED)))

That expression hurts my brain.  It is equivalent to

			if (!ap || (ap->flags & ATA_FLAG_DISABLED))

which I think is a heck of a lot clearer?

> +				continue;
> +
> +			port = (struct nv_port_priv *)ap->private_data;

unneeded cast?

> +			if (!port)
> +				continue;
> +			port_offset = nv_sgpio_tx_port_offset(ap);
> +			on_off = GET_ACTIVITY(tx.tx_port[port_offset]);
> +			if (nv_sgpio_update_led(&port->port_sgpio.activity, &on_off)) {
> +				tx.tx_port[port_offset] =
> +					SET_ACTIVITY(tx.tx_port[port_offset], on_off);
> +				host_sgpio->flags.need_update = 1;
> +		       }
> +		}
> +
> +		if (host_sgpio->flags.need_update) {
> +			sgpio_share->need_update = 1;
> +			spin_lock(&sgpio_share->lock);
> +			if (nv_sgpio_get_func(host)
> +				% NV_CNTRLR_SHARE_INIT == 0) {
> +				pcb->tx[host_offset].all &= mask;
> +				mask = mask << 16;
> +				tx.all &= mask;
> +			} else {
> +				tx.all &= mask;
> +				mask = mask << 16;
> +				pcb->tx[host_offset].all &= mask;
> +			}
> +			pcb->tx[host_offset].all |= tx.all;
> +			spin_unlock(&sgpio_share->lock);
> +		}
> +	}
> +
> +	if (sgpio_share->need_update) {
> +		if (nv_sgpio_send_cmd(sgpio_share, NV_SGPIO_CMD_WRITE_DATA)) {
> +			sgpio_share->need_update = 0;
> +			for (i = 0; i < NR_SGPIO_SHARE_HOST; i++) {
> +				host = sgpio_share->host[i];
> +				/* not blank slot */
> +				if (!host)
> +					break;
> +
> +				phost = (struct nv_host_priv *)host->private_data;

cast?

> +
> +				host_sgpio = &phost->host_sgpio;
> +				if (!host_sgpio->flags.sgpio_enabled)
> +					continue;
> +
> +				host_sgpio->flags.need_update = 0;
> +			}
> +		}
> +	}
> +
> +	nv_sgpio_set_timer(&sgpio_share->timer, NV_SGPIO_UPDATE_TICK);
> +	return;
> +}
> +
> +static u8 nv_sgpio_send_cmd(struct nv_sgpio_host_share *sgpio_share, u8 cmd)
> +{
> +	u8 csr;
> +	struct nv_host_sgpio *host_sgpio;
> +	struct nv_host_priv *phost;
> +
> +	phost = (struct nv_host_priv *)sgpio_share->host[0]->private_data;

etc..

> +	host_sgpio = &phost->host_sgpio;
> +	spin_lock(&sgpio_share->lock);
> +	csr = inb((unsigned long)host_sgpio->pcsr);
> +	if ((GET_SGPIO_STATUS(csr) != NV_SGPIO_STATE_OPERATIONAL) ||
> +	    (GET_CMD_STATUS(csr) == NV_SGPIO_CMD_ACTIVE)) {
> +		;
> +	} else {
> +		host_sgpio->pcb->cr0 =
> +			SET_ENABLE(host_sgpio->pcb->cr0);
> +		csr = 0;
> +		csr = SET_CMD(csr, cmd);
> +		outb(csr, (unsigned long)host_sgpio->pcsr);
> +	}
> +	spin_unlock(&sgpio_share->lock);
> +	return 1;
> +}
> +
> +static u8 nv_sgpio_update_led(struct nv_sgpio_led *led, u8 *on_off)
> +{
> +	u8 need_update = 0;
> +
> +	if (led->force_off > 0) {
> +		led->force_off--;
> +	} else if (led->flags.recent_activity ^ led->flags.last_state) {
> +		*on_off = led->flags.recent_activity;
> +		led->flags.last_state = led->flags.recent_activity;
> +		need_update = 1;
> +	} else if ((led->flags.recent_activity == NV_ON) &&
> +		   (led->flags.last_state == NV_ON) &&
> +		   (led->last_cons_active >= NV_SGPIO_MAX_ACTIVITY_ON)) {
> +		*on_off = NV_OFF;
> +		led->flags.last_state = NV_OFF;
> +		led->force_off = NV_SGPIO_MIN_FORCE_OFF;
> +		need_update = 1;
> +	}
> +
> +	if (*on_off == NV_ON)
> +		led->last_cons_active++;
> +	else
> +		led->last_cons_active = 0;
> +
> +	led->flags.recent_activity = NV_OFF;
> +	return need_update;
> +}
> +
> +static void nv_sgpio_reset(u8  *pcsr)
> +{
> +	u8 csr;
> +
> +	csr = inb((unsigned long)pcsr);
> +	if (GET_SGPIO_STATUS(csr) == NV_SGPIO_STATE_RESET) {
> +		csr = 0;
> +		csr = SET_CMD(csr, NV_SGPIO_CMD_RESET);
> +		outb(csr, (unsigned long)pcsr);
> +	}
> +	csr = 0;
> +	csr = SET_CMD(csr, NV_SGPIO_CMD_READ_PARAMS);
> +	outb(csr, (unsigned long)pcsr);
> +}
> +
> +static void nv_sgpio_host_cleanup(struct nv_host_priv *host)
> +{
> +	u8 csr;
> +	struct nv_host_sgpio *host_sgpio;
> +	struct nv_sgpio_host_share *sgpio_share;
> +
> +	if (!host)
> +		return;

Can this happen?

> +	host_sgpio = &host->host_sgpio;
> +	if (!host_sgpio->share_index)
> +		return;
> +
> +	sgpio_share = &nv_sgpio_share[host_sgpio->share_index];
> +	if (host_sgpio->flags.sgpio_enabled) {
> +		spin_lock(&sgpio_share->lock);
> +		host_sgpio->pcb->cr0 = SET_ENABLE(host_sgpio->pcb->cr0);
> +		csr = 0;
> +		csr = SET_CMD(csr, NV_SGPIO_CMD_WRITE_DATA);
> +		outb(csr, (unsigned long)host_sgpio->pcsr);
> +		spin_unlock(&sgpio_share->lock);
> +
> +		if (timer_pending(&sgpio_share->timer))
> +			del_timer(&sgpio_share->timer);

The timer could actually be executing on another CPU right now, in
which case I think we crash?

An unconditional del_timer_sync() might be safer.  Please check.

> +		host_sgpio->flags.sgpio_enabled = 0;
> +		host_sgpio->pcb->scratch_space = 0;
> +	}
> +}
> +
> +static void nv_sgpio_clear_all_leds(struct ata_port *ap)
> +{
> +	struct nv_port_priv *port = ap->private_data;
> +	struct nv_host_priv *host;
> +	u8 host_offset, port_offset;
> +	struct nv_host_sgpio *host_sgpio;
> +	struct nv_sgpio_host_share *sgpio_share;
> +
> +	if (!port || !ap->host)
> +		return;
> +	if (!ap->host->private_data)
> +		return;

Can all of these happen?

> +	host = ap->host->private_data;

missed a typecast :)

> +	host_sgpio = &host->host_sgpio;
> +	if (!host_sgpio->share_index)
> +		return;
> +
> +	sgpio_share = &nv_sgpio_share[host_sgpio->share_index];
> +	if (!host_sgpio->flags.sgpio_enabled)
> +		return;
> +
> +	host_offset = nv_sgpio_tx_host_offset(ap->host);
> +	port_offset = nv_sgpio_tx_port_offset(ap);
> +
> +	spin_lock(&sgpio_share->lock);
> +	host_sgpio->pcb->tx[host_offset].tx_port[port_offset] = 0;
> +	host_sgpio->flags.need_update = 1;
> +	spin_unlock(&sgpio_share->lock);
> +}
> +
>  static int __init nv_init(void)
>  {
>  	return pci_register_driver(&nv_pci_driver);

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH] sata_nv: sgpio for nvidia mcp55 -v3
  2009-01-22  2:32               ` [PATCH] sata_nv: sgpio for nvidia mcp55 -v3 Yinghai Lu
  2009-01-27  4:57                 ` Andrew Morton
@ 2009-04-13  8:18                 ` Jeff Garzik
  2009-04-13 15:50                   ` Yinghai Lu
  1 sibling, 1 reply; 27+ messages in thread
From: Jeff Garzik @ 2009-04-13  8:18 UTC (permalink / raw)
  To: Yinghai Lu
  Cc: Peer Chen, Kuan Luo, Christoph Hellwig, linux-kernel, linux-ide,
	Ingo Molnar, Andrew Morton

Yinghai Lu wrote:
> Impact: new features
> 
> based on patch on
>     http://marc.info/?l=linux-ide&m=116289338705418&w=2
> 
> 1. update the patch for 2.6.19 to latest upstream ( 2.6.28?)
> 2. fix shared sgpio to support several mcp55 + io55, so every mcp55 have
>    seperate spinlock
> 3. use scratch_source as numbering of sgpio instead of address of struct,
>    so could go through kexec/kdump
> 
> v2: revert NV_ON and NV_OFF, so turn on Activity LED all the time when disk is idle.
> v3: use one timer per sgpio instead of per nv_host, so for mcp55+io55 system will use 2 timers
>     instead of 6 timers.
> 
> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
> 
> ---
>  drivers/ata/sata_nv.c |  562 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 562 insertions(+)


Is this really over 500 LOC, just to blink some lights from the data 
xfer hot path?  :)

	Jeff




^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH] sata_nv: sgpio for nvidia mcp55 -v3
  2009-04-13  8:18                 ` Jeff Garzik
@ 2009-04-13 15:50                   ` Yinghai Lu
  0 siblings, 0 replies; 27+ messages in thread
From: Yinghai Lu @ 2009-04-13 15:50 UTC (permalink / raw)
  To: Jeff Garzik
  Cc: Peer Chen, Kuan Luo, Christoph Hellwig, linux-kernel, linux-ide,
	Ingo Molnar, Andrew Morton

On Mon, Apr 13, 2009 at 1:18 AM, Jeff Garzik <jeff@garzik.org> wrote:
> Yinghai Lu wrote:
>>
>> Impact: new features
>>
>> based on patch on
>>    http://marc.info/?l=linux-ide&m=116289338705418&w=2
>>
>> 1. update the patch for 2.6.19 to latest upstream ( 2.6.28?)
>> 2. fix shared sgpio to support several mcp55 + io55, so every mcp55 have
>>   seperate spinlock
>> 3. use scratch_source as numbering of sgpio instead of address of struct,
>>   so could go through kexec/kdump
>>
>> v2: revert NV_ON and NV_OFF, so turn on Activity LED all the time when
>> disk is idle.
>> v3: use one timer per sgpio instead of per nv_host, so for mcp55+io55
>> system will use 2 timers
>>    instead of 6 timers.
>>
>> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
>>
>> ---
>>  drivers/ata/sata_nv.c |  562
>> ++++++++++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 562 insertions(+)
>
>
> Is this really over 500 LOC, just to blink some lights from the data xfer
> hot path?  :)
>
there is timer to blink the activity led. per sgpio, normally one for
mcp55 and one for io55.
data xfer hot path only set some status.

YH

^ permalink raw reply	[flat|nested] 27+ messages in thread

end of thread, other threads:[~2009-04-13 15:50 UTC | newest]

Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <15F501D1A78BD343BE8F4D8DB854566B059FE0B3@hkemmail01.nvidia.com>
2006-10-31  8:43 ` [PATCH] SCSI: Add the SGPIO support for sata_nv.c Peer Chen
2006-10-31  8:43   ` Peer Chen
2006-10-31 10:40   ` Christoph Hellwig
2006-10-31 12:37     ` Prakash Punnoor
2006-11-07  9:55     ` Peer Chen
2006-11-07  9:55       ` Peer Chen
2007-11-07  4:09       ` Yinghai Lu
2009-01-18  9:20         ` Yinghai Lu
2009-01-18  9:22           ` [PATCH] sata_nv: sgpio for nvidia mcp55 Yinghai Lu
2009-01-21  2:07             ` [PATCH] sata_nv: sgpio for nvidia mcp55 -v2 Yinghai Lu
2009-01-21 14:30               ` Yan Seiner
2009-01-21 18:48                 ` Yinghai Lu
2009-01-22  4:46                   ` Yan Seiner
2009-01-22  5:07                     ` Yinghai Lu
2009-01-22 16:22                       ` Yan Seiner
2009-01-22 17:26                         ` Yinghai Lu
2009-01-23  1:44                           ` Yan Seiner
2009-01-23  1:58                             ` Yinghai Lu
2009-01-23  3:18                               ` Yan Seiner
2009-01-23  7:30                                 ` Yinghai Lu
2009-01-22  2:32               ` [PATCH] sata_nv: sgpio for nvidia mcp55 -v3 Yinghai Lu
2009-01-27  4:57                 ` Andrew Morton
2009-04-13  8:18                 ` Jeff Garzik
2009-04-13 15:50                   ` Yinghai Lu
2009-01-18 13:49           ` [PATCH] SCSI: Add the SGPIO support for sata_nv.c Yan Seiner
2009-01-20  7:36             ` Peer Chen
2006-11-01  1:39   ` Jeff Garzik

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.