Message-ID: <7d196a91-220a-41a5-8577-198b436d8440@bootlin.com>
Date: Thu, 27 Feb 2025 14:06:21 +0100
From: Louis Chauvet <louis.chauvet@...tlin.com>
To: Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
 linux-kernel@...r.kernel.org, Lyude Paul <lyude@...hat.com>,
 "Rafael J. Wysocki" <rafael@...nel.org>, Danilo Krummrich <dakr@...nel.org>
Cc: Alexander Lobakin <aleksander.lobakin@...el.com>,
 Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
 Bjorn Helgaas <bhelgaas@...gle.com>,
 Jonathan Cameron <Jonathan.Cameron@...wei.com>,
 Liam Girdwood <lgirdwood@...il.com>, Lukas Wunner <lukas@...ner.de>,
 Mark Brown <broonie@...nel.org>, Maíra Canal
 <mairacanal@...eup.net>, Robin Murphy <robin.murphy@....com>,
 Simona Vetter <simona.vetter@...ll.ch>, Zijun Hu <quic_zijuhu@...cinc.com>,
 linux-usb@...r.kernel.org, rust-for-linux@...r.kernel.org,
 José Expósito <jose.exposito89@...il.com>
Subject: Re: [PATCH v4 0/9] Driver core: Add faux bus devices



Le 10/02/2025 à 13:30, Greg Kroah-Hartman a écrit :
> For years/decades now, I've been complaining when I see people use
> platform devices for things that are obviously NOT platform devices.
> To finally fix this up, here is a "faux bus" that should be used instead
> of a platform device for these tiny and "fake" devices that people
> create all over the place.
> 
> The api is even simpler than the normal platform device api, just two
> functions, one to create a device and one to remove it.  When a device
> is created, if a probe/release callback is offered, they will be called
> at the proper time in the device's lifecycle.  When finished with the
> device, just destroy it and all should be good.
> 
> This simple api should also hopefully provide for a simple rust binding
> to it given the simple rules and lifecycle of the pointer passed back
> from the creation function (i.e. it is alive and valid for as long as
> you have not called destroy on it.)
> 
> I've also converted four different examples of platform device abuse, the
> dummy regulator driver, the USB phy code, the x86 microcode dvice, and
> the "regulator" device that wifi uses to load the firmware tables, to
> use this api.  In all cases, the logic either was identical, or became
> simpler, than before, a good sign (side note, a bug was fixed in the usb
> phy code that no one ever noticed before).
> 
> Note, unless there are major objections, I'm leaning toward getting
> patch 1 and 2 of this series merged during this -rc cycle so that all of
> the individual driver subsystem cleanups can go through those subsystems
> as needed, as well as allowing the rust developers to create a binding
> and get that merged easier.  Having patch 1 merged on its own isn't
> going to cause any changes if no one uses it, so that should be fine.

Hi all,

I have a maybe dumb question regarding the patches 3..9: do they break 
the UAPI?

With a platform device, the drivers appear under /sys/bus/platform, but 
with faux device, they appear under /sys/bus/faux.

I ask because I found out that one (see my reply to [2]) of the main drm 
library expects to find all the devices under pci, usb, platform, virtio 
and host1x buses [1], so at least for the vgem and vkms driver, this 
library will be broken (it will not crash, but previously detected 
devices will suddenly disappear).

I don't know what are the rules for /sys/bus, but changing a device from 
one bus to another seems to break userspace programs. How should we 
handle this situation? Should we fix the existing drivers? Or only new 
drivers should use it?

+CC: José Expósito

Thanks,
Louis Chauvet

[1]:https://gitlab.freedesktop.org/mesa/drm/-/blob/main/xf86drm.c#L4460-4515
[2]:https://lore.kernel.org/all/20250218165011.9123-21-jose.exposito89@gmail.com/

> Changes from v4:
>    - really dropped the internal name structure, remanants were left over
>      from the last patch series
>    - added the rust binding patch from Lyude (is this one of the first
>      patch series that adds a new kernel api AND the rust binding at the
>      same time?)
>    - added a parent pointer to the api so the devices can be in the tree
>      if the caller wants them
>    - made probe synchronous to prevent race when using the api (when the
>      create call returns the device is fully ready to go.)  Thanks to
>      testing of the drm driver change to find this issue.
>    - documentation tweaks
>    - #include <linux/container_of.h> finally added to faux.h
> 
> 
> Changes from v3:
>    - Dropped the USB phy porting, turned out to be incorrect, it really
>      did need a platform device
>    - converted more drivers to the faux_device api (tlclk, lis3lv02d,
>      vgem, and vkms)
>    - collected some reviewed-by
>    - lots of minor tweaks of the faux.c api, and documentation based on
>      review, see the changelog in patch 1 for details.
> 
> Changes from v2:
>    - lots of cleanups to faux.c based on reviews, see patch 1 for details
>    - actually tested the destroy device path, it worked first try!
>    - added 3 more example drivers
> 
> 
> 
> Greg Kroah-Hartman (8):
>    driver core: add a faux bus for use when a simple device/bus is needed
>    regulator: dummy: convert to use the faux device interface
>    x86/microcode: move away from using a fake platform device
>    wifi: cfg80211: move away from using a fake platform device
>    tlclk: convert to use faux_device
>    misc: lis3lv02d: convert to use faux_device
>    drm/vgem/vgem_drv convert to use faux_device
>    drm/vkms: convert to use faux_device
> 
> Lyude Paul (1):
>    rust/kernel: Add faux device bindings
> 
>   Documentation/driver-api/infrastructure.rst |   6 +
>   MAINTAINERS                                 |   2 +
>   arch/x86/kernel/cpu/microcode/core.c        |  14 +-
>   drivers/base/Makefile                       |   2 +-
>   drivers/base/base.h                         |   1 +
>   drivers/base/faux.c                         | 232 ++++++++++++++++++++
>   drivers/base/init.c                         |   1 +
>   drivers/char/tlclk.c                        |  32 +--
>   drivers/gpu/drm/vgem/vgem_drv.c             |  30 +--
>   drivers/gpu/drm/vkms/vkms_drv.c             |  28 +--
>   drivers/gpu/drm/vkms/vkms_drv.h             |   4 +-
>   drivers/misc/lis3lv02d/lis3lv02d.c          |  26 +--
>   drivers/misc/lis3lv02d/lis3lv02d.h          |   4 +-
>   drivers/regulator/dummy.c                   |  37 +---
>   include/linux/device/faux.h                 |  69 ++++++
>   net/wireless/reg.c                          |  28 +--
>   rust/bindings/bindings_helper.h             |   1 +
>   rust/kernel/faux.rs                         |  67 ++++++
>   rust/kernel/lib.rs                          |   1 +
>   samples/rust/Kconfig                        |  10 +
>   samples/rust/Makefile                       |   1 +
>   samples/rust/rust_driver_faux.rs            |  29 +++
>   22 files changed, 502 insertions(+), 123 deletions(-)
>   create mode 100644 drivers/base/faux.c
>   create mode 100644 include/linux/device/faux.h
>   create mode 100644 rust/kernel/faux.rs
>   create mode 100644 samples/rust/rust_driver_faux.rs
> 

-- 
Louis Chauvet, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Powered by blists - more mailing lists