[PATCH v2 0/2] Add CTS/RTS gpio support to STM32 UART

[PATCH v2 0/2] Add CTS/RTS gpio support to STM32 UART

[mani]

From: Manivannan Sadhasivam <mani@kernel.org>

Hello,

This patchset adds CTS/RTS gpio support to STM32 UART controller.
Eventhough the UART controller supports using dedicated CTS/RTS gpios,
sometimes we need to use different set of gpios for flow control.

This is necessary for the upcoming STM32MP1 based board called Stinger96
IoT-Box. On that board, a bluetooth chip is connected to one of the UART
controller but the CTS/RTS lines got swapped mistakenly. So this patchset
serves as a workaround for that hardware bug and also supports the
usecase of using any gpio for CTS/RTS functionality. As per the sugggestion
provided by Andy for v1, I've now switched to mctrl_gpio driver.

This patchset has been validated with Stinger96 IoT-Box connected to Murata
WiFi-BT combo chip.

Thanks,
Mani

Changes in v2:

As per the review by Andy:

* Switched to mctrl_gpio driver instead of using custom CTS/RTS
  implementation
* Removed the use of software flow control terminology.

Manivannan Sadhasivam (2):
  tty: serial: Add modem control gpio support for STM32 UART
  dt-bindings: serial: Document CTS/RTS gpios in STM32 UART

 .../bindings/serial/st,stm32-uart.yaml        | 14 ++++++
 drivers/tty/serial/Kconfig                    |  1 +
 drivers/tty/serial/stm32-usart.c              | 43 ++++++++++++++++++-
 drivers/tty/serial/stm32-usart.h              |  1 +
 4 files changed, 58 insertions(+), 1 deletion(-)

-- 
2.17.1

[PATCH v2 1/2] tty: serial: Add modem control gpio support for STM32 UART

[mani]

From: Manivannan Sadhasivam <mani@kernel.org>

STM32 UART controllers have the built in modem control support using
dedicated gpios which can be enabled using 'st,hw-flow-ctrl' flag in DT.
But there might be cases where the board design need to use different
gpios for modem control.

For supporting such cases, this commit adds modem control gpio support
to STM32 UART controller using mctrl_gpio driver.

Signed-off-by: Manivannan Sadhasivam <mani@kernel.org>
---
 drivers/tty/serial/Kconfig       |  1 +
 drivers/tty/serial/stm32-usart.c | 43 +++++++++++++++++++++++++++++++-
 drivers/tty/serial/stm32-usart.h |  1 +
 3 files changed, 44 insertions(+), 1 deletion(-)

diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
index 0aea76cd67ff..e7a6f2130684 100644
--- a/drivers/tty/serial/Kconfig
+++ b/drivers/tty/serial/Kconfig
@@ -1462,6 +1462,7 @@ config SERIAL_STM32
 	tristate "STMicroelectronics STM32 serial port support"
 	select SERIAL_CORE
 	depends on ARCH_STM32 || COMPILE_TEST
+	select SERIAL_MCTRL_GPIO if GPIOLIB
 	help
 	  This driver is for the on-chip Serial Controller on
 	  STMicroelectronics STM32 MCUs.
diff --git a/drivers/tty/serial/stm32-usart.c b/drivers/tty/serial/stm32-usart.c
index 5e93e8d40f59..026982259714 100644
--- a/drivers/tty/serial/stm32-usart.c
+++ b/drivers/tty/serial/stm32-usart.c
@@ -31,6 +31,7 @@
 #include <linux/tty_flip.h>
 #include <linux/tty.h>
 
+#include "serial_mctrl_gpio.h"
 #include "stm32-usart.h"
 
 static void stm32_stop_tx(struct uart_port *port);
@@ -510,12 +511,29 @@ static void stm32_set_mctrl(struct uart_port *port, unsigned int mctrl)
 		stm32_set_bits(port, ofs->cr3, USART_CR3_RTSE);
 	else
 		stm32_clr_bits(port, ofs->cr3, USART_CR3_RTSE);
