linux-kernel - Re: [PATCH v3 1/7] pinctrl: Renesas RZ/A1 pin and gpio controller

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 24 Mar 2017 16:42:47 +0100
From:   Linus Walleij <linus.walleij@...aro.org>
To:     Jacopo Mondi <jacopo+renesas@...ndi.org>
Cc:     Geert Uytterhoeven <geert+renesas@...der.be>,
        Laurent Pinchart <laurent.pinchart@...asonboard.com>,
        Chris Brandt <chris.brandt@...esas.com>,
        Rob Herring <robh+dt@...nel.org>,
        Mark Rutland <mark.rutland@....com>,
        Russell King <linux@...linux.org.uk>,
        Linux-Renesas <linux-renesas-soc@...r.kernel.org>,
        "linux-gpio@...r.kernel.org" <linux-gpio@...r.kernel.org>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v3 1/7] pinctrl: Renesas RZ/A1 pin and gpio controller

On Fri, Mar 24, 2017 at 4:22 PM, Jacopo Mondi <jacopo+renesas@...ndi.org> wrote:

I assume Geert will queue this driver even if it is outside of sh-pfc?

> Add combined gpio and pin controller driver for Renesas RZ/A1
> r7s72100 SoC.
>
> Signed-off-by: Jacopo Mondi <jacopo+renesas@...ndi.org>
> ---
>  drivers/pinctrl/Kconfig        |  10 +
>  drivers/pinctrl/Makefile       |   1 +
>  drivers/pinctrl/pinctrl-rza1.c | 961 +++++++++++++++++++++++++++++++++++++++++

So this is very different from the SH-PFC family and should not
be in drivers/pinctrl/sh-pfc?

> +config PINCTRL_RZA1
> +       bool "Renesas RZ/A1 gpio and pinctrl driver"
> +       depends on OF
> +       depends on ARCH_R7S72100 || COMPILE_TEST
> +       select GENERIC_PINCTRL_GROUPS
> +       select GENERIC_PINMUX_FUNCTIONS
> +       select GENERIC_PINCONF

If it is also a GPIO driver I guess it should
select GPIOLIB as well.

This was not possible in the past, but it is possible nowadays.

> +struct gpio_chip rza1_gpiochip_template = {
> +       .request                = rza1_gpio_request,
> +       .free                   = rza1_gpio_free,
> +       .get_direction          = rza1_gpio_get_direction,
> +       .direction_input        = rza1_gpio_direction_input,
> +       .direction_output       = rza1_gpio_direction_output,
> +       .get                    = rza1_gpio_get,
> +       .set                    = rza1_gpio_set,
> +};

We now also have .set_multiple() and more interestingly
.set_config() which can be backed by pinctrl if you want
to e.g. support debouncing and/or open drain/open source.

Maybe this is stuff your pin controller can do, but not needed
in the initial submission for sure.

> +static int rza1_pinmux_set(struct pinctrl_dev *pctldev, unsigned int selector,
> +                          unsigned int group)

Please name it rza1_set_mux() to correspond with the ops field.

Yours,
Linus Walleij