linux-kernel - Re: [PATCH v2 2/4] rust: i2c: add manual I2C device creation abstractions

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <aGgxfNh-sgkJls_h@cassiopeiae>
Date: Fri, 4 Jul 2025 21:54:36 +0200
From: Danilo Krummrich <dakr@...nel.org>
To: Igor Korotin <igor.korotin.linux@...il.com>
Cc: Miguel Ojeda <ojeda@...nel.org>, Alex Gaynor <alex.gaynor@...il.com>,
	Wolfram Sang <wsa+renesas@...g-engineering.com>,
	Boqun Feng <boqun.feng@...il.com>, Gary Guo <gary@...yguo.net>,
	Björn Roy Baron <bjorn3_gh@...tonmail.com>,
	Benno Lossin <lossin@...nel.org>,
	Andreas Hindborg <a.hindborg@...nel.org>,
	Alice Ryhl <aliceryhl@...gle.com>, Trevor Gross <tmgross@...ch.edu>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Viresh Kumar <viresh.kumar@...aro.org>,
	Asahi Lina <lina+kernel@...hilina.net>,
	Wedson Almeida Filho <wedsonaf@...il.com>,
	Alex Hung <alex.hung@....com>, Tamir Duberstein <tamird@...il.com>,
	Xiangfei Ding <dingxiangfei2009@...il.com>,
	linux-kernel@...r.kernel.org, rust-for-linux@...r.kernel.org,
	linux-i2c@...r.kernel.org
Subject: Re: [PATCH v2 2/4] rust: i2c: add manual I2C device creation
 abstractions

On Fri, Jul 04, 2025 at 04:39:12PM +0100, Igor Korotin wrote:
> -pub struct Device<Ctx: device::DeviceContext = device::Normal>(
> +pub struct Device<Ctx: device::DeviceContext = device::Normal, State: DeviceState = state::Borrowed>(
>      Opaque<bindings::i2c_client>,
>      PhantomData<Ctx>,
> +    PhantomData<State>,
>  );

I see what you're doing here, but I think you're thinking this way too
complicated.

I recommend not to reuse the Device type to register a new I2C client device,
it's adding too much complexity without any real value.

You also don't want the DeviceContext types for a device registration, since the
registration will never have any other DeviceContext than device::Normal (see
also my comment on the sample module).

DeviceContext types are only useful for &Device (i.e. references) given out for
a specific scope, such as probe(), remove(), etc.

The only thing you really want to do is to register a new I2C client device, get
a i2c::Registration instance and call i2c_unregister_device() when the
i2c::Registration is dropped.

This is exactly the same use-case as we have in the auxiliary bus. I highly
recommend looking at what auxiliary::Registration does [1].

Also note that if you want a reference to the device in the i2c::Registration,
you can also add a i2c::Registration::device() method that returns an
&i2c::Device, which through into() you can obtain an ARef<i2c::Device> from.

[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/rust/kernel/auxiliary.rs?h=v6.16-rc4#n299