Re: [PATCH] SCSI: Add the SGPIO support for sata_nv.c

2007-11-06 Thread Yinghai Lu
On Nov 7, 2006 1:55 AM, Peer Chen <[EMAIL PROTECTED]> 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
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] SCSI: Add the SGPIO support for sata_nv.c

2007-11-06 Thread Yinghai Lu
On Nov 7, 2006 1:55 AM, Peer Chen [EMAIL PROTECTED] 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
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] SCSI: Add the SGPIO support for sata_nv.c

2007-02-01 Thread Kristen Carlson Accardi
On Thu, 1 Feb 2007 11:55:35 -0800
"Andy Currid" <[EMAIL PROTECTED]> wrote:

> 
> I'm not sure I understand your point. To support SGPIO initiators
> incorporated
> into SATA host controllers, it is pretty much inevitable that some SGPIO
> support
> will have to be added directly into individual drivers for thoese
> controllers;

If SGPIO is defined as a generic protocol, could there be bits that are
used universally by all controllers that support a SGPIO interface, and
then of course chipset specific bits as far as how this is used?  For
example, AHCI 1.1 specifies that Enclosure Management via SGPIO is 
supported, and uses the message interface defined in SFF-8485.  If there
are any parts to your implementation that are remotely standard, it would
be preferrable in my opinion to split them out into a separate file
so that you have vendor specific code in the driver, and generic sgpio
defines in a separate file that can be shared somehow.

Probably I should have more appropriately asked whether there were any
bits in this that could be made common :).

Kristen

> the SGPIO standard does not define a register level interface for
> accessing
> this functionality, so vendor implementations vary.
> 
> It's possible that with an SGPIO initiator integrated within a SAS host
> controller,
> you may be able to use SMP frames to access it, using the messaging
> defined in
> SFF-8485 chapter 8 - but I wouldn't bank on it.
> 
> Andy
> 
> -Original Message-
> From: [EMAIL PROTECTED]
> [mailto:[EMAIL PROTECTED] On Behalf Of Kristen Carlson
> Accardi
> Sent: Wednesday, January 31, 2007 15:22
> To: Jeff Garzik
> Cc: Peer Chen; Christoph Hellwig; Prakash Punnoor; Andrew Morton;
> 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 Fri, 17 Nov 2006 13:04:30 -0500
> Jeff Garzik <[EMAIL PROTECTED]> wrote:
> 
> > Peer Chen wrote:
> > > I didn't get any comment from you guys for new patch, does someone
> take
> > > care this patch, do we still need some modification upon it? Or do
> we
> > > need re-submit it in other thread?
> > 
> > For my part, it's in my queue of things to review.  I need to figure
> out 
> > the best way to export this support.
> > 
> > Jeff
> 
> Adding SGPIO support directly into an individual SATA driver seems to be
> a very bad idea since SGPIO is a) a protocol that can be used by many 
> different SATA controllers and is not nvidia specific and b) can also
> be used by SAS.
> 
> 
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel"
> in
> the body of a message to [EMAIL PROTECTED]
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> ---
> 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.
> ---
> 
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


RE: [PATCH] SCSI: Add the SGPIO support for sata_nv.c

2007-02-01 Thread Andy Currid

I'm not sure I understand your point. To support SGPIO initiators
incorporated
into SATA host controllers, it is pretty much inevitable that some SGPIO
support
will have to be added directly into individual drivers for thoese
controllers;
the SGPIO standard does not define a register level interface for
accessing
this functionality, so vendor implementations vary.

It's possible that with an SGPIO initiator integrated within a SAS host
controller,
you may be able to use SMP frames to access it, using the messaging
defined in
SFF-8485 chapter 8 - but I wouldn't bank on it.

Andy

-Original Message-
From: [EMAIL PROTECTED]
[mailto:[EMAIL PROTECTED] On Behalf Of Kristen Carlson
Accardi
Sent: Wednesday, January 31, 2007 15:22
To: Jeff Garzik
Cc: Peer Chen; Christoph Hellwig; Prakash Punnoor; Andrew Morton;
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 Fri, 17 Nov 2006 13:04:30 -0500
Jeff Garzik <[EMAIL PROTECTED]> wrote:

> Peer Chen wrote:
> > I didn't get any comment from you guys for new patch, does someone
take
> > care this patch, do we still need some modification upon it? Or do
we
> > need re-submit it in other thread?
> 
> For my part, it's in my queue of things to review.  I need to figure
out 
> the best way to export this support.
> 
>   Jeff

Adding SGPIO support directly into an individual SATA driver seems to be
a very bad idea since SGPIO is a) a protocol that can be used by many 
different SATA controllers and is not nvidia specific and b) can also
be used by SAS.


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel"
in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
---
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.
---
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


