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

[Greg Kroah-Hartman]

On Mon, May 15, 2023 at 12:02:07PM -0400, Hugo Villeneuve wrote:
> From: Hugo Villeneuve <hvilleneuve@xxxxxxxxxxxx>
> 
> 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@xxxxxxxxxxxx>
> ---
>  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