Re: [PATCH] gpio: add gpio_of_helper

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi all,

Dne 22.10.2014 v 11:58 Pantelis Antoniou napsal(a):
Hi Alexandre,

On Oct 22, 2014, at 12:41 PM, Alexandre Courbot <gnurou@xxxxxxxxx> wrote:

On Wed, Oct 22, 2014 at 6:24 PM, Pantelis Antoniou
<panto@xxxxxxxxxxxxxxxxxxxxxxx> wrote:
Hi Alexandre,

On Oct 22, 2014, at 12:18 PM, Alexandre Courbot <gnurou@xxxxxxxxx> wrote:

On Wed, Oct 22, 2014 at 5:26 PM, Jiri Prchal <jiri.prchal@xxxxxxxxxxx> wrote:
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:

Let's continue discussion that "someone" need gpio_export_name.

- 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 (?)

OK, this was just first try mainly to open discussion.

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.

Dirty, but is functional.
I'm using something like this from 2.6.38! Than I switched to 3.8, devicetree,
find and apply this patch, but for other reasons moved to newer kernels and every
time need to rebase this.

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 ;)

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).

That is exactly what we need.

Think of stuff like controlling a sprinkler valve, or something like a mechanical
door detection open state.

We need to understand why this is needed, and then see if the DT
bindings are acceptable before going any further.

I can take care of the bindings if we get into some kind of agreement on the basic
interface. I’m CCing device tree and the DT maintainer.

Regards

— Pantelis

--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
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@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux SPI]     [Linux Kernel]     [Linux ARM (vger)]     [Linux ARM MSM]     [Linux Omap]     [Linux Arm]     [Linux Tegra]     [Fedora ARM]     [Linux for Samsung SOC]     [eCos]     [Linux Fastboot]     [Gcc Help]     [Git]     [DCCP]     [IETF Announce]     [Security]     [Linux MIPS]     [Yosemite Campsites]

  Powered by Linux