[U-Boot] [PATCH][v2] driver/ifc:Change accessor function to take care of endianness

Scott Wood scottwood at freescale.com
Mon Jan 20 23:51:03 CET 2014


On Sat, 2014-01-18 at 09:24 +0100, Wolfgang Denk wrote:
> Dear Prabhakar Kushwaha,
> 
> In message <1390028310-30861-1-git-send-email-prabhakar at freescale.com> you wrote:
> > IFC registers can be of type Little Endian or big Endian depending upon
> > Freescale SoC. Here SoC defines the register type of IFC IP.
> 
> As is, you are only adding dead code, as there is no place anywhere in
> the mainline code that defines CONFIG_SYS_FSL_IFC_LE

Yes, consider it RFC until we have patches for a target that needs LE.

> >  	/* Program ROW0/COL0 */
> > -	out_be32(&ifc->ifc_nand.row0, page_addr);
> > -	out_be32(&ifc->ifc_nand.col0, (oob ? IFC_NAND_COL_MS : 0) | column);
> > +	ifc_out32(&ifc->ifc_nand.row0, page_addr);
> > +	ifc_out32(&ifc->ifc_nand.col0, (oob ? IFC_NAND_COL_MS : 0) | column);
> 
> I seriously dislike the idea of introducing special I/O accessors for
> a single device driver.  If more drivers would follow that example, we
> will soon have a serious mess.

As the changelog says, we have chips coming out on which these registers
are little-endian, and thus we can't hardcode big-endian in the
driver.  

I don't know whether there's something more generic we could key off of
than IFC, but in Linux (and maybe U-Boot) we'll want to make this driven
by the device tree rather than at compile time (it's not as simple as
PPC versus ARM), so getting the knowledge from something other than the
IFC node would be awkward -- and it would be good to keep this code
similar between U-Boot and Linux.

What sort of mess are you envisioning?  This isn't implementing
accessors from scratch; it's just a wrapper.  It's local to IFC code.

-Scott




More information about the U-Boot mailing list