* [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