Is cpufreq-dt safe without regulator support?

Is cpufreq-dt safe without regulator support?

[wens Tsai]

Hi Viresh,

The sunxi platform added cpufreq support in 4.0.0-rc1 using
the generic cpufreq-dt driver. One issue we've run into is
the default OPPs we use include an overclocked/overvoltage
setting.

If the cpu is missing a regulator supply phandle, or the
kernel is built without regulators, cpufreq can increase
the clock rate without the needed voltage increase.

The former issue can be resolved by providing a dummy
regulator with the default voltage, effectively disabling
cpufreq.

The latter is possible. Without regulator support built-in,
even a correct, complete DTS can still fail. Should I just
remove the offending OPP? Still that does not fix the
problem when we actually do want overclocking.

To summarize, should cpufreq-dt probe fail when regulator
support isn't available? Should we differentiate between
not having a regulator phandle vs no regulator support?


Regards,
ChenYu

Is cpufreq-dt safe without regulator support?

[Viresh Kumar]

On 17 March 2015 at 13:15, wens Tsai <wens213@gmail.com> wrote:
> The sunxi platform added cpufreq support in 4.0.0-rc1 using
> the generic cpufreq-dt driver. One issue we've run into is
> the default OPPs we use include an overclocked/overvoltage
> setting.

There is a feature for that in cpufreq, in case you don't know, "boost"..

> If the cpu is missing a regulator supply phandle, or the
> kernel is built without regulators, cpufreq can increase
> the clock rate without the needed voltage increase.

Right.

> The former issue can be resolved by providing a dummy
> regulator with the default voltage, effectively disabling
> cpufreq.

How ? CPUFreq will try to change both volt and freq. Volt
wouldn't change as its a dummy regulator, but freq would
still be changed..

> The latter is possible. Without regulator support built-in,
> even a correct, complete DTS can still fail. Should I just
> remove the offending OPP? Still that does not fix the
> problem when we actually do want overclocking.
>
> To summarize, should cpufreq-dt probe fail when regulator
> support isn't available? Should we differentiate between

No. We can have regulator support enabled but no regulator
for a CPU. And so the current state of code looks fine.

> not having a regulator phandle vs no regulator support?

What I couldn't understand is, why would you miss the regulator
phandle in dts? Or not compile regulator support?

In case you want cpufreq-dt to be disabled if regulators aren't
compiled in, then what you should do is, don't create the platform
device required for probing cpufreq-dt driver.

Is cpufreq-dt safe without regulator support?

[Lucas Stach]

Am Dienstag, den 17.03.2015, 15:45 +0800 schrieb wens Tsai:
> Hi Viresh,
> 
> The sunxi platform added cpufreq support in 4.0.0-rc1 using
> the generic cpufreq-dt driver. One issue we've run into is
> the default OPPs we use include an overclocked/overvoltage
> setting.
> 
> If the cpu is missing a regulator supply phandle, or the
> kernel is built without regulators, cpufreq can increase
> the clock rate without the needed voltage increase.
> 
> The former issue can be resolved by providing a dummy
> regulator with the default voltage, effectively disabling
> cpufreq.
> 
> The latter is possible. Without regulator support built-in,
> even a correct, complete DTS can still fail. Should I just
> remove the offending OPP? Still that does not fix the
> problem when we actually do want overclocking.
> 
> To summarize, should cpufreq-dt probe fail when regulator
> support isn't available? Should we differentiate between
> not having a regulator phandle vs no regulator support?
> 
If you have an OPP in the DTS that is not stable with the boot voltage
then you need to handle this case in you platform code by disabling this
OPP if no regulator support is built into the kernel.

You can look at the i.MX code to see how this can be done. It is used
there to disable an OPP depending on fuse settings.

Regards,
Lucas
-- 
Pengutronix e.K.             | Lucas Stach                 |
Industrial Linux Solutions   | http://www.pengutronix.de/  |

Is cpufreq-dt safe without regulator support?

[Nishanth Menon]

On Tue, Mar 17, 2015 at 1:22 PM, Lucas Stach <l.stach@pengutronix.de> wrote:
> Am Dienstag, den 17.03.2015, 15:45 +0800 schrieb wens Tsai:
>> Hi Viresh,
>>
>> The sunxi platform added cpufreq support in 4.0.0-rc1 using
>> the generic cpufreq-dt driver. One issue we've run into is
>> the default OPPs we use include an overclocked/overvoltage
>> setting.
>>
>> If the cpu is missing a regulator supply phandle, or the
>> kernel is built without regulators, cpufreq can increase
>> the clock rate without the needed voltage increase.
>>
>> The former issue can be resolved by providing a dummy
>> regulator with the default voltage, effectively disabling
>> cpufreq.
>>
>> The latter is possible. Without regulator support built-in,
>> even a correct, complete DTS can still fail. Should I just
>> remove the offending OPP? Still that does not fix the
>> problem when we actually do want overclocking.
>>
>> To summarize, should cpufreq-dt probe fail when regulator
>> support isn't available? Should we differentiate between
>> not having a regulator phandle vs no regulator support?
>>
> If you have an OPP in the DTS that is not stable with the boot voltage
> then you need to handle this case in you platform code by disabling this
> OPP if no regulator support is built into the kernel.
>
> You can look at the i.MX code to see how this can be done. It is used
> there to disable an OPP depending on fuse settings.


Something we want to do generically?

---
Regards,
Nishanth Menon

Is cpufreq-dt safe without regulator support?

[wens Tsai]

On Tue, Mar 17, 2015 at 4:47 PM, Viresh Kumar <viresh.kumar@linaro.org> wrote:
> On 17 March 2015 at 13:15, wens Tsai <wens213@gmail.com> wrote:
>> The sunxi platform added cpufreq support in 4.0.0-rc1 using
>> the generic cpufreq-dt driver. One issue we've run into is
>> the default OPPs we use include an overclocked/overvoltage
>> setting.
>
> There is a feature for that in cpufreq, in case you don't know, "boost"..

I do now. Though cpufreq-dt and the OPP bindings don't
support this. Any chance the next version of OPP bindings will?

>> If the cpu is missing a regulator supply phandle, or the
>> kernel is built without regulators, cpufreq can increase
>> the clock rate without the needed voltage increase.
>
> Right.
>
>> The former issue can be resolved by providing a dummy
>> regulator with the default voltage, effectively disabling
>> cpufreq.
>
> How ? CPUFreq will try to change both volt and freq. Volt
> wouldn't change as its a dummy regulator, but freq would
> still be changed..

I suppose this is the desired behavior. I'm fine with it then.

>> The latter is possible. Without regulator support built-in,
>> even a correct, complete DTS can still fail. Should I just
>> remove the offending OPP? Still that does not fix the
>> problem when we actually do want overclocking.
>>
>> To summarize, should cpufreq-dt probe fail when regulator
>> support isn't available? Should we differentiate between
>
> No. We can have regulator support enabled but no regulator
> for a CPU. And so the current state of code looks fine.
>
>> not having a regulator phandle vs no regulator support?
>
> What I couldn't understand is, why would you miss the regulator
> phandle in dts? Or not compile regulator support?

I admit it is partly bad execution on my part. I added the OPPs
to the .dtsi files, but only the regulators to a handful of boards
that I have. All the other boards do not have a regulator phandle.

> In case you want cpufreq-dt to be disabled if regulators aren't
> compiled in, then what you should do is, don't create the platform
> device required for probing cpufreq-dt driver.

Thanks for the suggestion. I'll add if(IS_ENABLED()) around it.
I can also check for the regulator phandle here.

Regards
ChenYu

Is cpufreq-dt safe without regulator support?

[Viresh Kumar]

On 18 March 2015 at 07:10, wens Tsai <wens213@gmail.com> wrote:
> On Tue, Mar 17, 2015 at 4:47 PM, Viresh Kumar <viresh.kumar@linaro.org> wrote:
>> On 17 March 2015 at 13:15, wens Tsai <wens213@gmail.com> wrote:
>>> The sunxi platform added cpufreq support in 4.0.0-rc1 using
>>> the generic cpufreq-dt driver. One issue we've run into is
>>> the default OPPs we use include an overclocked/overvoltage
>>> setting.
>>
>> There is a feature for that in cpufreq, in case you don't know, "boost"..
>
> I do now. Though cpufreq-dt and the OPP bindings don't
> support this. Any chance the next version of OPP bindings will?

Yes, it is part of the new bindings already..

>>> If the cpu is missing a regulator supply phandle, or the
>>> kernel is built without regulators, cpufreq can increase
>>> the clock rate without the needed voltage increase.
>>
>> Right.
>>
>>> The former issue can be resolved by providing a dummy
>>> regulator with the default voltage, effectively disabling
>>> cpufreq.
>>
>> How ? CPUFreq will try to change both volt and freq. Volt
>> wouldn't change as its a dummy regulator, but freq would
>> still be changed..
>
> I suppose this is the desired behavior. I'm fine with it then.
>
>>> The latter is possible. Without regulator support built-in,
>>> even a correct, complete DTS can still fail. Should I just
>>> remove the offending OPP? Still that does not fix the
>>> problem when we actually do want overclocking.
>>>
>>> To summarize, should cpufreq-dt probe fail when regulator
>>> support isn't available? Should we differentiate between
>>
>> No. We can have regulator support enabled but no regulator
>> for a CPU. And so the current state of code looks fine.
>>
>>> not having a regulator phandle vs no regulator support?
>>
>> What I couldn't understand is, why would you miss the regulator
>> phandle in dts? Or not compile regulator support?
>
> I admit it is partly bad execution on my part. I added the OPPs
> to the .dtsi files, but only the regulators to a handful of boards
> that I have. All the other boards do not have a regulator phandle.
>
>> In case you want cpufreq-dt to be disabled if regulators aren't
>> compiled in, then what you should do is, don't create the platform
>> device required for probing cpufreq-dt driver.
>
> Thanks for the suggestion. I'll add if(IS_ENABLED()) around it.
> I can also check for the regulator phandle here.

