RE: [PATCH 2.6] cciss typo fix

RE: [PATCH 2.6] cciss typo fix

[Miller, Mike (OS Dev)]

> -----Original Message-----
> From: James Bottomley [mailto:James.Bottomley@SteelEye.com]
> 
> 
> On Fri, 2005-01-07 at 17:01 -0600, mike.miller@hp.com wrote:
> > -		*total_size = be32_to_cpu(*((__be32 *) 
> &buf->total_size[0]))+1;
> > -		*block_size = be32_to_cpu(*((__be32 *) 
> &buf->block_size[0]));
> > +		*total_size = be32_to_cpu(*((__u32 *) 
> &buf->total_size[0]))+1;
> > +		*block_size = be32_to_cpu(*((__u32 *) 
> &buf->block_size[0]));
> 
> I don't think that's a typo.  It was introduced by this patch:
> 
> ChangeSet 1.1988.24.79 2004/10/06 07:55:02 viro@www.linux.org.uk
>   [PATCH] cciss endianness and iomem annotations
>  
> The idea being that BE and LE numbers should be annotated differently,
> so the __be32 annotations look correct to me.  I think sparse 
> will warn
> if you make this change.

Hmmm, SuSE complained that __be32 was not defined in the kernel. Any other thoughts, anyone?

mikem

RE: [PATCH 2.6] cciss typo fix

[James Bottomley]

On Fri, 2005-01-07 at 17:24 -0600, Miller, Mike (OS Dev) wrote:
> Hmmm, SuSE complained that __be32 was not defined in the kernel. Any other thoughts, anyone?

Erm, well it might not be defined in the SuSE kernel ... that's based on
2.6.5, I believe.  These symbols are definitely defined in the mainline
kernel.

James

Re: [PATCH 2.6] cciss typo fix

[Jens Axboe]

On Fri, Jan 07 2005, Miller, Mike (OS Dev) wrote:
> > -----Original Message-----
> > From: James Bottomley [mailto:James.Bottomley@SteelEye.com]
> > 
> > 
> > On Fri, 2005-01-07 at 17:01 -0600, mike.miller@hp.com wrote:
> > > -		*total_size = be32_to_cpu(*((__be32 *) 
> > &buf->total_size[0]))+1;
> > > -		*block_size = be32_to_cpu(*((__be32 *) 
> > &buf->block_size[0]));
> > > +		*total_size = be32_to_cpu(*((__u32 *) 
> > &buf->total_size[0]))+1;
> > > +		*block_size = be32_to_cpu(*((__u32 *) 
> > &buf->block_size[0]));
> > 
> > I don't think that's a typo.  It was introduced by this patch:
> > 
> > ChangeSet 1.1988.24.79 2004/10/06 07:55:02 viro@www.linux.org.uk
> >   [PATCH] cciss endianness and iomem annotations
> >  
> > The idea being that BE and LE numbers should be annotated differently,
> > so the __be32 annotations look correct to me.  I think sparse 
> > will warn
> > if you make this change.
> 
> Hmmm, SuSE complained that __be32 was not defined in the kernel. Any
> other thoughts, anyone?

Hmm odd, no one should have complained, it should just have been added
to the compat header.

-- 
Jens Axboe

RE: [PATCH 2.6] cciss typo fix

[James Bottomley]

On Mon, 2005-01-10 at 09:45 -0600, Miller, Mike (OS Dev) wrote:
> Even if it were added to the compat header; is using __be32 correct in this context?

They should be.  The __be annotations track unconverted big endian
numbers.  be32_to_cpu checks that it's only taking __be annotated
variables (at least when sparse tracks it), so the cast is correct and
prevents sparse from warning.

James

RE: [PATCH 2.6] cciss typo fix

[Miller, Mike (OS Dev)]

> > > -		*block_size = be32_to_cpu(*((__be32 *) 
> > &buf->block_size[0]));
> > > +		*total_size = be32_to_cpu(*((__u32 *) 
> > &buf->total_size[0]))+1;
>


> From: Jens Axboe [mailto:axboe@suse.de]
> Hmm odd, no one should have complained, it should just have been added
> to the compat header.

Even if it were added to the compat header; is using __be32 correct in this context?

mikem

RE: [PATCH 2.6] cciss typo fix

[Miller, Mike (OS Dev)]

> -----Original Message-----
> From: James Bottomley [mailto:James.Bottomley@SteelEye.com]
> Erm, well it might not be defined in the SuSE kernel ... 
> that's based on
> 2.6.5, I believe.  These symbols are definitely defined in 
> the mainline
> kernel.

OK, then it looks like this isn't needed.

Re: [PATCH 2.6] cciss typo fix

[James Bottomley]

On Fri, 2005-01-07 at 17:01 -0600, mike.miller@hp.com wrote:
> -		*total_size = be32_to_cpu(*((__be32 *) &buf->total_size[0]))+1;
> -		*block_size = be32_to_cpu(*((__be32 *) &buf->block_size[0]));
> +		*total_size = be32_to_cpu(*((__u32 *) &buf->total_size[0]))+1;
> +		*block_size = be32_to_cpu(*((__u32 *) &buf->block_size[0]));

I don't think that's a typo.  It was introduced by this patch:

ChangeSet 1.1988.24.79 2004/10/06 07:55:02 viro@www.linux.org.uk
  [PATCH] cciss endianness and iomem annotations
 
The idea being that BE and LE numbers should be annotated differently,
so the __be32 annotations look correct to me.  I think sparse will warn
if you make this change.

James

[PATCH 2.6] cciss typo fix

[mike.miller]

This patch fixes a stupid typo. I thought I submitted this but I don't seem to
find it anywhere.

 cciss.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

Signed-off-by: Mike Miller <mike.miller@hp.com>
-------------------------------------------------------------------------------
diff -burNp lx2610-p001/drivers/block/cciss.c lx2610/drivers/block/cciss.c
--- lx2610-p001/drivers/block/cciss.c	2005-01-07 15:29:29.645792288 -0600
+++ lx2610/drivers/block/cciss.c	2005-01-07 15:29:59.490255240 -0600
@@ -1505,8 +1505,8 @@ cciss_read_capacity(int ctlr, int logvol
 		return_code = sendcmd(CCISS_READ_CAPACITY,
 			ctlr, buf, sizeof(*buf), 1, logvol, 0, NULL, TYPE_CMD);
 	if (return_code == IO_OK) {
-		*total_size = be32_to_cpu(*((__be32 *) &buf->total_size[0]))+1;
-		*block_size = be32_to_cpu(*((__be32 *) &buf->block_size[0]));
+		*total_size = be32_to_cpu(*((__u32 *) &buf->total_size[0]))+1;
+		*block_size = be32_to_cpu(*((__u32 *) &buf->block_size[0]));
 	} else { /* read capacity command failed */
 		printk(KERN_WARNING "cciss: read capacity failed\n");
 		*total_size = 0;