Re: in_be32() etc

[Benjamin Herrenschmidt <benh@kernel.crashing.org>]

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 ? :-)
 
> 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.

Cheers,
Ben.