[PATCH] PINCTRL: SiRF: add CSR SiRFprimaII GPIO driver depending on pinmux APIs

[Linus Walleij]

On Sun, Jun 10, 2012 at 3:16 PM, Barry Song <21cnbao at gmail.com> wrote:
> 2012/5/29 Linus Walleij <linus.walleij at linaro.org>:

>> I don't really have an opinion on this, you can do it either way.
>>
>> It's nice to split the files into separate drivers for smaller files and
>> cleanly cut devices.
>>
>> It's also nice to have it all in one file so you can get the whole picture
>> of how the hardware works.
>
> i think i'd better to have one driver for both. if i move to
> gpio-generic driver for gpio module, gpio-generic driver will not
> request gpio from pinctrl, but i need. and on the other hand,
> gpio-generic doesn't include irq_chip, but i need.
> so to me, gpio-generic driver is much like just for a single-function
> gpio controller not good for a multi-function pinmux/gpio/irq
> controller.

OK that makes sense I thought you didn't need to cross-correlate
them.

>> Usually the problem has been to cross-reference the GPIO ranges
>> from another driver, like adding the struct gpio_chip * to the ranges,
>> but it seems you don't have this problem in your driver? If you think
>> you will run into this problem, merge it into the pinctrl driver to
>> save yourself from lots of trouble.
>
> for the range issue, basically we have a (pinmux pin, gpio, irq) pair,
> one pinmux pin can be a gpio pin and an external irq source , so
> actually, i need to pinctrl_request_gpio() in my gpio_chip.request().
> i would think prima2 pinctrl driver can be much like pinctrl-nomadik.c
> and pinctrl-coh901.c, which are all of pinmux, gpio and irq chip.  i
> might follow them and improve pinctrl-sirf.c to include gpio/irq too.

Yep, but now I see you haven't used irqdomain to map between
gpios and IRQs so I will hit you with one more review comment.
We try to get all gpio offset -> IRQ mapping done using
irqdomains.

Yours,
Linus Walleij