Re: [PATCH] gpio: add gpio_of_helper
From: Jiří Prchal <hidden>
Date: 2014-10-23 09:38:30
Also in:
linux-devicetree
Dne 23.10.2014 v 10:53 Alexandre Courbot napsal(a):
On Thu, Oct 23, 2014 at 3:23 PM, Jiří Prchal [off-list ref] wrote:quoted
Dne 23.10.2014 v 07:08 Alexandre Courbot napsal(a):quoted
On Wed, Oct 22, 2014 at 6:58 PM, Pantelis Antoniou [off-list ref] wrote:quoted
Hi Alexandre,quoted
On Oct 22, 2014, at 12:41 PM, Alexandre Courbot [off-list ref] wrote: On Wed, Oct 22, 2014 at 6:24 PM, Pantelis Antoniou [off-list ref] wrote:quoted
Hi Alexandre,quoted
On Oct 22, 2014, at 12:18 PM, Alexandre Courbot [off-list ref] wrote: On Wed, Oct 22, 2014 at 5:26 PM, Jiri Prchal [off-list ref] wrote:quoted
This patch adds new driver "gpio-of-helper", witch has possibility to export gpios defined in dt. It exports them in defined name under /sysfs/class/gpio/name. It's rebased from Pantelis Antoniou patch to new kernel. Usage example: gpio { compatible = "gpio-of-helper"; status = "okay"; pinctrl-names = "default"; pinctrl-0 = <&pinctrl_gpio>; gsm_on { gpio-name = "gsm_on"; gpio = <&pioB 13 GPIO_ACTIVE_HIGH>; output; init-low; }; }; This patch needs Alexey Ignatov [PATCH] gpiolib: allow exporting gpios with custom names.We will need to see whether the pre-requisite patch can get merged first, but there are a couple of things that are wrong with your patch anyway: - it is missing a bindings documentation - it is using the legacy integer GPIOs instead of the descriptor interface (see include/linux/gpio/consumer.h). Since this is DT-based, there is absolutely no reason to not use the descriptors interface. - it seems quite long for what it needs to do - the MODULE_AUTHOR has not signed-off this patch (?) But what makes me nervous is that this encourages more usage of the sysfs interface, an in a way that is potentially harmful. Also, I don't know if the DT people will be happy with this idea. Since this concerns DT, please also add the devicetree list and get a Acked-by for the bindings you want to push by a DT maintainer.Since I’m the original perpetrator, let me put a few words here for posterity. This patch was meant as a quick and dirty method for doing the automatic export and pinmux configuration from DT on a 3.8 kernel. The kernel APIs have moved on since. It wasn’t meant to be submitted for mainline right as it is. Unfortunately (or fortunately for many people) it does what a lot of people need i.e. configures a board with heavily pinmuxed GPIO right at boot time. I’m open at having a small discussion about how to do what the users of this patch do ‘right’.Sure, although the discussion might turn out to not be that "small". :)Heh ;)quoted
Pinmux configuration sounds like a job for pinmux more than GPIO, and exporting potentially many GPIOs to user-space sounds like a work for a proper driver.I’m afraid that’s not the case. A great many users do not require anything more than setting a pinmux and the GPIO configuration (input/output). They can then do low speed I/O using the sysfs interface, without having to use any complex APIs (shell works just fine). Think of stuff like controlling a sprinkler valve, or something like a mechanical door detection open state.That sounds like any kind of arbitrary device that can be temporarily connected to a set of GPIOs that will be bit-banged. Is my interpretation correct?Not only temporarily. For example on/off switch of some hw on board like gsm modem.... which should be handled in-kernel.
How to? It's just connected to ttyS and after apply power it has to be switched on by pulling some pin.
quoted
Or some optocoupler isoleted in/outs for general use from userspace.... which should thus be configured from userspace.
Why not from dt?
quoted
quoted
In that case, I seriously doubt that this should be part of the DT. Right now the DT is part of the firmware, and an immutable description of the hardware layout of a board - definitely such devices do not fitThis is our case of gpio. It's like LEDs, they are hw layout of board, but they are in/out instead of LEDs are only output.quoted
into that definition. This might change once the Device Tree Overlays are merged, but we are not there yet. If the DT maintainers say this is a valid use-case for overlays, then I will reconsider my position, but in any case it really looks like this could/should be done from user-space.I don't think so as I explained - HW layout of boerd.If it is a static hardware layout that makes the case even clearer - these devices should have a proper kernel driver that configures these GPIOs, and exports a proper interface to userspace, not just raw GPIOs.
But we don't need anything else just raw named gpio.
quoted
In other way I'm looking forward for DT overlays.quoted
You have almost all the necessary pieces: you can export, configure and manipulate any GPIO from the sysfs interface. The only thing you would be missing is a way to give a name to a GPIO, something that can easily be fixed.But isn't it nice to have already exported gpios which are layouted out from board. All other unexported gpios aren't connected to anywhere on board. So user don't have to export or unexport anything.quoted
Since the devices you want to configure that way are typically removable or usage-specific, why would you want to store that information all the way up into the firmware? A commonly accepted wisdom is that what can be done in user-space should be done in user-space, and it really looks like this applies here.They are NOT removable.quoted
Say you buy a Jetson TK1 to control that sprinkler valve (a good use for all these GPU cores!). The mainline DT has no knowledge about the valve, so using your proposed way you will have to re-build and flash a custom device tree just to be able to use your sprinkler. If you decide to assign your board to something else, you will need another DT.But if you buy more "real world ready" board with for example 8 power outputs for sprinkler valves, why has user think about what gpio to export, if there could be valve0 - 7 ready. If he will use only valve0 does not mater, all unused gpio can't be used for anything else since they has layout for valves.The problem is indeed different. If your sprinkler valves are wired on your board and the lines cannot be used for anything else, then what you need is a driver for the valve that will acquire the GPIOs and export its own userspace interface under /sys/. You device tree must reflect the layout of the board ; in this case "here is a valve that uses this GPIO active-low". Saying "this GPIO is output active-low and named valve0" is not a good description of your hardware.
Maybe I didn't give good example. There is a board. It has 8 outputs. They are relay outputs. Let's give them rel0 - 7 name. And user can same of them use for valve, some for light... Ant board has 8 inputs. They are optocoupler isolated. And user can some of them use to get water level, light...
quoted
quoted
Instead, have a shell script or a systemd unit tmpfile that will perform the correct setup at boot time. Then you can easily switch usages from user-space (systemctl stop sprinkler && systemctl start doordetect). This is much, much more flexible. At least until the Device Tree Overlays are merged, but even then this seems overkill.I seriously think about it, but I think if LEDs are in DT, GPIOs should be there too. In other way, I don't like exporting without given name.LEDs are actually a perfect example of what you should do. They are a simple device often plugged to a GPIO that controls them. We could very well do what you propose: export GPIOs named "ledXX" and call it a day. Instead we have a driver that exports a proper device node in /sys with the operations that are relevant to a LED. Once your GPIOs are assigned to a given function, they are not "general purpose" anymore. Therefore you should expose the right abstraction to your users, even if that means writing some more code.
So making some kind of device "inout" wold be good idea?
-- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
-- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html