+
+	mctrl_gpio_set(stm32_port->gpios, mctrl);
 }
 
 static unsigned int stm32_get_mctrl(struct uart_port *port)
 {
+	struct stm32_port *stm32_port = to_stm32_port(port);
+	int ret;
+
 	/* This routine is used to get signals of: DCD, DSR, RI, and CTS */
-	return TIOCM_CAR | TIOCM_DSR | TIOCM_CTS;
+	ret = TIOCM_CAR | TIOCM_DSR | TIOCM_CTS;
+
+	return mctrl_gpio_get(stm32_port->gpios, &ret);
+}
+
+static void stm32_enable_ms(struct uart_port *port)
+{
+	mctrl_gpio_enable_ms(to_stm32_port(port)->gpios);
+}
+
+static void stm32_disable_ms(struct uart_port *port)
+{
+	mctrl_gpio_disable_ms(to_stm32_port(port)->gpios);
 }
 
 /* Transmit stop */
@@ -626,6 +644,9 @@ static void stm32_shutdown(struct uart_port *port)
 	u32 val, isr;
 	int ret;
 
+	/* Disable modem control interrupts */
+	stm32_disable_ms(port);
+
 	val = USART_CR1_TXEIE | USART_CR1_TE;
 	val |= stm32_port->cr1_irq | USART_CR1_RE;
 	val |= BIT(cfg->uart_enable_bit);
@@ -764,6 +785,12 @@ static void stm32_set_termios(struct uart_port *port, struct ktermios *termios,
 		cr3 |= USART_CR3_CTSE | USART_CR3_RTSE;
 	}
 
+	/* Handle modem control interrupts */
+	if (UART_ENABLE_MS(port, termios->c_cflag))
+		stm32_enable_ms(port);
+	else
+		stm32_disable_ms(port);
+
 	usartdiv = DIV_ROUND_CLOSEST(port->uartclk, baud);
 
 	/*
@@ -898,6 +925,7 @@ static const struct uart_ops stm32_uart_ops = {
 	.throttle	= stm32_throttle,
 	.unthrottle	= stm32_unthrottle,
 	.stop_rx	= stm32_stop_rx,
+	.enable_ms	= stm32_enable_ms,
 	.break_ctl	= stm32_break_ctl,
 	.startup	= stm32_startup,
 	.shutdown	= stm32_shutdown,
@@ -964,6 +992,19 @@ static int stm32_init_port(struct stm32_port *stm32port,
 		ret = -EINVAL;
 	}
 
+	stm32port->gpios = mctrl_gpio_init(&stm32port->port, 0);
+	if (IS_ERR(stm32port->gpios))
+		return PTR_ERR(stm32port->gpios);
+
+	/* Both CTS/RTS gpios and "st,hw-flow-ctrl" should not be specified */
+	if (stm32port->hw_flow_control) {
+		if (mctrl_gpio_to_gpiod(stm32port->gpios, UART_GPIO_CTS) ||
+		    mctrl_gpio_to_gpiod(stm32port->gpios, UART_GPIO_RTS)) {
+			dev_err(&pdev->dev, "Conflicting RTS/CTS config\n");
+			return -EINVAL;
+		}
+	}
+
 	return ret;
 }
 
diff --git a/drivers/tty/serial/stm32-usart.h b/drivers/tty/serial/stm32-usart.h
index db8bf0d4982d..d4c916e78d40 100644
--- a/drivers/tty/serial/stm32-usart.h
+++ b/drivers/tty/serial/stm32-usart.h
@@ -274,6 +274,7 @@ struct stm32_port {
 	bool fifoen;
 	int wakeirq;
 	int rdr_mask;		/* receive data register mask */
+	struct mctrl_gpios *gpios; /* modem control gpios */
 };
 
 static struct stm32_port stm32_ports[STM32_MAX_PORTS];
