* [patch 12/17] ips.c: fix build warning
@ 2008-03-28 21:48 akpm
2008-03-30 17:07 ` James Bottomley
0 siblings, 1 reply; 5+ messages in thread
From: akpm @ 2008-03-28 21:48 UTC (permalink / raw)
To: James.Bottomley
Cc: linux-scsi, akpm, Mark_Salyzyn, aacraid, fujita.tomonori, jeff
From: Andrew Morton <akpm@linux-foundation.org>
powerpc:
drivers/scsi/ips.c: In function 'ips_issue_copperhead':
drivers/scsi/ips.c:5436: warning: large integer implicitly truncated to unsigned type
it doesn't seem necessary to do outw(cpu_to_le32(...), ...) anyway.
Acked-by: Mark Salyzyn <Mark_Salyzyn@adaptec.com>
Cc: James Bottomley <James.Bottomley@HansenPartnership.com>
Cc: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
Cc: Jeff Garzik <jeff@garzik.org>
Cc: <aacraid@adaptec.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---
drivers/scsi/ips.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff -puN drivers/scsi/ips.c~ipsc-fix-build-warning drivers/scsi/ips.c
--- a/drivers/scsi/ips.c~ipsc-fix-build-warning
+++ a/drivers/scsi/ips.c
@@ -5399,7 +5399,7 @@ ips_issue_copperhead(ips_ha_t * ha, ips_
} /* end while */
outl(cpu_to_le32(scb->scb_busaddr), ha->io_addr + IPS_REG_CCSAR);
- outw(cpu_to_le32(IPS_BIT_START_CMD), ha->io_addr + IPS_REG_CCCR);
+ outw(cpu_to_le16(IPS_BIT_START_CMD), ha->io_addr + IPS_REG_CCCR);
return (IPS_SUCCESS);
}
_
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [patch 12/17] ips.c: fix build warning
2008-03-28 21:48 [patch 12/17] ips.c: fix build warning akpm
@ 2008-03-30 17:07 ` James Bottomley
2008-03-30 23:57 ` FUJITA Tomonori
0 siblings, 1 reply; 5+ messages in thread
From: James Bottomley @ 2008-03-30 17:07 UTC (permalink / raw)
To: akpm; +Cc: linux-scsi, Mark_Salyzyn, aacraid, fujita.tomonori, jeff
On Fri, 2008-03-28 at 14:48 -0700, akpm@linux-foundation.org wrote:
> From: Andrew Morton <akpm@linux-foundation.org>
>
> powerpc:
>
> drivers/scsi/ips.c: In function 'ips_issue_copperhead':
> drivers/scsi/ips.c:5436: warning: large integer implicitly truncated to unsigned type
>
> it doesn't seem necessary to do outw(cpu_to_le32(...), ...) anyway.
Actually, it's not only not necessary ... it's an actual bug on BE
platforms. Both writeX and outX/inX automatically convert from native
to bus endian (usually simply to LE since the bus is PCI). Therefore if
you add a cpu_to_leX, you actually write the wrong value on a BE system.
Mark, it looks like the correct fix is simply to strip all of these ...
has this driver actually been tested on a BE platform (like parisc or
PPC)?
James
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [patch 12/17] ips.c: fix build warning
2008-03-30 17:07 ` James Bottomley
@ 2008-03-30 23:57 ` FUJITA Tomonori
2008-04-03 17:28 ` James Bottomley
0 siblings, 1 reply; 5+ messages in thread
From: FUJITA Tomonori @ 2008-03-30 23:57 UTC (permalink / raw)
To: James.Bottomley
Cc: akpm, linux-scsi, Mark_Salyzyn, aacraid, fujita.tomonori, jeff
On Sun, 30 Mar 2008 12:07:22 -0500
James Bottomley <James.Bottomley@HansenPartnership.com> wrote:
> On Fri, 2008-03-28 at 14:48 -0700, akpm@linux-foundation.org wrote:
> > From: Andrew Morton <akpm@linux-foundation.org>
> >
> > powerpc:
> >
> > drivers/scsi/ips.c: In function 'ips_issue_copperhead':
> > drivers/scsi/ips.c:5436: warning: large integer implicitly truncated to unsigned type
> >
> > it doesn't seem necessary to do outw(cpu_to_le32(...), ...) anyway.
>
> Actually, it's not only not necessary ... it's an actual bug on BE
> platforms. Both writeX and outX/inX automatically convert from native
> to bus endian (usually simply to LE since the bus is PCI). Therefore if
> you add a cpu_to_leX, you actually write the wrong value on a BE system.
>
> Mark, it looks like the correct fix is simply to strip all of these ...
> has this driver actually been tested on a BE platform (like parisc or
> PPC)?
I guess that it hasn't. ips.c includes:
#if !defined(__i386__) && !defined(__ia64__) && !defined(__x86_64__)
#warning "This driver has only been tested on the x86/ia64/x86_64 platforms"
#endif
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [patch 12/17] ips.c: fix build warning
2008-03-30 23:57 ` FUJITA Tomonori
@ 2008-04-03 17:28 ` James Bottomley
2008-04-04 18:38 ` Mark Salyzyn
0 siblings, 1 reply; 5+ messages in thread
From: James Bottomley @ 2008-04-03 17:28 UTC (permalink / raw)
To: FUJITA Tomonori; +Cc: akpm, linux-scsi, Mark_Salyzyn, aacraid, jeff
On Mon, 2008-03-31 at 08:57 +0900, FUJITA Tomonori wrote:
> On Sun, 30 Mar 2008 12:07:22 -0500
> James Bottomley <James.Bottomley@HansenPartnership.com> wrote:
>
> > On Fri, 2008-03-28 at 14:48 -0700, akpm@linux-foundation.org wrote:
> > > From: Andrew Morton <akpm@linux-foundation.org>
> > >
> > > powerpc:
> > >
> > > drivers/scsi/ips.c: In function 'ips_issue_copperhead':
> > > drivers/scsi/ips.c:5436: warning: large integer implicitly truncated to unsigned type
> > >
> > > it doesn't seem necessary to do outw(cpu_to_le32(...), ...) anyway.
> >
> > Actually, it's not only not necessary ... it's an actual bug on BE
> > platforms. Both writeX and outX/inX automatically convert from native
> > to bus endian (usually simply to LE since the bus is PCI). Therefore if
> > you add a cpu_to_leX, you actually write the wrong value on a BE system.
> >
> > Mark, it looks like the correct fix is simply to strip all of these ...
> > has this driver actually been tested on a BE platform (like parisc or
> > PPC)?
>
> I guess that it hasn't. ips.c includes:
>
> #if !defined(__i386__) && !defined(__ia64__) && !defined(__x86_64__)
> #warning "This driver has only been tested on the x86/ia64/x86_64 platforms"
> #endif
At least it only warns, it doesn't #error
OK, does this look like the roughly correct fix (I suppose there's no
chance of finding someone to actually test it on a BE platform?).
James
---
diff --git a/drivers/scsi/ips.c b/drivers/scsi/ips.c
index c450194..7c615c7 100644
--- a/drivers/scsi/ips.c
+++ b/drivers/scsi/ips.c
@@ -2377,7 +2377,7 @@ ips_get_bios_version(ips_ha_t * ha, int intr)
if (inb(ha->io_addr + IPS_REG_FLDP) != 0x55)
return;
- outl(cpu_to_le32(1), ha->io_addr + IPS_REG_FLAP);
+ outl(1, ha->io_addr + IPS_REG_FLAP);
if (ha->pcidev->revision == IPS_REVID_TROMBONE64)
udelay(25); /* 25 us */
@@ -2385,21 +2385,21 @@ ips_get_bios_version(ips_ha_t * ha, int intr)
return;
/* Get Major version */
- outl(cpu_to_le32(0x1FF), ha->io_addr + IPS_REG_FLAP);
+ outl(0x1FF, ha->io_addr + IPS_REG_FLAP);
if (ha->pcidev->revision == IPS_REVID_TROMBONE64)
udelay(25); /* 25 us */
major = inb(ha->io_addr + IPS_REG_FLDP);
/* Get Minor version */
- outl(cpu_to_le32(0x1FE), ha->io_addr + IPS_REG_FLAP);
+ outl(0x1FE, ha->io_addr + IPS_REG_FLAP);
if (ha->pcidev->revision == IPS_REVID_TROMBONE64)
udelay(25); /* 25 us */
minor = inb(ha->io_addr + IPS_REG_FLDP);
/* Get SubMinor version */
- outl(cpu_to_le32(0x1FD), ha->io_addr + IPS_REG_FLAP);
+ outl(0x1FD, ha->io_addr + IPS_REG_FLAP);
if (ha->pcidev->revision == IPS_REVID_TROMBONE64)
udelay(25); /* 25 us */
@@ -4852,7 +4852,7 @@ ips_init_copperhead(ips_ha_t * ha)
return (0);
/* setup CCCR */
- outl(cpu_to_le32(0x1010), ha->io_addr + IPS_REG_CCCR);
+ outl(0x1010, ha->io_addr + IPS_REG_CCCR);
/* Enable busmastering */
outb(IPS_BIT_EBM, ha->io_addr + IPS_REG_SCPR);
@@ -5234,12 +5234,12 @@ ips_statinit(ips_ha_t * ha)
ha->adapt->p_status_tail = ha->adapt->status;
phys_status_start = ha->adapt->hw_status_start;
- outl(cpu_to_le32(phys_status_start), ha->io_addr + IPS_REG_SQSR);
- outl(cpu_to_le32(phys_status_start + IPS_STATUS_Q_SIZE),
+ outl(phys_status_start, ha->io_addr + IPS_REG_SQSR);
+ outl(phys_status_start + IPS_STATUS_Q_SIZE,
ha->io_addr + IPS_REG_SQER);
- outl(cpu_to_le32(phys_status_start + IPS_STATUS_SIZE),
+ outl(phys_status_start + IPS_STATUS_SIZE,
ha->io_addr + IPS_REG_SQHR);
- outl(cpu_to_le32(phys_status_start), ha->io_addr + IPS_REG_SQTR);
+ outl(phys_status_start, ha->io_addr + IPS_REG_SQTR);
ha->adapt->hw_status_tail = phys_status_start;
}
@@ -5296,7 +5296,7 @@ ips_statupd_copperhead(ips_ha_t * ha)
ha->adapt->hw_status_tail = ha->adapt->hw_status_start;
}
- outl(cpu_to_le32(ha->adapt->hw_status_tail),
+ outl(ha->adapt->hw_status_tail,
ha->io_addr + IPS_REG_SQTR);
return (ha->adapt->p_status_tail->value);
@@ -5398,8 +5398,8 @@ ips_issue_copperhead(ips_ha_t * ha, ips_scb_t * scb)
} /* end if */
} /* end while */
- outl(cpu_to_le32(scb->scb_busaddr), ha->io_addr + IPS_REG_CCSAR);
- outw(cpu_to_le32(IPS_BIT_START_CMD), ha->io_addr + IPS_REG_CCCR);
+ outl(scb->scb_busaddr, ha->io_addr + IPS_REG_CCSAR);
+ outw(IPS_BIT_START_CMD, ha->io_addr + IPS_REG_CCCR);
return (IPS_SUCCESS);
}
@@ -5484,7 +5484,7 @@ ips_issue_i2o(ips_ha_t * ha, ips_scb_t * scb)
ips_name, ha->host_num, scb->cmd.basic_io.command_id);
}
- outl(cpu_to_le32(scb->scb_busaddr), ha->io_addr + IPS_REG_I2O_INMSGQ);
+ outl(scb->scb_busaddr, ha->io_addr + IPS_REG_I2O_INMSGQ);
return (IPS_SUCCESS);
}
@@ -6376,7 +6376,7 @@ ips_program_bios(ips_ha_t * ha, char *buffer, uint32_t buffersize,
for (i = 0; i < buffersize; i++) {
/* write a byte */
- outl(cpu_to_le32(i + offset), ha->io_addr + IPS_REG_FLAP);
+ outl(i + offset, ha->io_addr + IPS_REG_FLAP);
if (ha->pcidev->revision == IPS_REVID_TROMBONE64)
udelay(25); /* 25 us */
@@ -6561,7 +6561,7 @@ ips_verify_bios(ips_ha_t * ha, char *buffer, uint32_t buffersize,
if (inb(ha->io_addr + IPS_REG_FLDP) != 0x55)
return (1);
- outl(cpu_to_le32(1), ha->io_addr + IPS_REG_FLAP);
+ outl(1, ha->io_addr + IPS_REG_FLAP);
if (ha->pcidev->revision == IPS_REVID_TROMBONE64)
udelay(25); /* 25 us */
if (inb(ha->io_addr + IPS_REG_FLDP) != 0xAA)
@@ -6570,7 +6570,7 @@ ips_verify_bios(ips_ha_t * ha, char *buffer, uint32_t buffersize,
checksum = 0xff;
for (i = 2; i < buffersize; i++) {
- outl(cpu_to_le32(i + offset), ha->io_addr + IPS_REG_FLAP);
+ outl(i + offset, ha->io_addr + IPS_REG_FLAP);
if (ha->pcidev->revision == IPS_REVID_TROMBONE64)
udelay(25); /* 25 us */
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [patch 12/17] ips.c: fix build warning
2008-04-03 17:28 ` James Bottomley
@ 2008-04-04 18:38 ` Mark Salyzyn
0 siblings, 0 replies; 5+ messages in thread
From: Mark Salyzyn @ 2008-04-04 18:38 UTC (permalink / raw)
To: James Bottomley; +Cc: Linux-Scsi, Andrew Morton, Jeff Garzik
I was not handy to respond in a timely manner ...
Zero testing on BE machines for ips driver. Since IBM manufactures
this hardware, they are probably the only company interested in
testing it on their big PPC platforms...
Yes, this looks like what the doctor ordered! ACK ACK ACK
Sincerely -- Mark Salyzyn
On Apr 3, 2008, at 1:28 PM, James Bottomley wrote:
> On Mon, 2008-03-31 at 08:57 +0900, FUJITA Tomonori wrote:
>> On Sun, 30 Mar 2008 12:07:22 -0500
>> James Bottomley <James.Bottomley@HansenPartnership.com> wrote:
>>
>>> On Fri, 2008-03-28 at 14:48 -0700, akpm@linux-foundation.org wrote:
>>>> From: Andrew Morton <akpm@linux-foundation.org>
>>>>
>>>> powerpc:
>>>>
>>>> drivers/scsi/ips.c: In function 'ips_issue_copperhead':
>>>> drivers/scsi/ips.c:5436: warning: large integer implicitly
>>>> truncated to unsigned type
>>>>
>>>> it doesn't seem necessary to do outw(cpu_to_le32(...), ...) anyway.
>>>
>>> Actually, it's not only not necessary ... it's an actual bug on BE
>>> platforms. Both writeX and outX/inX automatically convert from
>>> native
>>> to bus endian (usually simply to LE since the bus is PCI).
>>> Therefore if
>>> you add a cpu_to_leX, you actually write the wrong value on a BE
>>> system.
>>>
>>> Mark, it looks like the correct fix is simply to strip all of
>>> these ...
>>> has this driver actually been tested on a BE platform (like parisc
>>> or
>>> PPC)?
>>
>> I guess that it hasn't. ips.c includes:
>>
>> #if !defined(__i386__) && !defined(__ia64__) && !defined(__x86_64__)
>> #warning "This driver has only been tested on the x86/ia64/x86_64
>> platforms"
>> #endif
>
> At least it only warns, it doesn't #error
>
> OK, does this look like the roughly correct fix (I suppose there's no
> chance of finding someone to actually test it on a BE platform?).
>
> James
>
> ---
>
> diff --git a/drivers/scsi/ips.c b/drivers/scsi/ips.c
> index c450194..7c615c7 100644
> --- a/drivers/scsi/ips.c
> +++ b/drivers/scsi/ips.c
> @@ -2377,7 +2377,7 @@ ips_get_bios_version(ips_ha_t * ha, int intr)
> if (inb(ha->io_addr + IPS_REG_FLDP) != 0x55)
> return;
>
> - outl(cpu_to_le32(1), ha->io_addr +
> IPS_REG_FLAP);
> + outl(1, ha->io_addr + IPS_REG_FLAP);
> if (ha->pcidev->revision ==
> IPS_REVID_TROMBONE64)
> udelay(25); /* 25 us */
>
> @@ -2385,21 +2385,21 @@ ips_get_bios_version(ips_ha_t * ha, int intr)
> return;
>
> /* Get Major version */
> - outl(cpu_to_le32(0x1FF), ha->io_addr +
> IPS_REG_FLAP);
> + outl(0x1FF, ha->io_addr + IPS_REG_FLAP);
> if (ha->pcidev->revision ==
> IPS_REVID_TROMBONE64)
> udelay(25); /* 25 us */
>
> major = inb(ha->io_addr + IPS_REG_FLDP);
>
> /* Get Minor version */
> - outl(cpu_to_le32(0x1FE), ha->io_addr +
> IPS_REG_FLAP);
> + outl(0x1FE, ha->io_addr + IPS_REG_FLAP);
> if (ha->pcidev->revision ==
> IPS_REVID_TROMBONE64)
> udelay(25); /* 25 us */
>
> minor = inb(ha->io_addr + IPS_REG_FLDP);
>
> /* Get SubMinor version */
> - outl(cpu_to_le32(0x1FD), ha->io_addr +
> IPS_REG_FLAP);
> + outl(0x1FD, ha->io_addr + IPS_REG_FLAP);
> if (ha->pcidev->revision ==
> IPS_REVID_TROMBONE64)
> udelay(25); /* 25 us */
>
> @@ -4852,7 +4852,7 @@ ips_init_copperhead(ips_ha_t * ha)
> return (0);
>
> /* setup CCCR */
> - outl(cpu_to_le32(0x1010), ha->io_addr + IPS_REG_CCCR);
> + outl(0x1010, ha->io_addr + IPS_REG_CCCR);
>
> /* Enable busmastering */
> outb(IPS_BIT_EBM, ha->io_addr + IPS_REG_SCPR);
> @@ -5234,12 +5234,12 @@ ips_statinit(ips_ha_t * ha)
> ha->adapt->p_status_tail = ha->adapt->status;
>
> phys_status_start = ha->adapt->hw_status_start;
> - outl(cpu_to_le32(phys_status_start), ha->io_addr +
> IPS_REG_SQSR);
> - outl(cpu_to_le32(phys_status_start + IPS_STATUS_Q_SIZE),
> + outl(phys_status_start, ha->io_addr + IPS_REG_SQSR);
> + outl(phys_status_start + IPS_STATUS_Q_SIZE,
> ha->io_addr + IPS_REG_SQER);
> - outl(cpu_to_le32(phys_status_start + IPS_STATUS_SIZE),
> + outl(phys_status_start + IPS_STATUS_SIZE,
> ha->io_addr + IPS_REG_SQHR);
> - outl(cpu_to_le32(phys_status_start), ha->io_addr +
> IPS_REG_SQTR);
> + outl(phys_status_start, ha->io_addr + IPS_REG_SQTR);
>
> ha->adapt->hw_status_tail = phys_status_start;
> }
> @@ -5296,7 +5296,7 @@ ips_statupd_copperhead(ips_ha_t * ha)
> ha->adapt->hw_status_tail = ha->adapt->hw_status_start;
> }
>
> - outl(cpu_to_le32(ha->adapt->hw_status_tail),
> + outl(ha->adapt->hw_status_tail,
> ha->io_addr + IPS_REG_SQTR);
>
> return (ha->adapt->p_status_tail->value);
> @@ -5398,8 +5398,8 @@ ips_issue_copperhead(ips_ha_t * ha, ips_scb_t
> * scb)
> } /* end if */
> } /* end while */
>
> - outl(cpu_to_le32(scb->scb_busaddr), ha->io_addr +
> IPS_REG_CCSAR);
> - outw(cpu_to_le32(IPS_BIT_START_CMD), ha->io_addr +
> IPS_REG_CCCR);
> + outl(scb->scb_busaddr, ha->io_addr + IPS_REG_CCSAR);
> + outw(IPS_BIT_START_CMD, ha->io_addr + IPS_REG_CCCR);
>
> return (IPS_SUCCESS);
> }
> @@ -5484,7 +5484,7 @@ ips_issue_i2o(ips_ha_t * ha, ips_scb_t * scb)
> ips_name, ha->host_num, scb-
> >cmd.basic_io.command_id);
> }
>
> - outl(cpu_to_le32(scb->scb_busaddr), ha->io_addr +
> IPS_REG_I2O_INMSGQ);
> + outl(scb->scb_busaddr, ha->io_addr + IPS_REG_I2O_INMSGQ);
>
> return (IPS_SUCCESS);
> }
> @@ -6376,7 +6376,7 @@ ips_program_bios(ips_ha_t * ha, char *buffer,
> uint32_t buffersize,
>
> for (i = 0; i < buffersize; i++) {
> /* write a byte */
> - outl(cpu_to_le32(i + offset), ha->io_addr +
> IPS_REG_FLAP);
> + outl(i + offset, ha->io_addr + IPS_REG_FLAP);
> if (ha->pcidev->revision == IPS_REVID_TROMBONE64)
> udelay(25); /* 25 us */
>
> @@ -6561,7 +6561,7 @@ ips_verify_bios(ips_ha_t * ha, char *buffer,
> uint32_t buffersize,
> if (inb(ha->io_addr + IPS_REG_FLDP) != 0x55)
> return (1);
>
> - outl(cpu_to_le32(1), ha->io_addr + IPS_REG_FLAP);
> + outl(1, ha->io_addr + IPS_REG_FLAP);
> if (ha->pcidev->revision == IPS_REVID_TROMBONE64)
> udelay(25); /* 25 us */
> if (inb(ha->io_addr + IPS_REG_FLDP) != 0xAA)
> @@ -6570,7 +6570,7 @@ ips_verify_bios(ips_ha_t * ha, char *buffer,
> uint32_t buffersize,
> checksum = 0xff;
> for (i = 2; i < buffersize; i++) {
>
> - outl(cpu_to_le32(i + offset), ha->io_addr +
> IPS_REG_FLAP);
> + outl(i + offset, ha->io_addr + IPS_REG_FLAP);
> if (ha->pcidev->revision == IPS_REVID_TROMBONE64)
> udelay(25); /* 25 us */
>
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2008-04-04 18:38 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-03-28 21:48 [patch 12/17] ips.c: fix build warning akpm
2008-03-30 17:07 ` James Bottomley
2008-03-30 23:57 ` FUJITA Tomonori
2008-04-03 17:28 ` James Bottomley
2008-04-04 18:38 ` Mark Salyzyn
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox