[PATCH] aacraid: don't assign cpu_to_le32(constant) to u8

[PATCH] aacraid: don't assign cpu_to_le32(constant) to u8

[Stephen Rothwell]

Noticed on PowerPC allmod config build:

drivers/scsi/aacraid/commsup.c:1342: warning: large integer implicitly truncated to unsigned type
drivers/scsi/aacraid/commsup.c:1343: warning: large integer implicitly truncated to unsigned type
drivers/scsi/aacraid/commsup.c:1344: warning: large integer implicitly truncated to unsigned type

Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
---
 drivers/scsi/aacraid/commsup.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au

diff --git a/drivers/scsi/aacraid/commsup.c b/drivers/scsi/aacraid/commsup.c
index 240a0bb..b9682a8 100644
--- a/drivers/scsi/aacraid/commsup.c
+++ b/drivers/scsi/aacraid/commsup.c
@@ -1339,9 +1339,9 @@ int aac_check_health(struct aac_dev * aac)
 			aif = (struct aac_aifcmd *)hw_fib->data;
 			aif->command = cpu_to_le32(AifCmdEventNotify);
 		 	aif->seqnum = cpu_to_le32(0xFFFFFFFF);
-		 	aif->data[0] = cpu_to_le32(AifEnExpEvent);
-			aif->data[1] = cpu_to_le32(AifExeFirmwarePanic);
-		 	aif->data[2] = cpu_to_le32(AifHighPriority);
+		 	aif->data[0] = AifEnExpEvent;
+			aif->data[1] = AifExeFirmwarePanic;
+		 	aif->data[2] = AifHighPriority;
 			aif->data[3] = cpu_to_le32(BlinkLED);
 
 			/*
-- 
1.5.3.4

RE: [PATCH] aacraid: don't assign cpu_to_le32(constant) to u8

[Salyzyn, Mark]

ACK

Sincerely -- Mark Salyzyn

> -----Original Message-----
> From: Stephen Rothwell [mailto:sfr@canb.auug.org.au] 
> Sent: Wednesday, October 31, 2007 12:02 AM
> To: AACRAID
> Cc: linux-scsi@vger.kernel.org; LKML
> Subject: [PATCH] aacraid: don't assign cpu_to_le32(constant) to u8
> 
> Noticed on PowerPC allmod config build:
> 
> drivers/scsi/aacraid/commsup.c:1342: warning: large integer 
> implicitly truncated to unsigned type
> drivers/scsi/aacraid/commsup.c:1343: warning: large integer 
> implicitly truncated to unsigned type
> drivers/scsi/aacraid/commsup.c:1344: warning: large integer 
> implicitly truncated to unsigned type
> 
> Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
> ---
>  drivers/scsi/aacraid/commsup.c |    6 +++---
>  1 files changed, 3 insertions(+), 3 deletions(-)
> 
> -- 
> Cheers,
> Stephen Rothwell                    sfr@canb.auug.org.au
> 
> diff --git a/drivers/scsi/aacraid/commsup.c 
> b/drivers/scsi/aacraid/commsup.c
> index 240a0bb..b9682a8 100644
> --- a/drivers/scsi/aacraid/commsup.c
> +++ b/drivers/scsi/aacraid/commsup.c
> @@ -1339,9 +1339,9 @@ int aac_check_health(struct aac_dev * aac)
>  			aif = (struct aac_aifcmd *)hw_fib->data;
>  			aif->command = cpu_to_le32(AifCmdEventNotify);
>  		 	aif->seqnum = cpu_to_le32(0xFFFFFFFF);
> -		 	aif->data[0] = cpu_to_le32(AifEnExpEvent);
> -			aif->data[1] = cpu_to_le32(AifExeFirmwarePanic);
> -		 	aif->data[2] = cpu_to_le32(AifHighPriority);
> +		 	aif->data[0] = AifEnExpEvent;
> +			aif->data[1] = AifExeFirmwarePanic;
> +		 	aif->data[2] = AifHighPriority;
>  			aif->data[3] = cpu_to_le32(BlinkLED);
>  
>  			/*
> -- 
> 1.5.3.4
> 

[PATCHv2] aacraid: don't assign cpu_to_le32(constant) to u8

[Stephen Rothwell]

Noticed on PowerPC allmod config build:

drivers/scsi/aacraid/commsup.c:1342: warning: large integer implicitly truncated to unsigned type
drivers/scsi/aacraid/commsup.c:1343: warning: large integer implicitly truncated to unsigned type
drivers/scsi/aacraid/commsup.c:1344: warning: large integer implicitly truncated to unsigned type

Also fix some whitespace on the changed lines.

Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
---
 drivers/scsi/aacraid/commsup.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

This version just fixes a couple of whitespace anomolies on the lines I
changed.

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au

diff --git a/drivers/scsi/aacraid/commsup.c b/drivers/scsi/aacraid/commsup.c
index 240a0bb..3c2dbc0 100644
--- a/drivers/scsi/aacraid/commsup.c
+++ b/drivers/scsi/aacraid/commsup.c
@@ -1339,9 +1339,9 @@ int aac_check_health(struct aac_dev * aac)
 			aif = (struct aac_aifcmd *)hw_fib->data;
 			aif->command = cpu_to_le32(AifCmdEventNotify);
 		 	aif->seqnum = cpu_to_le32(0xFFFFFFFF);
-		 	aif->data[0] = cpu_to_le32(AifEnExpEvent);
-			aif->data[1] = cpu_to_le32(AifExeFirmwarePanic);
-		 	aif->data[2] = cpu_to_le32(AifHighPriority);
+			aif->data[0] = AifEnExpEvent;
+			aif->data[1] = AifExeFirmwarePanic;
+			aif->data[2] = AifHighPriority;
 			aif->data[3] = cpu_to_le32(BlinkLED);
 
 			/*
-- 
1.5.3.4

RE: [PATCHv2] aacraid: don't assign cpu_to_le32(constant) to u8

[Salyzyn, Mark]

ACK v2

Sincerely -- Mark Salyzyn

> -----Original Message-----
> From: Stephen Rothwell [mailto:sfr@canb.auug.org.au] 
> Sent: Thursday, November 01, 2007 2:32 AM
> To: AACRAID
> Cc: linux-scsi@vger.kernel.org; LKML
> Subject: [PATCHv2] aacraid: don't assign cpu_to_le32(constant) to u8
> 
> Noticed on PowerPC allmod config build:
> 
> drivers/scsi/aacraid/commsup.c:1342: warning: large integer 
> implicitly truncated to unsigned type
> drivers/scsi/aacraid/commsup.c:1343: warning: large integer 
> implicitly truncated to unsigned type
> drivers/scsi/aacraid/commsup.c:1344: warning: large integer 
> implicitly truncated to unsigned type
> 
> Also fix some whitespace on the changed lines.
> 
> Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
> ---
>  drivers/scsi/aacraid/commsup.c |    6 +++---
>  1 files changed, 3 insertions(+), 3 deletions(-)
> 
> This version just fixes a couple of whitespace anomolies on 
> the lines I
> changed.
> 
> -- 
> Cheers,
> Stephen Rothwell                    sfr@canb.auug.org.au
> 
> diff --git a/drivers/scsi/aacraid/commsup.c 
> b/drivers/scsi/aacraid/commsup.c
> index 240a0bb..3c2dbc0 100644
> --- a/drivers/scsi/aacraid/commsup.c
> +++ b/drivers/scsi/aacraid/commsup.c
> @@ -1339,9 +1339,9 @@ int aac_check_health(struct aac_dev * aac)
>  			aif = (struct aac_aifcmd *)hw_fib->data;
>  			aif->command = cpu_to_le32(AifCmdEventNotify);
>  		 	aif->seqnum = cpu_to_le32(0xFFFFFFFF);
> -		 	aif->data[0] = cpu_to_le32(AifEnExpEvent);
> -			aif->data[1] = cpu_to_le32(AifExeFirmwarePanic);
> -		 	aif->data[2] = cpu_to_le32(AifHighPriority);
> +			aif->data[0] = AifEnExpEvent;
> +			aif->data[1] = AifExeFirmwarePanic;
> +			aif->data[2] = AifHighPriority;
>  			aif->data[3] = cpu_to_le32(BlinkLED);
>  
>  			/*
> -- 
> 1.5.3.4
> 

Re: [PATCHv2] aacraid: don't assign cpu_to_le32(constant) to u8

[Andreas Schwab]

Stephen Rothwell <sfr@canb.auug.org.au> writes:

> diff --git a/drivers/scsi/aacraid/commsup.c b/drivers/scsi/aacraid/commsup.c
> index 240a0bb..3c2dbc0 100644
> --- a/drivers/scsi/aacraid/commsup.c
> +++ b/drivers/scsi/aacraid/commsup.c
> @@ -1339,9 +1339,9 @@ int aac_check_health(struct aac_dev * aac)
>  			aif = (struct aac_aifcmd *)hw_fib->data;
>  			aif->command = cpu_to_le32(AifCmdEventNotify);
>  		 	aif->seqnum = cpu_to_le32(0xFFFFFFFF);
> -		 	aif->data[0] = cpu_to_le32(AifEnExpEvent);
> -			aif->data[1] = cpu_to_le32(AifExeFirmwarePanic);
> -		 	aif->data[2] = cpu_to_le32(AifHighPriority);
> +			aif->data[0] = AifEnExpEvent;
> +			aif->data[1] = AifExeFirmwarePanic;
> +			aif->data[2] = AifHighPriority;
>  			aif->data[3] = cpu_to_le32(BlinkLED);

What about the last line?

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
PGP key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

[PATCH 1/1] aacraid: don't assign cpu_to_le32(int) to u8

[Salyzyn, Mark]

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

Good point, thanks. The intent of the management applications
utilization of this AIF report is to observe the LSB of the value of
integer value in BlinkLED. The actions of the cpu_to_le32 actually
breaks this and reports the wrong content in swapped architectures.

This attached follow-up patch is against current scsi-misc-2.6 *after*
the application of the 'don't assign cpu_to_le32(constant) to u8' patch
submitted by Stephen Rothwell which has already been taken by the -mm
tree. Inspection of other areas of the aacraid driver came up blank for
similar style bugs.

ObligatoryDisclaimer: Please accept my condolences regarding Outlook's
handling of patch attachments (inline gets damaged, use attachment).

Signed-off-by: Mark Salyzyn <aacraid@adaptec.com>

 drivers/scsi/aacraid/commsup.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff -ru a/drivers/scsi/aacraid/commsup.c
b/drivers/scsi/aacraid/commsup.c
--- a/drivers/scsi/aacraid/commsup.c    2007-11-07 10:35:16.603727464
-0500
+++ b/drivers/scsi/aacraid/commsup.c    2007-11-07 10:37:50.540311107
-0500
@@ -1342,7 +1342,7 @@
                        aif->data[0] = AifEnExpEvent;
                        aif->data[1] = AifExeFirmwarePanic;
                        aif->data[2] = AifHighPriority;
-                       aif->data[3] = cpu_to_le32(BlinkLED);
+                       aif->data[3] = BlinkLED;

                        /*
                         * Put the FIB onto the

Sincerely -- Mark Salyzyn

> -----Original Message-----
> From: Andreas Schwab [mailto:schwab@suse.de] 
> Sent: Thursday, November 01, 2007 9:31 AM
> To: Stephen Rothwell
> Cc: AACRAID; linux-scsi@vger.kernel.org; LKML
> Subject: Re: [PATCHv2] aacraid: don't assign 
> cpu_to_le32(constant) to u8
> 
> Stephen Rothwell <sfr@canb.auug.org.au> writes:
> 
> > diff --git a/drivers/scsi/aacraid/commsup.c 
> b/drivers/scsi/aacraid/commsup.c
> > index 240a0bb..3c2dbc0 100644
> > --- a/drivers/scsi/aacraid/commsup.c
> > +++ b/drivers/scsi/aacraid/commsup.c
> > @@ -1339,9 +1339,9 @@ int aac_check_health(struct aac_dev * aac)
> >  			aif = (struct aac_aifcmd *)hw_fib->data;
> >  			aif->command = cpu_to_le32(AifCmdEventNotify);
> >  		 	aif->seqnum = cpu_to_le32(0xFFFFFFFF);
> > -		 	aif->data[0] = cpu_to_le32(AifEnExpEvent);
> > -			aif->data[1] = cpu_to_le32(AifExeFirmwarePanic);
> > -		 	aif->data[2] = cpu_to_le32(AifHighPriority);
> > +			aif->data[0] = AifEnExpEvent;
> > +			aif->data[1] = AifExeFirmwarePanic;
> > +			aif->data[2] = AifHighPriority;
> >  			aif->data[3] = cpu_to_le32(BlinkLED);
> 
> What about the last line?
> 
> Andreas.

[-- Attachment #2: aacraid_BlinkLED.patch --]
[-- Type: application/octet-stream, Size: 461 bytes --]

diff -ru a/drivers/scsi/aacraid/commsup.c b/drivers/scsi/aacraid/commsup.c
--- a/drivers/scsi/aacraid/commsup.c	2007-11-07 10:35:16.603727464 -0500
+++ b/drivers/scsi/aacraid/commsup.c	2007-11-07 10:37:50.540311107 -0500
@@ -1342,7 +1342,7 @@
 			aif->data[0] = AifEnExpEvent;
 			aif->data[1] = AifExeFirmwarePanic;
 			aif->data[2] = AifHighPriority;
-			aif->data[3] = cpu_to_le32(BlinkLED);
+			aif->data[3] = BlinkLED;
 
 			/*
 			 * Put the FIB onto the

Re: [PATCH 1/1] aacraid: don't assign cpu_to_le32(int) to u8

[Christoph Hellwig]

On Wed, Nov 07, 2007 at 10:58:12AM -0500, Salyzyn, Mark wrote:
> Good point, thanks. The intent of the management applications
> utilization of this AIF report is to observe the LSB of the value of
> integer value in BlinkLED. The actions of the cpu_to_le32 actually
> breaks this and reports the wrong content in swapped architectures.
> 
> This attached follow-up patch is against current scsi-misc-2.6 *after*
> the application of the 'don't assign cpu_to_le32(constant) to u8' patch
> submitted by Stephen Rothwell which has already been taken by the -mm
> tree. Inspection of other areas of the aacraid driver came up blank for
> similar style bugs.

Did anyone run the driver through sparse to see if we have more issues like
this?

RE: [PATCH 1/1] aacraid: don't assign cpu_to_le32(int) to u8

[Salyzyn, Mark]

Christoph Hellwig [mailto:hch@infradead.org] sez:
> Did anyone run the driver through sparse to see if we have 
> more issues like this?

There are some warnings from sparse, none like this one. I will deal
with the warnings ...

Sincerely -- Mark Salyzyn

Re: [PATCH 1/1] aacraid: don't assign cpu_to_le32(int) to u8

[Christoph Hellwig]

On Wed, Nov 07, 2007 at 01:51:44PM -0500, Salyzyn, Mark wrote:
> Christoph Hellwig [mailto:hch@infradead.org] sez:
> > Did anyone run the driver through sparse to see if we have 
> > more issues like this?
> 
> There are some warnings from sparse, none like this one. I will deal
> with the warnings ...

Actually there are a lot of endianess warnings, fortunately most of them
harmless.  The patch below fixes all of them up (including the ones in
the patch I replied to), except for aac_init_adapter which is really odd
and I don't know what to do.


Signed-off-by: Christoph Hellwig <hch@lst.de>

Index: linux-2.6/drivers/scsi/aacraid/aachba.c
===================================================================
--- linux-2.6.orig/drivers/scsi/aacraid/aachba.c	2007-11-08 17:09:50.000000000 +0100
+++ linux-2.6/drivers/scsi/aacraid/aachba.c	2007-11-08 17:14:43.000000000 +0100
@@ -981,7 +981,7 @@
 	aac_fib_init(fib);
 	readcmd = (struct aac_read *) fib_data(fib);
 	readcmd->command = cpu_to_le32(VM_CtBlockRead);
-	readcmd->cid = cpu_to_le16(scmd_id(cmd));
+	readcmd->cid = cpu_to_le32(scmd_id(cmd));
 	readcmd->block = cpu_to_le32((u32)(lba&0xffffffff));
 	readcmd->count = cpu_to_le32(count * 512);
 
@@ -1072,7 +1072,7 @@
 	aac_fib_init(fib);
 	writecmd = (struct aac_write *) fib_data(fib);
 	writecmd->command = cpu_to_le32(VM_CtBlockWrite);
-	writecmd->cid = cpu_to_le16(scmd_id(cmd));
+	writecmd->cid = cpu_to_le32(scmd_id(cmd));
 	writecmd->block = cpu_to_le32((u32)(lba&0xffffffff));
 	writecmd->count = cpu_to_le32(count * 512);
 	writecmd->sg.count = cpu_to_le32(1);
@@ -1306,8 +1306,8 @@
 			  dev->supplement_adapter_info.VpdInfo.Tsid);
 		}
 		if (!aac_check_reset ||
-		  (dev->supplement_adapter_info.SupportedOptions2 &
-		  le32_to_cpu(AAC_OPTION_IGNORE_RESET))) {
+		    (dev->supplement_adapter_info.SupportedOptions2 &
+		     cpu_to_le32(AAC_OPTION_IGNORE_RESET))) {
 			printk(KERN_INFO "%s%d: Reset Adapter Ignored\n",
 			  dev->name, dev->id);
 		}
Index: linux-2.6/drivers/scsi/aacraid/commsup.c
===================================================================
--- linux-2.6.orig/drivers/scsi/aacraid/commsup.c	2007-11-08 17:09:50.000000000 +0100
+++ linux-2.6/drivers/scsi/aacraid/commsup.c	2007-11-08 17:14:43.000000000 +0100
@@ -796,13 +796,13 @@
 	 */
 	switch (le32_to_cpu(aifcmd->command)) {
 	case AifCmdDriverNotify:
-		switch (le32_to_cpu(((u32 *)aifcmd->data)[0])) {
+		switch (le32_to_cpu(((__le32 *)aifcmd->data)[0])) {
 		/*
 		 *	Morph or Expand complete
 		 */
 		case AifDenMorphComplete:
 		case AifDenVolumeExtendComplete:
-			container = le32_to_cpu(((u32 *)aifcmd->data)[1]);
+			container = le32_to_cpu(((__le32 *)aifcmd->data)[1]);
 			if (container >= dev->maximum_num_containers)
 				break;
 
@@ -835,25 +835,25 @@
 			if (container >= dev->maximum_num_containers)
 				break;
 			if ((dev->fsa_dev[container].config_waiting_on ==
-			    le32_to_cpu(*(u32 *)aifcmd->data)) &&
+			    le32_to_cpu(*(__le32 *)aifcmd->data)) &&
 			 time_before(jiffies, dev->fsa_dev[container].config_waiting_stamp + AIF_SNIFF_TIMEOUT))
 				dev->fsa_dev[container].config_waiting_on = 0;
 		} else for (container = 0;
 		    container < dev->maximum_num_containers; ++container) {
 			if ((dev->fsa_dev[container].config_waiting_on ==
-			    le32_to_cpu(*(u32 *)aifcmd->data)) &&
+			    le32_to_cpu(*(__le32 *)aifcmd->data)) &&
 			 time_before(jiffies, dev->fsa_dev[container].config_waiting_stamp + AIF_SNIFF_TIMEOUT))
 				dev->fsa_dev[container].config_waiting_on = 0;
 		}
 		break;
 
 	case AifCmdEventNotify:
