On 16/01/2020 16:20, lukasz.luba@xxxxxxx wrote: > From: Lukasz Luba <lukasz.luba@xxxxxxx> > > Add support of other devices into the Energy Model framework not only the > CPUs. Change the interface to be more unified which can handle other > devices as well. [...] > -The source of the information about the power consumed by CPUs can vary greatly > +The source of the information about the power consumed by devices can vary greatly > from one platform to another. These power costs can be estimated using > devicetree data in some cases. In others, the firmware will know better. > Alternatively, userspace might be best positioned. And so on. In order to avoid > @@ -26,7 +28,7 @@ framework, and interested clients reading the data from it:: > | Thermal (IPA) | | Scheduler (EAS) | | Other | > +---------------+ +-----------------+ +---------------+ > | | em_pd_energy() | > - | | em_cpu_get() | > + | em_dev_get() | em_cpu_get() | Looked really hard but can't find a em_dev_get() in the code? You mean em_get_pd() ? And why em_get_pd() and not em_pd_get()? > +---------+ | +---------+ > | | | > v v v > @@ -47,12 +49,12 @@ framework, and interested clients reading the data from it:: > | Device Tree | | Firmware | | ? | > +--------------+ +---------------+ +--------------+ [...] > +There is two API functions which provide the access to the energy model: > +em_cpu_get() which takes CPU id as an argument and em_dev_get() with device > +pointer as an argument. It depends on the subsystem which interface it is > +going to use. Would be really nice if this wouldn't be required. We should really aim for 1 framework == 1 set of interfaces. What happens if someone calls em_get_pd() on a CPU EM? E.g: static struct perf_domain *pd_init(int cpu) { - struct em_perf_domain *obj = em_cpu_get(cpu); + struct device *dev = get_cpu_device(cpu); + struct em_perf_domain *obj = em_pd_get(dev); struct perf_domain *pd; Two versions of one functionality will confuse API user from the beginning ... [...] > +enum em_type { > + EM_SIMPLE, > + EM_CPU, > +}; s/EM_SIMPLE/EM_DEV ? Right now I only see energy models and _one_ specific type (the CPU EM). So a tag 'is a CPU EM' would suffice. No need for EM_SIMPE ... [...]