From: Hugo Villeneuve hvilleneuve@dimonoff.com
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 "nxp,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 DT property.
Fixes: 679875d1d880 ("sc16is7xx: Separate GPIOs from modem control lines") Fixes: 21144bab4f11 ("sc16is7xx: Handle modem status lines") Cc: stable@vger.kernel.org # 6.1.x: 95982fad dt-bindings: sc16is7xx: Add property to change GPIO function Cc: stable@vger.kernel.org # 6.1.x: 1584d572 serial: sc16is7xx: refactor GPIO controller registration Cc: stable@vger.kernel.org # 6.1.x: ac2caa5a serial: sc16is7xx: remove obsolete out_thread label Cc: stable@vger.kernel.org # 6.1.x: d90961ad serial: sc16is7xx: mark IOCONTROL register as volatile Cc: stable@vger.kernel.org # 6.1.x: 6dae3bad serial: sc16is7xx: fix broken port 0 uart init Cc: stable@vger.kernel.org # 6.1.x Signed-off-by: Hugo Villeneuve hvilleneuve@dimonoff.com Reviewed-by: Andy Shevchenko andy.shevchenko@gmail.com Reviewed-by: Lech Perczak lech.perczak@camlingroup.com Tested-by: Lech Perczak lech.perczak@camlingroup.com --- drivers/tty/serial/sc16is7xx.c | 104 +++++++++++++++++++++++++++------ 1 file changed, 85 insertions(+), 19 deletions(-)
diff --git a/drivers/tty/serial/sc16is7xx.c b/drivers/tty/serial/sc16is7xx.c index 5b0aeef9d534..bc0a288f258d 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) @@ -337,7 +338,9 @@ struct sc16is7xx_port { struct clk *clk; #ifdef CONFIG_GPIOLIB struct gpio_chip gpio; + unsigned long gpio_valid_mask; #endif + u8 mctrl_mask; unsigned char buf[SC16IS7XX_FIFO_SIZE]; struct kthread_worker kworker; struct task_struct *kworker_task; @@ -448,35 +451,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) @@ -1351,14 +1349,43 @@ static int sc16is7xx_gpio_direction_output(struct gpio_chip *chip, return 0; }
+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 sc16is7xx_port *s) { if (!s->devtype->nr_gpio) return 0;
+ switch (s->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 == 0) + return 0; + s->gpio.owner = THIS_MODULE; s->gpio.parent = s->dev; s->gpio.label = dev_name(s->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 +1398,47 @@ static int sc16is7xx_setup_gpio_chip(struct sc16is7xx_port *s) } #endif
+/* + * Configure ports designated to operate as modem control lines. + */ +static int sc16is7xx_setup_mctrl_ports(struct sc16is7xx_port *s) +{ + int i; + int ret; + int count; + u32 mctrl_port[2]; + + count = device_property_count_u32(s->dev, + "nxp,modem-control-line-ports"); + if (count < 0 || count > ARRAY_SIZE(mctrl_port)) + return 0; + + ret = device_property_read_u32_array(s->dev, + "nxp,modem-control-line-ports", + mctrl_port, count); + if (ret) + return ret; + + s->mctrl_mask = 0; + + for (i = 0; i < count; i++) { + /* Use GPIO lines as modem control lines */ + if (mctrl_port[i] == 0) + s->mctrl_mask |= SC16IS7XX_IOCONTROL_MODEM_A_BIT; + else if (mctrl_port[i] == 1) + s->mctrl_mask |= SC16IS7XX_IOCONTROL_MODEM_B_BIT; + } + + if (s->mctrl_mask) + regmap_update_bits( + s->regmap, + SC16IS7XX_IOCONTROL_REG << SC16IS7XX_REG_SHIFT, + SC16IS7XX_IOCONTROL_MODEM_A_BIT | + SC16IS7XX_IOCONTROL_MODEM_B_BIT, s->mctrl_mask); + + return 0; +} + static const struct serial_rs485 sc16is7xx_rs485_supported = { .flags = SER_RS485_ENABLED | SER_RS485_RTS_AFTER_SEND, .delay_rts_before_send = 1, @@ -1479,12 +1547,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); @@ -1522,6 +1584,10 @@ static int sc16is7xx_probe(struct device *dev, s->p[u].irda_mode = true; }
+ ret = sc16is7xx_setup_mctrl_ports(s); + if (ret) + goto out_ports; + #ifdef CONFIG_GPIOLIB ret = sc16is7xx_setup_gpio_chip(s); if (ret) @@ -1548,7 +1614,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); #endif
@@ -1572,7 +1638,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
On Tue, Jul 25, 2023 at 10:23:38AM -0400, Hugo Villeneuve wrote:
From: Hugo Villeneuve hvilleneuve@dimonoff.com
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 "nxp,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 DT property.
Fixes: 679875d1d880 ("sc16is7xx: Separate GPIOs from modem control lines") Fixes: 21144bab4f11 ("sc16is7xx: Handle modem status lines") Cc: stable@vger.kernel.org # 6.1.x: 95982fad dt-bindings: sc16is7xx: Add property to change GPIO function Cc: stable@vger.kernel.org # 6.1.x: 1584d572 serial: sc16is7xx: refactor GPIO controller registration Cc: stable@vger.kernel.org # 6.1.x: ac2caa5a serial: sc16is7xx: remove obsolete out_thread label Cc: stable@vger.kernel.org # 6.1.x: d90961ad serial: sc16is7xx: mark IOCONTROL register as volatile Cc: stable@vger.kernel.org # 6.1.x: 6dae3bad serial: sc16is7xx: fix broken port 0 uart init
Where are these git commit ids from? I don't see them in Linus's tree, how are they supposed to be picked up by the stable developers if they are not valid ones?
confused,
greg k-h
On Mon, 31 Jul 2023 17:58:41 +0200 Greg KH gregkh@linuxfoundation.org wrote:
On Tue, Jul 25, 2023 at 10:23:38AM -0400, Hugo Villeneuve wrote:
From: Hugo Villeneuve hvilleneuve@dimonoff.com
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 "nxp,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 DT property.
Fixes: 679875d1d880 ("sc16is7xx: Separate GPIOs from modem control lines") Fixes: 21144bab4f11 ("sc16is7xx: Handle modem status lines") Cc: stable@vger.kernel.org # 6.1.x: 95982fad dt-bindings: sc16is7xx: Add property to change GPIO function Cc: stable@vger.kernel.org # 6.1.x: 1584d572 serial: sc16is7xx: refactor GPIO controller registration Cc: stable@vger.kernel.org # 6.1.x: ac2caa5a serial: sc16is7xx: remove obsolete out_thread label Cc: stable@vger.kernel.org # 6.1.x: d90961ad serial: sc16is7xx: mark IOCONTROL register as volatile Cc: stable@vger.kernel.org # 6.1.x: 6dae3bad serial: sc16is7xx: fix broken port 0 uart init
Where are these git commit ids from? I don't see them in Linus's tree, how are they supposed to be picked up by the stable developers if they are not valid ones?
confused,
greg k-h
Hi Greg, once again, I simply misinterpreted stable-kernel-rules.rst.
I wrongly assumed that, for example, this patch had, as a prerequisite, all the patches before it in this series, and that is why I listed them.
So I will remove them all, since this patch doesn't have any other requisites other than the previous patches in this series.
Maybe it would be good to add some notes about that in stable-kernel-rules.rst?
Thank you, Hugo.
On Thu, Aug 3, 2023 at 5:18 PM Hugo Villeneuve hugo@hugovil.com wrote:
On Mon, 31 Jul 2023 17:58:41 +0200 Greg KH gregkh@linuxfoundation.org wrote:
On Tue, Jul 25, 2023 at 10:23:38AM -0400, Hugo Villeneuve wrote:
...
Fixes: 679875d1d880 ("sc16is7xx: Separate GPIOs from modem control lines") Fixes: 21144bab4f11 ("sc16is7xx: Handle modem status lines") Cc: stable@vger.kernel.org # 6.1.x: 95982fad dt-bindings: sc16is7xx: Add property to change GPIO function Cc: stable@vger.kernel.org # 6.1.x: 1584d572 serial: sc16is7xx: refactor GPIO controller registration Cc: stable@vger.kernel.org # 6.1.x: ac2caa5a serial: sc16is7xx: remove obsolete out_thread label Cc: stable@vger.kernel.org # 6.1.x: d90961ad serial: sc16is7xx: mark IOCONTROL register as volatile Cc: stable@vger.kernel.org # 6.1.x: 6dae3bad serial: sc16is7xx: fix broken port 0 uart init
Where are these git commit ids from? I don't see them in Linus's tree, how are they supposed to be picked up by the stable developers if they are not valid ones?
confused,
...
I wrongly assumed that, for example, this patch had, as a prerequisite, all the patches before it in this series, and that is why I listed them.
The problem, as I understand it, is not that you listed them (how else will the backporter know that this patch requires something else?) but the format (you used wrong SHA-1 sums).
...
So I will remove them all, since this patch doesn't have any other requisites other than the previous patches in this series.
Maybe it would be good to add some notes about that in stable-kernel-rules.rst?
This probably is a good idea. Briefly looking at it I see no examples like yours there.
On Fri, Aug 04, 2023 at 12:04:29AM +0300, Andy Shevchenko wrote:
On Thu, Aug 3, 2023 at 5:18 PM Hugo Villeneuve hugo@hugovil.com wrote:
On Mon, 31 Jul 2023 17:58:41 +0200 Greg KH gregkh@linuxfoundation.org wrote:
On Tue, Jul 25, 2023 at 10:23:38AM -0400, Hugo Villeneuve wrote:
...
Fixes: 679875d1d880 ("sc16is7xx: Separate GPIOs from modem control lines") Fixes: 21144bab4f11 ("sc16is7xx: Handle modem status lines") Cc: stable@vger.kernel.org # 6.1.x: 95982fad dt-bindings: sc16is7xx: Add property to change GPIO function Cc: stable@vger.kernel.org # 6.1.x: 1584d572 serial: sc16is7xx: refactor GPIO controller registration Cc: stable@vger.kernel.org # 6.1.x: ac2caa5a serial: sc16is7xx: remove obsolete out_thread label Cc: stable@vger.kernel.org # 6.1.x: d90961ad serial: sc16is7xx: mark IOCONTROL register as volatile Cc: stable@vger.kernel.org # 6.1.x: 6dae3bad serial: sc16is7xx: fix broken port 0 uart init
Where are these git commit ids from? I don't see them in Linus's tree, how are they supposed to be picked up by the stable developers if they are not valid ones?
confused,
...
I wrongly assumed that, for example, this patch had, as a prerequisite, all the patches before it in this series, and that is why I listed them.
That's fine, but if you have already marked those patches for stable inclusion, no need to list them here too.
The problem, as I understand it, is not that you listed them (how else will the backporter know that this patch requires something else?) but the format (you used wrong SHA-1 sums).
Exactly, those are invalid sha1 values.
So I will remove them all, since this patch doesn't have any other requisites other than the previous patches in this series.
Maybe it would be good to add some notes about that in stable-kernel-rules.rst?
This probably is a good idea. Briefly looking at it I see no examples like yours there.
Because it's not a thing? Just mark all of these patches in the series as cc: stable@ and all will happen automatically for you. Nothing fancy or complex here, happens daily in other subsystems just fine :)
thanks,
greg k-h
linux-stable-mirror@lists.linaro.org
-
Andy Shevchenko -
Greg KH -
Hugo Villeneuve