Re: [RFC V1 0/8] CPUFreq: create platform-dev for DT based cpufreq drivers

[Arnd Bergmann <arnd@arndb.de>]

On Monday 01 December 2014 18:59:20 Viresh Kumar wrote:
> On 1 December 2014 at 18:24, Arnd Bergmann <arnd@arndb.de> wrote:
> > Thanks a lot for working on this, we really need to figure it out one day!
> 
> :)
> 
> > Your patches seem well-implemented, so if everybody thinks the general
> > approach is the best solution, we should do that. From my point of view,
> > there are two things I would do differently:
> >
> > - In the DT binding, I would strongly prefer anything but the root compatible
> >   property as the key for the new platforms. Clearly we have to keep using
> >   it for the backwards-compatibility case, as you do, but I think there
> >   are more appropriate places to put it. Sorting from most favorite to least
> >   favorite, my list would be:
> >         1. a new property in /cpus/
> >         2. a new property each /cpus/cpu@... node.
> 
> I did it this way earlier and named it dvfs-method but probably putting this
> into the /cpus/ node is far better. But then Sudeep asked to utilize
> compatible property only..
> 
> Are you fine with the name here? "dvfs-method"

No objection here, whatever makes sense to you.

> > +static bool dt_cpufreq_available(void)
> > +{
> > +       struct device_node *node;
> > +       bool ret;
> > +
> > +       node = of_find_node_by_path("/cpus");
> > +       if (!node)
> > +               return 0;
> > +
> > +       /* the specific property needs to be debated */
> > +       ret = of_property_read_bool("linux,cpu-frequency-scaling");
> 
> How can this be a bool? We need to differentiate on which binding
> wants to go for arm-bl or cupfreq-dt or any other driver. So we surely
> need a string ?

I guess a string would be better here, the idea here was to
have a different bool property per driver, which would also
work.

> > @@ -367,29 +404,19 @@ static int dt_cpufreq_probe(struct platform_device *pdev)
> >         if (!IS_ERR(cpu_reg))
> >                 regulator_put(cpu_reg);
> >
> > -       dt_cpufreq_driver.driver_data = dev_get_platdata(&pdev->dev);
> > -
> 
> We still need this, and its about how clocks are shared between CPUs.

I didn't see where this comes from. Who is setting up this platform
data?

	Arnd