-- 
2.17.1

[PATCH v2 2/2] dt-bindings: serial: Document CTS/RTS gpios in STM32 UART

[mani]

From: Manivannan Sadhasivam <mani@kernel.org>

Document the use of CTS/RTS gpios for flow control in STM32 UART
controller. These properties can be used instead of 'st,hw-flow-ctrl'
for making use of any gpio pins for flow control instead of dedicated
pins. It should be noted that both CTS/RTS and 'st,hw-flow-ctrl'
properties cannot co-exist in a design.

Signed-off-by: Manivannan Sadhasivam <mani@kernel.org>
---
 .../devicetree/bindings/serial/st,stm32-uart.yaml  | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/Documentation/devicetree/bindings/serial/st,stm32-uart.yaml b/Documentation/devicetree/bindings/serial/st,stm32-uart.yaml
index 238c44192d31..75b8521eb7cb 100644
--- a/Documentation/devicetree/bindings/serial/st,stm32-uart.yaml
+++ b/Documentation/devicetree/bindings/serial/st,stm32-uart.yaml
@@ -48,6 +48,12 @@ properties:
     minItems: 1
     maxItems: 2
 
+  cts-gpios:
+    maxItems: 1
+
+  rts-gpios:
+    maxItems: 1
+
   wakeup-source: true
 
   rs485-rts-delay: true
@@ -55,6 +61,14 @@ properties:
   linux,rs485-enabled-at-boot-time: true
   rs485-rx-during-tx: true
 
+if:
+  required:
+    - st,hw-flow-ctrl
+then:
+  properties:
+    cts-gpios: false
+    rts-gpios: false
+
 required:
   - compatible
   - reg
-- 
2.17.1

Re: [PATCH v2 0/2] Add CTS/RTS gpio support to STM32 UART

[Andy Shevchenko]

On Thu, Apr 16, 2020 at 8:58 PM <mani@kernel.org> wrote:
>
> From: Manivannan Sadhasivam <mani@kernel.org>
>
> Hello,
>
> This patchset adds CTS/RTS gpio support to STM32 UART controller.
> Eventhough the UART controller supports using dedicated CTS/RTS gpios,
> sometimes we need to use different set of gpios for flow control.
>
> This is necessary for the upcoming STM32MP1 based board called Stinger96
> IoT-Box. On that board, a bluetooth chip is connected to one of the UART
> controller but the CTS/RTS lines got swapped mistakenly. So this patchset
> serves as a workaround for that hardware bug and also supports the
> usecase of using any gpio for CTS/RTS functionality. As per the sugggestion
> provided by Andy for v1, I've now switched to mctrl_gpio driver.
>
> This patchset has been validated with Stinger96 IoT-Box connected to Murata
> WiFi-BT combo chip.
>

Looks good to me,
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

> Thanks,
> Mani
>
> Changes in v2:
>
> As per the review by Andy:
>
> * Switched to mctrl_gpio driver instead of using custom CTS/RTS
>   implementation
> * Removed the use of software flow control terminology.
>
> Manivannan Sadhasivam (2):
>   tty: serial: Add modem control gpio support for STM32 UART
>   dt-bindings: serial: Document CTS/RTS gpios in STM32 UART
>
>  .../bindings/serial/st,stm32-uart.yaml        | 14 ++++++
>  drivers/tty/serial/Kconfig                    |  1 +
>  drivers/tty/serial/stm32-usart.c              | 43 ++++++++++++++++++-
>  drivers/tty/serial/stm32-usart.h              |  1 +
>  4 files changed, 58 insertions(+), 1 deletion(-)
>
> --
> 2.17.1
>


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 1/2] tty: serial: Add modem control gpio support for STM32 UART

[Fabrice Gasnier]

