[PATCH v4 3/8] ARM: pxa: rename gpio_to_irq and irq_to_gpio

[Eric Miao]

On Fri, Oct 14, 2011 at 4:27 PM, Haojian Zhuang
<haojian.zhuang at gmail.com> wrote:
> On Fri, Oct 14, 2011 at 3:18 AM, Grant Likely <grant.likely at secretlab.ca> wrote:
>> On Thu, Oct 13, 2011 at 12:07:00PM +0800, Haojian Zhuang wrote:
>>> Avoid to define gpio_to_irq() and irq_to_gpio() for potential name
>>> confliction since multiple architecture will be built together.
>>>
>>> Signed-off-by: Haojian Zhuang <haojian.zhuang at marvell.com>
>>> ---
>>> @@ -55,6 +66,49 @@ static inline struct pxa_gpio_chip *gpio_to_pxachip(unsigned gpio)
>>>       return &pxa_gpio_chips[gpio_to_bank(gpio)];
>>>  }
>>>
>>> +static inline int gpio_is_pxa_type(int type)
>>> +{
>>> +     if (type & MMP_GPIO)
>>> +             return 0;
>>> +     return 1;
>>> +}
>>
>> or simply: "return (type & MMP_GPIO) == 0;"
>>
>>> +
>>> +static inline int gpio_is_mmp_type(int type)
>>> +{
>>> +     if (type & MMP_GPIO)
>>> +             return 1;
>>> +     return 0;
>>
>> return (type & MMP_GPIO) != 0;
>>
> Your code is better than me. I'll change it.
>
>>> +}
>>> +
>>> +static int pxa_gpio_to_irq(struct gpio_chip *chip, unsigned offset)
>>> +{
>>> +     int gpio;
>>> +
>>> +     gpio = chip->base + offset;
>>> +#ifdef CONFIG_ARCH_PXA
>>> +     if (gpio_is_pxa_type(gpio_type))
>>> +             return PXA_GPIO_TO_IRQ(gpio);
>>> +#endif
>>> +#ifdef CONFIG_ARCH_MMP
>>> +     if (gpio_is_mmp_type(gpio_type))
>>> +             return MMP_GPIO_TO_IRQ(gpio);
>>> +#endif
>>
>> Blech!  Ugly #ifdef blocks.  It would be better to have the #ifdef
>> around the gpio_is_*_type macros with empty versions when the arch
>> isn't enabled.
>>
> OK. I'll append some small API to cover the transition. So #ifdef will
> around the new API.
>
>>> +     return 0;
>>> +}
>>> +
>>> +int pxa_irq_to_gpio(int irq)
>>> +{
>>> +#ifdef CONFIG_ARCH_PXA
>>> +     if (gpio_is_pxa_type(gpio_type))
>>> +             return irq - PXA_GPIO_TO_IRQ(0);
>>> +#endif
>>> +#ifdef CONFIG_ARCH_MMP
>>> +     if (gpio_is_mmp_type(gpio_type))
>>> +             return irq - MMP_GPIO_TO_IRQ(0);
>>> +#endif
>>
>> Ditto here.
>>
>>> +static int pxa_gpio_nums(void)
>>> +{
>>> +     int count = 0;
>>> +
>>> +#ifdef CONFIG_ARCH_PXA
>>> +     if (cpu_is_pxa25x()) {
>>> +#ifdef CONFIG_CPU_PXA26x
>>> +             count = 89;
>>> +             gpio_type = PXA26X_GPIO;
>>> +#elif defined(CONFIG_PXA25x)
>>> +             count = 84;
>>> +             gpio_type = PXA26X_GPIO;
>>> +#endif /* CONFIG_CPU_PXA26x */
>>
>> This isn't multiplatform friendly.  My expectation is that new driver
>> code will not have either/or #ifdef CONFIG blocks like this.  The
>> driver should be written in a way that either option can be turned on.
>>
> Yes, it's not friendly. But the question is that I can't identify
> PXA26x by cpuid.
> Now we have to identify pxa25x and pxa26x depends on the macro
> CONFIG_CPU_PXA26x.
>

Haojian,

Please also separate the removal of global pxa_last_gpio into
another patch.