2024-04-19 12:45:30

by Konstantin Pugin

[permalink] [raw]
Subject: [PATCH v4 0/3] add support for EXAR XR20M1172 UART

From: Konstantin Pugin <[email protected]>

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

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 | 18 +++---
drivers/tty/serial/sc16is7xx.c | 62 +++++++++++++++++--
drivers/tty/serial/sc16is7xx_i2c.c | 1 +
drivers/tty/serial/sc16is7xx_spi.c | 1 +
5 files changed, 70 insertions(+), 13 deletions(-)


base-commit: c6795fbffc4547b40933ec368200bd4926a41b44
--
2.34.1



2024-04-19 12:45:32

by Konstantin Pugin

[permalink] [raw]
Subject: [PATCH v4 1/3] serial: sc16is7xx: announce support of SER_RS485_RTS_ON_SEND

From: Konstantin Pugin <[email protected]>

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


2024-04-19 12:45:52

by Konstantin Pugin

[permalink] [raw]
Subject: [PATCH v4 2/3] dt-bindings: sc16is7xx: Add compatible line for XR20M1172 UART

From: Konstantin Pugin <[email protected]>

Add EXAR XR20M1172 UART compatible line into devicetree documentation.

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


2024-04-19 12:46:06

by Konstantin Pugin

[permalink] [raw]
Subject: [PATCH v4 3/3] serial: sc16is7xx: add support for EXAR XR20M1172 UART

From: Konstantin Pugin <[email protected]>

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 | 18 +++++----
drivers/tty/serial/sc16is7xx.c | 60 ++++++++++++++++++++++++++++--
drivers/tty/serial/sc16is7xx_i2c.c | 1 +
drivers/tty/serial/sc16is7xx_spi.c | 1 +
4 files changed, 68 insertions(+), 12 deletions(-)

diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
index 4fdd7857ef4d..4380bfe7dfff 100644
--- a/drivers/tty/serial/Kconfig
+++ b/drivers/tty/serial/Kconfig
@@ -1029,15 +1029,17 @@ 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-compatible UARTs.
Supported ICs are:
-
- SC16IS740
- SC16IS741
- SC16IS750
- SC16IS752
- SC16IS760
- SC16IS762
+ NXP:
+ SC16IS740
+ SC16IS741
+ SC16IS750
+ SC16IS752
+ SC16IS760
+ SC16IS762
+ EXAR:
+ XR20M1172

The driver supports both I2C and SPI interfaces.

diff --git a/drivers/tty/serial/sc16is7xx.c b/drivers/tty/serial/sc16is7xx.c
index dfcc804f558f..b2aa2e487c84 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);

+static const struct sc16is7xx_devtype xr20m1172_devtype = {
+ .name = "XR20M1172",
+ .nr_gpio = 8,
+ .nr_uart = 2,
+};
+EXPORT_SYMBOL_GPL(sc16is762_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,7 @@ 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);
unsigned int lcr, flow = 0;
int baud;
unsigned long flags;
@@ -1084,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);
@@ -1684,6 +1735,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_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


2024-04-19 13:32:39

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v4 2/3] dt-bindings: sc16is7xx: Add compatible line for XR20M1172 UART

On 19/04/2024 14:45, Konstantin Pugin wrote:
> From: Konstantin Pugin <[email protected]>
>
> Add EXAR XR20M1172 UART compatible line into devicetree documentation.
>
> Reviewed-by: Vladimir Zapolskiy <[email protected]>
> Signed-off-by: Konstantin Pugin <[email protected]>
> ---
> Documentation/devicetree/bindings/serial/nxp,sc16is7xx.ya

This is fourth change, no cover letter, no changelog. Patch is trivial
but you do not make it easier to understand what is happening here.

Please provide proper changelog under ---.

(If you wrote changelog somewhere else and then decided not to send it
to us, it is like there was no changelog. I literally do not have it in
my inbox).


Acked-by: Krzysztof Kozlowski <[email protected]>


