[PATCH 1/1] drivers/gpio: Altera soft IP GPIO driver

[Linus Walleij]

On Mon, Jun 17, 2013 at 9:29 AM, Loh Tien Hock <thloh at altera.com> wrote:
> On Mon, Jun 17, 2013 at 2:38 PM, Linus Walleij <linus.walleij at linaro.org> wrote:
>> On Thu, Jun 6, 2013 at 10:05 AM,  <thloh at altera.com> wrote:
>>> +static void altera_gpio_irq_handler(unsigned int irq, struct irq_desc *desc)
>>> +{
>>> +       struct altera_gpio_chip *altera_gc = irq_desc_get_handler_data(desc);
>>> +       struct irq_chip *chip = irq_desc_get_chip(desc);
>>> +       struct of_mm_gpio_chip *mm_gc = &altera_gc->mmchip;
>>> +       unsigned long status;
>>> +
>>> +       int base;
>>> +       chip->irq_mask(&desc->irq_data);
>>> +
>>> +       if (altera_gc->level_trigger) {
>>> +               status = __raw_readl(mm_gc->regs + ALTERA_GPIO_DATA);
>>> +               status &= __raw_readl(mm_gc->regs + ALTERA_GPIO_IRQ_MASK);
>>> +
>>> +               for (base = 0; base < mm_gc->gc.ngpio; base++) {
>>> +                       if ((1 << base) & status) {
>>> +                               generic_handle_irq(irq_linear_revmap(
>>> +                                       altera_gc->irq, base));
>>> +                       }
>>> +               }
>>> +       } else {
>>> +               while ((status =
>>> +                       (__raw_readl(mm_gc->regs + ALTERA_GPIO_EDGE_CAP) &
>>> +                       __raw_readl(mm_gc->regs + ALTERA_GPIO_IRQ_MASK)))) {
>>> +                       __raw_writel(status,
>>> +                               mm_gc->regs + ALTERA_GPIO_EDGE_CAP);
>>> +                       for (base = 0; base < mm_gc->gc.ngpio; base++) {
>>> +                               if ((1 << base) & status) {
>>> +                                       generic_handle_irq(irq_linear_revmap(
>>> +                                               altera_gc->irq, base));
>>> +                               }
>>> +                       }
>>> +               }
>>> +       }
>>> +
>>> +       chip->irq_eoi(irq_desc_get_irq_data(desc));
>>> +       chip->irq_unmask(&desc->irq_data);
>>> +}
>>
>> Why is the above code using __raw_* accessors?
>>
>> Atleast use readl_relaxed() instead of __raw.
>
> Noted.
>
> A question regarding readl_relaxed(), is that because the endianess
> swapping? If that's the case, should codes in other functions use
> readl_relaxed()/writel_relaxed() as well?

Basically I think of it like this:

__raw_* = will just poke the registers without really taking anything
   into account, in native endianness.

readl/writel_relaxed() = same thing but always little-endian.

readl()/write() = always little-endian, and will go to lengths to also
drain buffers etc so the write hits the target registers after the PC
passed this instruction. To the extent possible, and may cause a
performance problem due to all buffer draining.

Yours,
Linus Walleij