Re: [PATCH] gpio: add gpio_of_helper

[Pantelis Antoniou <panto@xxxxxxxxxxxxxxxxxxxxxxx>]

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:
> 
> - 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’.

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