Linux Serial subsystem development
 help / color / mirror / Atom feed
* Re: [PATCH] tty: n_tty: fix KCSAN data-race in n_tty_flush_buffer / n_tty_lookahead_flow_ctrl
From: Osama Abdelkader @ 2026-03-13 23:16 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Jiri Slaby, Andy Shevchenko, Ilpo Järvinen, linux-kernel,
	linux-serial, syzbot+80806cf7508e92c7cc86
In-Reply-To: <2026031211-scared-riches-6a52@gregkh>

On Thu, Mar 12, 2026 at 03:21:03PM +0100, Greg Kroah-Hartman wrote:
> On Wed, Feb 11, 2026 at 10:08:38PM +0100, Osama Abdelkader wrote:
> > n_tty_lookahead_flow_ctrl() accesses ldata->lookahead_count without
> > holding termios_rwsem, while reset_buffer_flags() in n_tty_flush_buffer()
> > resets it with exclusive termios_rwsem held. This causes a data race
> > reported by KCSAN when a PTY is closed while flush_to_ldisc is still
> > processing lookahead data.
> 
> A data race of what exactly?  lookahead_count?

yes, ldata->lookahead_count.

> 
> > Fix by taking termios_rwsem (read) in n_tty_lookahead_flow_ctrl(),
> > consistent with __receive_buf() which also modifies lookahead_count
> > under the read lock.
> 
> This feels wrong.  I would like to see a LOT of testing and validation
> that this is correct before being able to take this patch.  How was that
> done?
>

To clarify the reported race:

The race is on `ldata->lookahead_count` between:

1. `n_tty_lookahead_flow_ctrl()`, which does `lookahead_count += count`
   from the `flush_to_ldisc()` workqueue path:

   `flush_to_ldisc() -> lookahead_bufs() -> tty_port_default_lookahead_buf() -> n_tty_lookahead_flow_ctrl()`

2. `reset_buffer_flags()`, which does `lookahead_count = 0`
   from `n_tty_flush_buffer()` under write `termios_rwsem`
   (for example during PTY close / hangup).

`__receive_buf()` already accesses the same field under read `termios_rwsem`,
so this lookahead path was the remaining unlocked access to that state.

Because `n_tty_lookahead_flow_ctrl()` performs a read-modify-write (`+= count`),
`READ_ONCE()/WRITE_ONCE()` would still allow lost updates, so taking read
`termios_rwsem` there matches the existing writer-side protection.

I also ran the change with lockdep enabled and did not observe lockdep warnings
on PTY open/close stress.
 
> thanks,
> 
> greg k-h

Thanks,
Osama

^ permalink raw reply

* Re: [PATCH 1/2] dt-bindings: serial: renesas,rsci: Document RZ/G3L SoC
From: Rob Herring (Arm) @ 2026-03-14  0:22 UTC (permalink / raw)
  To: Biju
  Cc: Biju Das, Magnus Damm, linux-kernel, Krzysztof Kozlowski,
	linux-renesas-soc, Geert Uytterhoeven, linux-serial, Conor Dooley,
	Greg Kroah-Hartman, Jiri Slaby, devicetree, Lad Prabhakar
In-Reply-To: <20260312082708.98835-2-biju.das.jz@bp.renesas.com>


On Thu, 12 Mar 2026 08:26:58 +0000, Biju wrote:
> From: Biju Das <biju.das.jz@bp.renesas.com>
> 
> Document the serial communication interface (RSCI) used on the Renesas
> RZ/G3L (R9A08G046) SoC. This SoC integrates the same RSCI IP block as
> the RZ/G3E (R9A09G047), but it has 3 clocks compared to 6 clocks on
> the RZ/G3E SoC. The RZ/G3L has a single TCLK with internal dividers,
> whereas the RZ/G3E has explicit clocks for TCLK and its dividers.
> 
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> ---
>  .../bindings/serial/renesas,rsci.yaml         | 26 +++++++++++++++++++
>  1 file changed, 26 insertions(+)
> 

Reviewed-by: Rob Herring (Arm) <robh@kernel.org>


^ permalink raw reply

* Re: [PATCH v6 2/6] dt-bindings: rng: atmel,at91-trng: add microchip,lan9691-trng
From: Herbert Xu @ 2026-03-14  5:06 UTC (permalink / raw)
  To: Robert Marko
  Cc: robh, krzk+dt, conor+dt, nicolas.ferre, alexandre.belloni,
	claudiu.beznea, olivia, radu_nicolae.pirea, richard.genoud,
	gregkh, jirislaby, horatiu.vultur, Ryan.Wanner, devicetree,
	linux-arm-kernel, linux-kernel, linux-crypto, linux-spi,
	linux-serial, daniel.machon, luka.perkov, Conor Dooley
In-Reply-To: <20260302112153.464422-3-robert.marko@sartura.hr>

On Mon, Mar 02, 2026 at 12:20:10PM +0100, Robert Marko wrote:
> Document Microchip LAN969X TRNG compatible.
> 
> Signed-off-by: Robert Marko <robert.marko@sartura.hr>
> Acked-by: Conor Dooley <conor.dooley@microchip.com>
> Reviewed-by: Claudiu Beznea <claudiu.beznea@tuxon.dev>
> ---
> Changes in v5:
> * Pick Reviewed-by from Claudiu
> 
> Changes in v3:
> * Pick Acked-by from Conor
> 
>  Documentation/devicetree/bindings/rng/atmel,at91-trng.yaml | 1 +
>  1 file changed, 1 insertion(+)

Patch applied.  Thanks.
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply

* Re: [PATCH v3 2/4] serdev: add rust private data to serdev_device
From: Greg Kroah-Hartman @ 2026-03-14  8:07 UTC (permalink / raw)
  To: Markus Probst
  Cc: Rob Herring, Jiri Slaby, Miguel Ojeda, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, Danilo Krummrich, Kari Argillander,
	Rafael J. Wysocki, Viresh Kumar, Boqun Feng, David Airlie,
	Simona Vetter, linux-serial, linux-kernel, rust-for-linux,
	linux-pm, driver-core, dri-devel
In-Reply-To: <20260313-rust_serdev-v3-2-c9a3af214f7f@posteo.de>

On Fri, Mar 13, 2026 at 06:12:31PM +0000, Markus Probst wrote:
> Add rust private data to `struct serdev_device`, as it is required by the
> rust abstraction added in the following commit
> (rust: add basic serial device bus abstractions).

why is rust "special" here?  What's wrong with the existing private
pointer in this structure?  Why must we add another one?

thanks,

greg k-h

^ permalink raw reply

* Re: [PATCH v3 2/4] serdev: add rust private data to serdev_device
From: Markus Probst @ 2026-03-14 11:42 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Rob Herring, Jiri Slaby, Miguel Ojeda, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, Danilo Krummrich, Kari Argillander,
	Rafael J. Wysocki, Viresh Kumar, Boqun Feng, David Airlie,
	Simona Vetter, linux-serial, linux-kernel, rust-for-linux,
	linux-pm, driver-core, dri-devel
In-Reply-To: <2026031402-absence-graph-af5d@gregkh>

[-- Attachment #1: Type: text/plain, Size: 1439 bytes --]

On Sat, 2026-03-14 at 09:07 +0100, Greg Kroah-Hartman wrote:
> On Fri, Mar 13, 2026 at 06:12:31PM +0000, Markus Probst wrote:
> > Add rust private data to `struct serdev_device`, as it is required by the
> > rust abstraction added in the following commit
> > (rust: add basic serial device bus abstractions).
> 
> why is rust "special" here?  What's wrong with the existing private
> pointer in this structure?  Why must we add another one?
Because in rust, the device drvdata will be set after probe has run. In
serdev, once the device has been opened, it can receive data. It must
be opened either inside probe or before probe, because it can only be
configured (baudrate, flow control etc.) and data written to after it
has been opened. Because it can receive data before drvdata has been
set yet, we need to ensure it waits on data receival for the probe to
be finished. Otherwise this would be a null pointer dereference. To do
this, we need to store a `Completion` for it to wait and a `bool` in
case the probe exits with an error. We cannot store this data in the
device drvdata, because this is where the drivers drvdata goes. We also
cannot create a wrapper of the drivers drvdata, because
`Device::drvdata::<T>()` would always fail in that case. That is why we
need a "rust_private_data" for this abstraction to store the
`Completion` and `bool`.

Thanks
- Markus Probst

> 
> thanks,
> 
> greg k-h

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 870 bytes --]

^ permalink raw reply

* Re: [PATCH v3 2/4] serdev: add rust private data to serdev_device
From: Greg Kroah-Hartman @ 2026-03-14 11:52 UTC (permalink / raw)
  To: Markus Probst
  Cc: Rob Herring, Jiri Slaby, Miguel Ojeda, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, Danilo Krummrich, Kari Argillander,
	Rafael J. Wysocki, Viresh Kumar, Boqun Feng, David Airlie,
	Simona Vetter, linux-serial, linux-kernel, rust-for-linux,
	linux-pm, driver-core, dri-devel
In-Reply-To: <a9618d034bd48ec21bd26060573770a548619402.camel@posteo.de>

On Sat, Mar 14, 2026 at 11:42:02AM +0000, Markus Probst wrote:
> On Sat, 2026-03-14 at 09:07 +0100, Greg Kroah-Hartman wrote:
> > On Fri, Mar 13, 2026 at 06:12:31PM +0000, Markus Probst wrote:
> > > Add rust private data to `struct serdev_device`, as it is required by the
> > > rust abstraction added in the following commit
> > > (rust: add basic serial device bus abstractions).
> > 
> > why is rust "special" here?  What's wrong with the existing private
> > pointer in this structure?  Why must we add another one?
> Because in rust, the device drvdata will be set after probe has run. In
> serdev, once the device has been opened, it can receive data. It must
> be opened either inside probe or before probe, because it can only be
> configured (baudrate, flow control etc.) and data written to after it
> has been opened. Because it can receive data before drvdata has been
> set yet, we need to ensure it waits on data receival for the probe to
> be finished. Otherwise this would be a null pointer dereference. To do
> this, we need to store a `Completion` for it to wait and a `bool` in
> case the probe exits with an error. We cannot store this data in the
> device drvdata, because this is where the drivers drvdata goes. We also
> cannot create a wrapper of the drivers drvdata, because
> `Device::drvdata::<T>()` would always fail in that case. That is why we
> need a "rust_private_data" for this abstraction to store the
> `Completion` and `bool`.

So why is this any different from any other bus type?  I don't see the
"uniqueness" here that has not required this to happen for PCI or USB or
anything else.

What am I missing?

Also, all of this information MUST be in the changelog text in order for
us to be able to accept it.  You need to say _why_ a change is needed,
not just _what_ the change does, as you know.

thanks,

greg k-h

^ permalink raw reply

* Re: [PATCH v3 2/4] serdev: add rust private data to serdev_device
From: Markus Probst @ 2026-03-14 12:08 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Rob Herring, Jiri Slaby, Miguel Ojeda, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, Danilo Krummrich, Kari Argillander,
	Rafael J. Wysocki, Viresh Kumar, Boqun Feng, David Airlie,
	Simona Vetter, linux-serial, linux-kernel, rust-for-linux,
	linux-pm, driver-core, dri-devel
In-Reply-To: <2026031422-shaded-matchbook-5078@gregkh>

[-- Attachment #1: Type: text/plain, Size: 2220 bytes --]

On Sat, 2026-03-14 at 12:52 +0100, Greg Kroah-Hartman wrote:
> On Sat, Mar 14, 2026 at 11:42:02AM +0000, Markus Probst wrote:
> > On Sat, 2026-03-14 at 09:07 +0100, Greg Kroah-Hartman wrote:
> > > On Fri, Mar 13, 2026 at 06:12:31PM +0000, Markus Probst wrote:
> > > > Add rust private data to `struct serdev_device`, as it is required by the
> > > > rust abstraction added in the following commit
> > > > (rust: add basic serial device bus abstractions).
> > > 
> > > why is rust "special" here?  What's wrong with the existing private
> > > pointer in this structure?  Why must we add another one?
> > Because in rust, the device drvdata will be set after probe has run. In
> > serdev, once the device has been opened, it can receive data. It must
> > be opened either inside probe or before probe, because it can only be
> > configured (baudrate, flow control etc.) and data written to after it
> > has been opened. Because it can receive data before drvdata has been
> > set yet, we need to ensure it waits on data receival for the probe to
> > be finished. Otherwise this would be a null pointer dereference. To do
> > this, we need to store a `Completion` for it to wait and a `bool` in
> > case the probe exits with an error. We cannot store this data in the
> > device drvdata, because this is where the drivers drvdata goes. We also
> > cannot create a wrapper of the drivers drvdata, because
> > `Device::drvdata::<T>()` would always fail in that case. That is why we
> > need a "rust_private_data" for this abstraction to store the
> > `Completion` and `bool`.
> 
> So why is this any different from any other bus type?  I don't see the
> "uniqueness" here that has not required this to happen for PCI or USB or
> anything else.
> 
> What am I missing?
In Short:
In serdev, we have to handle incoming device data (serdev calls on a
function pointer we provide in advance), even in the case that the
driver hasn't completed probe yet.

> 
> Also, all of this information MUST be in the changelog text in order for
> us to be able to accept it.  You need to say _why_ a change is needed,
> not just _what_ the change does, as you know.
Will do.

Thanks
- Markus Probst

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 870 bytes --]

^ permalink raw reply

* Re: [PATCH v3 2/4] serdev: add rust private data to serdev_device
From: Alice Ryhl @ 2026-03-14 13:24 UTC (permalink / raw)
  To: Markus Probst
  Cc: Greg Kroah-Hartman, Rob Herring, Jiri Slaby, Miguel Ojeda,
	Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
	Trevor Gross, Danilo Krummrich, Kari Argillander,
	Rafael J. Wysocki, Viresh Kumar, Boqun Feng, David Airlie,
	Simona Vetter, linux-serial, linux-kernel, rust-for-linux,
	linux-pm, driver-core, dri-devel
In-Reply-To: <fa25ed4ff4688a40bd601d695804def772e47af9.camel@posteo.de>

On Sat, Mar 14, 2026 at 12:08:09PM +0000, Markus Probst wrote:
> On Sat, 2026-03-14 at 12:52 +0100, Greg Kroah-Hartman wrote:
> > On Sat, Mar 14, 2026 at 11:42:02AM +0000, Markus Probst wrote:
> > > On Sat, 2026-03-14 at 09:07 +0100, Greg Kroah-Hartman wrote:
> > > > On Fri, Mar 13, 2026 at 06:12:31PM +0000, Markus Probst wrote:
> > > > > Add rust private data to `struct serdev_device`, as it is required by the
> > > > > rust abstraction added in the following commit
> > > > > (rust: add basic serial device bus abstractions).
> > > > 
> > > > why is rust "special" here?  What's wrong with the existing private
> > > > pointer in this structure?  Why must we add another one?
> > > Because in rust, the device drvdata will be set after probe has run. In
> > > serdev, once the device has been opened, it can receive data. It must
> > > be opened either inside probe or before probe, because it can only be
> > > configured (baudrate, flow control etc.) and data written to after it
> > > has been opened. Because it can receive data before drvdata has been
> > > set yet, we need to ensure it waits on data receival for the probe to
> > > be finished. Otherwise this would be a null pointer dereference. To do
> > > this, we need to store a `Completion` for it to wait and a `bool` in
> > > case the probe exits with an error. We cannot store this data in the
> > > device drvdata, because this is where the drivers drvdata goes. We also
> > > cannot create a wrapper of the drivers drvdata, because
> > > `Device::drvdata::<T>()` would always fail in that case. That is why we
> > > need a "rust_private_data" for this abstraction to store the
> > > `Completion` and `bool`.
> > 
> > So why is this any different from any other bus type?  I don't see the
> > "uniqueness" here that has not required this to happen for PCI or USB or
> > anything else.
> > 
> > What am I missing?
> In Short:
> In serdev, we have to handle incoming device data (serdev calls on a
> function pointer we provide in advance), even in the case that the
> driver hasn't completed probe yet.

