From: "Danilo Krummrich" <dakr@kernel.org>
To: "Greg Kroah-Hartman" <gregkh@linuxfoundation.org>
Cc: "Markus Probst" <markus.probst@posteo.de>,
"Rob Herring" <robh@kernel.org>,
"Jiri Slaby" <jirislaby@kernel.org>,
"Miguel Ojeda" <ojeda@kernel.org>, "Gary Guo" <gary@garyguo.net>,
"Björn Roy Baron" <bjorn3_gh@protonmail.com>,
"Benno Lossin" <lossin@kernel.org>,
"Andreas Hindborg" <a.hindborg@kernel.org>,
"Alice Ryhl" <aliceryhl@google.com>,
"Trevor Gross" <tmgross@umich.edu>,
"Kari Argillander" <kari.argillander@gmail.com>,
"Rafael J. Wysocki" <rafael@kernel.org>,
"Viresh Kumar" <viresh.kumar@linaro.org>,
"Boqun Feng" <boqun@kernel.org>,
"David Airlie" <airlied@gmail.com>,
"Simona Vetter" <simona@ffwll.ch>,
linux-serial@vger.kernel.org, linux-kernel@vger.kernel.org,
rust-for-linux@vger.kernel.org, linux-pm@vger.kernel.org,
driver-core@lists.linux.dev, dri-devel@lists.freedesktop.org
Subject: Re: [PATCH v3 2/4] serdev: add rust private data to serdev_device
Date: Sat, 14 Mar 2026 14:42:58 +0100 [thread overview]
Message-ID: <DH2JS8YL1NXY.VRKLBSTZ5ZYI@kernel.org> (raw)
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
next prev parent reply other threads:[~2026-03-14 13:43 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-03-13 18:12 [PATCH v3 0/4] rust: add basic serial device bus abstractions Markus Probst
2026-03-13 18:12 ` [PATCH v3 1/4] rust: devres: return reference in `devres::register` Markus Probst
2026-03-13 18:12 ` [PATCH v3 2/4] serdev: add rust private data to serdev_device Markus Probst
2026-03-14 8:07 ` Greg Kroah-Hartman
2026-03-14 11:42 ` Markus Probst
2026-03-14 11:52 ` Greg Kroah-Hartman
2026-03-14 12:08 ` Markus Probst
2026-03-14 13:24 ` Alice Ryhl
2026-03-14 13:31 ` Greg Kroah-Hartman
2026-03-14 13:42 ` Danilo Krummrich [this message]
2026-03-14 13:49 ` Markus Probst
2026-03-14 13:54 ` Danilo Krummrich
2026-03-14 14:58 ` Markus Probst
2026-03-20 16:53 ` Markus Probst
2026-03-20 16:59 ` Greg Kroah-Hartman
2026-03-20 17:13 ` Markus Probst
2026-03-20 19:59 ` Danilo Krummrich
2026-03-20 21:08 ` Markus Probst
2026-03-13 18:12 ` [PATCH v3 3/4] rust: add basic serial device bus abstractions Markus Probst
2026-03-13 18:12 ` [PATCH v3 4/4] samples: rust: add Rust serial device bus sample device driver Markus Probst
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=DH2JS8YL1NXY.VRKLBSTZ5ZYI@kernel.org \
--to=dakr@kernel.org \
--cc=a.hindborg@kernel.org \
--cc=airlied@gmail.com \
--cc=aliceryhl@google.com \
--cc=bjorn3_gh@protonmail.com \
--cc=boqun@kernel.org \
--cc=dri-devel@lists.freedesktop.org \
--cc=driver-core@lists.linux.dev \
--cc=gary@garyguo.net \
--cc=gregkh@linuxfoundation.org \
--cc=jirislaby@kernel.org \
--cc=kari.argillander@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pm@vger.kernel.org \
--cc=linux-serial@vger.kernel.org \
--cc=lossin@kernel.org \
--cc=markus.probst@posteo.de \
--cc=ojeda@kernel.org \
--cc=rafael@kernel.org \
--cc=robh@kernel.org \
--cc=rust-for-linux@vger.kernel.org \
--cc=simona@ffwll.ch \
--cc=tmgross@umich.edu \
--cc=viresh.kumar@linaro.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox