Re: [PATCH V4 8/8] cpufreq: Add Rust based cpufreq-dt driver
From: Greg KH
Date: Thu Jul 11 2024 - 13:22:16 EST
On Thu, Jul 11, 2024 at 06:41:29PM +0200, Greg KH wrote:
> On Thu, Jul 11, 2024 at 06:34:22PM +0200, Greg KH wrote:
> > On Thu, Jul 11, 2024 at 06:12:08PM +0200, Danilo Krummrich wrote:
> > > On Thu, Jul 11, 2024 at 04:37:50PM +0200, Greg KH wrote:
> > > > On Thu, Jul 11, 2024 at 03:21:31PM +0200, Danilo Krummrich wrote:
> > > > > (2) You require drivers to always implement a "dummy" struct platform_device,
> > > > > there is platform_device_register_simple() for that purpose.
> > > >
> > > > No, NEVER do that. platform devices are only for real platform devices,
> > > > do not abuse that interface any more than it already is.
> > >
> > > I thought we're talking about cases like [1] or [2], but please correct me if
> > > those are considered abusing the platform bus as well.
> > >
> > > (Those drivers read the CPU OF nodes, instead of OF nodes that represent a
> > > separate device.)
> > >
> > > [1] https://elixir.bootlin.com/linux/latest/source/drivers/cpuidle/cpuidle-riscv-sbi.c#L586
> > > [2] https://elixir.bootlin.com/linux/latest/source/drivers/cpuidle/cpuidle-psci.c#L441
> >
> > Yes, these are abuses of that and should be virtual devices as they have
> > nothing to do with the platform bus.
> >
> > > > > I think (2) is the preferred option.
> > > >
> > > > No, not at all, sorry.
> > > >
> > > > Use a real device, you have one somewhere that relates to this hardware,
> > > > otherwise you aren't controlling anything and then you can use a virtual
> > > > device.
> > >
> > > Of course we should stick to a real device if there is one, I didn't meant to
> > > say anything else.
> > >
> > > But since it came up now, some virtual drivers still require a parent device.
> >
> > Great, use the default one that the kernel gives you :)
> >
> > > For instance, in DRM we have vGEM [3] and vKMS [4], that use
> > > platform_device_register_simple() for this purpose.
> >
> > Again, abuse, please do not do so.
> >
> > > What should they use instead? I'm happy to fix things up if required.
> > >
> > > [3] https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/vgem/vgem_drv.c
> > > [4] https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/vkms/vkms_drv.c
> >
> > Use the virtual device interface please, that's what it is there for.
>
> To be specific, look at the devices in /sys/devices/virtual/ that's
> where yours should be showing up in, not in the root of /sys/devices/
> like they are by creating a "fake" platform device at the root of the
> device tree.
Ok, at first glance this seems a little bit more complex than what the
platform api gives you, let me knock something up next week during the
merge window to make this more simple and then let some interns at it to
sweep the kernel tree and fix up this proliferation of platform device
abuse.
thanks,
greg k-h