On Mon, Feb 3, 2014 at 7:26 PM, Jean-Jacques Hiblot <[email protected]> wrote: > Hi Alexandre, > > > 2014-02-03 Alexandre Courbot <[email protected]>: > >> Hi Jean-Jacques, >> >> Sorry for taking so much time to reply, I had to go through the AT91 >> thread several times to (hopefully) get a clear idea of what you need. >> >> >> On Thu, Jan 30, 2014 at 1:11 AM, Jean-Jacques Hiblot >> <[email protected]> wrote: >> > The patch implements a new requesting scheme for GPIOs that allow a gpio >> > to be >> > requested more than once. >> > >> > This new request scheme is: >> > * only 1 user can request a GPIO with a full control over the direction >> > of the >> > GPIO. Full control means being able to configure the gpio as an input >> > or as >> > an ouput. >> > * several users can request a GPIO with a limited control over the >> > direction. >> > Limited control means: the gpio can be configured as an input if >> > someone >> > doesn't have a full control of the direction. It can't be never be >> > configured >> > as an output. >> > * a GPIO used as an interrupt source can't be configured as an output. >> >> So if I understand correctly (correct me if I don't), the problem is >> that you need to be able to read the value of a GPIO that is currently >> being used as an interrupt source. One example of this happening is >> the touchscreen node of arch/arm/boot/dts/imx28-tx28.dts: >> >> touchscreen: tsc2007@48 { >> ... >> interrupt-parent = <&gpio3>; >> interrupts = <20 0>; >> pendown-gpio = <&gpio3 20 1>; >> }; >> >> While you are at it, you also want to allow a GPIO to be requested >> several times as long as these requests are not conflicting (which is >> a generalization of your initial need). > > exactly. Whle we're at it, we could try to make it work for other use cases. >> >> This should probably be >> considered dangerous for the integer-based interface, but with gpiod >> GPIOs are now assigned by platform files or firmware, so this sounds >> much more legitimate in this context. > > agreed. The integer-based interface must not be impacted by this. >> >> >> > To achieve this, a unique gpio_desc is returned by gpiod_request. The >> > old >> > gpio_desc is replaced by a gpio_hw_desc. Integer name space is still >> > supported >> > and a gpio requested by its number is requested with full direction >> > control. >> > >> > This patch is for RFC only. I feel that the API change need to be >> > discussed >> > before going further. Also there are probably some race conditions that >> > are >> > more important to fix now than when a gpio was an exclusive property. >> >> If I understand your goals correctly, I believe they can be reached by >> a simpler solution. For your initial problem the >> at91_gpio_irq_domain_xlate() should obtain a GPIO descriptor and call >> gpiod_lock_as_irq() on it. This will allow the GPIO from being >> requested as input later. Currently it is not possible to obtain a >> GPIO descriptor outside of gpiod_get() (which will request the GPIO at >> the same time), but it should be acceptable to consider that the >> holder of a gpio_chip * (either the GPIO driver itself, or in your >> case the AT91 pinctrl driver) is priviledged and can get access to any >> of the chip's descriptor through a new driver function (we already >> discussed doing so in https://lkml.org/lkml/2013/10/8/823 ). >> > For the touchscreen case, this is indeed a simple solution that would work.
Great - in this case I would suggest to go for it, as you can implement it immediately (you will need to implement that driver function that allows a driver to access any of its descriptor, this should be easy if you follow the mail linked above) and it really is the best-fit solution to this particular problem. > >> >> As for the multiple-consumer case, couldn't we avoid the complexity >> introduced by the different kinds of descriptors by simply using read >> and write reference counters in each GPIO descriptor? Basically a call >> to gpiod_get() would always return the corresponding descriptor as >> this means the GPIO is mapped. But when attempting to set the >> direction, the reference counters are checked to confirm that this >> would not put the GPIO into one of the forbidden cases (e.g. no write >> if FLAG_USED_AS_IRQ is set, only one writer, but as many readers as we >> want). This sounds like it could be implemented much more succintly, >> and should (IIUC) do what you wanted. >> > Actually it was the first approach I tried. It takes care of most of the > problem. But there are some drawbacks: > * no control of permissions for gpiod_set_value. A consumer requesting for > read would be able to set the gpio's value. > * need to modify the gpiod_free API to pass the same permissions flags as to > gpiod_request(). The consequence is that the flags need to be stored along > the gpio_desc* in the consumers' private data. > * same problem with the gpio's label. All valid points indeed. I am still a little bit turned off by the added complexity this brings to a subsystem that is supposed to remain simple to use (obtain a GPIO descriptor, drive the GPIO). We also need to consider all special cases (active-low, open-drain, etc) and make sure we handle all conflicts (what if a consumer requires open-source and the other open-drain?). I'm afraid this could quickly turn into a nightmare. Not that I am rejecting your idea. It's just that we are entering a new unknown zone with this and we really need to think it through. > There's another feature that I didn't post because its use case is probably > not very common. I wanted to be able to share output gpios. My use case is > the gpio tracing mechanism I posted a few weeks ago. Yes, I remember that patch. > To reduce the complexity of tracking the gpio used by the probes, I thought > that maybe this task could be delegated to the gpiolib. Implementation could > be very straightforward there: > * in gpiod_request (or equivalent) pass an ownership tag (NULL would be a > special default value) > * in the case were the ouput is already owned, check if the ownership tag > are the same and not NULL. If so the request succeeds otherwise it fails. So the two drivers would need to communicate that ownership tag so the second can "hijack" the GPIO with permission from the first? You could also pass a handle directly, like PRIME does for buffers (then we could plug kdbus in and have user processes exchange GPIO handles securely. Now I'm scared). I would be even more cautious about sharing output GPIOs. If possible at all, I'd really prefer to see a scheme where the two consuming drivers yield the GPIO when they don't need it. After all, if you enter a situation where both drivers want to drive the GPIO output, you are obviously going to have a problem. Alex. -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
