| Message ID | 2025021023-sandstorm-precise-9f5d@gregkh (mailing list archive) |
|---|---|
| Headers | show |
| Series | Driver core: Add faux bus devices | expand |
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 >
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? > > 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 My 2 cents is that. The library should be prepared for the change. AFAIU the concept of sys/bus the user space is supposed to check all as the same device theoretically may float from one bus to another.
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? > > 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). Why does a userspace tool want to walk bus types? Shouldn't it just be iterating over the userspace class type instead? classes are how devices are exposed to userspace, not through a bus. That way if there is a new bus type tomorrow (like this one), code will just keep working. What does the tool actually do in the platform device's directory? thanks, greg k-h
On Thu, Feb 27, 2025 at 07:30:29AM -0800, Greg Kroah-Hartman wrote: > 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? > > > > 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). > > Why does a userspace tool want to walk bus types? Shouldn't it just be > iterating over the userspace class type instead? classes are how > devices are exposed to userspace, not through a bus. That way if there > is a new bus type tomorrow (like this one), code will just keep working. > > What does the tool actually do in the platform device's directory? Yeah this should work. In the past, mostly for historical reasons (pci enumeration in Xserver due to everything being userspace drivers) this wasn't the case. But modern drm drivers should go hunt for a compatible drm_driver name, enumerating all drm devices of the right class (legacy aka display or render or accel), because that string name is the uapi promise for the driver-specific uapi. Anything that uses generic drm apis like kernel modesetting shouldn't care, unless you've managed to hard-code your device path in your config somewhere. But almost everything does automatic setup nowadays, at least as a fallback. Plus vgem and vkms are mostly for validation, that stuff we can fix without annoying real users. It's kinda like breaking debugfs, which you need anyway for running most of the igt testcases. tldr; I'm not worried, and if something breaks we need and can fix it. Cheers, Sima
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@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 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 >
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. 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