From: Randy Dunlap <[email protected]>
Fix printk format warnings:
drivers/block/cciss.c:2000: warning: long long int format, long unsigned int arg (arg 2)
drivers/block/cciss.c:2035: warning: long long int format, long unsigned int arg (arg 2)
Signed-off-by: Randy Dunlap <[email protected]>
---
drivers/block/cciss.c | 8 ++++----
1 files changed, 4 insertions(+), 4 deletions(-)
--- linux-2619-rc3-pv.orig/drivers/block/cciss.c
+++ linux-2619-rc3-pv/drivers/block/cciss.c
@@ -1992,8 +1992,8 @@ cciss_read_capacity(int ctlr, int logvol
*block_size = BLOCK_SIZE;
}
if (*total_size != (__u32) 0)
- printk(KERN_INFO " blocks= %lld block_size= %d\n",
- *total_size, *block_size);
+ printk(KERN_INFO " blocks= %llu block_size= %d\n",
+ (unsigned long long)*total_size, *block_size);
kfree(buf);
return;
}
@@ -2027,8 +2027,8 @@ cciss_read_capacity_16(int ctlr, int log
*total_size = 0;
*block_size = BLOCK_SIZE;
}
- printk(KERN_INFO " blocks= %lld block_size= %d\n",
- *total_size, *block_size);
+ printk(KERN_INFO " blocks= %llu block_size= %d\n",
+ (unsigned long long)*total_size, *block_size);
kfree(buf);
return;
}
---
> -----Original Message-----
> From: Randy Dunlap [mailto:[email protected]]
> Sent: Monday, October 23, 2006 11:46 PM
> To: ISS StorageDev; lkml
> Cc: akpm
> Subject: [PATCH cciss: fix printk format warning
>
> From: Randy Dunlap <[email protected]>
>
> Fix printk format warnings:
> drivers/block/cciss.c:2000: warning: long long int format,
> long unsigned int arg (arg 2)
> drivers/block/cciss.c:2035: warning: long long int format,
> long unsigned int arg (arg 2)
>
> Signed-off-by: Randy Dunlap <[email protected]>
Acked-by: Mike Miller <[email protected]>
> ---
>
> drivers/block/cciss.c | 8 ++++----
> 1 files changed, 4 insertions(+), 4 deletions(-)
>
> --- linux-2619-rc3-pv.orig/drivers/block/cciss.c
> +++ linux-2619-rc3-pv/drivers/block/cciss.c
> @@ -1992,8 +1992,8 @@ cciss_read_capacity(int ctlr, int logvol
> *block_size = BLOCK_SIZE;
> }
> if (*total_size != (__u32) 0)
> - printk(KERN_INFO " blocks= %lld block_size= %d\n",
> - *total_size, *block_size);
> + printk(KERN_INFO " blocks= %llu block_size= %d\n",
> + (unsigned long long)*total_size, *block_size);
> kfree(buf);
> return;
> }
> @@ -2027,8 +2027,8 @@ cciss_read_capacity_16(int ctlr, int log
> *total_size = 0;
> *block_size = BLOCK_SIZE;
> }
> - printk(KERN_INFO " blocks= %lld block_size= %d\n",
> - *total_size, *block_size);
> + printk(KERN_INFO " blocks= %llu block_size= %d\n",
> + (unsigned long long)*total_size, *block_size);
> kfree(buf);
> return;
> }
>
>
> ---
>
On Mon, 23 Oct 2006 21:46:08 -0700
Randy Dunlap <[email protected]> wrote:
> --- linux-2619-rc3-pv.orig/drivers/block/cciss.c
> +++ linux-2619-rc3-pv/drivers/block/cciss.c
> @@ -1992,8 +1992,8 @@ cciss_read_capacity(int ctlr, int logvol
> *block_size = BLOCK_SIZE;
> }
> if (*total_size != (__u32) 0)
Why is cciss_read_capacity casting *total_size to u32?
It is a sector_t. If it happens to have the value 0xnnnnnnnn00000000 then
this expression will incorrectly evaluate to `false'.
> > if (*total_size != (__u32) 0)
>
> Why is cciss_read_capacity casting *total_size to u32?
It's not -- it's actually casting 0 to __32 -- there's no cast on the
*total_size side of the comparison. However that just makes the cast
look even fishier.
- R.
Roland Dreier wrote:
> > > if (*total_size != (__u32) 0)
> >
> > Why is cciss_read_capacity casting *total_size to u32?
>
> It's not -- it's actually casting 0 to __32 -- there's no cast on the
> *total_size side of the comparison. However that just makes the cast
> look even fishier.
>
> - R.
OK, how about this one then?
c->busaddr = (__u32) cmd_dma_handle;
where cmd_dma_handle is a dma_addr_t (u32 or u64)
and then later:
pci_free_consistent(h->pdev, sizeof(CommandList_struct),
c, (dma_addr_t) c->busaddr);
--
~Randy
On Thu, 26 Oct 2006 16:19:56 -0700
Roland Dreier <[email protected]> wrote:
> > > if (*total_size != (__u32) 0)
> >
> > Why is cciss_read_capacity casting *total_size to u32?
>
> It's not -- it's actually casting 0 to __32
bah.
> -- there's no cast on the
> *total_size side of the comparison. However that just makes the cast
> look even fishier.
yup, it's harmless. Just something which was put in there to entertain passers-by.
Randy Dunlap wrote:
> Roland Dreier wrote:
>> > > if (*total_size != (__u32) 0)
>> > > Why is cciss_read_capacity casting *total_size to u32?
>>
>> It's not -- it's actually casting 0 to __32 -- there's no cast on the
>> *total_size side of the comparison. However that just makes the cast
>> look even fishier.
>>
>> - R.
>
> OK, how about this one then?
>
>
> c->busaddr = (__u32) cmd_dma_handle;
>
> where cmd_dma_handle is a dma_addr_t (u32 or u64)
>
> and then later:
>
> pci_free_consistent(h->pdev, sizeof(CommandList_struct),
> c, (dma_addr_t) c->busaddr);
>
One problem with this one is that it looks like the hardware
wants a 32-bit value for busaddr:
cciss.h: writel(c->busaddr, h->vaddr + SA5_REQUEST_PORT_OFFSET);
--
~Randy
> OK, how about this one then?
>
> c->busaddr = (__u32) cmd_dma_handle;
>
> where cmd_dma_handle is a dma_addr_t (u32 or u64)
It's super-fishy looking but actually I think it's OK, at least as
things stand now. As you see later from how it's freed:
> pci_free_consistent(h->pdev, sizeof(CommandList_struct),
> c, (dma_addr_t) c->busaddr);
this is the bus address of memory from pci_alloc_consistent(), and
since cciss never does pci_set_consistent_dma_mask(), the mask will
remain at the default 32 bits. So the driver is actually safe in
assuming that cmd_dma_handle fits into 32 bits. assuming that
cmd_dma_handle.
- R.
Roland Dreier wrote:
> > OK, how about this one then?
> >
> > c->busaddr = (__u32) cmd_dma_handle;
> >
> > where cmd_dma_handle is a dma_addr_t (u32 or u64)
>
> It's super-fishy looking but actually I think it's OK, at least as
> things stand now. As you see later from how it's freed:
>
> > pci_free_consistent(h->pdev, sizeof(CommandList_struct),
> > c, (dma_addr_t) c->busaddr);
>
> this is the bus address of memory from pci_alloc_consistent(), and
> since cciss never does pci_set_consistent_dma_mask(), the mask will
> remain at the default 32 bits. So the driver is actually safe in
> assuming that cmd_dma_handle fits into 32 bits. assuming that
> cmd_dma_handle.
Hm, OK. Thanks.
--
~Randy
> Roland Dreier wrote:
> > > > if (*total_size != (__u32) 0)
> > >
> > > Why is cciss_read_capacity casting *total_size to u32?
> >
> > It's not -- it's actually casting 0 to __32 -- there's no cast on the
> > *total_size side of the comparison. However that just makes the cast
> > look even fishier.
> >
> > - R.
>
> OK, how about this one then?
>
>
> c->busaddr = (__u32) cmd_dma_handle;
>
> where cmd_dma_handle is a dma_addr_t (u32 or u64)
The command register to which that value is written
is a 32 bit register. Cast it or not, only 32 bits
will be used. The DMA mask used to get that memory
should ensure it's 32 bit addressable.
> and then later:
>
> pci_free_consistent(h->pdev, sizeof(CommandList_struct),
> c, (dma_addr_t) c->busaddr);
On Fri, 27 Oct 2006 08:11:46 -0500 Cameron, Steve wrote:
> > Roland Dreier wrote:
> > > > > if (*total_size != (__u32) 0)
> > > >
> > > > Why is cciss_read_capacity casting *total_size to u32?
> > >
> > > It's not -- it's actually casting 0 to __32 -- there's no cast on the
> > > *total_size side of the comparison. However that just makes the cast
> > > look even fishier.
> > >
> > > - R.
> >
> > OK, how about this one then?
> >
> >
> > c->busaddr = (__u32) cmd_dma_handle;
> >
> > where cmd_dma_handle is a dma_addr_t (u32 or u64)
>
> The command register to which that value is written
> is a 32 bit register. Cast it or not, only 32 bits
> will be used. The DMA mask used to get that memory
> should ensure it's 32 bit addressable.
Got it. Thanks for replying.
> > and then later:
> >
> > pci_free_consistent(h->pdev, sizeof(CommandList_struct),
> > c, (dma_addr_t) c->busaddr);
---
~Randy
> -----Original Message-----
> From: Cameron, Steve
> Sent: Friday, October 27, 2006 8:12 AM
> To: Randy Dunlap; Roland Dreier
> Cc: Andrew Morton; ISS StorageDev; lkml
> Subject: RE: [PATCH cciss: fix printk format warning
>
> > Roland Dreier wrote:
> > > > > if (*total_size != (__u32) 0)
> > > >
> > > > Why is cciss_read_capacity casting *total_size to u32?
> > >
> > > It's not -- it's actually casting 0 to __32 -- there's no cast on
> > > the *total_size side of the comparison. However that
> just makes the
> > > cast look even fishier.
If the volume is >2TB read_capacity will return 8 F's. We've already
added 1 to total_size which equals 0. I only care if the lower 32 bits
are zero so that's the reason for the cast.
Does that make sense or am I out in the weeds?
mikem
> > >
> > > - R.
> >
> > OK, how about this one then?
> >
> >
> > c->busaddr = (__u32) cmd_dma_handle;
> >
> > where cmd_dma_handle is a dma_addr_t (u32 or u64)
>
> The command register to which that value is written is a 32
> bit register. Cast it or not, only 32 bits will be used.
> The DMA mask used to get that memory should ensure it's 32
> bit addressable.
>
> > and then later:
> >
> > pci_free_consistent(h->pdev, sizeof(CommandList_struct),
> > c, (dma_addr_t) c->busaddr);
>
>
>
>
>