Why can the function pointers be called during probe?

Alice

^ permalink raw reply

* Re: [PATCH v3 2/4] serdev: add rust private data to serdev_device
From: Greg Kroah-Hartman @ 2026-03-14 13:31 UTC (permalink / raw)
  To: Markus Probst
  Cc: Rob Herring, Jiri Slaby, Miguel Ojeda, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, Danilo Krummrich, Kari Argillander,
	Rafael J. Wysocki, Viresh Kumar, Boqun Feng, David Airlie,
	Simona Vetter, linux-serial, linux-kernel, rust-for-linux,
	linux-pm, driver-core, dri-devel
In-Reply-To: <fa25ed4ff4688a40bd601d695804def772e47af9.camel@posteo.de>

On Sat, Mar 14, 2026 at 12:08:09PM +0000, Markus Probst wrote:
> On Sat, 2026-03-14 at 12:52 +0100, Greg Kroah-Hartman wrote:
> > On Sat, Mar 14, 2026 at 11:42:02AM +0000, Markus Probst wrote:
> > > On Sat, 2026-03-14 at 09:07 +0100, Greg Kroah-Hartman wrote:
> > > > On Fri, Mar 13, 2026 at 06:12:31PM +0000, Markus Probst wrote:
> > > > > Add rust private data to `struct serdev_device`, as it is required by the
> > > > > rust abstraction added in the following commit
> > > > > (rust: add basic serial device bus abstractions).
> > > > 
> > > > why is rust "special" here?  What's wrong with the existing private
> > > > pointer in this structure?  Why must we add another one?
> > > Because in rust, the device drvdata will be set after probe has run. In
> > > serdev, once the device has been opened, it can receive data. It must
> > > be opened either inside probe or before probe, because it can only be
> > > configured (baudrate, flow control etc.) and data written to after it
> > > has been opened. Because it can receive data before drvdata has been
> > > set yet, we need to ensure it waits on data receival for the probe to
> > > be finished. Otherwise this would be a null pointer dereference. To do
> > > this, we need to store a `Completion` for it to wait and a `bool` in
> > > case the probe exits with an error. We cannot store this data in the
> > > device drvdata, because this is where the drivers drvdata goes. We also
> > > cannot create a wrapper of the drivers drvdata, because
> > > `Device::drvdata::<T>()` would always fail in that case. That is why we
> > > need a "rust_private_data" for this abstraction to store the
> > > `Completion` and `bool`.
> > 
> > So why is this any different from any other bus type?  I don't see the
> > "uniqueness" here that has not required this to happen for PCI or USB or
> > anything else.
> > 
> > What am I missing?
> In Short:
> In serdev, we have to handle incoming device data (serdev calls on a
> function pointer we provide in advance), even in the case that the
> driver hasn't completed probe yet.

But how is that any different from a USB or PCI driver doing the same
thing?  Why is serdev so unique here?  What specific serdev function
causes this and why isn't it an issue with the C api?  Can we change the
C code to not require this?

thanks,

greg k-h

^ permalink raw reply

* Re: [PATCH v3 2/4] serdev: add rust private data to serdev_device
From: Danilo Krummrich @ 2026-03-14 13:42 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Markus Probst, Rob Herring, Jiri Slaby, Miguel Ojeda, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, Kari Argillander, Rafael J. Wysocki, Viresh Kumar,
	Boqun Feng, David Airlie, Simona Vetter, linux-serial,
	linux-kernel, rust-for-linux, linux-pm, driver-core, dri-devel
In-Reply-To: <2026031447-margarita-untamed-e976@gregkh>

On Sat Mar 14, 2026 at 2:31 PM CET, Greg Kroah-Hartman wrote:
> On Sat, Mar 14, 2026 at 12:08:09PM +0000, Markus Probst wrote:
>> On Sat, 2026-03-14 at 12:52 +0100, Greg Kroah-Hartman wrote:
>> > On Sat, Mar 14, 2026 at 11:42:02AM +0000, Markus Probst wrote:
>> > > On Sat, 2026-03-14 at 09:07 +0100, Greg Kroah-Hartman wrote:
>> > > > On Fri, Mar 13, 2026 at 06:12:31PM +0000, Markus Probst wrote:
>> > > > > Add rust private data to `struct serdev_device`, as it is required by the
>> > > > > rust abstraction added in the following commit
>> > > > > (rust: add basic serial device bus abstractions).
>> > > > 
>> > > > why is rust "special" here?  What's wrong with the existing private
>> > > > pointer in this structure?  Why must we add another one?
>> > > Because in rust, the device drvdata will be set after probe has run. In
>> > > serdev, once the device has been opened, it can receive data. It must
>> > > be opened either inside probe or before probe, because it can only be
>> > > configured (baudrate, flow control etc.) and data written to after it
>> > > has been opened. Because it can receive data before drvdata has been
>> > > set yet, we need to ensure it waits on data receival for the probe to
>> > > be finished. Otherwise this would be a null pointer dereference. To do
>> > > this, we need to store a `Completion` for it to wait and a `bool` in
>> > > case the probe exits with an error. We cannot store this data in the
>> > > device drvdata, because this is where the drivers drvdata goes. We also
>> > > cannot create a wrapper of the drivers drvdata, because
>> > > `Device::drvdata::<T>()` would always fail in that case. That is why we
>> > > need a "rust_private_data" for this abstraction to store the
>> > > `Completion` and `bool`.
>> > 
>> > So why is this any different from any other bus type?  I don't see the
>> > "uniqueness" here that has not required this to happen for PCI or USB or
>> > anything else.
>> > 
>> > What am I missing?
>> In Short:
>> In serdev, we have to handle incoming device data (serdev calls on a
>> function pointer we provide in advance), even in the case that the
>> driver hasn't completed probe yet.
>
> But how is that any different from a USB or PCI driver doing the same
> thing?  Why is serdev so unique here?  What specific serdev function
> causes this and why isn't it an issue with the C api?  Can we change the
> C code to not require this?

I think the idea is to avoid bugs as in the mhz19b driver [1].

This driver's probe() looks like this:


	serdev_device_set_client_ops(serdev, &mhz19b_ops);
	ret = devm_serdev_device_open(dev, serdev);

	// Lots of other initialization.

	serdev_device_set_drvdata(serdev, indio_dev);

But the receive_buf() callback from mhz19b_ops dereferences the driver's private
data.

Now, maybe this is actually prevented to become an actual race, since some
regulator is only enabled subsequently:

	devm_regulator_get_enable(dev, "vin");

But in any case in Rust it would be unsound as with this a driver could easily
cause undefined behavior with safe APIs.

Maybe it is as simple as letting the abstraction call serdev_device_open() only
after the driver's probe() has completed, but maybe there are reasons why that
is not an option, that's a serdev question.

[1] https://elixir.bootlin.com/linux/v6.19.7/source/drivers/iio/chemical/mhz19b.c#L260

^ permalink raw reply

* Re: [PATCH v3 2/4] serdev: add rust private data to serdev_device
From: Markus Probst @ 2026-03-14 13:49 UTC (permalink / raw)
  To: Danilo Krummrich, Greg Kroah-Hartman
  Cc: Rob Herring, Jiri Slaby, Miguel Ojeda, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, Kari Argillander, Rafael J. Wysocki, Viresh Kumar,
	Boqun Feng, David Airlie, Simona Vetter, linux-serial,
	linux-kernel, rust-for-linux, linux-pm, driver-core, dri-devel
In-Reply-To: <DH2JS8YL1NXY.VRKLBSTZ5ZYI@kernel.org>

[-- Attachment #1: Type: text/plain, Size: 4000 bytes --]

On Sat, 2026-03-14 at 14:42 +0100, Danilo Krummrich wrote:
> On Sat Mar 14, 2026 at 2:31 PM CET, Greg Kroah-Hartman wrote:
> > On Sat, Mar 14, 2026 at 12:08:09PM +0000, Markus Probst wrote:
> > > On Sat, 2026-03-14 at 12:52 +0100, Greg Kroah-Hartman wrote:
> > > > On Sat, Mar 14, 2026 at 11:42:02AM +0000, Markus Probst wrote:
> > > > > On Sat, 2026-03-14 at 09:07 +0100, Greg Kroah-Hartman wrote:
> > > > > > On Fri, Mar 13, 2026 at 06:12:31PM +0000, Markus Probst wrote:
> > > > > > > Add rust private data to `struct serdev_device`, as it is required by the
> > > > > > > rust abstraction added in the following commit
> > > > > > > (rust: add basic serial device bus abstractions).
> > > > > > 
> > > > > > why is rust "special" here?  What's wrong with the existing private
> > > > > > pointer in this structure?  Why must we add another one?
> > > > > Because in rust, the device drvdata will be set after probe has run. In
> > > > > serdev, once the device has been opened, it can receive data. It must
> > > > > be opened either inside probe or before probe, because it can only be
> > > > > configured (baudrate, flow control etc.) and data written to after it
> > > > > has been opened. Because it can receive data before drvdata has been
> > > > > set yet, we need to ensure it waits on data receival for the probe to
> > > > > be finished. Otherwise this would be a null pointer dereference. To do
> > > > > this, we need to store a `Completion` for it to wait and a `bool` in
> > > > > case the probe exits with an error. We cannot store this data in the
> > > > > device drvdata, because this is where the drivers drvdata goes. We also
> > > > > cannot create a wrapper of the drivers drvdata, because
> > > > > `Device::drvdata::<T>()` would always fail in that case. That is why we
> > > > > need a "rust_private_data" for this abstraction to store the
> > > > > `Completion` and `bool`.
> > > > 
> > > > So why is this any different from any other bus type?  I don't see the
> > > > "uniqueness" here that has not required this to happen for PCI or USB or
> > > > anything else.
> > > > 
> > > > What am I missing?
> > > In Short:
> > > In serdev, we have to handle incoming device data (serdev calls on a
> > > function pointer we provide in advance), even in the case that the
> > > driver hasn't completed probe yet.
> > 
> > But how is that any different from a USB or PCI driver doing the same
> > thing?  Why is serdev so unique here?  What specific serdev function
> > causes this and why isn't it an issue with the C api?  Can we change the
> > C code to not require this?
> 
> I think the idea is to avoid bugs as in the mhz19b driver [1].
> 
> This driver's probe() looks like this:
> 
> 
> 	serdev_device_set_client_ops(serdev, &mhz19b_ops);
> 	ret = devm_serdev_device_open(dev, serdev);
> 
> 	// Lots of other initialization.
> 
> 	serdev_device_set_drvdata(serdev, indio_dev);
> 
> But the receive_buf() callback from mhz19b_ops dereferences the driver's private
> data.
> 
> Now, maybe this is actually prevented to become an actual race, since some
> regulator is only enabled subsequently:
> 
> 	devm_regulator_get_enable(dev, "vin");
> 
> But in any case in Rust it would be unsound as with this a driver could easily
> cause undefined behavior with safe APIs.
> 
> Maybe it is as simple as letting the abstraction call serdev_device_open() only
> after the driver's probe() has completed, but maybe there are reasons why that
> is not an option, that's a serdev question.
If we call it after probe, calls to `serdev_device_set_baudrate`,
`serdev_device_set_flow_control`, `serdev_device_set_parity`,
`serdev_device_write_buf`, `serdev_device_write`,
`serdev_device_write_flush`, which are exposed via the rust abstraction
would result in a null pointer dereference.

Thanks
- Markus Probst

> 
> [1] https://elixir.bootlin.com/linux/v6.19.7/source/drivers/iio/chemical/mhz19b.c#L260

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 870 bytes --]

^ permalink raw reply

* Re: [PATCH v3 2/4] serdev: add rust private data to serdev_device
From: Danilo Krummrich @ 2026-03-14 13:54 UTC (permalink / raw)
  To: Markus Probst
  Cc: Greg Kroah-Hartman, Rob Herring, Jiri Slaby, Miguel Ojeda,
	Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
	Alice Ryhl, Trevor Gross, Kari Argillander, Rafael J. Wysocki,
	Viresh Kumar, Boqun Feng, David Airlie, Simona Vetter,
	linux-serial, linux-kernel, rust-for-linux, linux-pm, driver-core,
	dri-devel
In-Reply-To: <cd773d17f15f3f2e5d047236721a882c35f64595.camel@posteo.de>

On Sat Mar 14, 2026 at 2:49 PM CET, Markus Probst wrote:
> On Sat, 2026-03-14 at 14:42 +0100, Danilo Krummrich wrote:
>> On Sat Mar 14, 2026 at 2:31 PM CET, Greg Kroah-Hartman wrote:
>> > On Sat, Mar 14, 2026 at 12:08:09PM +0000, Markus Probst wrote:
>> > > On Sat, 2026-03-14 at 12:52 +0100, Greg Kroah-Hartman wrote:
>> > > > On Sat, Mar 14, 2026 at 11:42:02AM +0000, Markus Probst wrote:
>> > > > > On Sat, 2026-03-14 at 09:07 +0100, Greg Kroah-Hartman wrote:
>> > > > > > On Fri, Mar 13, 2026 at 06:12:31PM +0000, Markus Probst wrote:
>> > > > > > > Add rust private data to `struct serdev_device`, as it is required by the
>> > > > > > > rust abstraction added in the following commit
>> > > > > > > (rust: add basic serial device bus abstractions).
>> > > > > > 
>> > > > > > why is rust "special" here?  What's wrong with the existing private
>> > > > > > pointer in this structure?  Why must we add another one?
>> > > > > Because in rust, the device drvdata will be set after probe has run. In
>> > > > > serdev, once the device has been opened, it can receive data. It must
>> > > > > be opened either inside probe or before probe, because it can only be
>> > > > > configured (baudrate, flow control etc.) and data written to after it
>> > > > > has been opened. Because it can receive data before drvdata has been
>> > > > > set yet, we need to ensure it waits on data receival for the probe to
>> > > > > be finished. Otherwise this would be a null pointer dereference. To do
>> > > > > this, we need to store a `Completion` for it to wait and a `bool` in
>> > > > > case the probe exits with an error. We cannot store this data in the
>> > > > > device drvdata, because this is where the drivers drvdata goes. We also
>> > > > > cannot create a wrapper of the drivers drvdata, because
>> > > > > `Device::drvdata::<T>()` would always fail in that case. That is why we
>> > > > > need a "rust_private_data" for this abstraction to store the
>> > > > > `Completion` and `bool`.
>> > > > 
>> > > > So why is this any different from any other bus type?  I don't see the
>> > > > "uniqueness" here that has not required this to happen for PCI or USB or
>> > > > anything else.
>> > > > 
>> > > > What am I missing?
>> > > In Short:
>> > > In serdev, we have to handle incoming device data (serdev calls on a
>> > > function pointer we provide in advance), even in the case that the
>> > > driver hasn't completed probe yet.
>> > 
>> > But how is that any different from a USB or PCI driver doing the same
>> > thing?  Why is serdev so unique here?  What specific serdev function
>> > causes this and why isn't it an issue with the C api?  Can we change the
>> > C code to not require this?
>> 
>> I think the idea is to avoid bugs as in the mhz19b driver [1].
>> 
>> This driver's probe() looks like this:
>> 
>> 
>> 	serdev_device_set_client_ops(serdev, &mhz19b_ops);
>> 	ret = devm_serdev_device_open(dev, serdev);
>> 
>> 	// Lots of other initialization.
>> 
>> 	serdev_device_set_drvdata(serdev, indio_dev);
>> 
>> But the receive_buf() callback from mhz19b_ops dereferences the driver's private
>> data.
>> 
>> Now, maybe this is actually prevented to become an actual race, since some
>> regulator is only enabled subsequently:
>> 
>> 	devm_regulator_get_enable(dev, "vin");
>> 
>> But in any case in Rust it would be unsound as with this a driver could easily
>> cause undefined behavior with safe APIs.
>> 
>> Maybe it is as simple as letting the abstraction call serdev_device_open() only
>> after the driver's probe() has completed, but maybe there are reasons why that
>> is not an option, that's a serdev question.
> If we call it after probe, calls to `serdev_device_set_baudrate`,
> `serdev_device_set_flow_control`, `serdev_device_set_parity`,
> `serdev_device_write_buf`, `serdev_device_write`,
> `serdev_device_write_flush`, which are exposed via the rust abstraction
> would result in a null pointer dereference.

Then maybe ensure that the driver's receive_buf() callback can only ever be
called after probe() has been completed? E.g. receive_buf() could be optional
and swapped out later on.

^ permalink raw reply

* Re: [PATCH v3 2/4] serdev: add rust private data to serdev_device
From: Markus Probst @ 2026-03-14 14:58 UTC (permalink / raw)
  To: Danilo Krummrich
  Cc: Greg Kroah-Hartman, Rob Herring, Jiri Slaby, Miguel Ojeda,
	Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
	Alice Ryhl, Trevor Gross, Kari Argillander, Rafael J. Wysocki,
	Viresh Kumar, Boqun Feng, David Airlie, Simona Vetter,
	linux-serial, linux-kernel, rust-for-linux, linux-pm, driver-core,
	dri-devel
In-Reply-To: <DH2K1DMGFWYR.1E2UZFI1IUZ0N@kernel.org>

[-- Attachment #1: Type: text/plain, Size: 4809 bytes --]

On Sat, 2026-03-14 at 14:54 +0100, Danilo Krummrich wrote:
> On Sat Mar 14, 2026 at 2:49 PM CET, Markus Probst wrote:
> > On Sat, 2026-03-14 at 14:42 +0100, Danilo Krummrich wrote:
> > > On Sat Mar 14, 2026 at 2:31 PM CET, Greg Kroah-Hartman wrote:
> > > > On Sat, Mar 14, 2026 at 12:08:09PM +0000, Markus Probst wrote:
> > > > > On Sat, 2026-03-14 at 12:52 +0100, Greg Kroah-Hartman wrote:
> > > > > > On Sat, Mar 14, 2026 at 11:42:02AM +0000, Markus Probst wrote:
> > > > > > > On Sat, 2026-03-14 at 09:07 +0100, Greg Kroah-Hartman wrote:
> > > > > > > > On Fri, Mar 13, 2026 at 06:12:31PM +0000, Markus Probst wrote:
> > > > > > > > > Add rust private data to `struct serdev_device`, as it is required by the
> > > > > > > > > rust abstraction added in the following commit
> > > > > > > > > (rust: add basic serial device bus abstractions).
> > > > > > > > 
> > > > > > > > why is rust "special" here?  What's wrong with the existing private
> > > > > > > > pointer in this structure?  Why must we add another one?
> > > > > > > Because in rust, the device drvdata will be set after probe has run. In
> > > > > > > serdev, once the device has been opened, it can receive data. It must
> > > > > > > be opened either inside probe or before probe, because it can only be
> > > > > > > configured (baudrate, flow control etc.) and data written to after it
> > > > > > > has been opened. Because it can receive data before drvdata has been
> > > > > > > set yet, we need to ensure it waits on data receival for the probe to
> > > > > > > be finished. Otherwise this would be a null pointer dereference. To do
> > > > > > > this, we need to store a `Completion` for it to wait and a `bool` in
> > > > > > > case the probe exits with an error. We cannot store this data in the
> > > > > > > device drvdata, because this is where the drivers drvdata goes. We also
> > > > > > > cannot create a wrapper of the drivers drvdata, because
> > > > > > > `Device::drvdata::<T>()` would always fail in that case. That is why we
> > > > > > > need a "rust_private_data" for this abstraction to store the
> > > > > > > `Completion` and `bool`.
> > > > > > 
> > > > > > So why is this any different from any other bus type?  I don't see the
> > > > > > "uniqueness" here that has not required this to happen for PCI or USB or
> > > > > > anything else.
> > > > > > 
> > > > > > What am I missing?
> > > > > In Short:
> > > > > In serdev, we have to handle incoming device data (serdev calls on a
> > > > > function pointer we provide in advance), even in the case that the
> > > > > driver hasn't completed probe yet.
> > > > 
> > > > But how is that any different from a USB or PCI driver doing the same
> > > > thing?  Why is serdev so unique here?  What specific serdev function
> > > > causes this and why isn't it an issue with the C api?  Can we change the
> > > > C code to not require this?
> > > 
> > > I think the idea is to avoid bugs as in the mhz19b driver [1].
> > > 
> > > This driver's probe() looks like this:
> > > 
> > > 
> > > 	serdev_device_set_client_ops(serdev, &mhz19b_ops);
> > > 	ret = devm_serdev_device_open(dev, serdev);
> > > 
> > > 	// Lots of other initialization.
> > > 
> > > 	serdev_device_set_drvdata(serdev, indio_dev);
> > > 
> > > But the receive_buf() callback from mhz19b_ops dereferences the driver's private
> > > data.
> > > 
> > > Now, maybe this is actually prevented to become an actual race, since some
> > > regulator is only enabled subsequently:
> > > 
> > > 	devm_regulator_get_enable(dev, "vin");
> > > 
> > > But in any case in Rust it would be unsound as with this a driver could easily
> > > cause undefined behavior with safe APIs.
> > > 
> > > Maybe it is as simple as letting the abstraction call serdev_device_open() only
> > > after the driver's probe() has completed, but maybe there are reasons why that
> > > is not an option, that's a serdev question.
> > If we call it after probe, calls to `serdev_device_set_baudrate`,
> > `serdev_device_set_flow_control`, `serdev_device_set_parity`,
> > `serdev_device_write_buf`, `serdev_device_write`,
> > `serdev_device_write_flush`, which are exposed via the rust abstraction
> > would result in a null pointer dereference.
> 
> Then maybe ensure that the driver's receive_buf() callback can only ever be
> called after probe() has been completed? E.g. receive_buf() could be optional
> and swapped out later on.
I am not exactly sure what you mean by "could be optional and swapped
out later on".

Also,
the function pointer cannot be changed while the device is open, as
this could introduce a race condition. In addition if it was prior set
to NULL and data was received, this data would be lost.

Thanks
- Markus Probst


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 870 bytes --]

^ permalink raw reply

* [PATCH] n_tty: add null check for tty->link in packet mode
From: Osama Abdelkader @ 2026-03-14 22:10 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Jiri Slaby, linux-kernel, linux-serial
  Cc: Osama Abdelkader

Add null check for tty->link before dereferencing in n_tty_read and
n_tty_poll. When the pty master closes, tty->link can be NULL while
the slave is still reading, causing a null pointer dereference.

