Re: [PATCH 05/37] ata: use get/put_endian helpers

[Andrew Morton <akpm@linux-foundation.org>]

On Thu, 29 May 2008 22:26:18 -0400 Mark Lord <liml@rtr.ca> wrote:

> Harvey Harrison wrote:
> > Signed-off-by: Harvey Harrison <harvey.harrison@gmail.com>
> > ---
> >  drivers/ata/pdc_adma.c   |   12 ++++++------
> >  drivers/ata/sata_qstor.c |   12 +++++-------
> >  2 files changed, 11 insertions(+), 13 deletions(-)
> > 
> > diff --git a/drivers/ata/pdc_adma.c b/drivers/ata/pdc_adma.c
> > index be53545..162eecb 100644
> > --- a/drivers/ata/pdc_adma.c
> > +++ b/drivers/ata/pdc_adma.c
> > @@ -284,11 +284,11 @@ static int adma_fill_sg(struct ata_queued_cmd *qc)
> >  		u32 len;
> >  
> >  		addr = (u32)sg_dma_address(sg);
> > -		*(__le32 *)(buf + i) = cpu_to_le32(addr);
> > +		put_le32(addr, (__le32 *)(buf + i));
> ..
> 
> I don't get it.
> 
> What's the point here?

Readability and maintainability.  Once one becomes familar with a
particular idion like this you can read it and say "ah, I know what
that's doing" rather than having to peer at every character and work it
out at each site where it happens.

As usual: a PITA now, but better long-term.

otoh,

- I think the args are backwards

- I don't like the use of the put_*() namespace.  It makes it look
  like a uaccess operation.