On 4/16/20 7:57 PM, mani@kernel.org wrote:
> From: Manivannan Sadhasivam <mani@kernel.org>
> 
> STM32 UART controllers have the built in modem control support using
> dedicated gpios which can be enabled using 'st,hw-flow-ctrl' flag in DT.
> But there might be cases where the board design need to use different
> gpios for modem control.
> 
> For supporting such cases, this commit adds modem control gpio support
> to STM32 UART controller using mctrl_gpio driver.
> 
> Signed-off-by: Manivannan Sadhasivam <mani@kernel.org>
> ---
>  drivers/tty/serial/Kconfig       |  1 +
>  drivers/tty/serial/stm32-usart.c | 43 +++++++++++++++++++++++++++++++-
>  drivers/tty/serial/stm32-usart.h |  1 +
>  3 files changed, 44 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
> index 0aea76cd67ff..e7a6f2130684 100644
> --- a/drivers/tty/serial/Kconfig
> +++ b/drivers/tty/serial/Kconfig
> @@ -1462,6 +1462,7 @@ config SERIAL_STM32
>  	tristate "STMicroelectronics STM32 serial port support"
>  	select SERIAL_CORE
>  	depends on ARCH_STM32 || COMPILE_TEST
> +	select SERIAL_MCTRL_GPIO if GPIOLIB
>  	help
>  	  This driver is for the on-chip Serial Controller on
>  	  STMicroelectronics STM32 MCUs.
> diff --git a/drivers/tty/serial/stm32-usart.c b/drivers/tty/serial/stm32-usart.c
> index 5e93e8d40f59..026982259714 100644
> --- a/drivers/tty/serial/stm32-usart.c
> +++ b/drivers/tty/serial/stm32-usart.c
> @@ -31,6 +31,7 @@
>  #include <linux/tty_flip.h>
>  #include <linux/tty.h>
>  
> +#include "serial_mctrl_gpio.h"
>  #include "stm32-usart.h"
>  
>  static void stm32_stop_tx(struct uart_port *port);
> @@ -510,12 +511,29 @@ static void stm32_set_mctrl(struct uart_port *port, unsigned int mctrl)
>  		stm32_set_bits(port, ofs->cr3, USART_CR3_RTSE);
>  	else
>  		stm32_clr_bits(port, ofs->cr3, USART_CR3_RTSE);
> +
> +	mctrl_gpio_set(stm32_port->gpios, mctrl);
>  }
>  
>  static unsigned int stm32_get_mctrl(struct uart_port *port)
>  {
> +	struct stm32_port *stm32_port = to_stm32_port(port);
> +	int ret;

Hi Mani,

Please find few minor remarks and a question from my side.

'ret' could be an unsigned int

> +
>  	/* This routine is used to get signals of: DCD, DSR, RI, and CTS */
> -	return TIOCM_CAR | TIOCM_DSR | TIOCM_CTS;
> +	ret = TIOCM_CAR | TIOCM_DSR | TIOCM_CTS;
> +
> +	return mctrl_gpio_get(stm32_port->gpios, &ret);
> +}
> +
> +static void stm32_enable_ms(struct uart_port *port)
> +{

Just a question here: purpose of your patch is to handle the gpio case.
So you may get modem control interrupts from gpios with this patch.

In other drivers, I can see the implementation checks gpio usage (like
in atmel_serial). When there's no gpio, the corresponding interrupt at
the serial controller level is enabled, e.g. :

	if (!mctrl_gpio_to_gpiod(atmel_port->gpios, UART_GPIO_CTS))
		ier |= ...

Do you need modem control interrupts in your case ?

In case the Stinger96 board signals gets fixed in a future revision,
would it be needed to enable modem control interrupts in the USART
controller ?

> +	mctrl_gpio_enable_ms(to_stm32_port(port)->gpios);
> +}
> +
> +static void stm32_disable_ms(struct uart_port *port)
> +{
> +	mctrl_gpio_disable_ms(to_stm32_port(port)->gpios);
>  }
>  
>  /* Transmit stop */
> @@ -626,6 +644,9 @@ static void stm32_shutdown(struct uart_port *port)
>  	u32 val, isr;
>  	int ret;
>  
> +	/* Disable modem control interrupts */
> +	stm32_disable_ms(port);
> +
>  	val = USART_CR1_TXEIE | USART_CR1_TE;
>  	val |= stm32_port->cr1_irq | USART_CR1_RE;
>  	val |= BIT(cfg->uart_enable_bit);
> @@ -764,6 +785,12 @@ static void stm32_set_termios(struct uart_port *port, struct ktermios *termios,
>  		cr3 |= USART_CR3_CTSE | USART_CR3_RTSE;
>  	}
>  
> +	/* Handle modem control interrupts */
> +	if (UART_ENABLE_MS(port, termios->c_cflag))
> +		stm32_enable_ms(port);
> +	else
> +		stm32_disable_ms(port);
> +
>  	usartdiv = DIV_ROUND_CLOSEST(port->uartclk, baud);
>  
>  	/*
> @@ -898,6 +925,7 @@ static const struct uart_ops stm32_uart_ops = {
>  	.throttle	= stm32_throttle,
>  	.unthrottle	= stm32_unthrottle,
>  	.stop_rx	= stm32_stop_rx,
> +	.enable_ms	= stm32_enable_ms,
>  	.break_ctl	= stm32_break_ctl,
>  	.startup	= stm32_startup,
>  	.shutdown	= stm32_shutdown,
> @@ -964,6 +992,19 @@ static int stm32_init_port(struct stm32_port *stm32port,
>  		ret = -EINVAL;

return -EINVAL;

>  	}
>  
> +	stm32port->gpios = mctrl_gpio_init(&stm32port->port, 0);
> +	if (IS_ERR(stm32port->gpios))

Please add error path: add a clk_disable_unprepare() here, before the
return.

> +		return PTR_ERR(stm32port->gpios);
> +
> +	/* Both CTS/RTS gpios and "st,hw-flow-ctrl" should not be specified */
> +	if (stm32port->hw_flow_control) {
> +		if (mctrl_gpio_to_gpiod(stm32port->gpios, UART_GPIO_CTS) ||
> +		    mctrl_gpio_to_gpiod(stm32port->gpios, UART_GPIO_RTS)) {
> +			dev_err(&pdev->dev, "Conflicting RTS/CTS config\n");

same here

Best Regards,
Thanks,
Fabrice

> +			return -EINVAL;
> +		}
> +	}
> +
>  	return ret;
>  }
>  
> diff --git a/drivers/tty/serial/stm32-usart.h b/drivers/tty/serial/stm32-usart.h
> index db8bf0d4982d..d4c916e78d40 100644
> --- a/drivers/tty/serial/stm32-usart.h
> +++ b/drivers/tty/serial/stm32-usart.h
> @@ -274,6 +274,7 @@ struct stm32_port {
>  	bool fifoen;
>  	int wakeirq;
>  	int rdr_mask;		/* receive data register mask */
> +	struct mctrl_gpios *gpios; /* modem control gpios */
>  };
>  
>  static struct stm32_port stm32_ports[STM32_MAX_PORTS];
> 

