All of lore.kernel.org
 help / color / mirror / Atom feed
From: Harvey Harrison <harvey.harrison@gmail.com>
To: Christoph Hellwig <hch@infradead.org>
Cc: Ben Hutchings <bhutchings@solarflare.com>,
	Ron Mercer <ron.mercer@qlogic.com>,
	netdev@vger.kernel.org
Subject: Re: [PATCH 3/5] qlge: bugfix: Fix endian issue regarding shadow registers.
Date: Wed, 24 Dec 2008 13:57:07 -0800	[thread overview]
Message-ID: <1230155827.1447.10.camel@brick> (raw)
In-Reply-To: <20081224211521.GA25617@infradead.org>

On Wed, 2008-12-24 at 16:15 -0500, Christoph Hellwig wrote:
> On Wed, Dec 24, 2008 at 06:35:31PM +0000, Ben Hutchings wrote:
> > > -static inline unsigned int ql_read_sh_reg(const volatile void  *addr)
> > > +static inline u32 ql_read_sh_reg(const volatile void  *addr)
> > >  {
> > > -	return *(volatile unsigned int __force *)addr;
> > > +	return le32_to_cpu(*(volatile unsigned int __force *)addr);
> > 
> > I think that should be:
> > 	???return le32_to_cpu(*(const volatile __le32 *)addr);
> 
> No.  It needs more love than that.
> 
> ql_read_sh_reg is only used to access .prod_idx_sh_reg in the rx_ring
> structure, which is an offset from the .rx_ring_shadow_reg_area member.
> So instead of all this werid casting .prod_idx_sh_reg should simply
> be a __le32 pointer, whithout volatile and the other mess, and then
> you can use le32_to_cpu directly and kill the helper.
> 
> And please verify the whole driver for endianess issues using sparse
> while you're at it...

I've already deleted the patches from my inbox, but if you send them
again off-list I'll have a look as well.

Harvey


  parent reply	other threads:[~2008-12-24 21:57 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-12-24 18:18 [PATCH 0/5] qlge: Fixes for powerpc platform Ron Mercer
2008-12-24 18:21 ` [PATCH 1/5] qlge: bugfux: Add missing pci_mapping_err checking Ron Mercer
2008-12-24 18:21 ` [PATCH 2/5] qlge: bugfix: Add missing pci_unmap_page call in receive path Ron Mercer
2008-12-24 18:21 ` [PATCH 3/5] qlge: bugfix: Fix endian issue regarding shadow registers Ron Mercer
2008-12-24 18:35   ` Ben Hutchings
2008-12-24 18:42     ` Ron Mercer
2008-12-24 21:15     ` Christoph Hellwig
2008-12-24 21:29       ` Ron Mercer
2008-12-24 21:57       ` Harvey Harrison [this message]
2008-12-24 18:21 ` [PATCH 4/5] qlge: bugfix: Fix rx ring and large and small rx buffer length settings Ron Mercer
2008-12-24 18:21 ` [PATCH 5/5] qlge: bugfix: Fix register access error checking Ron Mercer

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1230155827.1447.10.camel@brick \
    --to=harvey.harrison@gmail.com \
    --cc=bhutchings@solarflare.com \
    --cc=hch@infradead.org \
    --cc=netdev@vger.kernel.org \
    --cc=ron.mercer@qlogic.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.