Re: in_be32() etc

[Russell King - ARM Linux <linux@arm.linux.org.uk>]

On Fri, Feb 24, 2012 at 07:18:59AM +1100, Benjamin Herrenschmidt wrote:
> On Thu, 2012-02-23 at 11:29 +0000, Russell King - ARM Linux wrote:
> > What's this stuff doing in generic drivers?
> 
> Well, I suppose that's because the xilinx stuff used to be ppc
> only ? :-)

Note that's just the first one grep turned up.  I've no idea which specific
drivers the ST SPEAr people are wanting these accessors for (they didn't
include that information in their submission adding them to ARM.)

> > See drivers/gpio/gpio-xilinx.c:
> > static int xgpio_get(struct gpio_chip *gc, unsigned int gpio)
> > {
> >         struct of_mm_gpio_chip *mm_gc = to_of_mm_gpio_chip(gc);
> > 
> >         return (in_be32(mm_gc->regs + XGPIO_DATA_OFFSET) >> gpio) & 1;
> > }
> > 
> > include/linux/of_gpio.h:
> > struct of_mm_gpio_chip {
> >         struct gpio_chip gc;
> >         void (*save_regs)(struct of_mm_gpio_chip *mm_gc);
> >         void __iomem *regs;
> > };
> > 
> > Why am I being asked to add in_be32() etc to ARMs io.h ?  Why do we need
> > yet another set of IO accessors?  Is there something wrong with
> > ioread*()/ioread*be() etc?
> 
> Nope, nothing wrong with them, the driver should be fixed. in_be* is
> historical ppc stuff.
> 
> > My guess is this stems from a lack of proper review
> 
> That or history. Our readX/writeX used to be more PCI specific (have
> infrastructure to work around PCI bridge bugs) which some drivers
> avoided using the in_/out_ variants, in some case it's just pure
> history, etc... Some of these things are ancient.

So, if I tell the SPEAr people that any driver they come across using
these old in_XX should be converted to use ioread*() you'll be happy
for that to happen?