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

Signed-off-by: Markus Probst <markus.probst@posteo.de>
---
 include/linux/serdev.h | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/include/linux/serdev.h b/include/linux/serdev.h
index 5654c58eb73c..c74c345d60ae 100644
--- a/include/linux/serdev.h
+++ b/include/linux/serdev.h
@@ -33,12 +33,14 @@ struct serdev_device_ops {
 
 /**
  * struct serdev_device - Basic representation of an serdev device
- * @dev:	Driver model representation of the device.
- * @nr:		Device number on serdev bus.
- * @ctrl:	serdev controller managing this device.
- * @ops:	Device operations.
- * @write_comp	Completion used by serdev_device_write() internally
- * @write_lock	Lock to serialize access when writing data
+ * @dev:	       Driver model representation of the device.
+ * @nr:		       Device number on serdev bus.
+ * @ctrl:	       serdev controller managing this device.
+ * @ops:	       Device operations.
+ * @write_comp:	       Completion used by serdev_device_write() internally
+ * @write_lock:	       Lock to serialize access when writing data
+ * @rust_private_data: Private data for the rust abstraction. This should
+ *		       not be used by the C drivers.
  */
 struct serdev_device {
 	struct device dev;
@@ -47,6 +49,7 @@ struct serdev_device {
 	const struct serdev_device_ops *ops;
 	struct completion write_comp;
 	struct mutex write_lock;
+	void *rust_private_data;
 };
 
 static inline struct serdev_device *to_serdev_device(struct device *d)

-- 
2.52.0

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

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

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

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

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

On Sat, 2026-03-14 at 14:31 +0100, 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? 
> 
In PCI or USB we don't need to provide function pointers for callbacks
in advance, which will be can be called any time (even while probe).

> What specific serdev function
> causes this
> 
drivers/tty/serdev/serdev-ttyport.c basically only wraps the serdev
calls to tty calls. This isn't directly caused by a serdev function,
but by the tty part.

> why isn't it an issue with the C api?
> 
In C you can set the drvdata inside the probe and even with it not
being fully initialized.

With Rust the drvdata is only available after the probe.
But there is the posibility of serdev calling the provided callback
inside probe.

> Can we change the
> C code to not require this?
Serdev is very closely linked to tty.

There are 3 options:

1. Add a `rust_serdev_device_open` and `rust_serdev_device_ready`
function. `rust_serdev_device_open` would do the same thing as
`serdev_device_open`, but with calling `tty_buffer_lock_exclusive`
before opening the underlying tty port. `rust_serdev_device_ready`
would call `tty_buffer_unlock_exclusive`. Such functions would then
need to exist for every serdev controller (currently there is only
ttyport as serdev controller).

2. Rewrite parts of the tty subsystem.

Not sure what would need to be changed there yet. But this could also
affect the existing tty drivers, which are a lot in comparision to
serdev.

3. Keep the `rust_private_data` pointer in `serdev_device`.

This seems to be the simplest option to me.

Thanks
- Markus Probst

> 
> thanks,
> 
> greg k-h

On Fri, Mar 20, 2026 at 04:53:09PM +0000, Markus Probst wrote:
> On Sat, 2026-03-14 at 14:31 +0100, 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? 
> > 
> In PCI or USB we don't need to provide function pointers for callbacks
> in advance, which will be can be called any time (even while probe).

All class drivers are like this, once you register with them, the
function pointers you provide can be called before your probe call ends.
So this isn't unique as far as I can tell.

> > What specific serdev function
> > causes this
> > 
> drivers/tty/serdev/serdev-ttyport.c basically only wraps the serdev
> calls to tty calls. This isn't directly caused by a serdev function,
> but by the tty part.
> 
> > why isn't it an issue with the C api?
> > 
> In C you can set the drvdata inside the probe and even with it not
> being fully initialized.
> 
> With Rust the drvdata is only available after the probe.
> But there is the posibility of serdev calling the provided callback
> inside probe.

Other classes have this same issue, so why isn't this a problem for HID
and input and the like?

thanks,

greg k-h

On Fri, 2026-03-20 at 17:59 +0100, Greg Kroah-Hartman wrote:
> On Fri, Mar 20, 2026 at 04:53:09PM +0000, Markus Probst wrote:
> > On Sat, 2026-03-14 at 14:31 +0100, 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? 
> > > 
> > In PCI or USB we don't need to provide function pointers for callbacks
> > in advance, which will be can be called any time (even while probe).
> 
> All class drivers are like this, once you register with them, the
> function pointers you provide can be called before your probe call ends.
> So this isn't unique as far as I can tell.
> 
> > > What specific serdev function
> > > causes this
> > > 
> > drivers/tty/serdev/serdev-ttyport.c basically only wraps the serdev
> > calls to tty calls. This isn't directly caused by a serdev function,
> > but by the tty part.
> > 
> > > why isn't it an issue with the C api?
> > > 
> > In C you can set the drvdata inside the probe and even with it not
> > being fully initialized.
> > 
> > With Rust the drvdata is only available after the probe.
> > But there is the posibility of serdev calling the provided callback
> > inside probe.
> 
> Other classes have this same issue, so why isn't this a problem for HID
> and input and the like?
This is unique for a bus device.

A class device has its own Data (e. g. PwmOps), i. e. it will only be
registered after this Data has been initialized by the driver.

The receive_buf callback on the serdev device on the other hand must
happen on the drvdata, as there is no place to store its own Data.

The drvdata is only available at the end of the probe in Rust.

Thanks
- Markus Probst

> 
> thanks,
> 
> greg k-h

On 3/20/26 6:13 PM, Markus Probst wrote:
> This is unique for a bus device.
> 
> A class device has its own Data (e. g. PwmOps), i. e. it will only be
> registered after this Data has been initialized by the driver.
> 
> The receive_buf callback on the serdev device on the other hand must
> happen on the drvdata, as there is no place to store its own Data.
> 
> The drvdata is only available at the end of the probe in Rust.

In other words, devm_serdev_device_open() relies on the driver having its
private data set before this is called, since once devm_serdev_device_open() has
been called, the driver's receive_buf() callback may be called.

So, in C it is the driver's responsibility to do things in the correct order, in
Rust we want to make sure the driver can't do it out of order in the first place.

I'm still not sure whether the current approach is the best option. For
instance, we could also solve this with a separate callback. (Wasn't this even
what you had in a previous version?)

On Fri, 2026-03-20 at 20:59 +0100, Danilo Krummrich wrote:
> On 3/20/26 6:13 PM, Markus Probst wrote:
> > This is unique for a bus device.
> > 
> > A class device has its own Data (e. g. PwmOps), i. e. it will only be
> > registered after this Data has been initialized by the driver.
> > 
> > The receive_buf callback on the serdev device on the other hand must
> > happen on the drvdata, as there is no place to store its own Data.
> > 
> > The drvdata is only available at the end of the probe in Rust.
> 
> In other words, devm_serdev_device_open() relies on the driver having its
> private data set before this is called, since once devm_serdev_device_open() has
> been called, the driver's receive_buf() callback may be called.
> 
> So, in C it is the driver's responsibility to do things in the correct order, in
> Rust we want to make sure the driver can't do it out of order in the first place.
> 
> I'm still not sure whether the current approach is the best option. For
> instance, we could also solve this with a separate callback.
> 

> Wasn't this even
> what you had in a previous version?
Yes, but Kari pointed out that changing the ops while it is in use is
unsafe [1].

Thanks
- Markus Probst

[1]
https://lore.kernel.org/rust-for-linux/CAC=eVgR2WYdDTW3kOeemyQPP-H0aAUsrzn5Gk5zCe2hQEB709w@mail.gmail.com/

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

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

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.

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

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