Hi Quentin, On Tue, Jan 29, 2019 at 09:03:34AM +0000, Quentin Perret wrote: > Hi Matthias, > > On Monday 28 Jan 2019 at 11:02:51 (-0800), Matthias Kaehlcke wrote: > > > +/** > > > + * of_dev_pm_opp_get_cpu_power() - Estimates the power of a CPU > > > + * @mW: pointer to the power estimate in milli-watts > > > + * @KHz: pointer to the OPP's frequency, in kilo-hertz > > > > nit: should be kHz > > Right, I can change that :-) > > > > > > + * @cpu: CPU for which power needs to be estimated > > > + * > > > + * Computes the power estimated by @CPU at the first OPP above @KHz (ceil), > > > + * and updates @KHz and @mW accordingly. > > > + * > > > + * The power is estimated as P = C * V^2 * f, with C the CPU's capacitance > > > + * (read from the 'dynamic-power-coefficient' devicetree binding) and V and f > > > + * respectively the voltage and frequency of the OPP. > > > + * > > > + * Return: -ENODEV if the CPU device cannot be found, -EINVAL if the power > > > + * calculation failed because of missing parameters, 0 otherwise. > > > + */ > > > +int of_dev_pm_opp_get_cpu_power(unsigned long *mW, unsigned long *KHz, int cpu) > > > > I think it is more common to put the input parameters first, then the > > output ones, i.e. cpu, kHz, mW. > > Hmm, the issue is this must match the expectations of the EM framework: > > https://elixir.bootlin.com/linux/v5.0-rc4/source/include/linux/energy_model.h#L62 > > So, I don't really have a choice. Or we can change the core code if this > _really_ is a problem -- we don't have users yet so now is the best time > to do so I guess ... I see. I think it would be preferable to follow the common scheme, but it's also not a major issue, up to you whether you want to change the definition of the callback. It's certainly true that now would be the best time for a change. Cheers Matthias
