Re: [RFC 09/18] gpio: add driver for Atheros AR5312 SoC GPIO controller

[Sergey Ryazanov <ryazanov.s.a@xxxxxxxxx>]

2014-10-21 12:29 GMT+04:00 Linus Walleij <linus.walleij@xxxxxxxxxx>:
> On Mon, Sep 29, 2014 at 10:43 PM, Sergey Ryazanov
> <ryazanov.s.a@xxxxxxxxx> wrote:
>> 2014-09-29 13:18 GMT+04:00 Linus Walleij <linus.walleij@xxxxxxxxxx>:
>
>>>> +static u32 ar2315_gpio_intmask;
>>>> +static u32 ar2315_gpio_intval;
>>>> +static unsigned ar2315_gpio_irq_base;
>>>> +static void __iomem *ar2315_mem;
>>>
>>> Get rid of these local variables and put them into an allocated
>>> state container, see
>>> Documentation/driver-model/design-patterns.txt
>>>
>> AR2315 SoC contains only one GPIO unit, so there are no reasons to
>> increase driver complexity. But if you insist, I will add state
>> container.
>
> I insist. It makes the driver easier to maintain if it looks like
> most other drivers instead of using static locals.
>
No problem. I rewrite the driver using the state container.

>>> Convoluted, I would use an if() else construct rather than the ? operator.
>>>
>> Convoluted, but 3 lines shorter :) And checkpatch has no objections.
>
> True but it's me who is going to be maintaining this, not checkpatch.
>
I missed that. Added to my todo list.

BTW, the use of the irq_domain framework is required or I could
opencode some stuff?

> Yours,
> Linus Walleij

-- 
BR,
Sergey
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html