---

This is an automated instruction, just in case, because many review tags
are being ignored. If you know the process, you can skip it (please do
not feel offended by me posting it here - no bad intentions intended).
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

Best regards,
Krzysztof


2024-04-19 13:43:26

by Hugo Villeneuve

[permalink] [raw]
Subject: Re: [PATCH v4 0/3] add support for EXAR XR20M1172 UART

On Fri, 19 Apr 2024 15:45:00 +0300
Konstantin Pugin <[email protected]> wrote:

> From: Konstantin Pugin <[email protected]>
>
> 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.

Hi Konstantin,
it would probably be a good idea to also add the device to the list of supported devices for this driver in the Kconfig file.

Hugo.


>
> 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
>
> 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 | 18 +++---
> drivers/tty/serial/sc16is7xx.c | 62 +++++++++++++++++--
> drivers/tty/serial/sc16is7xx_i2c.c | 1 +
> drivers/tty/serial/sc16is7xx_spi.c | 1 +
> 5 files changed, 70 insertions(+), 13 deletions(-)
>
>
> base-commit: c6795fbffc4547b40933ec368200bd4926a41b44
> --
> 2.34.1
>
>
>


--
Hugo Villeneuve <[email protected]>

2024-04-19 13:44:48

by Konstantin P.

[permalink] [raw]
Subject: Re: [PATCH v4 1/3] serial: sc16is7xx: announce support of SER_RS485_RTS_ON_SEND

On Fri, Apr 19, 2024 at 4:29 PM Hugo Villeneuve <[email protected]> wrote:
>
> On Fri, 19 Apr 2024 15:45:01 +0300
> Konstantin Pugin <[email protected]> wrote:
>
> > From: Konstantin Pugin <[email protected]>
> >
> > 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.
>
> Hi Konstantin,
> I am using RS-485 mode, and currently I may need SC16IS7XX_EFCR_RTS_INVERT_BIT to be set for my setup to work (not sure, must check this). Does it mean that with this patch it will be cleared? I am not sure by reading your commit message...
>
> I need to look at this in more details, and test it on my hardware, which I will be able to do only next week. I will keep you posted.
>
> Hugo.

Hello, Hugo. My patch's changes change behaviour in one place. Now you
can select SC16IS7XX_EFCR_RTS_INVERT_BIT setting by passing
SER_RS485_RTS_AFTER_SEND (in this case this bit will be set) or
SER_RS485_RTS_ON_SEND (in this case this bit will be clear). Previous
behaviour was always setting the bit, even if SER_RS485_RTS_ON_SEND is
passed, so there was no way to run hardware with bit cleared, and for
my hardware it is a requirement.

>
> >
> > 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 <[email protected]>

2024-04-19 13:45:20

by Konstantin P.

[permalink] [raw]
Subject: Re: [PATCH v4 0/3] add support for EXAR XR20M1172 UART

On Fri, Apr 19, 2024 at 4:43 PM Hugo Villeneuve <[email protected]> wrote:
>
> On Fri, 19 Apr 2024 15:45:00 +0300
> Konstantin Pugin <[email protected]> wrote:
>
> > From: Konstantin Pugin <[email protected]>
> >
> > 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.
>
> Hi Konstantin,
> it would probably be a good idea to also add the device to the list of supported devices for this driver in the Kconfig file.
>
> Hugo.

It is already done in V4.
>
> >
> > 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
> >
> > 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 | 18 +++---
> > drivers/tty/serial/sc16is7xx.c | 62 +++++++++++++++++--
> > drivers/tty/serial/sc16is7xx_i2c.c | 1 +
> > drivers/tty/serial/sc16is7xx_spi.c | 1 +
> > 5 files changed, 70 insertions(+), 13 deletions(-)
> >
> >
> > base-commit: c6795fbffc4547b40933ec368200bd4926a41b44
> > --
> > 2.34.1
> >
> >
> >
>
>
> --
> Hugo Villeneuve <[email protected]>

2024-04-19 13:48:07

by Konstantin P.

[permalink] [raw]
Subject: Re: [PATCH v4 2/3] dt-bindings: sc16is7xx: Add compatible line for XR20M1172 UART

On Fri, Apr 19, 2024 at 4:32 PM Krzysztof Kozlowski <[email protected]> wrote:
>
> On 19/04/2024 14:45, Konstantin Pugin wrote:
> > From: Konstantin Pugin <[email protected]>
> >
> > Add EXAR XR20M1172 UART compatible line into devicetree documentation.
> >
> > Reviewed-by: Vladimir Zapolskiy <[email protected]>
> > Signed-off-by: Konstantin Pugin <[email protected]>
> > ---
> > Documentation/devicetree/bindings/serial/nxp,sc16is7xx.ya
>
> This is fourth change, no cover letter, no changelog. Patch is trivial
> but you do not make it easier to understand what is happening here.
>
> Please provide proper changelog under ---.
>
> (If you wrote changelog somewhere else and then decided not to send it
> to us, it is like there was no changelog. I literally do not have it in
> my inbox).
>
>
> Acked-by: Krzysztof Kozlowski <[email protected]>
>
>
> ---
>
> This is an automated instruction, just in case, because many review tags
> are being ignored. If you know the process, you can skip it (please do
> not feel offended by me posting it here - no bad intentions intended).
> 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
>
> Best regards,
> Krzysztof
>

I am sorry, I used git send-email, and send all 3 patches and cover
letter. I do not know why it was not ended up in your mailbox.
Link to all patches (version 4)
https://lore.kernel.org/linux-serial/[email protected]/.

Here is a git send-email log for cover letter:

