| Message ID | b1601c1dbdf68a4b812fcfaf73f3b5dea67f2025.1720680252.git.viresh.kumar@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Rust bindings for cpufreq and OPP core + sample driver | expand |
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 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. Again, do NOT abuse the platform subsystem. It's one reason I am loath to even want to allow rust bindings to it. greg k-h
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 > > > 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. For instance, in DRM we have vGEM [3] and vKMS [4], that use platform_device_register_simple() for this purpose. 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 > > Again, do NOT abuse the platform subsystem. It's one reason I am loath > to even want to allow rust bindings to it. How is that related to Rust? > > greg k-h >
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. thanks, greg k-h
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. thanks, greg k-h
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
On Thu, Jul 11, 2024 at 07:21:59PM +0200, Greg KH wrote: > 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. Yeah, I stared at this for the last 30 minutes and was just about to reply. I think that we probably want to get rid of this abuse, as there are quite a lot of examples for this. And considering that I wasn't able to find a rather straight forward replacement that makes it go into /sys/devices/virtual/ I think it's not super unexpected that this spreads. It looks like we probably want some kind virtual device API for that purpose? > > thanks, > > greg k-h >
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. For those drivers, wouldn't it be better if proper devices would be derived from the CPU OF nodes directly? This seems to be a common problem for cpuidle and cpufreq drivers. But it's quite a while ago I dealt with such drivers, maybe there are reasons not to do so. Anyway, using a virtual device for those seems a bit wrong to me. - Danilo
On Tue, Jul 16, 2024 at 05:15:25PM +0200, Danilo Krummrich 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. > > For those drivers, wouldn't it be better if proper devices would be derived from > the CPU OF nodes directly? This seems to be a common problem for cpuidle and > cpufreq drivers. Yes they should. > But it's quite a while ago I dealt with such drivers, maybe there are reasons > not to do so. I think people just got lazy :) thanks, greg k-h
On Tue, Jul 16, 2024 at 9:22 AM Greg KH <greg@kroah.com> wrote: > > On Tue, Jul 16, 2024 at 05:15:25PM +0200, Danilo Krummrich 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. > > > > For those drivers, wouldn't it be better if proper devices would be derived from > > the CPU OF nodes directly? This seems to be a common problem for cpuidle and > > cpufreq drivers. > > Yes they should. Well, which one do we bind? The cpufreq driver or cpuidle driver? Or there's the thermal f/w throttling as well. It's messy. Also, the CPUs already have a struct device associated with them for the topology stuff, but no driver IIRC. Another complication is it is not the CPU that determines what cpufreq/cpuidle drivers to use, but a platform decision. That decision may evolve as well which means it can't be driven from the DT. > > But it's quite a while ago I dealt with such drivers, maybe there are reasons > > not to do so. > > I think people just got lazy :) Virtual device was probably the right thing given there isn't directly any device we are controlling/programming. This driver is just built on top of other subsystems (clock and regulator). Rob
On Tue, Jul 16, 2024 at 09:53:15AM -0600, Rob Herring wrote: > On Tue, Jul 16, 2024 at 9:22 AM Greg KH <greg@kroah.com> wrote: > > > > On Tue, Jul 16, 2024 at 05:15:25PM +0200, Danilo Krummrich 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. > > > > > > For those drivers, wouldn't it be better if proper devices would be derived from > > > the CPU OF nodes directly? This seems to be a common problem for cpuidle and > > > cpufreq drivers. > > > > Yes they should. > > Well, which one do we bind? The cpufreq driver or cpuidle driver? Or > there's the thermal f/w throttling as well. It's messy. Also, the CPUs > already have a struct device associated with them for the topology > stuff, but no driver IIRC. I did not mean to say that they should bind to the CPU nodes compatible string, but rather have devices populated from sub-nodes of the CPU / CPU cluster nodes or from the SoC's 'simple-bus' or whatever makes sense for the specific HW. Generally, I think there should be something in the DT that populates the corresponding device, rather than having a virtual device. The device isn't really virtual, it controls some real HW. > > Another complication is it is not the CPU that determines what > cpufreq/cpuidle drivers to use, but a platform decision. That decision > may evolve as well which means it can't be driven from the DT. Often it's SoC specific, but that should be fine, right? Or do you mean something else? > > > > But it's quite a while ago I dealt with such drivers, maybe there are reasons > > > not to do so. > > > > I think people just got lazy :) > > Virtual device was probably the right thing given there isn't directly > any device we are controlling/programming. This driver is just built > on top of other subsystems (clock and regulator). The two examples I gave use a firmware interface to control the CPU's idle state. But even for the case you mention here, I'd still argue that the driver controls some real hardware, just not directly. It controls the semantics and is still HW specific. Having a dedicated DT node also makes it easy to provide the resources, e.g. regulators and clocks. - Danilo > > Rob >
diff --git a/drivers/cpufreq/Kconfig b/drivers/cpufreq/Kconfig index 94e55c40970a..eb9359bd3c5c 100644 --- a/drivers/cpufreq/Kconfig +++ b/drivers/cpufreq/Kconfig @@ -217,6 +217,18 @@ config CPUFREQ_DT If in doubt, say N. +config CPUFREQ_DT_RUST + tristate "Rust based Generic DT based cpufreq driver" + depends on HAVE_CLK && OF && RUST + select CPUFREQ_DT_PLATDEV + select PM_OPP + help + This adds a Rust based generic DT based cpufreq driver for frequency + management. It supports both uniprocessor (UP) and symmetric + multiprocessor (SMP) systems. + + If in doubt, say N. + config CPUFREQ_DT_PLATDEV tristate "Generic DT based cpufreq platdev driver" depends on OF diff --git a/drivers/cpufreq/Makefile b/drivers/cpufreq/Makefile index 8d141c71b016..4981d908b803 100644 --- a/drivers/cpufreq/Makefile +++ b/drivers/cpufreq/Makefile @@ -15,6 +15,7 @@ obj-$(CONFIG_CPU_FREQ_GOV_COMMON) += cpufreq_governor.o obj-$(CONFIG_CPU_FREQ_GOV_ATTR_SET) += cpufreq_governor_attr_set.o obj-$(CONFIG_CPUFREQ_DT) += cpufreq-dt.o +obj-$(CONFIG_CPUFREQ_DT_RUST) += rcpufreq_dt.o obj-$(CONFIG_CPUFREQ_DT_PLATDEV) += cpufreq-dt-platdev.o # Traces diff --git a/drivers/cpufreq/rcpufreq_dt.rs b/drivers/cpufreq/rcpufreq_dt.rs new file mode 100644 index 000000000000..3e86ad134e13 --- /dev/null +++ b/drivers/cpufreq/rcpufreq_dt.rs @@ -0,0 +1,222 @@ +// SPDX-License-Identifier: GPL-2.0 + +//! Rust based implementation of the cpufreq-dt driver. + +use core::format_args; + +use kernel::{ + b_str, c_str, clk, cpufreq, cpumask::Cpumask, define_of_id_table, device::Device, + devres::Devres, error::code::*, fmt, macros::vtable, module_platform_driver, of, opp, platform, + prelude::*, str::CString, sync::Arc, +}; + +// Finds exact supply name from the OF node. +fn find_supply_name_exact(np: &of::DeviceNode, name: &str) -> Option<CString> { + let name_cstr = CString::try_from_fmt(fmt!("{}-supply", name)).ok()?; + + np.find_property(&name_cstr).ok()?; + CString::try_from_fmt(fmt!("{}", name)).ok() +} + +// Finds supply name for the CPU from DT. +fn find_supply_names(dev: &Device, cpu: u32) -> Option<Vec<CString>> { + let np = of::DeviceNode::from_dev(dev).ok()?; + + // Try "cpu0" for older DTs. + let name = match cpu { + 0 => find_supply_name_exact(&np, "cpu0"), + _ => None, + } + .or(find_supply_name_exact(&np, "cpu"))?; + + let mut list = Vec::with_capacity(1, GFP_KERNEL).ok()?; + list.push(name, GFP_KERNEL).ok()?; + + Some(list) +} + +// Represents the cpufreq dt device. +struct CPUFreqDTDevice { + opp_table: opp::Table, + freq_table: opp::FreqTable, + #[allow(dead_code)] + mask: Cpumask, + #[allow(dead_code)] + token: Option<opp::ConfigToken>, + #[allow(dead_code)] + clk: clk::Clk, +} + +struct CPUFreqDTDriver; + +#[vtable] +impl opp::ConfigOps for CPUFreqDTDriver {} + +#[vtable] +impl cpufreq::Driver for CPUFreqDTDriver { + type Data = (); + type PData = Arc<CPUFreqDTDevice>; + + fn init(policy: &mut cpufreq::Policy) -> Result<Self::PData> { + let cpu = policy.cpu(); + let dev = Device::from_cpu(cpu)?; + let mut mask = Cpumask::new()?; + + mask.set(cpu); + + let token = match find_supply_names(&dev, cpu) { + Some(names) => Some( + opp::Config::<Self>::new() + .set_regulator_names(names)? + .set(&dev)?, + ), + _ => None, + }; + + // Get OPP-sharing information from "operating-points-v2" bindings. + let fallback = match opp::Table::of_sharing_cpus(&dev, &mut mask) { + Ok(_) => false, + Err(e) => { + if e != ENOENT { + return Err(e); + } + + // "operating-points-v2" not supported. If the platform hasn't + // set sharing CPUs, fallback to all CPUs share the `Policy` + // for backward compatibility. + opp::Table::sharing_cpus(&dev, &mut mask).is_err() + } + }; + + // Initialize OPP tables for all policy cpus. + // + // For platforms not using "operating-points-v2" bindings, we do this + // before updating policy cpus. Otherwise, we will end up creating + // duplicate OPPs for the CPUs. + // + // OPPs might be populated at runtime, don't fail for error here unless + // it is -EPROBE_DEFER. + let mut opp_table = match opp::Table::from_of_cpumask(&dev, &mask) { + Ok(table) => table, + Err(e) => { + if e == EPROBE_DEFER { + return Err(e); + } + + // The table is added dynamically ? + opp::Table::from_dev(&dev)? + } + }; + + // The OPP table must be initialized, statically or dynamically, by this point. + opp_table.opp_count()?; + + // Set sharing cpus for fallback scenario. + if fallback { + mask.set_all(); + opp_table.set_sharing_cpus(&mask)?; + } + + let mut transition_latency = opp_table.max_transition_latency() as u32; + if transition_latency == 0 { + transition_latency = cpufreq::ETERNAL_LATENCY; + } + + let freq_table = opp_table.to_cpufreq_table()?; + let clk = policy + .set_freq_table(freq_table.table()) + .set_dvfs_possible_from_any_cpu() + .set_suspend_freq((opp_table.suspend_freq() / 1000) as u32) + .set_transition_latency(transition_latency) + .set_clk(&dev, None)?; + + mask.copy(policy.cpus()); + + Ok(Arc::new( + CPUFreqDTDevice { + opp_table, + freq_table, + mask, + token, + clk, + }, + GFP_KERNEL, + )?) + } + + fn exit(_policy: &mut cpufreq::Policy, _data: Option<Self::PData>) -> Result<()> { + Ok(()) + } + + fn online(_policy: &mut cpufreq::Policy) -> Result<()> { + // We did light-weight tear down earlier, nothing to do here. + Ok(()) + } + + fn offline(_policy: &mut cpufreq::Policy) -> Result<()> { + // Preserve policy->data and don't free resources on light-weight + // tear down. + Ok(()) + } + + fn suspend(policy: &mut cpufreq::Policy) -> Result<()> { + policy.generic_suspend() + } + + fn verify(data: &mut cpufreq::PolicyData) -> Result<()> { + data.generic_verify() + } + + fn target_index(policy: &mut cpufreq::Policy, index: u32) -> Result<()> { + let data = match policy.data::<Self::PData>() { + Some(data) => data, + None => return Err(ENOENT), + }; + + let freq = data.freq_table.freq(index.try_into().unwrap())? as u64; + data.opp_table.set_rate(freq * 1000) + } + + fn get(policy: &mut cpufreq::Policy) -> Result<u32> { + policy.generic_get() + } + + fn set_boost(_policy: &mut cpufreq::Policy, _state: i32) -> Result<()> { + Ok(()) + } + + fn register_em(policy: &mut cpufreq::Policy) { + policy.register_em_opp() + } +} + +impl platform::Driver for CPUFreqDTDriver { + type Data = (); + + define_of_id_table! {(), [ + (of::DeviceId(b_str!("operating-points-v2")), None), + ]} + + fn probe(dev: &mut platform::Device, _id_info: Option<&Self::IdInfo>) -> Result<Self::Data> { + let drv = cpufreq::Registration::<CPUFreqDTDriver>::register( + c_str!("cpufreq-dt"), + (), + cpufreq::flags::NEED_INITIAL_FREQ_CHECK | cpufreq::flags::IS_COOLING_DEV, + true, + )?; + + Devres::new_foreign_owned(dev.as_ref(), drv, GFP_KERNEL)?; + + pr_info!("CPUFreq DT driver registered\n"); + + Ok(()) + } +} + +module_platform_driver! { + type: CPUFreqDTDriver, + name: "cpufreq_dt", + author: "Viresh Kumar <viresh.kumar@linaro.org>", + description: "Generic CPUFreq DT driver", + license: "GPL v2", +}
This commit adds a Rust based cpufreq-dt driver, which covers most of the functionality of the existing C based driver. Only a handful of things are left, like fetching platform data from cpufreq-dt-platdev.c. This is tested with the help of QEMU for now and switching of frequencies work as expected. Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> --- drivers/cpufreq/Kconfig | 12 ++ drivers/cpufreq/Makefile | 1 + drivers/cpufreq/rcpufreq_dt.rs | 222 +++++++++++++++++++++++++++++++++ 3 files changed, 235 insertions(+) create mode 100644 drivers/cpufreq/rcpufreq_dt.rs