Signed-off-by: Osama Abdelkader <osama.abdelkader@gmail.com>
---
 drivers/tty/n_tty.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index e6a0f5b40d0a..dc04b87364f6 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -2232,7 +2232,7 @@ static ssize_t n_tty_read(struct tty_struct *tty, struct file *file, u8 *kbuf,
 	add_wait_queue(&tty->read_wait, &wait);
 	while (nr) {
 		/* First test for status change. */
-		if (packet && tty->link->ctrl.pktstatus) {
+		if (packet && tty->link && tty->link->ctrl.pktstatus) {
 			u8 cs;
 			if (kb != kbuf)
 				break;
@@ -2444,7 +2444,7 @@ static __poll_t n_tty_poll(struct tty_struct *tty, struct file *file,
 		if (input_available_p(tty, 1))
 			mask |= EPOLLIN | EPOLLRDNORM;
 	}
-	if (tty->ctrl.packet && tty->link->ctrl.pktstatus)
+	if (tty->ctrl.packet && tty->link && tty->link->ctrl.pktstatus)
 		mask |= EPOLLPRI | EPOLLIN | EPOLLRDNORM;
 	if (test_bit(TTY_OTHER_CLOSED, &tty->flags))
 		mask |= EPOLLHUP;
-- 
2.43.0


^ permalink raw reply related

* [PATCH 3/3] MIPS: dts: loongson64g-package: Switch to Loongson UART driver
From: Rong Zhang @ 2026-03-14 23:41 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Jiri Slaby, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Thomas Bogendoerfer, Huacai Chen, Jiaxun Yang
  Cc: Rong Zhang, linux-kernel, linux-serial, linux-mips, devicetree,
	Yao Zi, Icenowy Zheng, Rong Zhang
In-Reply-To: <20260314234143.651298-1-rongrong@oss.cipunited.com>

Loongson64g is Loongson 3A4000, whose UART controller is compatible with
Loongson 2K1500, which is NS16550A-compatible with an additional
fractional frequency divisor register.

Update the compatible strings to reflect this, so that 3A4000 can
benefit from the fractional frequency divisor provided by loongson-uart.
This is required on some devices, otherwise their UART can't work at
some high baud rates, e.g., 115200.

Tested on Loongson-LS3A4000-7A1000-NUC-SE with a 25MHz UART clock.
Without fractional frequency divisor, the actual baud rate was 111607
(25MHz / 16 / 14, measured value: 111545) and some USB-to-UART
converters couldn't work with it at all. With fractional frequency
divisor, the measured baud rate becomes 115207, which is quite accurate.

Signed-off-by: Rong Zhang <rongrong@oss.cipunited.com>
---
 arch/mips/boot/dts/loongson/loongson64g-package.dtsi | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/mips/boot/dts/loongson/loongson64g-package.dtsi b/arch/mips/boot/dts/loongson/loongson64g-package.dtsi
index d4314f62ccc2..029daeedd0ab 100644
--- a/arch/mips/boot/dts/loongson/loongson64g-package.dtsi
+++ b/arch/mips/boot/dts/loongson/loongson64g-package.dtsi
@@ -40,7 +40,7 @@ liointc: interrupt-controller@3ff01400 {
 		};
 
 		cpu_uart0: serial@1fe00100 {
-			compatible = "ns16550a";
+			compatible = "loongson,ls3a4000-uart", "loongson,ls2k1500-uart", "ns16550a";
 			reg = <0 0x1fe00100 0x10>;
 			clock-frequency = <100000000>;
 			interrupt-parent = <&liointc>;
@@ -50,7 +50,7 @@ cpu_uart0: serial@1fe00100 {
 
 		cpu_uart1: serial@1fe00110 {
 			status = "disabled";
-			compatible = "ns16550a";
+			compatible = "loongson,ls3a4000-uart", "loongson,ls2k1500-uart", "ns16550a";
 			reg = <0 0x1fe00110 0x10>;
 			clock-frequency = <100000000>;
 			interrupts = <15 IRQ_TYPE_LEVEL_HIGH>;
-- 
2.53.0

^ permalink raw reply related

* [PATCH 1/3] dt-bindings: serial: 8250: Add Loongson 3A4000 uart compatible
From: Rong Zhang @ 2026-03-14 23:41 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Jiri Slaby, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Thomas Bogendoerfer, Huacai Chen, Jiaxun Yang
  Cc: Rong Zhang, linux-kernel, linux-serial, linux-mips, devicetree,
	Yao Zi, Icenowy Zheng, Rong Zhang
In-Reply-To: <20260314234143.651298-1-rongrong@oss.cipunited.com>

The UART controller on Loongson 3A4000 is compatible with Loongson
2K1500, which is NS16550A-compatible with an additional fractional
frequency divisor register.

Add loongson,ls3a4000-uart as compatible with loongson,ls2k1500-uart.

Signed-off-by: Rong Zhang <rongrong@oss.cipunited.com>
---
 Documentation/devicetree/bindings/serial/8250.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/serial/8250.yaml b/Documentation/devicetree/bindings/serial/8250.yaml
index 73851f19330d..1d1f2a22776c 100644
--- a/Documentation/devicetree/bindings/serial/8250.yaml
+++ b/Documentation/devicetree/bindings/serial/8250.yaml
@@ -179,6 +179,7 @@ properties:
           - const: ns16550a
       - items:
           - enum:
+              - loongson,ls3a4000-uart
               - loongson,ls3a5000-uart
               - loongson,ls3a6000-uart
               - loongson,ls2k2000-uart
-- 
2.53.0

^ permalink raw reply related

* [PATCH 2/3] serial: 8250: loongson: Enable building on MIPS Loongson64
From: Rong Zhang @ 2026-03-14 23:41 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Jiri Slaby, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Thomas Bogendoerfer, Huacai Chen, Jiaxun Yang
  Cc: Rong Zhang, linux-kernel, linux-serial, linux-mips, devicetree,
	Yao Zi, Icenowy Zheng, Rong Zhang
In-Reply-To: <20260314234143.651298-1-rongrong@oss.cipunited.com>

Loongson 3A4000 is a MIPS-based Loongson64 CPU which also supports
8250_loongson (loongson-uart).

Enable building on MIPS Loongson64 so that Loongson 3A4000 can benefit
from it.

Signed-off-by: Rong Zhang <rongrong@oss.cipunited.com>
---
 drivers/tty/serial/8250/Kconfig | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/tty/serial/8250/Kconfig b/drivers/tty/serial/8250/Kconfig
index fd4e8b6ab60d..fc3e58d62233 100644
--- a/drivers/tty/serial/8250/Kconfig
+++ b/drivers/tty/serial/8250/Kconfig
@@ -465,11 +465,12 @@ config SERIAL_8250_OMAP_TTYO_FIXUP
 config SERIAL_8250_LOONGSON
 	tristate "Loongson 8250 based serial port"
 	depends on SERIAL_8250
-	depends on LOONGARCH || COMPILE_TEST
+	depends on LOONGARCH || MACH_LOONGSON64 || COMPILE_TEST
 	help
-	  If you have a machine based on LoongArch CPU you can enable
-	  its onboard serial ports by enabling this option. The option
-	  is applicable to both devicetree and ACPI, say Y to this option.
+	  If you have a machine based on LoongArch CPU or MIPS-based Loongson
+	  3A4000 CPU you can enable its onboard serial ports by enabling this
+	  option. The option is applicable to both devicetree and ACPI, say Y
+	  to enable this option.
 	  If unsure, say N.
 
 config SERIAL_8250_LPC18XX
-- 
2.53.0

^ permalink raw reply related

* [PATCH 0/3] MIPS: dts: loongson64g-package: Switch to Loongson UART driver
From: Rong Zhang @ 2026-03-14 23:41 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Jiri Slaby, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Thomas Bogendoerfer, Huacai Chen, Jiaxun Yang
  Cc: Rong Zhang, linux-kernel, linux-serial, linux-mips, devicetree,
	Yao Zi, Icenowy Zheng, Rong Zhang

Loongson64g is Loongson 3A4000, whose UART controller is compatible with
Loongson 2K1500, which is NS16550A-compatible with an additional
fractional frequency divisor register.

Patch 1 adds loongson,ls3a4000-uart as compatible with
loongson,ls2k1500-uart.

Patch 2 enables building 8250_loongson (loongson-uart) on MIPS
Loongson64.

Patch 3 updates the compatible strings to reflect this, so that 3A4000
can benefit from the fractional frequency divisor provided by
loongson-uart. This is required on some devices, otherwise their UART
can't work at some high baud rates, e.g., 115200.

Tested on Loongson-LS3A4000-7A1000-NUC-SE with a 25MHz UART clock.
Without fractional frequency divisor, the actual baud rate was 111607
(25MHz / 16 / 14, measured value: 111545) and some USB-to-UART
converters couldn't work with it at all. With fractional frequency
divisor, the measured baud rate becomes 115207, which is quite accurate.

Rong Zhang (3):
  dt-bindings: serial: 8250: Add Loongson 3A4000 uart compatible
  serial: 8250: loongson: Enable building on MIPS Loongson64
  MIPS: dts: loongson64g-package: Switch to Loongson UART driver

 Documentation/devicetree/bindings/serial/8250.yaml   | 1 +
 arch/mips/boot/dts/loongson/loongson64g-package.dtsi | 4 ++--
 drivers/tty/serial/8250/Kconfig                      | 9 +++++----
 3 files changed, 8 insertions(+), 6 deletions(-)


base-commit: 69237f8c1f69112cca7388af7fab6d0ee45a2525
-- 
2.53.0

^ permalink raw reply

* Re: [PATCH] n_tty: add null check for tty->link in packet mode
From: Greg Kroah-Hartman @ 2026-03-15  6:57 UTC (permalink / raw)
  To: Osama Abdelkader; +Cc: Jiri Slaby, linux-kernel, linux-serial
In-Reply-To: <20260314221044.148442-1-osama.abdelkader@gmail.com>

On Sat, Mar 14, 2026 at 11:10:44PM +0100, Osama Abdelkader wrote:
> Add null check for tty->link before dereferencing in n_tty_read and
> n_tty_poll. When the pty master closes, tty->link can be NULL while
> the slave is still reading, causing a null pointer dereference.

How can that happen?

> Signed-off-by: Osama Abdelkader <osama.abdelkader@gmail.com>
> ---
>  drivers/tty/n_tty.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
> index e6a0f5b40d0a..dc04b87364f6 100644
> --- a/drivers/tty/n_tty.c
> +++ b/drivers/tty/n_tty.c
> @@ -2232,7 +2232,7 @@ static ssize_t n_tty_read(struct tty_struct *tty, struct file *file, u8 *kbuf,
>  	add_wait_queue(&tty->read_wait, &wait);
>  	while (nr) {
>  		/* First test for status change. */
> -		if (packet && tty->link->ctrl.pktstatus) {
> +		if (packet && tty->link && tty->link->ctrl.pktstatus) {
>  			u8 cs;
>  			if (kb != kbuf)
>  				break;
> @@ -2444,7 +2444,7 @@ static __poll_t n_tty_poll(struct tty_struct *tty, struct file *file,
>  		if (input_available_p(tty, 1))
>  			mask |= EPOLLIN | EPOLLRDNORM;
>  	}
> -	if (tty->ctrl.packet && tty->link->ctrl.pktstatus)
> +	if (tty->ctrl.packet && tty->link && tty->link->ctrl.pktstatus)

What happens if link changes right after you test it?  Where is the
lock?

And what changed to cause this to show up now?

thanks,

greg k-h

^ permalink raw reply

* Re: [PATCH 3/3] MIPS: dts: loongson64g-package: Switch to Loongson UART driver
From: Krzysztof Kozlowski @ 2026-03-15  9:08 UTC (permalink / raw)
  To: Rong Zhang
  Cc: Greg Kroah-Hartman, Jiri Slaby, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Thomas Bogendoerfer, Huacai Chen, Jiaxun Yang,
	linux-kernel, linux-serial, linux-mips, devicetree, Yao Zi,
	Icenowy Zheng, Rong Zhang
In-Reply-To: <20260314234143.651298-4-rongrong@oss.cipunited.com>

On Sun, Mar 15, 2026 at 07:41:43AM +0800, Rong Zhang wrote:
> Loongson64g is Loongson 3A4000, whose UART controller is compatible with
> Loongson 2K1500, which is NS16550A-compatible with an additional
> fractional frequency divisor register.
> 
> Update the compatible strings to reflect this, so that 3A4000 can
> benefit from the fractional frequency divisor provided by loongson-uart.
> This is required on some devices, otherwise their UART can't work at
> some high baud rates, e.g., 115200.
> 
> Tested on Loongson-LS3A4000-7A1000-NUC-SE with a 25MHz UART clock.
> Without fractional frequency divisor, the actual baud rate was 111607
> (25MHz / 16 / 14, measured value: 111545) and some USB-to-UART
> converters couldn't work with it at all. With fractional frequency
> divisor, the measured baud rate becomes 115207, which is quite accurate.
> 
> Signed-off-by: Rong Zhang <rongrong@oss.cipunited.com>
> ---
>  arch/mips/boot/dts/loongson/loongson64g-package.dtsi | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

DTS should be sent separately, don't mix independent patches targetting
serial tree.

Best regards,
Krzysztof


^ permalink raw reply

* Re: [PATCH 1/3] dt-bindings: serial: 8250: Add Loongson 3A4000 uart compatible
From: Krzysztof Kozlowski @ 2026-03-15  9:09 UTC (permalink / raw)
  To: Rong Zhang
  Cc: Greg Kroah-Hartman, Jiri Slaby, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Thomas Bogendoerfer, Huacai Chen, Jiaxun Yang,
	linux-kernel, linux-serial, linux-mips, devicetree, Yao Zi,
	Icenowy Zheng, Rong Zhang
In-Reply-To: <20260314234143.651298-2-rongrong@oss.cipunited.com>

On Sun, Mar 15, 2026 at 07:41:41AM +0800, Rong Zhang wrote:
> The UART controller on Loongson 3A4000 is compatible with Loongson
> 2K1500, which is NS16550A-compatible with an additional fractional
> frequency divisor register.
> 
> Add loongson,ls3a4000-uart as compatible with loongson,ls2k1500-uart.
> 
> Signed-off-by: Rong Zhang <rongrong@oss.cipunited.com>
> ---
>  Documentation/devicetree/bindings/serial/8250.yaml | 1 +
>  1 file changed, 1 insertion(+)

Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>

Best regards,
Krzysztof


^ permalink raw reply

* [PATCH v2 0/2] serial: 8250: loongson: Add support for MIPS-based Loongson 3A4000
From: Rong Zhang @ 2026-03-15 18:42 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Jiri Slaby, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Thomas Bogendoerfer, Huacai Chen, Jiaxun Yang
  Cc: Rong Zhang, linux-kernel, linux-serial, linux-mips, devicetree,
	Yao Zi, Icenowy Zheng, Rong Zhang

This series targets the serial tree.

The UART controller on Loongson 3A4000 is compatible with Loongson
2K1500, which is NS16550A-compatible with an additional fractional
frequency divisor register.

Patch 1 adds loongson,ls3a4000-uart as compatible with
loongson,ls2k1500-uart.

Patch 2 enables building 8250_loongson (loongson-uart) on MIPS
Loongson64.

Tested on Loongson-LS3A4000-7A1000-NUC-SE with a 25MHz UART clock.
Without fractional frequency divisor, the actual baud rate was 111607
(25MHz / 16 / 14, measured value: 111545) and some USB-to-UART
converters couldn't work with it at all. With fractional frequency
divisor, the measured baud rate becomes 115207, which is quite accurate.

The patch for the MIPS tree to update the compatible strings in the
loongson64g-package DTS is sent separately, as it's independant of this
series and can be applied in any order (the compatible strings there
still contain "ns16550a", so no regression will be introduced).

Changes in v2:
- Separated from v1 (patch 1,2): https://lore.kernel.org/r/20260314234143.651298-1-rongrong@oss.cipunited.com/
(thanks Krzysztof Kozlowski)

Rong Zhang (2):
  dt-bindings: serial: 8250: Add Loongson 3A4000 uart compatible
  serial: 8250: loongson: Enable building on MIPS Loongson64

 Documentation/devicetree/bindings/serial/8250.yaml | 1 +
 drivers/tty/serial/8250/Kconfig                    | 9 +++++----
 2 files changed, 6 insertions(+), 4 deletions(-)


base-commit: 267594792a71018788af69e836c52e34bb8054af
-- 
2.53.0

^ permalink raw reply

* [PATCH v2 1/2] dt-bindings: serial: 8250: Add Loongson 3A4000 uart compatible
From: Rong Zhang @ 2026-03-15 18:42 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Jiri Slaby, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Thomas Bogendoerfer, Huacai Chen, Jiaxun Yang
  Cc: Rong Zhang, linux-kernel, linux-serial, linux-mips, devicetree,
	Yao Zi, Icenowy Zheng, Rong Zhang, Krzysztof Kozlowski
In-Reply-To: <20260315184301.412844-1-rongrong@oss.cipunited.com>

The UART controller on Loongson 3A4000 is compatible with Loongson
2K1500, which is NS16550A-compatible with an additional fractional
frequency divisor register.

Add loongson,ls3a4000-uart as compatible with loongson,ls2k1500-uart.

Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
Signed-off-by: Rong Zhang <rongrong@oss.cipunited.com>
---
 Documentation/devicetree/bindings/serial/8250.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/serial/8250.yaml b/Documentation/devicetree/bindings/serial/8250.yaml
index 73851f19330d..1d1f2a22776c 100644
--- a/Documentation/devicetree/bindings/serial/8250.yaml
+++ b/Documentation/devicetree/bindings/serial/8250.yaml
@@ -179,6 +179,7 @@ properties:
           - const: ns16550a
       - items:
           - enum:
+              - loongson,ls3a4000-uart
               - loongson,ls3a5000-uart
               - loongson,ls3a6000-uart
               - loongson,ls2k2000-uart
-- 
2.53.0

^ permalink raw reply related

* [PATCH v2 2/2] serial: 8250: loongson: Enable building on MIPS Loongson64
From: Rong Zhang @ 2026-03-15 18:42 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Jiri Slaby, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Thomas Bogendoerfer, Huacai Chen, Jiaxun Yang
  Cc: Rong Zhang, linux-kernel, linux-serial, linux-mips, devicetree,
	Yao Zi, Icenowy Zheng, Rong Zhang
In-Reply-To: <20260315184301.412844-1-rongrong@oss.cipunited.com>

Loongson 3A4000 is a MIPS-based Loongson64 CPU which also supports
8250_loongson (loongson-uart).

Enable building on MIPS Loongson64 so that Loongson 3A4000 can benefit
from it.

Signed-off-by: Rong Zhang <rongrong@oss.cipunited.com>
---
 drivers/tty/serial/8250/Kconfig | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/tty/serial/8250/Kconfig b/drivers/tty/serial/8250/Kconfig
index fd4e8b6ab60d..fc3e58d62233 100644
--- a/drivers/tty/serial/8250/Kconfig
+++ b/drivers/tty/serial/8250/Kconfig
@@ -465,11 +465,12 @@ config SERIAL_8250_OMAP_TTYO_FIXUP
 config SERIAL_8250_LOONGSON
 	tristate "Loongson 8250 based serial port"
 	depends on SERIAL_8250
-	depends on LOONGARCH || COMPILE_TEST
+	depends on LOONGARCH || MACH_LOONGSON64 || COMPILE_TEST
 	help
-	  If you have a machine based on LoongArch CPU you can enable
-	  its onboard serial ports by enabling this option. The option
-	  is applicable to both devicetree and ACPI, say Y to this option.
+	  If you have a machine based on LoongArch CPU or MIPS-based Loongson
+	  3A4000 CPU you can enable its onboard serial ports by enabling this
+	  option. The option is applicable to both devicetree and ACPI, say Y
+	  to enable this option.
 	  If unsure, say N.
 
 config SERIAL_8250_LPC18XX
-- 
2.53.0

^ permalink raw reply related

* [PATCH v2] MIPS: dts: loongson64g-package: Switch to Loongson UART driver
From: Rong Zhang @ 2026-03-15 18:44 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Jiri Slaby, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Thomas Bogendoerfer, Huacai Chen, Jiaxun Yang
  Cc: Rong Zhang, linux-kernel, linux-serial, linux-mips, devicetree,
	Yao Zi, Icenowy Zheng, Rong Zhang

Loongson64g is Loongson 3A4000, whose UART controller is compatible with
Loongson 2K1500, which is NS16550A-compatible with an additional
fractional frequency divisor register.

Update the compatible strings to reflect this, so that 3A4000 can
benefit from the fractional frequency divisor provided by loongson-uart.
This is required on some devices, otherwise their UART can't work at
some high baud rates, e.g., 115200.

Tested on Loongson-LS3A4000-7A1000-NUC-SE with a 25MHz UART clock.
Without fractional frequency divisor, the actual baud rate was 111607
(25MHz / 16 / 14, measured value: 111545) and some USB-to-UART
converters couldn't work with it at all. With fractional frequency
divisor, the measured baud rate becomes 115207, which is quite accurate.

Signed-off-by: Rong Zhang <rongrong@oss.cipunited.com>
---
This patch targets the MIPS tree.

The series for the serial tree to update dt-bindings and enable building
8250_loongson (loongson-uart) on MIPS Loongson64 is sent separately, as
it's independant of this patch and can be applied in any order (the
compatible strings here still contain "ns16550a", so no regression will
be introduced).

Changes in v2:
- Separated from v1 (patch 3): https://lore.kernel.org/r/20260314234143.651298-1-rongrong@oss.cipunited.com/
(thanks Krzysztof Kozlowski)
---
 arch/mips/boot/dts/loongson/loongson64g-package.dtsi | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/mips/boot/dts/loongson/loongson64g-package.dtsi b/arch/mips/boot/dts/loongson/loongson64g-package.dtsi
index d4314f62ccc2..029daeedd0ab 100644
--- a/arch/mips/boot/dts/loongson/loongson64g-package.dtsi
+++ b/arch/mips/boot/dts/loongson/loongson64g-package.dtsi
@@ -40,7 +40,7 @@ liointc: interrupt-controller@3ff01400 {
 		};
 
 		cpu_uart0: serial@1fe00100 {
-			compatible = "ns16550a";
+			compatible = "loongson,ls3a4000-uart", "loongson,ls2k1500-uart", "ns16550a";
 			reg = <0 0x1fe00100 0x10>;
 			clock-frequency = <100000000>;
 			interrupt-parent = <&liointc>;
@@ -50,7 +50,7 @@ cpu_uart0: serial@1fe00100 {
 
 		cpu_uart1: serial@1fe00110 {
 			status = "disabled";
-			compatible = "ns16550a";
+			compatible = "loongson,ls3a4000-uart", "loongson,ls2k1500-uart", "ns16550a";
 			reg = <0 0x1fe00110 0x10>;
 			clock-frequency = <100000000>;
 			interrupts = <15 IRQ_TYPE_LEVEL_HIGH>;

base-commit: 267594792a71018788af69e836c52e34bb8054af
-- 
2.53.0

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox