public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
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

  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