* [PATCH v2 0/3] add support for EXAR XR20M1172 UART
@ 2024-04-18 13:57 Konstantin Pugin
2024-04-18 13:57 ` [PATCH v2 1/3] serial: sc16is7xx: announce support of SER_RS485_RTS_ON_SEND Konstantin Pugin
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Konstantin Pugin @ 2024-04-18 13:57 UTC (permalink / raw
Cc: Konstantin Pugin, Greg Kroah-Hartman, Jiri Slaby, Hugo Villeneuve,
Andy Shevchenko, Lech Perczak, Ilpo Järvinen,
Thomas Gleixner, linux-kernel, linux-serial
From: Konstantin Pugin <ria.freelander@gmail.com>
EXAR XR20M1172 UART is mostly SC16IS762-compatible, but
it has additional register which can change UART multiplier
to 4x and 8x, similar to UPF_MAGIC_MULTIPLIER does. So, I used this
flag to guard access to its specific DLD register. It seems than
other EXAR SPI UART modules also have this register, but I tested
only XR20M1172.
Changes in v2:
use full name in git authorship
Konstantin Pugin (3):
serial: sc16is7xx: announce support of SER_RS485_RTS_ON_SEND
serial: sc16is7xx: Add bindings documentation for EXAR XR20M1172 UART
serial: sc16is7xx: add support for EXAR XR20M1172 UART
.../bindings/serial/nxp,sc16is7xx.yaml | 1 +
drivers/tty/serial/sc16is7xx.c | 56 +++++++++++++++++--
2 files changed, 53 insertions(+), 4 deletions(-)
--
2.34.1
^ permalink raw reply [flat|nested] 7+ messages in thread* [PATCH v2 1/3] serial: sc16is7xx: announce support of SER_RS485_RTS_ON_SEND 2024-04-18 13:57 [PATCH v2 0/3] add support for EXAR XR20M1172 UART Konstantin Pugin @ 2024-04-18 13:57 ` Konstantin Pugin 2024-04-18 14:12 ` Andy Shevchenko 2024-04-18 13:57 ` [PATCH v2 2/3] serial: sc16is7xx: Add bindings documentation for EXAR XR20M1172 UART Konstantin Pugin 2024-04-18 13:57 ` [PATCH v2 3/3] serial: sc16is7xx: add support " Konstantin Pugin 2 siblings, 1 reply; 7+ messages in thread From: Konstantin Pugin @ 2024-04-18 13:57 UTC (permalink / raw Cc: Konstantin Pugin, Greg Kroah-Hartman, Jiri Slaby, Hugo Villeneuve, Andy Shevchenko, Lech Perczak, Ilpo Järvinen, Thomas Gleixner, linux-kernel, linux-serial From: Konstantin Pugin <ria.freelander@gmail.com> Hardware supports both modes, but after commit 4afeced55baa ("serial: core: fix sanitizing check for RTS settings") we always end up with SER_RS485_RTS_AFTER_SEND and always write in register SC16IS7XX_EFCR_RTS_INVERT_BIT, which breaks some hardware using these chips, Fixes: 267913ecf737 ("serial: sc16is7xx: Fill in rs485_supported") Signed-off-by: Konstantin Pugin <ria.freelander@gmail.com> --- drivers/tty/serial/sc16is7xx.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/tty/serial/sc16is7xx.c b/drivers/tty/serial/sc16is7xx.c index 929206a9a6e1..a300eebf1401 100644 --- a/drivers/tty/serial/sc16is7xx.c +++ b/drivers/tty/serial/sc16is7xx.c @@ -1458,7 +1458,7 @@ static int sc16is7xx_setup_mctrl_ports(struct sc16is7xx_port *s, } static const struct serial_rs485 sc16is7xx_rs485_supported = { - .flags = SER_RS485_ENABLED | SER_RS485_RTS_AFTER_SEND, + .flags = SER_RS485_ENABLED | SER_RS485_RTS_ON_SEND | SER_RS485_RTS_AFTER_SEND, .delay_rts_before_send = 1, .delay_rts_after_send = 1, /* Not supported but keep returning -EINVAL */ }; -- 2.34.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v2 1/3] serial: sc16is7xx: announce support of SER_RS485_RTS_ON_SEND 2024-04-18 13:57 ` [PATCH v2 1/3] serial: sc16is7xx: announce support of SER_RS485_RTS_ON_SEND Konstantin Pugin @ 2024-04-18 14:12 ` Andy Shevchenko 0 siblings, 0 replies; 7+ messages in thread From: Andy Shevchenko @ 2024-04-18 14:12 UTC (permalink / raw To: Konstantin Pugin Cc: Konstantin Pugin, Greg Kroah-Hartman, Jiri Slaby, Hugo Villeneuve, Lech Perczak, Ilpo Järvinen, Thomas Gleixner, linux-kernel, linux-serial On Thu, Apr 18, 2024 at 04:57:32PM +0300, Konstantin Pugin wrote: > From: Konstantin Pugin <ria.freelander@gmail.com> > > Hardware supports both modes, but after The hardware "both"? Please elaborate, like "The hardware supports both (A and B) modes, ..." > commit 4afeced55baa ("serial: core: fix sanitizing check for RTS settings") the commit > we always end up with SER_RS485_RTS_AFTER_SEND and always write in write to > register SC16IS7XX_EFCR_RTS_INVERT_BIT, the register > which breaks some hardware using these chips, Please, rewrap these lines to be approximately same width. Yes, you may split commit over the lines in the body of the commit message, but tags (like Fixes). Also it seems you forgot to add Vladimir's tags. It's your responsibility to do that if you send a new version with only cosmetic changes done. -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2 2/3] serial: sc16is7xx: Add bindings documentation for EXAR XR20M1172 UART 2024-04-18 13:57 [PATCH v2 0/3] add support for EXAR XR20M1172 UART Konstantin Pugin 2024-04-18 13:57 ` [PATCH v2 1/3] serial: sc16is7xx: announce support of SER_RS485_RTS_ON_SEND Konstantin Pugin @ 2024-04-18 13:57 ` Konstantin Pugin 2024-04-18 14:15 ` Andy Shevchenko 2024-04-18 13:57 ` [PATCH v2 3/3] serial: sc16is7xx: add support " Konstantin Pugin 2 siblings, 1 reply; 7+ messages in thread From: Konstantin Pugin @ 2024-04-18 13:57 UTC (permalink / raw Cc: Konstantin Pugin, Vladimir Zapolskiy, Greg Kroah-Hartman, Jiri Slaby, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Hugo Villeneuve, Andy Shevchenko, Lech Perczak, Ilpo Järvinen, Thomas Gleixner, linux-kernel, linux-serial, devicetree From: Konstantin Pugin <ria.freelander@gmail.com> This patch adds the devicetree documentation for the XR20M1172 UART. Reviewed-by: Vladimir Zapolskiy <vz@mleia.com> Signed-off-by: Konstantin Pugin <ria.freelander@gmail.com> --- Documentation/devicetree/bindings/serial/nxp,sc16is7xx.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/Documentation/devicetree/bindings/serial/nxp,sc16is7xx.yaml b/Documentation/devicetree/bindings/serial/nxp,sc16is7xx.yaml index 5dec15b7e7c3..68fe7b11961c 100644 --- a/Documentation/devicetree/bindings/serial/nxp,sc16is7xx.yaml +++ b/Documentation/devicetree/bindings/serial/nxp,sc16is7xx.yaml @@ -18,6 +18,7 @@ properties: - nxp,sc16is752 - nxp,sc16is760 - nxp,sc16is762 + - exar,xr20m1172 reg: maxItems: 1 -- 2.34.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v2 2/3] serial: sc16is7xx: Add bindings documentation for EXAR XR20M1172 UART 2024-04-18 13:57 ` [PATCH v2 2/3] serial: sc16is7xx: Add bindings documentation for EXAR XR20M1172 UART Konstantin Pugin @ 2024-04-18 14:15 ` Andy Shevchenko 0 siblings, 0 replies; 7+ messages in thread From: Andy Shevchenko @ 2024-04-18 14:15 UTC (permalink / raw To: Konstantin Pugin Cc: Konstantin Pugin, Vladimir Zapolskiy, Greg Kroah-Hartman, Jiri Slaby, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Hugo Villeneuve, Lech Perczak, Ilpo Järvinen, Thomas Gleixner, linux-kernel, linux-serial, devicetree On Thu, Apr 18, 2024 at 04:57:33PM +0300, Konstantin Pugin wrote: > From: Konstantin Pugin <ria.freelander@gmail.com> > > This patch adds the devicetree documentation for the XR20M1172 UART. Submitting Patches document says something about "imperative mood", please read that document in full anyway as it has a lot of nice requirements and recommendations. ... > - nxp,sc16is752 > - nxp,sc16is760 > - nxp,sc16is762 > + - exar,xr20m1172 Wouldn't it be better to keep it alphabetically ordered (at least avoid bringing more chaos if it's not ordered originally)? -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2 3/3] serial: sc16is7xx: add support for EXAR XR20M1172 UART 2024-04-18 13:57 [PATCH v2 0/3] add support for EXAR XR20M1172 UART Konstantin Pugin 2024-04-18 13:57 ` [PATCH v2 1/3] serial: sc16is7xx: announce support of SER_RS485_RTS_ON_SEND Konstantin Pugin 2024-04-18 13:57 ` [PATCH v2 2/3] serial: sc16is7xx: Add bindings documentation for EXAR XR20M1172 UART Konstantin Pugin @ 2024-04-18 13:57 ` Konstantin Pugin 2024-04-18 14:28 ` Andy Shevchenko 2 siblings, 1 reply; 7+ messages in thread From: Konstantin Pugin @ 2024-04-18 13:57 UTC (permalink / raw Cc: Konstantin Pugin, Greg Kroah-Hartman, Jiri Slaby, Hugo Villeneuve, Andy Shevchenko, Lech Perczak, Ilpo Järvinen, Thomas Gleixner, linux-kernel, linux-serial From: Konstantin Pugin <ria.freelander@gmail.com> Its register set is mostly compatible with SC16IS762, but it has a support for additional division rates of UART with special DLD register. So, add handling this register via UPF_MAGIC_MULTIPLIER port flag. Signed-off-by: Konstantin Pugin <ria.freelander@gmail.com> --- drivers/tty/serial/sc16is7xx.c | 54 ++++++++++++++++++++++++++++++++-- 1 file changed, 51 insertions(+), 3 deletions(-) diff --git a/drivers/tty/serial/sc16is7xx.c b/drivers/tty/serial/sc16is7xx.c index a300eebf1401..7fc1c19b3891 100644 --- a/drivers/tty/serial/sc16is7xx.c +++ b/drivers/tty/serial/sc16is7xx.c @@ -65,6 +65,7 @@ /* Special Register set: Only if ((LCR[7] == 1) && (LCR != 0xBF)) */ #define SC16IS7XX_DLL_REG (0x00) /* Divisor Latch Low */ #define SC16IS7XX_DLH_REG (0x01) /* Divisor Latch High */ +#define SC16IS7XX_DLD_REG (0x02) /* Divisor Latch Mode (only on EXAR chips) */ /* Enhanced Register set: Only if (LCR == 0xBF) */ #define SC16IS7XX_EFR_REG (0x02) /* Enhanced Features */ @@ -218,6 +219,20 @@ #define SC16IS7XX_TCR_RX_HALT(words) ((((words) / 4) & 0x0f) << 0) #define SC16IS7XX_TCR_RX_RESUME(words) ((((words) / 4) & 0x0f) << 4) +/* Divisor Latch Mode bits (EXAR extension) + * + * EXAR hardware is mostly compatible with SC16IS7XX, but supports additional feature: + * 4x and 8x divisor, instead of default 16x. It has a special register to program it. + * Bits 0 to 3 is fractional divisor, it used to set value of last 16 bits of + * uartclk * (16 / divisor) / baud, in case of default it will be uartclk / baud. + * Bits 4 and 5 used as switches, and should not be set to 1 simultaneously. + */ + +#define SC16IS7XX_DLD_16X 0 +#define SC16IS7XX_DLD_DIV(m) ((m) & 0xf) +#define SC16IS7XX_DLD_8X BIT(4) +#define SC16IS7XX_DLD_4X BIT(5) + /* * TLR register bits * If TLR[3:0] or TLR[7:4] are logical 0, the selectable trigger levels via the @@ -310,6 +325,7 @@ struct sc16is7xx_devtype { char name[10]; int nr_gpio; int nr_uart; + bool has_dld; }; #define SC16IS7XX_RECONF_MD (1 << 0) @@ -522,6 +538,13 @@ static const struct sc16is7xx_devtype sc16is762_devtype = { .nr_uart = 2, }; +static const struct sc16is7xx_devtype xr20m1172_devtype = { + .name = "XR20M1172", + .nr_gpio = 8, + .nr_uart = 2, + .has_dld = true, +}; + static bool sc16is7xx_regmap_volatile(struct device *dev, unsigned int reg) { switch (reg) { @@ -559,13 +582,29 @@ static int sc16is7xx_set_baud(struct uart_port *port, int baud) struct sc16is7xx_one *one = to_sc16is7xx_one(port, port); u8 lcr; u8 prescaler = 0; - unsigned long clk = port->uartclk, div = clk / 16 / baud; + u8 divisor = 16; + u8 dld_mode = SC16IS7XX_DLD_16X; + bool has_dld = !!(port->flags & UPF_MAGIC_MULTIPLIER); + unsigned long clk = port->uartclk, div, div16; + + if (has_dld) + while (DIV_ROUND_CLOSEST(port->uartclk, baud) < divisor) + divisor /= 2; + + div16 = clk * (16 / divisor) / baud; + div = div16 / 16; /* For divisor = 16, it is the same as clk / 16 / baud */ if (div >= BIT(16)) { prescaler = SC16IS7XX_MCR_CLKSEL_BIT; div /= 4; } + /* Count additional divisor for EXAR devices */ + if (divisor == 8) + dld_mode = SC16IS7XX_DLD_8X; + if (divisor == 4) + dld_mode = SC16IS7XX_DLD_4X; + /* Enable enhanced features */ sc16is7xx_efr_lock(port); sc16is7xx_port_update(port, SC16IS7XX_EFR_REG, @@ -586,12 +625,14 @@ static int sc16is7xx_set_baud(struct uart_port *port, int baud) regcache_cache_bypass(one->regmap, true); sc16is7xx_port_write(port, SC16IS7XX_DLH_REG, div / 256); sc16is7xx_port_write(port, SC16IS7XX_DLL_REG, div % 256); + if (has_dld) + sc16is7xx_port_write(port, SC16IS7XX_DLD_REG, dld_mode | SC16IS7XX_DLD_DIV(div16)); regcache_cache_bypass(one->regmap, false); /* Restore LCR and access to general register set */ sc16is7xx_port_write(port, SC16IS7XX_LCR_REG, lcr); - return DIV_ROUND_CLOSEST(clk / 16, div); + return DIV_ROUND_CLOSEST(clk / divisor, div); } static void sc16is7xx_handle_rx(struct uart_port *port, unsigned int rxlen, @@ -1014,6 +1055,7 @@ static void sc16is7xx_set_termios(struct uart_port *port, unsigned int lcr, flow = 0; int baud; unsigned long flags; + bool has_dld = !!(port->flags & UPF_MAGIC_MULTIPLIER); kthread_cancel_delayed_work_sync(&one->ms_work); @@ -1093,7 +1135,7 @@ static void sc16is7xx_set_termios(struct uart_port *port, /* Get baud rate generator configuration */ baud = uart_get_baud_rate(port, termios, old, port->uartclk / 16 / 4 / 0xffff, - port->uartclk / 16); + port->uartclk / (has_dld ? 4 : 16)); /* Setup baudrate generator */ baud = sc16is7xx_set_baud(port, baud); @@ -1550,6 +1592,9 @@ static int sc16is7xx_probe(struct device *dev, s->p[i].port.type = PORT_SC16IS7XX; s->p[i].port.fifosize = SC16IS7XX_FIFO_SIZE; s->p[i].port.flags = UPF_FIXED_TYPE | UPF_LOW_LATENCY; + /* If we have DLD register, then set UPF_MAGIC_MULTIPLIER flag */ + if (devtype->has_dld) + s->p[i].port.flags |= UPF_MAGIC_MULTIPLIER; s->p[i].port.iobase = i; /* * Use all ones as membase to make sure uart_configure_port() in @@ -1688,6 +1733,7 @@ static const struct of_device_id __maybe_unused sc16is7xx_dt_ids[] = { { .compatible = "nxp,sc16is752", .data = &sc16is752_devtype, }, { .compatible = "nxp,sc16is760", .data = &sc16is760_devtype, }, { .compatible = "nxp,sc16is762", .data = &sc16is762_devtype, }, + { .compatible = "exar,xr20m1172", .data = &xr20m1172_devtype, }, { } }; MODULE_DEVICE_TABLE(of, sc16is7xx_dt_ids); @@ -1776,6 +1822,7 @@ static const struct spi_device_id sc16is7xx_spi_id_table[] = { { "sc16is752", (kernel_ulong_t)&sc16is752_devtype, }, { "sc16is760", (kernel_ulong_t)&sc16is760_devtype, }, { "sc16is762", (kernel_ulong_t)&sc16is762_devtype, }, + { "xr20m1172", (kernel_ulong_t)&xr20m1172_devtype, }, { } }; @@ -1826,6 +1873,7 @@ static const struct i2c_device_id sc16is7xx_i2c_id_table[] = { { "sc16is752", (kernel_ulong_t)&sc16is752_devtype, }, { "sc16is760", (kernel_ulong_t)&sc16is760_devtype, }, { "sc16is762", (kernel_ulong_t)&sc16is762_devtype, }, + { "xr20m1172", (kernel_ulong_t)&xr20m1172_devtype, }, { } }; MODULE_DEVICE_TABLE(i2c, sc16is7xx_i2c_id_table); -- 2.34.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v2 3/3] serial: sc16is7xx: add support for EXAR XR20M1172 UART 2024-04-18 13:57 ` [PATCH v2 3/3] serial: sc16is7xx: add support " Konstantin Pugin @ 2024-04-18 14:28 ` Andy Shevchenko 0 siblings, 0 replies; 7+ messages in thread From: Andy Shevchenko @ 2024-04-18 14:28 UTC (permalink / raw To: Konstantin Pugin Cc: Konstantin Pugin, Greg Kroah-Hartman, Jiri Slaby, Hugo Villeneuve, Lech Perczak, Ilpo Järvinen, Thomas Gleixner, linux-kernel, linux-serial On Thu, Apr 18, 2024 at 04:57:34PM +0300, Konstantin Pugin wrote: > From: Konstantin Pugin <ria.freelander@gmail.com> > > Its register set is mostly compatible with SC16IS762, but "Its"? Whose? Elaborate, please. > it has a support for additional division rates of UART > with special DLD register. So, add handling this register > via UPF_MAGIC_MULTIPLIER port flag. Oh, can we avoid using this? You can redefine ->set_termios() if required and before factor out the common pieces to the helper functions. ... All three commit messages seems follow different text width, please keep it around ~60 for Subject and ~72 for the commit message. ... > /* Special Register set: Only if ((LCR[7] == 1) && (LCR != 0xBF)) */ > #define SC16IS7XX_DLL_REG (0x00) /* Divisor Latch Low */ > #define SC16IS7XX_DLH_REG (0x01) /* Divisor Latch High */ > +#define SC16IS7XX_DLD_REG (0x02) /* Divisor Latch Mode (only on EXAR chips) */ Is it called DLD in the datasheet? If so, can you match the comment to the datasheet, otherwise make it DLM and I would even go for the EXAR namespace. ... > +/* Divisor Latch Mode bits (EXAR extension) > + * > + * EXAR hardware is mostly compatible with SC16IS7XX, but supports additional feature: > + * 4x and 8x divisor, instead of default 16x. It has a special register to program it. > + * Bits 0 to 3 is fractional divisor, it used to set value of last 16 bits of > + * uartclk * (16 / divisor) / baud, in case of default it will be uartclk / baud. > + * Bits 4 and 5 used as switches, and should not be set to 1 simultaneously. > + */ /* * This is wrong multi-line comment * style for this subsystem. Use this * example. */ ... > +#define SC16IS7XX_DLD_16X 0 > +#define SC16IS7XX_DLD_DIV(m) ((m) & 0xf) GENMASK() (since you already use BIT() below) > +#define SC16IS7XX_DLD_8X BIT(4) > +#define SC16IS7XX_DLD_4X BIT(5) Perhaps also EXAR namespace. ... > struct sc16is7xx_one *one = to_sc16is7xx_one(port, port); > u8 lcr; > u8 prescaler = 0; > - unsigned long clk = port->uartclk, div = clk / 16 / baud; > + u8 divisor = 16; > + u8 dld_mode = SC16IS7XX_DLD_16X; > + bool has_dld = !!(port->flags & UPF_MAGIC_MULTIPLIER); > + unsigned long clk = port->uartclk, div, div16; Please, try to keep it in reversed xmas tree order (longer lines first). ... > + if (has_dld) Can we actually replace this with some ID checks? > + while (DIV_ROUND_CLOSEST(port->uartclk, baud) < divisor) > + divisor /= 2; Bit shifts and ffs() / fls() from bitops.h (or respective round*() / ilog2() / etc. from log2.h) will help you to avoid while loop. ... > + div16 = clk * (16 / divisor) / baud; > + div = div16 / 16; /* For divisor = 16, it is the same as clk / 16 / baud */ So, these may loose in precision, right? Wouldn't be better to have div16 = (clk * 16) / divisor / baud; div = div16 / 16; ? > if (div >= BIT(16)) { > prescaler = SC16IS7XX_MCR_CLKSEL_BIT; > div /= 4; > } > ... > { .compatible = "nxp,sc16is752", .data = &sc16is752_devtype, }, > { .compatible = "nxp,sc16is760", .data = &sc16is760_devtype, }, > { .compatible = "nxp,sc16is762", .data = &sc16is762_devtype, }, > + { .compatible = "exar,xr20m1172", .data = &xr20m1172_devtype, }, Sorted? ... > + { "xr20m1172", (kernel_ulong_t)&xr20m1172_devtype, }, This gives a hint about the above mentioned EXAR namespace for the definitions, i.e. use #define XR20M1172_... -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2024-04-18 14:28 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-04-18 13:57 [PATCH v2 0/3] add support for EXAR XR20M1172 UART Konstantin Pugin 2024-04-18 13:57 ` [PATCH v2 1/3] serial: sc16is7xx: announce support of SER_RS485_RTS_ON_SEND Konstantin Pugin 2024-04-18 14:12 ` Andy Shevchenko 2024-04-18 13:57 ` [PATCH v2 2/3] serial: sc16is7xx: Add bindings documentation for EXAR XR20M1172 UART Konstantin Pugin 2024-04-18 14:15 ` Andy Shevchenko 2024-04-18 13:57 ` [PATCH v2 3/3] serial: sc16is7xx: add support " Konstantin Pugin 2024-04-18 14:28 ` Andy Shevchenko
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).