diff mbox series

[v3,06/11] serial: sc16is7xx: fix bug when first setting GPIO direction

Message ID 20230525040324.3773741-7-hugo@hugovil.com
State New
Headers
Series serial: sc16is7xx: fix GPIO regression and rs485 improvements |

Commit Message

Hugo Villeneuve May 25, 2023, 4:03 a.m. UTC 
  From: Hugo Villeneuve <hvilleneuve@dimonoff.com>

When we want to configure a pin as an output pin with a value of logic
0, we end up as having a value of logic 1 on the output pin. Setting a
logic 0 a second time (or more) after that will correctly output a
logic 0 on the output pin.

By default, all GPIO pins are configured as inputs. When we enter
c16is7xx_gpio_direction_output() for the first time, we first set the
desired value in IOSTATE, and then we configure the pin as an output.
The datasheet states that writing to IOSTATE register will trigger a
transfer of the value to the I/O pin configured as output, so if the
pin is configured as an input, nothing will be transferred.

Therefore, set the direction first in IODIR, and then set the desired
value in IOSTATE.

This is what is done in NXP application note AN10587.

Fixes: dfeae619d781 ("serial: sc16is7xx")
Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com>
---
 drivers/tty/serial/sc16is7xx.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

Comments

Andy Shevchenko May 25, 2023, 11:10 a.m. UTC | #1 
Thu, May 25, 2023 at 12:03:20AM -0400, Hugo Villeneuve kirjoitti:
> From: Hugo Villeneuve <hvilleneuve@dimonoff.com>
> 
> When we want to configure a pin as an output pin with a value of logic
> 0, we end up as having a value of logic 1 on the output pin. Setting a
> logic 0 a second time (or more) after that will correctly output a
> logic 0 on the output pin.
> 
> By default, all GPIO pins are configured as inputs. When we enter
> c16is7xx_gpio_direction_output() for the first time, we first set the

Missing 's'.

> desired value in IOSTATE, and then we configure the pin as an output.
> The datasheet states that writing to IOSTATE register will trigger a
> transfer of the value to the I/O pin configured as output, so if the
> pin is configured as an input, nothing will be transferred.
> 
> Therefore, set the direction first in IODIR, and then set the desired
> value in IOSTATE.
> 
> This is what is done in NXP application note AN10587.
Hugo Villeneuve May 25, 2023, 2:24 p.m. UTC | #2 
On Thu, 25 May 2023 14:10:27 +0300
andy.shevchenko@gmail.com wrote:

> Thu, May 25, 2023 at 12:03:20AM -0400, Hugo Villeneuve kirjoitti:
> > From: Hugo Villeneuve <hvilleneuve@dimonoff.com>
> > 
> > When we want to configure a pin as an output pin with a value of logic
> > 0, we end up as having a value of logic 1 on the output pin. Setting a
> > logic 0 a second time (or more) after that will correctly output a
> > logic 0 on the output pin.
> > 
> > By default, all GPIO pins are configured as inputs. When we enter
> > c16is7xx_gpio_direction_output() for the first time, we first set the
> 
> Missing 's'.

Fixed.

> > desired value in IOSTATE, and then we configure the pin as an output.
> > The datasheet states that writing to IOSTATE register will trigger a
> > transfer of the value to the I/O pin configured as output, so if the
> > pin is configured as an input, nothing will be transferred.
> > 
> > Therefore, set the direction first in IODIR, and then set the desired
> > value in IOSTATE.
> > 
> > This is what is done in NXP application note AN10587.
> 
> -- 
> With Best Regards,
> Andy Shevchenko
> 
> 
>
diff mbox series

Patch

diff --git a/drivers/tty/serial/sc16is7xx.c b/drivers/tty/serial/sc16is7xx.c
index 8a2fc6f89d36..a5d8af0f6da0 100644
--- a/drivers/tty/serial/sc16is7xx.c
+++ b/drivers/tty/serial/sc16is7xx.c
@@ -1343,9 +1343,18 @@  static int sc16is7xx_gpio_direction_output(struct gpio_chip *chip,
 		state |= BIT(offset);
 	else
 		state &= ~BIT(offset);
-	sc16is7xx_port_write(port, SC16IS7XX_IOSTATE_REG, state);
+
+	/*
+	 * If we write IOSTATE first, and then IODIR, the output value is not
+	 * transferred to the corresponding I/O pin.
+	 * The datasheet states that each register bit will be transferred to
+	 * the corresponding I/O pin programmed as output when writing to
+	 * IOSTATE. Therefore, configure direction first with IODIR, and then
+	 * set value after with IOSTATE.
+	 */
 	sc16is7xx_port_update(port, SC16IS7XX_IODIR_REG, BIT(offset),
 			      BIT(offset));
+	sc16is7xx_port_write(port, SC16IS7XX_IOSTATE_REG, state);
 
 	return 0;
 }