Re: [RFC PATCH 0/3] rust: power_supply class abstraction and SMB347 charger driver

[Sebastian Reichel <sebastian.reichel@collabora.com>]

[-- Attachment #1: Type: text/plain, Size: 4609 bytes --]

Hi,

On Wed, Jul 08, 2026 at 09:47:35PM +0000, Bruce Robertson wrote:
> This series adds a Rust abstraction for the power supply class -- which
> does not currently exist in mainline or rust-next -- together with the
> SMBus helpers needed to drive an I2C charger from Rust,

Having Rust bindings for new drivers sounds good to me.

> and a Rust port of the Summit SMB347 battery charger as the first
> consumer.

Any reason for Summit SMB347 specifically? I would prefer not to
have two drivers for the same hardware. Looks like this is used
by just three relatively old devices:

rg -l '"summit,smb347"' arch
arch/arm/boot/dts/ti/omap/omap4-samsung-espresso10.dts
arch/arm/boot/dts/nvidia/tegra30-asus-nexus7-grouper-common.dtsi
arch/arm/boot/dts/samsung/exynos4412-p4note.dtsi

All are ARM32 based, so Rust is at least available by all users.
So I suppose replacing this C driver would be an option, OTOH I
don't want to motivate people to rewrite things either :)

> Motivation
> ----------
> Rust drivers for power supply hardware are currently blocked: there is
> no safe Rust interface to the power supply class, and the I2cClient
> abstraction exposes no register I/O. This series provides a minimal,
> self-contained path from binding an I2C charger to reporting charging
> state through sysfs, entirely in safe Rust, with the unsafe FFI confined
> to the two abstraction layers.
> 
> The series is structured abstraction-first:
> 
>   1/3  Safe SMBus read/write/update_bits over i2c::I2cClient.
>   2/3  A power_supply Driver trait, a generic get_property trampoline,
>        and an RAII Registration that owns the descriptor lifetime.
>   3/3  A Rust SMB347 charger driver consuming both: it binds over I2C
>        and reports STATUS, ONLINE and CHARGE_TYPE.

A new driver supporting CHARGE_TYPE should also support
CHARGE_TYPES.

> Testing
> -------
> Built and exercised against an emulated SMB347 using i2c-stub: seeding
> the chip's status registers and reading back the corresponding sysfs
> attributes (status, online, charge_type) confirms the full C->Rust->C
> path. No physical hardware or interrupt path has been tested.

If the idea is to just have a user for the Rust binding and
something to test/play around: Maybe convert test_power instead? It
would also decouple the power-supply Rust bindings from I2C/SMBus.
The disadvantage is, that this driver is a special-case, since it
has no parent device :)

Greetings,

-- Sebastian

> Open questions / known limitations (hence RFC)
> ----------------------------------------------
>   - get_property recovers the driver's private data via the parent
>     device's drvdata. The current code relies on the observed ordering
>     (callbacks only fire after probe() has set drvdata); the contract
>     should be made explicit.
>   - smbus_update_bits() is not atomic against concurrent callers; a lock
>     will be required before a charger IRQ handler is added (not yet
>     implemented).
>   - Only get_property and a handful of properties are wired up;
>     set_property, property_is_writeable and the IRQ-driven
>     power_supply_changed() notification are future work.
> 
> Feedback on the abstraction's shape -- especially the descriptor
> lifetime and the drvdata recovery -- would be very welcome.
> 
> checkpatch emits one MAINTAINERS warning on patch 3/3 (new driver file);
> it is a false positive -- the driver is covered by the existing POWER
> SUPPLY CLASS "F: drivers/power/supply/" glob and needs no new entry.
> Patch 2/3 adds the one file outside any existing glob
> (rust/kernel/power_supply.rs) and updates MAINTAINERS accordingly.
> 
> Based on rust-next (v7.2-rc1).
> 
> Bruce Robertson (3):
>   rust: i2c: add SMBus byte transfer helpers
>   rust: power_supply: add power supply class abstraction
>   power: supply: add Rust SMB347 charger driver
> 
>  MAINTAINERS                                 |   1 +
>  drivers/power/supply/Kconfig                |  14 ++
>  drivers/power/supply/Makefile               |   1 +
>  drivers/power/supply/smb347-charger_rust.rs | 180 ++++++++++++++++++++
>  rust/bindings/bindings_helper.h             |   1 +
>  rust/kernel/i2c.rs                          |  42 +++++
>  rust/kernel/lib.rs                          |   1 +
>  rust/kernel/power_supply.rs                 | 133 +++++++++++++++
>  8 files changed, 373 insertions(+)
>  create mode 100644 drivers/power/supply/smb347-charger_rust.rs
>  create mode 100644 rust/kernel/power_supply.rs
> 
> -- 
> 2.43.0
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]