2007-10-31 04:02:26

by Stephen Rothwell

[permalink] [raw]
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 <[email protected]>
---
drivers/scsi/aacraid/commsup.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)

--
Cheers,
Stephen Rothwell [email protected]

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


2007-10-31 13:54:20

by Mark Salyzyn

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

ACK

Sincerely -- Mark Salyzyn

> -----Original Message-----
> From: Stephen Rothwell [mailto:[email protected]]
> Sent: Wednesday, October 31, 2007 12:02 AM
> To: AACRAID
> Cc: [email protected]; 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 <[email protected]>
> ---
> drivers/scsi/aacraid/commsup.c | 6 +++---
> 1 files changed, 3 insertions(+), 3 deletions(-)
>
> --
> Cheers,
> Stephen Rothwell [email protected]
>
> 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
>

2007-11-01 06:32:30

by Stephen Rothwell

[permalink] [raw]
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 <[email protected]>
---
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 [email protected]

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

2007-11-01 13:11:35

by Mark Salyzyn

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

ACK v2

Sincerely -- Mark Salyzyn

> -----Original Message-----
> From: Stephen Rothwell [mailto:[email protected]]
> Sent: Thursday, November 01, 2007 2:32 AM
> To: AACRAID
> Cc: [email protected]; 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 <[email protected]>
> ---
> 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 [email protected]
>
> 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
>

2007-11-01 13:31:42

by Andreas Schwab

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

Stephen Rothwell <[email protected]> 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, [email protected]
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."

2007-11-07 15:58:25

by Mark Salyzyn

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

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 <[email protected]>

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:[email protected]]
> Sent: Thursday, November 01, 2007 9:31 AM
> To: Stephen Rothwell
> Cc: AACRAID; [email protected]; LKML
> Subject: Re: [PATCHv2] aacraid: don't assign
> cpu_to_le32(constant) to u8
>
> Stephen Rothwell <[email protected]> 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.


Attachments:
aacraid_BlinkLED.patch (461.00 B)
aacraid_BlinkLED.patch

2007-11-07 17:33:29

by Christoph Hellwig

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

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?

2007-11-07 18:51:57

by Mark Salyzyn

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

Christoph Hellwig [mailto:[email protected]] 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

2007-11-08 17:28:13

by Christoph Hellwig

[permalink] [raw]
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:[email protected]] 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 <[email protected]>

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

2007-11-08 18:09:42

by Mark Salyzyn

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

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:[email protected]]
> Sent: Thursday, November 08, 2007 12:28 PM
> To: Salyzyn, Mark
> Cc: Christoph Hellwig; Andreas Schwab; Stephen Rothwell;
> [email protected]; 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:[email protected]] 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.
>