Re: [RFC Patch] gpio: add GPIO hogging mechanism

[Pantelis Antoniou <pantelis.antoniou@gmail.com>]

Hi Benoit,

> On Oct 29, 2014, at 18:34 , Benoit Parrot <bparrot@ti.com> wrote:
> 
> Pantelis,
> 
> Thanks for the feedback.
> 
> Pantelis Antoniou <pantelis.antoniou@gmail.com> wrote on Wed [2014-Oct-29 10:53:44 +0200]:
>> Hi Benoit,
>> 
>>> On Oct 21, 2014, at 23:09 , Benoit Parrot <bparrot@ti.com> wrote:
>>> 
>>> Based on Boris Brezillion work this is a reworked patch
>>> of his initial GPIO hogging mechanism.
>>> This patch provides a way to initally configure specific GPIO
>>> when the gpio controller is probe.
>>> 
>>> The actual DT scanning to collect the GPIO specific data is performed
>>> as part of the gpiochip_add().
>>> 
>>> The purpose of this is to allows specific GPIOs to be configured
>>> without any driver specific code.
>>> This particularly usueful because board design are getting
>>> increassingly complex and given SoC pins can now have upward
>>> of 10 mux values a lot of connections are now dependent on
>>> external IO muxes to switch various modes and combination.
>>> 
>>> Specific drivers should not necessarily need to be aware of
>>> what accounts to a specific board implementation. This board level
>>> "description" should be best kept as part of the dts file.
>>> 
>> 
>> This look like it’s going to the right direction. I have a few general
>> comments at first.
>> 
>> 1) It relies on dubious DT binding of having sub-nodes of the
>> gpio device implicitly defining hogs.
> 
> I think in this instance the nodes are explicitly defining hogs.
> Please clarify. What would you like to see here?
>> 

Any subnodes are implicitly taken as hog definitions. This is not right because
gpio controllers might have subnodes that they use for another purpose.

>> 2) There is no way for having hogs inserted dynamically as far as I can tell, and
>> no way to remove a hog either.
> 
> The original patch was allowing that but, Linus's review comment suggested this feature be 
> part of the gpio-controller's gpiochip_add() hook only.
> 

If it’s not possible to remove a hog, then it’s no good for my use case in which
the gpios get exported and then removed.

>> 
>> 3) I’m not very fond of having this being part of the gpio controller. This
>> configuration conceptually has little to do with the gpio controller per se,
>> it is more of a board specific thing. Why not come up with a gpio-hog driver that
>> references GPIOs? That way with a single gpio-hog driver instance you could setup
>> all the GPIO-hogging configuration for all GPIOs on the board, even one that
>> lie on different GPIO controllers.
> 
> Again this follows Linus's review comment.
> I agree that it prevent a centralize spot where all hog would be defined but it has the advantages of not relying on PROBE_DEFER.
> I mean since all "gpio-hogs" would be defined in a single board dts file it would not be that hard to figure out the big picture anyways.
> 

There won’t be a single board dts file if you’re using things like overlays.

>> 
>> 
>>> Signed-off-by: Benoit Parrot <bparrot@ti.com>
>>> ---
>>> Documentation/devicetree/bindings/gpio/gpio.txt | 33 +++++++++
>>> drivers/gpio/gpiolib-of.c                       | 99 +++++++++++++++++++++++++
>>> drivers/gpio/gpiolib.c                          | 81 ++++++++++++++++++++
>>> include/linux/of_gpio.h                         | 11 +++
>>> 4 files changed, 224 insertions(+)
>>> 
>> 
>> Regards
>> 
>> — Pantelis
>> 
> 
> Regards,
> Benoit

Regards

— Pantelis