```
$ git send-email v4*.patch --cc-cmd='./scripts/get_maintainer.pl
--norolestats v4-0001-serial-sc16is7xx-announce-support-of-SER_RS485_RT.patch'
v4-0000-cover-letter.patch
v4-0001-serial-sc16is7xx-announce-support-of-SER_RS485_RT.patch
v4-0002-dt-bindings-sc16is7xx-Add-compatible-line-for-XR2.patch
v4-0003-serial-sc16is7xx-add-support-for-EXAR-XR20M1172-U.patch
To whom should the emails be sent (if anyone)?
Message-ID to be used as In-Reply-To for the first email (if any)?
(mbox) Adding cc: Konstantin Pugin <[email protected]> from
line 'From: Konstantin Pugin <[email protected]>'
/scripts/get_maintainer.pl: file 'v4-0000-cover-letter.patch' doesn't
appear to be a patch. Add -f to options?
(cc-cmd) Adding cc: Greg Kroah-Hartman <[email protected]>
from: './scripts/get_maintainer.pl --norolestats
v4-0001-serial-sc16is7xx-announce-support-of-SER_RS485_RT.patch'
(cc-cmd) Adding cc: Jiri Slaby <[email protected]> from:
'./scripts/get_maintainer.pl --norolestats
v4-0001-serial-sc16is7xx-announce-support-of-SER_RS485_RT.patch'
(cc-cmd) Adding cc: Hugo Villeneuve <[email protected]> from:
'./scripts/get_maintainer.pl --norolestats
v4-0001-serial-sc16is7xx-announce-support-of-SER_RS485_RT.patch'
(cc-cmd) Adding cc: Andy Shevchenko
<[email protected]> from: './scripts/get_maintainer.pl
--norolestats v4-0001-serial-sc16is7xx-announce-support-of-SER_RS485_RT.patch'
(cc-cmd) Adding cc: Lech Perczak <[email protected]> from:
'./scripts/get_maintainer.pl --norolestats
v4-0001-serial-sc16is7xx-announce-support-of-SER_RS485_RT.patch'
(cc-cmd) Adding cc: =?UTF-8?q?Ilpo=20J=C3=A4rvinen?=
<[email protected]> from: './scripts/get_maintainer.pl
--norolestats v4-0001-serial-sc16is7xx-announce-support-of-SER_RS485_RT.patch'
(cc-cmd) Adding cc: Thomas Gleixner <[email protected]> from:
'./scripts/get_maintainer.pl --norolestats
v4-0001-serial-sc16is7xx-announce-support-of-SER_RS485_RT.patch'
(cc-cmd) Adding cc: [email protected] from:
'./scripts/get_maintainer.pl --norolestats
v4-0001-serial-sc16is7xx-announce-support-of-SER_RS485_RT.patch'
(cc-cmd) Adding cc: [email protected] from:
'./scripts/get_maintainer.pl --norolestats
v4-0001-serial-sc16is7xx-announce-support-of-SER_RS485_RT.patch'

From: Konstantin Pugin <[email protected]>
To:
Cc: Konstantin Pugin <[email protected]>,
Greg Kroah-Hartman <[email protected]>,
Jiri Slaby <[email protected]>,
Hugo Villeneuve <[email protected]>,
Andy Shevchenko <[email protected]>,
Lech Perczak <[email protected]>,
=?UTF-8?q?Ilpo=20J=C3=A4rvinen?= <[email protected]>,
Thomas Gleixner <[email protected]>,
[email protected],
[email protected]
Subject: [PATCH v4 0/3] add support for EXAR XR20M1172 UART
Date: Fri, 19 Apr 2024 15:45:00 +0300
Message-Id: <[email protected]>
X-Mailer: git-send-email 2.34.1
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit

The Cc list above has been expanded by additional
addresses found in the patch commit message. By default
send-email prompts before sending whenever this occurs.
This behavior is controlled by the sendemail.confirm
configuration setting.

For additional information, run 'git send-email --help'.
To retain the current behavior, but squelch this message,
run 'git config --global sendemail.confirm auto'.

Send this email? ([y]es|[n]o|[e]dit|[q]uit|[a]ll):
OK. Log says:
Server: smtp.yandex.ru
MAIL FROM:<[email protected]>
RCPT TO:<[email protected]>
RCPT TO:<[email protected]>
RCPT TO:<[email protected]>
RCPT TO:<[email protected]>
RCPT TO:<[email protected]>
RCPT TO:<[email protected]>
RCPT TO:<[email protected]>
RCPT TO:<[email protected]>
RCPT TO:<[email protected]>
RCPT TO:<[email protected]>
From: Konstantin Pugin <[email protected]>
To:
Cc: Konstantin Pugin <[email protected]>,
Greg Kroah-Hartman <[email protected]>,
Jiri Slaby <[email protected]>,
Hugo Villeneuve <[email protected]>,
Andy Shevchenko <[email protected]>,
Lech Perczak <[email protected]>,
=?UTF-8?q?Ilpo=20J=C3=A4rvinen?= <[email protected]>,
Thomas Gleixner <[email protected]>,
[email protected],
[email protected]
Subject: [PATCH v4 0/3] add support for EXAR XR20M1172 UART
Date: Fri, 19 Apr 2024 15:45:00 +0300
Message-Id: <[email protected]>
X-Mailer: git-send-email 2.34.1
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit

Result: 250

```

2024-04-19 13:49:27

by Hugo Villeneuve

[permalink] [raw]
Subject: Re: [PATCH v4 1/3] serial: sc16is7xx: announce support of SER_RS485_RTS_ON_SEND

On Fri, 19 Apr 2024 15:45:01 +0300
Konstantin Pugin <[email protected]> wrote:

> From: Konstantin Pugin <[email protected]>
>
> 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.

Hi Konstantin,
I am using RS-485 mode, and currently I may need SC16IS7XX_EFCR_RTS_INVERT_BIT to be set for my setup to work (not sure, must check this). Does it mean that with this patch it will be cleared? I am not sure by reading your commit message...

I need to look at this in more details, and test it on my hardware, which I will be able to do only next week. I will keep you posted.

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 <[email protected]>

