in_be32() etc

[Grant Likely]

On Thu, Feb 23, 2012 at 1:25 PM, Russell King - ARM Linux
<linux at arm.linux.org.uk> wrote:
> 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?

yes

g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.