On Mon, Sep 29, 2014 at 11:18 AM, Linus Walleij
<linus.walleij@xxxxxxxxxx> wrote:
> On Sun, Sep 14, 2014 at 9:33 PM, Sergey Ryazanov <ryazanov.s.a@xxxxxxxxx> wrote:
>
>> Atheros AR5312 SoC have a builtin GPIO controller, which could be
>> accessed via memory mapped registers. This patch adds new driver
>> for them.
>>
>> Signed-off-by: Sergey Ryazanov <ryazanov.s.a@xxxxxxxxx>
>> Cc: Linus Walleij <linus.walleij@xxxxxxxxxx>
>> Cc: Alexandre Courbot <gnurou@xxxxxxxxx>
>> Cc: linux-gpio@xxxxxxxxxxxxxxx
> (...)
>> - /* Attempt to jump to the mips reset location - the boot loader
>> - * itself might be able to recover the system */
>> + /* Cold reset does not work on the AR2315/6, use the GPIO reset bits
>> + * a workaround. Give it some time to attempt a gpio based hardware
>> + * reset (atheros reference design workaround) */
>> + gpio_request_one(AR2315_RESET_GPIO, GPIOF_OUT_INIT_LOW, "Reset");
>> + mdelay(100);
>
> Please try to use the new GPIO descriptor API.
> See Documentation/gpio/consumer.txt
>
> (...)
>> +++ b/drivers/gpio/gpio-ar2315.c
>
>> +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
>
> Get rid of _gpio_irq_base altogether. (See comments below.)
>
> (...)
>> +static inline u32 ar2315_gpio_reg_read(unsigned reg)
>> +{
>> + return __raw_readl(ar2315_mem + reg);
>> +}
>
> Use readl_relaxed()
ar2315/ar5312 is big endian, so readl_relaxed would mess up the byte
order. There does not seem to be a big endian equivalent for _relaxed.
Regards
Jonas
