* Re: [RFC PATCH v1 2/9] uaccess: Convert INLINE_COPY_{TO/FROM}_USER to kconfig and reduce ifdefery
From: Andrew Cooper @ 2026-04-28 9:36 UTC (permalink / raw)
To: Yury Norov
Cc: Andrew Cooper, Christophe Leroy (CS GROUP), Andrew Morton,
Linus Torvalds, David Laight, Thomas Gleixner, linux-alpha,
Yury Norov, linux-kernel, linux-snps-arc, linux-arm-kernel,
linux-mips, linuxppc-dev, kvm, linux-riscv, linux-s390,
sparclinux, linux-um, dmaengine, linux-efi, linux-fsi, amd-gfx,
dri-devel, intel-gfx, linux-wpan, netdev, linux-wireless,
linux-spi, linux-media, linux-staging, linux-serial, linux-usb,
xen-devel, linux-fsdevel, ocfs2-devel, bpf, kasan-dev, linux-mm,
linux-x25, rust-for-linux, linux-sound, sound-open-firmware,
linux-csky, linux-hexagon, loongarch, linux-m68k, linux-openrisc,
linux-parisc, linux-sh, linux-arch
In-Reply-To: <ae_LeSk7XDEseaZb@yury>
On 27/04/2026 9:47 pm, Yury Norov wrote:
> On Mon, Apr 27, 2026 at 09:39:33PM +0100, Andrew Cooper wrote:
>> On 27/04/2026 7:39 pm, Yury Norov wrote:
>>> On Mon, Apr 27, 2026 at 07:13:43PM +0200, Christophe Leroy (CS GROUP) wrote:
>>>> Among the 21 architectures supported by the kernel, 16 define both
>>>> INLINE_COPY_TO_USER and INLINE_COPY_FROM_USER while the 5 other ones
>>>> don't define any of the two.
>>>>
>>>> To simplify and reduce risk of mistakes, convert them to a single
>>>> kconfig item named CONFIG_ARCH_WANTS_NOINLINE_COPY which will be
>>> We've got a special word for it: outline. Can you name it
>>> CONFIG_OUTLINE_USERCOPY, or similar?
>> You can't swap the "in" for "out" like this. "out of line" is the
>> opposite of "inline" in this context, while "outline" means something
>> different and unrelated.
> Check KASAN_OUTLINE vs KASAN_INLINE for example
Then I suggest it gets corrected before more examples try to copy this
non-english.
~Andrew
^ permalink raw reply
* Re: [RFC PATCH v1 4/9] uaccess: Introduce copy_{to/from}_user_partial()
From: Geert Uytterhoeven @ 2026-04-28 9:25 UTC (permalink / raw)
To: Christophe Leroy (CS GROUP)
Cc: Yury Norov, Andrew Morton, Linus Torvalds, David Laight,
Thomas Gleixner, linux-alpha, linux-kernel, linux-snps-arc,
linux-arm-kernel, linux-mips, linuxppc-dev, kvm, linux-riscv,
linux-s390, sparclinux, linux-um, dmaengine, linux-efi, linux-fsi,
amd-gfx, dri-devel, intel-gfx, linux-wpan, netdev, linux-wireless,
linux-spi, linux-media, linux-staging, linux-serial, linux-usb,
xen-devel, linux-fsdevel, ocfs2-devel, bpf, kasan-dev, linux-mm,
linux-x25, rust-for-linux, linux-sound, sound-open-firmware,
linux-csky, linux-hexagon, loongarch, linux-m68k, linux-openrisc,
linux-parisc, linux-sh, linux-arch
In-Reply-To: <c73b90236f2810edd47c84edd2a8d8e8e0c816da.1777306795.git.chleroy@kernel.org>
Hi Christophe,
Thanks for your patch!
On Mon, 27 Apr 2026 at 19:18, Christophe Leroy (CS GROUP)
<chleroy@kernel.org> wrote:
> Today there are approximately 3000 calls for copy_to_user() and
> 3000 calls to copy_from_user().
>
> The majority of callers of copy_{to/from}_user() don't care about the
> return value, they only check whether it is 0 or not, and when it is
> not 0 they handle it as a -EACCES.
I think the "a" can be dropped.
> In order to allow better optimisation of copy_{to/from}_user() when
> the size of the copy is known at build time, create new fonctions
functions
> named copy_{to/from}_user_partial() to be used by the few callers
> that are interested in partial copies and need to now how many
know
> bytes remain at the end of the copy.
>
> For the time being it is just the same as copy_{to/from}_user().
>
> Signed-off-by: Christophe Leroy (CS GROUP) <chleroy@kernel.org>
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: [PATCH v1] serial: qcom-geni: fix UART_RX_PAR_EN bit position
From: Konrad Dybcio @ 2026-04-28 9:28 UTC (permalink / raw)
To: prasanna.s, Greg Kroah-Hartman, Jiri Slaby, Sagar Dharia,
Girish Mahadevan, Karthikeyan Ramasubramanian, Doug Anderson
Cc: linux-arm-msm, linux-kernel, linux-serial, stable
In-Reply-To: <20260428-serial-bit-correct-v1-1-9131ad5b97d8@oss.qualcomm.com>
On 4/28/26 6:26 AM, Prasanna S via B4 Relay wrote:
> From: Prasanna S <prasanna.s@oss.qualcomm.com>
>
> UART_RX_PAR_EN is incorrectly defined as bit 3, which triggers false
> framing errors (S_GP_IRQ_1_EN) and causes received data to be dropped
> when parity is enabled and the parity bit is 0.
>
> Define UART_RX_PAR_EN as bit 4 of the SE_UART_RX_TRANS_CFG register, as
> specified in the reference manual.
>
> Fixes: c4f528795d1a ("tty: serial: msm_geni_serial: Add serial driver support for GENI based QUP")
> Cc: stable@vger.kernel.org
> Signed-off-by: Prasanna S <prasanna.s@oss.qualcomm.com>
> ---
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Konrad
^ permalink raw reply
* Re: [PATCH v5 2/4] serial: 8250_dw: build Renesas RZN1 CPR value from DW_UART_CPR_* definitions
From: Jia Wang @ 2026-04-28 9:07 UTC (permalink / raw)
To: Ilpo Järvinen
Cc: Andy Shevchenko, Jia Wang, Greg Kroah-Hartman, Jiri Slaby,
Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, LKML,
linux-serial, linux-riscv, devicetree
In-Reply-To: <23c80500-f2c1-0eb3-f640-00f7b108059b@linux.intel.com>
On 2026-04-28 11:41 +0300, Ilpo Järvinen wrote:
> On Tue, 28 Apr 2026, Andy Shevchenko wrote:
>
> > On Tue, Apr 28, 2026 at 01:26:27PM +0800, Jia Wang wrote:
> > > Replace the magic CPR value for Renesas RZ/N1 with a composition using
> > > DW_UART_CPR_* bit/field definitions and FIELD_PREP_CONST().
> > >
> > > Introduce a helper macro to convert a FIFO size (bytes) into the CPR
> > > FIFO_MODE field value, with BUILD_BUG_ON_ZERO() checks for alignment and
> > > bounds. Use it to replace the literal FIFO_MODE values in the RZN1.
> >
> > A couple of nit-picks below. After addressing them you can add
> >
> > Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> >
> > ...
> >
> > > #include <linux/bitfield.h>
> > > #include <linux/bits.h>
> > > +#include <linux/build_bug.h>
> > > +#include <linux/align.h>
> >
> > Preserve order, 'a' goes before 'b'.
> >
> > > #include <linux/io.h>
> > > #include <linux/types.h>
> >
> > ...
> >
> > > /* Helper for FIFO size calculation */
> > > #define DW_UART_CPR_FIFO_SIZE(a) (FIELD_GET(DW_UART_CPR_FIFO_MODE, (a)) * 16)
> >
> > > +#define DW_UART_CPR_FIFO_MODE_MAX 0x80
> >
> > You used decimal values elsewhere (id est 16), use upper limit in decimal
> > as well.
> >
> > > +#define DW_UART_CPR_FIFO_MODE_FROM_SIZE(size) \
> > > + (BUILD_BUG_ON_ZERO(!IS_ALIGNED((size), 16)) + \
> > > + BUILD_BUG_ON_ZERO(((size) / 16) > DW_UART_CPR_FIFO_MODE_MAX) + \
> > > + ((size) / 16))
> >
> > I don't see the need in having that maximum being defined separately (we don't
> > have that for 16, no need to have it for 128.
> >
> > Since some ISA:s have one assembly instruction to get both / and % divisions,
> > it's better to use that instead of IS_ALIGNED(). Can you check code generation
> > for x86_64 / x86?
>
> Do those BUILD_BUGs even generate code, especially when they are expected
> to only appear in a struct initializer?
>
> > #define DW_UART_CPR_FIFO_MODE_FROM_SIZE(size) \
> > (BUILD_BUG_ON_ZERO((size) > 2048) + BUILD_BUG_ON_ZERO((size) % 16) + ((size) / 16))
> >
> > Note, I dropped first division in order to show the upper limit in a plain
> > number since 16 is also FIFO size in bytes.
> >
> > Also note, this evaluates (size) three times, which might be problematic,
> > but I think we can leave with that for now.
>
> I'd put also FIELD_PREP_CONST() into the macro itself as I don't see much
> value for this macro outside of those .cpr_value initializations.
>
> IMO, the entire macro would be cleaner looking as a truly multi-line
> construct. Can we use static_assert()s in struct field initialization
> (I'm not sure), something along these lines:
>
> #define DW_UART_CPR_FIFO_MODE_FROM_SIZE(size) \
> ({ \
> typeof (size) __size = size; \
> \
> static_assert(IS_ALIGNED((__size), 16)); \
> static_assert(__size <= DW_UART_CPR_FIFO_MODE_MAX); \
> \
> FIELD_PREP_CONST(DW_UART_CPR_FIFO_MODE, __size / 16); \
> })
>
Thanks. I tried that approach, but the statement-expression form does
not work in this case because the helper is used in static initializers.
So I'll keep it as a plain expression macro for now, and just rework it
into a cleaner multi-line form.
> --
> i.
>
>
Best Regards,
Jia Wang
^ permalink raw reply
* [PATCH v2] serial: 8250: Clear CON_PRINTBUFFER on port re-registration
From: Fushuai Wang @ 2026-04-28 9:03 UTC (permalink / raw)
To: gregkh, jirislaby, ilpo.jarvinen, osama.abdelkader,
andy.shevchenko, kees
Cc: linux-kernel, linux-serial, wangfushuai
From: Fushuai Wang <wangfushuai@baidu.com>
When two PnP devices map to the same physical port, the serial8250 driver
removes and re-registers the console structure for the same port.
During re-registration, the console structure still has CON_PRINTBUFFER set
from the initial registration, which causes console_init_seq() to set
console->seq to syslog_seq. This results in re-printing the entire
system log buffer, which may lead to RCU stall on slow serial consoles.
Clear CON_PRINTBUFFER when re-registering a port to prevent duplicate
log printing.
Fixes: 835d844d1a28 ("8250_pnp: do pnp probe before legacy probe")
Signed-off-by: Fushuai Wang <wangfushuai@baidu.com>
---
V1->V2: Add Fixes tag
previous discussion: https://lore.kernel.org/all/20260416092917.27301-1-fushuai.wang@linux.dev/T/#u
Please ignore previous email if you received it before. There is something wrong with my email client.
drivers/tty/serial/8250/8250_core.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
index a428e88938eb..01b14392d9f7 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -694,6 +694,7 @@ int serial8250_register_8250_port(const struct uart_8250_port *up)
{
struct uart_8250_port *uart;
int ret;
+ bool was_removed = false;
if (up->port.uartclk == 0)
return -EINVAL;
@@ -716,8 +717,10 @@ int serial8250_register_8250_port(const struct uart_8250_port *up)
if (uart->port.type == PORT_8250_CIR)
return -ENODEV;
- if (uart->port.dev)
+ if (uart->port.dev) {
uart_remove_one_port(&serial8250_reg, &uart->port);
+ was_removed = true;
+ }
uart->port.ctrl_id = up->port.ctrl_id;
uart->port.port_id = up->port.port_id;
@@ -819,6 +822,10 @@ int serial8250_register_8250_port(const struct uart_8250_port *up)
&uart->capabilities);
serial8250_apply_quirks(uart);
+
+ if (was_removed && uart_console(&uart->port))
+ uart->port.cons->flags &= ~CON_PRINTBUFFER;
+
ret = uart_add_one_port(&serial8250_reg,
&uart->port);
if (ret)
--
2.36.1
^ permalink raw reply related
* [PATCH v2] serial: 8250: Clear CON_PRINTBUFFER on port re-registration
From: Fushuai Wang @ 2026-04-28 8:58 UTC (permalink / raw)
To: gregkh, jirislaby, ilpo.jarvinen, osama.abdelkader,
andy.shevchenko, kees
Cc: linux-kernel, linux-serial, wangfushuai
From: Fushuai Wang <wangfushuai@baidu.com>
When two PnP devices map to the same physical port, the serial8250 driver
removes and re-registers the console structure for the same port.
During re-registration, the console structure still has CON_PRINTBUFFER set
from the initial registration, which causes console_init_seq() to set
console->seq to syslog_seq. This results in re-printing the entire
system log buffer, which may lead to RCU stall on slow serial consoles.
Clear CON_PRINTBUFFER when re-registering a port to prevent duplicate
log printing.
Fixes:
Signed-off-by: Fushuai Wang <wangfushuai@baidu.com>
---
V1->V2: Add Fixes tag
previous discussion: https://lore.kernel.org/all/20260416092917.27301-1-fushuai.wang@linux.dev/T/#u
drivers/tty/serial/8250/8250_core.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
index a428e88938eb..01b14392d9f7 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -694,6 +694,7 @@ int serial8250_register_8250_port(const struct uart_8250_port *up)
{
struct uart_8250_port *uart;
int ret;
+ bool was_removed = false;
if (up->port.uartclk == 0)
return -EINVAL;
@@ -716,8 +717,10 @@ int serial8250_register_8250_port(const struct uart_8250_port *up)
if (uart->port.type == PORT_8250_CIR)
return -ENODEV;
- if (uart->port.dev)
+ if (uart->port.dev) {
uart_remove_one_port(&serial8250_reg, &uart->port);
+ was_removed = true;
+ }
uart->port.ctrl_id = up->port.ctrl_id;
uart->port.port_id = up->port.port_id;
@@ -819,6 +822,10 @@ int serial8250_register_8250_port(const struct uart_8250_port *up)
&uart->capabilities);
serial8250_apply_quirks(uart);
+
+ if (was_removed && uart_console(&uart->port))
+ uart->port.cons->flags &= ~CON_PRINTBUFFER;
+
ret = uart_add_one_port(&serial8250_reg,
&uart->port);
if (ret)
--
2.36.1
^ permalink raw reply related
* Re: [PATCH v5 2/4] serial: 8250_dw: build Renesas RZN1 CPR value from DW_UART_CPR_* definitions
From: Ilpo Järvinen @ 2026-04-28 8:41 UTC (permalink / raw)
To: Andy Shevchenko, Jia Wang
Cc: Greg Kroah-Hartman, Jiri Slaby, Paul Walmsley, Palmer Dabbelt,
Albert Ou, Alexandre Ghiti, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, LKML, linux-serial, linux-riscv, devicetree
In-Reply-To: <afBhkbGLsuqUitOl@ashevche-desk.local>
On Tue, 28 Apr 2026, Andy Shevchenko wrote:
> On Tue, Apr 28, 2026 at 01:26:27PM +0800, Jia Wang wrote:
> > Replace the magic CPR value for Renesas RZ/N1 with a composition using
> > DW_UART_CPR_* bit/field definitions and FIELD_PREP_CONST().
> >
> > Introduce a helper macro to convert a FIFO size (bytes) into the CPR
> > FIFO_MODE field value, with BUILD_BUG_ON_ZERO() checks for alignment and
> > bounds. Use it to replace the literal FIFO_MODE values in the RZN1.
>
> A couple of nit-picks below. After addressing them you can add
>
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
>
> ...
>
> > #include <linux/bitfield.h>
> > #include <linux/bits.h>
> > +#include <linux/build_bug.h>
> > +#include <linux/align.h>
>
> Preserve order, 'a' goes before 'b'.
>
> > #include <linux/io.h>
> > #include <linux/types.h>
>
> ...
>
> > /* Helper for FIFO size calculation */
> > #define DW_UART_CPR_FIFO_SIZE(a) (FIELD_GET(DW_UART_CPR_FIFO_MODE, (a)) * 16)
>
> > +#define DW_UART_CPR_FIFO_MODE_MAX 0x80
>
> You used decimal values elsewhere (id est 16), use upper limit in decimal
> as well.
>
> > +#define DW_UART_CPR_FIFO_MODE_FROM_SIZE(size) \
> > + (BUILD_BUG_ON_ZERO(!IS_ALIGNED((size), 16)) + \
> > + BUILD_BUG_ON_ZERO(((size) / 16) > DW_UART_CPR_FIFO_MODE_MAX) + \
> > + ((size) / 16))
>
> I don't see the need in having that maximum being defined separately (we don't
> have that for 16, no need to have it for 128.
>
> Since some ISA:s have one assembly instruction to get both / and % divisions,
> it's better to use that instead of IS_ALIGNED(). Can you check code generation
> for x86_64 / x86?
Do those BUILD_BUGs even generate code, especially when they are expected
to only appear in a struct initializer?
> #define DW_UART_CPR_FIFO_MODE_FROM_SIZE(size) \
> (BUILD_BUG_ON_ZERO((size) > 2048) + BUILD_BUG_ON_ZERO((size) % 16) + ((size) / 16))
>
> Note, I dropped first division in order to show the upper limit in a plain
> number since 16 is also FIFO size in bytes.
>
> Also note, this evaluates (size) three times, which might be problematic,
> but I think we can leave with that for now.
I'd put also FIELD_PREP_CONST() into the macro itself as I don't see much
value for this macro outside of those .cpr_value initializations.
IMO, the entire macro would be cleaner looking as a truly multi-line
construct. Can we use static_assert()s in struct field initialization
(I'm not sure), something along these lines:
#define DW_UART_CPR_FIFO_MODE_FROM_SIZE(size) \
({ \
typeof (size) __size = size; \
\
static_assert(IS_ALIGNED((__size), 16)); \
static_assert(__size <= DW_UART_CPR_FIFO_MODE_MAX); \
\
FIELD_PREP_CONST(DW_UART_CPR_FIFO_MODE, __size / 16); \
})
--
i.
^ permalink raw reply
* Re: [PATCH v5 2/4] serial: 8250_dw: build Renesas RZN1 CPR value from DW_UART_CPR_* definitions
From: Jia Wang @ 2026-04-28 8:36 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Jia Wang, Ilpo Järvinen, Greg Kroah-Hartman, Jiri Slaby,
Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, linux-kernel,
linux-serial, linux-riscv, devicetree
In-Reply-To: <afBhkbGLsuqUitOl@ashevche-desk.local>
On 2026-04-28 10:28 +0300, Andy Shevchenko wrote:
> On Tue, Apr 28, 2026 at 01:26:27PM +0800, Jia Wang wrote:
> > Replace the magic CPR value for Renesas RZ/N1 with a composition using
> > DW_UART_CPR_* bit/field definitions and FIELD_PREP_CONST().
> >
> > Introduce a helper macro to convert a FIFO size (bytes) into the CPR
> > FIFO_MODE field value, with BUILD_BUG_ON_ZERO() checks for alignment and
> > bounds. Use it to replace the literal FIFO_MODE values in the RZN1.
>
> A couple of nit-picks below. After addressing them you can add
>
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
>
Thanks for the review. I'll address them and add your Reviewed-by tag
in v6.
> ...
>
> > #include <linux/bitfield.h>
> > #include <linux/bits.h>
> > +#include <linux/build_bug.h>
> > +#include <linux/align.h>
>
> Preserve order, 'a' goes before 'b'.
>
Will fix in v6.
> > #include <linux/io.h>
> > #include <linux/types.h>
>
> ...
>
> > /* Helper for FIFO size calculation */
> > #define DW_UART_CPR_FIFO_SIZE(a) (FIELD_GET(DW_UART_CPR_FIFO_MODE, (a)) * 16)
>
> > +#define DW_UART_CPR_FIFO_MODE_MAX 0x80
>
> You used decimal values elsewhere (id est 16), use upper limit in decimal
> as well.
>
This define will be removed in v6.
> > +#define DW_UART_CPR_FIFO_MODE_FROM_SIZE(size) \
> > + (BUILD_BUG_ON_ZERO(!IS_ALIGNED((size), 16)) + \
> > + BUILD_BUG_ON_ZERO(((size) / 16) > DW_UART_CPR_FIFO_MODE_MAX) + \
> > + ((size) / 16))
>
> I don't see the need in having that maximum being defined separately (we don't
> have that for 16, no need to have it for 128.
>
> Since some ISA:s have one assembly instruction to get both / and % divisions,
> it's better to use that instead of IS_ALIGNED(). Can you check code generation
> for x86_64 / x86?
>
> #define DW_UART_CPR_FIFO_MODE_FROM_SIZE(size) \
> (BUILD_BUG_ON_ZERO((size) > 2048) + BUILD_BUG_ON_ZERO((size) % 16) + ((size) / 16))
>
> Note, I dropped first division in order to show the upper limit in a plain
> number since 16 is also FIFO size in bytes.
>
> Also note, this evaluates (size) three times, which might be problematic,
> but I think we can leave with that for now.
>
Makes sense. I'll fold the upper limit into the macro and switch to
(size) > 2048 and (size) % 16 checks for v6.
In the driver, the macro is used only in static initializers, so the
constants are fully resolved at compile time.
I also checked code generation on x86_64/x86: both IS_ALIGNED() and % 16
produce identical instructions at -O2. I'll still switch to % 16 as you
suggest for clarity.
>
> --
> With Best Regards,
> Andy Shevchenko
>
>
>
Best Regards,
Jia Wang
^ permalink raw reply
* [PATCH] serial: 8250_pci: skip WCH PCI serial devices
From: Jiawei liu @ 2026-04-28 8:26 UTC (permalink / raw)
To: linux-serial; +Cc: gregkh, jirislaby, linux-kernel
[-- Attachment #1.1: Type: text/plain, Size: 344 bytes --]
WCH PCI serial devices implement vendor-specific extensions
beyond the standard 16C550 UART. These are not supported by
the generic 8250 PCI driver, and binding them may lead to
incorrect or suboptimal operation.
Skip these devices during probe to avoid misbinding.
Signed-off-by: Jiawei Liu <liujiawei4419@gmail.com>
[-- Attachment #1.2: Type: text/html, Size: 462 bytes --]
[-- Attachment #2: 0001-serial-8250-pci-skip-WCH-devices.patch --]
[-- Type: application/octet-stream, Size: 7464 bytes --]
From 5e4d65278bcd30f9f877fe7b7ffeaaaf9c840983 Mon Sep 17 00:00:00 2001
From: Jiawei Liu <liujiawei4419@gmail.com>
Date: Tue, 28 Apr 2026 15:12:27 +0800
Subject: [PATCH 1/2] serial: 8250: pci: skip WCH devices
WCH PCI serial devices implement vendor-specific extensions
beyond the standard 16C550 UART. These are not supported by
the generic 8250 PCI driver, and binding them may lead to
incorrect or suboptimal operation.
Skip these devices during probe to avoid misbinding.
Signed-off-by: Jiawei Liu <liujiawei4419@gmail.com>
---
drivers/tty/serial/8250/8250_pci.c | 214 +----------------------------
1 file changed, 2 insertions(+), 212 deletions(-)
diff --git a/drivers/tty/serial/8250/8250_pci.c b/drivers/tty/serial/8250/8250_pci.c
index 2fbd8f2603b5f..2d6de1c33daa2 100644
--- a/drivers/tty/serial/8250/8250_pci.c
+++ b/drivers/tty/serial/8250/8250_pci.c
@@ -1900,85 +1900,6 @@ static int pci_eg20t_init(struct pci_dev *dev)
#endif
}
-static int
-pci_wch_ch353_setup(struct serial_private *priv,
- const struct pciserial_board *board,
- struct uart_8250_port *port, int idx)
-{
- if (!IS_ENABLED(CONFIG_HAS_IOPORT))
- return serial_8250_warn_need_ioport(priv->dev);
-
- port->port.flags |= UPF_FIXED_TYPE;
- port->port.type = PORT_16550A;
- return pci_default_setup(priv, board, port, idx);
-}
-
-static int
-pci_wch_ch355_setup(struct serial_private *priv,
- const struct pciserial_board *board,
- struct uart_8250_port *port, int idx)
-{
- if (!IS_ENABLED(CONFIG_HAS_IOPORT))
- return serial_8250_warn_need_ioport(priv->dev);
-
- port->port.flags |= UPF_FIXED_TYPE;
- port->port.type = PORT_16550A;
- return pci_default_setup(priv, board, port, idx);
-}
-
-static int
-pci_wch_ch38x_setup(struct serial_private *priv,
- const struct pciserial_board *board,
- struct uart_8250_port *port, int idx)
-{
- if (!IS_ENABLED(CONFIG_HAS_IOPORT))
- return serial_8250_warn_need_ioport(priv->dev);
-
- port->port.flags |= UPF_FIXED_TYPE;
- port->port.type = PORT_16850;
- return pci_default_setup(priv, board, port, idx);
-}
-
-
-#define CH384_XINT_ENABLE_REG 0xEB
-#define CH384_XINT_ENABLE_BIT 0x02
-
-static int pci_wch_ch38x_init(struct pci_dev *dev)
-{
- int max_port;
- unsigned long iobase;
-
- if (!IS_ENABLED(CONFIG_HAS_IOPORT))
- return serial_8250_warn_need_ioport(dev);
-
- switch (dev->device) {
- case 0x3853: /* 8 ports */
- max_port = 8;
- break;
- default:
- return -EINVAL;
- }
-
- iobase = pci_resource_start(dev, 0);
- outb(CH384_XINT_ENABLE_BIT, iobase + CH384_XINT_ENABLE_REG);
-
- return max_port;
-}
-
-static void pci_wch_ch38x_exit(struct pci_dev *dev)
-{
- unsigned long iobase;
-
- if (!IS_ENABLED(CONFIG_HAS_IOPORT)) {
- serial_8250_warn_need_ioport(dev);
- return;
- }
-
- iobase = pci_resource_start(dev, 0);
- outb(0x0, iobase + CH384_XINT_ENABLE_REG);
-}
-
-
static int
pci_sunix_setup(struct serial_private *priv,
const struct pciserial_board *board,
@@ -2881,88 +2802,6 @@ static struct pci_serial_quirk pci_serial_quirks[] = {
.subdevice = PCI_ANY_ID,
.setup = pci_omegapci_setup,
},
- /* WCH CH353 1S1P card (16550 clone) */
- {
- .vendor = PCI_VENDOR_ID_WCHCN,
- .device = PCI_DEVICE_ID_WCHCN_CH353_1S1P,
- .subvendor = PCI_ANY_ID,
- .subdevice = PCI_ANY_ID,
- .setup = pci_wch_ch353_setup,
- },
- /* WCH CH353 2S1P card (16550 clone) */
- {
- .vendor = PCI_VENDOR_ID_WCHCN,
- .device = PCI_DEVICE_ID_WCHCN_CH353_2S1P,
- .subvendor = PCI_ANY_ID,
- .subdevice = PCI_ANY_ID,
- .setup = pci_wch_ch353_setup,
- },
- /* WCH CH353 4S card (16550 clone) */
- {
- .vendor = PCI_VENDOR_ID_WCHCN,
- .device = PCI_DEVICE_ID_WCHCN_CH353_4S,
- .subvendor = PCI_ANY_ID,
- .subdevice = PCI_ANY_ID,
- .setup = pci_wch_ch353_setup,
- },
- /* WCH CH353 2S1PF card (16550 clone) */
- {
- .vendor = PCI_VENDOR_ID_WCHCN,
- .device = PCI_DEVICE_ID_WCHCN_CH353_2S1PF,
- .subvendor = PCI_ANY_ID,
- .subdevice = PCI_ANY_ID,
- .setup = pci_wch_ch353_setup,
- },
- /* WCH CH352 2S card (16550 clone) */
- {
- .vendor = PCI_VENDOR_ID_WCHCN,
- .device = PCI_DEVICE_ID_WCHCN_CH352_2S,
- .subvendor = PCI_ANY_ID,
- .subdevice = PCI_ANY_ID,
- .setup = pci_wch_ch353_setup,
- },
- /* WCH CH355 4S card (16550 clone) */
- {
- .vendor = PCI_VENDOR_ID_WCHCN,
- .device = PCI_DEVICE_ID_WCHCN_CH355_4S,
- .subvendor = PCI_ANY_ID,
- .subdevice = PCI_ANY_ID,
- .setup = pci_wch_ch355_setup,
- },
- /* WCH CH382 2S card (16850 clone) */
- {
- .vendor = PCI_VENDOR_ID_WCHIC,
- .device = PCI_DEVICE_ID_WCHIC_CH382_2S,
- .subvendor = PCI_ANY_ID,
- .subdevice = PCI_ANY_ID,
- .setup = pci_wch_ch38x_setup,
- },
- /* WCH CH382 2S1P card (16850 clone) */
- {
- .vendor = PCI_VENDOR_ID_WCHIC,
- .device = PCI_DEVICE_ID_WCHIC_CH382_2S1P,
- .subvendor = PCI_ANY_ID,
- .subdevice = PCI_ANY_ID,
- .setup = pci_wch_ch38x_setup,
- },
- /* WCH CH384 4S card (16850 clone) */
- {
- .vendor = PCI_VENDOR_ID_WCHIC,
- .device = PCI_DEVICE_ID_WCHIC_CH384_4S,
- .subvendor = PCI_ANY_ID,
- .subdevice = PCI_ANY_ID,
- .setup = pci_wch_ch38x_setup,
- },
- /* WCH CH384 8S card (16850 clone) */
- {
- .vendor = PCI_VENDOR_ID_WCHIC,
- .device = PCI_DEVICE_ID_WCHIC_CH384_8S,
- .subvendor = PCI_ANY_ID,
- .subdevice = PCI_ANY_ID,
- .init = pci_wch_ch38x_init,
- .exit = pci_wch_ch38x_exit,
- .setup = pci_wch_ch38x_setup,
- },
/*
* Broadcom TruManage (NetXtreme)
*/
@@ -3214,9 +3053,7 @@ enum pci_board_num_t {
pbn_fintek_F81504A,
pbn_fintek_F81508A,
pbn_fintek_F81512A,
- pbn_wch382_2,
- pbn_wch384_4,
- pbn_wch384_8,
+
pbn_sunix_pci_1s,
pbn_sunix_pci_2s,
pbn_sunix_pci_4s,
@@ -3932,27 +3769,7 @@ static struct pciserial_board pci_boards[] = {
.uart_offset = 8,
.base_baud = 115200,
},
- [pbn_wch382_2] = {
- .flags = FL_BASE0,
- .num_ports = 2,
- .base_baud = 115200,
- .uart_offset = 8,
- .first_offset = 0xC0,
- },
- [pbn_wch384_4] = {
- .flags = FL_BASE0,
- .num_ports = 4,
- .base_baud = 115200,
- .uart_offset = 8,
- .first_offset = 0xC0,
- },
- [pbn_wch384_8] = {
- .flags = FL_BASE0,
- .num_ports = 8,
- .base_baud = 115200,
- .uart_offset = 8,
- .first_offset = 0x00,
- },
+
[pbn_sunix_pci_1s] = {
.num_ports = 1,
.base_baud = 921600,
@@ -6179,33 +5996,6 @@ static const struct pci_device_id serial_pci_tbl[] = {
PCI_ANY_ID, PCI_ANY_ID,
0, 0, pbn_b0_bt_2_115200 },
- /*
- * WCH CH353 series devices: The 2S1P is handled by parport_serial
- * so not listed here.
- */
- { PCI_VENDOR_ID_WCHCN, PCI_DEVICE_ID_WCHCN_CH353_4S,
- PCI_ANY_ID, PCI_ANY_ID,
- 0, 0, pbn_b0_bt_4_115200 },
-
- { PCI_VENDOR_ID_WCHCN, PCI_DEVICE_ID_WCHCN_CH353_2S1PF,
- PCI_ANY_ID, PCI_ANY_ID,
- 0, 0, pbn_b0_bt_2_115200 },
-
- { PCI_VENDOR_ID_WCHCN, PCI_DEVICE_ID_WCHCN_CH355_4S,
- PCI_ANY_ID, PCI_ANY_ID,
- 0, 0, pbn_b0_bt_4_115200 },
-
- { PCI_VENDOR_ID_WCHIC, PCI_DEVICE_ID_WCHIC_CH382_2S,
- PCI_ANY_ID, PCI_ANY_ID,
- 0, 0, pbn_wch382_2 },
-
- { PCI_VENDOR_ID_WCHIC, PCI_DEVICE_ID_WCHIC_CH384_4S,
- PCI_ANY_ID, PCI_ANY_ID,
- 0, 0, pbn_wch384_4 },
-
- { PCI_VENDOR_ID_WCHIC, PCI_DEVICE_ID_WCHIC_CH384_8S,
- PCI_ANY_ID, PCI_ANY_ID,
- 0, 0, pbn_wch384_8 },
/*
* Realtek RealManage
*/
--
2.34.1
^ permalink raw reply related
* Re: [PATCH v5 4/4] serial: 8250_dw: Use a fixed CPR value for UltraRISC DP1000 UART
From: Andy Shevchenko @ 2026-04-28 8:19 UTC (permalink / raw)
To: Jia Wang
Cc: Ilpo Järvinen, Greg Kroah-Hartman, Jiri Slaby, Paul Walmsley,
Palmer Dabbelt, Albert Ou, Alexandre Ghiti, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, linux-kernel, linux-serial,
linux-riscv, devicetree
In-Reply-To: <20260428-ultrarisc-serial-v5-4-97de63b1e3eb@ultrarisc.com>
On Tue, Apr 28, 2026 at 01:26:29PM +0800, Jia Wang wrote:
> The UltraRISC DP1000 UART does not provide the standard CPR register used
> by 8250_dw to discover port capabilities.
>
> Provide a fixed CPR value for the DP1000-specific compatible so the
> driver can configure the port correctly.
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH] serial: max3100: unwind port allocation on add failure
From: Andy Shevchenko @ 2026-04-28 7:33 UTC (permalink / raw)
To: 박명훈
Cc: Greg Kroah-Hartman, Jiri Slaby, linux-kernel, linux-serial,
Ijae Kim
In-Reply-To: <20260428070742.40072-1-pakmyeonghun@bagmyeonghun-ui-MacBookPro.local>
On Tue, Apr 28, 2026 at 04:07:27PM +0900, 박명훈 wrote:
> max3100_probe() reports errors from uart_add_one_port(), but then
> continues initialization and returns success. The device is left with
> per-port state in max3100s[] even though serial core did not add the
> port.
>
> Return the uart_add_one_port() error instead. Free the per-port state
> and, when no other chips remain, unregister the UART driver that probe
> registered.
NAK.
Please, try to understand why we do like this and how important is to have at
least some of the ports available (yes, I understand that this is not a robust
solution, but it might help in the unlikely cases when some of the ports are
failed to get instantiated.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH v5 2/4] serial: 8250_dw: build Renesas RZN1 CPR value from DW_UART_CPR_* definitions
From: Andy Shevchenko @ 2026-04-28 7:28 UTC (permalink / raw)
To: Jia Wang
Cc: Ilpo Järvinen, Greg Kroah-Hartman, Jiri Slaby, Paul Walmsley,
Palmer Dabbelt, Albert Ou, Alexandre Ghiti, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, linux-kernel, linux-serial,
linux-riscv, devicetree
In-Reply-To: <20260428-ultrarisc-serial-v5-2-97de63b1e3eb@ultrarisc.com>
On Tue, Apr 28, 2026 at 01:26:27PM +0800, Jia Wang wrote:
> Replace the magic CPR value for Renesas RZ/N1 with a composition using
> DW_UART_CPR_* bit/field definitions and FIELD_PREP_CONST().
>
> Introduce a helper macro to convert a FIFO size (bytes) into the CPR
> FIFO_MODE field value, with BUILD_BUG_ON_ZERO() checks for alignment and
> bounds. Use it to replace the literal FIFO_MODE values in the RZN1.
A couple of nit-picks below. After addressing them you can add
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
...
> #include <linux/bitfield.h>
> #include <linux/bits.h>
> +#include <linux/build_bug.h>
> +#include <linux/align.h>
Preserve order, 'a' goes before 'b'.
> #include <linux/io.h>
> #include <linux/types.h>
...
> /* Helper for FIFO size calculation */
> #define DW_UART_CPR_FIFO_SIZE(a) (FIELD_GET(DW_UART_CPR_FIFO_MODE, (a)) * 16)
> +#define DW_UART_CPR_FIFO_MODE_MAX 0x80
You used decimal values elsewhere (id est 16), use upper limit in decimal
as well.
> +#define DW_UART_CPR_FIFO_MODE_FROM_SIZE(size) \
> + (BUILD_BUG_ON_ZERO(!IS_ALIGNED((size), 16)) + \
> + BUILD_BUG_ON_ZERO(((size) / 16) > DW_UART_CPR_FIFO_MODE_MAX) + \
> + ((size) / 16))
I don't see the need in having that maximum being defined separately (we don't
have that for 16, no need to have it for 128.
Since some ISA:s have one assembly instruction to get both / and % divisions,
it's better to use that instead of IS_ALIGNED(). Can you check code generation
for x86_64 / x86?
#define DW_UART_CPR_FIFO_MODE_FROM_SIZE(size) \
(BUILD_BUG_ON_ZERO((size) > 2048) + BUILD_BUG_ON_ZERO((size) % 16) + ((size) / 16))
Note, I dropped first division in order to show the upper limit in a plain
number since 16 is also FIFO size in bytes.
Also note, this evaluates (size) three times, which might be problematic,
but I think we can leave with that for now.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH v5 1/4] serial: 8250_dwlib: move DesignWare register definitions to header
From: Andy Shevchenko @ 2026-04-28 7:15 UTC (permalink / raw)
To: Jia Wang
Cc: Ilpo Järvinen, Greg Kroah-Hartman, Jiri Slaby, Paul Walmsley,
Palmer Dabbelt, Albert Ou, Alexandre Ghiti, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, linux-kernel, linux-serial,
linux-riscv, devicetree
In-Reply-To: <20260428-ultrarisc-serial-v5-1-97de63b1e3eb@ultrarisc.com>
On Tue, Apr 28, 2026 at 01:26:26PM +0800, Jia Wang wrote:
> Move the DW_UART_* register offsets and CPR bit/field definitions from
> 8250_dwlib.c into 8250_dwlib.h so they can be shared by 8250_dw and
> 8250_dwlib users.
>
> Add an include guard for 8250_dwlib.h.
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* [PATCH] serial: max3100: unwind port allocation on add failure
From: 박명훈 @ 2026-04-28 7:07 UTC (permalink / raw)
To: Greg Kroah-Hartman, Jiri Slaby
Cc: linux-kernel, linux-serial, Ijae Kim, Myeonghun Pak
From: Myeonghun Pak <mhun512@gmail.com>
max3100_probe() reports errors from uart_add_one_port(), but then
continues initialization and returns success. The device is left with
per-port state in max3100s[] even though serial core did not add the
port.
Return the uart_add_one_port() error instead. Free the per-port state
and, when no other chips remain, unregister the UART driver that probe
registered.
Fixes: 7831d56b0a35 ("tty: MAX3100")
Co-developed-by: Ijae Kim <ae878000@gmail.com>
Signed-off-by: Ijae Kim <ae878000@gmail.com>
Signed-off-by: Myeonghun Pak <mhun512@gmail.com>
---
drivers/tty/serial/max3100.c | 20 +++++++++++++++++++-
1 file changed, 19 insertions(+), 1 deletion(-)
diff --git a/drivers/tty/serial/max3100.c b/drivers/tty/serial/max3100.c
index 475b0a6efc..83a9db1819 100644
--- a/drivers/tty/serial/max3100.c
+++ b/drivers/tty/serial/max3100.c
@@ -732,13 +732,31 @@ static int max3100_probe(struct spi_device *spi)
device_property_read_u32(dev, "clock-frequency", &max3100s[i]->port.uartclk);
retval = uart_add_one_port(&max3100_uart_driver, &max3100s[i]->port);
- if (retval < 0)
+ if (retval < 0) {
dev_err_probe(dev, retval, "uart_add_one_port failed for line %d\n", i);
+ goto err_free_port;
+ }
/* set shutdown mode to save power. Will be woken-up on open */
max3100_sr(max3100s[i], MAX3100_WC | MAX3100_SHDN, &rx);
mutex_unlock(&max3100s_lock);
return 0;
+
+err_free_port:
+ kfree(max3100s[i]);
+ max3100s[i] = NULL;
+
+ for (i = 0; i < MAX_MAX3100; i++)
+ if (max3100s[i])
+ break;
+
+ if (i == MAX_MAX3100) {
+ uart_unregister_driver(&max3100_uart_driver);
+ uart_driver_registered = 0;
+ }
+
+ mutex_unlock(&max3100s_lock);
+ return retval;
}
static void max3100_remove(struct spi_device *spi)
--
2.50.1
^ permalink raw reply related
* [PATCH] serial: altera_jtaguart: handle uart_add_one_port() failures
From: Myeonghun Pak @ 2026-04-28 6:44 UTC (permalink / raw)
To: Tobias Klauser, Greg Kroah-Hartman, Jiri Slaby
Cc: linux-serial, linux-kernel, Myeonghun Pak, Ijae Kim
altera_jtaguart_probe() maps the register window before registering the
UART port, but it ignores failures from uart_add_one_port(). If port
registration fails, probe still returns success and the mapping remains
live until a later remove path that is not part of probe failure cleanup.
Return the uart_add_one_port() error and unmap the register window on
that failure path.
Fixes: 5bcd601049c6 ("serial: Add driver for the Altera JTAG UART")
Co-developed-by: Ijae Kim <ae878000@gmail.com>
Signed-off-by: Ijae Kim <ae878000@gmail.com>
Signed-off-by: Myeonghun Pak <mhun512@gmail.com>
---
drivers/tty/serial/altera_jtaguart.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/tty/serial/altera_jtaguart.c b/drivers/tty/serial/altera_jtaguart.c
index d47a62d1c9..15588b6dc3 100644
--- a/drivers/tty/serial/altera_jtaguart.c
+++ b/drivers/tty/serial/altera_jtaguart.c
@@ -379,6 +379,7 @@ static int altera_jtaguart_probe(struct platform_device *pdev)
struct resource *res_mem;
int i = pdev->id;
int irq;
+ int ret;
/* -1 emphasizes that the platform must have one port, no .N suffix */
if (i == -1)
@@ -418,7 +419,12 @@ static int altera_jtaguart_probe(struct platform_device *pdev)
port->flags = UPF_BOOT_AUTOCONF;
port->dev = &pdev->dev;
- uart_add_one_port(&altera_jtaguart_driver, port);
+ ret = uart_add_one_port(&altera_jtaguart_driver, port);
+ if (ret) {
+ iounmap(port->membase);
+ port->membase = NULL;
+ return ret;
+ }
return 0;
}
--
2.51.0
^ permalink raw reply related
* [PATCH v5 2/4] serial: 8250_dw: build Renesas RZN1 CPR value from DW_UART_CPR_* definitions
From: Jia Wang @ 2026-04-28 5:26 UTC (permalink / raw)
To: Ilpo Järvinen, Andy Shevchenko, Greg Kroah-Hartman,
Jiri Slaby, Paul Walmsley, Palmer Dabbelt, Albert Ou,
Alexandre Ghiti, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: linux-kernel, linux-serial, linux-riscv, devicetree, Jia Wang
In-Reply-To: <20260428-ultrarisc-serial-v5-0-97de63b1e3eb@ultrarisc.com>
Replace the magic CPR value for Renesas RZ/N1 with a composition using
DW_UART_CPR_* bit/field definitions and FIELD_PREP_CONST().
Introduce a helper macro to convert a FIFO size (bytes) into the CPR
FIFO_MODE field value, with BUILD_BUG_ON_ZERO() checks for alignment and
bounds. Use it to replace the literal FIFO_MODE values in the RZN1.
Signed-off-by: Jia Wang <wangjia@ultrarisc.com>
---
drivers/tty/serial/8250/8250_dw.c | 11 ++++++++++-
drivers/tty/serial/8250/8250_dwlib.h | 7 +++++++
2 files changed, 17 insertions(+), 1 deletion(-)
diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c
index 467755bf0092..5cf3bb74b285 100644
--- a/drivers/tty/serial/8250/8250_dw.c
+++ b/drivers/tty/serial/8250/8250_dw.c
@@ -937,7 +937,16 @@ static const struct dw8250_platform_data dw8250_armada_38x_data = {
static const struct dw8250_platform_data dw8250_renesas_rzn1_data = {
.usr_reg = DW_UART_USR,
- .cpr_value = 0x00012f32,
+ .cpr_value = FIELD_PREP_CONST(DW_UART_CPR_ABP_DATA_WIDTH, 2) |
+ DW_UART_CPR_AFCE_MODE |
+ DW_UART_CPR_THRE_MODE |
+ DW_UART_CPR_ADDITIONAL_FEATURES |
+ DW_UART_CPR_FIFO_ACCESS |
+ DW_UART_CPR_FIFO_STAT |
+ DW_UART_CPR_SHADOW |
+ DW_UART_CPR_DMA_EXTRA |
+ FIELD_PREP_CONST(DW_UART_CPR_FIFO_MODE,
+ DW_UART_CPR_FIFO_MODE_FROM_SIZE(16)),
.quirks = DW_UART_QUIRK_CPR_VALUE | DW_UART_QUIRK_IS_DMA_FC,
};
diff --git a/drivers/tty/serial/8250/8250_dwlib.h b/drivers/tty/serial/8250/8250_dwlib.h
index 2f26f9ecacbe..c1f87cd42ecc 100644
--- a/drivers/tty/serial/8250/8250_dwlib.h
+++ b/drivers/tty/serial/8250/8250_dwlib.h
@@ -6,6 +6,8 @@
#include <linux/bitfield.h>
#include <linux/bits.h>
+#include <linux/build_bug.h>
+#include <linux/align.h>
#include <linux/io.h>
#include <linux/types.h>
@@ -70,6 +72,11 @@
/* Helper for FIFO size calculation */
#define DW_UART_CPR_FIFO_SIZE(a) (FIELD_GET(DW_UART_CPR_FIFO_MODE, (a)) * 16)
+#define DW_UART_CPR_FIFO_MODE_MAX 0x80
+#define DW_UART_CPR_FIFO_MODE_FROM_SIZE(size) \
+ (BUILD_BUG_ON_ZERO(!IS_ALIGNED((size), 16)) + \
+ BUILD_BUG_ON_ZERO(((size) / 16) > DW_UART_CPR_FIFO_MODE_MAX) + \
+ ((size) / 16))
struct dw8250_port_data {
/* Port properties */
--
2.34.1
^ permalink raw reply related
* [PATCH v5 0/4] serial: 8250_dw: Add support for UltraRISC DP1000 UART
From: Jia Wang @ 2026-04-28 5:26 UTC (permalink / raw)
To: Ilpo Järvinen, Andy Shevchenko, Greg Kroah-Hartman,
Jiri Slaby, Paul Walmsley, Palmer Dabbelt, Albert Ou,
Alexandre Ghiti, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: linux-kernel, linux-serial, linux-riscv, devicetree, Jia Wang,
Conor Dooley
This patch series adds support for the UltraRISC DP1000 UART controller.
The series includes four patches. The first two are preparatory cleanups;
the last two add the DP1000 compatible and fixed CPR handling.
The patches have been tested on an UltraRISC DP1000 development board with
Linux v7.1-rc1, verifying basic UART functionality.
Signed-off-by: Jia Wang <wangjia@ultrarisc.com>
---
Changes in v5:
- Rebased onto Linux v7.1-rc1.
- Patch 1:
* Reorder and document the moved DesignWare register/bit definitions.
- Patch 2:
* Add a FIFO size -> CPR FIFO_MODE helper and use it for RZ/N1.
- Patch 4:
* Use the FIFO_MODE helper for DP1000.
- Link to v4: https://patch.msgid.link/20260424-ultrarisc-serial-v4-0-1765a0b4c4a0@ultrarisc.com
Changes in v4:
- Added two preparatory patches before the original series, shifting patch
numbers (former 1/2 -> now 3/4).
- Patch 1:
* Move all DesignWare UART register/field definitions into 8250_dwlib.h
for shared use with 8250_dw.
- Patch 2:
* Converted the Renesas RZ/N1 CPR magic value to use DW_UART_CPR_* macros
and FIELD_PREP_CONST().
- Patch 4:
* Converted the UltraRISC DP1000 CPR magic value to use
DW_UART_CPR_* macros and FIELD_PREP_CONST() (value unchanged).
- Link to v3: https://patch.msgid.link/20260421-ultrarisc-serial-v3-0-3d7f09c2420e@ultrarisc.com
Changes in v3:
- Rebased on Linux v7.0-rc7.
- Patch 1:
* Removed separate `items` entry for DP1000, merging it into the
existing `enum` to comply with the schema.
* Updated commit message to describe DP1000 UART hardware differences.
- Patch 2:
* Drop the custom quirk for missing CPR register.
* Switch to using DW_UART_QUIRK_CPR_VALUE to provide a fixed CPR value.
- Link to v2: https://patch.msgid.link/20260316-ultrarisc-serial-v2-0-6ab3e7fa891c@ultrarisc.com
Changes in v2:
- Rebased on Linux v7.0-rc4 (previously on v7.0-rc2).
- Reordered patch series: DT binding patch comes before driver changes.
- Updated commit message for DT binding patch.
- Link to v1: https://patch.msgid.link/20260316-ultrarisc-serial-v1-0-c464f3e933a5@ultrarisc.com
---
Jia Wang (4):
serial: 8250_dwlib: move DesignWare register definitions to header
serial: 8250_dw: build Renesas RZN1 CPR value from DW_UART_CPR_* definitions
dt-bindings: serial: snps-dw-apb-uart: Add UltraRISC DP1000 UART
serial: 8250_dw: Use a fixed CPR value for UltraRISC DP1000 UART
.../bindings/serial/snps-dw-apb-uart.yaml | 1 +
drivers/tty/serial/8250/8250_dw.c | 33 ++++++----
drivers/tty/serial/8250/8250_dwlib.c | 49 --------------
drivers/tty/serial/8250/8250_dwlib.h | 74 ++++++++++++++++++++++
4 files changed, 96 insertions(+), 61 deletions(-)
---
base-commit: 3b3bea6d4b9c162f9e555905d96b8c1da67ecd5b
change-id: 20260309-ultrarisc-serial-64ff637edf26
Best regards,
--
Jia Wang <wangjia@ultrarisc.com>
^ permalink raw reply
* [PATCH v5 4/4] serial: 8250_dw: Use a fixed CPR value for UltraRISC DP1000 UART
From: Jia Wang @ 2026-04-28 5:26 UTC (permalink / raw)
To: Ilpo Järvinen, Andy Shevchenko, Greg Kroah-Hartman,
Jiri Slaby, Paul Walmsley, Palmer Dabbelt, Albert Ou,
Alexandre Ghiti, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: linux-kernel, linux-serial, linux-riscv, devicetree, Jia Wang
In-Reply-To: <20260428-ultrarisc-serial-v5-0-97de63b1e3eb@ultrarisc.com>
The UltraRISC DP1000 UART does not provide the standard CPR register used
by 8250_dw to discover port capabilities.
Provide a fixed CPR value for the DP1000-specific compatible so the
driver can configure the port correctly.
Signed-off-by: Jia Wang <wangjia@ultrarisc.com>
---
drivers/tty/serial/8250/8250_dw.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c
index 5cf3bb74b285..6fa7c8440919 100644
--- a/drivers/tty/serial/8250/8250_dw.c
+++ b/drivers/tty/serial/8250/8250_dw.c
@@ -960,6 +960,16 @@ static const struct dw8250_platform_data dw8250_intc10ee = {
.quirks = DW_UART_QUIRK_IER_KICK,
};
+static const struct dw8250_platform_data dw8250_ultrarisc_dp1000_data = {
+ .usr_reg = DW_UART_USR,
+ .cpr_value = FIELD_PREP_CONST(DW_UART_CPR_ABP_DATA_WIDTH, 2) |
+ DW_UART_CPR_THRE_MODE |
+ DW_UART_CPR_DMA_EXTRA |
+ FIELD_PREP_CONST(DW_UART_CPR_FIFO_MODE,
+ DW_UART_CPR_FIFO_MODE_FROM_SIZE(32)),
+ .quirks = DW_UART_QUIRK_CPR_VALUE,
+};
+
static const struct of_device_id dw8250_of_match[] = {
{ .compatible = "snps,dw-apb-uart", .data = &dw8250_dw_apb },
{ .compatible = "cavium,octeon-3860-uart", .data = &dw8250_octeon_3860_data },
@@ -967,6 +977,7 @@ static const struct of_device_id dw8250_of_match[] = {
{ .compatible = "renesas,rzn1-uart", .data = &dw8250_renesas_rzn1_data },
{ .compatible = "sophgo,sg2044-uart", .data = &dw8250_skip_set_rate_data },
{ .compatible = "starfive,jh7100-uart", .data = &dw8250_skip_set_rate_data },
+ { .compatible = "ultrarisc,dp1000-uart", .data = &dw8250_ultrarisc_dp1000_data },
{ /* Sentinel */ }
};
MODULE_DEVICE_TABLE(of, dw8250_of_match);
--
2.34.1
^ permalink raw reply related
* [PATCH v5 3/4] dt-bindings: serial: snps-dw-apb-uart: Add UltraRISC DP1000 UART
From: Jia Wang @ 2026-04-28 5:26 UTC (permalink / raw)
To: Ilpo Järvinen, Andy Shevchenko, Greg Kroah-Hartman,
Jiri Slaby, Paul Walmsley, Palmer Dabbelt, Albert Ou,
Alexandre Ghiti, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: linux-kernel, linux-serial, linux-riscv, devicetree, Jia Wang,
Conor Dooley
In-Reply-To: <20260428-ultrarisc-serial-v5-0-97de63b1e3eb@ultrarisc.com>
UltraRISC DP1000 integrates a Synopsys DesignWare APB UART, but it does
not provide the standard CPR and UCV registers.
Signed-off-by: Jia Wang <wangjia@ultrarisc.com>
Acked-by: Conor Dooley <conor.dooley@microchip.com>
---
Documentation/devicetree/bindings/serial/snps-dw-apb-uart.yaml | 1 +
1 file changed, 1 insertion(+)
diff --git a/Documentation/devicetree/bindings/serial/snps-dw-apb-uart.yaml b/Documentation/devicetree/bindings/serial/snps-dw-apb-uart.yaml
index 685c1eceb782..49f51b002879 100644
--- a/Documentation/devicetree/bindings/serial/snps-dw-apb-uart.yaml
+++ b/Documentation/devicetree/bindings/serial/snps-dw-apb-uart.yaml
@@ -78,6 +78,7 @@ properties:
- starfive,jh7100-hsuart
- starfive,jh7100-uart
- starfive,jh7110-uart
+ - ultrarisc,dp1000-uart
- const: snps,dw-apb-uart
- const: snps,dw-apb-uart
--
2.34.1
^ permalink raw reply related
* [PATCH v5 1/4] serial: 8250_dwlib: move DesignWare register definitions to header
From: Jia Wang @ 2026-04-28 5:26 UTC (permalink / raw)
To: Ilpo Järvinen, Andy Shevchenko, Greg Kroah-Hartman,
Jiri Slaby, Paul Walmsley, Palmer Dabbelt, Albert Ou,
Alexandre Ghiti, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: linux-kernel, linux-serial, linux-riscv, devicetree, Jia Wang
In-Reply-To: <20260428-ultrarisc-serial-v5-0-97de63b1e3eb@ultrarisc.com>
Move the DW_UART_* register offsets and CPR bit/field definitions from
8250_dwlib.c into 8250_dwlib.h so they can be shared by 8250_dw and
8250_dwlib users.
Add an include guard for 8250_dwlib.h.
Signed-off-by: Jia Wang <wangjia@ultrarisc.com>
---
drivers/tty/serial/8250/8250_dw.c | 11 ------
drivers/tty/serial/8250/8250_dwlib.c | 49 --------------------------
drivers/tty/serial/8250/8250_dwlib.h | 67 ++++++++++++++++++++++++++++++++++++
3 files changed, 67 insertions(+), 60 deletions(-)
diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c
index 94beadb4024d..467755bf0092 100644
--- a/drivers/tty/serial/8250/8250_dw.c
+++ b/drivers/tty/serial/8250/8250_dw.c
@@ -34,22 +34,11 @@
#include "8250_dwlib.h"
-/* Offsets for the DesignWare specific registers */
-#define DW_UART_USR 0x1f /* UART Status Register */
-#define DW_UART_DMASA 0xa8 /* DMA Software Ack */
-
#define OCTEON_UART_USR 0x27 /* UART Status Register */
#define RZN1_UART_TDMACR 0x10c /* DMA Control Register Transmit Mode */
#define RZN1_UART_RDMACR 0x110 /* DMA Control Register Receive Mode */
-/* DesignWare specific register fields */
-#define DW_UART_IIR_IID GENMASK(3, 0)
-
-#define DW_UART_MCR_SIRE BIT(6)
-
-#define DW_UART_USR_BUSY BIT(0)
-
/* Renesas specific register fields */
#define RZN1_UART_xDMACR_DMA_EN BIT(0)
#define RZN1_UART_xDMACR_1_WORD_BURST (0 << 1)
diff --git a/drivers/tty/serial/8250/8250_dwlib.c b/drivers/tty/serial/8250/8250_dwlib.c
index b055d89cfb39..8859e66d2d71 100644
--- a/drivers/tty/serial/8250/8250_dwlib.c
+++ b/drivers/tty/serial/8250/8250_dwlib.c
@@ -13,55 +13,6 @@
#include "8250_dwlib.h"
-/* Offsets for the DesignWare specific registers */
-#define DW_UART_TCR 0xac /* Transceiver Control Register (RS485) */
-#define DW_UART_DE_EN 0xb0 /* Driver Output Enable Register */
-#define DW_UART_RE_EN 0xb4 /* Receiver Output Enable Register */
-#define DW_UART_DLF 0xc0 /* Divisor Latch Fraction Register */
-#define DW_UART_RAR 0xc4 /* Receive Address Register */
-#define DW_UART_TAR 0xc8 /* Transmit Address Register */
-#define DW_UART_LCR_EXT 0xcc /* Line Extended Control Register */
-#define DW_UART_CPR 0xf4 /* Component Parameter Register */
-#define DW_UART_UCV 0xf8 /* UART Component Version */
-
-/* Receive / Transmit Address Register bits */
-#define DW_UART_ADDR_MASK GENMASK(7, 0)
-
-/* Line Status Register bits */
-#define DW_UART_LSR_ADDR_RCVD BIT(8)
-
-/* Transceiver Control Register bits */
-#define DW_UART_TCR_RS485_EN BIT(0)
-#define DW_UART_TCR_RE_POL BIT(1)
-#define DW_UART_TCR_DE_POL BIT(2)
-#define DW_UART_TCR_XFER_MODE GENMASK(4, 3)
-#define DW_UART_TCR_XFER_MODE_DE_DURING_RE FIELD_PREP(DW_UART_TCR_XFER_MODE, 0)
-#define DW_UART_TCR_XFER_MODE_SW_DE_OR_RE FIELD_PREP(DW_UART_TCR_XFER_MODE, 1)
-#define DW_UART_TCR_XFER_MODE_DE_OR_RE FIELD_PREP(DW_UART_TCR_XFER_MODE, 2)
-
-/* Line Extended Control Register bits */
-#define DW_UART_LCR_EXT_DLS_E BIT(0)
-#define DW_UART_LCR_EXT_ADDR_MATCH BIT(1)
-#define DW_UART_LCR_EXT_SEND_ADDR BIT(2)
-#define DW_UART_LCR_EXT_TRANSMIT_MODE BIT(3)
-
-/* Component Parameter Register bits */
-#define DW_UART_CPR_ABP_DATA_WIDTH GENMASK(1, 0)
-#define DW_UART_CPR_AFCE_MODE BIT(4)
-#define DW_UART_CPR_THRE_MODE BIT(5)
-#define DW_UART_CPR_SIR_MODE BIT(6)
-#define DW_UART_CPR_SIR_LP_MODE BIT(7)
-#define DW_UART_CPR_ADDITIONAL_FEATURES BIT(8)
-#define DW_UART_CPR_FIFO_ACCESS BIT(9)
-#define DW_UART_CPR_FIFO_STAT BIT(10)
-#define DW_UART_CPR_SHADOW BIT(11)
-#define DW_UART_CPR_ENCODED_PARMS BIT(12)
-#define DW_UART_CPR_DMA_EXTRA BIT(13)
-#define DW_UART_CPR_FIFO_MODE GENMASK(23, 16)
-
-/* Helper for FIFO size calculation */
-#define DW_UART_CPR_FIFO_SIZE(a) (FIELD_GET(DW_UART_CPR_FIFO_MODE, (a)) * 16)
-
/*
* divisor = div(I) + div(F)
* "I" means integer, "F" means fractional
diff --git a/drivers/tty/serial/8250/8250_dwlib.h b/drivers/tty/serial/8250/8250_dwlib.h
index 7dd2a8e7b780..2f26f9ecacbe 100644
--- a/drivers/tty/serial/8250/8250_dwlib.h
+++ b/drivers/tty/serial/8250/8250_dwlib.h
@@ -1,11 +1,76 @@
/* SPDX-License-Identifier: GPL-2.0+ */
/* Synopsys DesignWare 8250 library header file. */
+#ifndef _SERIAL_8250_DWLIB_H_
+#define _SERIAL_8250_DWLIB_H_
+
+#include <linux/bitfield.h>
+#include <linux/bits.h>
#include <linux/io.h>
#include <linux/types.h>
#include "8250.h"
+/* Offsets for the DesignWare specific registers */
+#define DW_UART_USR 0x1f /* UART Status Register */
+#define DW_UART_DMASA 0xa8 /* DMA Software Ack */
+#define DW_UART_TCR 0xac /* Transceiver Control Register (RS485) */
+#define DW_UART_DE_EN 0xb0 /* Driver Output Enable Register */
+#define DW_UART_RE_EN 0xb4 /* Receiver Output Enable Register */
+#define DW_UART_DLF 0xc0 /* Divisor Latch Fraction Register */
+#define DW_UART_RAR 0xc4 /* Receive Address Register */
+#define DW_UART_TAR 0xc8 /* Transmit Address Register */
+#define DW_UART_LCR_EXT 0xcc /* Line Extended Control Register */
+#define DW_UART_CPR 0xf4 /* Component Parameter Register */
+#define DW_UART_UCV 0xf8 /* UART Component Version */
+
+/* Interrupt ID Register bits */
+#define DW_UART_IIR_IID GENMASK(3, 0)
+
+/* Modem Control Register bits */
+#define DW_UART_MCR_SIRE BIT(6)
+
+/* Line Status Register bits */
+#define DW_UART_LSR_ADDR_RCVD BIT(8)
+
+/* UART Status Register bits */
+#define DW_UART_USR_BUSY BIT(0)
+
+/* Transceiver Control Register bits */
+#define DW_UART_TCR_RS485_EN BIT(0)
+#define DW_UART_TCR_RE_POL BIT(1)
+#define DW_UART_TCR_DE_POL BIT(2)
+#define DW_UART_TCR_XFER_MODE GENMASK(4, 3)
+#define DW_UART_TCR_XFER_MODE_DE_DURING_RE FIELD_PREP(DW_UART_TCR_XFER_MODE, 0)
+#define DW_UART_TCR_XFER_MODE_SW_DE_OR_RE FIELD_PREP(DW_UART_TCR_XFER_MODE, 1)
+#define DW_UART_TCR_XFER_MODE_DE_OR_RE FIELD_PREP(DW_UART_TCR_XFER_MODE, 2)
+
+/* Receive / Transmit Address Register bits */
+#define DW_UART_ADDR_MASK GENMASK(7, 0)
+
+/* Line Extended Control Register bits */
+#define DW_UART_LCR_EXT_DLS_E BIT(0)
+#define DW_UART_LCR_EXT_ADDR_MATCH BIT(1)
+#define DW_UART_LCR_EXT_SEND_ADDR BIT(2)
+#define DW_UART_LCR_EXT_TRANSMIT_MODE BIT(3)
+
+/* Component Parameter Register bits */
+#define DW_UART_CPR_ABP_DATA_WIDTH GENMASK(1, 0)
+#define DW_UART_CPR_AFCE_MODE BIT(4)
+#define DW_UART_CPR_THRE_MODE BIT(5)
+#define DW_UART_CPR_SIR_MODE BIT(6)
+#define DW_UART_CPR_SIR_LP_MODE BIT(7)
+#define DW_UART_CPR_ADDITIONAL_FEATURES BIT(8)
+#define DW_UART_CPR_FIFO_ACCESS BIT(9)
+#define DW_UART_CPR_FIFO_STAT BIT(10)
+#define DW_UART_CPR_SHADOW BIT(11)
+#define DW_UART_CPR_ENCODED_PARMS BIT(12)
+#define DW_UART_CPR_DMA_EXTRA BIT(13)
+#define DW_UART_CPR_FIFO_MODE GENMASK(23, 16)
+
+/* Helper for FIFO size calculation */
+#define DW_UART_CPR_FIFO_SIZE(a) (FIELD_GET(DW_UART_CPR_FIFO_MODE, (a)) * 16)
+
struct dw8250_port_data {
/* Port properties */
int line;
@@ -38,3 +103,5 @@ static inline void dw8250_writel_ext(struct uart_port *p, int offset, u32 reg)
else
writel(reg, p->membase + offset);
}
+
+#endif /* _SERIAL_8250_DWLIB_H_ */
--
2.34.1
^ permalink raw reply related
* [PATCH v1] serial: qcom-geni: fix UART_RX_PAR_EN bit position
From: Prasanna S via B4 Relay @ 2026-04-28 4:26 UTC (permalink / raw)
To: Greg Kroah-Hartman, Jiri Slaby, Sagar Dharia, Girish Mahadevan,
Karthikeyan Ramasubramanian, Doug Anderson
Cc: linux-arm-msm, linux-kernel, linux-serial, stable, Prasanna S
From: Prasanna S <prasanna.s@oss.qualcomm.com>
UART_RX_PAR_EN is incorrectly defined as bit 3, which triggers false
framing errors (S_GP_IRQ_1_EN) and causes received data to be dropped
when parity is enabled and the parity bit is 0.
Define UART_RX_PAR_EN as bit 4 of the SE_UART_RX_TRANS_CFG register, as
specified in the reference manual.
Fixes: c4f528795d1a ("tty: serial: msm_geni_serial: Add serial driver support for GENI based QUP")
Cc: stable@vger.kernel.org
Signed-off-by: Prasanna S <prasanna.s@oss.qualcomm.com>
---
drivers/tty/serial/qcom_geni_serial.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c
index b365dd5da3cb..5139a9d21b2b 100644
--- a/drivers/tty/serial/qcom_geni_serial.c
+++ b/drivers/tty/serial/qcom_geni_serial.c
@@ -50,7 +50,7 @@
#define TX_STOP_BIT_LEN_2 2
/* SE_UART_RX_TRANS_CFG */
-#define UART_RX_PAR_EN BIT(3)
+#define UART_RX_PAR_EN BIT(4)
/* SE_UART_RX_WORD_LEN */
#define RX_WORD_LEN_MASK GENMASK(9, 0)
---
base-commit: dd6c438c3e64a5ff0b5d7e78f7f9be547803ef1b
change-id: 20260424-serial-bit-correct-f5314b627718
Best regards,
--
Prasanna S <prasanna.s@oss.qualcomm.com>
^ permalink raw reply related
* Re: [RFC PATCH v1 7/9] x86: Add unsafe_copy_from_user()
From: Yury Norov @ 2026-04-27 22:30 UTC (permalink / raw)
To: Linus Torvalds
Cc: Christophe Leroy (CS GROUP), Andrew Morton, David Laight,
Thomas Gleixner, linux-alpha, Yury Norov, linux-kernel,
linux-snps-arc, linux-arm-kernel, linux-mips, linuxppc-dev, kvm,
linux-riscv, linux-s390, sparclinux, linux-um, dmaengine,
linux-efi, linux-fsi, amd-gfx, dri-devel, intel-gfx, linux-wpan,
netdev, linux-wireless, linux-spi, linux-media, linux-staging,
linux-serial, linux-usb, xen-devel, linux-fsdevel, ocfs2-devel,
bpf, kasan-dev, linux-mm, linux-x25, rust-for-linux, linux-sound,
sound-open-firmware, linux-csky, linux-hexagon, loongarch,
linux-m68k, linux-openrisc, linux-parisc, linux-sh, linux-arch
In-Reply-To: <CAHk-=wgPrLy0FR3sEWBYQuNAac1axDASYMnTuPuxEU0WytzL7w@mail.gmail.com>
On Mon, Apr 27, 2026 at 02:52:05PM -0700, Linus Torvalds wrote:
> On Mon, 27 Apr 2026 at 12:19, Yury Norov <ynorov@nvidia.com> wrote:
> >
> > This is what Linus said when added x86 implementation for copy_from_user()
> > in c512c69187197:
>
> Note that some things have happily changed in the six+ years since...
>
> > That's partly because we have no current users of it, but also partly
> > because the copy_from_user() case is slightly different and cannot
> > efficiently be implemented in terms of a unsafe_get_user() loop (because
> > gcc can't do asm goto with outputs).
>
> now everybody can do asm goto with outputs.
>
> Yes, it's disabled on older versions, so it's not *always* available,
> but all modern versions do it. And if you care about performance, you
> won't be using an old compiler.
The minimal GCC version is 8.1, and asm goto with outputs is supported
since GCC-11. That would brake the build, if we just switch to using it
without "CC_IS_GCC && (GCC_VERSION >= 110100)" guard.
Is it worth to maintain 2 version of the function? I don't know...
Thanks,
Yury
^ permalink raw reply
* Re: [RFC PATCH v1 7/9] x86: Add unsafe_copy_from_user()
From: Linus Torvalds @ 2026-04-27 21:52 UTC (permalink / raw)
To: Yury Norov
Cc: Christophe Leroy (CS GROUP), Andrew Morton, David Laight,
Thomas Gleixner, linux-alpha, Yury Norov, linux-kernel,
linux-snps-arc, linux-arm-kernel, linux-mips, linuxppc-dev, kvm,
linux-riscv, linux-s390, sparclinux, linux-um, dmaengine,
linux-efi, linux-fsi, amd-gfx, dri-devel, intel-gfx, linux-wpan,
netdev, linux-wireless, linux-spi, linux-media, linux-staging,
linux-serial, linux-usb, xen-devel, linux-fsdevel, ocfs2-devel,
bpf, kasan-dev, linux-mm, linux-x25, rust-for-linux, linux-sound,
sound-open-firmware, linux-csky, linux-hexagon, loongarch,
linux-m68k, linux-openrisc, linux-parisc, linux-sh, linux-arch
In-Reply-To: <ae-2yLWSGnfeTvh1@yury>
On Mon, 27 Apr 2026 at 12:19, Yury Norov <ynorov@nvidia.com> wrote:
>
> This is what Linus said when added x86 implementation for copy_from_user()
> in c512c69187197:
Note that some things have happily changed in the six+ years since...
> That's partly because we have no current users of it, but also partly
> because the copy_from_user() case is slightly different and cannot
> efficiently be implemented in terms of a unsafe_get_user() loop (because
> gcc can't do asm goto with outputs).
now everybody can do asm goto with outputs.
Yes, it's disabled on older versions, so it's not *always* available,
but all modern versions do it. And if you care about performance, you
won't be using an old compiler.
Linus
^ permalink raw reply
* Re: [RFC PATCH v1 5/9] uaccess: Switch to copy_{to/from}_user_partial() when relevant
From: Linus Torvalds @ 2026-04-27 21:39 UTC (permalink / raw)
To: David Laight
Cc: Christophe Leroy (CS GROUP), Yury Norov, Andrew Morton,
Thomas Gleixner, linux-alpha, linux-kernel, linux-snps-arc,
linux-arm-kernel, linux-mips, linuxppc-dev, kvm, linux-riscv,
linux-s390, sparclinux, linux-um, dmaengine, linux-efi, linux-fsi,
amd-gfx, dri-devel, intel-gfx, linux-wpan, netdev, linux-wireless,
linux-spi, linux-media, linux-staging, linux-serial, linux-usb,
xen-devel, linux-fsdevel, ocfs2-devel, bpf, kasan-dev, linux-mm,
linux-x25, rust-for-linux, linux-sound, sound-open-firmware,
linux-csky, linux-hexagon, loongarch, linux-m68k, linux-openrisc,
linux-parisc, linux-sh, linux-arch
In-Reply-To: <20260427222914.1cb2dd3b@pumpkin>
On Mon, 27 Apr 2026 at 14:29, David Laight <david.laight.linux@gmail.com> wrote:
>
> I think there is a slight difference in that the normal copy_to_user()
> will determine the exact offset of the error by retrying with byte copies.
I have this dim memory that we decided that you can't reply on byte
exactness anyway, because not all architectures gave that guarantee
for the user copies.
But that thing came up many years ago, I might mis-remember.
Linus
^ permalink raw reply
* Re: [RFC PATCH v1 5/9] uaccess: Switch to copy_{to/from}_user_partial() when relevant
From: David Laight @ 2026-04-27 21:29 UTC (permalink / raw)
To: Linus Torvalds
Cc: Christophe Leroy (CS GROUP), Yury Norov, Andrew Morton,
Thomas Gleixner, linux-alpha, linux-kernel, linux-snps-arc,
linux-arm-kernel, linux-mips, linuxppc-dev, kvm, linux-riscv,
linux-s390, sparclinux, linux-um, dmaengine, linux-efi, linux-fsi,
amd-gfx, dri-devel, intel-gfx, linux-wpan, netdev, linux-wireless,
linux-spi, linux-media, linux-staging, linux-serial, linux-usb,
xen-devel, linux-fsdevel, ocfs2-devel, bpf, kasan-dev, linux-mm,
linux-x25, rust-for-linux, linux-sound, sound-open-firmware,
linux-csky, linux-hexagon, loongarch, linux-m68k, linux-openrisc,
linux-parisc, linux-sh, linux-arch
In-Reply-To: <CAHk-=whC1DZojwdMB1=sJWG2=dsCdfyU8N6tDE1qx50HRZ-WJQ@mail.gmail.com>
On Mon, 27 Apr 2026 12:01:23 -0700
Linus Torvalds <torvalds@linux-foundation.org> wrote:
> On Mon, 27 Apr 2026 at 10:18, Christophe Leroy (CS GROUP)
> <chleroy@kernel.org> wrote:
> >
> > In a subsequent patch, copy_{to/from}_user() will be modified to
> > return -EFAULT when copy fails.
>
> Please don't do this.
>
> This is a maintenance nightmare, and changes pretty much three decades
> of semantics, and will cause *very* subtle backporting issues if
> somebody happens to rely on the old / new behavior.
>
> I understand the reasoning for the change, but I really don't think
> the pain of creating yet another user copy interface is worth it.
>
> We already have a lot of different versions of user copies for
> different reasons, and while they all tend to have a good reason (and
> some not-so-good, but historical reasons) for existing, this one
> doesn't seem worth it.
>
> The main - perhaps only - reason for this "partial" version is that
> you want to do that "automatically inlined and optimized fixed-sized
> case".
>
> But here's the thing: I think you can already do that. Yes, it
> requires some improvements to unsafe_copy_from_user(), but *that*
> interface doesn't have three decades of history associated with it,
> _and_ you're extending on that one anyway in this series.
>
> "unsafe_copy_from_user()" is very odd, is meant only for small simple
> copies that can be inlined and it's special-cased for 'objtool' anyway
> (because objtool would have complained about an out-of-line call,
> although it could have been special-cased other ways).
>
> In other words: unsafe_copy_from_user() is *very* close to what you
> want for that "Oh, I noticed that it's a small fixed-size copy, so I
> want to special-case copy-from-user for that".
>
> The _only_ issue with unsafe_copy_from_user() is that you can't see
> that there were partial successes. But if *that* was fixed, then this
> whole "create a new copy_from_user interface" issue would just go
> away.
>
> So please - let's just change unsafe_copy_from_user() to be usable for
> the partial case.
>
> And the thing is, all the existing unsafe_copy_from_user()
> implementations already effectively *have* the "how much did I not
> copy" internally, and they actually do extra work to hide it, ie they
> have things like that
>
> int _i;
>
> that is "how many bytes have I copied" in the powerpc implementation,
> or the x86 code does
>
> size_t __ucu_len = (_len);
>
> where that "ucu_len" is updated as you go along and is literally the
> "how many bytes are left to copy" return value that is missing from
> this interface.
>
> So what I would suggest is
>
> - introduce a new user accessor helper that is used for *both*
> unsafe_copy_to/from_user() *and* the "inline small constant-sized
> normal copy_to/from_user()" calls
>
> - it's the same thing as the existing unsafe_copy_to/from_user()
> implementation, except it exposes how many bytes are left to be copied
> to the exception label.
I think there is a slight difference in that the normal copy_to_user()
will determine the exact offset of the error by retrying with byte copies.
There is also the issue of misaligned copies.
Then there is the 'bugbear' of hardened user copies.
Chasing down the stack to find whether the kernel buffer crosses
a stack frame is probably more expensive than the copy for the typically
small copies that will use on-stack buffers.
David
>
> IOW, it would look something like
>
> #define unsafe_copy_to_user_outlen(_dst,_src,_len,label)...
>
> which is exactly the same as the current unsafe_copy_to_user(),
> *except* it changes "_len" as it does along.
>
> And then you use that for both the "real" unsafe_copy_user and for the
> "small constant values" case.
>
> Just as an example, attached is a completely stupid rough draft of a
> patch that does this for x86 and only for unsafe_copy_to_user().
>
> And I made a very very hacky change to kernel/sys.c to see what the
> code generation looks like.
>
> This is what it results in on x86 with clang (with all the magic
> .section data edited out):
>
> ... edited out the code to generate the times
> ... this is the actual user copy:
> # HERE!
> movabsq $81985529216486895, %rcx # imm = 0x123456789ABCDEF
> cmpq %rcx, %rbx
> cmovaq %rcx, %rbx
> stac
> movq %r13, (%rbx) # exception to .LBB45_8
> movq %r14, 8(%rbx) # exception to .LBB45_8
> movq %r15, 16(%rbx) # exception to .LBB45_8
> movq %rax, 24(%rbx) # exception to .LBB45_8
> clac
> .LBB45_6:
> movq jiffies(%rip), %rdi
> callq jiffies_64_to_clock_t
> .LBB45_7:
> addq $16, %rsp
> popq %rbx
> popq %r12
> popq %r13
> popq %r14
> popq %r15
> retq
> .LBB45_8:
> clac
> movq $-14, %rax
> jmp .LBB45_7
>
> and notice how the compiler noticed that the 'outlen' isn't actually
> used, and turned the exception label into just a "return -EFAULT" and
> never actually generated any code for updating remaining lengths?
>
> That actually looks pretty much optimal for a 32-byte user copy.
>
> And it didn't involve changing the semantics at all.
>
> Just to check, I changed that "times()" system call to return the
> number of bytes uncopied instead (to emulate the "I actually want to
> know what's left" case), and it generated this:
>
> # HERE!
> movabsq $81985529216486895, %rcx # imm = 0x123456789ABCDEF
> cmpq %rcx, %rbx
> cmovaq %rcx, %rbx
> stac
> movl $32, %ecx
> movq %r13, (%rbx) # exception to .LBB45_7
> movl $24, %ecx
> movq %r15, 8(%rbx) # exception to .LBB45_7
> movl $16, %ecx
> movq %r14, 16(%rbx) # exception to .LBB45_7
> movl $8, %ecx
> movq %rax, 24(%rbx) # exception to .LBB45_7
> clac
> xorl %ecx, %ecx
> .LBB45_8:
> movq %rcx, %rax
> addq $16, %rsp
> popq %rbx
> popq %r12
> popq %r13
> popq %r14
> popq %r15
> retq
> .LBB45_6:
> movq jiffies(%rip), %rdi
> jmp jiffies_64_to_clock_t # TAILCALL
> .LBB45_7:
> clac
> jmp .LBB45_8
>
> so it all seems to work - although obviously the above is *not* the normal case.
>
> NOTE NOTE NOTE! The attached patch is entirely untested. I obviously
> did some "test code generation" with it, but I only *looked* at the
> result, and maybe it has some fundamental problem that I just didn't
> notice. So treat this as a "how about this approach" patch, not as
> anything more serious than that.
>
> And the kerrnel/sys.c hack is very obviously just that: a complate
> hack for testing.
>
> A real patch would do that "for small constant-sized copies, turn
> copy_to_user() automatically into "_small_copy_to_user()".
>
> The attached is *not* a real patch. Treat it with the contempt it deserves.
>
> Linus
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox