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.
Yes, in datasheet this register is called "DLD - Divisor Fractional"
or "DLD - Divisor Fractional Register", calling depends on datasheet
version.
I am sorry about too many submissions and top post reply. About second -
I do not know how to reply properly to this ML from GMail phone app. About first - I just
get very good feedback from Andy Shevchenko, and want to fix his review picks ASAP.
Changes in v2:
- use full name in git authorship
Changes in v3:
- change formatting of commit messages to unify width
- rework commit messages according to code review
- add XR20M117X namespace for EXAR-specific register
- do not use UPF_MAGIC_MULTIPLIER for checking EXAR chip,
use s->devtype directly
- replace while loop to fls function and expanded check
- sort compatibles
- reformat multiline comment.
Changes in v4:
- rebase onto tty-next branch
- added Kconfig mention of the chip
- used rounddown_power_of_two instead of fls and manual shift
- used FIELD_PREP instead of custom macro
- removed has_dld bit from common struct, replaced by check function,
which checks directly by s->devtype
- fixed tab count
- properly apply Vladimir Zapolskiy's tag to patch 2 only
Changes in v5:
- fixes for tty-next branch
- address a new code review picks
- send properly to all participants
- added Ack tag
Changes in v6:
- KConfig fixes
- New code review fixes
Changes in v7:
- Added missed tag
- Added missed v5 fixes
Konstantin Pugin (3):
serial: sc16is7xx: announce support of SER_RS485_RTS_ON_SEND
dt-bindings: sc16is7xx: Add compatible line for XR20M1172 UART
serial: sc16is7xx: add support for EXAR XR20M1172 UART
.../bindings/serial/nxp,sc16is7xx.yaml | 1 +
drivers/tty/serial/Kconfig | 3 +-
drivers/tty/serial/sc16is7xx.c | 63 +++++++++++++++++--
drivers/tty/serial/sc16is7xx.h | 1 +
drivers/tty/serial/sc16is7xx_i2c.c | 1 +
drivers/tty/serial/sc16is7xx_spi.c | 1 +
6 files changed, 64 insertions(+), 6 deletions(-)
base-commit: f70f95b485d78838ad28dbec804b986d11ad7bb0
--
2.34.1
The hardware supports both RTS_ON_SEND and RTS_AFTER_SEND modes, but
after the commit 4afeced55baa ("serial: core: fix sanitizing check for
RTS settings") we always end up with SER_RS485_RTS_AFTER_SEND set and
always write to the register field 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 <[email protected]>
---
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 03cf30e20b75..dfcc804f558f 100644
--- a/drivers/tty/serial/sc16is7xx.c
+++ b/drivers/tty/serial/sc16is7xx.c
@@ -1449,7 +1449,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
XR20M1172 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 by appropriate devicetree bindings.
Signed-off-by: Konstantin Pugin <[email protected]>
---
drivers/tty/serial/Kconfig | 3 +-
drivers/tty/serial/sc16is7xx.c | 61 ++++++++++++++++++++++++++++--
drivers/tty/serial/sc16is7xx.h | 1 +
drivers/tty/serial/sc16is7xx_i2c.c | 1 +
drivers/tty/serial/sc16is7xx_spi.c | 1 +
5 files changed, 62 insertions(+), 5 deletions(-)
diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
index 4fdd7857ef4d..9d0438cfe147 100644
--- a/drivers/tty/serial/Kconfig
+++ b/drivers/tty/serial/Kconfig
@@ -1029,7 +1029,7 @@ config SERIAL_SC16IS7XX_CORE
select SERIAL_SC16IS7XX_SPI if SPI_MASTER
select SERIAL_SC16IS7XX_I2C if I2C
help
- Core driver for NXP SC16IS7xx UARTs.
+ Core driver for NXP SC16IS7xx and compatible UARTs.
Supported ICs are:
SC16IS740
@@ -1038,6 +1038,7 @@ config SERIAL_SC16IS7XX_CORE
SC16IS752
SC16IS760
SC16IS762
+ XR20M1172 (Exar)
The driver supports both I2C and SPI interfaces.
diff --git a/drivers/tty/serial/sc16is7xx.c b/drivers/tty/serial/sc16is7xx.c
index dfcc804f558f..09c9e52d7ec2 100644
--- a/drivers/tty/serial/sc16is7xx.c
+++ b/drivers/tty/serial/sc16is7xx.c
@@ -10,6 +10,7 @@
#undef DEFAULT_SYMBOL_NAMESPACE
#define DEFAULT_SYMBOL_NAMESPACE SERIAL_NXP_SC16IS7XX
+#include <linux/bitfield.h>
#include <linux/clk.h>
#include <linux/delay.h>
#include <linux/device.h>
@@ -68,6 +69,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 XR20M117X_DLD_REG (0x02) /* Divisor Fractional Register */
/* Enhanced Register set: Only if (LCR == 0xBF) */
#define SC16IS7XX_EFR_REG (0x02) /* Enhanced Features */
@@ -221,6 +223,20 @@
#define SC16IS7XX_TCR_RX_HALT(words) ((((words) / 4) & 0x0f) << 0)
#define SC16IS7XX_TCR_RX_RESUME(words) ((((words) / 4) & 0x0f) << 4)
+/*
+ * Divisor Fractional Register 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 XR20M117X_DLD_16X 0
+#define XR20M117X_DLD_DIV_MASK GENMASK(3, 0)
+#define XR20M117X_DLD_8X BIT(4)
+#define XR20M117X_DLD_4X BIT(5)
+
/*
* TLR register bits
* If TLR[3:0] or TLR[7:4] are logical 0, the selectable trigger levels via the
@@ -523,6 +539,13 @@ const struct sc16is7xx_devtype sc16is762_devtype = {
};
EXPORT_SYMBOL_GPL(sc16is762_devtype);
+const struct sc16is7xx_devtype xr20m1172_devtype = {
+ .name = "XR20M1172",
+ .nr_gpio = 8,
+ .nr_uart = 2,
+};
+EXPORT_SYMBOL_GPL(xr20m1172_devtype);
+
static bool sc16is7xx_regmap_volatile(struct device *dev, unsigned int reg)
{
switch (reg) {
@@ -555,18 +578,43 @@ static bool sc16is7xx_regmap_noinc(struct device *dev, unsigned int reg)
return reg == SC16IS7XX_RHR_REG;
}
+static bool sc16is7xx_has_dld(struct device *dev)
+{
+ struct sc16is7xx_port *s = dev_get_drvdata(dev);
+
+ if (s->devtype == &xr20m1172_devtype)
+ return true;
+ return false;
+}
+
static int sc16is7xx_set_baud(struct uart_port *port, int baud)
{
struct sc16is7xx_one *one = to_sc16is7xx_one(port, port);
- u8 lcr;
+ unsigned long clk = port->uartclk, div, div16;
+ bool has_dld = sc16is7xx_has_dld(port->dev);
+ u8 dld_mode = XR20M117X_DLD_16X;
u8 prescaler = 0;
- unsigned long clk = port->uartclk, div = clk / 16 / baud;
+ u8 divisor = 16;
+ u8 lcr;
+
+ if (has_dld && DIV_ROUND_CLOSEST(clk, baud) < 16)
+ divisor = rounddown_pow_of_two(DIV_ROUND_CLOSEST(clk, baud));
+
+ div16 = (clk * 16) / divisor / baud;
+ div = div16 / 16;
if (div >= BIT(16)) {
prescaler = SC16IS7XX_MCR_CLKSEL_BIT;
div /= 4;
}
+ /* Count additional divisor for EXAR devices */
+ if (divisor == 8)
+ dld_mode = XR20M117X_DLD_8X;
+ if (divisor == 4)
+ dld_mode = XR20M117X_DLD_4X;
+ dld_mode |= FIELD_PREP(XR20M117X_DLD_DIV_MASK, div16);
+
/* Enable enhanced features */
sc16is7xx_efr_lock(port);
sc16is7xx_port_update(port, SC16IS7XX_EFR_REG,
@@ -587,12 +635,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, XR20M117X_DLD_REG, dld_mode);
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,
@@ -1002,6 +1052,8 @@ static void sc16is7xx_set_termios(struct uart_port *port,
const struct ktermios *old)
{
struct sc16is7xx_one *one = to_sc16is7xx_one(port, port);
+ bool has_dld = sc16is7xx_has_dld(port->dev);
+ u8 divisor = has_dld ? 4 : 16
unsigned int lcr, flow = 0;
int baud;
unsigned long flags;
@@ -1084,7 +1136,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 / divisor);
/* Setup baudrate generator */
baud = sc16is7xx_set_baud(port, baud);
@@ -1684,6 +1736,7 @@ void sc16is7xx_remove(struct device *dev)
EXPORT_SYMBOL_GPL(sc16is7xx_remove);
const struct of_device_id __maybe_unused sc16is7xx_dt_ids[] = {
+ { .compatible = "exar,xr20m1172", .data = &xr20m1172_devtype, },
{ .compatible = "nxp,sc16is740", .data = &sc16is74x_devtype, },
{ .compatible = "nxp,sc16is741", .data = &sc16is74x_devtype, },
{ .compatible = "nxp,sc16is750", .data = &sc16is750_devtype, },
diff --git a/drivers/tty/serial/sc16is7xx.h b/drivers/tty/serial/sc16is7xx.h
index afb784eaee45..eb2e3bc86f15 100644
--- a/drivers/tty/serial/sc16is7xx.h
+++ b/drivers/tty/serial/sc16is7xx.h
@@ -28,6 +28,7 @@ extern const struct sc16is7xx_devtype sc16is750_devtype;
extern const struct sc16is7xx_devtype sc16is752_devtype;
extern const struct sc16is7xx_devtype sc16is760_devtype;
extern const struct sc16is7xx_devtype sc16is762_devtype;
+extern const struct sc16is7xx_devtype xr20m1172_devtype;
const char *sc16is7xx_regmap_name(u8 port_id);
diff --git a/drivers/tty/serial/sc16is7xx_i2c.c b/drivers/tty/serial/sc16is7xx_i2c.c
index 3ed47c306d85..839de902821b 100644
--- a/drivers/tty/serial/sc16is7xx_i2c.c
+++ b/drivers/tty/serial/sc16is7xx_i2c.c
@@ -46,6 +46,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);
diff --git a/drivers/tty/serial/sc16is7xx_spi.c b/drivers/tty/serial/sc16is7xx_spi.c
index 73df36f8a7fd..2b278282dbd0 100644
--- a/drivers/tty/serial/sc16is7xx_spi.c
+++ b/drivers/tty/serial/sc16is7xx_spi.c
@@ -69,6 +69,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, },
{ }
};
MODULE_DEVICE_TABLE(spi, sc16is7xx_spi_id_table);
--
2.34.1
Add EXAR XR20M1172 UART compatible line into devicetree documentation.
Acked-by: Krzysztof Kozlowski <[email protected]>
Reviewed-by: Vladimir Zapolskiy <[email protected]>
Signed-off-by: Konstantin Pugin <[email protected]>
---
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..c4bedf23368b 100644
--- a/Documentation/devicetree/bindings/serial/nxp,sc16is7xx.yaml
+++ b/Documentation/devicetree/bindings/serial/nxp,sc16is7xx.yaml
@@ -12,6 +12,7 @@ maintainers:
properties:
compatible:
enum:
+ - exar,xr20m1172
- nxp,sc16is740
- nxp,sc16is741
- nxp,sc16is750
--
2.34.1
On Mon, Apr 22, 2024 at 4:45 PM Krzysztof Kozlowski <[email protected]> wrote:
>
> On 22/04/2024 15:32, Konstantin Pugin wrote:
> > 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.
> > Yes, in datasheet this register is called "DLD - Divisor Fractional"
> > or "DLD - Divisor Fractional Register", calling depends on datasheet
> > version.
> >
> > I am sorry about too many submissions and top post reply. About second -
> > I do not know how to reply properly to this ML from GMail phone app. About first - I just
> > get very good feedback from Andy Shevchenko, and want to fix his review picks ASAP.
> >
>
> One patchset per 24h.
>
> Plus, you already got such review comment:
>
> This is a friendly reminder during the review process.
>
> It looks like you received a tag and forgot to add it.
>
> If you do not know the process, here is a short explanation:
> Please add Acked-by/Reviewed-by/Tested-by tags when posting new
> versions, under or above your Signed-off-by tag. Tag is "received", when
> provided in a message replied to you on the mailing list. Tools like b4
> can help here. However, there's no need to repost patches *only* to add
> the tags. The upstream maintainer will do that for tags received on the
> version they apply.
>
> https://elixir.bootlin.com/linux/v6.5-rc3/source/Documentation/process/submitting-patches.rst#L577
>
> If a tag was not added on purpose, please state why and what changed.
>
> Just start using b4.
There is not only for tag. I submit fixes for version 4 by mistake,
so, repost to 7 was necessary, because v6 was not work (as v4). But v7
should be based on v5, and v5 is tested better around tty-next.
> Best regards,
> Krzysztof
>
On 22/04/2024 15:50, Konstantin P. wrote:
> On Mon, Apr 22, 2024 at 4:45 PM Krzysztof Kozlowski <[email protected]> wrote:
>>
>> On 22/04/2024 15:32, Konstantin Pugin wrote:
>>> 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.
>>> Yes, in datasheet this register is called "DLD - Divisor Fractional"
>>> or "DLD - Divisor Fractional Register", calling depends on datasheet
>>> version.
>>>
>>> I am sorry about too many submissions and top post reply. About second -
>>> I do not know how to reply properly to this ML from GMail phone app. About first - I just
>>> get very good feedback from Andy Shevchenko, and want to fix his review picks ASAP.
>>>
>>
>> One patchset per 24h.
>>
>> Plus, you already got such review comment:
>>
>> This is a friendly reminder during the review process.
>>
>> It looks like you received a tag and forgot to add it.
>>
>> If you do not know the process, here is a short explanation:
>> Please add Acked-by/Reviewed-by/Tested-by tags when posting new
>> versions, under or above your Signed-off-by tag. Tag is "received", when
>> provided in a message replied to you on the mailing list. Tools like b4
>> can help here. However, there's no need to repost patches *only* to add
>> the tags. The upstream maintainer will do that for tags received on the
>> version they apply.
>>
>> https://elixir.bootlin.com/linux/v6.5-rc3/source/Documentation/process/submitting-patches.rst#L577
>>
>> If a tag was not added on purpose, please state why and what changed.
>>
>> Just start using b4.
>
> There is not only for tag. I submit fixes for version 4 by mistake,
> so, repost to 7 was necessary, because v6 was not work (as v4). But v7
> should be based on v5, and v5 is tested better around tty-next.
???
You got tag, didn't you? Then explain why you decided to skip it. In the
changelog of patchset which ignores/skips the tag.
Best regards,
Krzysztof
On 22/04/2024 15:32, Konstantin Pugin wrote:
> 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.
> Yes, in datasheet this register is called "DLD - Divisor Fractional"
> or "DLD - Divisor Fractional Register", calling depends on datasheet
> version.
>
> I am sorry about too many submissions and top post reply. About second -
> I do not know how to reply properly to this ML from GMail phone app. About first - I just
> get very good feedback from Andy Shevchenko, and want to fix his review picks ASAP.
>
One patchset per 24h.
Plus, you already got such review comment:
This is a friendly reminder during the review process.
It looks like you received a tag and forgot to add it.
If you do not know the process, here is a short explanation:
Please add Acked-by/Reviewed-by/Tested-by tags when posting new
versions, under or above your Signed-off-by tag. Tag is "received", when
provided in a message replied to you on the mailing list. Tools like b4
can help here. However, there's no need to repost patches *only* to add
the tags. The upstream maintainer will do that for tags received on the
version they apply.
https://elixir.bootlin.com/linux/v6.5-rc3/source/Documentation/process/submitting-patches.rst#L577
If a tag was not added on purpose, please state why and what changed.
Just start using b4.
Best regards,
Krzysztof
On Mon, Apr 22, 2024 at 05:30:13PM +0300, Konstantin P. wrote:
> I do not skip it, it added to patch 2, as you requested.
You still continue top-posting!
It's not good.
You missed _my_ tag.
But please, please, wait a bit, you really need to slow down.
> On Mon, Apr 22, 2024, 16:51 Krzysztof Kozlowski <[email protected]> wrote:
> > On 22/04/2024 15:50, Konstantin P. wrote:
> > > On Mon, Apr 22, 2024 at 4:45 PM Krzysztof Kozlowski <[email protected]>
> > wrote:
> > >> On 22/04/2024 15:32, Konstantin Pugin wrote:
> > >>> 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.
> > >>> Yes, in datasheet this register is called "DLD - Divisor Fractional"
> > >>> or "DLD - Divisor Fractional Register", calling depends on datasheet
> > >>> version.
> > >>>
> > >>> I am sorry about too many submissions and top post reply. About second
> > -
> > >>> I do not know how to reply properly to this ML from GMail phone app.
> > About first - I just
> > >>> get very good feedback from Andy Shevchenko, and want to fix his
> > review picks ASAP.
> > >>>
> > >>
> > >> One patchset per 24h.
> > >>
> > >> Plus, you already got such review comment:
> > >>
> > >> This is a friendly reminder during the review process.
> > >>
> > >> It looks like you received a tag and forgot to add it.
> > >>
> > >> If you do not know the process, here is a short explanation:
> > >> Please add Acked-by/Reviewed-by/Tested-by tags when posting new
> > >> versions, under or above your Signed-off-by tag. Tag is "received", when
> > >> provided in a message replied to you on the mailing list. Tools like b4
> > >> can help here. However, there's no need to repost patches *only* to add
> > >> the tags. The upstream maintainer will do that for tags received on the
> > >> version they apply.
> > >>
> > >>
> > https://elixir.bootlin.com/linux/v6.5-rc3/source/Documentation/process/submitting-patches.rst#L577
> > >>
> > >> If a tag was not added on purpose, please state why and what changed.
> > >>
> > >> Just start using b4.
> > >
> > > There is not only for tag. I submit fixes for version 4 by mistake,
> > > so, repost to 7 was necessary, because v6 was not work (as v4). But v7
> > > should be based on v5, and v5 is tested better around tty-next.
> >
> > ???
> >
> > You got tag, didn't you? Then explain why you decided to skip it. In the
> > changelog of patchset which ignores/skips the tag.
--
With Best Regards,
Andy Shevchenko
On Mon, 22 Apr 2024 17:30:13 +0300
"Konstantin P." <[email protected]> wrote:
> I do not skip it, it added to patch 2, as you requested.
Once again, please do not top post.
Hugo.
> On Mon, Apr 22, 2024, 16:51 Krzysztof Kozlowski <[email protected]> wrote:
>
> > On 22/04/2024 15:50, Konstantin P. wrote:
> > > On Mon, Apr 22, 2024 at 4:45 PM Krzysztof Kozlowski <[email protected]>
> > wrote:
> > >>
> > >> On 22/04/2024 15:32, Konstantin Pugin wrote:
> > >>> 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.
> > >>> Yes, in datasheet this register is called "DLD - Divisor Fractional"
> > >>> or "DLD - Divisor Fractional Register", calling depends on datasheet
> > >>> version.
> > >>>
> > >>> I am sorry about too many submissions and top post reply. About second
> > -
> > >>> I do not know how to reply properly to this ML from GMail phone app.
> > About first - I just
> > >>> get very good feedback from Andy Shevchenko, and want to fix his
> > review picks ASAP.
> > >>>
> > >>
> > >> One patchset per 24h.
> > >>
> > >> Plus, you already got such review comment:
> > >>
> > >> This is a friendly reminder during the review process.
> > >>
> > >> It looks like you received a tag and forgot to add it.
> > >>
> > >> If you do not know the process, here is a short explanation:
> > >> Please add Acked-by/Reviewed-by/Tested-by tags when posting new
> > >> versions, under or above your Signed-off-by tag. Tag is "received", when
> > >> provided in a message replied to you on the mailing list. Tools like b4
> > >> can help here. However, there's no need to repost patches *only* to add
> > >> the tags. The upstream maintainer will do that for tags received on the
> > >> version they apply.
> > >>
> > >>
> > https://elixir.bootlin.com/linux/v6.5-rc3/source/Documentation/process/submitting-patches.rst#L577
> > >>
> > >> If a tag was not added on purpose, please state why and what changed.
> > >>
> > >> Just start using b4.
> > >
> > > There is not only for tag. I submit fixes for version 4 by mistake,
> > > so, repost to 7 was necessary, because v6 was not work (as v4). But v7
> > > should be based on v5, and v5 is tested better around tty-next.
> >
> > ???
> >
> > You got tag, didn't you? Then explain why you decided to skip it. In the
> > changelog of patchset which ignores/skips the tag.
> >
> > Best regards,
> > Krzysztof
> >
> >
--
Hugo Villeneuve
On Mon, Apr 22, 2024 at 5:45 PM Andy Shevchenko <[email protected]> wrote:
>
> On Mon, Apr 22, 2024 at 05:30:13PM +0300, Konstantin P. wrote:
> > I do not skip it, it added to patch 2, as you requested.
>
> You still continue top-posting!
> It's not good.
>
> You missed _my_ tag.
>
> But please, please, wait a bit, you really need to slow down.
>
> > On Mon, Apr 22, 2024, 16:51 Krzysztof Kozlowski <[email protected]> wrote:
> > > On 22/04/2024 15:50, Konstantin P. wrote:
> > > > On Mon, Apr 22, 2024 at 4:45 PM Krzysztof Kozlowski <[email protected]>
> > > wrote:
> > > >> On 22/04/2024 15:32, Konstantin Pugin wrote:
>
> > > >>> 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.
> > > >>> Yes, in datasheet this register is called "DLD - Divisor Fractional"
> > > >>> or "DLD - Divisor Fractional Register", calling depends on datasheet
> > > >>> version.
> > > >>>
> > > >>> I am sorry about too many submissions and top post reply. About second
> > > -
> > > >>> I do not know how to reply properly to this ML from GMail phone app.
> > > About first - I just
> > > >>> get very good feedback from Andy Shevchenko, and want to fix his
> > > review picks ASAP.
> > > >>>
> > > >>
> > > >> One patchset per 24h.
> > > >>
> > > >> Plus, you already got such review comment:
> > > >>
> > > >> This is a friendly reminder during the review process.
> > > >>
> > > >> It looks like you received a tag and forgot to add it.
> > > >>
> > > >> If you do not know the process, here is a short explanation:
> > > >> Please add Acked-by/Reviewed-by/Tested-by tags when posting new
> > > >> versions, under or above your Signed-off-by tag. Tag is "received", when
> > > >> provided in a message replied to you on the mailing list. Tools like b4
> > > >> can help here. However, there's no need to repost patches *only* to add
> > > >> the tags. The upstream maintainer will do that for tags received on the
> > > >> version they apply.
> > > >>
> > > >>
> > > https://elixir.bootlin.com/linux/v6.5-rc3/source/Documentation/process/submitting-patches.rst#L577
> > > >>
> > > >> If a tag was not added on purpose, please state why and what changed.
> > > >>
> > > >> Just start using b4.
> > > >
> > > > There is not only for tag. I submit fixes for version 4 by mistake,
> > > > so, repost to 7 was necessary, because v6 was not work (as v4). But v7
> > > > should be based on v5, and v5 is tested better around tty-next.
> > >
> > > ???
> > >
> > > You got tag, didn't you? Then explain why you decided to skip it. In the
> > > changelog of patchset which ignores/skips the tag.
>
> --
> With Best Regards,
> Andy Shevchenko
>
>
I am sorry about your tag, I did not notice it, if I do new version, I
will for sure add it. About top-posting - I do not know, how not to
top-post from GMail phone app(
Also, I added a linux-serial mailing list into all my mail, I do not
know why my emails are missing.
On Mon, Apr 22, 2024 at 6:28 PM Conor Dooley <[email protected]> wrote:
>
> On Mon, Apr 22, 2024 at 04:32:14PM +0300, Konstantin Pugin wrote:
> > Add EXAR XR20M1172 UART compatible line into devicetree documentation.
>
> I thought I had already pointed out a need for the commit message to
> explain why this exar device belongs in this file. IIRC you said it was
> in the driver commit and cover letter, but it needs to go here too.
>
> Thanks,
> Conor.
>
> >
> > Acked-by: Krzysztof Kozlowski <[email protected]>
> > Reviewed-by: Vladimir Zapolskiy <[email protected]>
> > Signed-off-by: Konstantin Pugin <[email protected]>
> > ---
> > 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..c4bedf23368b 100644
> > --- a/Documentation/devicetree/bindings/serial/nxp,sc16is7xx.yaml
> > +++ b/Documentation/devicetree/bindings/serial/nxp,sc16is7xx.yaml
> > @@ -12,6 +12,7 @@ maintainers:
> > properties:
> > compatible:
> > enum:
> > + - exar,xr20m1172
> > - nxp,sc16is740
> > - nxp,sc16is741
> > - nxp,sc16is750
> > --
> > 2.34.1
> >
Okay, will add again (another v6 missed).
On Mon, Apr 22, 2024 at 04:32:14PM +0300, Konstantin Pugin wrote:
> Add EXAR XR20M1172 UART compatible line into devicetree documentation.
I thought I had already pointed out a need for the commit message to
explain why this exar device belongs in this file. IIRC you said it was
in the driver commit and cover letter, but it needs to go here too.
Thanks,
Conor.
>
> Acked-by: Krzysztof Kozlowski <[email protected]>
> Reviewed-by: Vladimir Zapolskiy <[email protected]>
> Signed-off-by: Konstantin Pugin <[email protected]>
> ---
> 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..c4bedf23368b 100644
> --- a/Documentation/devicetree/bindings/serial/nxp,sc16is7xx.yaml
> +++ b/Documentation/devicetree/bindings/serial/nxp,sc16is7xx.yaml
> @@ -12,6 +12,7 @@ maintainers:
> properties:
> compatible:
> enum:
> + - exar,xr20m1172
> - nxp,sc16is740
> - nxp,sc16is741
> - nxp,sc16is750
> --
> 2.34.1
>
On Mon, Apr 22, 2024 at 05:57:31PM +0300, Konstantin P. wrote:
> On Mon, Apr 22, 2024 at 5:45 PM Andy Shevchenko <[email protected]> wrote:
> > On Mon, Apr 22, 2024 at 05:30:13PM +0300, Konstantin P. wrote:
> > > I do not skip it, it added to patch 2, as you requested.
> >
> > You still continue top-posting!
> > It's not good.
> >
> > You missed _my_ tag.
> >
> > But please, please, wait a bit, you really need to slow down.
> >
> > > On Mon, Apr 22, 2024, 16:51 Krzysztof Kozlowski <[email protected]> wrote:
> > > > On 22/04/2024 15:50, Konstantin P. wrote:
> > > > > On Mon, Apr 22, 2024 at 4:45 PM Krzysztof Kozlowski <[email protected]>
> > > > wrote:
> > > > >> On 22/04/2024 15:32, Konstantin Pugin wrote:
> >
> > > > >>> 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.
> > > > >>> Yes, in datasheet this register is called "DLD - Divisor Fractional"
> > > > >>> or "DLD - Divisor Fractional Register", calling depends on datasheet
> > > > >>> version.
> > > > >>>
> > > > >>> I am sorry about too many submissions and top post reply. About second
> > > > -
> > > > >>> I do not know how to reply properly to this ML from GMail phone app.
> > > > About first - I just
> > > > >>> get very good feedback from Andy Shevchenko, and want to fix his
> > > > review picks ASAP.
> > > > >>>
> > > > >>
> > > > >> One patchset per 24h.
> > > > >>
> > > > >> Plus, you already got such review comment:
> > > > >>
> > > > >> This is a friendly reminder during the review process.
> > > > >>
> > > > >> It looks like you received a tag and forgot to add it.
> > > > >>
> > > > >> If you do not know the process, here is a short explanation:
> > > > >> Please add Acked-by/Reviewed-by/Tested-by tags when posting new
> > > > >> versions, under or above your Signed-off-by tag. Tag is "received", when
> > > > >> provided in a message replied to you on the mailing list. Tools like b4
> > > > >> can help here. However, there's no need to repost patches *only* to add
> > > > >> the tags. The upstream maintainer will do that for tags received on the
> > > > >> version they apply.
> > > > >>
> > > > >>
> > > > https://elixir.bootlin.com/linux/v6.5-rc3/source/Documentation/process/submitting-patches.rst#L577
> > > > >>
> > > > >> If a tag was not added on purpose, please state why and what changed.
> > > > >>
> > > > >> Just start using b4.
> > > > >
> > > > > There is not only for tag. I submit fixes for version 4 by mistake,
> > > > > so, repost to 7 was necessary, because v6 was not work (as v4). But v7
> > > > > should be based on v5, and v5 is tested better around tty-next.
> > > >
> > > > ???
> > > >
> > > > You got tag, didn't you? Then explain why you decided to skip it. In the
> > > > changelog of patchset which ignores/skips the tag.
> >
> I am sorry about your tag, I did not notice it, if I do new version, I
> will for sure add it. About top-posting - I do not know, how not to
> top-post from GMail phone app(
I remember I was able to answer from mobile phone, but I stopped using GMail
App as it's awfully made. So, you may do it via browser and web-gmail.
> Also, I added a linux-serial mailing list into all my mail, I do not
> know why my emails are missing.
I have a script [1] that I'm using almost on a daily-basis, you may try it or
take an ideas, or even patch and send a PR if you think it can be made better.
[1]: https://github.com/andy-shev/home-bin-tools/blob/master/ge2maintainer.sh
--
With Best Regards,
Andy Shevchenko
On Tue, Apr 23, 2024 at 8:28 PM Hugo Villeneuve <[email protected]> wrote:
>
> On Mon, 22 Apr 2024 16:32:15 +0300
> Konstantin Pugin <[email protected]> wrote:
>
> Hi Konstantin,
>
> > XR20M1172 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 by appropriate devicetree bindings.
> >
> > Signed-off-by: Konstantin Pugin <[email protected]>
> > ---
> > drivers/tty/serial/Kconfig | 3 +-
> > drivers/tty/serial/sc16is7xx.c | 61 ++++++++++++++++++++++++++++--
> > drivers/tty/serial/sc16is7xx.h | 1 +
> > drivers/tty/serial/sc16is7xx_i2c.c | 1 +
> > drivers/tty/serial/sc16is7xx_spi.c | 1 +
> > 5 files changed, 62 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
> > index 4fdd7857ef4d..9d0438cfe147 100644
> > --- a/drivers/tty/serial/Kconfig
> > +++ b/drivers/tty/serial/Kconfig
> > @@ -1029,7 +1029,7 @@ config SERIAL_SC16IS7XX_CORE
> > select SERIAL_SC16IS7XX_SPI if SPI_MASTER
> > select SERIAL_SC16IS7XX_I2C if I2C
> > help
> > - Core driver for NXP SC16IS7xx UARTs.
> > + Core driver for NXP SC16IS7xx and compatible UARTs.
> > Supported ICs are:
> >
> > SC16IS740
> > @@ -1038,6 +1038,7 @@ config SERIAL_SC16IS7XX_CORE
> > SC16IS752
> > SC16IS760
> > SC16IS762
> > + XR20M1172 (Exar)
> >
> > The driver supports both I2C and SPI interfaces.
> >
> > diff --git a/drivers/tty/serial/sc16is7xx.c b/drivers/tty/serial/sc16is7xx.c
> > index dfcc804f558f..09c9e52d7ec2 100644
> > --- a/drivers/tty/serial/sc16is7xx.c
> > +++ b/drivers/tty/serial/sc16is7xx.c
> > @@ -10,6 +10,7 @@
> > #undef DEFAULT_SYMBOL_NAMESPACE
> > #define DEFAULT_SYMBOL_NAMESPACE SERIAL_NXP_SC16IS7XX
> >
> > +#include <linux/bitfield.h>
> > #include <linux/clk.h>
> > #include <linux/delay.h>
> > #include <linux/device.h>
> > @@ -68,6 +69,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 XR20M117X_DLD_REG (0x02) /* Divisor Fractional Register */
> >
> > /* Enhanced Register set: Only if (LCR == 0xBF) */
> > #define SC16IS7XX_EFR_REG (0x02) /* Enhanced Features */
> > @@ -221,6 +223,20 @@
> > #define SC16IS7XX_TCR_RX_HALT(words) ((((words) / 4) & 0x0f) << 0)
> > #define SC16IS7XX_TCR_RX_RESUME(words) ((((words) / 4) & 0x0f) << 4)
> >
> > +/*
> > + * Divisor Fractional Register 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 XR20M117X_DLD_16X 0
> > +#define XR20M117X_DLD_DIV_MASK GENMASK(3, 0)
> > +#define XR20M117X_DLD_8X BIT(4)
> > +#define XR20M117X_DLD_4X BIT(5)
> > +
> > /*
> > * TLR register bits
> > * If TLR[3:0] or TLR[7:4] are logical 0, the selectable trigger levels via the
> > @@ -523,6 +539,13 @@ const struct sc16is7xx_devtype sc16is762_devtype = {
> > };
> > EXPORT_SYMBOL_GPL(sc16is762_devtype);
> >
> > +const struct sc16is7xx_devtype xr20m1172_devtype = {
> > + .name = "XR20M1172",
> > + .nr_gpio = 8,
> > + .nr_uart = 2,
> > +};
> > +EXPORT_SYMBOL_GPL(xr20m1172_devtype);
> > +
> > static bool sc16is7xx_regmap_volatile(struct device *dev, unsigned int reg)
> > {
> > switch (reg) {
> > @@ -555,18 +578,43 @@ static bool sc16is7xx_regmap_noinc(struct device *dev, unsigned int reg)
> > return reg == SC16IS7XX_RHR_REG;
> > }
> >
> > +static bool sc16is7xx_has_dld(struct device *dev)
> > +{
> > + struct sc16is7xx_port *s = dev_get_drvdata(dev);
> > +
> > + if (s->devtype == &xr20m1172_devtype)
> > + return true;
> > + return false;
> > +}
> > +
> > static int sc16is7xx_set_baud(struct uart_port *port, int baud)
> > {
> > struct sc16is7xx_one *one = to_sc16is7xx_one(port, port);
> > - u8 lcr;
> > + unsigned long clk = port->uartclk, div, div16;
> > + bool has_dld = sc16is7xx_has_dld(port->dev);
> > + u8 dld_mode = XR20M117X_DLD_16X;
> > u8 prescaler = 0;
> > - unsigned long clk = port->uartclk, div = clk / 16 / baud;
> > + u8 divisor = 16;
> > + u8 lcr;
> > +
> > + if (has_dld && DIV_ROUND_CLOSEST(clk, baud) < 16)
> > + divisor = rounddown_pow_of_two(DIV_ROUND_CLOSEST(clk, baud));
> > +
> > + div16 = (clk * 16) / divisor / baud;
> > + div = div16 / 16;
> >
> > if (div >= BIT(16)) {
> > prescaler = SC16IS7XX_MCR_CLKSEL_BIT;
> > div /= 4;
> > }
> >
> > + /* Count additional divisor for EXAR devices */
> > + if (divisor == 8)
> > + dld_mode = XR20M117X_DLD_8X;
> > + if (divisor == 4)
> > + dld_mode = XR20M117X_DLD_4X;
> > + dld_mode |= FIELD_PREP(XR20M117X_DLD_DIV_MASK, div16);
> > +
> > /* Enable enhanced features */
> > sc16is7xx_efr_lock(port);
> > sc16is7xx_port_update(port, SC16IS7XX_EFR_REG,
> > @@ -587,12 +635,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, XR20M117X_DLD_REG, dld_mode);
> > 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,
> > @@ -1002,6 +1052,8 @@ static void sc16is7xx_set_termios(struct uart_port *port,
> > const struct ktermios *old)
> > {
> > struct sc16is7xx_one *one = to_sc16is7xx_one(port, port);
> > + bool has_dld = sc16is7xx_has_dld(port->dev);
> > + u8 divisor = has_dld ? 4 : 16
>
> You are missing a semicolumn here, and compilation aborts. Looks like
> you didn't test.
>
> Hugo.
>
>
> > unsigned int lcr, flow = 0;
> > int baud;
> > unsigned long flags;
> > @@ -1084,7 +1136,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 / divisor);
> >
> > /* Setup baudrate generator */
> > baud = sc16is7xx_set_baud(port, baud);
> > @@ -1684,6 +1736,7 @@ void sc16is7xx_remove(struct device *dev)
> > EXPORT_SYMBOL_GPL(sc16is7xx_remove);
> >
> > const struct of_device_id __maybe_unused sc16is7xx_dt_ids[] = {
> > + { .compatible = "exar,xr20m1172", .data = &xr20m1172_devtype, },
> > { .compatible = "nxp,sc16is740", .data = &sc16is74x_devtype, },
> > { .compatible = "nxp,sc16is741", .data = &sc16is74x_devtype, },
> > { .compatible = "nxp,sc16is750", .data = &sc16is750_devtype, },
> > diff --git a/drivers/tty/serial/sc16is7xx.h b/drivers/tty/serial/sc16is7xx.h
> > index afb784eaee45..eb2e3bc86f15 100644
> > --- a/drivers/tty/serial/sc16is7xx.h
> > +++ b/drivers/tty/serial/sc16is7xx.h
> > @@ -28,6 +28,7 @@ extern const struct sc16is7xx_devtype sc16is750_devtype;
> > extern const struct sc16is7xx_devtype sc16is752_devtype;
> > extern const struct sc16is7xx_devtype sc16is760_devtype;
> > extern const struct sc16is7xx_devtype sc16is762_devtype;
> > +extern const struct sc16is7xx_devtype xr20m1172_devtype;
> >
> > const char *sc16is7xx_regmap_name(u8 port_id);
> >
> > diff --git a/drivers/tty/serial/sc16is7xx_i2c.c b/drivers/tty/serial/sc16is7xx_i2c.c
> > index 3ed47c306d85..839de902821b 100644
> > --- a/drivers/tty/serial/sc16is7xx_i2c.c
> > +++ b/drivers/tty/serial/sc16is7xx_i2c.c
> > @@ -46,6 +46,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);
> > diff --git a/drivers/tty/serial/sc16is7xx_spi.c b/drivers/tty/serial/sc16is7xx_spi.c
> > index 73df36f8a7fd..2b278282dbd0 100644
> > --- a/drivers/tty/serial/sc16is7xx_spi.c
> > +++ b/drivers/tty/serial/sc16is7xx_spi.c
> > @@ -69,6 +69,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, },
> > { }
> > };
> > MODULE_DEVICE_TABLE(spi, sc16is7xx_spi_id_table);
> > --
> > 2.34.1
> >
> >
> >
>
>
> --
> Hugo Villeneuve
WIll fix tomorrow (or today, if I will have time). Maybe I send not
latest version.
On Mon, 22 Apr 2024 16:32:15 +0300
Konstantin Pugin <[email protected]> wrote:
Hi Konstantin,
> XR20M1172 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 by appropriate devicetree bindings.
>
> Signed-off-by: Konstantin Pugin <[email protected]>
> ---
> drivers/tty/serial/Kconfig | 3 +-
> drivers/tty/serial/sc16is7xx.c | 61 ++++++++++++++++++++++++++++--
> drivers/tty/serial/sc16is7xx.h | 1 +
> drivers/tty/serial/sc16is7xx_i2c.c | 1 +
> drivers/tty/serial/sc16is7xx_spi.c | 1 +
> 5 files changed, 62 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
> index 4fdd7857ef4d..9d0438cfe147 100644
> --- a/drivers/tty/serial/Kconfig
> +++ b/drivers/tty/serial/Kconfig
> @@ -1029,7 +1029,7 @@ config SERIAL_SC16IS7XX_CORE
> select SERIAL_SC16IS7XX_SPI if SPI_MASTER
> select SERIAL_SC16IS7XX_I2C if I2C
> help
> - Core driver for NXP SC16IS7xx UARTs.
> + Core driver for NXP SC16IS7xx and compatible UARTs.
> Supported ICs are:
>
> SC16IS740
> @@ -1038,6 +1038,7 @@ config SERIAL_SC16IS7XX_CORE
> SC16IS752
> SC16IS760
> SC16IS762
> + XR20M1172 (Exar)
>
> The driver supports both I2C and SPI interfaces.
>
> diff --git a/drivers/tty/serial/sc16is7xx.c b/drivers/tty/serial/sc16is7xx.c
> index dfcc804f558f..09c9e52d7ec2 100644
> --- a/drivers/tty/serial/sc16is7xx.c
> +++ b/drivers/tty/serial/sc16is7xx.c
> @@ -10,6 +10,7 @@
> #undef DEFAULT_SYMBOL_NAMESPACE
> #define DEFAULT_SYMBOL_NAMESPACE SERIAL_NXP_SC16IS7XX
>
> +#include <linux/bitfield.h>
> #include <linux/clk.h>
> #include <linux/delay.h>
> #include <linux/device.h>
> @@ -68,6 +69,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 XR20M117X_DLD_REG (0x02) /* Divisor Fractional Register */
>
> /* Enhanced Register set: Only if (LCR == 0xBF) */
> #define SC16IS7XX_EFR_REG (0x02) /* Enhanced Features */
> @@ -221,6 +223,20 @@
> #define SC16IS7XX_TCR_RX_HALT(words) ((((words) / 4) & 0x0f) << 0)
> #define SC16IS7XX_TCR_RX_RESUME(words) ((((words) / 4) & 0x0f) << 4)
>
> +/*
> + * Divisor Fractional Register 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 XR20M117X_DLD_16X 0
> +#define XR20M117X_DLD_DIV_MASK GENMASK(3, 0)
> +#define XR20M117X_DLD_8X BIT(4)
> +#define XR20M117X_DLD_4X BIT(5)
> +
> /*
> * TLR register bits
> * If TLR[3:0] or TLR[7:4] are logical 0, the selectable trigger levels via the
> @@ -523,6 +539,13 @@ const struct sc16is7xx_devtype sc16is762_devtype = {
> };
> EXPORT_SYMBOL_GPL(sc16is762_devtype);
>
> +const struct sc16is7xx_devtype xr20m1172_devtype = {
> + .name = "XR20M1172",
> + .nr_gpio = 8,
> + .nr_uart = 2,
> +};
> +EXPORT_SYMBOL_GPL(xr20m1172_devtype);
> +
> static bool sc16is7xx_regmap_volatile(struct device *dev, unsigned int reg)
> {
> switch (reg) {
> @@ -555,18 +578,43 @@ static bool sc16is7xx_regmap_noinc(struct device *dev, unsigned int reg)
> return reg == SC16IS7XX_RHR_REG;
> }
>
> +static bool sc16is7xx_has_dld(struct device *dev)
> +{
> + struct sc16is7xx_port *s = dev_get_drvdata(dev);
> +
> + if (s->devtype == &xr20m1172_devtype)
> + return true;
> + return false;
> +}
> +
> static int sc16is7xx_set_baud(struct uart_port *port, int baud)
> {
> struct sc16is7xx_one *one = to_sc16is7xx_one(port, port);
> - u8 lcr;
> + unsigned long clk = port->uartclk, div, div16;
> + bool has_dld = sc16is7xx_has_dld(port->dev);
> + u8 dld_mode = XR20M117X_DLD_16X;
> u8 prescaler = 0;
> - unsigned long clk = port->uartclk, div = clk / 16 / baud;
> + u8 divisor = 16;
> + u8 lcr;
> +
> + if (has_dld && DIV_ROUND_CLOSEST(clk, baud) < 16)
> + divisor = rounddown_pow_of_two(DIV_ROUND_CLOSEST(clk, baud));
> +
> + div16 = (clk * 16) / divisor / baud;
> + div = div16 / 16;
>
> if (div >= BIT(16)) {
> prescaler = SC16IS7XX_MCR_CLKSEL_BIT;
> div /= 4;
> }
>
> + /* Count additional divisor for EXAR devices */
> + if (divisor == 8)
> + dld_mode = XR20M117X_DLD_8X;
> + if (divisor == 4)
> + dld_mode = XR20M117X_DLD_4X;
> + dld_mode |= FIELD_PREP(XR20M117X_DLD_DIV_MASK, div16);
> +
> /* Enable enhanced features */
> sc16is7xx_efr_lock(port);
> sc16is7xx_port_update(port, SC16IS7XX_EFR_REG,
> @@ -587,12 +635,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, XR20M117X_DLD_REG, dld_mode);
> 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,
> @@ -1002,6 +1052,8 @@ static void sc16is7xx_set_termios(struct uart_port *port,
> const struct ktermios *old)
> {
> struct sc16is7xx_one *one = to_sc16is7xx_one(port, port);
> + bool has_dld = sc16is7xx_has_dld(port->dev);
> + u8 divisor = has_dld ? 4 : 16
You are missing a semicolumn here, and compilation aborts. Looks like
you didn't test.
Hugo.
> unsigned int lcr, flow = 0;
> int baud;
> unsigned long flags;
> @@ -1084,7 +1136,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 / divisor);
>
> /* Setup baudrate generator */
> baud = sc16is7xx_set_baud(port, baud);
> @@ -1684,6 +1736,7 @@ void sc16is7xx_remove(struct device *dev)
> EXPORT_SYMBOL_GPL(sc16is7xx_remove);
>
> const struct of_device_id __maybe_unused sc16is7xx_dt_ids[] = {
> + { .compatible = "exar,xr20m1172", .data = &xr20m1172_devtype, },
> { .compatible = "nxp,sc16is740", .data = &sc16is74x_devtype, },
> { .compatible = "nxp,sc16is741", .data = &sc16is74x_devtype, },
> { .compatible = "nxp,sc16is750", .data = &sc16is750_devtype, },
> diff --git a/drivers/tty/serial/sc16is7xx.h b/drivers/tty/serial/sc16is7xx.h
> index afb784eaee45..eb2e3bc86f15 100644
> --- a/drivers/tty/serial/sc16is7xx.h
> +++ b/drivers/tty/serial/sc16is7xx.h
> @@ -28,6 +28,7 @@ extern const struct sc16is7xx_devtype sc16is750_devtype;
> extern const struct sc16is7xx_devtype sc16is752_devtype;
> extern const struct sc16is7xx_devtype sc16is760_devtype;
> extern const struct sc16is7xx_devtype sc16is762_devtype;
> +extern const struct sc16is7xx_devtype xr20m1172_devtype;
>
> const char *sc16is7xx_regmap_name(u8 port_id);
>
> diff --git a/drivers/tty/serial/sc16is7xx_i2c.c b/drivers/tty/serial/sc16is7xx_i2c.c
> index 3ed47c306d85..839de902821b 100644
> --- a/drivers/tty/serial/sc16is7xx_i2c.c
> +++ b/drivers/tty/serial/sc16is7xx_i2c.c
> @@ -46,6 +46,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);
> diff --git a/drivers/tty/serial/sc16is7xx_spi.c b/drivers/tty/serial/sc16is7xx_spi.c
> index 73df36f8a7fd..2b278282dbd0 100644
> --- a/drivers/tty/serial/sc16is7xx_spi.c
> +++ b/drivers/tty/serial/sc16is7xx_spi.c
> @@ -69,6 +69,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, },
> { }
> };
> MODULE_DEVICE_TABLE(spi, sc16is7xx_spi_id_table);
> --
> 2.34.1
>
>
>
--
Hugo Villeneuve
On Mon, 22 Apr 2024 16:32:13 +0300
Konstantin Pugin <[email protected]> wrote:
Hi Konstantin,
> The hardware supports both RTS_ON_SEND and RTS_AFTER_SEND modes, but
> after the commit 4afeced55baa ("serial: core: fix sanitizing check for
> RTS settings") we always end up with SER_RS485_RTS_AFTER_SEND set and
> always write to the register field SC16IS7XX_EFCR_RTS_INVERT_BIT, which
> breaks some hardware using these chips.
it took me a long time to properly understand what you meant in your
commit message. In my setup, I have the property rs485-rts-active-low
defined, and couldn't reproduce the problem. It is only after I
commented this property that I got a warning about the "invalid RTS
setting"... I also got it by defining rs485-rts-active-high.
I suggest the following changes to the commit message:
----
When specifying flag SER_RS485_RTS_ON_SEND in RS485 configuration,
we get the following warning after commit 4afeced55baa ("serial: core:
fix sanitizing check for RTS settings"):
invalid RTS setting, using RTS_AFTER_SEND instead
This results in SER_RS485_RTS_AFTER_SEND being set and the
driver always write to the register field SC16IS7XX_EFCR_RTS_INVERT_BIT,
which breaks some hardware using these chips.
The hardware supports both RTS_ON_SEND and RTS_AFTER_SEND modes, so fix
this by announcing support for RTS_ON_SEND.
---
If you agree to these changes, feel free to add:
Tested-by: Hugo Villeneuve <[email protected]>
Hugo.
>
> Fixes: 267913ecf737 ("serial: sc16is7xx: Fill in rs485_supported")
> Signed-off-by: Konstantin Pugin <[email protected]>
> ---
> 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 03cf30e20b75..dfcc804f558f 100644
> --- a/drivers/tty/serial/sc16is7xx.c
> +++ b/drivers/tty/serial/sc16is7xx.c
> @@ -1449,7 +1449,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
>
>
>
--
Hugo Villeneuve