2024-04-19 14:01:15

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v4 2/3] dt-bindings: sc16is7xx: Add compatible line for XR20M1172 UART

On 19/04/2024 15:49, Konstantin P. wrote:
>> 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
>>
>> Best regards,
>> Krzysztof
>>
>
> I am sorry, I used git send-email, and send all 3 patches and cover
> letter. I do not know why it was not ended up in your mailbox.
> Link to all patches (version 4)
> https://lore.kernel.org/linux-serial/[email protected]/.
>
> Here is a git send-email log for cover letter:

So read the log and you will see that it does not send to everyone.
That's how cc-cmd works.

I would suggest to switch to b4 or patman.

Best regards,
Krzysztof


2024-04-19 14:10:57

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH v4 2/3] dt-bindings: sc16is7xx: Add compatible line for XR20M1172 UART

On Fri, Apr 19, 2024 at 03:45:02PM +0300, Konstantin Pugin wrote:
> From: Konstantin Pugin <[email protected]>
>
> Add EXAR XR20M1172 UART compatible line into devicetree documentation.

What you're doing is obvious from the diff, why this exar device is
related to the nxp devices is what should be mentioned here.

Thanks,
Conor.

>
> 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
>


Attachments:
(No filename) (1.08 kB)
signature.asc (235.00 B)
Download all attachments

2024-04-19 14:23:56

by Konstantin P.

[permalink] [raw]
Subject: Re: [PATCH v4 2/3] dt-bindings: sc16is7xx: Add compatible line for XR20M1172 UART

On Fri, Apr 19, 2024 at 5:08 PM Conor Dooley <[email protected]> wrote:
>
> On Fri, Apr 19, 2024 at 03:45:02PM +0300, Konstantin Pugin wrote:
> > From: Konstantin Pugin <[email protected]>
> >
> > Add EXAR XR20M1172 UART compatible line into devicetree documentation.
>
> What you're doing is obvious from the diff, why this exar device is
> related to the nxp devices is what should be mentioned here.
>
> Thanks,
> Conor.

It is already mentioned in cover letter and in previous patches in the
series. Do I need to repeat it in DTS patch?
If so, I will do it.

Citation from my cover letter:

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.

Also, comparision from NXP itself:
http://www.bdtic.com/download/nxp/75017168.pdf (pp12-13 is about XR20M1172).

> >
> > 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
> >

2024-04-19 14:24:26

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v4 2/3] dt-bindings: sc16is7xx: Add compatible line for XR20M1172 UART

On 19/04/2024 16:17, Konstantin P. wrote:
> On Fri, Apr 19, 2024 at 5:08 PM Conor Dooley <[email protected]> wrote:
>>
>> On Fri, Apr 19, 2024 at 03:45:02PM +0300, Konstantin Pugin wrote:
>>> From: Konstantin Pugin <[email protected]>
>>>
>>> Add EXAR XR20M1172 UART compatible line into devicetree documentation.
>>
>> What you're doing is obvious from the diff, why this exar device is
>> related to the nxp devices is what should be mentioned here.
>>
>> Thanks,
>> Conor.
>
> It is already mentioned in cover letter and in previous patches in the
> series. Do I need to repeat it in DTS patch?
> If so, I will do it.
>
> Citation from my cover letter:
>
> 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.

Commits must stand on their own. Cover letter is not merged. This is the
place where you add new hardware, so here you describe and explain the
hardware.

Best regards,
Krzysztof


2024-04-19 14:32:49

by Konstantin P.

[permalink] [raw]
Subject: Re: [PATCH v4 2/3] dt-bindings: sc16is7xx: Add compatible line for XR20M1172 UART

On Fri, Apr 19, 2024 at 5:24 PM Krzysztof Kozlowski <[email protected]> wrote:
>
> On 19/04/2024 16:17, Konstantin P. wrote:
> > On Fri, Apr 19, 2024 at 5:08 PM Conor Dooley <[email protected]> wrote:
> >>
> >> On Fri, Apr 19, 2024 at 03:45:02PM +0300, Konstantin Pugin wrote:
> >>> From: Konstantin Pugin <[email protected]>
> >>>
> >>> Add EXAR XR20M1172 UART compatible line into devicetree documentation.
> >>
> >> What you're doing is obvious from the diff, why this exar device is
> >> related to the nxp devices is what should be mentioned here.
> >>
> >> Thanks,
> >> Conor.
> >
> > It is already mentioned in cover letter and in previous patches in the
> > series. Do I need to repeat it in DTS patch?
> > If so, I will do it.
> >
> > Citation from my cover letter:
> >
> > 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.
>
> Commits must stand on their own. Cover letter is not merged. This is the
> place where you add new hardware, so here you describe and explain the
> hardware.

It is also described in patch 3 in the series. I need to repeat this
description in patch 2 too?

Cite from patch 3:

XR20M1172 register set is mostly compatible with SC16IS762, but it has
a support for additional division rates of UART with special DLD register.

> Best regards,
> Krzysztof
>

2024-04-19 14:52:56

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH v4 2/3] dt-bindings: sc16is7xx: Add compatible line for XR20M1172 UART

On Fri, Apr 19, 2024 at 05:34:44PM +0300, Konstantin P. wrote:
> On Fri, Apr 19, 2024 at 5:24 PM Krzysztof Kozlowski <[email protected]> wrote:
> > On 19/04/2024 16:17, Konstantin P. wrote:

..

> > Commits must stand on their own. Cover letter is not merged. This is the
> > place where you add new hardware, so here you describe and explain the
> > hardware.
>
> It is also described in patch 3 in the series. I need to repeat this
> description in patch 2 too?
>
> Cite from patch 3:
>
> XR20M1172 register set is mostly compatible with SC16IS762, but it has
> a support for additional division rates of UART with special DLD register.

The point is, if I got it correctly, to have a few words in the description
of the DT binding itself, so whoever reads the bindings (w/o even accessing
the Git history of the project) may understand this.

--
With Best Regards,
Andy Shevchenko



2024-04-19 14:57:04

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH v4 2/3] dt-bindings: sc16is7xx: Add compatible line for XR20M1172 UART

On Fri, Apr 19, 2024 at 05:52:38PM +0300, Andy Shevchenko wrote:
> On Fri, Apr 19, 2024 at 05:34:44PM +0300, Konstantin P. wrote:
> > On Fri, Apr 19, 2024 at 5:24 PM Krzysztof Kozlowski <[email protected]> wrote:
> > > On 19/04/2024 16:17, Konstantin P. wrote:
>
> ...
>
> > > Commits must stand on their own. Cover letter is not merged. This is the
> > > place where you add new hardware, so here you describe and explain the
> > > hardware.
> >
> > It is also described in patch 3 in the series. I need to repeat this
> > description in patch 2 too?
> >
> > Cite from patch 3:
> >
> > XR20M1172 register set is mostly compatible with SC16IS762, but it has
> > a support for additional division rates of UART with special DLD register.
>
> The point is, if I got it correctly, to have a few words in the description
> of the DT binding itself, so whoever reads the bindings (w/o even accessing
> the Git history of the project) may understand this.

Yes, each patch must independently justify the change - although in this
case it is about the history not people reading the bindings as I was
talking about the commit message and not the contents of the diff.
It's especially true when I only get sent the bindings patch, although
from other messages in this thread that may not have been intentional.

Thanks,
Conor.


Attachments:
(No filename) (1.32 kB)
signature.asc (235.00 B)
Download all attachments

2024-04-19 14:57:44

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH v4 2/3] dt-bindings: sc16is7xx: Add compatible line for XR20M1172 UART

