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  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:   Tue, 1 Dec 2020 15:37:38 +0100
From:   Linus Walleij <linus.walleij@...aro.org>
To:     Manivannan Sadhasivam <mani@...nel.org>
Cc:     Johan Hovold <johan@...nel.org>,
        Greg KH <gregkh@...uxfoundation.org>,
        linux-usb <linux-usb@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        patong.mxl@...il.com,
        Mauro Carvalho Chehab <mchehab+huawei@...nel.org>,
        angelo.dureghello@...esys.com,
        "open list:GPIO SUBSYSTEM" <linux-gpio@...r.kernel.org>
Subject: Re: [PATCH v5 2/3] usb: serial: xr_serial: Add gpiochip support

On Sun, Nov 22, 2020 at 6:08 PM Manivannan Sadhasivam <mani@...nel.org> wrote:

> Add gpiochip support for Maxlinear/Exar USB to serial converter
> for controlling the available gpios.
>
> Inspired from cp210x usb to serial converter driver.
>
> Cc: Linus Walleij <linus.walleij@...aro.org>
> Cc: linux-gpio@...r.kernel.org
> Signed-off-by: Manivannan Sadhasivam <mani@...nel.org>

This looks good to me overall, provided that it plays well with the
serial port.

One minor notice:

> +enum gpio_pins {
> +       GPIO_RI = 0,
> +       GPIO_CD,
> +       GPIO_DSR,
> +       GPIO_DTR,
> +       GPIO_CTS,
> +       GPIO_RTS,
> +       GPIO_MAX,
> +};

You know the names of the pins...

> +       port_priv->gc.ngpio = 6;
> +       port_priv->gc.label = "xr_gpios";
> +       port_priv->gc.request = xr_gpio_request;
> +       port_priv->gc.free = xr_gpio_free;
> +       port_priv->gc.get_direction = xr_gpio_direction_get;
> +       port_priv->gc.direction_input = xr_gpio_direction_input;
> +       port_priv->gc.direction_output = xr_gpio_direction_output;
> +       port_priv->gc.get = xr_gpio_get;
> +       port_priv->gc.set = xr_gpio_set;
> +       port_priv->gc.owner = THIS_MODULE;
> +       port_priv->gc.parent = &port->dev;
> +       port_priv->gc.base = -1;
> +       port_priv->gc.can_sleep = true;

So assign port_priv->gc.names here as well with an array
of strings with the names ("RI", "CD", ... etc).
This makes it look really nice in userspace if you do
e.g. "lsgpio".

With that:
Reviewed-by: Linus Walleij <linus.walleij@...aro.org>

Yours,
Linus Walleij

Powered by blists - more mailing lists