Hi everyone, Thanks for CCing me Louis, On Thu, Feb 27, 2025 at 02:06:21PM +0100, Louis Chauvet wrote: > > > 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? I made a quick test with drm_info [1] and VKMS is not listed after moving it to the faux bus. As far as I can tell, the reason is that drmGetDevices() doesn't return the devices in the faux bus. However, as drm_info is designed to list information as close as it is provided by libdrm, I'm not sure if this should be considered a regressions or not. Just for reference, Mutter handles the bus change correctly, and it uses udev [2]. I think I should do something similar in my IGT tests and use udev instead of drmGetDevices(). [1] https://gitlab.freedesktop.org/emersion/drm_info [2] https://gitlab.gnome.org/GNOME/mutter/-/blob/main/src/backends/meta-udev.c?ref_type=heads#L174 > 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@xxxxxxxxx/ > > > 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 In regard of the VKMS patch, in Documentation/gpu/vkms.rst, "IGT_DEVICE" needs to be updated to point to the new path. It can be done in this series or I can send a follow up patch. Best wishes, Jose > > > > 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 >
