* Re: [PATCH 00/11] Convert moduleparams to seq_buf
From: Petr Pavlu @ 2026-05-26 6:53 UTC (permalink / raw)
To: Kees Cook
Cc: Luis Chamberlain, Pengpeng Hou, Richard Weinberger, Anton Ivanov,
Johannes Berg, Rafael J. Wysocki, Len Brown, Corey Minyard,
Gabriel Somlo, Michael S. Tsirkin, Jani Nikula, Joonas Lahtinen,
Rodrigo Vivi, Tvrtko Ursulin, David Airlie, Simona Vetter,
Bart Van Assche, Jason Gunthorpe, Leon Romanovsky,
Laurent Pinchart, Hans de Goede, Mauro Carvalho Chehab,
Bjorn Helgaas, Hannes Reinecke, James E.J. Bottomley,
Martin K. Petersen, Daniel Lezcano, Zhang Rui, Lukasz Luba,
Greg Kroah-Hartman, Jiri Slaby, Alan Stern, Jason Wang, Xuan Zhuo,
Eugenio Pérez, Jason Baron, Jim Cromie, Tiwei Bie,
Benjamin Berg, Ilpo Järvinen, David E. Box,
Maciej W. Rozycki, Srinivas Pandruvada, Peter Zijlstra,
Heiko Carstens, Vasily Gorbik, Sean Christopherson, Paolo Bonzini,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, Vinod Koul, Frank Li, Daniel Gomez, Sami Tolvanen,
Aaron Tomlin, Alexander Potapenko, Marco Elver, Dmitry Vyukov,
Andrew Morton, John Johansen, Paul Moore, James Morris,
Serge E. Hallyn, Andy Shevchenko, Georgia Garcia, kvm, dmaengine,
linux-modules, kasan-dev, linux-mm, apparmor,
linux-security-module, linux-um, linux-acpi, openipmi-developer,
qemu-devel, intel-gfx, dri-devel, linux-rdma, linux-media,
linux-pci, linux-scsi, linux-pm, linuxppc-dev, linux-serial,
linux-usb, usb-storage, virtualization, linux-kernel, linux-arch,
netdev, linux-fsdevel, linux-hardening
In-Reply-To: <20260521133315.work.845-kees@kernel.org>
On 5/21/26 3:33 PM, Kees Cook wrote:
> Hi,
>
> I tried to trim the CC list here, but it's still pretty huge...
>
> We've had a long-standing issue with "write to a string pointer" callbacks
> that don't bounds check the destination (and for which the bounds is
> also not part of the callback prototype, even if it is "known" to be
> PAGE_SIZE, which sysfs_emit() depends on). Both moduleparams and sysfs
> use this pattern. As a first step, and to test the migration method,
> migrate moduleparams first.
>
> There are 2 "mechanical" treewide patches that are handled by Coccinelle:
> - treewide: Convert struct kernel_param_ops initializers to DEFINE_KERNEL_PARAM_OPS
> - treewide: Convert custom kernel_param_ops .get callbacks to seq_buf via cocci
>
> The last treewide patch is manual, and may need to be broken up into
> per-subsystem patches, though I'd prefer to avoid this, as it would
> extend the migration from 1 relase to at least 2 releases. (1 to
> release the migration infrastructure, then 1 release to collect all the
> subsystem changes, and possibly 1 more release to remove the migration
> infrastructure.)
>
> Thoughts, questions?
This looks reasonable to me. I added a few minor comments on the patches
but they already look solid.
--
Thanks,
Petr
^ permalink raw reply
* Re: [PATCH v3 2/2] serial: qcom-geni: Add tracepoints for Qualcomm GENI serial driver
From: Praveen Talari @ 2026-05-26 5:06 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers, Jiri Slaby,
Konrad Dybcio, linux-kernel, linux-trace-kernel, linux-arm-msm,
linux-serial, Mukesh Kumar Savaliya, Aniket Randive,
chandana.chiluveru, jyothi.seerapu
In-Reply-To: <2026052258-scrooge-friction-fe21@gregkh>
Hi
On 22-05-2026 15:17, Greg Kroah-Hartman wrote:
> On Mon, May 18, 2026 at 11:26:56PM +0530, Praveen Talari wrote:
>> Add tracing to the Qualcomm GENI serial driver to improve runtime
>> observability.
>>
>> Trace hooks are added at key points including termios and clock
>> configuration, manual control get/set, interrupt handling, and data
>> TX/RX paths.
>>
>> Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
>> Signed-off-by: Praveen Talari <praveen.talari@oss.qualcomm.com>
>> ---
>> v2->v3:
>> - Updated commit text(removed example as it was available on cover
>> letter).
>> ---
>> drivers/tty/serial/qcom_geni_serial.c | 27 +++++++++++++++++++++++----
>> 1 file changed, 23 insertions(+), 4 deletions(-)
> This patch did not apply to my tree :(
Do you mean these patches are not applied cleanly?If yes, i will push on
linux-next tip.
Thanks,
Praveen Talari
^ permalink raw reply
* Re: [PATCH v3] serial: max310x: fix compile errors if CONFIG_SPI_MASTER is disabled
From: Randy Dunlap @ 2026-05-25 17:43 UTC (permalink / raw)
To: Hugo Villeneuve, Greg Kroah-Hartman, Jiri Slaby, Hugo Villeneuve
Cc: kernel test robot, linux-kernel, linux-serial
In-Reply-To: <20260521153333.2336642-1-hugo@hugovil.com>
On 5/21/26 8:33 AM, Hugo Villeneuve wrote:
> From: Hugo Villeneuve <hvilleneuve@dimonoff.com>
>
> Since commit 20ffe4b3330a8 ("serial: max310x: allow driver to be built with
> SPI or I2C"), if I2C is enabled and SPI_MASTER is disabled, we have these
> compile errors:
>
> drivers/tty/serial/max310x.c: In function 'max310x_uart_init':
> drivers/tty/serial/max310x.c: error: 'max310x_spi_driver' undeclared...
> drivers/tty/serial/max310x.c: In function ‘max310x_uart_init’:
> drivers/tty/serial/max310x.c: error: label ‘err_spi_register’
> defined but not used...
> drivers/tty/serial/max310x.c: error: ‘regcfg’ defined but not used
>
> Fix by properly encapsulating i2c/spi code/variables in their respective
> context with IS_ENABLED() macros for CONFIG_I2C and CONFIG_SPI_MASTER.
>
> Also fix link failure with SERIAL_MAX310X=y and I2C=m by modifying Kconfig
> depends.
>
> Fixes: 20ffe4b3330a8 ("serial: max310x: allow driver to be built with SPI or I2C")
> Reported-by: kernel test robot <lkp@intel.com>
> Closes: https://lore.kernel.org/oe-kbuild-all/202605121847.N9DVLNg2-lkp@intel.com/
> Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com>
> ---
> note: not Cc-ing stable as the commit is still in tty-next, and even if the
> errors originate from original commit that added I2C support, they were not
> trigerred because the driver could not be selected/compiled if
> CONFIG_SPI_MASTER was disabled.
>
> Changes for v3:
> - Fix link failure with SERIAL_MAX310X=y and I2C=m (Arnd Bergmann)
>
> Changes for v2:
> - replace #ifdef with #if IS_ENABLED() to suppoirt both built-in and modules
> options
> ---
> drivers/tty/serial/Kconfig | 2 +-
> drivers/tty/serial/max310x.c | 48 +++++++++++++++++++-----------------
> 2 files changed, 27 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
> index f834e5d292fd7..4accbfa75074c 100644
> --- a/drivers/tty/serial/Kconfig
> +++ b/drivers/tty/serial/Kconfig
> @@ -321,7 +321,7 @@ config SERIAL_MAX3100
>
> config SERIAL_MAX310X
> tristate "MAX310X support"
> - depends on SPI_MASTER || I2C
> + depends on (SPI_MASTER && !I2C) || I2C
> select SERIAL_CORE
> select REGMAP_SPI if SPI_MASTER
> select REGMAP_I2C if I2C
Other than preferring Arnd's Kconfig dependencies (easier to
read/understand IMO), this is all good.
Thanks.
Acked-by: Randy Dunlap <rdunlap@infradead.org>
Tested-by: Randy Dunlap <rdunlap@infradead.org> # build-tested
--
~Randy
^ permalink raw reply
* Re: [PATCH 08/11] params: Convert generic kernel_param_ops .get helpers to seq_buf
From: Petr Pavlu @ 2026-05-25 17:10 UTC (permalink / raw)
To: Kees Cook
Cc: Luis Chamberlain, Pengpeng Hou, Richard Weinberger, Anton Ivanov,
Johannes Berg, Rafael J. Wysocki, Len Brown, Corey Minyard,
Gabriel Somlo, Michael S. Tsirkin, Jani Nikula, Joonas Lahtinen,
Rodrigo Vivi, Tvrtko Ursulin, David Airlie, Simona Vetter,
Bart Van Assche, Jason Gunthorpe, Leon Romanovsky,
Laurent Pinchart, Hans de Goede, Mauro Carvalho Chehab,
Bjorn Helgaas, Hannes Reinecke, James E.J. Bottomley,
Martin K. Petersen, Daniel Lezcano, Zhang Rui, Lukasz Luba,
Greg Kroah-Hartman, Jiri Slaby, Alan Stern, Jason Wang, Xuan Zhuo,
Eugenio Pérez, Jason Baron, Jim Cromie, Tiwei Bie,
Benjamin Berg, Ilpo Järvinen, David E. Box,
Maciej W. Rozycki, Srinivas Pandruvada, Peter Zijlstra,
Heiko Carstens, Vasily Gorbik, Sean Christopherson, Paolo Bonzini,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, Vinod Koul, Frank Li, Daniel Gomez, Sami Tolvanen,
Aaron Tomlin, Alexander Potapenko, Marco Elver, Dmitry Vyukov,
Andrew Morton, John Johansen, Paul Moore, James Morris,
Serge E. Hallyn, Andy Shevchenko, Georgia Garcia, kvm, dmaengine,
linux-modules, kasan-dev, linux-mm, apparmor,
linux-security-module, linux-um, linux-acpi, openipmi-developer,
qemu-devel, intel-gfx, dri-devel, linux-rdma, linux-media,
linux-pci, linux-scsi, linux-pm, linuxppc-dev, linux-serial,
linux-usb, usb-storage, virtualization, linux-kernel, linux-arch,
netdev, linux-fsdevel, linux-hardening
In-Reply-To: <20260521133326.2465264-8-kees@kernel.org>
On 5/21/26 3:33 PM, Kees Cook wrote:
> Convert the generic struct kernel_param_ops .get helpers in
> kernel/params.c directly to the seq_buf signature, drop their legacy
> "char *" form, and refresh prototypes in <linux/moduleparam.h>:
>
> param_get_byte/short/ushort/int/uint/long/ulong/ullong/hexint
> param_get_charp/bool/invbool/string
> param_array_get
>
> The STANDARD_PARAM_DEF() macro expands to a seq_buf body for every
> numeric helper. param_array_get() now writes element output directly
> into the parent seq_buf when the element ops provide .get; it only
> allocates the per-call PAGE_SIZE bounce buffer when the element ops
> still use the legacy .get_str path. The common "rewrite the prior
> element's trailing newline as a comma" step lives outside both
> branches so the two paths share it.
>
> The non-core changes in this commit (arch/x86/kvm, mm/kfence,
> drivers/dma/dmatest, security/apparmor) are the small set of callers that
> directly invoke one of the converted generic helpers from their own .get
> callback (e.g. an apparmor wrapper that adds a capability check and then
> delegates to param_get_bool()). Because the helpers' signature changes
> here, these wrappers must move in lockstep. Each of them is updated
> to take "struct seq_buf *" and pass it through; param_get_debug() in
> apparmor also pulls aa_print_debug_params() (and its val_mask_to_str()
> helper, in security/apparmor/lib.c) over to seq_buf, since that is the
> only consumer. No other behavioural change is intended.
>
> Custom .get callbacks that do not delegate to a generic helper (and
> therefore still match the .get_str signature) are routed automatically
> to the .get_str field by the DEFINE_KERNEL_PARAM_OPS _Generic dispatcher
> and are deliberately left alone here, to be changed separately within
> their respective subsystems.
>
> Signed-off-by: Kees Cook <kees@kernel.org>
> ---
> [...]
> @@ -453,36 +457,46 @@ static int param_array_set(const char *val, const struct kernel_param *kp)
> arr->num ?: &temp_num);
> }
>
> -static int param_array_get(char *buffer, const struct kernel_param *kp)
> +static int param_array_get(struct seq_buf *s, const struct kernel_param *kp)
> {
> - int i, off, ret;
> - char *elem_buf;
> const struct kparam_array *arr = kp->arr;
> struct kernel_param p = *kp;
> + char *elem_buf = NULL;
> + int i, ret = 0;
>
> - elem_buf = kmalloc(PAGE_SIZE, GFP_KERNEL);
> - if (!elem_buf)
> - return -ENOMEM;
> + for (i = 0; i < (arr->num ? *arr->num : arr->max); i++) {
> + size_t before = s->len;
>
> - for (i = off = 0; i < (arr->num ? *arr->num : arr->max); i++) {
> p.arg = arr->elem + arr->elemsize * i;
> check_kparam_locked(p.mod);
> - ret = arr->ops->get_str(elem_buf, &p);
> - if (ret < 0)
> - goto out;
> - ret = min(ret, (int)(PAGE_SIZE - 1 - off));
> - if (!ret)
> +
> + if (arr->ops->get) {
> + ret = arr->ops->get(s, &p);
> + if (ret < 0)
> + goto out;
> + } else {
> + if (!elem_buf) {
> + elem_buf = kmalloc(PAGE_SIZE, GFP_KERNEL);
> + if (!elem_buf) {
> + ret = -ENOMEM;
> + goto out;
> + }
> + }
> + ret = arr->ops->get_str(elem_buf, &p);
> + if (ret < 0)
> + goto out;
> + seq_buf_putmem(s, elem_buf, ret);
> + }
> +
> + /* Nothing got written (e.g. overflow) — stop. */
> + if (s->len == before)
> break;
> +
> /* Replace the previous element's trailing newline with a comma. */
> - if (i)
> - buffer[off - 1] = ',';
> - memcpy(buffer + off, elem_buf, ret);
> - off += ret;
> - if (off == PAGE_SIZE - 1)
> - break;
> + if (i && s->buffer[before - 1] == '\n')
> + s->buffer[before - 1] = ',';
> }
> - buffer[off] = '\0';
> - ret = off;
> + ret = 0;
> out:
> kfree(elem_buf);
> return ret;
Since you're almost completely rewriting the logic in param_array_get(),
I suggest tightening it up a bit. The function could warn or return an
error when a kernel_param_ops::get/get_str() call adds a string that
doesn't terminate with '\n', specifically, when the call adds either
a zero-length string or a non-zero-length string that ends with
a different character (unless an overflow occurred).
The updated code silently stops the loop when a get call returns
a zero-length string. Similarly, handling of a string not terminated by
'\n' is halfway there because of the added check
"s->buffer[before - 1] == '\n'".
--
Thanks,
Petr
^ permalink raw reply
* Re: [PATCH 07/11] moduleparam: Route DEFINE_KERNEL_PARAM_OPS get pointer via _Generic
From: Petr Pavlu @ 2026-05-25 16:24 UTC (permalink / raw)
To: Kees Cook
Cc: Luis Chamberlain, Pengpeng Hou, Richard Weinberger, Anton Ivanov,
Johannes Berg, Rafael J. Wysocki, Len Brown, Corey Minyard,
Gabriel Somlo, Michael S. Tsirkin, Jani Nikula, Joonas Lahtinen,
Rodrigo Vivi, Tvrtko Ursulin, David Airlie, Simona Vetter,
Bart Van Assche, Jason Gunthorpe, Leon Romanovsky,
Laurent Pinchart, Hans de Goede, Mauro Carvalho Chehab,
Bjorn Helgaas, Hannes Reinecke, James E.J. Bottomley,
Martin K. Petersen, Daniel Lezcano, Zhang Rui, Lukasz Luba,
Greg Kroah-Hartman, Jiri Slaby, Alan Stern, Jason Wang, Xuan Zhuo,
Eugenio Pérez, Jason Baron, Jim Cromie, Tiwei Bie,
Benjamin Berg, Ilpo Järvinen, David E. Box,
Maciej W. Rozycki, Srinivas Pandruvada, Peter Zijlstra,
Heiko Carstens, Vasily Gorbik, Sean Christopherson, Paolo Bonzini,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, Vinod Koul, Frank Li, Daniel Gomez, Sami Tolvanen,
Aaron Tomlin, Alexander Potapenko, Marco Elver, Dmitry Vyukov,
Andrew Morton, John Johansen, Paul Moore, James Morris,
Serge E. Hallyn, Andy Shevchenko, Georgia Garcia, kvm, dmaengine,
linux-modules, kasan-dev, linux-mm, apparmor,
linux-security-module, linux-um, linux-acpi, openipmi-developer,
qemu-devel, intel-gfx, dri-devel, linux-rdma, linux-media,
linux-pci, linux-scsi, linux-pm, linuxppc-dev, linux-serial,
linux-usb, usb-storage, virtualization, linux-kernel, linux-arch,
netdev, linux-fsdevel, linux-hardening
In-Reply-To: <20260521133326.2465264-7-kees@kernel.org>
On 5/21/26 3:33 PM, Kees Cook wrote:
> Make the DEFINE_KERNEL_PARAM_OPS family route their _get argument to
> either .get (struct seq_buf *) or .get_str (char *) at compile time
> based on the pointer's actual function signature. Two helper macros
> do the routing:
>
> _KERNEL_PARAM_OPS_GET - return the pointer if it has the seq_buf
> signature, otherwise NULL of that type
> _KERNEL_PARAM_OPS_GET_STR - mirror image for the char * signature
>
> Both use _Generic; only the two valid function-pointer types are
> listed, so any third-party type is a compile error rather than
> silently falling through.
>
> Now a callback whose body has been migrated from char * to struct
> seq_buf * needs no change at its kernel_param_ops initialization site,
> because the macro picks up the new type automatically and assigns to
> the correct field.
>
> Signed-off-by: Kees Cook <kees@kernel.org>
> ---
> include/linux/moduleparam.h | 33 ++++++++++++++++++++++++++-------
> 1 file changed, 26 insertions(+), 7 deletions(-)
>
> diff --git a/include/linux/moduleparam.h b/include/linux/moduleparam.h
> index c52120f6ac28..795bc7c654ef 100644
> --- a/include/linux/moduleparam.h
> +++ b/include/linux/moduleparam.h
> @@ -85,15 +85,32 @@ struct kernel_param_ops {
> *
> * static DEFINE_KERNEL_PARAM_OPS(my_ops, my_set, my_get);
> *
> - * Routing the @_set and @_get function pointers through the macro
> - * (rather than naming the struct fields at every call site) lets the
> - * field layout change in one place when callbacks are migrated to a
> - * new signature.
> + * @_get may be either of:
> + * int (*)(struct seq_buf *, const struct kernel_param *) (seq_buf)
> + * int (*)(char *, const struct kernel_param *) (legacy)
> + *
> + * The macro uses _Generic to route the function pointer to the
> + * matching field (.get or .get_str) at compile time, leaving the
> + * other field NULL. Each helper matches the wrong prototype signature
> + * and returns NULL, falling through to the default branch otherwise;
> + * if @_get has neither expected signature the assignment to the
> + * fields gets a normal compile-time type-mismatch error.
> */
> +#define _KERNEL_PARAM_OPS_GET(_get) \
> + _Generic((_get), \
> + int (*)(char *, const struct kernel_param *): NULL, \
> + default: (_get))
> +
> +#define _KERNEL_PARAM_OPS_GET_STR(_get) \
> + _Generic((_get), \
> + int (*)(struct seq_buf *, const struct kernel_param *): NULL, \
> + default: (_get))
> +
> #define DEFINE_KERNEL_PARAM_OPS(_name, _set, _get) \
> const struct kernel_param_ops _name = { \
> .set = (_set), \
> - .get_str = (_get), \
> + .get = _KERNEL_PARAM_OPS_GET(_get), \
> + .get_str = _KERNEL_PARAM_OPS_GET_STR(_get), \
> }
>
> /* As DEFINE_KERNEL_PARAM_OPS, with KERNEL_PARAM_OPS_FL_NOARG set. */
> @@ -101,14 +118,16 @@ struct kernel_param_ops {
> const struct kernel_param_ops _name = { \
> .flags = KERNEL_PARAM_OPS_FL_NOARG, \
> .set = (_set), \
> - .get_str = (_get), \
> + .get = _KERNEL_PARAM_OPS_GET(_get), \
> + .get_str = _KERNEL_PARAM_OPS_GET_STR(_get), \
> }
>
> /* As DEFINE_KERNEL_PARAM_OPS, with an additional .free callback. */
> #define DEFINE_KERNEL_PARAM_OPS_FREE(_name, _set, _get, _free) \
> const struct kernel_param_ops _name = { \
> .set = (_set), \
> - .get_str = (_get), \
> + .get = _KERNEL_PARAM_OPS_GET(_get), \
> + .get_str = _KERNEL_PARAM_OPS_GET_STR(_get), \
> .free = (_free), \
> }
>
Reviewed-by: Petr Pavlu <petr.pavlu@suse.com>
-- Petr
^ permalink raw reply
* Re: [PATCH 06/11] moduleparam: Add seq_buf-based .get callback alongside .get_str
From: Petr Pavlu @ 2026-05-25 16:19 UTC (permalink / raw)
To: Kees Cook
Cc: Luis Chamberlain, Pengpeng Hou, Richard Weinberger, Anton Ivanov,
Johannes Berg, Rafael J. Wysocki, Len Brown, Corey Minyard,
Gabriel Somlo, Michael S. Tsirkin, Jani Nikula, Joonas Lahtinen,
Rodrigo Vivi, Tvrtko Ursulin, David Airlie, Simona Vetter,
Bart Van Assche, Jason Gunthorpe, Leon Romanovsky,
Laurent Pinchart, Hans de Goede, Mauro Carvalho Chehab,
Bjorn Helgaas, Hannes Reinecke, James E.J. Bottomley,
Martin K. Petersen, Daniel Lezcano, Zhang Rui, Lukasz Luba,
Greg Kroah-Hartman, Jiri Slaby, Alan Stern, Jason Wang, Xuan Zhuo,
Eugenio Pérez, Jason Baron, Jim Cromie, Tiwei Bie,
Benjamin Berg, Ilpo Järvinen, David E. Box,
Maciej W. Rozycki, Srinivas Pandruvada, Peter Zijlstra,
Heiko Carstens, Vasily Gorbik, Sean Christopherson, Paolo Bonzini,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, Vinod Koul, Frank Li, Daniel Gomez, Sami Tolvanen,
Aaron Tomlin, Alexander Potapenko, Marco Elver, Dmitry Vyukov,
Andrew Morton, John Johansen, Paul Moore, James Morris,
Serge E. Hallyn, Andy Shevchenko, Georgia Garcia, kvm, dmaengine,
linux-modules, kasan-dev, linux-mm, apparmor,
linux-security-module, linux-um, linux-acpi, openipmi-developer,
qemu-devel, intel-gfx, dri-devel, linux-rdma, linux-media,
linux-pci, linux-scsi, linux-pm, linuxppc-dev, linux-serial,
linux-usb, usb-storage, virtualization, linux-kernel, linux-arch,
netdev, linux-fsdevel, linux-hardening
In-Reply-To: <20260521133326.2465264-6-kees@kernel.org>
On 5/21/26 3:33 PM, Kees Cook wrote:
> Add a new struct kernel_param_ops::get callback whose signature
> takes a struct seq_buf instead of a raw char buffer:
>
> int (*get)(struct seq_buf *sb, const struct kernel_param *kp);
>
> The previously-legacy .get field is now .get_str (char *buffer);
> .get is the new seq_buf-aware form. param_attr_show() prefers .get
> when set, otherwise falls back to .get_str. WARN_ON_ONCE() if both
> are set. Return contract for .get:
>
> < 0 : errno propagated to userspace; seq_buf contents discarded
> = 0 : success; length derived from seq_buf_used()
> > 0 : forbidden; the dispatcher WARN_ON_ONCE()s and treats as 0
>
> The default policy on seq_buf_has_overflowed() is silent truncation,
> matching scnprintf()/sysfs_emit() behaviour. Callbacks that want a
> specific overflow errno can check seq_buf_has_overflowed() and
> return their preferred error.
>
> No callbacks use .get yet; the legacy path is still the only one in use
> after this commit. A subsequent commit teaches DEFINE_KERNEL_PARAM_OPS
> to route initializers by type.
>
> Signed-off-by: Kees Cook <kees@kernel.org>
Reviewed-by: Petr Pavlu <petr.pavlu@suse.com>
-- Petr
^ permalink raw reply
* Re: [PATCH 04/11] treewide: Convert struct kernel_param_ops initializers to DEFINE_KERNEL_PARAM_OPS
From: Petr Pavlu @ 2026-05-25 13:35 UTC (permalink / raw)
To: Kees Cook
Cc: Luis Chamberlain, Pengpeng Hou, Richard Weinberger, Anton Ivanov,
Johannes Berg, Rafael J. Wysocki, Len Brown, Corey Minyard,
Gabriel Somlo, Michael S. Tsirkin, Jani Nikula, Joonas Lahtinen,
Rodrigo Vivi, Tvrtko Ursulin, David Airlie, Simona Vetter,
Bart Van Assche, Jason Gunthorpe, Leon Romanovsky,
Laurent Pinchart, Hans de Goede, Mauro Carvalho Chehab,
Bjorn Helgaas, Hannes Reinecke, James E.J. Bottomley,
Martin K. Petersen, Daniel Lezcano, Zhang Rui, Lukasz Luba,
Greg Kroah-Hartman, Jiri Slaby, Alan Stern, Jason Wang, Xuan Zhuo,
Eugenio Pérez, Jason Baron, Jim Cromie, Tiwei Bie,
Benjamin Berg, Ilpo Järvinen, David E. Box,
Maciej W. Rozycki, Srinivas Pandruvada, Peter Zijlstra,
Heiko Carstens, Vasily Gorbik, Sean Christopherson, Paolo Bonzini,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, Vinod Koul, Frank Li, Daniel Gomez, Sami Tolvanen,
Aaron Tomlin, Alexander Potapenko, Marco Elver, Dmitry Vyukov,
Andrew Morton, John Johansen, Paul Moore, James Morris,
Serge E. Hallyn, Andy Shevchenko, Georgia Garcia, kvm, dmaengine,
linux-modules, kasan-dev, linux-mm, apparmor,
linux-security-module, linux-um, linux-acpi, openipmi-developer,
qemu-devel, intel-gfx, dri-devel, linux-rdma, linux-media,
linux-pci, linux-scsi, linux-pm, linuxppc-dev, linux-serial,
linux-usb, usb-storage, virtualization, linux-kernel, linux-arch,
netdev, linux-fsdevel, linux-hardening
In-Reply-To: <20260521133326.2465264-4-kees@kernel.org>
On 5/21/26 3:33 PM, Kees Cook wrote:
> Using Coccinelle, rewrite every struct kernel_param_ops initializer that
> sets .get into a DEFINE_KERNEL_PARAM_OPS-family macro invocation,
> for example:
>
> @@
> declarer name DEFINE_KERNEL_PARAM_OPS;
> identifier OPS;
> expression SET, GET;
> @@
> - const struct kernel_param_ops OPS = {
> - .set = SET,
> - .get = GET,
> - };
> + DEFINE_KERNEL_PARAM_OPS(OPS, SET, GET);
>
> Using the macro for initialization means future changes can manipulate
> the struct layout and callback prototypes without having to change every
> initializer.
Nit: For consistency, I suggest also converting the few remaining
kernel_param_ops instances that specify only .set and no .get, such as
simdisk_param_ops_filename.
--
Thanks,
Petr
^ permalink raw reply
* Re: [PATCH 03/11] moduleparam: Add DEFINE_KERNEL_PARAM_OPS macro family
From: Petr Pavlu @ 2026-05-25 13:27 UTC (permalink / raw)
To: Kees Cook
Cc: Luis Chamberlain, Pengpeng Hou, Richard Weinberger, Anton Ivanov,
Johannes Berg, Rafael J. Wysocki, Len Brown, Corey Minyard,
Gabriel Somlo, Michael S. Tsirkin, Jani Nikula, Joonas Lahtinen,
Rodrigo Vivi, Tvrtko Ursulin, David Airlie, Simona Vetter,
Bart Van Assche, Jason Gunthorpe, Leon Romanovsky,
Laurent Pinchart, Hans de Goede, Mauro Carvalho Chehab,
Bjorn Helgaas, Hannes Reinecke, James E.J. Bottomley,
Martin K. Petersen, Daniel Lezcano, Zhang Rui, Lukasz Luba,
Greg Kroah-Hartman, Jiri Slaby, Alan Stern, Jason Wang, Xuan Zhuo,
Eugenio Pérez, Jason Baron, Jim Cromie, Tiwei Bie,
Benjamin Berg, Ilpo Järvinen, David E. Box,
Maciej W. Rozycki, Srinivas Pandruvada, Peter Zijlstra,
Heiko Carstens, Vasily Gorbik, Sean Christopherson, Paolo Bonzini,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, Vinod Koul, Frank Li, Daniel Gomez, Sami Tolvanen,
Aaron Tomlin, Alexander Potapenko, Marco Elver, Dmitry Vyukov,
Andrew Morton, John Johansen, Paul Moore, James Morris,
Serge E. Hallyn, Andy Shevchenko, Georgia Garcia, kvm, dmaengine,
linux-modules, kasan-dev, linux-mm, apparmor,
linux-security-module, linux-um, linux-acpi, openipmi-developer,
qemu-devel, intel-gfx, dri-devel, linux-rdma, linux-media,
linux-pci, linux-scsi, linux-pm, linuxppc-dev, linux-serial,
linux-usb, usb-storage, virtualization, linux-kernel, linux-arch,
netdev, linux-fsdevel, linux-hardening
In-Reply-To: <20260521133326.2465264-3-kees@kernel.org>
On 5/21/26 3:33 PM, Kees Cook wrote:
> Add macros that define a struct kernel_param_ops initializer through a
> macro so the underlying field layout can evolve without touching every
> call site. Three variants cover the three cases:
>
> DEFINE_KERNEL_PARAM_OPS(name, set, get) // basic
> DEFINE_KERNEL_PARAM_OPS_NOARG(name, set, get) // set KERNEL_PARAM_OPS_FL_NOARG
> DEFINE_KERNEL_PARAM_OPS_FREE(name, set, get, free) // also set .free
>
> Callers prefix their own visibility qualifiers, e.g.:
>
> static DEFINE_KERNEL_PARAM_OPS(my_ops, my_set, my_get);
>
> Also update module_param_call() and STANDARD_PARAM_DEF() to use
> DEFINE_KERNEL_PARAM_OPS internally so the generated ops table will go
> through the same macro as everything else.
>
> Subsequent commits convert all open-coded struct kernel_param_ops
> definitions to use these macros, in preparation for migrating to a
> seq_buf .get API.
>
> Signed-off-by: Kees Cook <kees@kernel.org>
> ---
> include/linux/moduleparam.h | 36 ++++++++++++++++++++++++++++++++++--
> kernel/params.c | 6 ++----
> 2 files changed, 36 insertions(+), 6 deletions(-)
>
> diff --git a/include/linux/moduleparam.h b/include/linux/moduleparam.h
> index 075f28585074..26bf45b36d02 100644
> --- a/include/linux/moduleparam.h
> +++ b/include/linux/moduleparam.h
> @@ -68,6 +68,39 @@ struct kernel_param_ops {
> void (*free)(void *arg);
> };
>
> +/*
> + * Define a const struct kernel_param_ops initializer. Callers prefix with
> + * any required visibility qualifiers (typically "static"):
> + *
> + * static DEFINE_KERNEL_PARAM_OPS(my_ops, my_set, my_get);
> + *
> + * Routing the @_set and @_get function pointers through the macro
> + * (rather than naming the struct fields at every call site) lets the
> + * field layout change in one place when callbacks are migrated to a
> + * new signature.
> + */
Nit: The newly introduced DEFINE_KERNEL_PARAM_OPS*() macros remain in
place at the end of the series after the migration is complete and this
comment is removed in patch 7. It would be helpful to describe in the
commit message why these macros are generally preferable to defining
kernel_param_ops instances directly.
I assume the motivation is that the structure is simple enough and using
macros then makes defining kernel_param_ops instances a bit more
concise. A minor disadvantage is that some analysis tools, such as
ctags, may no longer see the generated definition, but that is also the
case for DEFINE_MUTEX() and other similar macros.
--
Thanks,
Petr
^ permalink raw reply
* [PATCH 3/3] serial: max310x: honour rs485 properties from per-port DT subnode
From: Tapio Reijonen @ 2026-05-25 9:43 UTC (permalink / raw)
To: Greg Kroah-Hartman, Jiri Slaby, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Hugo Villeneuve
Cc: linux-kernel, linux-serial, devicetree, Tapio Reijonen
In-Reply-To: <20260525-b4-max310x-rs485-dt-v1-0-e6c19b4d5592@vaisala.com>
The MAX310x DT binding pulls in /schemas/serial/rs485.yaml via its
allOf list, advertising the rs485-* properties defined there - none
of which were honoured at runtime, because the driver never called
uart_get_rs485_mode().
All ports share the parent SPI/I2C device, so uart_get_rs485_mode()
called directly on each port would read the same chip-level fwnode
for every call. Walk dev->of_node's children for the one named "port"
with matching reg, and temporarily retarget the parent device's
fwnode while uart_get_rs485_mode() runs, so each port picks up its
own subnode's properties. Probe is serialised, so the swap is safe.
For single-port variants (max3107, max3108), fall back to the chip's
own fwnode when no port@0 subnode is present, so existing DTs that
declare rs485 properties at the top level keep working.
Signed-off-by: Tapio Reijonen <tapio.reijonen@vaisala.com>
---
drivers/tty/serial/max310x.c | 37 +++++++++++++++++++++++++++++++++++++
1 file changed, 37 insertions(+)
diff --git a/drivers/tty/serial/max310x.c b/drivers/tty/serial/max310x.c
index 5cb7d01e404663dc25b88bc7b4f8df61be2135ec..745498034293cf74c8b4d25b45739e787f1843de 100644
--- a/drivers/tty/serial/max310x.c
+++ b/drivers/tty/serial/max310x.c
@@ -1426,6 +1426,9 @@ static int max310x_probe(struct device *dev, const struct max310x_devtype *devty
#endif
for (i = 0; i < devtype->nr; i++) {
+ struct fwnode_handle *saved_fwnode = dev_fwnode(dev);
+ struct device_node *port_np = NULL;
+ struct device_node *child;
unsigned int line;
line = find_first_zero_bit(max310x_lines, MAX310X_UART_NRMAX);
@@ -1435,6 +1438,40 @@ static int max310x_probe(struct device *dev, const struct max310x_devtype *devty
}
s->p[i].port.line = line;
+ /* Locate the matching "port@i" DT subnode, if any. */
+ for_each_available_child_of_node(dev->of_node, child) {
+ u32 reg;
+
+ if (!of_node_name_eq(child, "port"))
+ continue;
+ if (of_property_read_u32(child, "reg", ®))
+ continue;
+ if (reg == i) {
+ port_np = child;
+ break;
+ }
+ }
+
+ /*
+ * Temporarily retarget dev's fwnode to the per-port subnode
+ * so uart_get_rs485_mode() picks up the per-port properties.
+ * For single-port variants, fall back to the chip's own
+ * fwnode so legacy DTs that declare rs485 properties at the
+ * top level keep working.
+ */
+ if (port_np) {
+ device_set_node(dev, of_fwnode_handle(port_np));
+ ret = uart_get_rs485_mode(&s->p[i].port);
+ device_set_node(dev, saved_fwnode);
+ of_node_put(port_np);
+ if (ret)
+ goto out_uart;
+ } else if (devtype->nr == 1) {
+ ret = uart_get_rs485_mode(&s->p[i].port);
+ if (ret)
+ goto out_uart;
+ }
+
/* Register port */
ret = uart_add_one_port(&max310x_uart, &s->p[i].port);
if (ret)
--
2.47.3
^ permalink raw reply related
* [PATCH 2/3] dt-bindings: serial: maxim,max310x: allow per-port subnodes for rs485
From: Tapio Reijonen @ 2026-05-25 9:43 UTC (permalink / raw)
To: Greg Kroah-Hartman, Jiri Slaby, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Hugo Villeneuve
Cc: linux-kernel, linux-serial, devicetree, Tapio Reijonen
In-Reply-To: <20260525-b4-max310x-rs485-dt-v1-0-e6c19b4d5592@vaisala.com>
The MAX310x is a multi-port UART (up to four ports). The existing
binding pulls in /schemas/serial/rs485.yaml at the top level, which
only describes a single port - sufficient for max3107 but ambiguous
for max14830 where each port can have its own RS485 wiring.
Add a "port@N" pattern (N = 0..3) carrying rs485 properties on a
per-port basis. When port@N subnodes are present, the chip node also
needs #address-cells = <1> and #size-cells = <0>; allow both. Top-
level rs485 properties remain accepted for compatibility.
Signed-off-by: Tapio Reijonen <tapio.reijonen@vaisala.com>
---
.../devicetree/bindings/serial/maxim,max310x.yaml | 60 ++++++++++++++++++++++
1 file changed, 60 insertions(+)
diff --git a/Documentation/devicetree/bindings/serial/maxim,max310x.yaml b/Documentation/devicetree/bindings/serial/maxim,max310x.yaml
index 889eeaca64a027b4d9e8ec87bcf63fcc8fd9d55b..b7095c9abacde81e69c1138e817a1d5bdfaf14d7 100644
--- a/Documentation/devicetree/bindings/serial/maxim,max310x.yaml
+++ b/Documentation/devicetree/bindings/serial/maxim,max310x.yaml
@@ -40,6 +40,34 @@ properties:
minItems: 1
maxItems: 16
+ "#address-cells":
+ const: 1
+
+ "#size-cells":
+ const: 0
+
+patternProperties:
+ "^port@[0-3]$":
+ type: object
+ description:
+ Per-port subnode that carries the RS-485 properties from
+ /schemas/serial/rs485.yaml# for a single UART channel of the
+ chip. Only those rs485-* properties (and the per-port reg) are
+ accepted here; device-wide settings stay on the parent node.
+
+ allOf:
+ - $ref: /schemas/serial/rs485.yaml#
+
+ properties:
+ reg:
+ description: Port number on the chip.
+ maximum: 3
+
+ required:
+ - reg
+
+ unevaluatedProperties: false
+
required:
- compatible
- reg
@@ -72,3 +100,35 @@ examples:
#gpio-cells = <2>;
};
};
+
+ - |
+ #include <dt-bindings/interrupt-controller/irq.h>
+ spi {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ serial@0 {
+ compatible = "maxim,max14830";
+ reg = <0>;
+ spi-max-frequency = <26000000>;
+ clocks = <&xtal4m>;
+ clock-names = "xtal";
+ interrupt-parent = <&gpio3>;
+ interrupts = <7 IRQ_TYPE_LEVEL_LOW>;
+ gpio-controller;
+ #gpio-cells = <2>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@0 {
+ reg = <0>;
+ rs485-rts-active-low;
+ linux,rs485-enabled-at-boot-time;
+ };
+
+ port@2 {
+ reg = <2>;
+ rs485-rts-active-low;
+ };
+ };
+ };
--
2.47.3
^ permalink raw reply related
* [PATCH 1/3] serial: max310x: register GPIO controller before adding UART ports
From: Tapio Reijonen @ 2026-05-25 9:43 UTC (permalink / raw)
To: Greg Kroah-Hartman, Jiri Slaby, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Hugo Villeneuve
Cc: linux-kernel, linux-serial, devicetree, Tapio Reijonen
In-Reply-To: <20260525-b4-max310x-rs485-dt-v1-0-e6c19b4d5592@vaisala.com>
The MAX310x exposes four GPIOs per UART port via an in-driver
gpio_chip. devm_gpiochip_add_data() used to run after the per-port
uart_add_one_port() loop, so a device-tree consumer referencing one of
the chip's own GPIOs (for example rs485-term-gpios = <&max310x 0 ...>)
could not resolve it during port registration: the GPIO provider it
waits for is the very driver still trying to register, and the lookup
returns -EPROBE_DEFER on its own provider, deferring probe forever.
Split the per-port setup into two passes around the gpio_chip
registration:
1. Initialise per-port state - port struct fields, regmap binding,
IRQ disable, work queues. The gpio_chip callbacks dereference
s->p[i].regmap via to_max310x_port() and become callable as soon
as the chip is visible to gpiolib, so every entry must be
populated first.
2. devm_gpiochip_add_data() - register the gpio_chip.
3. Allocate a line, uart_add_one_port(), set_bit(), max310x_power().
Keeping line allocation, registration and set_bit() together
preserves the existing "bit set <=> port registered" rollback
invariant that out_uart relies on.
Signed-off-by: Tapio Reijonen <tapio.reijonen@vaisala.com>
---
drivers/tty/serial/max310x.c | 54 +++++++++++++++++++++++++++-----------------
1 file changed, 33 insertions(+), 21 deletions(-)
diff --git a/drivers/tty/serial/max310x.c b/drivers/tty/serial/max310x.c
index ac7d3f197c3a5ce3531d5607f48e21a807314021..5cb7d01e404663dc25b88bc7b4f8df61be2135ec 100644
--- a/drivers/tty/serial/max310x.c
+++ b/drivers/tty/serial/max310x.c
@@ -1364,17 +1364,12 @@ static int max310x_probe(struct device *dev, const struct max310x_devtype *devty
dev_dbg(dev, "Reference clock set to %i Hz\n", uartclk);
+ /*
+ * Set up each port's state before registering the gpiochip,
+ * since the gpiochip callbacks will read s->p[i].regmap as
+ * soon as gpiolib exposes the controller.
+ */
for (i = 0; i < devtype->nr; i++) {
- unsigned int line;
-
- line = find_first_zero_bit(max310x_lines, MAX310X_UART_NRMAX);
- if (line == MAX310X_UART_NRMAX) {
- ret = -ERANGE;
- goto out_uart;
- }
-
- /* Initialize port data */
- s->p[i].port.line = line;
s->p[i].port.dev = dev;
s->p[i].port.irq = irq;
s->p[i].port.type = PORT_MAX310X;
@@ -1404,20 +1399,16 @@ static int max310x_probe(struct device *dev, const struct max310x_devtype *devty
INIT_WORK(&s->p[i].md_work, max310x_md_proc);
/* Initialize queue for changing RS485 mode */
INIT_WORK(&s->p[i].rs_work, max310x_rs_proc);
-
- /* Register port */
- ret = uart_add_one_port(&max310x_uart, &s->p[i].port);
- if (ret)
- goto out_uart;
-
- set_bit(line, max310x_lines);
-
- /* Go to suspend mode */
- max310x_power(&s->p[i].port, 0);
}
#ifdef CONFIG_GPIOLIB
- /* Setup GPIO controller */
+ /*
+ * Register the GPIO controller before adding the UART ports so
+ * that consumers referencing the chip's own GPIOs from device
+ * tree (for example rs485-term-gpios = <&max310x ...>) can
+ * resolve them at uart_add_one_port() time instead of receiving
+ * -EPROBE_DEFER from their own provider.
+ */
s->gpio.owner = THIS_MODULE;
s->gpio.parent = dev;
s->gpio.label = devtype->name;
@@ -1434,6 +1425,27 @@ static int max310x_probe(struct device *dev, const struct max310x_devtype *devty
goto out_uart;
#endif
+ for (i = 0; i < devtype->nr; i++) {
+ unsigned int line;
+
+ line = find_first_zero_bit(max310x_lines, MAX310X_UART_NRMAX);
+ if (line == MAX310X_UART_NRMAX) {
+ ret = -ERANGE;
+ goto out_uart;
+ }
+ s->p[i].port.line = line;
+
+ /* Register port */
+ ret = uart_add_one_port(&max310x_uart, &s->p[i].port);
+ if (ret)
+ goto out_uart;
+
+ set_bit(line, max310x_lines);
+
+ /* Go to suspend mode */
+ max310x_power(&s->p[i].port, 0);
+ }
+
/* Setup interrupt */
ret = devm_request_threaded_irq(dev, irq, NULL, max310x_ist,
IRQF_ONESHOT | IRQF_SHARED, dev_name(dev), s);
--
2.47.3
^ permalink raw reply related
* [PATCH 0/3] serial: max310x: honour per-port DT RS485 properties
From: Tapio Reijonen @ 2026-05-25 9:43 UTC (permalink / raw)
To: Greg Kroah-Hartman, Jiri Slaby, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Hugo Villeneuve
Cc: linux-kernel, linux-serial, devicetree, Tapio Reijonen
The MAX310x DT binding pulls in /schemas/serial/rs485.yaml via its allOf
list, but the driver has never actually called uart_get_rs485_mode(), so
none of the advertised rs485-* properties take effect at runtime.
This series wires per-port RS485 DT configuration end to end:
Patch 1 reorders the probe so the gpio_chip is registered before
uart_add_one_port(). A port can then reference one of the chip's own
GPIOs (e.g. rs485-term-gpios = <&max310x ...>) without -EPROBE_DEFER
from its own provider - prerequisite for patch 3.
Patch 2 extends the binding with a "port@N" subnode pattern (N = 0..3)
carrying per-port RS485 properties. All ports share the same parent
SPI/I2C device, so per-port differentiation requires distinct DT
subnodes. Top-level RS485 properties remain accepted for compatibility
with single-port chips (max3107, max3108).
Patch 3 reads each port's RS485 properties from its own subnode by
temporarily retargeting dev->fwnode while uart_get_rs485_mode() runs.
For single-port variants, falls back to the chip's own fwnode when no
port@0 subnode is present.
Note for maintainers: patch 3 mutates the parent SPI/I2C device's
fwnode around the uart_get_rs485_mode() call so the underlying
property/GPIO lookups resolve against the per-port DT subnode. Probe
is serialised, so the swap is locally safe, but I'd appreciate
feedback on whether this idiom is acceptable. If a cleaner shape is
preferred (a serial_core helper that takes a fwnode directly, or
one struct device per port), I'll respin accordingly.
Tested on max14830 (SPI, 4 ports): each ttyMAXn port comes up with the
rs485 flags and delays configured in its port@N subnode, and the
termination GPIO sourced from the MAX310x's own gpio_chip is resolved
without probe deferral.
Signed-off-by: Tapio Reijonen <tapio.reijonen@vaisala.com>
---
Tapio Reijonen (3):
serial: max310x: register GPIO controller before adding UART ports
dt-bindings: serial: maxim,max310x: allow per-port subnodes for rs485
serial: max310x: honour rs485 properties from per-port DT subnode
.../devicetree/bindings/serial/maxim,max310x.yaml | 60 ++++++++++++++
drivers/tty/serial/max310x.c | 91 +++++++++++++++++-----
2 files changed, 130 insertions(+), 21 deletions(-)
---
base-commit: 79bd2dded182b1d458b18e62684b7f82ffc682e5
change-id: 20260525-b4-max310x-rs485-dt-ebff12af9976
Best regards,
--
Tapio Reijonen <tapio.reijonen@vaisala.com>
^ permalink raw reply
* [PATCH v2 0/6] MIPS: SiByte: Fix serial device regressions
From: Maciej W. Rozycki @ 2026-05-24 23:12 UTC (permalink / raw)
To: Thomas Bogendoerfer, Greg Kroah-Hartman, Jiri Slaby
Cc: Elena Reshetova, David Windsor, Kees Cook, Hans Liljestrand,
linux-mips, linux-serial, linux-kernel
Hi,
This is v2 of the series addressing feedback to original 4/4 where the
plain revert was questioned.
I proposed using a spinlock instead of the atomic counter, which was then
agreed upon. In the course of making the change I have realised that from
the algorithm's correctness point of view the request and release of the
resource in question needs to be done under the spinlock as well, because
in particular setserial(8) can cause port resources to be released and
requested in parallel for individual channels of a single DUART. It makes
that change a fix rather than a cleanup.
I chose to keep the revert anyway as it's a regression that oughtn't to
have happened in the first place, and it's a self-contained change, as is
the newly-added fix. This will help backporting the new fix too, should
someone wish to, although I don't think we need to rush pushing it to
-stable.
I've added a MAINTAINERS entry on top too, as previously stated. No
change except for the patch headings to patches 1/4 through 4/4, which
have become 1/6 through 4/6 now. The original cover letter follows.
Starting from commit 84a9582fd203 ("serial: core: Start managing serial
controllers to enable runtime PM") the driver for the SiByte onchip DUART
has stopped working due to a null pointer dereference in the serial core,
which means a kernel crash at bootstrap if the driver has been enabled, as
is usually the case for the system console.
This patch series addresses the issue by switching the driver away from
legacy probing to using platform devices. A notable consequence of this
is the serial console is only switched from the bootconsole handler that
uses firmware calls over to our serial driver at the time the driver is
being properly brought up. This causes messages to be produced to the
console between the device reset and console setup, which in turn causes
the firmware still being called via the bootconsole handler to loop
forever owing to the transmitter having been disabled.
Therefore introductory change 2/4 is included to fix the issue by doing a
rudimentary device setup right after reset, using parameters compatible
with those used by the firmware (115200n8). There is auxiliary change 1/4
included as well that prevents message corruption from happening at reset,
which becomes more prominent due to the change in timing, with the driver
switch to the platform device, of messages produced to the console.
Additionally there is a revert included as change 4/4 for a regression
introduced by an earlier change that caused previously good code to fail
requesting the control register resource shared between DUART channels,
and issue a warning to the kernel log. Not to be backported as strictly
speaking the driver works just fine despite the missing reservation.
See individual change descriptions for details. Verified with a SWARM
(BCM91250A) system.
Please apply.
Previous iterations:
- v1 at: <https://lore.kernel.org/r/alpine.DEB.2.21.2604130239560.29980@angie.orcam.me.uk/>.
Maciej
^ permalink raw reply
* [PATCH v2 1/6] serial: sb1250-duart: Fix console message clobbering at channel resets
From: Maciej W. Rozycki @ 2026-05-24 23:12 UTC (permalink / raw)
To: Thomas Bogendoerfer, Greg Kroah-Hartman, Jiri Slaby
Cc: Elena Reshetova, David Windsor, Kees Cook, Hans Liljestrand,
linux-mips, linux-serial, linux-kernel
In-Reply-To: <alpine.DEB.2.21.2605241602220.1450@angie.orcam.me.uk>
Ensure any characters outstanding have been sent before issuing channel
resets so as to prevent messages issued to the bootconsole from getting
clobbered.
Contrary to device documentation at the time the transmitter empty bit
is set only the transmit FIFO has been drained and there is still data
outstanding in the transmitter shift register, so wait an extra amount
of time for that register to drain too. This also prevents subsequent
messages produced to the console from getting clobbered, owing to what
seems a transmitter synchronisation issue.
When called from sbd_serial_console_init() it is too early for fsleep()
to work and even before lpj has been calculated and therefore the delay
is actually not sufficient for the transmitter to drain and is merely a
placeholder now. This will be addressed in a follow-up change.
Fixes: 84a9582fd203 ("serial: core: Start managing serial controllers to enable runtime PM")
Signed-off-by: Maciej W. Rozycki <macro@orcam.me.uk>
Cc: stable@vger.kernel.org # v6.5+
---
Changes from v1 (1/4),
<https://lore.kernel.org/r/alpine.DEB.2.21.2604130321540.29980@angie.orcam.me.uk/>:
- Sanitise the change heading.
---
drivers/tty/serial/sb1250-duart.c | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
linux-serial-sb1250-duart-reset-drain.diff
Index: linux-macro/drivers/tty/serial/sb1250-duart.c
===================================================================
--- linux-macro.orig/drivers/tty/serial/sb1250-duart.c
+++ linux-macro/drivers/tty/serial/sb1250-duart.c
@@ -516,6 +516,28 @@ static void sbd_init_port(struct sbd_por
if (sport->initialised)
return;
+ /*
+ * Contrary to documentation, which says that the transmitter
+ * empty bit is set when "there are no characters to send and
+ * the transmitter is idle," the bit is already set by hardware
+ * once the transmit FIFO has been drained only and while the
+ * transmitter shift register still holds data being supplied
+ * to the line. Consequently issuing a transmitter reset at
+ * this point causes the final character outstanding to be lost.
+ *
+ * Moreover, resetting the transmitter while transmission is
+ * in progress appears to make the transmitter go out of sync
+ * and subsequent characters issued after the transmitter has
+ * been reprogrammed and re-enabled are sent corrupted or with
+ * their bit patterns shifted.
+ *
+ * So once the transmitter empty bit has been set wait an extra
+ * amount of time, sufficient for the transmitter shift register
+ * to drain at 115200bps, which is the baud rate setting used by
+ * a standard CFE firmware compilation.
+ */
+ sbd_line_drain(sport);
+ udelay(100);
/* There is no DUART reset feature, so just set some sane defaults. */
write_sbdchn(sport, R_DUART_CMD, V_DUART_MISC_CMD_RESET_TX);
^ permalink raw reply
* [PATCH RFC] serial: core: Don't clobber the baud rate on hangup via B0
From: Maciej W. Rozycki @ 2026-05-24 23:14 UTC (permalink / raw)
To: Theodore Tso, Greg Kroah-Hartman, Jiri Slaby
Cc: Randy Dunlap, Dr. David Alan Gilbert, Krzysztof Kozlowski,
Gerhard Engleder, Jiayuan Chen, Joseph Tilahun, linux-serial,
linux-kernel
Requesting hangup via the B0 baud rate has this nasty side effect of
clobbering the previously set rate for the port unless it happens to be
9600bps. Consequently if there's a consumer still active at the other
end, then it receives garbled data from any other producers outputting
messages to the port, such as init(8) or the kernel console.
There doesn't appear to be any particular reason for this clobbering
other than that we have been doing it since 2.1.131, so take a saner
approach and try to retain the old baud rate where available before
resorting to 9600bps.
Signed-off-by: Maciej W. Rozycki <macro@orcam.me.uk>
---
Ted,
This code ultimately evolved from a patch of yours[1], so would you be
able by any chance to recreate your rationale beyond resetting the baud
rate to 9600bps rather than just keeping the old one for B0?
Otherwise, does anyone know or can come up with any actual use case for
this baud rate setting? There seems no sensible way to restore the baud
rate required by init(8) and the kernel console other than by hand, while
I can't see a way for software that actually issued a hangup request via
the B0 artificial baud rate to withdraw the request other than by choosing
another explicit baud rate. So there appears to me to be no usable way
there of relying on the implicit 9600bps baud rate set via the B0 baud
rate.
Have I missed anything?
I have dug into this having been irritated by the occasional clobbering
of init(8)/console output observed and now finding a reproducible way to
clobber the port in the course of working on a recent set of changes for
the sb1250-duart driver. This change has made messages get through with
no damage. Many of my systems in the lab actually use the baud rate of
9600bps, which covers the issue, but the CFE firmware sets the baud rate
to 115200bps for systems that use the sb1250-duart driver.
Maciej
References:
[1] "Was: patch to drivers/char/serial.c to fix kernel lock-up",
<https://lkml.org/lkml/1998/12/5/126>
---
drivers/tty/serial/serial_core.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
linux-serial-uart-get-baud-rate-hup.diff
Index: linux-macro/drivers/tty/serial/serial_core.c
===================================================================
--- linux-macro.orig/drivers/tty/serial/serial_core.c
+++ linux-macro/drivers/tty/serial/serial_core.c
@@ -461,8 +461,8 @@ EXPORT_SYMBOL(uart_update_timeout);
* @max: maximum acceptable baud rate
*
* Decode the termios structure into a numeric baud rate, taking account of the
- * magic 38400 baud rate (with spd_* flags), and mapping the %B0 rate to 9600
- * baud.
+ * magic 38400 baud rate (with spd_* flags), and mapping the %B0 rate to the
+ * rate provided by the @old termios setting if available, otherwise 9600 baud.
*
* If the new baud rate is invalid, try the @old termios setting. If it's still
* invalid, we try 9600 baud. If that is also invalid 0 is returned.
@@ -516,7 +516,7 @@ uart_get_baud_rate(struct uart_port *por
*/
if (baud == 0) {
hung_up = 1;
- baud = 9600;
+ baud = old ? tty_termios_baud_rate(old) : 9600;
}
if (baud >= min && baud <= max)
^ permalink raw reply
* [PATCH v2 6/6] MAINTAINERS: Add self for the sb1250-duart serial driver
From: Maciej W. Rozycki @ 2026-05-24 23:12 UTC (permalink / raw)
To: Thomas Bogendoerfer, Greg Kroah-Hartman, Jiri Slaby
Cc: Elena Reshetova, David Windsor, Kees Cook, Hans Liljestrand,
linux-mips, linux-serial, linux-kernel
In-Reply-To: <alpine.DEB.2.21.2605241602220.1450@angie.orcam.me.uk>
I wrote this driver and seem to have been the main remaining user now.
Signed-off-by: Maciej W. Rozycki <macro@orcam.me.uk>
---
New change in v2.
---
MAINTAINERS | 5 +++++
1 file changed, 5 insertions(+)
linux-serial-sb1250-duart-maintainers.diff
Index: linux-macro/MAINTAINERS
===================================================================
--- linux-macro.orig/MAINTAINERS
+++ linux-macro/MAINTAINERS
@@ -23658,6 +23658,11 @@ R: Marc Murphy <marc.murphy@sancloud.com
S: Supported
F: arch/arm/boot/dts/ti/omap/am335x-sancloud*
+SB1250-DUART BROADCOM SOC SERIAL DRIVER
+M: "Maciej W. Rozycki" <macro@orcam.me.uk>
+S: Maintained
+F: drivers/tty/serial/sb1250-duart.c
+
SC1200 WDT DRIVER
M: Zwane Mwaikambo <zwanem@gmail.com>
S: Maintained
^ permalink raw reply
* [PATCH v2 5/6] serial: sb1250-duart: Switch to spinlock protection for shared resource
From: Maciej W. Rozycki @ 2026-05-24 23:12 UTC (permalink / raw)
To: Thomas Bogendoerfer, Greg Kroah-Hartman, Jiri Slaby
Cc: Elena Reshetova, David Windsor, Kees Cook, Hans Liljestrand,
linux-mips, linux-serial, linux-kernel
In-Reply-To: <alpine.DEB.2.21.2605241602220.1450@angie.orcam.me.uk>
The control register block is shared between DUART channels and so its
resource has to be requested by the first channel claimed and released
by the last one dropped.
It is currently handled with an atomic counter, which however does not
protect against a situation where request_mem_region() has failed, but
another CPU has seen the map guard nonzero and refrained from calling
this function for another channel where it should have (and likely also
fail). This parallel execution scenario can in principle be arranged
via the TIOCSSERIAL ioctl.
Switch to using an ordinary counter then and spinlock protection for the
counter updates along with the corresponding resource request/release
calls, so that the case described above is covered.
Fixes: b45d52797432 ("sb1250-duart.c: SB1250 DUART serial support")
Signed-off-by: Maciej W. Rozycki <macro@orcam.me.uk>
---
New change in v2.
---
drivers/tty/serial/sb1250-duart.c | 28 ++++++++++++++++++----------
1 file changed, 18 insertions(+), 10 deletions(-)
linux-serial-sb1250-duart-map-guard-spinlock.diff
Index: linux-macro/drivers/tty/serial/sb1250-duart.c
===================================================================
--- linux-macro.orig/drivers/tty/serial/sb1250-duart.c
+++ linux-macro/drivers/tty/serial/sb1250-duart.c
@@ -86,7 +86,8 @@ struct sbd_port {
struct sbd_duart {
struct sbd_port sport[2];
unsigned long mapctrl;
- atomic_t map_guard;
+ spinlock_t map_lock;
+ int map_guard;
};
#define to_sport(uport) container_of(uport, struct sbd_port, port)
@@ -662,16 +663,18 @@ static void sbd_release_port(struct uart
{
struct sbd_port *sport = to_sport(uport);
struct sbd_duart *duart = sport->duart;
- int map_guard;
+ unsigned long flags;
iounmap(sport->memctrl);
sport->memctrl = NULL;
iounmap(uport->membase);
uport->membase = NULL;
- map_guard = atomic_add_return(-1, &duart->map_guard);
- if (!map_guard)
+ spin_lock_irqsave(&duart->map_lock, flags);
+ if (!--duart->map_guard)
release_mem_region(duart->mapctrl, DUART_CHANREG_SPACING);
+ spin_unlock_irqrestore(&duart->map_lock, flags);
+
release_mem_region(uport->mapbase, DUART_CHANREG_SPACING);
}
@@ -706,7 +709,7 @@ static int sbd_request_port(struct uart_
{
const char *err = KERN_ERR "sbd: Unable to reserve MMIO resource\n";
struct sbd_duart *duart = to_sport(uport)->duart;
- int map_guard;
+ unsigned long flags;
int ret = 0;
if (!request_mem_region(uport->mapbase, DUART_CHANREG_SPACING,
@@ -714,22 +717,26 @@ static int sbd_request_port(struct uart_
printk(err);
return -EBUSY;
}
- map_guard = atomic_add_return(1, &duart->map_guard);
- if (map_guard == 1) {
+
+ spin_lock_irqsave(&duart->map_lock, flags);
+ if (!duart->map_guard++) {
if (!request_mem_region(duart->mapctrl, DUART_CHANREG_SPACING,
"sb1250-duart")) {
- atomic_add(-1, &duart->map_guard);
+ --duart->map_guard;
printk(err);
ret = -EBUSY;
}
}
+ spin_unlock_irqrestore(&duart->map_lock, flags);
+
if (!ret) {
ret = sbd_map_port(uport);
if (ret) {
- map_guard = atomic_add_return(-1, &duart->map_guard);
- if (!map_guard)
+ spin_lock_irqsave(&duart->map_lock, flags);
+ if (!--duart->map_guard)
release_mem_region(duart->mapctrl,
DUART_CHANREG_SPACING);
+ spin_unlock_irqrestore(&duart->map_lock, flags);
}
}
if (ret) {
@@ -800,6 +807,7 @@ static int __init sbd_probe(struct platf
chip = pdev->id;
sbd_duarts[chip].mapctrl = mem_resource->start +
DUART_CHANREG_SPACING * 3;
+ spin_lock_init(&sbd_duarts[chip].map_lock);
for (side = 0; side < DUART_MAX_SIDE; side++) {
struct sbd_port *sport = &sbd_duarts[chip].sport[side];
struct uart_port *uport = &sport->port;
^ permalink raw reply
* [PATCH v2 4/6] Revert "drivers: convert sbd_duart.map_guard from atomic_t to refcount_t"
From: Maciej W. Rozycki @ 2026-05-24 23:12 UTC (permalink / raw)
To: Thomas Bogendoerfer, Greg Kroah-Hartman, Jiri Slaby
Cc: Elena Reshetova, David Windsor, Kees Cook, Hans Liljestrand,
linux-mips, linux-serial, linux-kernel
In-Reply-To: <alpine.DEB.2.21.2605241602220.1450@angie.orcam.me.uk>
Revert commit 22a33651a56f ("drivers: convert sbd_duart.map_guard from
atomic_t to refcount_t"), which broke perfectly valid code:
------------[ cut here ]------------
WARNING: CPU: 1 PID: 1 at lib/refcount.c:114 sbd_request_port+0x54/0x140
refcount_t: increment on 0; use-after-free.
CPU: 1 PID: 1 Comm: swapper/0 Not tainted 4.11.0-rc2+ #34
Stack : 0000000014001fe0 0000000000000000 ffffffff80830000 0000000000000000
ffffffff8127bc7a ffffffff8016fe08 ffffffff808d0000 ffffffff808d0000
ffffffff807aa828 ffffffff80822337 ffffffff808ce188 a8000001860b0000
0000000000000001 0000000000000001 00000000000001c8 ffffffff808a3090
00000000000000bb ffffffff801b09d4 a80000018609bb68 ffffffff801231cc
ffffffff812a0000 ffffffff80171388 0000000000001000 ffffffff807aa828
0000000000000001 0000000000000001 0000000000000000 0000000000000000
0000000000000000 a80000018609bab0 0000000000000000 ffffffff803c47cc
0000000000000000 0000000000000000 0000000000000000 0000000000000000
ffffffff807cb648 ffffffff8010bff8 0000000014001fe1 ffffffff803c47cc
...
Call Trace:
[<ffffffff8010bff8>] show_stack+0x28/0x88
[<ffffffff803c47cc>] dump_stack+0x8c/0xc0
[<ffffffff801aff5c>] __warn+0xe0/0x114
[<ffffffff801233f0>] warn_slowpath_fmt+0x40/0x50
[<ffffffff80455bcc>] sbd_request_port+0x54/0x140
[<ffffffff804563a4>] sbd_config_port+0x2c/0x68
---[ end trace f666d696412caa3e ]---
(report at the offending commit) -- sbd_request_port() is called twice
per DUART instance, to reserve a resource holding the control register
block shared between the two channels, so there's no slightest chance
for an overflow. Also this doesn't stop the driver from working and
it's just the reservation that is missing as a result, i.e.:
10060100-100601ff : sb1250-duart
10060200-100602ff : sb1250-duart
as from the offending change, vs:
10060100-100601ff : sb1250-duart
10060200-100602ff : sb1250-duart
10060300-100603ff : sb1250-duart
beforehand, which is surely why the breakage has gone so long unnoticed.
"If it ain't broke, don't fix it," so just revert the broken commit.
Fixes: 22a33651a56f ("drivers: convert sbd_duart.map_guard from atomic_t to refcount_t")
Signed-off-by: Maciej W. Rozycki <macro@orcam.me.uk>
---
No change from v1 (4/4),
<https://lore.kernel.org/r/alpine.DEB.2.21.2604130416440.29980@angie.orcam.me.uk/>.
---
drivers/tty/serial/sb1250-duart.c | 20 ++++++++++++--------
1 file changed, 12 insertions(+), 8 deletions(-)
linux-serial-sb1250-duart-map-guard-atomic.diff
Index: linux-macro/drivers/tty/serial/sb1250-duart.c
===================================================================
--- linux-macro.orig/drivers/tty/serial/sb1250-duart.c
+++ linux-macro/drivers/tty/serial/sb1250-duart.c
@@ -34,8 +34,8 @@
#include <linux/tty_flip.h>
#include <linux/types.h>
-#include <linux/refcount.h>
-#include <linux/io.h>
+#include <linux/atomic.h>
+#include <asm/io.h>
#include <asm/sibyte/sb1250.h>
#include <asm/sibyte/sb1250_uart.h>
@@ -86,7 +86,7 @@ struct sbd_port {
struct sbd_duart {
struct sbd_port sport[2];
unsigned long mapctrl;
- refcount_t map_guard;
+ atomic_t map_guard;
};
#define to_sport(uport) container_of(uport, struct sbd_port, port)
@@ -662,13 +662,15 @@ static void sbd_release_port(struct uart
{
struct sbd_port *sport = to_sport(uport);
struct sbd_duart *duart = sport->duart;
+ int map_guard;
iounmap(sport->memctrl);
sport->memctrl = NULL;
iounmap(uport->membase);
uport->membase = NULL;
- if(refcount_dec_and_test(&duart->map_guard))
+ map_guard = atomic_add_return(-1, &duart->map_guard);
+ if (!map_guard)
release_mem_region(duart->mapctrl, DUART_CHANREG_SPACING);
release_mem_region(uport->mapbase, DUART_CHANREG_SPACING);
}
@@ -704,6 +706,7 @@ static int sbd_request_port(struct uart_
{
const char *err = KERN_ERR "sbd: Unable to reserve MMIO resource\n";
struct sbd_duart *duart = to_sport(uport)->duart;
+ int map_guard;
int ret = 0;
if (!request_mem_region(uport->mapbase, DUART_CHANREG_SPACING,
@@ -711,11 +714,11 @@ static int sbd_request_port(struct uart_
printk(err);
return -EBUSY;
}
- refcount_inc(&duart->map_guard);
- if (refcount_read(&duart->map_guard) == 1) {
+ map_guard = atomic_add_return(1, &duart->map_guard);
+ if (map_guard == 1) {
if (!request_mem_region(duart->mapctrl, DUART_CHANREG_SPACING,
"sb1250-duart")) {
- refcount_dec(&duart->map_guard);
+ atomic_add(-1, &duart->map_guard);
printk(err);
ret = -EBUSY;
}
@@ -723,7 +726,8 @@ static int sbd_request_port(struct uart_
if (!ret) {
ret = sbd_map_port(uport);
if (ret) {
- if (refcount_dec_and_test(&duart->map_guard))
+ map_guard = atomic_add_return(-1, &duart->map_guard);
+ if (!map_guard)
release_mem_region(duart->mapctrl,
DUART_CHANREG_SPACING);
}
^ permalink raw reply
* [PATCH v2 3/6] serial: sb1250-duart: Convert to use a platform device
From: Maciej W. Rozycki @ 2026-05-24 23:12 UTC (permalink / raw)
To: Thomas Bogendoerfer, Greg Kroah-Hartman, Jiri Slaby
Cc: Elena Reshetova, David Windsor, Kees Cook, Hans Liljestrand,
linux-mips, linux-serial, linux-kernel
In-Reply-To: <alpine.DEB.2.21.2605241602220.1450@angie.orcam.me.uk>
Prevent a crash from happening as the first serial port is initialised:
pata-swarm: PATA interface at GenBus slot 4
workingset: timestamp_bits=62 max_order=18 bucket_order=0
Block layer SCSI generic (bsg) driver version 0.4 loaded (major 253)
CPU 1 Unable to handle kernel paging request at virtual address 0000000000000208, epc == ffffffff8067f8f8, ra == ffffffff80666330
Oops[#1]:
CPU: 1 UID: 0 PID: 1 Comm: swapper/0 Not tainted 6.19.0-dirty #27 NONE
$ 0 : 0000000000000000 0000000014001fe0 0000000000000020 ffffffff80666130
$ 4 : 0000000000000000 a800000100e6f118 ffffffff8112cbc0 0000000000000000
$ 8 : 0000000000000002 0000000000000000 0000000000000000 0000000001a80000
$12 : 0000000000000000 ffffffff809fd488 000000004ddf14dd ffffffff00000000
$16 : a800000100e6f000 0000000000000000 ffffffff8112c1d0 a800000100e6f000
$20 : 0000000000000000 00000000000004d0 0000000000000004 ffffffff8112c1d0
$24 : 0000000000000001 0000000000000003
$28 : a80000010007c000 a80000010007fcb0 00000000000000ef ffffffff80666330
Hi : fffffffffffffdb9
Lo : 0000000000000035
epc : ffffffff8067f8f8 __dev_fwnode+0x0/0x8
ra : ffffffff80666330 serial_base_ctrl_add+0xb8/0x180
Status: 14001fe3 KX SX UX KERNEL EXL IE
Cause : 80800008 (ExcCode 02)
BadVA : 0000000000000208
PrId : 03040102 (SiByte SB1)
Process swapper/0 (pid: 1, threadinfo=(____ptrval____), task=(____ptrval____), tls=0000000000000000)
Stack : 0000000000000000 ffffffff80cd5178 ffffffff80cd0000 ffffffff8112c1c8
0000000000000260 ffffffff806655c4 a800000100275bc0 ffffffff8064ac88
004000408112c000 0000000000000002 0000000000000000 ffffffff801965d0
a800000100786ba0 ffffffff80cd5178 a800000100786ba0 0000000000000004
a800000100275bc0 0000000000000000 0000000000000000 ffffffff80cd5178
0000000000000000 ffffffff8112c1c8 0000000000000260 00000000000004d0
0000000000000004 ffffffff80bf0000 00000000000000ef ffffffff80d171dc
ffffffff80d17120 ffffffff80d25658 0000000000000000 ffffffff80d50000
ffffffff80d2f928 ffffffff80d50000 ffffffff80d25698 ffffffff80cfcecc
00ffffff80b84428 0000000000000000 0000000000000006 0000000000000006
...
Call Trace:
[<ffffffff8067f8f8>] __dev_fwnode+0x0/0x8
[<ffffffff80666330>] serial_base_ctrl_add+0xb8/0x180
[<ffffffff806655c4>] serial_core_register_port+0x174/0x8e0
[<ffffffff80d171dc>] sbd_init+0xbc/0xf4
[<ffffffff80cfcecc>] do_one_initcall+0x64/0x150
[<ffffffff80cfd284>] kernel_init_freeable+0x25c/0x30c
[<ffffffff809ff6e4>] kernel_init+0x24/0x118
[<ffffffff80112d20>] ret_from_kernel_thread+0x14/0x1c
Code: 67bd0010 03e00008 2402ffea <03e00008> dc820208 67bdffa0 ffbe0050 ffb70048 ffb60040
---[ end trace 0000000000000000 ]---
-- where a pointer is dereferenced that has been derived from a null
pointer to the port's parent device.
Since no device is available with legacy probing and it's not anymore a
preferable way to discover devices anyway, switch the driver to using a
platform device and use it as the port's parent device.
Use platform_driver_probe() because SB1250 DUART devices are embedded
onchip the SoC and therefore not that straightforward to remove.
An unfortunate consequence of the switch to a platform device is we now
hand the console over from the bootconsole much later in the bootstrap.
The CFE console handler appears good enough though to work so late and
in particular with interrupts enabled.
Conversely only starting the console port so late lets the reset code
fully utilise our delay handlers, so switch from udelay() to fsleep()
for transmitter draining so as to avoid busy-waiting for an excessive
amount of time.
Since there is one way only remaining to reach sbd_init_port() now, drop
the port initialisation marker as no longer needed and go through the
channel resets unconditionally.
Fixes: 84a9582fd203 ("serial: core: Start managing serial controllers to enable runtime PM")
Signed-off-by: Maciej W. Rozycki <macro@orcam.me.uk>
Cc: stable@vger.kernel.org # needs to use .remove_new for <= 6.10
---
Changes from v1 (3/4),
<https://lore.kernel.org/r/alpine.DEB.2.21.2604130357420.29980@angie.orcam.me.uk/>:
- Sanitise the change heading.
---
arch/mips/sibyte/swarm/platform.c | 97 +++++++++++++++++++++++--
drivers/tty/serial/sb1250-duart.c | 147 +++++++++++++-------------------------
2 files changed, 145 insertions(+), 99 deletions(-)
linux-serial-sb1250-duart-platform.diff
Index: linux-macro/arch/mips/sibyte/swarm/platform.c
===================================================================
--- linux-macro.orig/arch/mips/sibyte/swarm/platform.c
+++ linux-macro/arch/mips/sibyte/swarm/platform.c
@@ -8,7 +8,13 @@
#include <asm/sibyte/board.h>
#include <asm/sibyte/sb1250_genbus.h>
+#if defined(CONFIG_SIBYTE_BCM1x80)
+#include <asm/sibyte/bcm1480_regs.h>
+#include <asm/sibyte/bcm1480_int.h>
+#else
#include <asm/sibyte/sb1250_regs.h>
+#include <asm/sibyte/sb1250_int.h>
+#endif
#if defined(CONFIG_SIBYTE_SWARM) || defined(CONFIG_SIBYTE_LITTLESUR)
@@ -85,6 +91,82 @@ device_initcall(swarm_pata_init);
#endif /* defined(CONFIG_SIBYTE_SWARM) || defined(CONFIG_SIBYTE_LITTLESUR) */
+#if defined(CONFIG_SIBYTE_BCM1x80)
+static struct resource sb1250_duart_resources[][2] = {
+ {
+ {
+ .name = "sb1250-duart0",
+ .flags = IORESOURCE_MEM,
+ .start = A_BCM1480_DUART0,
+ .end = (A_BCM1480_DUART0 +
+ 4 * BCM1480_DUART_CHANREG_SPACING - 1),
+ },
+ {
+ .name = "sb1250-duart0",
+ .flags = IORESOURCE_IRQ,
+ .start = K_BCM1480_INT_UART_0,
+ .end = K_BCM1480_INT_UART_1,
+ },
+ },
+ {
+ {
+ .name = "sb1250-duart1",
+ .flags = IORESOURCE_MEM,
+ .start = A_BCM1480_DUART1,
+ .end = (A_BCM1480_DUART1 +
+ 4 * BCM1480_DUART_CHANREG_SPACING - 1),
+ },
+ {
+ .name = "sb1250-duart1",
+ .flags = IORESOURCE_IRQ,
+ .start = K_BCM1480_INT_UART_2,
+ .end = K_BCM1480_INT_UART_3,
+ },
+ },
+};
+#else /* !defined(CONFIG_SIBYTE_BCM1x80) */
+static struct resource sb1250_duart_resources[][2] = {
+ {
+ {
+ .name = "sb1250-duart0",
+ .flags = IORESOURCE_MEM,
+ .start = A_DUART,
+ .end = A_DUART + 4 * DUART_CHANREG_SPACING - 1,
+ },
+ {
+ .name = "sb1250-duart0",
+ .flags = IORESOURCE_IRQ,
+ .start = K_INT_UART_0,
+ .end = K_INT_UART_1,
+ },
+ },
+};
+#endif /* !defined(CONFIG_SIBYTE_BCM1x80) */
+
+static struct platform_device sb1250_duart_device[] = {
+ {
+ .name = "sb1250-duart",
+ .id = 0,
+ .resource = sb1250_duart_resources[0],
+ .num_resources = ARRAY_SIZE(sb1250_duart_resources[0]),
+ },
+#if defined(CONFIG_SIBYTE_BCM1x80)
+ {
+ .name = "sb1250-duart",
+ .id = 1,
+ .resource = sb1250_duart_resources[1],
+ .num_resources = ARRAY_SIZE(sb1250_duart_resources[1]),
+ },
+#endif
+};
+
+static struct platform_device *sb1250_duart_devices[] __initdata = {
+ &sb1250_duart_device[0],
+#if defined(CONFIG_SIBYTE_BCM1x80)
+ &sb1250_duart_device[1],
+#endif
+};
+
#define sb1250_dev_struct(num) \
static struct resource sb1250_res##num = { \
.name = "SB1250 MAC " __stringify(num), \
@@ -113,28 +195,31 @@ static struct platform_device *sb1250_de
static int __init sb1250_device_init(void)
{
- int ret;
+ int ret1, ret2;
+
+ ret1 = platform_add_devices(sb1250_duart_devices,
+ ARRAY_SIZE(sb1250_duart_devices));
/* Set the number of available units based on the SOC type. */
switch (soc_type) {
case K_SYS_SOC_TYPE_BCM1250:
case K_SYS_SOC_TYPE_BCM1250_ALT:
- ret = platform_add_devices(sb1250_devs, 3);
+ ret2 = platform_add_devices(sb1250_devs, 3);
break;
case K_SYS_SOC_TYPE_BCM1120:
case K_SYS_SOC_TYPE_BCM1125:
case K_SYS_SOC_TYPE_BCM1125H:
case K_SYS_SOC_TYPE_BCM1250_ALT2: /* Hybrid */
- ret = platform_add_devices(sb1250_devs, 2);
+ ret2 = platform_add_devices(sb1250_devs, 2);
break;
case K_SYS_SOC_TYPE_BCM1x55:
case K_SYS_SOC_TYPE_BCM1x80:
- ret = platform_add_devices(sb1250_devs, 4);
+ ret2 = platform_add_devices(sb1250_devs, 4);
break;
default:
- ret = -ENODEV;
+ ret2 = 0;
break;
}
- return ret;
+ return ret1 ? ret1 : ret2;
}
device_initcall(sb1250_device_init);
Index: linux-macro/drivers/tty/serial/sb1250-duart.c
===================================================================
--- linux-macro.orig/drivers/tty/serial/sb1250-duart.c
+++ linux-macro/drivers/tty/serial/sb1250-duart.c
@@ -3,7 +3,7 @@
* Support for the asynchronous serial interface (DUART) included
* in the BCM1250 and derived System-On-a-Chip (SOC) devices.
*
- * Copyright (c) 2007 Maciej W. Rozycki
+ * Copyright (c) 2007, 2026 Maciej W. Rozycki
*
* Derived from drivers/char/sb1250_duart.c for which the following
* copyright applies:
@@ -25,6 +25,7 @@
#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/major.h>
+#include <linux/platform_device.h>
#include <linux/serial.h>
#include <linux/serial_core.h>
#include <linux/spinlock.h>
@@ -45,10 +46,6 @@
#include <asm/sibyte/bcm1480_regs.h>
#include <asm/sibyte/bcm1480_int.h>
-#define SBD_CHANREGS(line) A_BCM1480_DUART_CHANREG((line), 0)
-#define SBD_CTRLREGS(line) A_BCM1480_DUART_CTRLREG((line), 0)
-#define SBD_INT(line) (K_BCM1480_INT_UART_0 + (line))
-
#define DUART_CHANREG_SPACING BCM1480_DUART_CHANREG_SPACING
#define R_DUART_IMRREG(line) R_BCM1480_DUART_IMRREG(line)
@@ -59,10 +56,6 @@
#include <asm/sibyte/sb1250_regs.h>
#include <asm/sibyte/sb1250_int.h>
-#define SBD_CHANREGS(line) A_DUART_CHANREG((line), 0)
-#define SBD_CTRLREGS(line) A_DUART_CTRLREG(0)
-#define SBD_INT(line) (K_INT_UART_0 + (line))
-
#else
#error invalid SB1250 UART configuration
@@ -85,7 +78,6 @@ struct sbd_port {
struct uart_port port;
unsigned char __iomem *memctrl;
int tx_stopped;
- int initialised;
};
/*
@@ -100,6 +92,7 @@ struct sbd_duart {
#define to_sport(uport) container_of(uport, struct sbd_port, port)
static struct sbd_duart sbd_duarts[DUART_MAX_CHIP];
+static struct uart_driver sbd_reg;
/*
@@ -514,8 +507,6 @@ static void sbd_init_port(struct sbd_por
{
struct uart_port *uport = &sport->port;
- if (sport->initialised)
- return;
/*
* Contrary to documentation, which says that the transmitter
* empty bit is set when "there are no characters to send and
@@ -537,7 +528,7 @@ static void sbd_init_port(struct sbd_por
* a standard CFE firmware compilation.
*/
sbd_line_drain(sport);
- udelay(100);
+ fsleep(100);
/* There is no DUART reset feature, so just set some sane defaults. */
write_sbdchn(sport, R_DUART_CMD, V_DUART_MISC_CMD_RESET_TX);
@@ -554,8 +545,6 @@ static void sbd_init_port(struct sbd_por
/* Re-enable transmission for the initial PROM-based console. */
write_sbdchn(sport, R_DUART_CMD, M_DUART_TX_EN);
-
- sport->initialised = 1;
}
static void sbd_set_termios(struct uart_port *uport, struct ktermios *termios,
@@ -794,50 +783,54 @@ static const struct uart_ops sbd_ops = {
};
/* Initialize SB1250 DUART port structures. */
-static void __init sbd_probe_duarts(void)
+static int __init sbd_probe(struct platform_device *pdev)
{
- static int probed;
+ struct resource *mem_resource, *irq_resource;
int chip, side;
- int max_lines, line;
- if (probed)
- return;
+ mem_resource = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ irq_resource = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
+ if (!mem_resource || !irq_resource)
+ return -ENODEV;
- /* Set the number of available units based on the SOC type. */
- switch (soc_type) {
- case K_SYS_SOC_TYPE_BCM1x55:
- case K_SYS_SOC_TYPE_BCM1x80:
- max_lines = 4;
- break;
- default:
- /* Assume at least two serial ports at the normal address. */
- max_lines = 2;
- break;
- }
+ chip = pdev->id;
+ sbd_duarts[chip].mapctrl = mem_resource->start +
+ DUART_CHANREG_SPACING * 3;
+ for (side = 0; side < DUART_MAX_SIDE; side++) {
+ struct sbd_port *sport = &sbd_duarts[chip].sport[side];
+ struct uart_port *uport = &sport->port;
- probed = 1;
+ sport->duart = &sbd_duarts[chip];
- for (chip = 0, line = 0; chip < DUART_MAX_CHIP && line < max_lines;
- chip++) {
- sbd_duarts[chip].mapctrl = SBD_CTRLREGS(line);
+ uport->dev = &pdev->dev;
+ uport->irq = irq_resource->start + side;
+ uport->uartclk = 100000000 / 20 * 16;
+ uport->fifosize = 16;
+ uport->iotype = UPIO_MEM;
+ uport->flags = UPF_BOOT_AUTOCONF;
+ uport->ops = &sbd_ops;
+ uport->line = chip * DUART_MAX_SIDE + side;
+ uport->mapbase = mem_resource->start +
+ DUART_CHANREG_SPACING * (side + 1);
+ uport->has_sysrq = IS_ENABLED(CONFIG_SERIAL_SB1250_DUART_CONSOLE);
+ if (uart_add_one_port(&sbd_reg, uport))
+ uport->dev = NULL;
+ }
- for (side = 0; side < DUART_MAX_SIDE && line < max_lines;
- side++, line++) {
- struct sbd_port *sport = &sbd_duarts[chip].sport[side];
- struct uart_port *uport = &sport->port;
+ return 0;
+}
- sport->duart = &sbd_duarts[chip];
+static void __exit sbd_remove(struct platform_device *pdev)
+{
+ int chip, side;
- uport->irq = SBD_INT(line);
- uport->uartclk = 100000000 / 20 * 16;
- uport->fifosize = 16;
- uport->iotype = UPIO_MEM;
- uport->flags = UPF_BOOT_AUTOCONF;
- uport->ops = &sbd_ops;
- uport->line = line;
- uport->mapbase = SBD_CHANREGS(line);
- uport->has_sysrq = IS_ENABLED(CONFIG_SERIAL_SB1250_DUART_CONSOLE);
- }
+ chip = pdev->id;
+ for (side = DUART_MAX_SIDE - 1; side >= 0; side--) {
+ struct sbd_port *sport = &sbd_duarts[chip].sport[side];
+ struct uart_port *uport = &sport->port;
+
+ if (uport->dev)
+ uart_remove_one_port(&sbd_reg, uport);
}
}
@@ -895,23 +888,14 @@ static int __init sbd_console_setup(stru
int bits = 8;
int parity = 'n';
int flow = 'n';
- int ret;
if (!sport->duart)
return -ENXIO;
-
- ret = sbd_map_port(uport);
- if (ret)
- return ret;
-
- sbd_init_port(sport);
-
if (options)
uart_parse_options(options, &baud, &parity, &bits, &flow);
return uart_set_options(uport, co, baud, parity, bits, flow);
}
-static struct uart_driver sbd_reg;
static struct console sbd_console = {
.name = "duart",
.write = sbd_console_write,
@@ -922,16 +906,6 @@ static struct console sbd_console = {
.data = &sbd_reg
};
-static int __init sbd_serial_console_init(void)
-{
- sbd_probe_duarts();
- register_console(&sbd_console);
-
- return 0;
-}
-
-console_initcall(sbd_serial_console_init);
-
#define SERIAL_SB1250_DUART_CONSOLE &sbd_console
#else
#define SERIAL_SB1250_DUART_CONSOLE NULL
@@ -948,43 +922,30 @@ static struct uart_driver sbd_reg = {
.cons = SERIAL_SB1250_DUART_CONSOLE,
};
+static struct platform_driver sbd_driver = {
+ .remove = __exit_p(sbd_remove),
+ .driver = { .name = "sb1250-duart" },
+};
+
/* Set up the driver and register it. */
static int __init sbd_init(void)
{
- int i, ret;
-
- sbd_probe_duarts();
+ int ret;
ret = uart_register_driver(&sbd_reg);
if (ret)
return ret;
+ ret = platform_driver_probe(&sbd_driver, sbd_probe);
+ if (ret)
+ uart_unregister_driver(&sbd_reg);
- for (i = 0; i < DUART_MAX_CHIP * DUART_MAX_SIDE; i++) {
- struct sbd_duart *duart = &sbd_duarts[i / DUART_MAX_SIDE];
- struct sbd_port *sport = &duart->sport[i % DUART_MAX_SIDE];
- struct uart_port *uport = &sport->port;
-
- if (sport->duart)
- uart_add_one_port(&sbd_reg, uport);
- }
-
- return 0;
+ return ret;
}
/* Unload the driver. Unregister stuff, get ready to go away. */
static void __exit sbd_exit(void)
{
- int i;
-
- for (i = DUART_MAX_CHIP * DUART_MAX_SIDE - 1; i >= 0; i--) {
- struct sbd_duart *duart = &sbd_duarts[i / DUART_MAX_SIDE];
- struct sbd_port *sport = &duart->sport[i % DUART_MAX_SIDE];
- struct uart_port *uport = &sport->port;
-
- if (sport->duart)
- uart_remove_one_port(&sbd_reg, uport);
- }
-
+ platform_driver_unregister(&sbd_driver);
uart_unregister_driver(&sbd_reg);
}
^ permalink raw reply
* [PATCH v2 2/6] serial: sb1250-duart: Fix bootconsole handover lockup
From: Maciej W. Rozycki @ 2026-05-24 23:12 UTC (permalink / raw)
To: Thomas Bogendoerfer, Greg Kroah-Hartman, Jiri Slaby
Cc: Elena Reshetova, David Windsor, Kees Cook, Hans Liljestrand,
linux-mips, linux-serial, linux-kernel
In-Reply-To: <alpine.DEB.2.21.2605241602220.1450@angie.orcam.me.uk>
Calling sbd_init_port() in the course of setting up the serial device
causes line parameters to be messed up and the transmitter disabled.
We've been lucky in that no message is usually produced to the kernel
log between this call and the later call to uart_set_options() in the
course of console setup done by sbd_serial_console_init(), or the system
would hang as the console output handler in CFE tried to access a port
whose transmitter has been disabled and line parameters messed up.
It'll change with the next change to the driver, so fix sbd_init_port()
such that line parameters are set for 115200n8 console operation as with
the CFE firmware and the transmitter re-enabled after reset.
Fixes: 84a9582fd203 ("serial: core: Start managing serial controllers to enable runtime PM")
Signed-off-by: Maciej W. Rozycki <macro@orcam.me.uk>
Cc: stable@vger.kernel.org # v6.5+
---
Changes from v1 (2/4),
<https://lore.kernel.org/r/alpine.DEB.2.21.2604130338210.29980@angie.orcam.me.uk/>:
- Sanitise the change heading.
---
drivers/tty/serial/sb1250-duart.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
linux-serial-sb1250-duart-prom-console.diff
Index: linux-macro/drivers/tty/serial/sb1250-duart.c
===================================================================
--- linux-macro.orig/drivers/tty/serial/sb1250-duart.c
+++ linux-macro/drivers/tty/serial/sb1250-duart.c
@@ -542,14 +542,19 @@ static void sbd_init_port(struct sbd_por
/* There is no DUART reset feature, so just set some sane defaults. */
write_sbdchn(sport, R_DUART_CMD, V_DUART_MISC_CMD_RESET_TX);
write_sbdchn(sport, R_DUART_CMD, V_DUART_MISC_CMD_RESET_RX);
- write_sbdchn(sport, R_DUART_MODE_REG_1, V_DUART_BITS_PER_CHAR_8);
+ write_sbdchn(sport, R_DUART_MODE_REG_1,
+ V_DUART_PARITY_MODE_NONE | V_DUART_BITS_PER_CHAR_8);
write_sbdchn(sport, R_DUART_MODE_REG_2, 0);
+ write_sbdchn(sport, R_DUART_CLK_SEL, V_DUART_BAUD_RATE(115200));
write_sbdchn(sport, R_DUART_FULL_CTL,
V_DUART_INT_TIME(0) | V_DUART_SIG_FULL(15));
write_sbdchn(sport, R_DUART_OPCR_X, 0);
write_sbdchn(sport, R_DUART_AUXCTL_X, 0);
write_sbdshr(sport, R_DUART_IMRREG((uport->line) % 2), 0);
+ /* Re-enable transmission for the initial PROM-based console. */
+ write_sbdchn(sport, R_DUART_CMD, M_DUART_TX_EN);
+
sport->initialised = 1;
}
^ permalink raw reply
* [PATCH v2 2/2] ACPI: SPCR: Support UART clock frequency field
From: Markus Probst @ 2026-05-24 22:35 UTC (permalink / raw)
To: Rafael J. Wysocki, Len Brown, Geert Uytterhoeven,
Thomas Bogendoerfer, Ard Biesheuvel, Ilias Apalodimas,
Greg Kroah-Hartman, Jiri Slaby
Cc: linux-acpi, linux-kernel, linux-m68k, linux-mips, linux-efi,
linux-serial, Markus Probst
In-Reply-To: <20260525-acpi_spcr-v2-0-c042089d73ca@posteo.de>
The Microsoft Serial Port Console Redirection (SPCR) specification
revision 1.08 comprises additional field: UART Clock Frequency [1].
It contains a non-zero value indicating the UART clock frequency in Hz.
Link: https://learn.microsoft.com/en-us/windows-hardware/drivers/serports/serial-port-console-redirection-table [1]
Signed-off-by: Markus Probst <markus.probst@posteo.de>
---
drivers/acpi/spcr.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/acpi/spcr.c b/drivers/acpi/spcr.c
index cfacbe53f279..16f94073fde6 100644
--- a/drivers/acpi/spcr.c
+++ b/drivers/acpi/spcr.c
@@ -228,7 +228,7 @@ int __init acpi_parse_spcr(bool enable_earlycon, bool enable_console)
pr_info("console: %s\n", opts);
if (enable_earlycon)
- setup_earlycon(opts, 0);
+ setup_earlycon(opts, table->header.revision >= 3 ? table->uart_clk_freq : 0);
if (enable_console)
err = add_preferred_console(uart, 0, opts + strlen(uart) + 1);
--
2.53.0
^ permalink raw reply related
* [PATCH v2 1/2] serial: earlycon: add uart_clk_freq parameter
From: Markus Probst @ 2026-05-24 22:35 UTC (permalink / raw)
To: Rafael J. Wysocki, Len Brown, Geert Uytterhoeven,
Thomas Bogendoerfer, Ard Biesheuvel, Ilias Apalodimas,
Greg Kroah-Hartman, Jiri Slaby
Cc: linux-acpi, linux-kernel, linux-m68k, linux-mips, linux-efi,
linux-serial, Markus Probst
In-Reply-To: <20260525-acpi_spcr-v2-0-c042089d73ca@posteo.de>
Add `uart_clk_freq` parameter to `setup_earlycon`. This allows the
options string to be reused with `add_preferred_console`, while still
allowing to set the uart clock frequency. This will be used in the
following commit ("ACPI: SPCR: Support UART clock frequency field").
No logical change intended.
Signed-off-by: Markus Probst <markus.probst@posteo.de>
---
arch/m68k/virt/config.c | 2 +-
arch/mips/mti-malta/malta-init.c | 2 +-
drivers/acpi/spcr.c | 2 +-
drivers/firmware/efi/earlycon.c | 2 +-
drivers/tty/serial/earlycon.c | 17 ++++++++++++-----
include/linux/serial_core.h | 7 +++++--
6 files changed, 21 insertions(+), 11 deletions(-)
diff --git a/arch/m68k/virt/config.c b/arch/m68k/virt/config.c
index b338e2a8da6a..2c35ec15a51b 100644
--- a/arch/m68k/virt/config.c
+++ b/arch/m68k/virt/config.c
@@ -83,7 +83,7 @@ void __init config_virt(void)
snprintf(earlycon, sizeof(earlycon), "early_gf_tty,0x%08x",
virt_bi_data.tty.mmio);
- setup_earlycon(earlycon);
+ setup_earlycon(earlycon, 0);
mach_init_IRQ = virt_init_IRQ;
mach_sched_init = virt_sched_init;
diff --git a/arch/mips/mti-malta/malta-init.c b/arch/mips/mti-malta/malta-init.c
index 82b0fd8576a2..88ef17967ced 100644
--- a/arch/mips/mti-malta/malta-init.c
+++ b/arch/mips/mti-malta/malta-init.c
@@ -75,7 +75,7 @@ static void __init console_config(void)
if ((strstr(fw_getcmdline(), "earlycon=")) == NULL) {
sprintf(console_string, "uart8250,io,0x3f8,%d%c%c", baud,
parity, bits);
- setup_earlycon(console_string);
+ setup_earlycon(console_string, 0);
}
if ((strstr(fw_getcmdline(), "console=")) == NULL) {
diff --git a/drivers/acpi/spcr.c b/drivers/acpi/spcr.c
index 73cb933fdc89..cfacbe53f279 100644
--- a/drivers/acpi/spcr.c
+++ b/drivers/acpi/spcr.c
@@ -228,7 +228,7 @@ int __init acpi_parse_spcr(bool enable_earlycon, bool enable_console)
pr_info("console: %s\n", opts);
if (enable_earlycon)
- setup_earlycon(opts);
+ setup_earlycon(opts, 0);
if (enable_console)
err = add_preferred_console(uart, 0, opts + strlen(uart) + 1);
diff --git a/drivers/firmware/efi/earlycon.c b/drivers/firmware/efi/earlycon.c
index 3d060d59968c..0e3c2cb08966 100644
--- a/drivers/firmware/efi/earlycon.c
+++ b/drivers/firmware/efi/earlycon.c
@@ -221,7 +221,7 @@ static bool __initdata fb_probed;
void __init efi_earlycon_reprobe(void)
{
if (fb_probed)
- setup_earlycon("efifb");
+ setup_earlycon("efifb", 0);
}
static int __init efi_earlycon_setup(struct earlycon_device *device,
diff --git a/drivers/tty/serial/earlycon.c b/drivers/tty/serial/earlycon.c
index ab9af37f6cda..a419943e083b 100644
--- a/drivers/tty/serial/earlycon.c
+++ b/drivers/tty/serial/earlycon.c
@@ -135,11 +135,14 @@ static int __init parse_options(struct earlycon_device *device, char *options)
return 0;
}
-static int __init register_earlycon(char *buf, const struct earlycon_id *match)
+static int __init register_earlycon(char *buf, unsigned int uart_clk_freq,
+ const struct earlycon_id *match)
{
int err;
struct uart_port *port = &early_console_dev.port;
+ port->uartclk = uart_clk_freq;
+
/* On parsing error, pass the options buf to the setup function */
if (buf && !parse_options(&early_console_dev, buf))
buf = NULL;
@@ -164,7 +167,8 @@ static int __init register_earlycon(char *buf, const struct earlycon_id *match)
/**
* setup_earlycon - match and register earlycon console
- * @buf: earlycon param string
+ * @buf: earlycon param string
+ * @uart_clk_freq: uart clock frequency in Hz or 0 for BASE_BAUD*16
*
* Registers the earlycon console matching the earlycon specified
* in the param string @buf. Acceptable param strings are of the form
@@ -177,10 +181,13 @@ static int __init register_earlycon(char *buf, const struct earlycon_id *match)
* <options> string in the 'options' parameter; all other forms set
* the parameter to NULL.
*
+ * If the uart clock frequency is specified in the 'options' parameter,
+ * the value of the param @uart_clk_freq will be ignored.
+ *
* Returns 0 if an attempt to register the earlycon was made,
* otherwise negative error code
*/
-int __init setup_earlycon(char *buf)
+int __init setup_earlycon(char *buf, unsigned int uart_clk_freq)
{
const struct earlycon_id *match;
bool empty_compatible = true;
@@ -209,7 +216,7 @@ int __init setup_earlycon(char *buf)
} else
buf = NULL;
- return register_earlycon(buf, match);
+ return register_earlycon(buf, uart_clk_freq, match);
}
if (empty_compatible) {
@@ -241,7 +248,7 @@ static int __init param_setup_earlycon(char *buf)
}
}
- err = setup_earlycon(buf);
+ err = setup_earlycon(buf, 0);
if (err == -ENOENT || err == -EALREADY)
return 0;
return err;
diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
index 666430b47899..5c60fda9dd3a 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -1097,10 +1097,13 @@ int of_setup_earlycon(const struct earlycon_id *match, unsigned long node,
#ifdef CONFIG_SERIAL_EARLYCON
extern bool earlycon_acpi_spcr_enable __initdata;
-int setup_earlycon(char *buf);
+int setup_earlycon(char *buf, unsigned int uart_clk_freq);
#else
static const bool earlycon_acpi_spcr_enable EARLYCON_USED_OR_UNUSED;
-static inline int setup_earlycon(char *buf) { return 0; }
+static inline int setup_earlycon(char *buf, unsigned int uart_clk_freq)
+{
+ return 0;
+}
#endif
/* Variant of uart_console_registered() when the console_list_lock is held. */
--
2.53.0
^ permalink raw reply related
* [PATCH v2 0/2] ACPI: SPCR: Support UART clock frequency field
From: Markus Probst @ 2026-05-24 22:35 UTC (permalink / raw)
To: Rafael J. Wysocki, Len Brown, Geert Uytterhoeven,
Thomas Bogendoerfer, Ard Biesheuvel, Ilias Apalodimas,
Greg Kroah-Hartman, Jiri Slaby
Cc: linux-acpi, linux-kernel, linux-m68k, linux-mips, linux-efi,
linux-serial, Markus Probst
Support the uart clock frequency in the SPCR table.
See the commit messages for details.
Signed-off-by: Markus Probst <markus.probst@posteo.de>
---
Changes in v2:
- fix uart_clk_freq possibly being interpreted as parity/bits/flow
- Link to v1: https://patch.msgid.link/20260505-acpi_spcr-v1-1-fd4bc6f4eb53@posteo.de
---
Markus Probst (2):
serial: earlycon: add uart_clk_freq parameter
ACPI: SPCR: Support UART clock frequency field
arch/m68k/virt/config.c | 2 +-
arch/mips/mti-malta/malta-init.c | 2 +-
drivers/acpi/spcr.c | 2 +-
drivers/firmware/efi/earlycon.c | 2 +-
drivers/tty/serial/earlycon.c | 17 ++++++++++++-----
include/linux/serial_core.h | 7 +++++--
6 files changed, 21 insertions(+), 11 deletions(-)
---
base-commit: aa61612ab641d7d62b0b6889f2c7c9251489f6e3
change-id: 20260430-acpi_spcr-61902fd923f2
^ permalink raw reply
* Re: [PATCH] tty: serial: samsung: Remove redundant port lock acquisition in rx helpers
From: Krzysztof Kozlowski @ 2026-05-24 19:42 UTC (permalink / raw)
To: Tudor Ambarus, Alim Akhtar, Greg Kroah-Hartman, Jiri Slaby,
Ben Dooks
Cc: linux-arm-kernel, linux-samsung-soc, linux-kernel, linux-serial,
john.ogness, peter.griffin, andre.draszik, jyescas, kernel-team,
stable, John Ogness
In-Reply-To: <20260515-samsung-tty-flow-control-deadlock-v1-1-93255edbc9bc@linaro.org>
On 15/05/2026 14:41, Tudor Ambarus wrote:
> Sashiko identified a deadlock when the console flow is engaged [1].
>
> When console flow control is enabled (UPF_CONS_FLOW),
> s3c24xx_serial_stop_tx() calls s3c24xx_serial_rx_enable() and
> s3c24xx_serial_start_tx() calls s3c24xx_serial_rx_disable().
>
> The serial core framework invokes the .stop_tx() and .start_tx()
> callbacks with the port->lock spinlock already held. Furthermore, all
> internal driver paths that invoke stop_tx (such as the DMA TX
> completion handler s3c24xx_serial_tx_dma_complete() or the PIO TX IRQ
> handler s3c24xx_serial_tx_irq()) also acquire port->lock prior to
> calling it. (Note that s3c24xx_serial_start_tx() is only invoked by the
> serial core).
>
> However, s3c24xx_serial_rx_enable() and s3c24xx_serial_rx_disable()
> unconditionally attempt to acquire port->lock again using
> uart_port_lock_irqsave(). Since spinlocks are not recursive, this
> causes a deadlock on the same CPU when console flow control is engaged.
>
> Remove the redundant lock acquisition from both rx helper functions.
>
Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
Best regards,
Krzysztof
^ permalink raw reply
* [PATCH v1] tty: serial: rp2: Use named initializer for pci_device_id::driver_data
From: Uwe Kleine-König (The Capable Hub) @ 2026-05-24 13:19 UTC (permalink / raw)
To: Kevin Cernekee, Greg Kroah-Hartman, Jiri Slaby; +Cc: linux-serial, linux-kernel
The .driver_data member of the struct pci_device_id array were
initialized by list expressions relying on hidden assignment of .class
and .class_mask in PCI_VDEVICE().
Make the initialization more robust by using a named initializer. This
robustness is relevant for a planned change to struct pci_device_id that
replaces .driver_data by an anonymous union.
This change doesn't introduce changes to the compiled pci_device_id
array. Tested on x86 and arm64.
Signed-off-by: Uwe Kleine-König (The Capable Hub) <u.kleine-koenig@baylibre.com>
---
drivers/tty/serial/rp2.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/tty/serial/rp2.c b/drivers/tty/serial/rp2.c
index 6d99a02dd439..51e81ec0ffdb 100644
--- a/drivers/tty/serial/rp2.c
+++ b/drivers/tty/serial/rp2.c
@@ -197,7 +197,7 @@ struct rp2_card {
};
#define RP_ID(prod) PCI_VDEVICE(RP, (prod))
-#define RP_CAP(ports, smpte) (((ports) << 8) | ((smpte) << 0))
+#define RP_CAP(ports, smpte) .driver_data = (((ports) << 8) | ((smpte) << 0))
static inline void rp2_decode_cap(const struct pci_device_id *id,
int *ports, int *smpte)
base-commit: 254f49634ee16a731174d2ae34bc50bd5f45e731
--
2.47.3
^ permalink raw reply related
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