[Top] [All Lists]

Re: [PATCH 04/14] OF: pinctrl: MIPS: lantiq: implement lantiq/xway pinct

To: Linus Walleij <>
Subject: Re: [PATCH 04/14] OF: pinctrl: MIPS: lantiq: implement lantiq/xway pinctrl support
From: John Crispin <>
Date: Tue, 08 May 2012 16:12:10 +0200
Cc:,, Ralf Baechle <>
In-reply-to: <>
References: <> <> <>
User-agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv: Gecko/20111114 Icedove/3.1.16
Hi Linus,

Thanks. I will fold your comments with Stephen's into a V2 and resend it.

> Shouldn't this be:
> depends on SOC_TYPE_XWAY
> depends on PINCTRL_LANTIQ
> ?
> So LANTIQ selects it's pinctrl driver, the the xway SoC
> selects its driver and they both are dependent on their
> respective system.
The whole select/depends part is broken. I will clean this up properly

>> diff --git a/drivers/pinctrl/pinctrl-lantiq.h 
>> b/drivers/pinctrl/pinctrl-lantiq.h
>> +#define ARRAY_AND_SIZE(x)      (x), ARRAY_SIZE(x)
I was actually considering to drop this. Having a "," inside a macro is
a bit ugly.
It leads to the calling code invoking the function with N-1 parameters,
although the function takes N parameters. I find this a bit

>> +/* macros to help us access the registers */
>> +#define gpio_getbit(m, r, p)   (!!(ltq_r32(m + r) & (1 << p)))
>> +#define gpio_setbit(m, r, p)   ltq_w32_mask(0, (1 << p), m + r)
>> +#define gpio_clearbit(m, r, p) ltq_w32_mask((1 << p), 0, m + r)
> So what makes this arch so fantastic that it needs its own read/write 
> functions?
> (Just curious...)
Nothing. Its a legacy macro from a few years ago when I first added
lantiq support inside openwrt. I personally like the macro. I use it
wherever I access lantiq registers.
When accessing generic memory ranges, as in the nand driver, I use
writeb() and co.

Matter of taste really. I would prefer to keep it this way if there are
no guidelines against it.

>> +/* ---------  gpio_chip related code --------- */
>> +
>> +int gpio_to_irq(unsigned int gpio)
>> +{
>> +       return -EINVAL;
>> +}
>> +EXPORT_SYMBOL(gpio_to_irq);
>> +
>> +int irq_to_gpio(unsigned int gpio)
>> +{
>> +       return -EINVAL;
>> +}
>> +EXPORT_SYMBOL(irq_to_gpio);
> Can't you just leave them undefined?

I just checked how ARM does it. They use

Let me talk to Ralf about this and make a MIPS version of said header file.


<Prev in Thread] Current Thread [Next in Thread>