Re: [PATCH v3 2/4] serdev: add rust private data to serdev_device

[Danilo Krummrich]

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