Thread (34 messages) 34 messages, 6 authors, 2015-01-12

Re: [Patch v2 1/2] gpio: add GPIO hogging mechanism

From: Linus Walleij <hidden>
Date: 2014-12-02 14:28:15
Also in: linux-devicetree, lkml

On Tue, Dec 2, 2014 at 3:10 PM, Alexandre Courbot [off-list ref] wrote:
On Tue, Dec 2, 2014 at 9:22 AM, Benoit Parrot [off-list ref] wrote:
quoted
quoted
quoted
+       }
+
+       if (tmp > MAX_PHANDLE_ARGS) {
+               desc = ERR_PTR(-EINVAL);
+               goto out;
+       }
+
+       gg_data.gpiospec.args_count = tmp;
+       gg_data.gpiospec.np = chip_np;
+       for (i = 0; i < tmp; i++) {
+               ret = of_property_read_u32(np, "gpios",
+                                          &gg_data.gpiospec.args[i]);
+               if (ret) {
+                       desc = ERR_PTR(ret);
+                       goto out;
+               }
+       }
+
+       gpiochip_find(&gg_data, of_gpiochip_find_and_xlate);
This seems to work but only supports one GPIO per hog node. It would
be nice to be able to specify several GPIOs to which the same settings
need to be applied.
This is on purpose following Linus Walleij's comment.
Could you point me to his comment? My bad for not remembering what he
said, but I'd like to understand why.
Said in previous message I think: either one-per-node or lists directly
in the gpiochip node.
quoted
quoted
Using this function means that a GPIO chip module cannot be unloaded
if it uses GPIO hogs. Is it the intended behavior? If not, please use
gpiochip_request_own_desc() instead, and make sure to call
gpiochip_free_own_desc() for each hog when the driver is unloaded.
So I guess we could add a undo_gpio_hog() function and hook it up under of_gpiochip_remove().
Now instead of maintaining a seperate structure just to keep track of hogged descriptor,
would it be acceptable to add a new "gpio_desc.flags" value in gpiolib.h says:

   #define FLAG_GPIO_IS_HOGGED 10

And key on that at removal time instead of creating a list and having to maintain that?
Definitely, that would be even better I think.
Clever. Go for this.
quoted
quoted
I would suggest to factorize this code that is similar to the one
found in __gpiod_get_index(). Do all the DT parsing in a function that
just returns a descriptor and the
I would tend to agree.
But as Linus suggested I was trying to contain the changes to gpiolib_of.c only.
If we add a FLAG_GPIO_IS_HOGGED and undo the hogs when the chip is
unloaded, I would say that this becomes a gpiolib feature. Moving it
here would also allow non-DT GPIO providers to implement hogs (it
should be particularly easy to implement for platform data). Linus, do
you agree?
Yes, this is better and leaves the door open for other users.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help