2023-06-01 16:43:50

by Hugo Villeneuve

[permalink] [raw]
Subject: [PATCH v5 0/9] serial: sc16is7xx: fix GPIO regression and rs485 improvements

From: Hugo Villeneuve <[email protected]>

Hello,
this patch series mainly fixes a GPIO regression and improve RS485 flags and
properties detection from DT.

It now also includes various small fixes and improvements that were previously
sent as separate patches, but that made testing everything difficult.

Patch 1 fixes an issue with init of first port during probing.

Patch 2 fixes an issue when debugging IOcontrol register, but it is also
necessary for patch "fix regression with GPIO configuration" to work.

Patch 3 is a refactor of GPIO registration code in preparation for patch 5.

Patches 4 and 5 fix a GPIO regression by (re)allowing to choose GPIO function
for GPIO pins shared with modem status lines.

Patch 6 fixes a bug with the output value when first setting the GPIO direction.

Patch 7 allows to read common rs485 device-tree flags and properties.

Patch 8 introduces a delay after a reset operation to respect datasheet
timing recommandations.

Patch 9 improves comments about chip variants.

I have tested the changes on a custom board with two SC16IS752 DUART using a
Variscite IMX8MN NANO SOM.

Thank you.

Link: [v1] https://lkml.org/lkml/2023/5/17/967
[v1] https://lkml.org/lkml/2023/5/17/777
[v1] https://lkml.org/lkml/2023/5/17/780
[v1] https://lkml.org/lkml/2023/5/17/785
[v1] https://lkml.org/lkml/2023/5/17/1311
[v2] https://lkml.org/lkml/2023/5/18/516
[v3] https://lkml.org/lkml/2023/5/25/7
[v4] https://lkml.org/lkml/2023/5/29/656

Changes for V3:
- Integrated all patches into single serie to facilitate debugging and tests.
- Reduce number of exported GPIOs depending on new property
nxp,modem-control-line-ports
- Added additional example in DT bindings

Changes for V4:
- Increase reset post delay to relax scheduler.
- Put comments patches at the end.
- Remove Fixes tag for patch "mark IOCONTROL register as volatile".
- Improve commit messages after reviews.
- Fix coding style issues after reviews.
- Change GPIO registration to always register the maximum number of GPIOs
supported by the chip, but maks-out GPIOs declared as modem control lines.
- Add patch to refactor GPIO registration.
- Remove patch "serial: sc16is7xx: fix syntax error in comments".
- Remove patch "add dump registers function"

Changes for V5:
- Change patch order to facilitate stable backport(s).
- Change duplicate device addresses in DT binding examples.
- Use GENMASK for bit masks.
- Replace of_property_for_each_u32() with device_property_read_u32_array
- Add "Cc: stable..." tags

Hugo Villeneuve (9):
serial: sc16is7xx: fix broken port 0 uart init
serial: sc16is7xx: mark IOCONTROL register as volatile
serial: sc16is7xx: refactor GPIO controller registration
dt-bindings: sc16is7xx: Add property to change GPIO function
serial: sc16is7xx: fix regression with GPIO configuration
serial: sc16is7xx: fix bug when first setting GPIO direction
serial: sc16is7xx: add call to get rs485 DT flags and properties
serial: sc16is7xx: add post reset delay
serial: sc16is7xx: improve comments about variants

.../bindings/serial/nxp,sc16is7xx.txt | 46 +++++
drivers/tty/serial/sc16is7xx.c | 168 +++++++++++++-----
2 files changed, 174 insertions(+), 40 deletions(-)


base-commit: 929ed21dfdb6ee94391db51c9eedb63314ef6847
--
2.30.2



2023-06-01 16:52:40

by Hugo Villeneuve

[permalink] [raw]
Subject: [PATCH v5 1/9] serial: sc16is7xx: fix broken port 0 uart init

From: Hugo Villeneuve <[email protected]>

The sc16is7xx_config_rs485() function is called only for the second
port (index 1, channel B), causing initialization problems for the
first port.

For the sc16is7xx driver, port->membase and port->mapbase are not set,
and their default values are 0. And we set port->iobase to the device
index. This means that when the first device is registered using the
uart_add_one_port() function, the following values will be in the port
structure:
port->membase = 0
port->mapbase = 0
port->iobase = 0

Therefore, the function uart_configure_port() in serial_core.c will
exit early because of the following check:
/*
* If there isn't a port here, don't do anything further.
*/
if (!port->iobase && !port->mapbase && !port->membase)
return;

Typically, I2C and SPI drivers do not set port->membase and
port->mapbase.

The max310x driver sets port->membase to ~0 (all ones). By
implementing the same change in this driver, uart_configure_port() is
now correctly executed for all ports.

Fixes: dfeae619d781 ("serial: sc16is7xx")
Cc: <[email protected]> # 6.1.x
Signed-off-by: Hugo Villeneuve <[email protected]>
Reviewed-by: Ilpo Järvinen <[email protected]>
Reviewed-by: Lech Perczak <[email protected]>
Tested-by: Lech Perczak <[email protected]>
---
drivers/tty/serial/sc16is7xx.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/drivers/tty/serial/sc16is7xx.c b/drivers/tty/serial/sc16is7xx.c
index abad091baeea..faa51a58671f 100644
--- a/drivers/tty/serial/sc16is7xx.c
+++ b/drivers/tty/serial/sc16is7xx.c
@@ -1436,6 +1436,7 @@ static int sc16is7xx_probe(struct device *dev,
s->p[i].port.fifosize = SC16IS7XX_FIFO_SIZE;
s->p[i].port.flags = UPF_FIXED_TYPE | UPF_LOW_LATENCY;
s->p[i].port.iobase = i;
+ s->p[i].port.membase = (void __iomem *)~0;
s->p[i].port.iotype = UPIO_PORT;
s->p[i].port.uartclk = freq;
s->p[i].port.rs485_config = sc16is7xx_config_rs485;
--
2.30.2


2023-06-01 16:54:00

by Hugo Villeneuve

[permalink] [raw]
Subject: [PATCH v5 3/9] serial: sc16is7xx: refactor GPIO controller registration

From: Hugo Villeneuve <[email protected]>

In preparation for upcoming patch "fix regression with GPIO
configuration". To facilitate review and make code more modular.

Cc: <[email protected]> # 6.1.x
Signed-off-by: Hugo Villeneuve <[email protected]>
Reviewed-by: Lech Perczak <[email protected]>
Tested-by: Lech Perczak <[email protected]>
---
drivers/tty/serial/sc16is7xx.c | 39 ++++++++++++++++++++--------------
1 file changed, 23 insertions(+), 16 deletions(-)

diff --git a/drivers/tty/serial/sc16is7xx.c b/drivers/tty/serial/sc16is7xx.c
index 0c903d44429c..279d7dcb1447 100644
--- a/drivers/tty/serial/sc16is7xx.c
+++ b/drivers/tty/serial/sc16is7xx.c
@@ -1349,6 +1349,26 @@ static int sc16is7xx_gpio_direction_output(struct gpio_chip *chip,

return 0;
}
+
+static int sc16is7xx_setup_gpio_chip(struct device *dev)
+{
+ struct sc16is7xx_port *s = dev_get_drvdata(dev);
+
+ if (!s->devtype->nr_gpio)
+ return 0;
+
+ s->gpio.owner = THIS_MODULE;
+ s->gpio.parent = dev;
+ s->gpio.label = dev_name(dev);
+ s->gpio.direction_input = sc16is7xx_gpio_direction_input;
+ s->gpio.get = sc16is7xx_gpio_get;
+ s->gpio.direction_output = sc16is7xx_gpio_direction_output;
+ s->gpio.set = sc16is7xx_gpio_set;
+ s->gpio.base = -1;
+ s->gpio.ngpio = s->devtype->nr_gpio;
+ s->gpio.can_sleep = 1;
+ return gpiochip_add_data(&s->gpio, s);
+}
#endif

static const struct serial_rs485 sc16is7xx_rs485_supported = {
@@ -1502,22 +1522,9 @@ static int sc16is7xx_probe(struct device *dev,
}

#ifdef CONFIG_GPIOLIB
- if (devtype->nr_gpio) {
- /* Setup GPIO cotroller */
- s->gpio.owner = THIS_MODULE;
- s->gpio.parent = dev;
- s->gpio.label = dev_name(dev);
- s->gpio.direction_input = sc16is7xx_gpio_direction_input;
- s->gpio.get = sc16is7xx_gpio_get;
- s->gpio.direction_output = sc16is7xx_gpio_direction_output;
- s->gpio.set = sc16is7xx_gpio_set;
- s->gpio.base = -1;
- s->gpio.ngpio = devtype->nr_gpio;
- s->gpio.can_sleep = 1;
- ret = gpiochip_add_data(&s->gpio, s);
- if (ret)
- goto out_thread;
- }
+ ret = sc16is7xx_setup_gpio_chip(dev, mctrl_mask);
+ if (ret)
+ goto out_thread;
#endif

/*
--
2.30.2


2023-06-01 16:55:25

by Hugo Villeneuve

[permalink] [raw]
Subject: [PATCH v5 2/9] serial: sc16is7xx: mark IOCONTROL register as volatile

From: Hugo Villeneuve <[email protected]>

Bit SRESET (3) is cleared when a reset operation is completed. Having
the IOCONTROL register as non-volatile will always read SRESET as 1,
which is incorrect.

Also, if IOCONTROL register is not a volatile register, the upcoming
patch "serial: sc16is7xx: fix regression with GPIO configuration"
doesn't work when setting some shared GPIO lines as modem control
lines.

Therefore mark IOCONTROL register as a volatile register.

Cc: <[email protected]> # 6.1.x
Signed-off-by: Hugo Villeneuve <[email protected]>
Reviewed-by: Lech Perczak <[email protected]>
Tested-by: Lech Perczak <[email protected]>
---
drivers/tty/serial/sc16is7xx.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/drivers/tty/serial/sc16is7xx.c b/drivers/tty/serial/sc16is7xx.c
index faa51a58671f..0c903d44429c 100644
--- a/drivers/tty/serial/sc16is7xx.c
+++ b/drivers/tty/serial/sc16is7xx.c
@@ -488,6 +488,7 @@ static bool sc16is7xx_regmap_volatile(struct device *dev, unsigned int reg)
case SC16IS7XX_TXLVL_REG:
case SC16IS7XX_RXLVL_REG:
case SC16IS7XX_IOSTATE_REG:
+ case SC16IS7XX_IOCONTROL_REG:
return true;
default:
break;
--
2.30.2


2023-06-01 16:55:25

by Hugo Villeneuve

[permalink] [raw]
Subject: [PATCH v5 5/9] serial: sc16is7xx: fix regression with GPIO configuration

From: Hugo Villeneuve <[email protected]>

Commit 679875d1d880 ("sc16is7xx: Separate GPIOs from modem control lines")
and commit 21144bab4f11 ("sc16is7xx: Handle modem status lines")
changed the function of the GPIOs pins to act as modem control
lines without any possibility of selecting GPIO function.

As a consequence, applications that depends on GPIO lines configured
by default as GPIO pins no longer work as expected.

Also, the change to select modem control lines function was done only
for channel A of dual UART variants (752/762). This was not documented
in the log message.

Allow to specify GPIO or modem control line function in the device
tree, and for each of the ports (A or B).

Do so by using the new device-tree property named
"modem-control-line-ports" (property added in separate patch).

When registering GPIO chip controller, mask-out GPIO pins declared as
modem control lines according to this new "modem-control-line-ports"
DT property.

Boards that need to have GPIOS configured as modem control lines
should add that property to their device tree. Here is a list of
boards using the sc16is7xx driver in their device tree and that may
need to be modified:
arm64/boot/dts/freescale/fsl-ls1012a-frdm.dts
mips/boot/dts/ingenic/cu1830-neo.dts
mips/boot/dts/ingenic/cu1000-neo.dts

Fixes: 679875d1d880 ("sc16is7xx: Separate GPIOs from modem control lines")
Fixes: 21144bab4f11 ("sc16is7xx: Handle modem status lines")
Cc: <[email protected]> # 6.1.x: 35210b22 dt-bindings: sc16is7xx: Add property to change GPIO function
Cc: <[email protected]> # 6.1.x: 7d61ca47 serial: sc16is7xx: refactor GPIO controller registration
Cc: <[email protected]> # 6.1.x: 322470ed serial: sc16is7xx: mark IOCONTROL register as volatile
Cc: <[email protected]> # 6.1.x: a0077362 serial: sc16is7xx: fix broken port 0 uart init
Cc: <[email protected]> # 6.1.x
Signed-off-by: Hugo Villeneuve <[email protected]>
---
drivers/tty/serial/sc16is7xx.c | 100 ++++++++++++++++++++++++++-------
1 file changed, 80 insertions(+), 20 deletions(-)

diff --git a/drivers/tty/serial/sc16is7xx.c b/drivers/tty/serial/sc16is7xx.c
index 279d7dcb1447..ad6b9d613b33 100644
--- a/drivers/tty/serial/sc16is7xx.c
+++ b/drivers/tty/serial/sc16is7xx.c
@@ -236,7 +236,8 @@

/* IOControl register bits (Only 750/760) */
#define SC16IS7XX_IOCONTROL_LATCH_BIT (1 << 0) /* Enable input latching */
-#define SC16IS7XX_IOCONTROL_MODEM_BIT (1 << 1) /* Enable GPIO[7:4] as modem pins */
+#define SC16IS7XX_IOCONTROL_MODEM_A_BIT (1 << 1) /* Enable GPIO[7:4] as modem A pins */
+#define SC16IS7XX_IOCONTROL_MODEM_B_BIT (1 << 2) /* Enable GPIO[3:0] as modem B pins */
#define SC16IS7XX_IOCONTROL_SRESET_BIT (1 << 3) /* Software Reset */

/* EFCR register bits */
@@ -301,12 +302,12 @@
/* Misc definitions */
#define SC16IS7XX_FIFO_SIZE (64)
#define SC16IS7XX_REG_SHIFT 2
+#define SC16IS7XX_GPIOS_PER_BANK 4

struct sc16is7xx_devtype {
char name[10];
int nr_gpio;
int nr_uart;
- int has_mctrl;
};

#define SC16IS7XX_RECONF_MD (1 << 0)
@@ -336,6 +337,7 @@ struct sc16is7xx_port {
struct clk *clk;
#ifdef CONFIG_GPIOLIB
struct gpio_chip gpio;
+ unsigned long gpio_valid_mask;
#endif
unsigned char buf[SC16IS7XX_FIFO_SIZE];
struct kthread_worker kworker;
@@ -447,35 +449,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,
.nr_uart = 1,
- .has_mctrl = 1,
};

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

static const struct sc16is7xx_devtype sc16is760_devtype = {
.name = "SC16IS760",
- .nr_gpio = 4,
+ .nr_gpio = 8,
.nr_uart = 1,
- .has_mctrl = 1,
};

static const struct sc16is7xx_devtype sc16is762_devtype = {
.name = "SC16IS762",
- .nr_gpio = 0,
+ .nr_gpio = 8,
.nr_uart = 2,
- .has_mctrl = 1,
};

static bool sc16is7xx_regmap_volatile(struct device *dev, unsigned int reg)
@@ -1350,16 +1347,45 @@ static int sc16is7xx_gpio_direction_output(struct gpio_chip *chip,
return 0;
}

-static int sc16is7xx_setup_gpio_chip(struct device *dev)
+static int sc16is7xx_gpio_init_valid_mask(struct gpio_chip *chip,
+ unsigned long *valid_mask,
+ unsigned int ngpios)
+{
+ struct sc16is7xx_port *s = gpiochip_get_data(chip);
+
+ *valid_mask = s->gpio_valid_mask;
+
+ return 0;
+}
+
+static int sc16is7xx_setup_gpio_chip(struct device *dev, u8 mctrl_mask)
{
struct sc16is7xx_port *s = dev_get_drvdata(dev);

if (!s->devtype->nr_gpio)
return 0;

+ switch (mctrl_mask) {
+ case 0:
+ s->gpio_valid_mask = GENMASK(7, 0);
+ break;
+ case SC16IS7XX_IOCONTROL_MODEM_A_BIT:
+ s->gpio_valid_mask = GENMASK(3, 0);
+ break;
+ case SC16IS7XX_IOCONTROL_MODEM_B_BIT:
+ s->gpio_valid_mask = GENMASK(7, 4);
+ break;
+ default:
+ break;
+ }
+
+ if (!s->gpio_valid_mask)
+ return 0;
+
s->gpio.owner = THIS_MODULE;
s->gpio.parent = dev;
s->gpio.label = dev_name(dev);
+ s->gpio.init_valid_mask = sc16is7xx_gpio_init_valid_mask;
s->gpio.direction_input = sc16is7xx_gpio_direction_input;
s->gpio.get = sc16is7xx_gpio_get;
s->gpio.direction_output = sc16is7xx_gpio_direction_output;
@@ -1371,6 +1397,43 @@ static int sc16is7xx_setup_gpio_chip(struct device *dev)
}
#endif

+static int sc16is7xx_setup_mctrl_ports(struct device *dev)
+{
+ struct sc16is7xx_port *s = dev_get_drvdata(dev);
+ int i;
+ int ret;
+ int count;
+ u32 mctrl_port[2];
+ u8 mctrl_mask = 0;
+
+ count = device_property_count_u32(dev, "nxp,modem-control-line-ports");
+ if (count < 0 || count > ARRAY_SIZE(mctrl_port))
+ return mctrl_mask;
+
+ ret = device_property_read_u32_array(dev, "nxp,modem-control-line-ports",
+ mctrl_port, count);
+ if (ret)
+ return mctrl_mask;
+
+ for (i = 0; i < count; i++) {
+ /* Use GPIO lines as modem control lines */
+ if (mctrl_port[i] == 0)
+ mctrl_mask |= SC16IS7XX_IOCONTROL_MODEM_A_BIT;
+ else if (mctrl_port[i] == 1)
+ mctrl_mask |= SC16IS7XX_IOCONTROL_MODEM_B_BIT;
+ }
+
+ if (!mctrl_mask)
+ return mctrl_mask;
+
+ regmap_update_bits(s->regmap,
+ SC16IS7XX_IOCONTROL_REG << SC16IS7XX_REG_SHIFT,
+ SC16IS7XX_IOCONTROL_MODEM_A_BIT |
+ SC16IS7XX_IOCONTROL_MODEM_B_BIT, mctrl_mask);
+
+ return mctrl_mask;
+}
+
static const struct serial_rs485 sc16is7xx_rs485_supported = {
.flags = SER_RS485_ENABLED | SER_RS485_RTS_AFTER_SEND,
.delay_rts_before_send = 1,
@@ -1383,6 +1446,7 @@ static int sc16is7xx_probe(struct device *dev,
{
unsigned long freq = 0, *pfreq = dev_get_platdata(dev);
unsigned int val;
+ u8 mctrl_mask = 0;
u32 uartclk = 0;
int i, ret;
struct sc16is7xx_port *s;
@@ -1478,12 +1542,6 @@ static int sc16is7xx_probe(struct device *dev,
SC16IS7XX_EFCR_RXDISABLE_BIT |
SC16IS7XX_EFCR_TXDISABLE_BIT);

- /* Use GPIO lines as modem status registers */
- if (devtype->has_mctrl)
- sc16is7xx_port_write(&s->p[i].port,
- SC16IS7XX_IOCONTROL_REG,
- SC16IS7XX_IOCONTROL_MODEM_BIT);
-
/* Initialize kthread work structs */
kthread_init_work(&s->p[i].tx_work, sc16is7xx_tx_proc);
kthread_init_work(&s->p[i].reg_work, sc16is7xx_reg_proc);
@@ -1521,6 +1579,8 @@ static int sc16is7xx_probe(struct device *dev,
s->p[u].irda_mode = true;
}

+ mctrl_mask = sc16is7xx_setup_mctrl_ports(dev);
+
#ifdef CONFIG_GPIOLIB
ret = sc16is7xx_setup_gpio_chip(dev, mctrl_mask);
if (ret)
@@ -1547,7 +1607,7 @@ static int sc16is7xx_probe(struct device *dev,
return 0;

#ifdef CONFIG_GPIOLIB
- if (devtype->nr_gpio)
+ if (s->gpio_valid_mask)
gpiochip_remove(&s->gpio);

out_thread:
@@ -1573,7 +1633,7 @@ static void sc16is7xx_remove(struct device *dev)
int i;

#ifdef CONFIG_GPIOLIB
- if (s->devtype->nr_gpio)
+ if (s->gpio_valid_mask)
gpiochip_remove(&s->gpio);
#endif

--
2.30.2


2023-06-01 16:56:46

by Hugo Villeneuve

[permalink] [raw]
Subject: [PATCH v5 6/9] serial: sc16is7xx: fix bug when first setting GPIO direction

From: Hugo Villeneuve <[email protected]>

When configuring 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
sc16is7xx_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")
Cc: <[email protected]>
Signed-off-by: Hugo Villeneuve <[email protected]>
Reviewed-by: Lech Perczak <[email protected]>
Tested-by: Lech Perczak <[email protected]>
---
drivers/tty/serial/sc16is7xx.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/tty/serial/sc16is7xx.c b/drivers/tty/serial/sc16is7xx.c
index ad6b9d613b33..2fa09baf28e3 100644
--- a/drivers/tty/serial/sc16is7xx.c
+++ b/drivers/tty/serial/sc16is7xx.c
@@ -1340,9 +1340,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;
}
--
2.30.2


2023-06-01 16:58:10

by Hugo Villeneuve

[permalink] [raw]
Subject: [PATCH v5 8/9] serial: sc16is7xx: add post reset delay

From: Hugo Villeneuve <[email protected]>

Make sure we wait at least 3us before initiating communication with
the device after reset.

Signed-off-by: Hugo Villeneuve <[email protected]>
Reviewed-by: Lech Perczak <[email protected]>
Tested-by: Lech Perczak <[email protected]>
---
drivers/tty/serial/sc16is7xx.c | 6 ++++++
1 file changed, 6 insertions(+)

diff --git a/drivers/tty/serial/sc16is7xx.c b/drivers/tty/serial/sc16is7xx.c
index 388c1d490853..9b2d58023c55 100644
--- a/drivers/tty/serial/sc16is7xx.c
+++ b/drivers/tty/serial/sc16is7xx.c
@@ -1521,6 +1521,12 @@ static int sc16is7xx_probe(struct device *dev,
regmap_write(s->regmap, SC16IS7XX_IOCONTROL_REG << SC16IS7XX_REG_SHIFT,
SC16IS7XX_IOCONTROL_SRESET_BIT);

+ /*
+ * After reset, the host must wait at least 3us before initializing a
+ * communication with the device.
+ */
+ usleep_range(5, 10);
+
for (i = 0; i < devtype->nr_uart; ++i) {
s->p[i].line = i;
/* Initialize port data */
--
2.30.2


2023-06-01 16:58:22

by Hugo Villeneuve

[permalink] [raw]
Subject: [PATCH v5 4/9] dt-bindings: sc16is7xx: Add property to change GPIO function

From: Hugo Villeneuve <[email protected]>

Some variants in this series of UART controllers have GPIO pins that
are shared between GPIO and modem control lines.

The pin mux mode (GPIO or modem control lines) can be set for each
ports (channels) supported by the variant.

This adds a property to the device tree to set the GPIO pin mux to
modem control lines on selected ports if needed.

Cc: <[email protected]> # 6.1.x
Signed-off-by: Hugo Villeneuve <[email protected]>
Acked-by: Conor Dooley <[email protected]>
Reviewed-by: Lech Perczak <[email protected]>
---
.../bindings/serial/nxp,sc16is7xx.txt | 46 +++++++++++++++++++
1 file changed, 46 insertions(+)

diff --git a/Documentation/devicetree/bindings/serial/nxp,sc16is7xx.txt b/Documentation/devicetree/bindings/serial/nxp,sc16is7xx.txt
index 0fa8e3e43bf8..1a7e4bff0456 100644
--- a/Documentation/devicetree/bindings/serial/nxp,sc16is7xx.txt
+++ b/Documentation/devicetree/bindings/serial/nxp,sc16is7xx.txt
@@ -23,6 +23,9 @@ Optional properties:
1 = active low.
- irda-mode-ports: An array that lists the indices of the port that
should operate in IrDA mode.
+- nxp,modem-control-line-ports: An array that lists the indices of the port that
+ should have shared GPIO lines configured as
+ modem control lines.

Example:
sc16is750: sc16is750@51 {
@@ -35,6 +38,26 @@ Example:
#gpio-cells = <2>;
};

+ sc16is752: sc16is752@53 {
+ compatible = "nxp,sc16is752";
+ reg = <0x53>;
+ clocks = <&clk20m>;
+ interrupt-parent = <&gpio3>;
+ interrupts = <7 IRQ_TYPE_EDGE_FALLING>;
+ nxp,modem-control-line-ports = <1>; /* Port 1 as modem control lines */
+ gpio-controller; /* Port 0 as GPIOs */
+ #gpio-cells = <2>;
+ };
+
+ sc16is752: sc16is752@54 {
+ compatible = "nxp,sc16is752";
+ reg = <0x54>;
+ clocks = <&clk20m>;
+ interrupt-parent = <&gpio3>;
+ interrupts = <7 IRQ_TYPE_EDGE_FALLING>;
+ nxp,modem-control-line-ports = <0 1>; /* Ports 0 and 1 as modem control lines */
+ };
+
* spi as bus

Required properties:
@@ -59,6 +82,9 @@ Optional properties:
1 = active low.
- irda-mode-ports: An array that lists the indices of the port that
should operate in IrDA mode.
+- nxp,modem-control-line-ports: An array that lists the indices of the port that
+ should have shared GPIO lines configured as
+ modem control lines.

Example:
sc16is750: sc16is750@0 {
@@ -70,3 +96,23 @@ Example:
gpio-controller;
#gpio-cells = <2>;
};
+
+ sc16is752: sc16is752@1 {
+ compatible = "nxp,sc16is752";
+ reg = <1>;
+ clocks = <&clk20m>;
+ interrupt-parent = <&gpio3>;
+ interrupts = <7 IRQ_TYPE_EDGE_FALLING>;
+ nxp,modem-control-line-ports = <1>; /* Port 1 as modem control lines */
+ gpio-controller; /* Port 0 as GPIOs */
+ #gpio-cells = <2>;
+ };
+
+ sc16is752: sc16is752@2 {
+ compatible = "nxp,sc16is752";
+ reg = <2>;
+ clocks = <&clk20m>;
+ interrupt-parent = <&gpio3>;
+ interrupts = <7 IRQ_TYPE_EDGE_FALLING>;
+ nxp,modem-control-line-ports = <0 1>; /* Ports 0 and 1 as modem control lines */
+ };
--
2.30.2


2023-06-01 16:58:54

by Hugo Villeneuve

[permalink] [raw]
Subject: [PATCH v5 9/9] serial: sc16is7xx: improve comments about variants

From: Hugo Villeneuve <[email protected]>

Replace 740/750/760 with generic terms like 74x/75x/76x to account for
variants like 741, 752 and 762.

Signed-off-by: Hugo Villeneuve <[email protected]>
Reviewed-by: Lech Perczak <[email protected]>
---
drivers/tty/serial/sc16is7xx.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/tty/serial/sc16is7xx.c b/drivers/tty/serial/sc16is7xx.c
index 9b2d58023c55..59338f5ca180 100644
--- a/drivers/tty/serial/sc16is7xx.c
+++ b/drivers/tty/serial/sc16is7xx.c
@@ -223,7 +223,7 @@
* trigger levels. Trigger levels from 4 characters to 60 characters are
* available with a granularity of four.
*
- * When the trigger level setting in TLR is zero, the SC16IS740/750/760 uses the
+ * When the trigger level setting in TLR is zero, the SC16IS74x/75x/76x uses the
* trigger level setting defined in FCR. If TLR has non-zero trigger level value
* the trigger level defined in FCR is discarded. This applies to both transmit
* FIFO and receive FIFO trigger level setting.
@@ -234,7 +234,7 @@
#define SC16IS7XX_TLR_TX_TRIGGER(words) ((((words) / 4) & 0x0f) << 0)
#define SC16IS7XX_TLR_RX_TRIGGER(words) ((((words) / 4) & 0x0f) << 4)

-/* IOControl register bits (Only 750/760) */
+/* IOControl register bits (Only 75x/76x) */
#define SC16IS7XX_IOCONTROL_LATCH_BIT (1 << 0) /* Enable input latching */
#define SC16IS7XX_IOCONTROL_MODEM_A_BIT (1 << 1) /* Enable GPIO[7:4] as modem A pins */
#define SC16IS7XX_IOCONTROL_MODEM_B_BIT (1 << 2) /* Enable GPIO[3:0] as modem B pins */
@@ -249,9 +249,9 @@
#define SC16IS7XX_EFCR_RTS_INVERT_BIT (1 << 5) /* RTS output inversion */
#define SC16IS7XX_EFCR_IRDA_MODE_BIT (1 << 7) /* IrDA mode
* 0 = rate upto 115.2 kbit/s
- * - Only 750/760
+ * - Only 75x/76x
* 1 = rate upto 1.152 Mbit/s
- * - Only 760
+ * - Only 76x
*/

/* EFR register bits */
--
2.30.2


2023-06-01 16:58:59

by Hugo Villeneuve

[permalink] [raw]
Subject: [PATCH v5 7/9] serial: sc16is7xx: add call to get rs485 DT flags and properties

From: Hugo Villeneuve <[email protected]>

Add call to uart_get_rs485_mode() to probe for RS485 flags and
properties from device tree.

Signed-off-by: Hugo Villeneuve <[email protected]>
Reviewed-by: Ilpo Järvinen <[email protected]>
Reviewed-by: Lech Perczak <[email protected]>
Tested-by: Lech Perczak <[email protected]>
---
drivers/tty/serial/sc16is7xx.c | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/drivers/tty/serial/sc16is7xx.c b/drivers/tty/serial/sc16is7xx.c
index 2fa09baf28e3..388c1d490853 100644
--- a/drivers/tty/serial/sc16is7xx.c
+++ b/drivers/tty/serial/sc16is7xx.c
@@ -1544,6 +1544,10 @@ static int sc16is7xx_probe(struct device *dev,
goto out_ports;
}

+ ret = uart_get_rs485_mode(&s->p[i].port);
+ if (ret)
+ goto out_ports;
+
/* Disable all interrupts */
sc16is7xx_port_write(&s->p[i].port, SC16IS7XX_IER_REG, 0);
/* Disable TX/RX */
--
2.30.2


2023-06-01 18:19:43

by Maarten Brock

[permalink] [raw]
Subject: Re: [PATCH v5 3/9] serial: sc16is7xx: refactor GPIO controller registration

Hugo Villeneuve schreef op 2023-06-01 18:31:
> From: Hugo Villeneuve <[email protected]>
>
> In preparation for upcoming patch "fix regression with GPIO
> configuration". To facilitate review and make code more modular.
>
> Cc: <[email protected]> # 6.1.x
> Signed-off-by: Hugo Villeneuve <[email protected]>
> Reviewed-by: Lech Perczak <[email protected]>
> Tested-by: Lech Perczak <[email protected]>
> ---
> drivers/tty/serial/sc16is7xx.c | 39 ++++++++++++++++++++--------------
> 1 file changed, 23 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/tty/serial/sc16is7xx.c
> b/drivers/tty/serial/sc16is7xx.c
> index 0c903d44429c..279d7dcb1447 100644
> --- a/drivers/tty/serial/sc16is7xx.c
> +++ b/drivers/tty/serial/sc16is7xx.c
> @@ -1349,6 +1349,26 @@ static int
> sc16is7xx_gpio_direction_output(struct gpio_chip *chip,
>
> return 0;
> }
> +
> +static int sc16is7xx_setup_gpio_chip(struct device *dev)

Only one parameter, but...

> + ret = sc16is7xx_setup_gpio_chip(dev, mctrl_mask);

called with two.

Maarten


2023-06-01 18:49:33

by Hugo Villeneuve

[permalink] [raw]
Subject: Re: [PATCH v5 3/9] serial: sc16is7xx: refactor GPIO controller registration

On Thu, 01 Jun 2023 19:28:01 +0200
[email protected] wrote:

> Hugo Villeneuve schreef op 2023-06-01 18:31:
> > From: Hugo Villeneuve <[email protected]>
> >
> > In preparation for upcoming patch "fix regression with GPIO
> > configuration". To facilitate review and make code more modular.
> >
> > Cc: <[email protected]> # 6.1.x
> > Signed-off-by: Hugo Villeneuve <[email protected]>
> > Reviewed-by: Lech Perczak <[email protected]>
> > Tested-by: Lech Perczak <[email protected]>
> > ---
> > drivers/tty/serial/sc16is7xx.c | 39 ++++++++++++++++++++--------------
> > 1 file changed, 23 insertions(+), 16 deletions(-)
> >
> > diff --git a/drivers/tty/serial/sc16is7xx.c
> > b/drivers/tty/serial/sc16is7xx.c
> > index 0c903d44429c..279d7dcb1447 100644
> > --- a/drivers/tty/serial/sc16is7xx.c
> > +++ b/drivers/tty/serial/sc16is7xx.c
> > @@ -1349,6 +1349,26 @@ static int
> > sc16is7xx_gpio_direction_output(struct gpio_chip *chip,
> >
> > return 0;
> > }
> > +
> > +static int sc16is7xx_setup_gpio_chip(struct device *dev)
>
> Only one parameter, but...
>
> > + ret = sc16is7xx_setup_gpio_chip(dev, mctrl_mask);
>
> called with two.

Hi Maarten,
sorry about that, I will fix this for V6.

Somehow my automated compile script tested each patch separately, including this one, but the defconfig didn't enable the module sc6is7xx, so the error went unnoticed. I wrongly assumed that "make alldefconfig" enabled all modules, I will instead use "make allyesconfig".

Hugo.