Re: [PATCH v2 1/2] tty: serial: Add modem control gpio support for STM32 UART

[Manivannan Sadhasivam]

Hi,

On Fri, Apr 17, 2020 at 03:15:07PM +0200, Fabrice Gasnier wrote:
> On 4/16/20 7:57 PM, mani@kernel.org wrote:
> > From: Manivannan Sadhasivam <mani@kernel.org>
> > 
> > STM32 UART controllers have the built in modem control support using
> > dedicated gpios which can be enabled using 'st,hw-flow-ctrl' flag in DT.
> > But there might be cases where the board design need to use different
> > gpios for modem control.
> > 
> > For supporting such cases, this commit adds modem control gpio support
> > to STM32 UART controller using mctrl_gpio driver.
> > 
> > Signed-off-by: Manivannan Sadhasivam <mani@kernel.org>
> > ---
> >  drivers/tty/serial/Kconfig       |  1 +
> >  drivers/tty/serial/stm32-usart.c | 43 +++++++++++++++++++++++++++++++-
> >  drivers/tty/serial/stm32-usart.h |  1 +
> >  3 files changed, 44 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
> > index 0aea76cd67ff..e7a6f2130684 100644
> > --- a/drivers/tty/serial/Kconfig
> > +++ b/drivers/tty/serial/Kconfig
> > @@ -1462,6 +1462,7 @@ config SERIAL_STM32
> >  	tristate "STMicroelectronics STM32 serial port support"
> >  	select SERIAL_CORE
> >  	depends on ARCH_STM32 || COMPILE_TEST
> > +	select SERIAL_MCTRL_GPIO if GPIOLIB
> >  	help
> >  	  This driver is for the on-chip Serial Controller on
> >  	  STMicroelectronics STM32 MCUs.
> > diff --git a/drivers/tty/serial/stm32-usart.c b/drivers/tty/serial/stm32-usart.c
> > index 5e93e8d40f59..026982259714 100644
> > --- a/drivers/tty/serial/stm32-usart.c
> > +++ b/drivers/tty/serial/stm32-usart.c
> > @@ -31,6 +31,7 @@
> >  #include <linux/tty_flip.h>
> >  #include <linux/tty.h>
> >  
> > +#include "serial_mctrl_gpio.h"
> >  #include "stm32-usart.h"
> >  
> >  static void stm32_stop_tx(struct uart_port *port);
> > @@ -510,12 +511,29 @@ static void stm32_set_mctrl(struct uart_port *port, unsigned int mctrl)
> >  		stm32_set_bits(port, ofs->cr3, USART_CR3_RTSE);
> >  	else
> >  		stm32_clr_bits(port, ofs->cr3, USART_CR3_RTSE);
> > +
> > +	mctrl_gpio_set(stm32_port->gpios, mctrl);
> >  }
> >  
> >  static unsigned int stm32_get_mctrl(struct uart_port *port)
> >  {
> > +	struct stm32_port *stm32_port = to_stm32_port(port);
> > +	int ret;
> 
> Hi Mani,
> 
> Please find few minor remarks and a question from my side.
> 
> 'ret' could be an unsigned int
>

Ok
 
> > +
> >  	/* This routine is used to get signals of: DCD, DSR, RI, and CTS */
> > -	return TIOCM_CAR | TIOCM_DSR | TIOCM_CTS;
> > +	ret = TIOCM_CAR | TIOCM_DSR | TIOCM_CTS;
> > +
> > +	return mctrl_gpio_get(stm32_port->gpios, &ret);
> > +}
> > +
> > +static void stm32_enable_ms(struct uart_port *port)
> > +{
> 
> Just a question here: purpose of your patch is to handle the gpio case.
> So you may get modem control interrupts from gpios with this patch.
> 

We will only get CTS gpio interrupt which will get handled by mctrl_gpio driver.

> In other drivers, I can see the implementation checks gpio usage (like
> in atmel_serial). When there's no gpio, the corresponding interrupt at
> the serial controller level is enabled, e.g. :
> 
> 	if (!mctrl_gpio_to_gpiod(atmel_port->gpios, UART_GPIO_CTS))
> 		ier |= ...
> 

Those drivers support both gpio and hardware modem control interrupts, so
they are checking to make sure either one of them is enabled at a time. But
on STM32 UART, modem interrupts are not enabled (USART_CR3_CTSIE etc...)

> Do you need modem control interrupts in your case ?
> 

Nope. Since modem control interrupts in UART block are not enabled currently,
I don't bother about it.

> In case the Stinger96 board signals gets fixed in a future revision,
> would it be needed to enable modem control interrupts in the USART
> controller ?
> 

I don't think so. The current driver supports hardware flow control without
using interrupts, so we are fine.

> > +	mctrl_gpio_enable_ms(to_stm32_port(port)->gpios);
> > +}
> > +
> > +static void stm32_disable_ms(struct uart_port *port)
> > +{
> > +	mctrl_gpio_disable_ms(to_stm32_port(port)->gpios);
> >  }
> >  
> >  /* Transmit stop */
> > @@ -626,6 +644,9 @@ static void stm32_shutdown(struct uart_port *port)
> >  	u32 val, isr;
> >  	int ret;
> >  
> > +	/* Disable modem control interrupts */
> > +	stm32_disable_ms(port);
> > +
> >  	val = USART_CR1_TXEIE | USART_CR1_TE;
> >  	val |= stm32_port->cr1_irq | USART_CR1_RE;
> >  	val |= BIT(cfg->uart_enable_bit);
> > @@ -764,6 +785,12 @@ static void stm32_set_termios(struct uart_port *port, struct ktermios *termios,
> >  		cr3 |= USART_CR3_CTSE | USART_CR3_RTSE;
> >  	}
> >  
> > +	/* Handle modem control interrupts */
> > +	if (UART_ENABLE_MS(port, termios->c_cflag))
> > +		stm32_enable_ms(port);
> > +	else
> > +		stm32_disable_ms(port);
> > +
> >  	usartdiv = DIV_ROUND_CLOSEST(port->uartclk, baud);
> >  
> >  	/*
> > @@ -898,6 +925,7 @@ static const struct uart_ops stm32_uart_ops = {
> >  	.throttle	= stm32_throttle,
> >  	.unthrottle	= stm32_unthrottle,
> >  	.stop_rx	= stm32_stop_rx,
> > +	.enable_ms	= stm32_enable_ms,
> >  	.break_ctl	= stm32_break_ctl,
> >  	.startup	= stm32_startup,
> >  	.shutdown	= stm32_shutdown,
> > @@ -964,6 +992,19 @@ static int stm32_init_port(struct stm32_port *stm32port,
> >  		ret = -EINVAL;
> 
> return -EINVAL;
> 
> >  	}
> >  
> > +	stm32port->gpios = mctrl_gpio_init(&stm32port->port, 0);
> > +	if (IS_ERR(stm32port->gpios))
> 
> Please add error path: add a clk_disable_unprepare() here, before the
> return.
> 

Ok.

Thanks,
Mani

> > +		return PTR_ERR(stm32port->gpios);
> > +
> > +	/* Both CTS/RTS gpios and "st,hw-flow-ctrl" should not be specified */
> > +	if (stm32port->hw_flow_control) {
> > +		if (mctrl_gpio_to_gpiod(stm32port->gpios, UART_GPIO_CTS) ||
> > +		    mctrl_gpio_to_gpiod(stm32port->gpios, UART_GPIO_RTS)) {
> > +			dev_err(&pdev->dev, "Conflicting RTS/CTS config\n");
> 
> same here
> 
> Best Regards,
> Thanks,
> Fabrice
> 
> > +			return -EINVAL;
> > +		}
> > +	}
> > +
> >  	return ret;
> >  }
> >  
> > diff --git a/drivers/tty/serial/stm32-usart.h b/drivers/tty/serial/stm32-usart.h
> > index db8bf0d4982d..d4c916e78d40 100644
> > --- a/drivers/tty/serial/stm32-usart.h
> > +++ b/drivers/tty/serial/stm32-usart.h
> > @@ -274,6 +274,7 @@ struct stm32_port {
> >  	bool fifoen;
> >  	int wakeirq;
> >  	int rdr_mask;		/* receive data register mask */
> > +	struct mctrl_gpios *gpios; /* modem control gpios */
> >  };
> >  
> >  static struct stm32_port stm32_ports[STM32_MAX_PORTS];
> >