Messages in this thread | | | Date | Thu, 12 Jul 2018 01:22:12 +0200 | From | Paul Cercueil <> | Subject | Re: [PATCH 0/5] pinctrl_gpio_get_direction & ingenic fixes |
| |
Hi Linus,
Le lun. 9 juil. 2018 à 14:09, Linus Walleij <linus.walleij@linaro.org> a écrit : > Hi folks, > > On Wed, Jun 27, 2018 at 7:18 PM Andy Shevchenko > <andy.shevchenko@gmail.com> wrote: > >> Even if GPIO and pin muxing has only one set of buffers to indicate >> input or output (same registers in use) it's a GPIO driver business >> to >> get direction from GPIO part of IP. >> >> Looking into the existing code I would rather say that >> pinctrl-ingenic.c should incorporate gpio-ingenic.c as they are >> (partially) sharing same registers. > > Usually we only split the functionality into two drivers if the two > features > pin control and GPIO are explicitly in different hardware blocks, > and typically not sharing the same memory range. > > If these registers are intermingled and the hardware actually > just one piece of silicon, I would suggest to try to merge the > two drivers into a combined pin control and GPIO driver > inside drivers/pinctrl/pinctrl-ingenic.c. > > We have a few drivers like that already, good textbook > examples of how to do this include > drivers/pinctrl/pinctrl-sx150x.c where the two blocks are > handled in one driver using both APIs. > > Paul could you have a look at if we can simply merge these > two into one big driver? It is much more natural to write > into the same set of registers when we do that.
Well I wish you had told me that when I submitted the ingenic pinctrl/gpio patchset :)
I won't have much time before 4.19-rc1, but I can have a look after that.
> If you still prefer to proceed with the GPIO/pinctrl as separate > drivers we need to look into this patch set, which I am > a bit ambivalent about, because it makes sense but at the > same time I want to keep GPIO and pin control business > separate because separation of concerns is just nice.
Well I can still implement the get_direction() function in the GPIO driver by reading the registers instead of calling into pinctrl.
I just thought it felt illogic as set_direction() does that.
> Yours, > Linus Walleij
Thanks, -Paul
| |