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.