RE: [PATCH] SCSI: Add the SGPIO support for sata_nv.c

2007-02-01 Thread Andy Currid

I'm not sure I understand your point. To support SGPIO initiators
incorporated
into SATA host controllers, it is pretty much inevitable that some SGPIO
support
will have to be added directly into individual drivers for thoese
controllers;
the SGPIO standard does not define a register level interface for
accessing
this functionality, so vendor implementations vary.

It's possible that with an SGPIO initiator integrated within a SAS host
controller,
you may be able to use SMP frames to access it, using the messaging
defined in
SFF-8485 chapter 8 - but I wouldn't bank on it.

Andy

-Original Message-
From: [EMAIL PROTECTED]
[mailto:[EMAIL PROTECTED] On Behalf Of Kristen Carlson
Accardi
Sent: Wednesday, January 31, 2007 15:22
To: Jeff Garzik
Cc: Peer Chen; Christoph Hellwig; Prakash Punnoor; Andrew Morton;
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 Fri, 17 Nov 2006 13:04:30 -0500
Jeff Garzik [EMAIL PROTECTED] wrote:

 Peer Chen wrote:
  I didn't get any comment from you guys for new patch, does someone
take
  care this patch, do we still need some modification upon it? Or do
we
  need re-submit it in other thread?
 
 For my part, it's in my queue of things to review.  I need to figure
out 
 the best way to export this support.
 
   Jeff

Adding SGPIO support directly into an individual SATA driver seems to be
a very bad idea since SGPIO is a) a protocol that can be used by many 
different SATA controllers and is not nvidia specific and b) can also
be used by SAS.


-
To unsubscribe from this list: send the line unsubscribe linux-kernel
in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
---
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.
---
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] SCSI: Add the SGPIO support for sata_nv.c

2007-02-01 Thread Kristen Carlson Accardi
On Thu, 1 Feb 2007 11:55:35 -0800
Andy Currid [EMAIL PROTECTED] wrote:

 
 I'm not sure I understand your point. To support SGPIO initiators
 incorporated
 into SATA host controllers, it is pretty much inevitable that some SGPIO
 support
 will have to be added directly into individual drivers for thoese
 controllers;

If SGPIO is defined as a generic protocol, could there be bits that are
used universally by all controllers that support a SGPIO interface, and
then of course chipset specific bits as far as how this is used?  For
example, AHCI 1.1 specifies that Enclosure Management via SGPIO is 
supported, and uses the message interface defined in SFF-8485.  If there
are any parts to your implementation that are remotely standard, it would
be preferrable in my opinion to split them out into a separate file
so that you have vendor specific code in the driver, and generic sgpio
defines in a separate file that can be shared somehow.

Probably I should have more appropriately asked whether there were any
bits in this that could be made common :).

Kristen

 the SGPIO standard does not define a register level interface for
 accessing
 this functionality, so vendor implementations vary.
 
 It's possible that with an SGPIO initiator integrated within a SAS host
 controller,
 you may be able to use SMP frames to access it, using the messaging
 defined in
 SFF-8485 chapter 8 - but I wouldn't bank on it.
 
 Andy
 
 -Original Message-
 From: [EMAIL PROTECTED]
 [mailto:[EMAIL PROTECTED] On Behalf Of Kristen Carlson
 Accardi
 Sent: Wednesday, January 31, 2007 15:22
 To: Jeff Garzik
 Cc: Peer Chen; Christoph Hellwig; Prakash Punnoor; Andrew Morton;
 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 Fri, 17 Nov 2006 13:04:30 -0500
 Jeff Garzik [EMAIL PROTECTED] wrote:
 
  Peer Chen wrote:
   I didn't get any comment from you guys for new patch, does someone
 take
   care this patch, do we still need some modification upon it? Or do
 we
   need re-submit it in other thread?
  
  For my part, it's in my queue of things to review.  I need to figure
 out 
  the best way to export this support.
  
  Jeff
 
 Adding SGPIO support directly into an individual SATA driver seems to be
 a very bad idea since SGPIO is a) a protocol that can be used by many 
 different SATA controllers and is not nvidia specific and b) can also
 be used by SAS.
 
 
 -
 To unsubscribe from this list: send the line unsubscribe linux-kernel
 in
 the body of a message to [EMAIL PROTECTED]
 More majordomo info at  http://vger.kernel.org/majordomo-info.html
 Please read the FAQ at  http://www.tux.org/lkml/
 ---
 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.
 ---
 
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] SCSI: Add the SGPIO support for sata_nv.c

2007-01-31 Thread Kristen Carlson Accardi
On Fri, 17 Nov 2006 13:04:30 -0500
Jeff Garzik <[EMAIL PROTECTED]> wrote:

> Peer Chen wrote:
> > I didn't get any comment from you guys for new patch, does someone take
> > care this patch, do we still need some modification upon it? Or do we
> > need re-submit it in other thread?
> 
> For my part, it's in my queue of things to review.  I need to figure out 
> the best way to export this support.
> 
>   Jeff

Adding SGPIO support directly into an individual SATA driver seems to be
a very bad idea since SGPIO is a) a protocol that can be used by many 
different SATA controllers and is not nvidia specific and b) can also
be used by SAS.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] SCSI: Add the SGPIO support for sata_nv.c

2007-01-31 Thread Kristen Carlson Accardi
On Fri, 17 Nov 2006 13:04:30 -0500
Jeff Garzik [EMAIL PROTECTED] wrote:

 Peer Chen wrote:
  I didn't get any comment from you guys for new patch, does someone take
  care this patch, do we still need some modification upon it? Or do we
  need re-submit it in other thread?
 
 For my part, it's in my queue of things to review.  I need to figure out 
 the best way to export this support.
 
   Jeff

Adding SGPIO support directly into an individual SATA driver seems to be
a very bad idea since SGPIO is a) a protocol that can be used by many 
different SATA controllers and is not nvidia specific and b) can also
be used by SAS.
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] SCSI: Add the SGPIO support for sata_nv.c

2007-01-09 Thread Robert Hancock

Since I've been the one making the most changes in sata_nv lately I
figured I would make some more comments on this patch:


Subject:
RE: [PATCH] SCSI: Add the SGPIO support for sata_nv.c
From:
"Peer Chen" <[EMAIL PROTECTED]>
Date:
Tue, 7 Nov 2006 17:55:11 +0800
To:
"Christoph Hellwig" <[EMAIL PROTECTED]>

To:
"Christoph Hellwig" <[EMAIL PROTECTED]>
CC:
<[EMAIL PROTECTED]>, , 
, "Kuan Luo" <[EMAIL PROTECTED]>



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 <[EMAIL PROTECTED]>
Singed-off-by: Peer Chen <[EMAIL PROTECTED]>


BRs
Peer Chen

  
---
  



--- linux-2.6.19-rc4-git9/drivers/ata/sata_nv.c.orig2006-11-06 
08:47:49.0 +0800
+++ linux-2.6.19-rc4-git9/drivers/ata/sata_nv.c 2006-11-07 08:36:54.0 
+0800


First off, can you rebase against the current libata-dev version of 
sata_nv? There have been

a ton of changes since 2.6.19-rc4-git9.

Also, it would be best to make sure the code passes the checks from the 
"Sparse" tool

using make C=1. sata_nv now passes this, it would be a shame to break that.


@@ -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_DATA3
+
+/* SGPIO command status defines */
+#define NV_SGPIO_CMD_OK0
+#define NV_SGPIO_CMD_ACTIVE1
+#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_OFFSET0x58
+#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)));
+}


Maybe some comment about what these functions do? It's not entirely obvious.


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

Re: [PATCH] SCSI: Add the SGPIO support for sata_nv.c

2007-01-09 Thread Robert Hancock

Since I've been the one making the most changes in sata_nv lately I
figured I would make some more comments on this patch:


Subject:
RE: [PATCH] SCSI: Add the SGPIO support for sata_nv.c
From:
Peer Chen [EMAIL PROTECTED]
Date:
Tue, 7 Nov 2006 17:55:11 +0800
To:
Christoph Hellwig [EMAIL PROTECTED]

To:
Christoph Hellwig [EMAIL PROTECTED]
CC:
[EMAIL PROTECTED], linux-kernel@vger.kernel.org, 
linux-ide@vger.kernel.org, Kuan Luo [EMAIL PROTECTED]



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 [EMAIL PROTECTED]
Singed-off-by: Peer Chen [EMAIL PROTECTED]


BRs
Peer Chen

  
---
  



--- linux-2.6.19-rc4-git9/drivers/ata/sata_nv.c.orig2006-11-06 
08:47:49.0 +0800
+++ linux-2.6.19-rc4-git9/drivers/ata/sata_nv.c 2006-11-07 08:36:54.0 
+0800


First off, can you rebase against the current libata-dev version of 
sata_nv? There have been

a ton of changes since 2.6.19-rc4-git9.

Also, it would be best to make sure the code passes the checks from the 
Sparse tool

using make C=1. sata_nv now passes this, it would be a shame to break that.


@@ -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_DATA3
+
+/* SGPIO command status defines */
+#define NV_SGPIO_CMD_OK0
+#define NV_SGPIO_CMD_ACTIVE1
+#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_OFFSET0x58
+#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)));
+}


Maybe some comment about what these functions do? It's not entirely obvious.


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

Re: [PATCH] SCSI: Add the SGPIO support for sata_nv.c

2006-11-17 Thread Jeff Garzik

Peer Chen wrote:

I didn't get any comment from you guys for new patch, does someone take
care this patch, do we still need some modification upon it? Or do we
need re-submit it in other thread?


For my part, it's in my queue of things to review.  I need to figure out 
the best way to export this support.


Jeff



-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] SCSI: Add the SGPIO support for sata_nv.c

2006-11-17 Thread Heikki Orsila
On Fri, Nov 17, 2006 at 04:36:37PM +0800, Peer Chen wrote:
> I didn't get any comment from you guys for new patch, does someone take
> care this patch, do we still need some modification upon it? Or do we
> need re-submit it in other thread?

One small change suggestion:

> +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;   
> +}  

Make it shorter ->

return ent->device == PCI_DEVICE_ID_NVIDIA_NFORCE_MCP55_SATA2;

 - Heikki
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


RE: [PATCH] SCSI: Add the SGPIO support for sata_nv.c

2006-11-17 Thread Peer Chen
I didn't get any comment from you guys for new patch, does someone take
care this patch, do we still need some modification upon it? Or do we
need re-submit it in other thread?

BRs
Peer Chen

-Original Message-
From: Peer Chen 
Sent: Tuesday, November 07, 2006 5:55 PM
To: 'Christoph Hellwig'
Cc: [EMAIL PROTECTED]; linux-kernel@vger.kernel.org;
linux-ide@vger.kernel.org; Kuan Luo
Subject: RE: [PATCH] SCSI: Add the SGPIO support for sata_nv.c

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 <[EMAIL PROTECTED]>
Singed-off-by: Peer Chen <[EMAIL PROTECTED]>


BRs
Peer Chen

-Original Message-
From: Christoph Hellwig [mailto:[EMAIL PROTECTED] 
Sent: Tuesday, October 31, 2006 6:41 PM
To: Peer Chen
Cc: [EMAIL PROTECTED]; 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 <[EMAIL PROTECTED]>

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_tnv_sgpio_lock;

please use DEFINE_SPINLOCK to initialize the lock statically.

> +static unsigned long nv_sgpio_tstamp;

> 

RE: [PATCH] SCSI: Add the SGPIO support for sata_nv.c

2006-11-17 Thread Peer Chen
I didn't get any comment from you guys for new patch, does someone take
care this patch, do we still need some modification upon it? Or do we
need re-submit it in other thread?

BRs
Peer Chen

-Original Message-
From: Peer Chen 
Sent: Tuesday, November 07, 2006 5:55 PM
To: 'Christoph Hellwig'
Cc: [EMAIL PROTECTED]; linux-kernel@vger.kernel.org;
linux-ide@vger.kernel.org; Kuan Luo
Subject: RE: [PATCH] SCSI: Add the SGPIO support for sata_nv.c

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 [EMAIL PROTECTED]
Singed-off-by: Peer Chen [EMAIL PROTECTED]


BRs
Peer Chen

-Original Message-
From: Christoph Hellwig [mailto:[EMAIL PROTECTED] 
Sent: Tuesday, October 31, 2006 6:41 PM
To: Peer Chen
Cc: [EMAIL PROTECTED]; 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 [EMAIL PROTECTED]

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_tnv_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

Re: [PATCH] SCSI: Add the SGPIO support for sata_nv.c

2006-11-17 Thread Heikki Orsila
On Fri, Nov 17, 2006 at 04:36:37PM +0800, Peer Chen wrote:
 I didn't get any comment from you guys for new patch, does someone take
 care this patch, do we still need some modification upon it? Or do we
 need re-submit it in other thread?

One small change suggestion:

 +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;   
 +}  

Make it shorter -

return ent-device == PCI_DEVICE_ID_NVIDIA_NFORCE_MCP55_SATA2;

 - Heikki
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] SCSI: Add the SGPIO support for sata_nv.c

2006-11-17 Thread Jeff Garzik

Peer Chen wrote:

I didn't get any comment from you guys for new patch, does someone take
care this patch, do we still need some modification upon it? Or do we
need re-submit it in other thread?


For my part, it's in my queue of things to review.  I need to figure out 
the best way to export this support.


Jeff



-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/