On Fri, Apr 19, 2024 at 04:24:16PM +0200, Krzysztof Kozlowski wrote:
> On 19/04/2024 16:17, Konstantin P. wrote:
> > On Fri, Apr 19, 2024 at 5:08 PM Conor Dooley <[email protected]> wrote:

..

> Commits must stand on their own. Cover letter is not merged.

While the first is true, the second one depends on the maintainer and tooling.
For the patch series some maintainers either already use custom, or started
using `b4` feature to make cover letter to be a merge commit-like message.

Yet, it's not the case for TTY subsystem AFAIK.

> This is the place where you add new hardware, so here you describe and
> explain the hardware.

--
With Best Regards,
Andy Shevchenko



2024-04-20 06:05:43

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH v4 3/3] serial: sc16is7xx: add support for EXAR XR20M1172 UART

Hi Konstantin,

kernel test robot noticed the following build errors:

[auto build test ERROR on c6795fbffc4547b40933ec368200bd4926a41b44]

url: https://github.com/intel-lab-lkp/linux/commits/Konstantin-Pugin/serial-sc16is7xx-announce-support-of-SER_RS485_RTS_ON_SEND/20240420-004819
base: c6795fbffc4547b40933ec368200bd4926a41b44
patch link: https://lore.kernel.org/r/20240419124506.1531035-4-rilian.la.te%40ya.ru
patch subject: [PATCH v4 3/3] serial: sc16is7xx: add support for EXAR XR20M1172 UART
config: arc-randconfig-002-20240420 (https://download.01.org/0day-ci/archive/20240420/[email protected]/config)
compiler: arceb-elf-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240420/[email protected]/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/

All errors (new ones prefixed by >>):

>> drivers/tty/serial/sc16is7xx_i2c.c:49:42: error: 'xr20m1172_devtype' undeclared here (not in a function)
49 | { "xr20m1172", (kernel_ulong_t)&xr20m1172_devtype, },
| ^~~~~~~~~~~~~~~~~
--
>> drivers/tty/serial/sc16is7xx_spi.c:72:42: error: 'xr20m1172_devtype' undeclared here (not in a function)
72 | { "xr20m1172", (kernel_ulong_t)&xr20m1172_devtype, },
| ^~~~~~~~~~~~~~~~~
--
{standard input}: Assembler messages:
>> {standard input}:16: Error: symbol `__export_symbol_sc16is762_devtype' is already defined


vim +/xr20m1172_devtype +49 drivers/tty/serial/sc16is7xx_i2c.c

40
41 static const struct i2c_device_id sc16is7xx_i2c_id_table[] = {
42 { "sc16is74x", (kernel_ulong_t)&sc16is74x_devtype, },
43 { "sc16is740", (kernel_ulong_t)&sc16is74x_devtype, },
44 { "sc16is741", (kernel_ulong_t)&sc16is74x_devtype, },
45 { "sc16is750", (kernel_ulong_t)&sc16is750_devtype, },
46 { "sc16is752", (kernel_ulong_t)&sc16is752_devtype, },
47 { "sc16is760", (kernel_ulong_t)&sc16is760_devtype, },
48 { "sc16is762", (kernel_ulong_t)&sc16is762_devtype, },
> 49 { "xr20m1172", (kernel_ulong_t)&xr20m1172_devtype, },
50 { }
51 };
52 MODULE_DEVICE_TABLE(i2c, sc16is7xx_i2c_id_table);
53

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

2024-04-20 07:29:00

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH v4 3/3] serial: sc16is7xx: add support for EXAR XR20M1172 UART

Hi Konstantin,

kernel test robot noticed the following build errors:

[auto build test ERROR on c6795fbffc4547b40933ec368200bd4926a41b44]

url: https://github.com/intel-lab-lkp/linux/commits/Konstantin-Pugin/serial-sc16is7xx-announce-support-of-SER_RS485_RTS_ON_SEND/20240420-004819
base: c6795fbffc4547b40933ec368200bd4926a41b44
patch link: https://lore.kernel.org/r/20240419124506.1531035-4-rilian.la.te%40ya.ru
patch subject: [PATCH v4 3/3] serial: sc16is7xx: add support for EXAR XR20M1172 UART
config: hexagon-randconfig-001-20240420 (https://download.01.org/0day-ci/archive/20240420/[email protected]/config)
compiler: clang version 19.0.0git (https://github.com/llvm/llvm-project 7089c359a3845323f6f30c44a47dd901f2edfe63)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240420/[email protected]/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/

All errors (new ones prefixed by >>):

In file included from drivers/tty/serial/sc16is7xx_spi.c:7:
In file included from include/linux/regmap.h:20:
In file included from include/linux/iopoll.h:14:
In file included from include/linux/io.h:13:
In file included from arch/hexagon/include/asm/io.h:328:
include/asm-generic/io.h:547:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
547 | val = __raw_readb(PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:560:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
560 | val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr));
| ~~~~~~~~~~ ^
include/uapi/linux/byteorder/little_endian.h:37:51: note: expanded from macro '__le16_to_cpu'
37 | #define __le16_to_cpu(x) ((__force __u16)(__le16)(x))
| ^
In file included from drivers/tty/serial/sc16is7xx_spi.c:7:
In file included from include/linux/regmap.h:20:
In file included from include/linux/iopoll.h:14:
In file included from include/linux/io.h:13:
In file included from arch/hexagon/include/asm/io.h:328:
include/asm-generic/io.h:573:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
573 | val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
| ~~~~~~~~~~ ^
include/uapi/linux/byteorder/little_endian.h:35:51: note: expanded from macro '__le32_to_cpu'
35 | #define __le32_to_cpu(x) ((__force __u32)(__le32)(x))
| ^
In file included from drivers/tty/serial/sc16is7xx_spi.c:7:
In file included from include/linux/regmap.h:20:
In file included from include/linux/iopoll.h:14:
In file included from include/linux/io.h:13:
In file included from arch/hexagon/include/asm/io.h:328:
include/asm-generic/io.h:584:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
584 | __raw_writeb(value, PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:594:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
594 | __raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:604:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
604 | __raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
In file included from drivers/tty/serial/sc16is7xx_spi.c:8:
In file included from include/linux/spi/spi.h:17:
In file included from include/linux/scatterlist.h:8:
In file included from include/linux/mm.h:2208:
include/linux/vmstat.h:522:36: warning: arithmetic between different enumeration types ('enum node_stat_item' and 'enum lru_list') [-Wenum-enum-conversion]
522 | return node_stat_name(NR_LRU_BASE + lru) + 3; // skip "nr_"
| ~~~~~~~~~~~ ^ ~~~
>> drivers/tty/serial/sc16is7xx_spi.c:72:34: error: use of undeclared identifier 'xr20m1172_devtype'
72 | { "xr20m1172", (kernel_ulong_t)&xr20m1172_devtype, },
| ^
7 warnings and 1 error generated.


vim +/xr20m1172_devtype +72 drivers/tty/serial/sc16is7xx_spi.c

63
64 static const struct spi_device_id sc16is7xx_spi_id_table[] = {
65 { "sc16is74x", (kernel_ulong_t)&sc16is74x_devtype, },
66 { "sc16is740", (kernel_ulong_t)&sc16is74x_devtype, },
67 { "sc16is741", (kernel_ulong_t)&sc16is74x_devtype, },
68 { "sc16is750", (kernel_ulong_t)&sc16is750_devtype, },
69 { "sc16is752", (kernel_ulong_t)&sc16is752_devtype, },
70 { "sc16is760", (kernel_ulong_t)&sc16is760_devtype, },
71 { "sc16is762", (kernel_ulong_t)&sc16is762_devtype, },
> 72 { "xr20m1172", (kernel_ulong_t)&xr20m1172_devtype, },
73 { }
74 };
75 MODULE_DEVICE_TABLE(spi, sc16is7xx_spi_id_table);
76

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki