Re: [PATCH 1/6] serial: sa1100: add support for mctrl gpios

["Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>]

On Fri, May 31, 2019 at 12:13:47PM +0100, Russell King wrote:
> Add support for the generic mctrl gpio helper.  This will allow us to
> convert several board files to use the gpiod tables to assign GPIOs to
> serial ports, rather than needing to have private function callbacks.
> 
> If the generic mctrl gpio helper fails, ignore the mctrl gpios rather
> than preventing the (possibly console) serial port from being created.
> 
> Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Acked-by: Linus Walleij <linus.walleij@linaro.org>
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> ---
>  drivers/tty/serial/Kconfig  |  1 +
>  drivers/tty/serial/sa1100.c | 42 ++++++++++++++++++++++++++++++++++++++----
>  2 files changed, 39 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
> index 72966bc0ac76..f4372ac1a774 100644
> --- a/drivers/tty/serial/Kconfig
> +++ b/drivers/tty/serial/Kconfig
> @@ -511,6 +511,7 @@ config SERIAL_SA1100
>  	bool "SA1100 serial port support"
>  	depends on ARCH_SA1100
>  	select SERIAL_CORE
> +	select SERIAL_MCTRL_GPIO if GPIOLIB
>  	help
>  	  If you have a machine based on a SA1100/SA1110 StrongARM(R) CPU you
>  	  can enable its onboard serial port by enabling this option.
> diff --git a/drivers/tty/serial/sa1100.c b/drivers/tty/serial/sa1100.c
> index a399772be3fc..97bdfeccbea9 100644
> --- a/drivers/tty/serial/sa1100.c
> +++ b/drivers/tty/serial/sa1100.c
> @@ -28,6 +28,8 @@
>  #include <mach/hardware.h>
>  #include <mach/irqs.h>
>  
> +#include "serial_mctrl_gpio.h"
> +
>  /* We've been assigned a range on the "Low-density serial ports" major */
>  #define SERIAL_SA1100_MAJOR	204
>  #define MINOR_START		5
> @@ -77,6 +79,7 @@ struct sa1100_port {
>  	struct uart_port	port;
>  	struct timer_list	timer;
>  	unsigned int		old_status;
> +	struct mctrl_gpios	*gpios;
>  };
>  
>  /*
> @@ -174,6 +177,8 @@ static void sa1100_enable_ms(struct uart_port *port)
>  		container_of(port, struct sa1100_port, port);
>  
>  	mod_timer(&sport->timer, jiffies);
> +
> +	mctrl_gpio_enable_ms(sport->gpios);
>  }
>  
>  static void
> @@ -322,11 +327,21 @@ static unsigned int sa1100_tx_empty(struct uart_port *port)
>  
>  static unsigned int sa1100_get_mctrl(struct uart_port *port)
>  {
> -	return TIOCM_CTS | TIOCM_DSR | TIOCM_CAR;
> +	struct sa1100_port *sport =
> +		container_of(port, struct sa1100_port, port);
> +	int ret = TIOCM_CTS | TIOCM_DSR | TIOCM_CAR;
> +
> +	mctrl_gpio_get(sport->gpios, &ret);
> +
> +	return ret;
>  }
>  
>  static void sa1100_set_mctrl(struct uart_port *port, unsigned int mctrl)
>  {
> +	struct sa1100_port *sport =
> +		container_of(port, struct sa1100_port, port);
> +
> +	mctrl_gpio_set(sport->gpios, mctrl);
>  }
>  
>  /*
> @@ -842,6 +857,27 @@ static int sa1100_serial_resume(struct platform_device *dev)
>  	return 0;
>  }
>  
> +static int sa1100_serial_add_one_port(struct sa1100_port *sport, struct platform_device *dev)
> +{
> +	sport->port.dev = &dev->dev;
> +	sport->gpios = mctrl_gpio_init_noauto(sport->port.dev, 0);

the _noauto function was only introduced to ease a transition. I think
the driver would benefit to use mctrl_gpio_init() instead.

Getting rid of mctrl_gpio_init_noauto() was on my todo list for some
time, but it was pushed down too far :-|

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |