linux-kernel - Re: [RFC PATCH] Revert "sc16is7xx: Separate GPIOs from modem control lines"

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]

Message-ID: <2023051551-quickstep-outshine-5526@gregkh>
Date:   Mon, 15 May 2023 18:20:02 +0200
From:   Greg Kroah-Hartman <gregkh@...uxfoundation.org>
To:     Hugo Villeneuve <hugo@...ovil.com>
Cc:     Jiri Slaby <jirislaby@...nel.org>,
        Lech Perczak <l.perczak@...lintechnologies.com>,
        Tomasz Moń <tomasz.mon@...lingroup.com>,
        Hugo Villeneuve <hvilleneuve@...onoff.com>,
        linux-serial@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [RFC PATCH] Revert "sc16is7xx: Separate GPIOs from modem control
 lines"

On Mon, May 15, 2023 at 12:02:07PM -0400, Hugo Villeneuve wrote:
> From: Hugo Villeneuve <hvilleneuve@...onoff.com>
> 
> This reverts commit 679875d1d8802669590ef4d69b0e7d13207ebd61.
> 
> Because of this commit, it is no longer possible to use the 16 GPIO
> lines as dedicated GPIOs on the SC16IS752.
> 
> Reverting it makes it work again.
> 
> The log message of the original commit states:
>     "Export only the GPIOs that are not shared with hardware modem
>     control lines"
> 
> But there is no explanation as to why this decision was taken to
> permanently set the function of the GPIO lines as modem control
> lines. AFAIK, there is no problem with using these lines as GPIO or modem
> control lines.
> 
> Maybe after reverting this commit, we could define a new
> device-tree property named, for example,
> "use-modem-control-lines", so that both options can be supported.
> 
> Fixes: 679875d1d880 ("sc16is7xx: Separate GPIOs from modem control
> lines")

Please do not line-wrap these lines.

> 

Nor is a blank line needed here.

> Signed-off-by: Hugo Villeneuve <hvilleneuve@...onoff.com>
> ---
>  drivers/tty/serial/sc16is7xx.c | 14 ++++----------
>  1 file changed, 4 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/tty/serial/sc16is7xx.c b/drivers/tty/serial/sc16is7xx.c
> index 5bd98e4316f5..25f1b2f6ec51 100644
> --- a/drivers/tty/serial/sc16is7xx.c
> +++ b/drivers/tty/serial/sc16is7xx.c
> @@ -306,7 +306,6 @@ struct sc16is7xx_devtype {
>  	char	name[10];
>  	int	nr_gpio;
>  	int	nr_uart;
> -	int	has_mctrl;
>  };
>  
>  #define SC16IS7XX_RECONF_MD		(1 << 0)
> @@ -447,35 +446,30 @@ static const struct sc16is7xx_devtype sc16is74x_devtype = {
>  	.name		= "SC16IS74X",
>  	.nr_gpio	= 0,
>  	.nr_uart	= 1,
> -	.has_mctrl	= 0,
>  };
>  
>  static const struct sc16is7xx_devtype sc16is750_devtype = {
>  	.name		= "SC16IS750",
> -	.nr_gpio	= 4,
> +	.nr_gpio	= 8,

I think this one line change is all you really need here, right?  the
otner changes do nothing in this patch, so you should just create a new
one changing this value.  Oh, and this one:

>  	.nr_uart	= 1,
> -	.has_mctrl	= 1,
>  };
>  
>  static const struct sc16is7xx_devtype sc16is752_devtype = {
>  	.name		= "SC16IS752",
> -	.nr_gpio	= 0,
> +	.nr_gpio	= 8,

right?

Don't mess with the has_mctrl stuff, that's not relevant here.

thanks,

greg k-h