-		switch (le32_to_cpu(((u32 *)aifcmd->data)[0])) {
+		switch (le32_to_cpu(((__le32 *)aifcmd->data)[0])) {
 		/*
 		 *	Add an Array.
 		 */
 		case AifEnAddContainer:
-			container = le32_to_cpu(((u32 *)aifcmd->data)[1]);
+			container = le32_to_cpu(((__le32 *)aifcmd->data)[1]);
 			if (container >= dev->maximum_num_containers)
 				break;
 			dev->fsa_dev[container].config_needed = ADD;
@@ -866,7 +866,7 @@
 		 *	Delete an Array.
 		 */
 		case AifEnDeleteContainer:
-			container = le32_to_cpu(((u32 *)aifcmd->data)[1]);
+			container = le32_to_cpu(((__le32 *)aifcmd->data)[1]);
 			if (container >= dev->maximum_num_containers)
 				break;
 			dev->fsa_dev[container].config_needed = DELETE;
@@ -880,7 +880,7 @@
 		 * waiting on something else, setup to wait on a Config Change.
 		 */
 		case AifEnContainerChange:
-			container = le32_to_cpu(((u32 *)aifcmd->data)[1]);
+			container = le32_to_cpu(((__le32 *)aifcmd->data)[1]);
 			if (container >= dev->maximum_num_containers)
 				break;
 			if (dev->fsa_dev[container].config_waiting_on &&
@@ -905,13 +905,13 @@
 			if (container >= dev->maximum_num_containers)
 				break;
 			if ((dev->fsa_dev[container].config_waiting_on ==
-			    le32_to_cpu(*(u32 *)aifcmd->data)) &&
+			    le32_to_cpu(*(__le32 *)aifcmd->data)) &&
 			 time_before(jiffies, dev->fsa_dev[container].config_waiting_stamp + AIF_SNIFF_TIMEOUT))
 				dev->fsa_dev[container].config_waiting_on = 0;
 		} else for (container = 0;
 		    container < dev->maximum_num_containers; ++container) {
 			if ((dev->fsa_dev[container].config_waiting_on ==
-			    le32_to_cpu(*(u32 *)aifcmd->data)) &&
+			    le32_to_cpu(*(__le32 *)aifcmd->data)) &&
 			 time_before(jiffies, dev->fsa_dev[container].config_waiting_stamp + AIF_SNIFF_TIMEOUT))
 				dev->fsa_dev[container].config_waiting_on = 0;
 		}
@@ -926,9 +926,9 @@
 		 * wait for a container change.
 		 */
 
-		if ((((u32 *)aifcmd->data)[1] == cpu_to_le32(AifJobCtrZero))
-		 && ((((u32 *)aifcmd->data)[6] == ((u32 *)aifcmd->data)[5])
-		  || (((u32 *)aifcmd->data)[4] == cpu_to_le32(AifJobStsSuccess)))) {
+		if (((__le32 *)aifcmd->data)[1] == cpu_to_le32(AifJobCtrZero) &&
+		    (((__le32 *)aifcmd->data)[6] == ((__le32 *)aifcmd->data)[5] ||
+		     ((__le32 *)aifcmd->data)[4] == cpu_to_le32(AifJobStsSuccess))) {
 			for (container = 0;
 			    container < dev->maximum_num_containers;
 			    ++container) {
@@ -943,9 +943,9 @@
 					jiffies;
 			}
 		}
-		if ((((u32 *)aifcmd->data)[1] == cpu_to_le32(AifJobCtrZero))
-		 && (((u32 *)aifcmd->data)[6] == 0)
-		 && (((u32 *)aifcmd->data)[4] == cpu_to_le32(AifJobStsRunning))) {
+		if (((__le32 *)aifcmd->data)[1] == cpu_to_le32(AifJobCtrZero) &&
+		    ((__le32 *)aifcmd->data)[6] == 0 &&
+		    ((__le32 *)aifcmd->data)[4] == cpu_to_le32(AifJobStsRunning)) {
 			for (container = 0;
 			    container < dev->maximum_num_containers;
 			    ++container) {
@@ -1339,10 +1339,10 @@
 			aif = (struct aac_aifcmd *)hw_fib->data;
 			aif->command = cpu_to_le32(AifCmdEventNotify);
 		 	aif->seqnum = cpu_to_le32(0xFFFFFFFF);
-		 	aif->data[0] = cpu_to_le32(AifEnExpEvent);
-			aif->data[1] = cpu_to_le32(AifExeFirmwarePanic);
-		 	aif->data[2] = cpu_to_le32(AifHighPriority);
-			aif->data[3] = cpu_to_le32(BlinkLED);
+		 	aif->data[0] = AifEnExpEvent;
+			aif->data[1] = AifExeFirmwarePanic;
+		 	aif->data[2] = AifHighPriority;
+			aif->data[3] = BlinkLED;
 
 			/*
 			 * Put the FIB onto the
@@ -1373,8 +1373,8 @@
 	printk(KERN_ERR "%s: Host adapter BLINK LED 0x%x\n", aac->name, BlinkLED);
 
 	if (!aac_check_reset ||
-		(aac->supplement_adapter_info.SupportedOptions2 &
-			le32_to_cpu(AAC_OPTION_IGNORE_RESET)))
+	    (aac->supplement_adapter_info.SupportedOptions2 &
+	     cpu_to_le32(AAC_OPTION_IGNORE_RESET)))
 		goto out;
 	host = aac->scsi_host_ptr;
 	if (aac->thread->pid != current->pid)
@@ -1655,11 +1655,11 @@
 				struct fib *fibptr;
 
 				if ((fibptr = aac_fib_alloc(dev))) {
-					u32 * info;
+					__le32 *info;
 
 					aac_fib_init(fibptr);
 
-					info = (u32 *) fib_data(fibptr);
+					info = (__le32 *) fib_data(fibptr);
 					if (now.tv_usec > 500000)
 						++now.tv_sec;
 
Index: linux-2.6/drivers/scsi/aacraid/dpcsup.c
===================================================================
--- linux-2.6.orig/drivers/scsi/aacraid/dpcsup.c	2007-11-08 17:09:50.000000000 +0100
+++ linux-2.6/drivers/scsi/aacraid/dpcsup.c	2007-11-08 17:14:43.000000000 +0100
@@ -229,11 +229,9 @@
  *	all QE there are and wake up all the waiters before exiting.
  */
 
-unsigned int aac_intr_normal(struct aac_dev * dev, u32 Index)
+unsigned int aac_intr_normal(struct aac_dev * dev, u32 index)
 {
-	u32 index = le32_to_cpu(Index);
-
-	dprintk((KERN_INFO "aac_intr_normal(%p,%x)\n", dev, Index));
+	dprintk((KERN_INFO "aac_intr_normal(%p,%x)\n", dev, index));
 	if ((index & 0x00000002L)) {
 		struct hw_fib * hw_fib;
 		struct fib * fib;
@@ -301,7 +299,7 @@
 
 		if (hwfib->header.Command == cpu_to_le16(NuFileSystem))
 		{
-			u32 *pstatus = (u32 *)hwfib->data;
+			__le32 *pstatus = (__le32 *)hwfib->data;
 			if (*pstatus & cpu_to_le32(0xffff0000))
 				*pstatus = cpu_to_le32(ST_OK);
 		}
Index: linux-2.6/drivers/scsi/aacraid/linit.c
===================================================================
--- linux-2.6.orig/drivers/scsi/aacraid/linit.c	2007-11-08 17:09:50.000000000 +0100
+++ linux-2.6/drivers/scsi/aacraid/linit.c	2007-11-08 17:14:43.000000000 +0100
@@ -584,8 +584,8 @@
 	 * support a register, instead of a commanded, reset.
 	 */
 	if ((aac->supplement_adapter_info.SupportedOptions2 &
-	  le32_to_cpu(AAC_OPTION_MU_RESET|AAC_OPTION_IGNORE_RESET)) ==
-	  le32_to_cpu(AAC_OPTION_MU_RESET))
+	     cpu_to_le32(AAC_OPTION_MU_RESET|AAC_OPTION_IGNORE_RESET)) ==
+	     cpu_to_le32(AAC_OPTION_MU_RESET))
 		aac_reset_adapter(aac, 2); /* Bypass wait for command quiesce */
 	return SUCCESS; /* Cause an immediate retry of the command with a ten second delay after successful tur */
 }
Index: linux-2.6/drivers/scsi/aacraid/rx.c
===================================================================
--- linux-2.6.orig/drivers/scsi/aacraid/rx.c	2007-11-08 17:09:50.000000000 +0100
+++ linux-2.6/drivers/scsi/aacraid/rx.c	2007-11-08 17:14:43.000000000 +0100
@@ -465,7 +465,7 @@
 	u32 var;
 
 	if (!(dev->supplement_adapter_info.SupportedOptions2 &
-	  le32_to_cpu(AAC_OPTION_MU_RESET)) || (bled >= 0) || (bled == -2)) {
+	    cpu_to_le32(AAC_OPTION_MU_RESET)) || (bled >= 0) || (bled == -2)) {
 		if (bled)
 			printk(KERN_ERR "%s%d: adapter kernel panic'd %x.\n",
 				dev->name, dev->id, bled);

RE: [PATCH 1/1] aacraid: don't assign cpu_to_le32(int) to u8

[Salyzyn, Mark]

Resounding ACK.

I just finished *exactly* the same set of changes, composed the patch
and was about to hit send when this one came over the wire from you!
There was absolutely no differences between our patches (save for the
fact I did not place the AIF ones in as they are already in the queue,
one is already on -mm).

I am going to return to this at some future date and figure out the
problems surrounding the context imbalances that are present, making
code that determines which context it is called from (sysfs, error
recovery or from the background thread) and plays with the various locks
confuses sparse. Rewriting so that the contexts are less programmatic is
in order...

Sincerely -- Mark Salyzyn

> -----Original Message-----
> From: Christoph Hellwig [mailto:hch@infradead.org] 
> Sent: Thursday, November 08, 2007 12:28 PM
> To: Salyzyn, Mark
> Cc: Christoph Hellwig; Andreas Schwab; Stephen Rothwell; 
> linux-scsi@vger.kernel.org; LKML
> Subject: Re: [PATCH 1/1] aacraid: don't assign cpu_to_le32(int) to u8
> 
> On Wed, Nov 07, 2007 at 01:51:44PM -0500, Salyzyn, Mark wrote:
> > Christoph Hellwig [mailto:hch@infradead.org] sez:
> > > Did anyone run the driver through sparse to see if we have 
> > > more issues like this?
> > 
> > There are some warnings from sparse, none like this one. I will deal
> > with the warnings ...
> 
> Actually there are a lot of endianess warnings, fortunately 
> most of them
> harmless.  The patch below fixes all of them up (including the ones in
> the patch I replied to), except for aac_init_adapter which is 
> really odd
> and I don't know what to do.
> 

[RFC] aacraid: driver feature flags?

[Salyzyn, Mark]

Feature enhancement.

Do we have a developing standard on how a driver may report to user tools via sysfs what features are supported or not by the driver? In the following I am proposing a 'flags' entry that will reside in the host controllers tree, with a newline separated list of arbitrary ascii named features that indicate whether the combination of driver and controller has support for said feature. Breaking from the one-line output typical of sysfs entries, newline was added to tailor for grep, or simple gets line by line string match within an application. I added one for a compiler time check for existence of debug print output, one for an optional manifest defined enhanced status reporting in the logs, and one for runtime reporting whether the controller and driver supports arrays larger than 2TB to my example below.

Suggestions?

This following patch is against current scsi-misc-2.6

ObligatoryDisclaimer: Please accept my condolences regarding Outlook's handling of patch attachments, so one can NOT use the following to patch, it is only present to demonstrate the idea.

 drivers/scsi/aacraid/linit.c |   25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff -ru a/drivers/scsi/aacraid/linit.c b/drivers/scsi/aacraid/linit.c
--- a/drivers/scsi/aacraid/linit.c      2008-01-11 11:19:11.378159940 -0500
+++ b/drivers/scsi/aacraid/linit.c      2008-01-11 11:23:19.040145945 -0500
@@ -788,6 +788,23 @@
        return len;
 }

+static ssize_t aac_show_flags(struct class_device *class_dev, char *buf)
+{
+       int len = 0;
+       struct aac_dev *dev = (struct aac_dev*)class_to_shost(class_dev)->hostdata;
+
+       if (nblank(dprintk(x)))
+               len = snprintf(buf, PAGE_SIZE, "dprintk\n");
+#      ifdef AAC_DETAILED_STATUS_INFO
+               len += snprintf(buf + len, PAGE_SIZE - len,
+                 "AAC_DETAILED_STATUS_INFO\n");
+#      endif
+       if (dev->raw_io_interface && dev->raw_io_64)
+               len += snprintf(buf + len, PAGE_SIZE - len,
+                 "SAI_READ_CAPACITY_16\n");
+       return len;
+}
+
 static ssize_t aac_show_kernel_version(struct class_device *class_dev,
                char *buf)
 {
@@ -897,6 +914,13 @@
        },
        .show = aac_show_vendor,
 };
+static struct class_device_attribute aac_flags = {
+       .attr = {
+               .name = "flags",
+               .mode = S_IRUGO,
+       },
+       .show = aac_show_flags,
+};
 static struct class_device_attribute aac_kernel_version = {
        .attr = {
                .name = "hba_kernel_version",
@@ -951,6 +975,7 @@
 static struct class_device_attribute *aac_attrs[] = {
        &aac_model,
        &aac_vendor,
+       &aac_flags,
        &aac_kernel_version,
        &aac_monitor_version,
        &aac_bios_version,