Re: [rtc-linux] [PATCH V2 3/5] gpio: add support for AMS AS3722 gpio driver

[Linus Walleij]

On Fri, Sep 20, 2013 at 2:30 PM, Laxman Dewangan <ldewangan@xxxxxxxxxx> wrote:

> The AS3722 is a compact system PMU suitable for mobile phones, tablets etc.
>
> Add a driver to support accessing the 8 GPIOs found on the AMS AS3722
> PMIC using gpiolib.
>
> Signed-off-by: Laxman Dewangan <ldewangan@xxxxxxxxxx>
> Signed-off-by: Florian Lobmaier <florian.lobmaier@xxxxxxx>

OK...

> +Required subnode properties:
> +---------------------------
> +reg: The GPIO number on which the properties need to be applied.
> +
> +Optional subnode properties:
> +---------------------------
> +bias-pull-up: The Pull-up for the pin to be enable.
> +bias-pull-down: Pull down of the pins to be enable.
> +bias-high-impedance: High impedance of the pin to be enable.
> +open-drain: Pin is open drain type.
> +function: IO functionality of the pins. The valid options are:
> +       gpio, intrrupt-output, vsup-vbat-low-undeb, interrupt-input,
> +       pwm-input, voltage-stby, oc-powergood-sd0, powergood-output,
> +       clk32k-output, watchdog-input, soft-reset-input, pwm-output,
> +       vsup-vbat-low-deb, oc-powergood-sd6
> +    Missing the function property will set the pin in GPIO mode.

This is pin control. Do not try to shoehorn pin control drivers into
the GPIO subsystem. Take a good day off, read through
Documentation/pinctrl.txt and come back with a thoroughly rewritten
driver in drivers/pinctrl/pinctrl-as3722.c.

We already have generic device tree bindings for all of the above
properties, and a library for generic pin config in the pin control
subsystem to handle them. Look at recent drivers for
inspiration.

> diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig

Move this driver to the pinctrl subsystem.

> index b6ed304..544a612 100644
> --- a/drivers/gpio/Kconfig
> +++ b/drivers/gpio/Kconfig
> @@ -368,6 +368,12 @@ config GPIO_ARIZONA
>         help
>           Support for GPIOs on Wolfson Arizona class devices.
>
> +config GPIO_AS3722
> +       bool "AMS AS3722 PMICs GPIO"
> +       depends on MFD_AS3722
> +       help
> +         Select this option to enable GPIO driver for the AMS AS3722 PMIC.

It should
select PINMUX
select GENERIC_PINCONF

> +static char const *as3722_gpio_iosf[] = {
> +       "gpio",
> +       "intrrupt-output",

intrrupt?

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/