I think the right thing to do is move the OPP bindings to the .dts
files instead and get rid of all the hacks you are planning to add.

Is cpufreq-dt safe without regulator support?

[Maxime Ripard]

On Wed, Mar 18, 2015 at 09:23:54AM +0530, Viresh Kumar wrote:
> >>> If the cpu is missing a regulator supply phandle, or the
> >>> kernel is built without regulators, cpufreq can increase
> >>> the clock rate without the needed voltage increase.
> >>
> >> Right.
> >>
> >>> The former issue can be resolved by providing a dummy
> >>> regulator with the default voltage, effectively disabling
> >>> cpufreq.
> >>
> >> How ? CPUFreq will try to change both volt and freq. Volt
> >> wouldn't change as its a dummy regulator, but freq would
> >> still be changed..
> >
> > I suppose this is the desired behavior. I'm fine with it then.
> >
> >>> The latter is possible. Without regulator support built-in,
> >>> even a correct, complete DTS can still fail. Should I just
> >>> remove the offending OPP? Still that does not fix the
> >>> problem when we actually do want overclocking.
> >>>
> >>> To summarize, should cpufreq-dt probe fail when regulator
> >>> support isn't available? Should we differentiate between
> >>
> >> No. We can have regulator support enabled but no regulator
> >> for a CPU. And so the current state of code looks fine.
> >>
> >>> not having a regulator phandle vs no regulator support?
> >>
> >> What I couldn't understand is, why would you miss the regulator
> >> phandle in dts? Or not compile regulator support?
> >
> > I admit it is partly bad execution on my part. I added the OPPs
> > to the .dtsi files, but only the regulators to a handful of boards
> > that I have. All the other boards do not have a regulator phandle.
> >
> >> In case you want cpufreq-dt to be disabled if regulators aren't
> >> compiled in, then what you should do is, don't create the platform
> >> device required for probing cpufreq-dt driver.
> >
> > Thanks for the suggestion. I'll add if(IS_ENABLED()) around it.
> > I can also check for the regulator phandle here.
> 
> I think the right thing to do is move the OPP bindings to the .dts
> files instead and get rid of all the hacks you are planning to add.

Another solution would be to have the safe OPPs in the DTSI, and the
one that requires some PMIC support in the board DTS.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150318/deb480d3/attachment.sig>

Is cpufreq-dt safe without regulator support?

[Viresh Kumar]

On 18 March 2015 at 15:33, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:
> Another solution would be to have the safe OPPs in the DTSI, and the
> one that requires some PMIC support in the board DTS.

Yes .dtsi and .dts files do get merged, but I don't think that will
happen in case
of OPPs as well. As we are talking about a single field with any array of OPP
pairs.

Is cpufreq-dt safe without regulator support?

[Maxime Ripard]

On Wed, Mar 18, 2015 at 03:41:36PM +0530, Viresh Kumar wrote:
> On 18 March 2015 at 15:33, Maxime Ripard
> <maxime.ripard@free-electrons.com> wrote:
> > Another solution would be to have the safe OPPs in the DTSI, and the
> > one that requires some PMIC support in the board DTS.
> 
> Yes .dtsi and .dts files do get merged, but I don't think that will
> happen in case
> of OPPs as well. As we are talking about a single field with any array of OPP
> pairs.

Sorry, what I meant was to redefine the whole OPPs set in the DTS,
overriding the one in the DTSI.

That would add some duplication, but still far less than what you
first suggested in our case.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150318/4bd1ef80/attachment.sig>

Is cpufreq-dt safe without regulator support?

[Viresh Kumar]

On 18 March 2015 at 15:50, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:
> Sorry, what I meant was to redefine the whole OPPs set in the DTS,
> overriding the one in the DTSI.
>
> That would add some duplication, but still far less than what you
> first suggested in our case.

Okay, I wasn't aware of the fact that we can override entries this way.
But still it doesn't look right to me from readability point of view.

It would be very easy for people to confuse when they will see two
similar entries of OPP tables, one in dtsi and other one in dts.

Its okay if duplication is there, atleast its readable.

Over that, all this mess will get fixed up with the new OPP bindings,
which are currently under discussion over lists.

--
viresh