Re: [Letux-kernel] [RFC PATCH 0/3] Enable 1GHz support on omap36xx

Re: [Letux-kernel] [RFC PATCH 0/3] Enable 1GHz support on omap36xx

[H. Nikolaus Schaller]

Hi Adam,

> Am 02.09.2019 um 23:10 schrieb Adam Ford <aford173@gmail.com>:
> 
> On Mon, Sep 2, 2019 at 10:46 AM H. Nikolaus Schaller <hns@goldelico.com> wrote:
>> 
>> 
>> 
>> But my tests show that decoding works now. So you already might give it a try.
> 
> I am traveling all this week, but I have an omap3530, DM3730
> (omap3630), and an AM3517 that I use for testing.

now as the omap3430 and omap3630 opp-v2 tables are installed,
we could add am35x7 as well.

What needs to be done:

1. add OPP-v2 table to am3517.dtsi

for example copy skeleton from omap36xx.dtsi

and define reasonable clock speeds. I would think about
150 MHz, 300 MHz, 600MHz.

Debatable is if we need a clock-latency definition.

2. change all voltages to 1.2V

			opp-microvolt = <1200000 1200000 1200000>;

There is no point to specify 3 voltages <target min max> here since we
will never need a min and a max value.

			opp-microvolt = <1200000>;

should also be ok (AFAIK, parser handles single-value records).

3. AFAIK there is no speed binned eFuse...

But the ti-cpufreq driver always wants to read some eFuse register.

So please check if you can read 0x4800244C and 0x4830A204
like on omap36xx and if they produce stable values (and not
random noise).

If yes, we simply assume that am3517 is similar enough to omap3630,
ignore that there is no eFuse, but read the register anyways and
then ignore the bit if it is 0 or 1.

This means that all OPPs can get

			opp-supported-hw = <0xffffffff 0xffffffff>;

There could also be a default handler if this property is missing,
but I have not researched this.

4. add compatible to ti-cpufreq
and share the register offsets, bit masks etc. with omap3630:

	{ .compatible = "ti,am33xx", .data = &am3x_soc_data, },
	{ .compatible = "ti,am3517", .data = &omap36xx_soc_data, },
	{ .compatible = "ti,am43", .data = &am4x_soc_data, },
	{ .compatible = "ti,dra7", .data = &dra7_soc_data },
	{ .compatible = "ti,omap3430", .data = &omap34xx_soc_data, },
	{ .compatible = "ti,omap3630", .data = &omap36xx_soc_data, },

5. configure for CONFIG_ARM_TI_CPUFREQ=y

This should IMHO suffice.

Since I can't test anything I can't define working OPP points
and therefore I can't provide patches myself. Hope you can make
it work this way.

BR,
Nikolaus

Re: [Letux-kernel] [RFC PATCH 0/3] Enable 1GHz support on omap36xx

[Adam Ford]

On Sat, Sep 7, 2019 at 2:38 AM H. Nikolaus Schaller <hns@goldelico.com> wrote:
>
> Hi Adam,
>
> > Am 02.09.2019 um 23:10 schrieb Adam Ford <aford173@gmail.com>:
> >
> > On Mon, Sep 2, 2019 at 10:46 AM H. Nikolaus Schaller <hns@goldelico.com> wrote:
> >>
> >>
> >>
> >> But my tests show that decoding works now. So you already might give it a try.
> >
> > I am traveling all this week, but I have an omap3530, DM3730
> > (omap3630), and an AM3517 that I use for testing.
>
> now as the omap3430 and omap3630 opp-v2 tables are installed,
> we could add am35x7 as well.
>
> What needs to be done:
>
> 1. add OPP-v2 table to am3517.dtsi
>
> for example copy skeleton from omap36xx.dtsi
>
> and define reasonable clock speeds. I would think about
> 150 MHz, 300 MHz, 600MHz.

This might be more of a question for TI, but  can we match the 3430
list of frequencies?

Something like:
    125000  1200000
    250000  1200000
    500000  1200000
    550000  1200000
    600000  1200000


>
> Debatable is if we need a clock-latency definition.
>
> 2. change all voltages to 1.2V
>
>                         opp-microvolt = <1200000 1200000 1200000>;
>
> There is no point to specify 3 voltages <target min max> here since we
> will never need a min and a max value.
>
>                         opp-microvolt = <1200000>;
>
> should also be ok (AFAIK, parser handles single-value records).
>
> 3. AFAIK there is no speed binned eFuse...
>
> But the ti-cpufreq driver always wants to read some eFuse register.
>
> So please check if you can read 0x4800244C and 0x4830A204
> like on omap36xx and if they produce stable values (and not
> random noise).

For the AM3517,

0x4800244C = 0000 0cc0
0x4830A204 = 1b86 802f

The AM3517 shows these are valid addresses and I read them multiple
times and they yielded the same results even after power cycling.


>
> If yes, we simply assume that am3517 is similar enough to omap3630,
> ignore that there is no eFuse, but read the register anyways and
> then ignore the bit if it is 0 or 1.
>
> This means that all OPPs can get
>
>                         opp-supported-hw = <0xffffffff 0xffffffff>;
>
> There could also be a default handler if this property is missing,
> but I have not researched this.
>
Like this?

opp1-125000000 {
     opp-hz = /bits/ 64 <125000000>;
     opp-microvolt = <1200000>;
     opp-supported-hw = <0xffffffff 0xffffffff>;
};

opp2-250000000 {
     opp-hz = /bits/ 64 <250000000>;
     opp-microvolt = <1200000>;
    opp-supported-hw = <0xffffffff 0xffffffff>;
     opp-suspend;
};

opp3-500000000 {
     opp-hz = /bits/ 64 <500000000>;
     opp-microvolt = <1200000>;
     opp-supported-hw = <0xffffffff 0xffffffff>;
};

opp4-550000000 {
     opp-hz = /bits/ 64 <550000000>;
     opp-microvolt = <1200000>;
     opp-supported-hw = <0xffffffff 0xffffffff>;
};

opp5-600000000 {
     opp-hz = /bits/ 64 <600000000>;
     opp-microvolt = <1200000>;
     opp-supported-hw = <0xffffffff 0xffffffff>;
};

What does opp-suspend do?  I noticed it in the 34xx.dtsi

> 4. add compatible to ti-cpufreq
> and share the register offsets, bit masks etc. with omap3630:
>
>         { .compatible = "ti,am33xx", .data = &am3x_soc_data, },
>         { .compatible = "ti,am3517", .data = &omap36xx_soc_data, },
>         { .compatible = "ti,am43", .data = &am4x_soc_data, },
>         { .compatible = "ti,dra7", .data = &dra7_soc_data },
>         { .compatible = "ti,omap3430", .data = &omap34xx_soc_data, },
>         { .compatible = "ti,omap3630", .data = &omap36xx_soc_data, },
>
> 5. configure for CONFIG_ARM_TI_CPUFREQ=y
>
> This should IMHO suffice.

If you're OK with what I am proposing, I'll run some tests and submit
a patch.  I won't promise to fully understand what's happening.  :-)

>
> Since I can't test anything I can't define working OPP points
> and therefore I can't provide patches myself. Hope you can make
> it work this way.
>
> BR,
> Nikolaus

Re: [Letux-kernel] [RFC PATCH 0/3] Enable 1GHz support on omap36xx

[H. Nikolaus Schaller]

Hi Adam,

> Am 09.09.2019 um 16:26 schrieb Adam Ford <aford173@gmail.com>:
> 
> On Sat, Sep 7, 2019 at 2:38 AM H. Nikolaus Schaller <hns@goldelico.com> wrote:
>> 
>> Hi Adam,
>> 
>>> Am 02.09.2019 um 23:10 schrieb Adam Ford <aford173@gmail.com>:
>>> 
>>> On Mon, Sep 2, 2019 at 10:46 AM H. Nikolaus Schaller <hns@goldelico.com> wrote:
>>>> 
>>>> 
>>>> 
>>>> But my tests show that decoding works now. So you already might give it a try.
>>> 
>>> I am traveling all this week, but I have an omap3530, DM3730
>>> (omap3630), and an AM3517 that I use for testing.
>> 
>> now as the omap3430 and omap3630 opp-v2 tables are installed,
>> we could add am35x7 as well.
>> 
>> What needs to be done:
>> 
>> 1. add OPP-v2 table to am3517.dtsi
>> 
>> for example copy skeleton from omap36xx.dtsi
>> 
>> and define reasonable clock speeds. I would think about
>> 150 MHz, 300 MHz, 600MHz.
> 
> This might be more of a question for TI, but  can we match the 3430
> list of frequencies?
> 
> Something like:
>    125000  1200000
>    250000  1200000
>    500000  1200000
>    550000  1200000
>    600000  1200000

And another question: is it more derived from omap3430 or omap3630?

> 
> 
>> 
>> Debatable is if we need a clock-latency definition.
>> 
>> 2. change all voltages to 1.2V
>> 
>>                        opp-microvolt = <1200000 1200000 1200000>;
>> 
>> There is no point to specify 3 voltages <target min max> here since we
>> will never need a min and a max value.
>> 
>>                        opp-microvolt = <1200000>;
>> 
>> should also be ok (AFAIK, parser handles single-value records).
>> 
>> 3. AFAIK there is no speed binned eFuse...
>> 
>> But the ti-cpufreq driver always wants to read some eFuse register.
>> 
>> So please check if you can read 0x4800244C and 0x4830A204
>> like on omap36xx and if they produce stable values (and not
>> random noise).
> 
> For the AM3517,
> 
> 0x4800244C = 0000 0cc0

If it behaves like an dm3730 (Table 1-6) this would be read as 800/600MHz
and some reserved code in bit 7:6.

If it behaves like an omap3530 (Table 1-6) this would bean 600MHz but reserved
value for IVA Frequency.


> 0x4830A204 = 1b86 802f

would be decoded (Table 1-7) as "AM/DM37x ES1.1"

omap35xx would have a different code (Table 1-9). Most similar is "OMAP35x ES2.0" with 0x1B7A E02F

So this seems to answer that the am3517 is indeed a derivative of the am/dm37xx.

Therefore the only OPPs would be OPP50 (300 MHz) and OPP100 (600 MHz).

Only tests or TI internal documentations could show if the am3517 still
runs stable at newly invented "OPP25" (150MHz).

> 
> The AM3517 shows these are valid addresses and I read them multiple
> times and they yielded the same results even after power cycling.
> 
> 
>> 
>> If yes, we simply assume that am3517 is similar enough to omap3630,
>> ignore that there is no eFuse, but read the register anyways and
>> then ignore the bit if it is 0 or 1.
>> 
>> This means that all OPPs can get
>> 
>>                        opp-supported-hw = <0xffffffff 0xffffffff>;
>> 
>> There could also be a default handler if this property is missing,
>> but I have not researched this.
>> 
> Like this?
> 
> opp1-125000000 {
>     opp-hz = /bits/ 64 <125000000>;
>     opp-microvolt = <1200000>;
>     opp-supported-hw = <0xffffffff 0xffffffff>;
> };
> 
> opp2-250000000 {
>     opp-hz = /bits/ 64 <250000000>;
>     opp-microvolt = <1200000>;
>    opp-supported-hw = <0xffffffff 0xffffffff>;
>     opp-suspend;
> };
> 
> opp3-500000000 {
>     opp-hz = /bits/ 64 <500000000>;
>     opp-microvolt = <1200000>;
>     opp-supported-hw = <0xffffffff 0xffffffff>;
> };
> 
> opp4-550000000 {
>     opp-hz = /bits/ 64 <550000000>;
>     opp-microvolt = <1200000>;
>     opp-supported-hw = <0xffffffff 0xffffffff>;
> };
> 
> opp5-600000000 {
>     opp-hz = /bits/ 64 <600000000>;
>     opp-microvolt = <1200000>;
>     opp-supported-hw = <0xffffffff 0xffffffff>;
> };

Yes.

> 
> What does opp-suspend do?  I noticed it in the 34xx.dtsi

Good question. I think it is the OPP to be chosen before suspend:

https://www.kernel.org/doc/Documentation/devicetree/bindings/opp/opp.txt
says

- opp-suspend: Marks the OPP to be used during device suspend. Only one OPP in
  the table should have this.

But that doesn't mean the drivers make use of this marker.

This makes me also wonder if we should tag the OPP1G and OPP6 as "turbo-mode"...

Another question that came up by private mail from André was if we
should better disable the turbo OPPs of omap34xx and 36xx by default
(status = "disabled";) because there are concerns about overheating
the chips and we have no thermal regulation like for omap4 & 5.

But this would mean that every board DTS would have to set it explicitly
to "enabled".

And another concern is if the 1GHz OPP doesn't also need to switch the
ABB bias LDO to a different mode. This is not done by the ti-cpufreq driver.
Maybe it is done by some driver in mach-omap but I have not searched for.

So the concern is that we will run the turbo modes outside of the TI specs
while before applying the patch set this would be a lesser problem (OPP130
should also be thermally limited to 90°C).

I.e. users of 1GHz capable boards will not only see 25% more speed but
suddenly higher SoC temperatures than the years before.

> 
>> 4. add compatible to ti-cpufreq
>> and share the register offsets, bit masks etc. with omap3630:
>> 
>>        { .compatible = "ti,am33xx", .data = &am3x_soc_data, },
>>        { .compatible = "ti,am3517", .data = &omap36xx_soc_data, },
>>        { .compatible = "ti,am43", .data = &am4x_soc_data, },
>>        { .compatible = "ti,dra7", .data = &dra7_soc_data },
>>        { .compatible = "ti,omap3430", .data = &omap34xx_soc_data, },
>>        { .compatible = "ti,omap3630", .data = &omap36xx_soc_data, },
>> 
>> 5. configure for CONFIG_ARM_TI_CPUFREQ=y
>> 
>> This should IMHO suffice.
> 
> If you're OK with what I am proposing, I'll run some tests and submit
> a patch.  I won't promise to fully understand what's happening.  :-)

Same for me :)

BR and thanks,
Nikolaus

Re: [Letux-kernel] [RFC PATCH 0/3] Enable 1GHz support on omap36xx

[Adam Ford]

On Mon, Sep 9, 2019 at 9:57 AM H. Nikolaus Schaller <hns@goldelico.com> wrote:
>
> Hi Adam,
>
> > Am 09.09.2019 um 16:26 schrieb Adam Ford <aford173@gmail.com>:
> >
> > On Sat, Sep 7, 2019 at 2:38 AM H. Nikolaus Schaller <hns@goldelico.com> wrote:
> >>
> >> Hi Adam,
> >>
> >>> Am 02.09.2019 um 23:10 schrieb Adam Ford <aford173@gmail.com>:
> >>>
> >>> On Mon, Sep 2, 2019 at 10:46 AM H. Nikolaus Schaller <hns@goldelico.com> wrote:
> >>>>
> >>>>
> >>>>
> >>>> But my tests show that decoding works now. So you already might give it a try.
> >>>
> >>> I am traveling all this week, but I have an omap3530, DM3730
> >>> (omap3630), and an AM3517 that I use for testing.
> >>
> >> now as the omap3430 and omap3630 opp-v2 tables are installed,
> >> we could add am35x7 as well.
> >>
> >> What needs to be done:
> >>
> >> 1. add OPP-v2 table to am3517.dtsi
> >>
> >> for example copy skeleton from omap36xx.dtsi
> >>
> >> and define reasonable clock speeds. I would think about
> >> 150 MHz, 300 MHz, 600MHz.
> >
> > This might be more of a question for TI, but  can we match the 3430
> > list of frequencies?
> >
> > Something like:
> >    125000  1200000
> >    250000  1200000
> >    500000  1200000
> >    550000  1200000
> >    600000  1200000
>
> And another question: is it more derived from omap3430 or omap3630?
>
> >
> >
> >>
> >> Debatable is if we need a clock-latency definition.
> >>
> >> 2. change all voltages to 1.2V
> >>
> >>                        opp-microvolt = <1200000 1200000 1200000>;
> >>
> >> There is no point to specify 3 voltages <target min max> here since we
> >> will never need a min and a max value.
> >>
> >>                        opp-microvolt = <1200000>;
> >>
> >> should also be ok (AFAIK, parser handles single-value records).
> >>
> >> 3. AFAIK there is no speed binned eFuse...
> >>
> >> But the ti-cpufreq driver always wants to read some eFuse register.
> >>
> >> So please check if you can read 0x4800244C and 0x4830A204
> >> like on omap36xx and if they produce stable values (and not
> >> random noise).
> >
> > For the AM3517,
> >
> > 0x4800244C = 0000 0cc0
>
> If it behaves like an dm3730 (Table 1-6) this would be read as 800/600MHz
> and some reserved code in bit 7:6.
>
> If it behaves like an omap3530 (Table 1-6) this would bean 600MHz but reserved
> value for IVA Frequency.
>
>
> > 0x4830A204 = 1b86 802f
>
> would be decoded (Table 1-7) as "AM/DM37x ES1.1"
>
> omap35xx would have a different code (Table 1-9). Most similar is "OMAP35x ES2.0" with 0x1B7A E02F
>
> So this seems to answer that the am3517 is indeed a derivative of the am/dm37xx.
>
> Therefore the only OPPs would be OPP50 (300 MHz) and OPP100 (600 MHz).
>
> Only tests or TI internal documentations could show if the am3517 still
> runs stable at newly invented "OPP25" (150MHz).
>
> >
> > The AM3517 shows these are valid addresses and I read them multiple
> > times and they yielded the same results even after power cycling.
> >
> >
> >>
> >> If yes, we simply assume that am3517 is similar enough to omap3630,
> >> ignore that there is no eFuse, but read the register anyways and
> >> then ignore the bit if it is 0 or 1.
> >>
> >> This means that all OPPs can get
> >>
> >>                        opp-supported-hw = <0xffffffff 0xffffffff>;
> >>
> >> There could also be a default handler if this property is missing,
> >> but I have not researched this.
> >>
> > Like this?
> >
> > opp1-125000000 {
> >     opp-hz = /bits/ 64 <125000000>;
> >     opp-microvolt = <1200000>;
> >     opp-supported-hw = <0xffffffff 0xffffffff>;
> > };
> >
> > opp2-250000000 {
> >     opp-hz = /bits/ 64 <250000000>;
> >     opp-microvolt = <1200000>;
> >    opp-supported-hw = <0xffffffff 0xffffffff>;
> >     opp-suspend;
> > };
> >
> > opp3-500000000 {
> >     opp-hz = /bits/ 64 <500000000>;
> >     opp-microvolt = <1200000>;
> >     opp-supported-hw = <0xffffffff 0xffffffff>;
> > };
> >
> > opp4-550000000 {
> >     opp-hz = /bits/ 64 <550000000>;
> >     opp-microvolt = <1200000>;
> >     opp-supported-hw = <0xffffffff 0xffffffff>;
> > };
> >
> > opp5-600000000 {
> >     opp-hz = /bits/ 64 <600000000>;
> >     opp-microvolt = <1200000>;
> >     opp-supported-hw = <0xffffffff 0xffffffff>;
> > };
>
> Yes.
>
> >
> > What does opp-suspend do?  I noticed it in the 34xx.dtsi
>
> Good question. I think it is the OPP to be chosen before suspend:
>
> https://www.kernel.org/doc/Documentation/devicetree/bindings/opp/opp.txt
> says
>
> - opp-suspend: Marks the OPP to be used during device suspend. Only one OPP in
>   the table should have this.
>
> But that doesn't mean the drivers make use of this marker.
>
> This makes me also wonder if we should tag the OPP1G and OPP6 as "turbo-mode"...
>
> Another question that came up by private mail from André was if we
> should better disable the turbo OPPs of omap34xx and 36xx by default
> (status = "disabled";) because there are concerns about overheating
> the chips and we have no thermal regulation like for omap4 & 5.
>
> But this would mean that every board DTS would have to set it explicitly
> to "enabled".
>
> And another concern is if the 1GHz OPP doesn't also need to switch the
> ABB bias LDO to a different mode. This is not done by the ti-cpufreq driver.
> Maybe it is done by some driver in mach-omap but I have not searched for.

ti-omap5-opp-supply.txt shows

Required Properties for Device Node:
- vdd-supply: phandle to regulator controlling VDD supply
- vbb-supply: phandle to regulator controlling Body Bias supply
      (Usually Adaptive Body Bias regulator)

Looking at the ti-cpufreq.c driver, it appears as if there is both a
vdd and a vbb listed in the table (line 302).  AFAICT, we should be
able to just set it up pointing VDD to the PMIC and VBB at the ABB
unless I am missing something.

Currently, I have the follwing in my device tree:

/* Reroute power feeding the CPU to come from the external PMIC */
&reg_arm
{
     vin-supply = <&sw1a_reg>;
};

&reg_soc
{
      vin-supply = <&sw1c_reg>;
};

Is this still correct with the new cpufreq driver?   I am wondering if
we need VDD and VBB references under the cpu entry.  The
ti-omap5-opp-supply ready me also reads:

/* Device Node (CPU)  */
cpus {
     cpu0: cpu@0 {
          device_type = "cpu";

...

          vdd-supply = <&vcc>;
          vbb-supply = <&abb_mpu>;
     };
};

This isn't in the ti-cpufreq.txt, however it might actually be
utilized based on a quick skim of the driver.

adam
>
> So the concern is that we will run the turbo modes outside of the TI specs
> while before applying the patch set this would be a lesser problem (OPP130
> should also be thermally limited to 90°C).
>
> I.e. users of 1GHz capable boards will not only see 25% more speed but
> suddenly higher SoC temperatures than the years before.
>
> >
> >> 4. add compatible to ti-cpufreq
> >> and share the register offsets, bit masks etc. with omap3630:
> >>
> >>        { .compatible = "ti,am33xx", .data = &am3x_soc_data, },
> >>        { .compatible = "ti,am3517", .data = &omap36xx_soc_data, },
> >>        { .compatible = "ti,am43", .data = &am4x_soc_data, },
> >>        { .compatible = "ti,dra7", .data = &dra7_soc_data },
> >>        { .compatible = "ti,omap3430", .data = &omap34xx_soc_data, },
> >>        { .compatible = "ti,omap3630", .data = &omap36xx_soc_data, },
> >>
> >> 5. configure for CONFIG_ARM_TI_CPUFREQ=y
> >>
> >> This should IMHO suffice.
> >
> > If you're OK with what I am proposing, I'll run some tests and submit
> > a patch.  I won't promise to fully understand what's happening.  :-)
>
> Same for me :)
>
> BR and thanks,
> Nikolaus
>

Re: [Letux-kernel] [RFC PATCH 0/3] Enable 1GHz support on omap36xx

[Tony Lindgren]

Hi,

* H. Nikolaus Schaller <hns@goldelico.com> [190909 14:57]:
> Another question that came up by private mail from André was if we
> should better disable the turbo OPPs of omap34xx and 36xx by default
> (status = "disabled";) because there are concerns about overheating
> the chips and we have no thermal regulation like for omap4 & 5.
> 
> But this would mean that every board DTS would have to set it explicitly
> to "enabled".

Yes I started thinking about that too. I think there is a requirement
to do the scaling via the voltage processor for the higher modes.
And there needs to be some way to automatically change to a lower
OPP in some cases.

For normal OPPs, using the twl regulator directly should be OK.

For the higher modes, maybe we could pass the callback functions
from arch/arm/mach-omap2/voltage.c for the twl regulator so the
voltage processor hardware can handle them directly. Or add a
separate regulator driver operating the voltages like Nishanth
posted patches for earlier.

Regards,

Tony

Re: [Letux-kernel] [RFC PATCH 0/3] Enable 1GHz support on omap36xx

[Adam Ford]

On Mon, Sep 9, 2019 at 11:20 AM Adam Ford <aford173@gmail.com> wrote:
>
> On Mon, Sep 9, 2019 at 9:57 AM H. Nikolaus Schaller <hns@goldelico.com> wrote:
> >
> > Hi Adam,
> >
> > > Am 09.09.2019 um 16:26 schrieb Adam Ford <aford173@gmail.com>:
> > >
> > > On Sat, Sep 7, 2019 at 2:38 AM H. Nikolaus Schaller <hns@goldelico.com> wrote:
> > >>
> > >> Hi Adam,
> > >>
> > >>> Am 02.09.2019 um 23:10 schrieb Adam Ford <aford173@gmail.com>:
> > >>>
> > >>> On Mon, Sep 2, 2019 at 10:46 AM H. Nikolaus Schaller <hns@goldelico.com> wrote:
> > >>>>
> > >>>>
> > >>>>
> > >>>> But my tests show that decoding works now. So you already might give it a try.
> > >>>
> > >>> I am traveling all this week, but I have an omap3530, DM3730
> > >>> (omap3630), and an AM3517 that I use for testing.
> > >>
> > >> now as the omap3430 and omap3630 opp-v2 tables are installed,
> > >> we could add am35x7 as well.
> > >>
> > >> What needs to be done:
> > >>
> > >> 1. add OPP-v2 table to am3517.dtsi
> > >>
> > >> for example copy skeleton from omap36xx.dtsi
> > >>
> > >> and define reasonable clock speeds. I would think about
> > >> 150 MHz, 300 MHz, 600MHz.
> > >
> > > This might be more of a question for TI, but  can we match the 3430
> > > list of frequencies?
> > >
> > > Something like:
> > >    125000  1200000
> > >    250000  1200000
> > >    500000  1200000
> > >    550000  1200000
> > >    600000  1200000
> >
> > And another question: is it more derived from omap3430 or omap3630?
> >
> > >
> > >
> > >>
> > >> Debatable is if we need a clock-latency definition.
> > >>
> > >> 2. change all voltages to 1.2V
> > >>
> > >>                        opp-microvolt = <1200000 1200000 1200000>;
> > >>
> > >> There is no point to specify 3 voltages <target min max> here since we
> > >> will never need a min and a max value.
> > >>
> > >>                        opp-microvolt = <1200000>;
> > >>
> > >> should also be ok (AFAIK, parser handles single-value records).
> > >>
> > >> 3. AFAIK there is no speed binned eFuse...
> > >>
> > >> But the ti-cpufreq driver always wants to read some eFuse register.
> > >>
> > >> So please check if you can read 0x4800244C and 0x4830A204
> > >> like on omap36xx and if they produce stable values (and not
> > >> random noise).
> > >
> > > For the AM3517,
> > >
> > > 0x4800244C = 0000 0cc0
> >
> > If it behaves like an dm3730 (Table 1-6) this would be read as 800/600MHz
> > and some reserved code in bit 7:6.
> >
> > If it behaves like an omap3530 (Table 1-6) this would bean 600MHz but reserved
> > value for IVA Frequency.
> >
> >
> > > 0x4830A204 = 1b86 802f
> >
> > would be decoded (Table 1-7) as "AM/DM37x ES1.1"
> >
> > omap35xx would have a different code (Table 1-9). Most similar is "OMAP35x ES2.0" with 0x1B7A E02F
> >
> > So this seems to answer that the am3517 is indeed a derivative of the am/dm37xx.
> >
> > Therefore the only OPPs would be OPP50 (300 MHz) and OPP100 (600 MHz).
> >
> > Only tests or TI internal documentations could show if the am3517 still
> > runs stable at newly invented "OPP25" (150MHz).
> >
> > >
> > > The AM3517 shows these are valid addresses and I read them multiple
> > > times and they yielded the same results even after power cycling.
> > >
> > >
> > >>
> > >> If yes, we simply assume that am3517 is similar enough to omap3630,
> > >> ignore that there is no eFuse, but read the register anyways and
> > >> then ignore the bit if it is 0 or 1.
> > >>
> > >> This means that all OPPs can get
> > >>
> > >>                        opp-supported-hw = <0xffffffff 0xffffffff>;
> > >>
> > >> There could also be a default handler if this property is missing,
> > >> but I have not researched this.
> > >>
> > > Like this?
> > >
> > > opp1-125000000 {
> > >     opp-hz = /bits/ 64 <125000000>;
> > >     opp-microvolt = <1200000>;
> > >     opp-supported-hw = <0xffffffff 0xffffffff>;
> > > };
> > >
> > > opp2-250000000 {
> > >     opp-hz = /bits/ 64 <250000000>;
> > >     opp-microvolt = <1200000>;
> > >    opp-supported-hw = <0xffffffff 0xffffffff>;
> > >     opp-suspend;
> > > };
> > >
> > > opp3-500000000 {
> > >     opp-hz = /bits/ 64 <500000000>;
> > >     opp-microvolt = <1200000>;
> > >     opp-supported-hw = <0xffffffff 0xffffffff>;
> > > };
> > >
> > > opp4-550000000 {
> > >     opp-hz = /bits/ 64 <550000000>;
> > >     opp-microvolt = <1200000>;
> > >     opp-supported-hw = <0xffffffff 0xffffffff>;
> > > };
> > >
> > > opp5-600000000 {
> > >     opp-hz = /bits/ 64 <600000000>;
> > >     opp-microvolt = <1200000>;
> > >     opp-supported-hw = <0xffffffff 0xffffffff>;
> > > };
> >
> > Yes.
> >
> > >
> > > What does opp-suspend do?  I noticed it in the 34xx.dtsi
> >
> > Good question. I think it is the OPP to be chosen before suspend:
> >
> > https://www.kernel.org/doc/Documentation/devicetree/bindings/opp/opp.txt
> > says
> >
> > - opp-suspend: Marks the OPP to be used during device suspend. Only one OPP in
> >   the table should have this.
> >
> > But that doesn't mean the drivers make use of this marker.
> >
> > This makes me also wonder if we should tag the OPP1G and OPP6 as "turbo-mode"...
> >
> > Another question that came up by private mail from André was if we
> > should better disable the turbo OPPs of omap34xx and 36xx by default
> > (status = "disabled";) because there are concerns about overheating
> > the chips and we have no thermal regulation like for omap4 & 5.
> >
> > But this would mean that every board DTS would have to set it explicitly
> > to "enabled".
> >
> > And another concern is if the 1GHz OPP doesn't also need to switch the
> > ABB bias LDO to a different mode. This is not done by the ti-cpufreq driver.
> > Maybe it is done by some driver in mach-omap but I have not searched for.
>
> ti-omap5-opp-supply.txt shows
>
> Required Properties for Device Node:
> - vdd-supply: phandle to regulator controlling VDD supply
> - vbb-supply: phandle to regulator controlling Body Bias supply
>       (Usually Adaptive Body Bias regulator)
>
> Looking at the ti-cpufreq.c driver, it appears as if there is both a
> vdd and a vbb listed in the table (line 302).  AFAICT, we should be
> able to just set it up pointing VDD to the PMIC and VBB at the ABB
> unless I am missing something.
>
> Currently, I have the follwing in my device tree:
>
> /* Reroute power feeding the CPU to come from the external PMIC */
> &reg_arm
> {
>      vin-supply = <&sw1a_reg>;
> };
>
> &reg_soc
> {
>       vin-supply = <&sw1c_reg>;
> };
>
Ignore that.  (wrong board)
Correction,

cpus {
     cpu@0 {
          cpu0-supply = <&vcc>;
     };
};

I had too many boards.
> Is this still correct with the new cpufreq driver?   I am wondering if
> we need VDD and VBB references under the cpu entry.  The
> ti-omap5-opp-supply ready me also reads:
>
> /* Device Node (CPU)  */
> cpus {
>      cpu0: cpu@0 {
>           device_type = "cpu";
>
> ...
>
>           vdd-supply = <&vcc>;
>           vbb-supply = <&abb_mpu>;
>      };
> };
>
> This isn't in the ti-cpufreq.txt, however it might actually be
> utilized based on a quick skim of the driver.
>
> adam
> >
> > So the concern is that we will run the turbo modes outside of the TI specs
> > while before applying the patch set this would be a lesser problem (OPP130
> > should also be thermally limited to 90°C).
> >
> > I.e. users of 1GHz capable boards will not only see 25% more speed but
> > suddenly higher SoC temperatures than the years before.
> >
> > >
> > >> 4. add compatible to ti-cpufreq
> > >> and share the register offsets, bit masks etc. with omap3630:
> > >>
> > >>        { .compatible = "ti,am33xx", .data = &am3x_soc_data, },
> > >>        { .compatible = "ti,am3517", .data = &omap36xx_soc_data, },
> > >>        { .compatible = "ti,am43", .data = &am4x_soc_data, },
> > >>        { .compatible = "ti,dra7", .data = &dra7_soc_data },
> > >>        { .compatible = "ti,omap3430", .data = &omap34xx_soc_data, },
> > >>        { .compatible = "ti,omap3630", .data = &omap36xx_soc_data, },
> > >>
> > >> 5. configure for CONFIG_ARM_TI_CPUFREQ=y
> > >>
> > >> This should IMHO suffice.
> > >
> > > If you're OK with what I am proposing, I'll run some tests and submit
> > > a patch.  I won't promise to fully understand what's happening.  :-)
> >
> > Same for me :)
> >
> > BR and thanks,
> > Nikolaus
> >

Re: [Letux-kernel] [RFC PATCH 0/3] Enable 1GHz support on omap36xx

[Adam Ford]

On Mon, Sep 9, 2019 at 11:32 AM Tony Lindgren <tony@atomide.com> wrote:
>
> Hi,
>
> * H. Nikolaus Schaller <hns@goldelico.com> [190909 14:57]:
> > Another question that came up by private mail from André was if we
> > should better disable the turbo OPPs of omap34xx and 36xx by default
> > (status = "disabled";) because there are concerns about overheating
> > the chips and we have no thermal regulation like for omap4 & 5.

I thought there was a thermal sensor?

cpu_thermal: cpu_thermal {
        polling-delay-passive = <250>; /* milliseconds */
        polling-delay = <1000>; /* milliseconds */
        coefficients = <0 20000>;

                        /* sensor       ID */
        thermal-sensors = <&bandgap     0>;
};

Can this driver somehow notify the cpufreq that we've hit some limit?
I know it's not as accurate as one would like, but even for non-1GHz
versions, having it downclock would be a good thing when running at
extreme temps.

adam
> >
> > But this would mean that every board DTS would have to set it explicitly
> > to "enabled".
>
> Yes I started thinking about that too. I think there is a requirement
> to do the scaling via the voltage processor for the higher modes.
> And there needs to be some way to automatically change to a lower
> OPP in some cases.
>
> For normal OPPs, using the twl regulator directly should be OK.
>
> For the higher modes, maybe we could pass the callback functions
> from arch/arm/mach-omap2/voltage.c for the twl regulator so the
> voltage processor hardware can handle them directly. Or add a
> separate regulator driver operating the voltages like Nishanth
> posted patches for earlier.
>
> Regards,
>
> Tony

Re: [Letux-kernel] [RFC PATCH 0/3] Enable 1GHz support on omap36xx

[H. Nikolaus Schaller]

Hi Tony,

> Am 09.09.2019 um 18:32 schrieb Tony Lindgren <tony@atomide.com>:
> 
> Hi,
> 
> * H. Nikolaus Schaller <hns@goldelico.com> [190909 14:57]:
>> Another question that came up by private mail from André was if we
>> should better disable the turbo OPPs of omap34xx and 36xx by default
>> (status = "disabled";) because there are concerns about overheating
>> the chips and we have no thermal regulation like for omap4 & 5.
>> 
>> But this would mean that every board DTS would have to set it explicitly
>> to "enabled".
> 
> Yes I started thinking about that too. I think there is a requirement
> to do the scaling via the voltage processor for the higher modes.

It depends on how you read the little footnotes...

Table 4-18. Processor Voltages Without SmartReflex:

	• This table defines the safe VDD1 (vdd_mpu_iva) voltage ranges to be used before using the SmartReflex AVS feature for OPPs calibration.
	• Values are defined when SmartReflexTM feature is deactivated. They can be lower when SmartReflexTM is activated.
	• OPP130 and OPP1G are not available above TJ of 90C.
	• (6)  OPP1G is a high performance operating point which has following requirements:
		• –  ABB LDO must be set to FBB (Forward Body Bias) mode when switching to this OPP. It requires having a 1 F capacitor connected to cap_vdd_bb_mpu_iva.
		• –  AVS (Adaptive Voltage Scaling) power technique must be used to achieve optimum operating voltage.

So I read this as:

* OPP130 and OPP1G should be guarded by 90°C thermal framework
* OPP1G should also set the ABB LDO to FBB mode
* AVS does only reduce voltage levels (to save energy = heat = problem)
* only if we want "optimum operating voltage" (read as: "lowest possible voltage" = "highest energy saving") we must use AVS

I.e. we do not necessarily need AVS or SmartReflex or help from the
twl4030 (except for changing the voltage).

> And there needs to be some way to automatically change to a lower
> OPP in some cases.

That should probably be done through the thermal framework like
on omap4 & omap5?

> 
> For normal OPPs, using the twl regulator directly should be OK.

Maybe for the turbo OPPs as well.

> For the higher modes, maybe we could pass the callback functions
> from arch/arm/mach-omap2/voltage.c for the twl regulator so the
> voltage processor hardware can handle them directly. Or add a
> separate regulator driver operating the voltages like Nishanth
> posted patches for earlier.

So in my (limited) understanding it would suffice to set the ABB LDO
to FBB mode for OPP1G.

And make sure that the TJ does not exceed 90°C by reducing the cpufreq
through the thermal framework. But: the thermal sensors of the omap3
are quite odd (they seem to jump up by 10° after first use).

BR,
Nikolaus

Re: [Letux-kernel] [RFC PATCH 0/3] Enable 1GHz support on omap36xx

[H. Nikolaus Schaller]

> Am 09.09.2019 um 18:38 schrieb Adam Ford <aford173@gmail.com>:
> 
> On Mon, Sep 9, 2019 at 11:32 AM Tony Lindgren <tony@atomide.com> wrote:
>> 
>> Hi,
>> 
>> * H. Nikolaus Schaller <hns@goldelico.com> [190909 14:57]:
>>> Another question that came up by private mail from André was if we
>>> should better disable the turbo OPPs of omap34xx and 36xx by default
>>> (status = "disabled";) because there are concerns about overheating
>>> the chips and we have no thermal regulation like for omap4 & 5.
> 
> I thought there was a thermal sensor?

Yes.

> 
> cpu_thermal: cpu_thermal {
>        polling-delay-passive = <250>; /* milliseconds */
>        polling-delay = <1000>; /* milliseconds */
>        coefficients = <0 20000>;
> 
>                        /* sensor       ID */
>        thermal-sensors = <&bandgap     0>;
> };
> 
> Can this driver somehow notify the cpufreq that we've hit some limit?
> I know it's not as accurate as one would like, but even for non-1GHz
> versions, having it downclock would be a good thing when running at
> extreme temps.

Indeed it is not really reliable. For me it jumps up by 10° between first
reading within the next second (and seems to stay at this offset after first use).

But yes, I think it should be possible to use it similar to omap5-core-thermal.dtsi

Maybe we have to add "trips" and "core_crit". This must obviously be linked to
the cpufreq system. Or is it done automatically?

BR,
Nikolaus

Re: [Letux-kernel] [RFC PATCH 0/3] Enable 1GHz support on omap36xx

[H. Nikolaus Schaller]

> Am 09.09.2019 um 18:54 schrieb H. Nikolaus Schaller <hns@goldelico.com>:
> 
> Hi Tony,
> 
>> Am 09.09.2019 um 18:32 schrieb Tony Lindgren <tony@atomide.com>:
>> 
>> Hi,
>> 
>> * H. Nikolaus Schaller <hns@goldelico.com> [190909 14:57]:
>>> Another question that came up by private mail from André was if we
>>> should better disable the turbo OPPs of omap34xx and 36xx by default
>>> (status = "disabled";) because there are concerns about overheating
>>> the chips and we have no thermal regulation like for omap4 & 5.
>>> 
>>> But this would mean that every board DTS would have to set it explicitly
>>> to "enabled".
>> 
>> Yes I started thinking about that too. I think there is a requirement
>> to do the scaling via the voltage processor for the higher modes.
> 
> It depends on how you read the little footnotes...
> 
> Table 4-18. Processor Voltages Without SmartReflex:
> 
> 	• This table defines the safe VDD1 (vdd_mpu_iva) voltage ranges to be used before using the SmartReflex AVS feature for OPPs calibration.
> 	• Values are defined when SmartReflexTM feature is deactivated. They can be lower when SmartReflexTM is activated.
> 	• OPP130 and OPP1G are not available above TJ of 90C.
> 	• (6)  OPP1G is a high performance operating point which has following requirements:
> 		• –  ABB LDO must be set to FBB (Forward Body Bias) mode when switching to this OPP. It requires having a 1 F capacitor connected to cap_vdd_bb_mpu_iva.
> 		• –  AVS (Adaptive Voltage Scaling) power technique must be used to achieve optimum operating voltage.
> 
> So I read this as:
> 
> * OPP130 and OPP1G should be guarded by 90°C thermal framework
> * OPP1G should also set the ABB LDO to FBB mode
> * AVS does only reduce voltage levels (to save energy = heat = problem)
> * only if we want "optimum operating voltage" (read as: "lowest possible voltage" = "highest energy saving") we must use AVS
> 
> I.e. we do not necessarily need AVS or SmartReflex or help from the
> twl4030 (except for changing the voltage).
> 
>> And there needs to be some way to automatically change to a lower
>> OPP in some cases.
> 
> That should probably be done through the thermal framework like
> on omap4 & omap5?
> 
>> 
>> For normal OPPs, using the twl regulator directly should be OK.
> 
> Maybe for the turbo OPPs as well.
> 
>> For the higher modes, maybe we could pass the callback functions
>> from arch/arm/mach-omap2/voltage.c for the twl regulator so the
>> voltage processor hardware can handle them directly. Or add a
>> separate regulator driver operating the voltages like Nishanth
>> posted patches for earlier.
> 
> So in my (limited) understanding it would suffice to set the ABB LDO
> to FBB mode for OPP1G.

Ok, we have to check if the ti,abb-v2 "LDO" driver 
drivers/regulator/ti-abb-regulator.c
can handle that with a DT entry similar to:

https://elixir.bootlin.com/linux/latest/source/arch/arm/boot/dts/omap5.dtsi#L365

Needs a little time to add to a new version of the patch set.

> And make sure that the TJ does not exceed 90°C by reducing the cpufreq
> through the thermal framework. But: the thermal sensors of the omap3
> are quite odd (they seem to jump up by 10° after first use).

I'll leave this out for the moment for future study.

BR and thanks,
Nikolaus

Re: [Letux-kernel] [RFC PATCH 0/3] Enable 1GHz support on omap36xx

[Adam Ford]

On Mon, Sep 9, 2019 at 1:11 PM H. Nikolaus Schaller <hns@goldelico.com> wrote:
>
>
> > Am 09.09.2019 um 18:54 schrieb H. Nikolaus Schaller <hns@goldelico.com>:
> >
> > Hi Tony,
> >
> >> Am 09.09.2019 um 18:32 schrieb Tony Lindgren <tony@atomide.com>:
> >>
> >> Hi,
> >>
> >> * H. Nikolaus Schaller <hns@goldelico.com> [190909 14:57]:
> >>> Another question that came up by private mail from André was if we
> >>> should better disable the turbo OPPs of omap34xx and 36xx by default
> >>> (status = "disabled";) because there are concerns about overheating
> >>> the chips and we have no thermal regulation like for omap4 & 5.
> >>>
> >>> But this would mean that every board DTS would have to set it explicitly
> >>> to "enabled".
> >>
> >> Yes I started thinking about that too. I think there is a requirement
> >> to do the scaling via the voltage processor for the higher modes.
> >
> > It depends on how you read the little footnotes...
> >
> > Table 4-18. Processor Voltages Without SmartReflex:
> >
> >       • This table defines the safe VDD1 (vdd_mpu_iva) voltage ranges to be used before using the SmartReflex AVS feature for OPPs calibration.
> >       • Values are defined when SmartReflexTM feature is deactivated. They can be lower when SmartReflexTM is activated.
> >       • OPP130 and OPP1G are not available above TJ of 90C.
> >       • (6)  OPP1G is a high performance operating point which has following requirements:
> >               • –  ABB LDO must be set to FBB (Forward Body Bias) mode when switching to this OPP. It requires having a 1 F capacitor connected to cap_vdd_bb_mpu_iva.
> >               • –  AVS (Adaptive Voltage Scaling) power technique must be used to achieve optimum operating voltage.
> >
> > So I read this as:
> >
> > * OPP130 and OPP1G should be guarded by 90°C thermal framework
> > * OPP1G should also set the ABB LDO to FBB mode
> > * AVS does only reduce voltage levels (to save energy = heat = problem)
> > * only if we want "optimum operating voltage" (read as: "lowest possible voltage" = "highest energy saving") we must use AVS
> >
> > I.e. we do not necessarily need AVS or SmartReflex or help from the
> > twl4030 (except for changing the voltage).
> >
> >> And there needs to be some way to automatically change to a lower
> >> OPP in some cases.
> >
> > That should probably be done through the thermal framework like
> > on omap4 & omap5?
> >
> >>
> >> For normal OPPs, using the twl regulator directly should be OK.
> >
> > Maybe for the turbo OPPs as well.
> >
> >> For the higher modes, maybe we could pass the callback functions
> >> from arch/arm/mach-omap2/voltage.c for the twl regulator so the
> >> voltage processor hardware can handle them directly. Or add a
> >> separate regulator driver operating the voltages like Nishanth
> >> posted patches for earlier.
> >
> > So in my (limited) understanding it would suffice to set the ABB LDO
> > to FBB mode for OPP1G.
>
> Ok, we have to check if the ti,abb-v2 "LDO" driver
> drivers/regulator/ti-abb-regulator.c
> can handle that with a DT entry similar to:
>
> https://elixir.bootlin.com/linux/latest/source/arch/arm/boot/dts/omap5.dtsi#L365

At least for the 3630, the ti-abb-regulator driver is the same driver,
but different structures based on v1, v2, or v3 are used based on
which compatible flag is used.

I tried enabling the vbb-supply in the device tree, but the driver
doesn't load it without .multi_regulator being true.

cpus {
/* OMAP3630/OMAP37xx variants OPP50 to OPP130 and OPP1G */
     cpu: cpu@0 {
          operating-points-v2 = <&cpu0_opp_table>;
          vbb-supply = <&abb_mpu_iva>;
          clock-latency = <300000>; /* From omap-cpufreq driver */
     };
};

I enabled that in the 3630 structure, but then the opp must list
voltage points for both regulators.
Looking at the entry for abb_mpu_iva, it appears to have voltages that
directly match the OPP table, so I made a duplicate entry:

 opp-microvolt = <1012500 1012500 1012500>,
                           <1012500 1012500 1012500>;

and similar for 600, 800 and 1000 similar to the way dra7.dtsi does
it, but then I got some nasty errors and crashes.

I started undoing the stuff, and I wanted to see if the abb_mpu_iva
regulator was even running, but I would get -22 errors when I went to
read the voltage.

# cat /sys/devices/platform/68000000.ocp/483072f0.regulator-abb-mpu/regulator/regulator.5/microvolts
-22

If someone has any suggestions on how to test the abb_mpu_iva driver,
let me know.

>
> Needs a little time to add to a new version of the patch set.
>
> > And make sure that the TJ does not exceed 90°C by reducing the cpufreq
> > through the thermal framework. But: the thermal sensors of the omap3
> > are quite odd (they seem to jump up by 10° after first use).

Can we just do a dummy read to get it out of the way?  ;-)

>
> I'll leave this out for the moment for future study.
Works for me, Baby steps.

Thanks for all your work.

adam
>
> BR and thanks,
> Nikolaus
>

Re: [Letux-kernel] [RFC PATCH 0/3] Enable 1GHz support on omap36xx

[H. Nikolaus Schaller]

Hi Adam,

> Am 09.09.2019 um 21:13 schrieb Adam Ford <aford173@gmail.com>:
> 
> On Mon, Sep 9, 2019 at 1:11 PM H. Nikolaus Schaller <hns@goldelico.com> wrote:
>> 
>> Ok, we have to check if the ti,abb-v2 "LDO" driver
>> drivers/regulator/ti-abb-regulator.c
>> can handle that with a DT entry similar to:
>> 
>> https://elixir.bootlin.com/linux/latest/source/arch/arm/boot/dts/omap5.dtsi#L365
> 
> At least for the 3630, the ti-abb-regulator driver is the same driver,
> but different structures based on v1, v2, or v3 are used based on
> which compatible flag is used.
> 
> I tried enabling the vbb-supply in the device tree, but the driver
> doesn't load it without .multi_regulator being true.
> 
> cpus {
> /* OMAP3630/OMAP37xx variants OPP50 to OPP130 and OPP1G */
>     cpu: cpu@0 {
>          operating-points-v2 = <&cpu0_opp_table>;
>          vbb-supply = <&abb_mpu_iva>;

Oh, that is great that the app_mpu_iva already exists!

So we just need plumbing pieces together.

>          clock-latency = <300000>; /* From omap-cpufreq driver */
>     };
> };
> 
> I enabled that in the 3630 structure, but then the opp must list
> voltage points for both regulators.
> Looking at the entry for abb_mpu_iva, it appears to have voltages that
> directly match the OPP table, so I made a duplicate entry:
> 
> opp-microvolt = <1012500 1012500 1012500>,
>                           <1012500 1012500 1012500>;
> 
> and similar for 600, 800 and 1000 similar to the way dra7.dtsi does

Yes.

> it, but then I got some nasty errors and crashes.

I have done the same but not (yet) seen a crash or error. Maybe you had
a typo?

> 
> I started undoing the stuff, and I wanted to see if the abb_mpu_iva
> regulator was even running, but I would get -22 errors when I went to
> read the voltage.
> 
> # cat /sys/devices/platform/68000000.ocp/483072f0.regulator-abb-mpu/regulator/regulator.5/microvolts
> -22

So it reports wrong voltage settings of -22µV...

I have tested and have the same.

root@letux:~# cd /sys/bus/platform/devices/483072f0.regulator-abb-mpu/regulator/regulator.3/
root@letux:/sys/bus/platform/devices/483072f0.regulator-abb-mpu/regulator/regulator.3# ls -l
total 0
lrwxrwxrwx 1 root root    0 Jan  1 00:02 device -> ../../../483072f0.regulator-abb-mpu
-r--r--r-- 1 root root 4096 Jan  1 00:02 max_microvolts
-r--r--r-- 1 root root 4096 Jan  1 00:02 microvolts
-r--r--r-- 1 root root 4096 Jan  1 00:02 min_microvolts
-r--r--r-- 1 root root 4096 Jan  1 00:02 name
-r--r--r-- 1 root root 4096 Jan  1 00:02 num_users
lrwxrwxrwx 1 root root    0 Jan  1 00:02 of_node -> ../../../../../../firmware/devicetree/base/ocp@68000000/regulator-abb-mpu
drwxr-xr-x 2 root root    0 Jan  1 00:02 power
-r--r--r-- 1 root root 4096 Jan  1 00:02 requested_microamps
lrwxrwxrwx 1 root root    0 Jan  1 00:02 subsystem -> ../../../../../../class/regulator
-r--r--r-- 1 root root 4096 Jan  1 00:02 suspend_disk_state
-r--r--r-- 1 root root 4096 Jan  1 00:02 suspend_mem_state
-r--r--r-- 1 root root 4096 Jan  1 00:02 suspend_standby_state
-r--r--r-- 1 root root 4096 Jan  1 00:02 type
-rw-r--r-- 1 root root 4096 Jan  1 00:02 uevent
root@letux:/sys/bus/platform/devices/483072f0.regulator-abb-mpu/regulator/regulator.3# cat *
cat: device: Is a directory
1375000
-22
1012500
abb_mpu_iva
1
cat: of_node: Is a directory
cat: power: Is a directory
0
cat: subsystem: Is a directory
disabled
disabled
disabled
voltage
OF_NAME=regulator-abb-mpu
OF_FULLNAME=/ocp@68000000/regulator-abb-mpu
OF_COMPATIBLE_0=ti,abb-v1
OF_COMPATIBLE_N=1
root@letux:/sys/bus/platform/devices/483072f0.regulator-abb-mpu/regulator/regulator.3# cd
root@letux:~# cpufreq-info 
cpufrequtils 008: cpufreq-info (C) Dominik Brodowski 2004-2009
Report errors and bugs to cpufreq@vger.kernel.org, please.
analyzing CPU 0:
  driver: cpufreq-dt
  CPUs which run at the same hardware frequency: 0
  CPUs which need to have their frequency coordinated by software: 0
  maximum transition latency: 300 us.
  hardware limits: 300 MHz - 1000 MHz
  available frequency steps: 300 MHz, 600 MHz, 800 MHz, 1000 MHz
  available cpufreq governors: conservative, userspace, powersave, ondemand, performance
  current policy: frequency should be within 300 MHz and 1000 MHz.
                  The governor "ondemand" may decide which speed to use
                  within this range.
  current CPU frequency is 300 MHz (asserted by call to hardware).
  cpufreq stats: 300 MHz:31.36%, 600 MHz:4.23%, 800 MHz:3.76%, 1000 MHz:60.65%  (1933)
root@letux:~# 

So it runs with different OPPs... My chip may also be more robust to wrong ABB FBB setting.

> 
> If someone has any suggestions on how to test the abb_mpu_iva driver,
> let me know.

Well, next I want to look into the code for cat microvolts and
maybe add some printk to understand the result...

A first result is that it comes from

	/* We do not know where the OPP voltage is at the moment */
	abb->current_info_idx = -EINVAL;

But this is not treated as an -EINVAL but value of -22 microvolts...
Maybe an error check is missing somewhere in the regulator core.

I just need more time to track it down.

BR and thanks for the description,
Nikolaus

Re: [Letux-kernel] [RFC PATCH 0/3] Enable 1GHz support on omap36xx

[Adam Ford]

On Tue, Sep 10, 2019 at 11:59 AM H. Nikolaus Schaller <hns@goldelico.com> wrote:
>
> Hi Adam,
>
> > Am 09.09.2019 um 21:13 schrieb Adam Ford <aford173@gmail.com>:
> >
> > On Mon, Sep 9, 2019 at 1:11 PM H. Nikolaus Schaller <hns@goldelico.com> wrote:
> >>
> >> Ok, we have to check if the ti,abb-v2 "LDO" driver
> >> drivers/regulator/ti-abb-regulator.c
> >> can handle that with a DT entry similar to:
> >>
> >> https://elixir.bootlin.com/linux/latest/source/arch/arm/boot/dts/omap5.dtsi#L365
> >
> > At least for the 3630, the ti-abb-regulator driver is the same driver,
> > but different structures based on v1, v2, or v3 are used based on
> > which compatible flag is used.
> >
> > I tried enabling the vbb-supply in the device tree, but the driver
> > doesn't load it without .multi_regulator being true.
> >
> > cpus {
> > /* OMAP3630/OMAP37xx variants OPP50 to OPP130 and OPP1G */
> >     cpu: cpu@0 {
> >          operating-points-v2 = <&cpu0_opp_table>;
> >          vbb-supply = <&abb_mpu_iva>;
>
> Oh, that is great that the app_mpu_iva already exists!
>
> So we just need plumbing pieces together.
>
> >          clock-latency = <300000>; /* From omap-cpufreq driver */
> >     };
> > };
> >
> > I enabled that in the 3630 structure, but then the opp must list
> > voltage points for both regulators.
> > Looking at the entry for abb_mpu_iva, it appears to have voltages that
> > directly match the OPP table, so I made a duplicate entry:
> >
> > opp-microvolt = <1012500 1012500 1012500>,
> >                           <1012500 1012500 1012500>;

Out of curiosity, if we're only going to use one value for the opp
voltage, do we need to have 3 listed?  when I looked at the driver
yesterday, it appears to support either 1 or 3 entries per opp.
If we're going to support two regulators, showing them as
opp-microvolt = <1012500>, <1012500>; would be cleaner and can fit on
one line.

> >
> > and similar for 600, 800 and 1000 similar to the way dra7.dtsi does
>
> Yes.
>
> > it, but then I got some nasty errors and crashes.
>
> I have done the same but not (yet) seen a crash or error. Maybe you had
> a typo?

Can you send me an updated patch?  I'd like to try to get where you
are that doesn't crash.

>
> >
> > I started undoing the stuff, and I wanted to see if the abb_mpu_iva
> > regulator was even running, but I would get -22 errors when I went to
> > read the voltage.
> >
> > # cat /sys/devices/platform/68000000.ocp/483072f0.regulator-abb-mpu/regulator/regulator.5/microvolts
> > -22
>
> So it reports wrong voltage settings of -22µV...
>
> I have tested and have the same.
>
> root@letux:~# cd /sys/bus/platform/devices/483072f0.regulator-abb-mpu/regulator/regulator.3/
> root@letux:/sys/bus/platform/devices/483072f0.regulator-abb-mpu/regulator/regulator.3# ls -l
> total 0
> lrwxrwxrwx 1 root root    0 Jan  1 00:02 device -> ../../../483072f0.regulator-abb-mpu
> -r--r--r-- 1 root root 4096 Jan  1 00:02 max_microvolts
> -r--r--r-- 1 root root 4096 Jan  1 00:02 microvolts
> -r--r--r-- 1 root root 4096 Jan  1 00:02 min_microvolts
> -r--r--r-- 1 root root 4096 Jan  1 00:02 name
> -r--r--r-- 1 root root 4096 Jan  1 00:02 num_users
> lrwxrwxrwx 1 root root    0 Jan  1 00:02 of_node -> ../../../../../../firmware/devicetree/base/ocp@68000000/regulator-abb-mpu
> drwxr-xr-x 2 root root    0 Jan  1 00:02 power
> -r--r--r-- 1 root root 4096 Jan  1 00:02 requested_microamps
> lrwxrwxrwx 1 root root    0 Jan  1 00:02 subsystem -> ../../../../../../class/regulator
> -r--r--r-- 1 root root 4096 Jan  1 00:02 suspend_disk_state
> -r--r--r-- 1 root root 4096 Jan  1 00:02 suspend_mem_state
> -r--r--r-- 1 root root 4096 Jan  1 00:02 suspend_standby_state
> -r--r--r-- 1 root root 4096 Jan  1 00:02 type
> -rw-r--r-- 1 root root 4096 Jan  1 00:02 uevent
> root@letux:/sys/bus/platform/devices/483072f0.regulator-abb-mpu/regulator/regulator.3# cat *
> cat: device: Is a directory
> 1375000
> -22
> 1012500
> abb_mpu_iva
> 1
> cat: of_node: Is a directory
> cat: power: Is a directory
> 0
> cat: subsystem: Is a directory
> disabled
> disabled
> disabled
> voltage
> OF_NAME=regulator-abb-mpu
> OF_FULLNAME=/ocp@68000000/regulator-abb-mpu
> OF_COMPATIBLE_0=ti,abb-v1
> OF_COMPATIBLE_N=1

I concur with your findings.

> root@letux:/sys/bus/platform/devices/483072f0.regulator-abb-mpu/regulator/regulator.3# cd
> root@letux:~# cpufreq-info
> cpufrequtils 008: cpufreq-info (C) Dominik Brodowski 2004-2009
> Report errors and bugs to cpufreq@vger.kernel.org, please.
> analyzing CPU 0:
>   driver: cpufreq-dt
>   CPUs which run at the same hardware frequency: 0
>   CPUs which need to have their frequency coordinated by software: 0
>   maximum transition latency: 300 us.
>   hardware limits: 300 MHz - 1000 MHz
>   available frequency steps: 300 MHz, 600 MHz, 800 MHz, 1000 MHz
>   available cpufreq governors: conservative, userspace, powersave, ondemand, performance
>   current policy: frequency should be within 300 MHz and 1000 MHz.
>                   The governor "ondemand" may decide which speed to use
>                   within this range.
>   current CPU frequency is 300 MHz (asserted by call to hardware).
>   cpufreq stats: 300 MHz:31.36%, 600 MHz:4.23%, 800 MHz:3.76%, 1000 MHz:60.65%  (1933)
> root@letux:~#
>
> So it runs with different OPPs... My chip may also be more robust to wrong ABB FBB setting.
>
> >
> > If someone has any suggestions on how to test the abb_mpu_iva driver,
> > let me know.
>
> Well, next I want to look into the code for cat microvolts and
> maybe add some printk to understand the result...
>
> A first result is that it comes from
>
>         /* We do not know where the OPP voltage is at the moment */
>         abb->current_info_idx = -EINVAL;
>
> But this is not treated as an -EINVAL but value of -22 microvolts...
> Maybe an error check is missing somewhere in the regulator core.

I assumed this to be -EINVAL, but I'd be happy to be wrong.

>
> I just need more time to track it down.
>
> BR and thanks for the description,
> Nikolaus
>

Re: [Letux-kernel] [RFC PATCH 0/3] Enable 1GHz support on omap36xx

[H. Nikolaus Schaller]

Hi,

> Am 10.09.2019 um 20:30 schrieb Adam Ford <aford173@gmail.com>:
> 
> On Tue, Sep 10, 2019 at 11:59 AM H. Nikolaus Schaller <hns@goldelico.com> wrote:
>> 
>> Hi Adam,
>> 
>>> Am 09.09.2019 um 21:13 schrieb Adam Ford <aford173@gmail.com>:
>>> 
>>> On Mon, Sep 9, 2019 at 1:11 PM H. Nikolaus Schaller <hns@goldelico.com> wrote:
>>>> 
>>>> Ok, we have to check if the ti,abb-v2 "LDO" driver
>>>> drivers/regulator/ti-abb-regulator.c
>>>> can handle that with a DT entry similar to:
>>>> 
>>>> https://elixir.bootlin.com/linux/latest/source/arch/arm/boot/dts/omap5.dtsi#L365
>>> 
>>> At least for the 3630, the ti-abb-regulator driver is the same driver,
>>> but different structures based on v1, v2, or v3 are used based on
>>> which compatible flag is used.
>>> 
>>> I tried enabling the vbb-supply in the device tree, but the driver
>>> doesn't load it without .multi_regulator being true.
>>> 
>>> cpus {
>>> /* OMAP3630/OMAP37xx variants OPP50 to OPP130 and OPP1G */
>>>    cpu: cpu@0 {
>>>         operating-points-v2 = <&cpu0_opp_table>;
>>>         vbb-supply = <&abb_mpu_iva>;
>> 
>> Oh, that is great that the app_mpu_iva already exists!
>> 
>> So we just need plumbing pieces together.
>> 
>>>         clock-latency = <300000>; /* From omap-cpufreq driver */
>>>    };
>>> };
>>> 
>>> I enabled that in the 3630 structure, but then the opp must list
>>> voltage points for both regulators.
>>> Looking at the entry for abb_mpu_iva, it appears to have voltages that
>>> directly match the OPP table, so I made a duplicate entry:
>>> 
>>> opp-microvolt = <1012500 1012500 1012500>,
>>>                          <1012500 1012500 1012500>;
> 
> Out of curiosity, if we're only going to use one value for the opp
> voltage, do we need to have 3 listed?  when I looked at the driver
> yesterday, it appears to support either 1 or 3 entries per opp.
> If we're going to support two regulators, showing them as
> opp-microvolt = <1012500>, <1012500>; would be cleaner and can fit on
> one line.

Well, IMHO it would be cleaner to specify min and max values as well
since the data sheets define them. It is also not clear if we need
them for AVS or such mechanisms.

> 
>>> 
>>> and similar for 600, 800 and 1000 similar to the way dra7.dtsi does
>> 
>> Yes.
>> 
>>> it, but then I got some nasty errors and crashes.
>> 
>> I have done the same but not (yet) seen a crash or error. Maybe you had
>> a typo?
> 
> Can you send me an updated patch?  I'd like to try to get where you
> are that doesn't crash.

Yes, as soon as I have access.

> 
>> 
>>> 
>>> I started undoing the stuff, and I wanted to see if the abb_mpu_iva
>>> regulator was even running, but I would get -22 errors when I went to
>>> read the voltage.
>>> 
>>> # cat /sys/devices/platform/68000000.ocp/483072f0.regulator-abb-mpu/regulator/regulator.5/microvolts
>>> -22
>> 
>> So it reports wrong voltage settings of -22µV...
>> 
>> I have tested and have the same.
>> 
>> root@letux:~# cd /sys/bus/platform/devices/483072f0.regulator-abb-mpu/regulator/regulator.3/
>> root@letux:/sys/bus/platform/devices/483072f0.regulator-abb-mpu/regulator/regulator.3# ls -l
>> total 0
>> lrwxrwxrwx 1 root root    0 Jan  1 00:02 device -> ../../../483072f0.regulator-abb-mpu
>> -r--r--r-- 1 root root 4096 Jan  1 00:02 max_microvolts
>> -r--r--r-- 1 root root 4096 Jan  1 00:02 microvolts
>> -r--r--r-- 1 root root 4096 Jan  1 00:02 min_microvolts
>> -r--r--r-- 1 root root 4096 Jan  1 00:02 name
>> -r--r--r-- 1 root root 4096 Jan  1 00:02 num_users
>> lrwxrwxrwx 1 root root    0 Jan  1 00:02 of_node -> ../../../../../../firmware/devicetree/base/ocp@68000000/regulator-abb-mpu
>> drwxr-xr-x 2 root root    0 Jan  1 00:02 power
>> -r--r--r-- 1 root root 4096 Jan  1 00:02 requested_microamps
>> lrwxrwxrwx 1 root root    0 Jan  1 00:02 subsystem -> ../../../../../../class/regulator
>> -r--r--r-- 1 root root 4096 Jan  1 00:02 suspend_disk_state
>> -r--r--r-- 1 root root 4096 Jan  1 00:02 suspend_mem_state
>> -r--r--r-- 1 root root 4096 Jan  1 00:02 suspend_standby_state
>> -r--r--r-- 1 root root 4096 Jan  1 00:02 type
>> -rw-r--r-- 1 root root 4096 Jan  1 00:02 uevent
>> root@letux:/sys/bus/platform/devices/483072f0.regulator-abb-mpu/regulator/regulator.3# cat *
>> cat: device: Is a directory
>> 1375000
>> -22
>> 1012500
>> abb_mpu_iva
>> 1
>> cat: of_node: Is a directory
>> cat: power: Is a directory
>> 0
>> cat: subsystem: Is a directory
>> disabled
>> disabled
>> disabled
>> voltage
>> OF_NAME=regulator-abb-mpu
>> OF_FULLNAME=/ocp@68000000/regulator-abb-mpu
>> OF_COMPATIBLE_0=ti,abb-v1
>> OF_COMPATIBLE_N=1
> 
> I concur with your findings.
> 
>> root@letux:/sys/bus/platform/devices/483072f0.regulator-abb-mpu/regulator/regulator.3# cd
>> root@letux:~# cpufreq-info
>> cpufrequtils 008: cpufreq-info (C) Dominik Brodowski 2004-2009
>> Report errors and bugs to cpufreq@vger.kernel.org, please.
>> analyzing CPU 0:
>>  driver: cpufreq-dt
>>  CPUs which run at the same hardware frequency: 0
>>  CPUs which need to have their frequency coordinated by software: 0
>>  maximum transition latency: 300 us.
>>  hardware limits: 300 MHz - 1000 MHz
>>  available frequency steps: 300 MHz, 600 MHz, 800 MHz, 1000 MHz
>>  available cpufreq governors: conservative, userspace, powersave, ondemand, performance
>>  current policy: frequency should be within 300 MHz and 1000 MHz.
>>                  The governor "ondemand" may decide which speed to use
>>                  within this range.
>>  current CPU frequency is 300 MHz (asserted by call to hardware).
>>  cpufreq stats: 300 MHz:31.36%, 600 MHz:4.23%, 800 MHz:3.76%, 1000 MHz:60.65%  (1933)
>> root@letux:~#
>> 
>> So it runs with different OPPs... My chip may also be more robust to wrong ABB FBB setting.
>> 
>>> 
>>> If someone has any suggestions on how to test the abb_mpu_iva driver,
>>> let me know.
>> 
>> Well, next I want to look into the code for cat microvolts and
>> maybe add some printk to understand the result...
>> 
>> A first result is that it comes from
>> 
>>        /* We do not know where the OPP voltage is at the moment */
>>        abb->current_info_idx = -EINVAL;
>> 
>> But this is not treated as an -EINVAL but value of -22 microvolts...
>> Maybe an error check is missing somewhere in the regulator core.
> 
> I assumed this to be -EINVAL, but I'd be happy to be wrong.

It seems that cat microvolts stringifies the int returned from reading
the regulator voltage.

Since it is initialized to -EINVAL it returns "-22" as string instead of
converting into an errno return when reading /sys... So one step is
missing a proper error check.

But that is just a symptom that there is no call to set a good voltage.

BR,
Nikolaus

Re: [Letux-kernel] [RFC PATCH 0/3] Enable 1GHz support on omap36xx

[H. Nikolaus Schaller]

Hi Adam,

> Am 10.09.2019 um 20:51 schrieb H. Nikolaus Schaller <hns@goldelico.com>:
> 
> Hi,
> 
>> Am 10.09.2019 um 20:30 schrieb Adam Ford <aford173@gmail.com>:
>> 
>> On Tue, Sep 10, 2019 at 11:59 AM H. Nikolaus Schaller <hns@goldelico.com> wrote:
>>> 

>> 
>> I assumed this to be -EINVAL, but I'd be happy to be wrong.
> 
> It seems that cat microvolts stringifies the int returned from reading
> the regulator voltage.
> 
> Since it is initialized to -EINVAL it returns "-22" as string instead of
> converting into an errno return when reading /sys... So one step is
> missing a proper error check.

Ok, found it in regulator_uV_show().

ret = sprintf(buf, "%d\n", regulator_get_voltage_rdev(rdev));

simply prints the result into a string.

But regulator_get_voltage_rdev() (or _regulator_get_voltage() before v5.3-rc1)
may return errors like -EPROBE_DEFER or -EINVAL or whatever
rdev->desc->ops->get_voltage_sel(rdev) returns.

So this is clearly a bug in regulator_uV_show().

> But that is just a symptom that there is no call to set a good voltage.

That is the next issue to find...

BR,
Nikolaus

Re: [Letux-kernel] [RFC PATCH 0/3] Enable 1GHz support on omap36xx

[Adam Ford]

On Tue, Sep 10, 2019 at 1:51 PM H. Nikolaus Schaller <hns@goldelico.com> wrote:
>
> Hi,
>
> > Am 10.09.2019 um 20:30 schrieb Adam Ford <aford173@gmail.com>:
> >
> > On Tue, Sep 10, 2019 at 11:59 AM H. Nikolaus Schaller <hns@goldelico.com> wrote:
> >>
> >> Hi Adam,
> >>
> >>> Am 09.09.2019 um 21:13 schrieb Adam Ford <aford173@gmail.com>:
> >>>
> >>> On Mon, Sep 9, 2019 at 1:11 PM H. Nikolaus Schaller <hns@goldelico.com> wrote:
> >>>>
> >>>> Ok, we have to check if the ti,abb-v2 "LDO" driver
> >>>> drivers/regulator/ti-abb-regulator.c
> >>>> can handle that with a DT entry similar to:
> >>>>
> >>>> https://elixir.bootlin.com/linux/latest/source/arch/arm/boot/dts/omap5.dtsi#L365
> >>>
> >>> At least for the 3630, the ti-abb-regulator driver is the same driver,
> >>> but different structures based on v1, v2, or v3 are used based on
> >>> which compatible flag is used.
> >>>
> >>> I tried enabling the vbb-supply in the device tree, but the driver
> >>> doesn't load it without .multi_regulator being true.
> >>>
> >>> cpus {
> >>> /* OMAP3630/OMAP37xx variants OPP50 to OPP130 and OPP1G */
> >>>    cpu: cpu@0 {
> >>>         operating-points-v2 = <&cpu0_opp_table>;
> >>>         vbb-supply = <&abb_mpu_iva>;
> >>
> >> Oh, that is great that the app_mpu_iva already exists!
> >>
> >> So we just need plumbing pieces together.
> >>
> >>>         clock-latency = <300000>; /* From omap-cpufreq driver */
> >>>    };
> >>> };
> >>>
> >>> I enabled that in the 3630 structure, but then the opp must list
> >>> voltage points for both regulators.
> >>> Looking at the entry for abb_mpu_iva, it appears to have voltages that
> >>> directly match the OPP table, so I made a duplicate entry:
> >>>
> >>> opp-microvolt = <1012500 1012500 1012500>,
> >>>                          <1012500 1012500 1012500>;
> >
> > Out of curiosity, if we're only going to use one value for the opp
> > voltage, do we need to have 3 listed?  when I looked at the driver
> > yesterday, it appears to support either 1 or 3 entries per opp.
> > If we're going to support two regulators, showing them as
> > opp-microvolt = <1012500>, <1012500>; would be cleaner and can fit on
> > one line.
>
> Well, IMHO it would be cleaner to specify min and max values as well
> since the data sheets define them. It is also not clear if we need
> them for AVS or such mechanisms.
>
> >
> >>>
> >>> and similar for 600, 800 and 1000 similar to the way dra7.dtsi does
> >>
> >> Yes.
> >>
> >>> it, but then I got some nasty errors and crashes.
> >>
> >> I have done the same but not (yet) seen a crash or error. Maybe you had
> >> a typo?
> >
> > Can you send me an updated patch?  I'd like to try to get where you
> > are that doesn't crash.
>
> Yes, as soon as I have access.
>
> >
> >>
> >>>
> >>> I started undoing the stuff, and I wanted to see if the abb_mpu_iva
> >>> regulator was even running, but I would get -22 errors when I went to
> >>> read the voltage.
> >>>
> >>> # cat /sys/devices/platform/68000000.ocp/483072f0.regulator-abb-mpu/regulator/regulator.5/microvolts
> >>> -22
> >>
> >> So it reports wrong voltage settings of -22µV...
> >>
> >> I have tested and have the same.
> >>
> >> root@letux:~# cd /sys/bus/platform/devices/483072f0.regulator-abb-mpu/regulator/regulator.3/
> >> root@letux:/sys/bus/platform/devices/483072f0.regulator-abb-mpu/regulator/regulator.3# ls -l
> >> total 0
> >> lrwxrwxrwx 1 root root    0 Jan  1 00:02 device -> ../../../483072f0.regulator-abb-mpu
> >> -r--r--r-- 1 root root 4096 Jan  1 00:02 max_microvolts
> >> -r--r--r-- 1 root root 4096 Jan  1 00:02 microvolts
> >> -r--r--r-- 1 root root 4096 Jan  1 00:02 min_microvolts
> >> -r--r--r-- 1 root root 4096 Jan  1 00:02 name
> >> -r--r--r-- 1 root root 4096 Jan  1 00:02 num_users
> >> lrwxrwxrwx 1 root root    0 Jan  1 00:02 of_node -> ../../../../../../firmware/devicetree/base/ocp@68000000/regulator-abb-mpu
> >> drwxr-xr-x 2 root root    0 Jan  1 00:02 power
> >> -r--r--r-- 1 root root 4096 Jan  1 00:02 requested_microamps
> >> lrwxrwxrwx 1 root root    0 Jan  1 00:02 subsystem -> ../../../../../../class/regulator
> >> -r--r--r-- 1 root root 4096 Jan  1 00:02 suspend_disk_state
> >> -r--r--r-- 1 root root 4096 Jan  1 00:02 suspend_mem_state
> >> -r--r--r-- 1 root root 4096 Jan  1 00:02 suspend_standby_state
> >> -r--r--r-- 1 root root 4096 Jan  1 00:02 type
> >> -rw-r--r-- 1 root root 4096 Jan  1 00:02 uevent
> >> root@letux:/sys/bus/platform/devices/483072f0.regulator-abb-mpu/regulator/regulator.3# cat *
> >> cat: device: Is a directory
> >> 1375000
> >> -22
> >> 1012500
> >> abb_mpu_iva
> >> 1
> >> cat: of_node: Is a directory
> >> cat: power: Is a directory
> >> 0
> >> cat: subsystem: Is a directory
> >> disabled
> >> disabled
> >> disabled
> >> voltage
> >> OF_NAME=regulator-abb-mpu
> >> OF_FULLNAME=/ocp@68000000/regulator-abb-mpu
> >> OF_COMPATIBLE_0=ti,abb-v1
> >> OF_COMPATIBLE_N=1
> >
> > I concur with your findings.
> >
> >> root@letux:/sys/bus/platform/devices/483072f0.regulator-abb-mpu/regulator/regulator.3# cd
> >> root@letux:~# cpufreq-info
> >> cpufrequtils 008: cpufreq-info (C) Dominik Brodowski 2004-2009
> >> Report errors and bugs to cpufreq@vger.kernel.org, please.
> >> analyzing CPU 0:
> >>  driver: cpufreq-dt
> >>  CPUs which run at the same hardware frequency: 0
> >>  CPUs which need to have their frequency coordinated by software: 0
> >>  maximum transition latency: 300 us.
> >>  hardware limits: 300 MHz - 1000 MHz
> >>  available frequency steps: 300 MHz, 600 MHz, 800 MHz, 1000 MHz
> >>  available cpufreq governors: conservative, userspace, powersave, ondemand, performance
> >>  current policy: frequency should be within 300 MHz and 1000 MHz.
> >>                  The governor "ondemand" may decide which speed to use
> >>                  within this range.
> >>  current CPU frequency is 300 MHz (asserted by call to hardware).
> >>  cpufreq stats: 300 MHz:31.36%, 600 MHz:4.23%, 800 MHz:3.76%, 1000 MHz:60.65%  (1933)
> >> root@letux:~#
> >>
> >> So it runs with different OPPs... My chip may also be more robust to wrong ABB FBB setting.
> >>
> >>>
> >>> If someone has any suggestions on how to test the abb_mpu_iva driver,
> >>> let me know.
> >>
> >> Well, next I want to look into the code for cat microvolts and
> >> maybe add some printk to understand the result...
> >>
> >> A first result is that it comes from
> >>
> >>        /* We do not know where the OPP voltage is at the moment */
> >>        abb->current_info_idx = -EINVAL;
> >>
> >> But this is not treated as an -EINVAL but value of -22 microvolts...
> >> Maybe an error check is missing somewhere in the regulator core.
> >
> > I assumed this to be -EINVAL, but I'd be happy to be wrong.
>
> It seems that cat microvolts stringifies the int returned from reading
> the regulator voltage.
>
> Since it is initialized to -EINVAL it returns "-22" as string instead of
> converting into an errno return when reading /sys... So one step is
> missing a proper error check.
>
> But that is just a symptom that there is no call to set a good voltage.

I unrolled the patches to see what a stock kernel does.  When I 'cat
num_users' it returns 1.  Do you know if there is a way to determine
who the user is?  The stock tree doesn't appear to have any users of
this regulator.

adam
>
> BR,
> Nikolaus
>

Re: [Letux-kernel] [RFC PATCH 0/3] Enable 1GHz support on omap36xx

[H. Nikolaus Schaller]

Ok,

> Am 10.09.2019 um 20:51 schrieb H. Nikolaus Schaller <hns@goldelico.com>:
> 
>>>> it, but then I got some nasty errors and crashes.
>>> 
>>> I have done the same but not (yet) seen a crash or error. Maybe you had
>>> a typo?
>> 
>> Can you send me an updated patch?  I'd like to try to get where you
>> are that doesn't crash.
> 
> Yes, as soon as I have access.

it turns out that my patch breaks cpufreq completely...
So it looks as if *I* have a typo :)

Hence I am likely running at constant speed and the
VDD1 regulator is fixed a 1.200V.

root@letux:~# dmesg|fgrep opp
[    2.426208] cpu cpu0: opp_parse_supplies: Invalid number of elements in opp-microvolt property (6) with supplies (1)
[    2.438140] cpu cpu0: _of_add_opp_table_v2: Failed to add OPP, -22
root@letux:~# cat /sys/class/regulator/regulator.8/microvolts 
1200000
root@letux:~# 

The error message looks as if we have to enable multi_regulator.
And that may need to rename cpu0-supply to vdd-supply (unless the
names can be configured).

BR,
Nikolaus

Re: [Letux-kernel] [RFC PATCH 0/3] Enable 1GHz support on omap36xx

[Adam Ford]

On Tue, Sep 10, 2019 at 2:55 PM H. Nikolaus Schaller <hns@goldelico.com> wrote:
>
> Ok,
>
> > Am 10.09.2019 um 20:51 schrieb H. Nikolaus Schaller <hns@goldelico.com>:
> >
> >>>> it, but then I got some nasty errors and crashes.
> >>>
> >>> I have done the same but not (yet) seen a crash or error. Maybe you had
> >>> a typo?
> >>
> >> Can you send me an updated patch?  I'd like to try to get where you
> >> are that doesn't crash.
> >
> > Yes, as soon as I have access.
>
> it turns out that my patch breaks cpufreq completely...
> So it looks as if *I* have a typo :)
>
> Hence I am likely running at constant speed and the
> VDD1 regulator is fixed a 1.200V.
>
> root@letux:~# dmesg|fgrep opp
> [    2.426208] cpu cpu0: opp_parse_supplies: Invalid number of elements in opp-microvolt property (6) with supplies (1)
> [    2.438140] cpu cpu0: _of_add_opp_table_v2: Failed to add OPP, -22
> root@letux:~# cat /sys/class/regulator/regulator.8/microvolts
> 1200000
> root@letux:~#
>
> The error message looks as if we have to enable multi_regulator.

That will enable both vdd and vbb regulators from what I can tell in the driver.

> And that may need to rename cpu0-supply to vdd-supply (unless the
> names can be configured).

That is consistent with what I found.  vdd-supply = <&vcc>; and
vbb-supply = <&abb_mpu_iva>;
I put them both under the cpu node.  Unfortunately, when I did that,
my board crashed.

I am thinking it has something to do with the abb_mpu_iva driver
because until this point, we've always operated at 800MHz or lower
which all have the same behavior in abb_mpu_iva.

With the patch you posted for the regulator, without the update to
cpufreq,  and with debugging enabled, I received the following in
dmesg:

[    1.112518] ti_abb 483072f0.regulator-abb-mpu: Missing
'efuse-address' IO resource
[    1.112579] ti_abb 483072f0.regulator-abb-mpu: [0]v=1012500 ABB=0
ef=0x0 rbb=0x0 fbb=0x0 vset=0x0
[    1.112609] ti_abb 483072f0.regulator-abb-mpu: [1]v=1200000 ABB=0
ef=0x0 rbb=0x0 fbb=0x0 vset=0x0
[    1.112609] ti_abb 483072f0.regulator-abb-mpu: [2]v=1325000 ABB=0
ef=0x0 rbb=0x0 fbb=0x0 vset=0x0
[    1.112640] ti_abb 483072f0.regulator-abb-mpu: [3]v=1375000 ABB=1
ef=0x0 rbb=0x0 fbb=0x0 vset=0x0
[    1.112731] ti_abb 483072f0.regulator-abb-mpu: ti_abb_init_timings:
Clk_rate=13000000, sr2_cnt=0x00000032


adam
>
> BR,
> Nikolaus
>

Re: [Letux-kernel] [RFC PATCH 0/3] Enable 1GHz support on omap36xx

[Adam Ford]

On Tue, Sep 10, 2019 at 3:06 PM Adam Ford <aford173@gmail.com> wrote:
>
> On Tue, Sep 10, 2019 at 2:55 PM H. Nikolaus Schaller <hns@goldelico.com> wrote:
> >
> > Ok,
> >
> > > Am 10.09.2019 um 20:51 schrieb H. Nikolaus Schaller <hns@goldelico.com>:
> > >
> > >>>> it, but then I got some nasty errors and crashes.
> > >>>
> > >>> I have done the same but not (yet) seen a crash or error. Maybe you had
> > >>> a typo?
> > >>
> > >> Can you send me an updated patch?  I'd like to try to get where you
> > >> are that doesn't crash.
> > >
> > > Yes, as soon as I have access.
> >
> > it turns out that my patch breaks cpufreq completely...
> > So it looks as if *I* have a typo :)
> >
> > Hence I am likely running at constant speed and the
> > VDD1 regulator is fixed a 1.200V.
> >
> > root@letux:~# dmesg|fgrep opp
> > [    2.426208] cpu cpu0: opp_parse_supplies: Invalid number of elements in opp-microvolt property (6) with supplies (1)
> > [    2.438140] cpu cpu0: _of_add_opp_table_v2: Failed to add OPP, -22
> > root@letux:~# cat /sys/class/regulator/regulator.8/microvolts
> > 1200000
> > root@letux:~#
> >
> > The error message looks as if we have to enable multi_regulator.
>
> That will enable both vdd and vbb regulators from what I can tell in the driver.
>
> > And that may need to rename cpu0-supply to vdd-supply (unless the
> > names can be configured).
>
> That is consistent with what I found.  vdd-supply = <&vcc>; and
> vbb-supply = <&abb_mpu_iva>;
> I put them both under the cpu node.  Unfortunately, when I did that,
> my board crashed.
>
> I am thinking it has something to do with the abb_mpu_iva driver
> because until this point, we've always operated at 800MHz or lower
> which all have the same behavior in abb_mpu_iva.
>
> With the patch you posted for the regulator, without the update to
> cpufreq,  and with debugging enabled, I received the following in
> dmesg:
>
> [    1.112518] ti_abb 483072f0.regulator-abb-mpu: Missing
> 'efuse-address' IO resource
> [    1.112579] ti_abb 483072f0.regulator-abb-mpu: [0]v=1012500 ABB=0
> ef=0x0 rbb=0x0 fbb=0x0 vset=0x0
> [    1.112609] ti_abb 483072f0.regulator-abb-mpu: [1]v=1200000 ABB=0
> ef=0x0 rbb=0x0 fbb=0x0 vset=0x0
> [    1.112609] ti_abb 483072f0.regulator-abb-mpu: [2]v=1325000 ABB=0
> ef=0x0 rbb=0x0 fbb=0x0 vset=0x0
> [    1.112640] ti_abb 483072f0.regulator-abb-mpu: [3]v=1375000 ABB=1
> ef=0x0 rbb=0x0 fbb=0x0 vset=0x0
> [    1.112731] ti_abb 483072f0.regulator-abb-mpu: ti_abb_init_timings:
> Clk_rate=13000000, sr2_cnt=0x00000032
>

Using an unmodified kernel, I changed the device tree to make the abb
regulator power the cpu instead of vcc.  After doing so, I was able to
read the microvolt value, and it matched the processor's desired OPP
voltage, and the debug code showed the abb regulator was attempting to
set the various index based on the needed voltage.  I think the abb
driver is working correctly.

I think tomorrow, I will re-apply the patches and tweak it again to
support both vdd and vbb regulators.  If it crashes again, I'll look
more closely at the logs to see if I can determine why.  I think it's
pretty close.  I also need to go back and find the SmartReflex stuff
as well.

adam
>
> adam
> >
> > BR,
> > Nikolaus
> >

Re: [Letux-kernel] [RFC PATCH 0/3] Enable 1GHz support on omap36xx

[Adam Ford]

On Tue, Sep 10, 2019 at 7:24 PM Adam Ford <aford173@gmail.com> wrote:
>
> On Tue, Sep 10, 2019 at 3:06 PM Adam Ford <aford173@gmail.com> wrote:
> >
> > On Tue, Sep 10, 2019 at 2:55 PM H. Nikolaus Schaller <hns@goldelico.com> wrote:
> > >
> > > Ok,
> > >
> > > > Am 10.09.2019 um 20:51 schrieb H. Nikolaus Schaller <hns@goldelico.com>:
> > > >
> > > >>>> it, but then I got some nasty errors and crashes.
> > > >>>
> > > >>> I have done the same but not (yet) seen a crash or error. Maybe you had
> > > >>> a typo?
> > > >>
> > > >> Can you send me an updated patch?  I'd like to try to get where you
> > > >> are that doesn't crash.
> > > >
> > > > Yes, as soon as I have access.
> > >
> > > it turns out that my patch breaks cpufreq completely...
> > > So it looks as if *I* have a typo :)
> > >
> > > Hence I am likely running at constant speed and the
> > > VDD1 regulator is fixed a 1.200V.
> > >
> > > root@letux:~# dmesg|fgrep opp
> > > [    2.426208] cpu cpu0: opp_parse_supplies: Invalid number of elements in opp-microvolt property (6) with supplies (1)
> > > [    2.438140] cpu cpu0: _of_add_opp_table_v2: Failed to add OPP, -22
> > > root@letux:~# cat /sys/class/regulator/regulator.8/microvolts
> > > 1200000
> > > root@letux:~#
> > >
> > > The error message looks as if we have to enable multi_regulator.
> >
> > That will enable both vdd and vbb regulators from what I can tell in the driver.
> >
> > > And that may need to rename cpu0-supply to vdd-supply (unless the
> > > names can be configured).
> >
> > That is consistent with what I found.  vdd-supply = <&vcc>; and
> > vbb-supply = <&abb_mpu_iva>;
> > I put them both under the cpu node.  Unfortunately, when I did that,
> > my board crashed.
> >
> > I am thinking it has something to do with the abb_mpu_iva driver
> > because until this point, we've always operated at 800MHz or lower
> > which all have the same behavior in abb_mpu_iva.
> >
> > With the patch you posted for the regulator, without the update to
> > cpufreq,  and with debugging enabled, I received the following in
> > dmesg:
> >
> > [    1.112518] ti_abb 483072f0.regulator-abb-mpu: Missing
> > 'efuse-address' IO resource
> > [    1.112579] ti_abb 483072f0.regulator-abb-mpu: [0]v=1012500 ABB=0
> > ef=0x0 rbb=0x0 fbb=0x0 vset=0x0
> > [    1.112609] ti_abb 483072f0.regulator-abb-mpu: [1]v=1200000 ABB=0
> > ef=0x0 rbb=0x0 fbb=0x0 vset=0x0
> > [    1.112609] ti_abb 483072f0.regulator-abb-mpu: [2]v=1325000 ABB=0
> > ef=0x0 rbb=0x0 fbb=0x0 vset=0x0
> > [    1.112640] ti_abb 483072f0.regulator-abb-mpu: [3]v=1375000 ABB=1
> > ef=0x0 rbb=0x0 fbb=0x0 vset=0x0
> > [    1.112731] ti_abb 483072f0.regulator-abb-mpu: ti_abb_init_timings:
> > Clk_rate=13000000, sr2_cnt=0x00000032
> >
>
> Using an unmodified kernel, I changed the device tree to make the abb
> regulator power the cpu instead of vcc.  After doing so, I was able to
> read the microvolt value, and it matched the processor's desired OPP
> voltage, and the debug code showed the abb regulator was attempting to
> set the various index based on the needed voltage.  I think the abb
> driver is working correctly.
>
> I think tomorrow, I will re-apply the patches and tweak it again to
> support both vdd and vbb regulators.  If it crashes again, I'll look
> more closely at the logs to see if I can determine why.  I think it's
> pretty close.  I also need to go back and find the SmartReflex stuff
> as well.
>
Well, I couldn't give it up for the night, so I thought I'd show my data dump

[    9.807647] ------------[ cut here ]------------
[    9.812469] WARNING: CPU: 0 PID: 68 at drivers/opp/core.c:630
dev_pm_opp_set_rate+0x3cc/0x480
[    9.821044] Modules linked in: sha256_generic sha256_arm cfg80211
joydev mousedev evdev snd_soc_omap_twl4030(+) leds_gpio led_class
panel_simple pwm_omap_dmtimer gpio_keys pwm_bl cpufreq_dt omap3_isp v
ideobuf2_dma_contig videobuf2_memops videobuf2_v4l2 videobuf2_common
bq27xxx_battery_hdq v4l2_fwnode snd_soc_omap_mcbsp bq27xxx_battery
snd_soc_ti_sdma omap_wdt videodev mc omap_hdq wlcore_sdio wire cn ph
y_twl4030_usb hwmon omap2430 musb_hdrc omap_mailbox twl4030_wdt
watchdog udc_core rtc_twl snd_soc_twl4030 ohci_platform(+)
snd_soc_core snd_pcm_dmaengine ohci_hcd snd_pcm ehci_omap(+)
twl4030_pwrbutton sn
d_timer twl4030_charger snd pwm_twl_led pwm_twl ehci_hcd industrialio
soundcore twl4030_keypad matrix_keymap usbcore at24 tsc2004
tsc200x_core usb_common omap_ssi hsi omapdss omapdss_base drm
drm_panel_or
ientation_quirks cec
[    9.894470] CPU: 0 PID: 68 Comm: kworker/0:2 Not tainted
5.3.0-rc3-00785-gfdfc7f21c6b7-dirty #5
[    9.903198] Hardware name: Generic OMAP36xx (Flattened Device Tree)
[    9.909515] Workqueue: events dbs_work_handler
[    9.914031] [<c01122d8>] (unwind_backtrace) from [<c010c8b8>]
(show_stack+0x10/0x14)
[    9.921813] [<c010c8b8>] (show_stack) from [<c089e858>]
(dump_stack+0xb4/0xd4)
[    9.929107] [<c089e858>] (dump_stack) from [<c0139eb0>]
(__warn.part.3+0xa8/0xd4)
[    9.936614] [<c0139eb0>] (__warn.part.3) from [<c013a034>]
(warn_slowpath_null+0x40/0x4c)
[    9.944854] [<c013a034>] (warn_slowpath_null) from [<c06d666c>]
(dev_pm_opp_set_rate+0x3cc/0x480)
[    9.953796] [<c06d666c>] (dev_pm_opp_set_rate) from [<bf1790ac>]
(set_target+0x30/0x58 [cpufreq_dt])
[    9.963012] [<bf1790ac>] (set_target [cpufreq_dt]) from
[<c06db05c>] (__cpufreq_driver_target+0x188/0x514)
[    9.972717] [<c06db05c>] (__cpufreq_driver_target) from
[<c06de050>] (od_dbs_update+0x130/0x15c)
[    9.981567] [<c06de050>] (od_dbs_update) from [<c06deb08>]
(dbs_work_handler+0x28/0x58)
[    9.989624] [<c06deb08>] (dbs_work_handler) from [<c0154ab0>]
(process_one_work+0x20c/0x500)
[    9.998107] [<c0154ab0>] (process_one_work) from [<c0155e8c>]
(worker_thread+0x2c/0x5bc)
[   10.006256] [<c0155e8c>] (worker_thread) from [<c015ab88>]
(kthread+0x134/0x148)
[   10.013702] [<c015ab88>] (kthread) from [<c01010e8>]
(ret_from_fork+0x14/0x2c)
[   10.020965] Exception stack(0xde63bfb0 to 0xde63bff8)
[   10.026062] bfa0:                                     00000000
00000000 00000000 00000000
[   10.034271] bfc0: 00000000 00000000 00000000 00000000 00000000
00000000 00000000 00000000
[   10.042510] bfe0: 00000000 00000000 00000000 00000000 00000013 00000000
[   10.049224] ---[ end trace cf0e15fa4bd57700 ]---
[   10.053894] cpu cpu0: multiple regulators are not supported
[   10.059509] cpufreq: __target_index: Failed to change cpu frequency: -22



> adam
> >
> > adam
> > >
> > > BR,
> > > Nikolaus
> > >

Re: [Letux-kernel] [RFC PATCH 0/3] Enable 1GHz support on omap36xx

[H. Nikolaus Schaller]

Hi Adam,

> Am 11.09.2019 um 02:41 schrieb Adam Ford <aford173@gmail.com>:
>>>> 

>>>> The error message looks as if we have to enable multi_regulator.

>>> 
>>> That will enable both vdd and vbb regulators from what I can tell in the driver.
>>> 
>>>> And that may need to rename cpu0-supply to vdd-supply (unless the
>>>> names can be configured).
>>> 
>>> That is consistent with what I found.  vdd-supply = <&vcc>; and
>>> vbb-supply = <&abb_mpu_iva>;
>>> I put them both under the cpu node.  Unfortunately, when I did that,
>>> my board crashed.
>>> 
>>> I am thinking it has something to do with the abb_mpu_iva driver
>>> because until this point, we've always operated at 800MHz or lower
>>> which all have the same behavior in abb_mpu_iva.
>>> 
>>> With the patch you posted for the regulator, without the update to
>>> cpufreq,  and with debugging enabled, I received the following in
>>> dmesg:
>>> 
>>> [    1.112518] ti_abb 483072f0.regulator-abb-mpu: Missing
>>> 'efuse-address' IO resource
>>> [    1.112579] ti_abb 483072f0.regulator-abb-mpu: [0]v=1012500 ABB=0
>>> ef=0x0 rbb=0x0 fbb=0x0 vset=0x0
>>> [    1.112609] ti_abb 483072f0.regulator-abb-mpu: [1]v=1200000 ABB=0
>>> ef=0x0 rbb=0x0 fbb=0x0 vset=0x0
>>> [    1.112609] ti_abb 483072f0.regulator-abb-mpu: [2]v=1325000 ABB=0
>>> ef=0x0 rbb=0x0 fbb=0x0 vset=0x0
>>> [    1.112640] ti_abb 483072f0.regulator-abb-mpu: [3]v=1375000 ABB=1
>>> ef=0x0 rbb=0x0 fbb=0x0 vset=0x0
>>> [    1.112731] ti_abb 483072f0.regulator-abb-mpu: ti_abb_init_timings:
>>> Clk_rate=13000000, sr2_cnt=0x00000032
>>> 
>> 
>> Using an unmodified kernel, I changed the device tree to make the abb
>> regulator power the cpu instead of vcc.  After doing so, I was able to
>> read the microvolt value, and it matched the processor's desired OPP
>> voltage, and the debug code showed the abb regulator was attempting to
>> set the various index based on the needed voltage.  I think the abb
>> driver is working correctly.
>> 
>> I think tomorrow, I will re-apply the patches and tweak it again to
>> support both vdd and vbb regulators.  If it crashes again, I'll look
>> more closely at the logs to see if I can determine why.  I think it's
>> pretty close.  I also need to go back and find the SmartReflex stuff
>> as well.
>> 
> Well, I couldn't give it up for the night, so I thought I'd show my data dump
> 
> [    9.807647] ------------[ cut here ]------------
> [    9.812469] WARNING: CPU: 0 PID: 68 at drivers/opp/core.c:630
> dev_pm_opp_set_rate+0x3cc/0x480
> [    9.821044] Modules linked in: sha256_generic sha256_arm cfg80211
> joydev mousedev evdev snd_soc_omap_twl4030(+) leds_gpio led_class
> panel_simple pwm_omap_dmtimer gpio_keys pwm_bl cpufreq_dt omap3_isp v
> ideobuf2_dma_contig videobuf2_memops videobuf2_v4l2 videobuf2_common
> bq27xxx_battery_hdq v4l2_fwnode snd_soc_omap_mcbsp bq27xxx_battery
> snd_soc_ti_sdma omap_wdt videodev mc omap_hdq wlcore_sdio wire cn ph
> y_twl4030_usb hwmon omap2430 musb_hdrc omap_mailbox twl4030_wdt
> watchdog udc_core rtc_twl snd_soc_twl4030 ohci_platform(+)
> snd_soc_core snd_pcm_dmaengine ohci_hcd snd_pcm ehci_omap(+)
> twl4030_pwrbutton sn
> d_timer twl4030_charger snd pwm_twl_led pwm_twl ehci_hcd industrialio
> soundcore twl4030_keypad matrix_keymap usbcore at24 tsc2004
> tsc200x_core usb_common omap_ssi hsi omapdss omapdss_base drm
> drm_panel_or
> ientation_quirks cec
> [    9.894470] CPU: 0 PID: 68 Comm: kworker/0:2 Not tainted
> 5.3.0-rc3-00785-gfdfc7f21c6b7-dirty #5
> [    9.903198] Hardware name: Generic OMAP36xx (Flattened Device Tree)
> [    9.909515] Workqueue: events dbs_work_handler
> [    9.914031] [<c01122d8>] (unwind_backtrace) from [<c010c8b8>]
> (show_stack+0x10/0x14)
> [    9.921813] [<c010c8b8>] (show_stack) from [<c089e858>]
> (dump_stack+0xb4/0xd4)
> [    9.929107] [<c089e858>] (dump_stack) from [<c0139eb0>]
> (__warn.part.3+0xa8/0xd4)
> [    9.936614] [<c0139eb0>] (__warn.part.3) from [<c013a034>]
> (warn_slowpath_null+0x40/0x4c)
> [    9.944854] [<c013a034>] (warn_slowpath_null) from [<c06d666c>]
> (dev_pm_opp_set_rate+0x3cc/0x480)
> [    9.953796] [<c06d666c>] (dev_pm_opp_set_rate) from [<bf1790ac>]
> (set_target+0x30/0x58 [cpufreq_dt])
> [    9.963012] [<bf1790ac>] (set_target [cpufreq_dt]) from
> [<c06db05c>] (__cpufreq_driver_target+0x188/0x514)
> [    9.972717] [<c06db05c>] (__cpufreq_driver_target) from
> [<c06de050>] (od_dbs_update+0x130/0x15c)
> [    9.981567] [<c06de050>] (od_dbs_update) from [<c06deb08>]
> (dbs_work_handler+0x28/0x58)
> [    9.989624] [<c06deb08>] (dbs_work_handler) from [<c0154ab0>]
> (process_one_work+0x20c/0x500)
> [    9.998107] [<c0154ab0>] (process_one_work) from [<c0155e8c>]
> (worker_thread+0x2c/0x5bc)
> [   10.006256] [<c0155e8c>] (worker_thread) from [<c015ab88>]
> (kthread+0x134/0x148)
> [   10.013702] [<c015ab88>] (kthread) from [<c01010e8>]
> (ret_from_fork+0x14/0x2c)
> [   10.020965] Exception stack(0xde63bfb0 to 0xde63bff8)
> [   10.026062] bfa0:                                     00000000
> 00000000 00000000 00000000
> [   10.034271] bfc0: 00000000 00000000 00000000 00000000 00000000
> 00000000 00000000 00000000
> [   10.042510] bfe0: 00000000 00000000 00000000 00000000 00000013 00000000
> [   10.049224] ---[ end trace cf0e15fa4bd57700 ]---
> [   10.053894] cpu cpu0: multiple regulators are not supported
> [   10.059509] cpufreq: __target_index: Failed to change cpu frequency: -22

I have the same:

[    4.701354] cpu cpu0: multiple regulators are not supported
[    4.707794] cpufreq: __target_index: Failed to change cpu frequency: -22

Comes from within dev_pm_opp_set_rate().

It appears that we also have to define opp_table->set_opp to make use
of _set_opp_custom(). And I am not sure which custom-opp-setter we should
use. Maybe ti_opp_supply_set_opp() is ok. Or not.

BR,
Nikolaus

Re: [Letux-kernel] [RFC PATCH 0/3] Enable 1GHz support on omap36xx

[H. Nikolaus Schaller]

> Am 11.09.2019 um 07:13 schrieb H. Nikolaus Schaller <hns@goldelico.com>:
> 
> Hi Adam,
> 
>> Am 11.09.2019 um 02:41 schrieb Adam Ford <aford173@gmail.com>:
>>>>> 
> 
>>>>> The error message looks as if we have to enable multi_regulator.
> 
>>>> 
>>>> That will enable both vdd and vbb regulators from what I can tell in the driver.
>>>> 
>>>>> And that may need to rename cpu0-supply to vdd-supply (unless the
>>>>> names can be configured).
>>>> 
>>>> That is consistent with what I found.  vdd-supply = <&vcc>; and
>>>> vbb-supply = <&abb_mpu_iva>;
>>>> I put them both under the cpu node.  Unfortunately, when I did that,
>>>> my board crashed.
>>>> 
>>>> I am thinking it has something to do with the abb_mpu_iva driver
>>>> because until this point, we've always operated at 800MHz or lower
>>>> which all have the same behavior in abb_mpu_iva.
>>>> 
>>>> With the patch you posted for the regulator, without the update to
>>>> cpufreq,  and with debugging enabled, I received the following in
>>>> dmesg:
>>>> 
>>>> [    1.112518] ti_abb 483072f0.regulator-abb-mpu: Missing
>>>> 'efuse-address' IO resource
>>>> [    1.112579] ti_abb 483072f0.regulator-abb-mpu: [0]v=1012500 ABB=0
>>>> ef=0x0 rbb=0x0 fbb=0x0 vset=0x0
>>>> [    1.112609] ti_abb 483072f0.regulator-abb-mpu: [1]v=1200000 ABB=0
>>>> ef=0x0 rbb=0x0 fbb=0x0 vset=0x0
>>>> [    1.112609] ti_abb 483072f0.regulator-abb-mpu: [2]v=1325000 ABB=0
>>>> ef=0x0 rbb=0x0 fbb=0x0 vset=0x0
>>>> [    1.112640] ti_abb 483072f0.regulator-abb-mpu: [3]v=1375000 ABB=1
>>>> ef=0x0 rbb=0x0 fbb=0x0 vset=0x0
>>>> [    1.112731] ti_abb 483072f0.regulator-abb-mpu: ti_abb_init_timings:
>>>> Clk_rate=13000000, sr2_cnt=0x00000032
>>>> 
>>> 
>>> Using an unmodified kernel, I changed the device tree to make the abb
>>> regulator power the cpu instead of vcc.  After doing so, I was able to
>>> read the microvolt value, and it matched the processor's desired OPP
>>> voltage, and the debug code showed the abb regulator was attempting to
>>> set the various index based on the needed voltage.  I think the abb
>>> driver is working correctly.
>>> 
>>> I think tomorrow, I will re-apply the patches and tweak it again to
>>> support both vdd and vbb regulators.  If it crashes again, I'll look
>>> more closely at the logs to see if I can determine why.  I think it's
>>> pretty close.  I also need to go back and find the SmartReflex stuff
>>> as well.
>>> 
>> Well, I couldn't give it up for the night, so I thought I'd show my data dump
>> 
>> [    9.807647] ------------[ cut here ]------------
>> [    9.812469] WARNING: CPU: 0 PID: 68 at drivers/opp/core.c:630
>> dev_pm_opp_set_rate+0x3cc/0x480
>> [    9.821044] Modules linked in: sha256_generic sha256_arm cfg80211
>> joydev mousedev evdev snd_soc_omap_twl4030(+) leds_gpio led_class
>> panel_simple pwm_omap_dmtimer gpio_keys pwm_bl cpufreq_dt omap3_isp v
>> ideobuf2_dma_contig videobuf2_memops videobuf2_v4l2 videobuf2_common
>> bq27xxx_battery_hdq v4l2_fwnode snd_soc_omap_mcbsp bq27xxx_battery
>> snd_soc_ti_sdma omap_wdt videodev mc omap_hdq wlcore_sdio wire cn ph
>> y_twl4030_usb hwmon omap2430 musb_hdrc omap_mailbox twl4030_wdt
>> watchdog udc_core rtc_twl snd_soc_twl4030 ohci_platform(+)
>> snd_soc_core snd_pcm_dmaengine ohci_hcd snd_pcm ehci_omap(+)
>> twl4030_pwrbutton sn
>> d_timer twl4030_charger snd pwm_twl_led pwm_twl ehci_hcd industrialio
>> soundcore twl4030_keypad matrix_keymap usbcore at24 tsc2004
>> tsc200x_core usb_common omap_ssi hsi omapdss omapdss_base drm
>> drm_panel_or
>> ientation_quirks cec
>> [    9.894470] CPU: 0 PID: 68 Comm: kworker/0:2 Not tainted
>> 5.3.0-rc3-00785-gfdfc7f21c6b7-dirty #5
>> [    9.903198] Hardware name: Generic OMAP36xx (Flattened Device Tree)
>> [    9.909515] Workqueue: events dbs_work_handler
>> [    9.914031] [<c01122d8>] (unwind_backtrace) from [<c010c8b8>]
>> (show_stack+0x10/0x14)
>> [    9.921813] [<c010c8b8>] (show_stack) from [<c089e858>]
>> (dump_stack+0xb4/0xd4)
>> [    9.929107] [<c089e858>] (dump_stack) from [<c0139eb0>]
>> (__warn.part.3+0xa8/0xd4)
>> [    9.936614] [<c0139eb0>] (__warn.part.3) from [<c013a034>]
>> (warn_slowpath_null+0x40/0x4c)
>> [    9.944854] [<c013a034>] (warn_slowpath_null) from [<c06d666c>]
>> (dev_pm_opp_set_rate+0x3cc/0x480)
>> [    9.953796] [<c06d666c>] (dev_pm_opp_set_rate) from [<bf1790ac>]
>> (set_target+0x30/0x58 [cpufreq_dt])
>> [    9.963012] [<bf1790ac>] (set_target [cpufreq_dt]) from
>> [<c06db05c>] (__cpufreq_driver_target+0x188/0x514)
>> [    9.972717] [<c06db05c>] (__cpufreq_driver_target) from
>> [<c06de050>] (od_dbs_update+0x130/0x15c)
>> [    9.981567] [<c06de050>] (od_dbs_update) from [<c06deb08>]
>> (dbs_work_handler+0x28/0x58)
>> [    9.989624] [<c06deb08>] (dbs_work_handler) from [<c0154ab0>]
>> (process_one_work+0x20c/0x500)
>> [    9.998107] [<c0154ab0>] (process_one_work) from [<c0155e8c>]
>> (worker_thread+0x2c/0x5bc)
>> [   10.006256] [<c0155e8c>] (worker_thread) from [<c015ab88>]
>> (kthread+0x134/0x148)
>> [   10.013702] [<c015ab88>] (kthread) from [<c01010e8>]
>> (ret_from_fork+0x14/0x2c)
>> [   10.020965] Exception stack(0xde63bfb0 to 0xde63bff8)
>> [   10.026062] bfa0:                                     00000000
>> 00000000 00000000 00000000
>> [   10.034271] bfc0: 00000000 00000000 00000000 00000000 00000000
>> 00000000 00000000 00000000
>> [   10.042510] bfe0: 00000000 00000000 00000000 00000000 00000013 00000000
>> [   10.049224] ---[ end trace cf0e15fa4bd57700 ]---
>> [   10.053894] cpu cpu0: multiple regulators are not supported
>> [   10.059509] cpufreq: __target_index: Failed to change cpu frequency: -22
> 
> I have the same:
> 
> [    4.701354] cpu cpu0: multiple regulators are not supported
> [    4.707794] cpufreq: __target_index: Failed to change cpu frequency: -22
> 
> Comes from within dev_pm_opp_set_rate().
> 
> It appears that we also have to define opp_table->set_opp to make use
> of _set_opp_custom(). And I am not sure which custom-opp-setter we should
> use. Maybe ti_opp_supply_set_opp() is ok. Or not.

This appears to be set by dra7.dtsi through loading the
"ti,omap5-opp-supply" compatible driver:

https://elixir.bootlin.com/linux/v5.3-rc8/source/arch/arm/boot/dts/dra7.dtsi#L699

Maybe we can use "ti,omap-opp-supply" here, which does not read
additional eFuses?

See also

https://www.kernel.org/doc/Documentation/devicetree/bindings/opp/ti-omap5-opp-supply.txt

And, if I understand the code ti_opp_supply_set_opp() correctly, we may not have
to rename cpu0-suppy to vdd-supply because that driver takes the first
regulator as vdd and the second as vbb.

Something like

opp_supply_mpu_iva: opp_supply {
	compatible = "ti,omap-opp-supply";
	ti,absolute-max-voltage-uv = <1375000>;
};

But that is a quite wild guess...

BR,
Nikolaus

Re: [Letux-kernel] [RFC PATCH 0/3] Enable 1GHz support on omap36xx

[H. Nikolaus Schaller]

> Am 11.09.2019 um 08:03 schrieb H. Nikolaus Schaller <hns@goldelico.com>:
> 
> 
>> Am 11.09.2019 um 07:13 schrieb H. Nikolaus Schaller <hns@goldelico.com>:
>> 
>> Hi Adam,
>> 
>>> Am 11.09.2019 um 02:41 schrieb Adam Ford <aford173@gmail.com>:
>>>>>> 
>> 
>>>>>> The error message looks as if we have to enable multi_regulator.
>> 
>>>>> 
>>>>> That will enable both vdd and vbb regulators from what I can tell in the driver.
>>>>> 
>>>>>> And that may need to rename cpu0-supply to vdd-supply (unless the
>>>>>> names can be configured).
>>>>> 
>>>>> That is consistent with what I found.  vdd-supply = <&vcc>; and
>>>>> vbb-supply = <&abb_mpu_iva>;
>>>>> I put them both under the cpu node.  Unfortunately, when I did that,
>>>>> my board crashed.
>>>>> 
>>>>> I am thinking it has something to do with the abb_mpu_iva driver
>>>>> because until this point, we've always operated at 800MHz or lower
>>>>> which all have the same behavior in abb_mpu_iva.
>>>>> 
>>>>> With the patch you posted for the regulator, without the update to
>>>>> cpufreq,  and with debugging enabled, I received the following in
>>>>> dmesg:
>>>>> 
>>>>> [    1.112518] ti_abb 483072f0.regulator-abb-mpu: Missing
>>>>> 'efuse-address' IO resource
>>>>> [    1.112579] ti_abb 483072f0.regulator-abb-mpu: [0]v=1012500 ABB=0
>>>>> ef=0x0 rbb=0x0 fbb=0x0 vset=0x0
>>>>> [    1.112609] ti_abb 483072f0.regulator-abb-mpu: [1]v=1200000 ABB=0
>>>>> ef=0x0 rbb=0x0 fbb=0x0 vset=0x0
>>>>> [    1.112609] ti_abb 483072f0.regulator-abb-mpu: [2]v=1325000 ABB=0
>>>>> ef=0x0 rbb=0x0 fbb=0x0 vset=0x0
>>>>> [    1.112640] ti_abb 483072f0.regulator-abb-mpu: [3]v=1375000 ABB=1
>>>>> ef=0x0 rbb=0x0 fbb=0x0 vset=0x0
>>>>> [    1.112731] ti_abb 483072f0.regulator-abb-mpu: ti_abb_init_timings:
>>>>> Clk_rate=13000000, sr2_cnt=0x00000032
>>>>> 
>>>> 
>>>> Using an unmodified kernel, I changed the device tree to make the abb
>>>> regulator power the cpu instead of vcc.  After doing so, I was able to
>>>> read the microvolt value, and it matched the processor's desired OPP
>>>> voltage, and the debug code showed the abb regulator was attempting to
>>>> set the various index based on the needed voltage.  I think the abb
>>>> driver is working correctly.
>>>> 
>>>> I think tomorrow, I will re-apply the patches and tweak it again to
>>>> support both vdd and vbb regulators.  If it crashes again, I'll look
>>>> more closely at the logs to see if I can determine why.  I think it's
>>>> pretty close.  I also need to go back and find the SmartReflex stuff
>>>> as well.
>>>> 
>>> Well, I couldn't give it up for the night, so I thought I'd show my data dump
>>> 
>>> [    9.807647] ------------[ cut here ]------------
>>> [    9.812469] WARNING: CPU: 0 PID: 68 at drivers/opp/core.c:630
>>> dev_pm_opp_set_rate+0x3cc/0x480
>>> [    9.821044] Modules linked in: sha256_generic sha256_arm cfg80211
>>> joydev mousedev evdev snd_soc_omap_twl4030(+) leds_gpio led_class
>>> panel_simple pwm_omap_dmtimer gpio_keys pwm_bl cpufreq_dt omap3_isp v
>>> ideobuf2_dma_contig videobuf2_memops videobuf2_v4l2 videobuf2_common
>>> bq27xxx_battery_hdq v4l2_fwnode snd_soc_omap_mcbsp bq27xxx_battery
>>> snd_soc_ti_sdma omap_wdt videodev mc omap_hdq wlcore_sdio wire cn ph
>>> y_twl4030_usb hwmon omap2430 musb_hdrc omap_mailbox twl4030_wdt
>>> watchdog udc_core rtc_twl snd_soc_twl4030 ohci_platform(+)
>>> snd_soc_core snd_pcm_dmaengine ohci_hcd snd_pcm ehci_omap(+)
>>> twl4030_pwrbutton sn
>>> d_timer twl4030_charger snd pwm_twl_led pwm_twl ehci_hcd industrialio
>>> soundcore twl4030_keypad matrix_keymap usbcore at24 tsc2004
>>> tsc200x_core usb_common omap_ssi hsi omapdss omapdss_base drm
>>> drm_panel_or
>>> ientation_quirks cec
>>> [    9.894470] CPU: 0 PID: 68 Comm: kworker/0:2 Not tainted
>>> 5.3.0-rc3-00785-gfdfc7f21c6b7-dirty #5
>>> [    9.903198] Hardware name: Generic OMAP36xx (Flattened Device Tree)
>>> [    9.909515] Workqueue: events dbs_work_handler
>>> [    9.914031] [<c01122d8>] (unwind_backtrace) from [<c010c8b8>]
>>> (show_stack+0x10/0x14)
>>> [    9.921813] [<c010c8b8>] (show_stack) from [<c089e858>]
>>> (dump_stack+0xb4/0xd4)
>>> [    9.929107] [<c089e858>] (dump_stack) from [<c0139eb0>]
>>> (__warn.part.3+0xa8/0xd4)
>>> [    9.936614] [<c0139eb0>] (__warn.part.3) from [<c013a034>]
>>> (warn_slowpath_null+0x40/0x4c)
>>> [    9.944854] [<c013a034>] (warn_slowpath_null) from [<c06d666c>]
>>> (dev_pm_opp_set_rate+0x3cc/0x480)
>>> [    9.953796] [<c06d666c>] (dev_pm_opp_set_rate) from [<bf1790ac>]
>>> (set_target+0x30/0x58 [cpufreq_dt])
>>> [    9.963012] [<bf1790ac>] (set_target [cpufreq_dt]) from
>>> [<c06db05c>] (__cpufreq_driver_target+0x188/0x514)
>>> [    9.972717] [<c06db05c>] (__cpufreq_driver_target) from
>>> [<c06de050>] (od_dbs_update+0x130/0x15c)
>>> [    9.981567] [<c06de050>] (od_dbs_update) from [<c06deb08>]
>>> (dbs_work_handler+0x28/0x58)
>>> [    9.989624] [<c06deb08>] (dbs_work_handler) from [<c0154ab0>]
>>> (process_one_work+0x20c/0x500)
>>> [    9.998107] [<c0154ab0>] (process_one_work) from [<c0155e8c>]
>>> (worker_thread+0x2c/0x5bc)
>>> [   10.006256] [<c0155e8c>] (worker_thread) from [<c015ab88>]
>>> (kthread+0x134/0x148)
>>> [   10.013702] [<c015ab88>] (kthread) from [<c01010e8>]
>>> (ret_from_fork+0x14/0x2c)
>>> [   10.020965] Exception stack(0xde63bfb0 to 0xde63bff8)
>>> [   10.026062] bfa0:                                     00000000
>>> 00000000 00000000 00000000
>>> [   10.034271] bfc0: 00000000 00000000 00000000 00000000 00000000
>>> 00000000 00000000 00000000
>>> [   10.042510] bfe0: 00000000 00000000 00000000 00000000 00000013 00000000
>>> [   10.049224] ---[ end trace cf0e15fa4bd57700 ]---
>>> [   10.053894] cpu cpu0: multiple regulators are not supported
>>> [   10.059509] cpufreq: __target_index: Failed to change cpu frequency: -22
>> 
>> I have the same:
>> 
>> [    4.701354] cpu cpu0: multiple regulators are not supported
>> [    4.707794] cpufreq: __target_index: Failed to change cpu frequency: -22
>> 
>> Comes from within dev_pm_opp_set_rate().
>> 
>> It appears that we also have to define opp_table->set_opp to make use
>> of _set_opp_custom(). And I am not sure which custom-opp-setter we should
>> use. Maybe ti_opp_supply_set_opp() is ok. Or not.
> 
> This appears to be set by dra7.dtsi through loading the
> "ti,omap5-opp-supply" compatible driver:
> 
> https://elixir.bootlin.com/linux/v5.3-rc8/source/arch/arm/boot/dts/dra7.dtsi#L699
> 
> Maybe we can use "ti,omap-opp-supply" here, which does not read
> additional eFuses?
> 
> See also
> 
> https://www.kernel.org/doc/Documentation/devicetree/bindings/opp/ti-omap5-opp-supply.txt
> 
> And, if I understand the code ti_opp_supply_set_opp() correctly, we may not have
> to rename cpu0-suppy to vdd-supply because that driver takes the first
> regulator as vdd and the second as vbb.
> 
> Something like
> 
> opp_supply_mpu_iva: opp_supply {
> 	compatible = "ti,omap-opp-supply";
> 	ti,absolute-max-voltage-uv = <1375000>;
> };
> 
> But that is a quite wild guess...

Well,
seems to boot without complaints and do something reasonable!

[  144.816009] regulator regulator.3: ti_abb_get_voltage_sel
[  144.821685] regulator regulator.3: ti_abb_get_voltage_sel idx=2
[  144.828521] regulator regulator.3: ti_abb_set_voltage_sel: choose sel 1
[  145.133605] regulator regulator.3: ti_abb_get_voltage_sel
[  145.139404] regulator regulator.3: ti_abb_get_voltage_sel idx=1
[  145.146881] regulator regulator.3: ti_abb_set_voltage_sel: choose sel 0
[  145.174163] regulator regulator.3: ti_abb_set_voltage_sel: choose sel 1
[  145.449493] regulator regulator.3: ti_abb_set_voltage_sel: choose sel 0
[  145.485534] regulator regulator.3: ti_abb_set_voltage_sel: choose sel 2

(I have added printk to ti_abb_get_voltage_sel/ti_abb_set_voltage_sel).

I can also see that there are transitions on the voltages (reg.8 is vdd and reg.3 is abb).

root@letux:~# cat /sys/class/regulator/regulator.8/microvolts 
1012500
root@letux:~# cat /sys/class/regulator/regulator.8/microvolts 
1325000
root@letux:~# cat /sys/class/regulator/regulator.8/microvolts 
1200000
root@letux:~#

root@letux:~# cat /sys/class/regulator/regulator.3/microvolts
1325000
root@letux:~# cat /sys/class/regulator/regulator.3/microvolts
1200000
root@letux:~# cat /sys/class/regulator/regulator.3/microvolts
1200000
root@letux:~#

What I haven't checked so far is if it toggles the ABB FBB bit.

I have pushed my current work (the last 4 commits) to

http://git.goldelico.com/?p=letux-kernel.git;a=shortlog;h=refs/heads/work-dm3730-1ghz

so that you can inspect/compare/test/check before I tidy up and add
the patches for our OPP-v2 patch set.

BR,
Nikolaus

Re: [Letux-kernel] [RFC PATCH 0/3] Enable 1GHz support on omap36xx

[Adam Ford]

On Wed, Sep 11, 2019 at 1:50 AM H. Nikolaus Schaller <hns@goldelico.com> wrote:
>
>
> > Am 11.09.2019 um 08:03 schrieb H. Nikolaus Schaller <hns@goldelico.com>:
> >
> >
> >> Am 11.09.2019 um 07:13 schrieb H. Nikolaus Schaller <hns@goldelico.com>:
> >>
> >> Hi Adam,
> >>
> >>> Am 11.09.2019 um 02:41 schrieb Adam Ford <aford173@gmail.com>:
> >>>>>>
> >>
> >>>>>> The error message looks as if we have to enable multi_regulator.
> >>
> >>>>>
> >>>>> That will enable both vdd and vbb regulators from what I can tell in the driver.
> >>>>>
> >>>>>> And that may need to rename cpu0-supply to vdd-supply (unless the
> >>>>>> names can be configured).
> >>>>>
> >>>>> That is consistent with what I found.  vdd-supply = <&vcc>; and
> >>>>> vbb-supply = <&abb_mpu_iva>;
> >>>>> I put them both under the cpu node.  Unfortunately, when I did that,
> >>>>> my board crashed.
> >>>>>
> >>>>> I am thinking it has something to do with the abb_mpu_iva driver
> >>>>> because until this point, we've always operated at 800MHz or lower
> >>>>> which all have the same behavior in abb_mpu_iva.
> >>>>>
> >>>>> With the patch you posted for the regulator, without the update to
> >>>>> cpufreq,  and with debugging enabled, I received the following in
> >>>>> dmesg:
> >>>>>
> >>>>> [    1.112518] ti_abb 483072f0.regulator-abb-mpu: Missing
> >>>>> 'efuse-address' IO resource
> >>>>> [    1.112579] ti_abb 483072f0.regulator-abb-mpu: [0]v=1012500 ABB=0
> >>>>> ef=0x0 rbb=0x0 fbb=0x0 vset=0x0
> >>>>> [    1.112609] ti_abb 483072f0.regulator-abb-mpu: [1]v=1200000 ABB=0
> >>>>> ef=0x0 rbb=0x0 fbb=0x0 vset=0x0
> >>>>> [    1.112609] ti_abb 483072f0.regulator-abb-mpu: [2]v=1325000 ABB=0
> >>>>> ef=0x0 rbb=0x0 fbb=0x0 vset=0x0
> >>>>> [    1.112640] ti_abb 483072f0.regulator-abb-mpu: [3]v=1375000 ABB=1
> >>>>> ef=0x0 rbb=0x0 fbb=0x0 vset=0x0
> >>>>> [    1.112731] ti_abb 483072f0.regulator-abb-mpu: ti_abb_init_timings:
> >>>>> Clk_rate=13000000, sr2_cnt=0x00000032
> >>>>>
> >>>>
> >>>> Using an unmodified kernel, I changed the device tree to make the abb
> >>>> regulator power the cpu instead of vcc.  After doing so, I was able to
> >>>> read the microvolt value, and it matched the processor's desired OPP
> >>>> voltage, and the debug code showed the abb regulator was attempting to
> >>>> set the various index based on the needed voltage.  I think the abb
> >>>> driver is working correctly.
> >>>>
> >>>> I think tomorrow, I will re-apply the patches and tweak it again to
> >>>> support both vdd and vbb regulators.  If it crashes again, I'll look
> >>>> more closely at the logs to see if I can determine why.  I think it's
> >>>> pretty close.  I also need to go back and find the SmartReflex stuff
> >>>> as well.
> >>>>
> >>> Well, I couldn't give it up for the night, so I thought I'd show my data dump
> >>>
> >>> [    9.807647] ------------[ cut here ]------------
> >>> [    9.812469] WARNING: CPU: 0 PID: 68 at drivers/opp/core.c:630
> >>> dev_pm_opp_set_rate+0x3cc/0x480
> >>> [    9.821044] Modules linked in: sha256_generic sha256_arm cfg80211
> >>> joydev mousedev evdev snd_soc_omap_twl4030(+) leds_gpio led_class
> >>> panel_simple pwm_omap_dmtimer gpio_keys pwm_bl cpufreq_dt omap3_isp v
> >>> ideobuf2_dma_contig videobuf2_memops videobuf2_v4l2 videobuf2_common
> >>> bq27xxx_battery_hdq v4l2_fwnode snd_soc_omap_mcbsp bq27xxx_battery
> >>> snd_soc_ti_sdma omap_wdt videodev mc omap_hdq wlcore_sdio wire cn ph
> >>> y_twl4030_usb hwmon omap2430 musb_hdrc omap_mailbox twl4030_wdt
> >>> watchdog udc_core rtc_twl snd_soc_twl4030 ohci_platform(+)
> >>> snd_soc_core snd_pcm_dmaengine ohci_hcd snd_pcm ehci_omap(+)
> >>> twl4030_pwrbutton sn
> >>> d_timer twl4030_charger snd pwm_twl_led pwm_twl ehci_hcd industrialio
> >>> soundcore twl4030_keypad matrix_keymap usbcore at24 tsc2004
> >>> tsc200x_core usb_common omap_ssi hsi omapdss omapdss_base drm
> >>> drm_panel_or
> >>> ientation_quirks cec
> >>> [    9.894470] CPU: 0 PID: 68 Comm: kworker/0:2 Not tainted
> >>> 5.3.0-rc3-00785-gfdfc7f21c6b7-dirty #5
> >>> [    9.903198] Hardware name: Generic OMAP36xx (Flattened Device Tree)
> >>> [    9.909515] Workqueue: events dbs_work_handler
> >>> [    9.914031] [<c01122d8>] (unwind_backtrace) from [<c010c8b8>]
> >>> (show_stack+0x10/0x14)
> >>> [    9.921813] [<c010c8b8>] (show_stack) from [<c089e858>]
> >>> (dump_stack+0xb4/0xd4)
> >>> [    9.929107] [<c089e858>] (dump_stack) from [<c0139eb0>]
> >>> (__warn.part.3+0xa8/0xd4)
> >>> [    9.936614] [<c0139eb0>] (__warn.part.3) from [<c013a034>]
> >>> (warn_slowpath_null+0x40/0x4c)
> >>> [    9.944854] [<c013a034>] (warn_slowpath_null) from [<c06d666c>]
> >>> (dev_pm_opp_set_rate+0x3cc/0x480)
> >>> [    9.953796] [<c06d666c>] (dev_pm_opp_set_rate) from [<bf1790ac>]
> >>> (set_target+0x30/0x58 [cpufreq_dt])
> >>> [    9.963012] [<bf1790ac>] (set_target [cpufreq_dt]) from
> >>> [<c06db05c>] (__cpufreq_driver_target+0x188/0x514)
> >>> [    9.972717] [<c06db05c>] (__cpufreq_driver_target) from
> >>> [<c06de050>] (od_dbs_update+0x130/0x15c)
> >>> [    9.981567] [<c06de050>] (od_dbs_update) from [<c06deb08>]
> >>> (dbs_work_handler+0x28/0x58)
> >>> [    9.989624] [<c06deb08>] (dbs_work_handler) from [<c0154ab0>]
> >>> (process_one_work+0x20c/0x500)
> >>> [    9.998107] [<c0154ab0>] (process_one_work) from [<c0155e8c>]
> >>> (worker_thread+0x2c/0x5bc)
> >>> [   10.006256] [<c0155e8c>] (worker_thread) from [<c015ab88>]
> >>> (kthread+0x134/0x148)
> >>> [   10.013702] [<c015ab88>] (kthread) from [<c01010e8>]
> >>> (ret_from_fork+0x14/0x2c)
> >>> [   10.020965] Exception stack(0xde63bfb0 to 0xde63bff8)
> >>> [   10.026062] bfa0:                                     00000000
> >>> 00000000 00000000 00000000
> >>> [   10.034271] bfc0: 00000000 00000000 00000000 00000000 00000000
> >>> 00000000 00000000 00000000
> >>> [   10.042510] bfe0: 00000000 00000000 00000000 00000000 00000013 00000000
> >>> [   10.049224] ---[ end trace cf0e15fa4bd57700 ]---
> >>> [   10.053894] cpu cpu0: multiple regulators are not supported
> >>> [   10.059509] cpufreq: __target_index: Failed to change cpu frequency: -22
> >>
> >> I have the same:
> >>
> >> [    4.701354] cpu cpu0: multiple regulators are not supported
> >> [    4.707794] cpufreq: __target_index: Failed to change cpu frequency: -22
> >>
> >> Comes from within dev_pm_opp_set_rate().
> >>
> >> It appears that we also have to define opp_table->set_opp to make use
> >> of _set_opp_custom(). And I am not sure which custom-opp-setter we should
> >> use. Maybe ti_opp_supply_set_opp() is ok. Or not.
> >
> > This appears to be set by dra7.dtsi through loading the
> > "ti,omap5-opp-supply" compatible driver:
> >
> > https://elixir.bootlin.com/linux/v5.3-rc8/source/arch/arm/boot/dts/dra7.dtsi#L699
> >
> > Maybe we can use "ti,omap-opp-supply" here, which does not read
> > additional eFuses?
> >
> > See also
> >
> > https://www.kernel.org/doc/Documentation/devicetree/bindings/opp/ti-omap5-opp-supply.txt
> >
> > And, if I understand the code ti_opp_supply_set_opp() correctly, we may not have
> > to rename cpu0-suppy to vdd-supply because that driver takes the first
> > regulator as vdd and the second as vbb.
> >
> > Something like
> >
> > opp_supply_mpu_iva: opp_supply {
> >       compatible = "ti,omap-opp-supply";
> >       ti,absolute-max-voltage-uv = <1375000>;
> > };
> >
> > But that is a quite wild guess...
>
> Well,
> seems to boot without complaints and do something reasonable!
>
> [  144.816009] regulator regulator.3: ti_abb_get_voltage_sel
> [  144.821685] regulator regulator.3: ti_abb_get_voltage_sel idx=2
> [  144.828521] regulator regulator.3: ti_abb_set_voltage_sel: choose sel 1
> [  145.133605] regulator regulator.3: ti_abb_get_voltage_sel
> [  145.139404] regulator regulator.3: ti_abb_get_voltage_sel idx=1
> [  145.146881] regulator regulator.3: ti_abb_set_voltage_sel: choose sel 0
> [  145.174163] regulator regulator.3: ti_abb_set_voltage_sel: choose sel 1
> [  145.449493] regulator regulator.3: ti_abb_set_voltage_sel: choose sel 0
> [  145.485534] regulator regulator.3: ti_abb_set_voltage_sel: choose sel 2
>
> (I have added printk to ti_abb_get_voltage_sel/ti_abb_set_voltage_sel).
>
> I can also see that there are transitions on the voltages (reg.8 is vdd and reg.3 is abb).

I concur.  I have good results with the added ti,omap-opp-supply entry.


>
> root@letux:~# cat /sys/class/regulator/regulator.8/microvolts
> 1012500
> root@letux:~# cat /sys/class/regulator/regulator.8/microvolts
> 1325000
> root@letux:~# cat /sys/class/regulator/regulator.8/microvolts
> 1200000
> root@letux:~#
>
> root@letux:~# cat /sys/class/regulator/regulator.3/microvolts
> 1325000
> root@letux:~# cat /sys/class/regulator/regulator.3/microvolts
> 1200000
> root@letux:~# cat /sys/class/regulator/regulator.3/microvolts
> 1200000
> root@letux:~#
>
> What I haven't checked so far is if it toggles the ABB FBB bit.
>
> I have pushed my current work (the last 4 commits) to
>
> http://git.goldelico.com/?p=letux-kernel.git;a=shortlog;h=refs/heads/work-dm3730-1ghz
>

I noticed the FIXME note on the omap36xx.dtsi file where you added the
vdd-supply reference.  For what its worth, I searched for a list of
all files that reference omap3630, then built a bunch of dtb's

make `cat dtb-list` ARCH=arm CROSS_COMPILE="ccache arm-linux-" -j8
  DTC     arch/arm/boot/dts/omap3-beagle-xm.dtb
  DTC     arch/arm/boot/dts/omap3-cm-t3730.dtb
  DTC     arch/arm/boot/dts/omap3-evm-37xx.dtb
  DTC     arch/arm/boot/dts/omap3-igep0020.dtb
  DTC     arch/arm/boot/dts/omap3-igep0020-rev-f.dtb
  DTC     arch/arm/boot/dts/omap3-igep0030.dtb
  DTC     arch/arm/boot/dts/omap3-igep0030-rev-g.dtb
  DTC     arch/arm/boot/dts/omap3-lilly-dbb056.dtb
  DTC     arch/arm/boot/dts/omap3-n950.dtb
  DTC     arch/arm/boot/dts/omap3-n9.dtb
  DTC     arch/arm/boot/dts/omap3-overo-storm-alto35.dtb
  DTC     arch/arm/boot/dts/omap3-overo-storm-chestnut43.dtb
  DTC     arch/arm/boot/dts/omap3-overo-storm-gallop43.dtb
  DTC     arch/arm/boot/dts/omap3-overo-storm-palo35.dtb
  DTC     arch/arm/boot/dts/omap3-overo-storm-palo43.dtb
  DTC     arch/arm/boot/dts/omap3-overo-storm-summit.dtb
  DTC     arch/arm/boot/dts/omap3-overo-storm-tobi.dtb
  DTC     arch/arm/boot/dts/omap3-overo-storm-tobiduo.dtb
  DTC     arch/arm/boot/dts/omap3-pandora-1ghz.dtb
  DTC     arch/arm/boot/dts/omap3-sbc-t3730.dtb
  DTC     arch/arm/boot/dts/omap3-sniper.dtb
  DTC     arch/arm/boot/dts/omap3-zoom3.dtb
  DTC     arch/arm/boot/dts/omap3-gta04a5.dtb
  DTC     arch/arm/boot/dts/omap3-gta04a5one.dtb
  DTC     arch/arm/boot/dts/omap3-gta04a3.dtb
  DTC     arch/arm/boot/dts/omap3-gta04a4.dtb

I think it's probably safe to leave the vcc-supply there, but you may
want to add a note that users who do not use twl4030 should add
something to their device tree to specify the vcc-supply.

At this point, I doubt anyone will do new designs on omap3 SoC's.

> so that you can inspect/compare/test/check before I tidy up and add
> the patches for our OPP-v2 patch set.

I think it looks good.  Maybe Tony and or TI people have some
comments, but it appears to set both regulators now.

Nice job!

adam
>
> BR,
> Nikolaus
>

Re: [Letux-kernel] [RFC PATCH 0/3] Enable 1GHz support on omap36xx

[H. Nikolaus Schaller]

Hi,

> Am 11.09.2019 um 14:43 schrieb Adam Ford <aford173@gmail.com>:
> 
>> 
>> I can also see that there are transitions on the voltages (reg.8 is vdd and reg.3 is abb).
> 
> I concur.  I have good results with the added ti,omap-opp-supply entry.

Great!

There are some subtleties for testing.

* I have added turbo-mode; to OPP6 / OPP1G
* which means they are available but not used by the ondemand govenor
* to enable them one has to echo 1 >/sys/devices/system/cpu/cpufreq/boost

Testing is also easily done through cpufreq-set -f 800m or-f 1g

Then I can see the microvolts change and also registers
PRM_LDO_ABB_CTRL 0x483072F4 bit 3:0 1=bypass 5=FBB
PRM_LDO_ABB_SETUP 0x483072F0 0x00=bypass 0x11=FBB

I have added both of this as descriptive notes to the commit messages.

What I have to check is if it behaves as expected on a dm3730 without
1GHz rating.

> I noticed the FIXME note on the omap36xx.dtsi file where you added the
> vdd-supply reference.  For what its worth, I searched for a list of
> all files that reference omap3630, then built a bunch of dtb's
> 
> make `cat dtb-list` ARCH=arm CROSS_COMPILE="ccache arm-linux-" -j8
>  DTC     arch/arm/boot/dts/omap3-beagle-xm.dtb
>  DTC     arch/arm/boot/dts/omap3-cm-t3730.dtb
>  DTC     arch/arm/boot/dts/omap3-evm-37xx.dtb
>  DTC     arch/arm/boot/dts/omap3-igep0020.dtb
>  DTC     arch/arm/boot/dts/omap3-igep0020-rev-f.dtb
>  DTC     arch/arm/boot/dts/omap3-igep0030.dtb
>  DTC     arch/arm/boot/dts/omap3-igep0030-rev-g.dtb
>  DTC     arch/arm/boot/dts/omap3-lilly-dbb056.dtb
>  DTC     arch/arm/boot/dts/omap3-n950.dtb
>  DTC     arch/arm/boot/dts/omap3-n9.dtb
>  DTC     arch/arm/boot/dts/omap3-overo-storm-alto35.dtb
>  DTC     arch/arm/boot/dts/omap3-overo-storm-chestnut43.dtb
>  DTC     arch/arm/boot/dts/omap3-overo-storm-gallop43.dtb
>  DTC     arch/arm/boot/dts/omap3-overo-storm-palo35.dtb
>  DTC     arch/arm/boot/dts/omap3-overo-storm-palo43.dtb
>  DTC     arch/arm/boot/dts/omap3-overo-storm-summit.dtb
>  DTC     arch/arm/boot/dts/omap3-overo-storm-tobi.dtb
>  DTC     arch/arm/boot/dts/omap3-overo-storm-tobiduo.dtb
>  DTC     arch/arm/boot/dts/omap3-pandora-1ghz.dtb
>  DTC     arch/arm/boot/dts/omap3-sbc-t3730.dtb
>  DTC     arch/arm/boot/dts/omap3-sniper.dtb
>  DTC     arch/arm/boot/dts/omap3-zoom3.dtb
>  DTC     arch/arm/boot/dts/omap3-gta04a5.dtb
>  DTC     arch/arm/boot/dts/omap3-gta04a5one.dtb
>  DTC     arch/arm/boot/dts/omap3-gta04a3.dtb
>  DTC     arch/arm/boot/dts/omap3-gta04a4.dtb

Quite a lot...

> I think it's probably safe to leave the vcc-supply there, but you may
> want to add a note that users who do not use twl4030 should add
> something to their device tree to specify the vcc-supply.
> 
> At this point, I doubt anyone will do new designs on omap3 SoC's.

Well, I am not sure if there are out-of-tree boards with omap3
where we could break everything. I know of at least one such
board.

Therefore I have looked where the cpu0-supply vs. vdd-supply
is decoded. It turns out to be also the ti-cpufreq driver
which we already tweak for omap3 support.

So I just have to modify ca. 10 LOC to add this "cpu0" to the
SoC description tables and process it once during probe time.

Then, it works with unmodified board.dtb
defining cpu0-supply = <&vcc> or whatever regulator.

The only question that comes up is if this change is am3517
compatible. I.e. can we still use the omap36xx_soc_data for
it as well which now expects two regulators... So you
might now see an error message that cpu0-supply and vbb-supply
are missing or not the right number of regulators is given.

We may have to add an am3517_soc_data table, but that would
be straightforward now.

> 
>> so that you can inspect/compare/test/check before I tidy up and add
>> the patches for our OPP-v2 patch set.
> 
> I think it looks good.  Maybe Tony and or TI people have some
> comments, but it appears to set both regulators now.
> 
> Nice job!

With your kind help!

Now it's time to wrap up and post a new PATCH set version for
review.

Best regards,
Nikolaus

Re: [Letux-kernel] [RFC PATCH 0/3] Enable 1GHz support on omap36xx

[Adam Ford]

On Wed, Sep 11, 2019 at 10:46 AM H. Nikolaus Schaller <hns@goldelico.com> wrote:
>
> Hi,
>
> > Am 11.09.2019 um 14:43 schrieb Adam Ford <aford173@gmail.com>:
> >
> >>
> >> I can also see that there are transitions on the voltages (reg.8 is vdd and reg.3 is abb).
> >
> > I concur.  I have good results with the added ti,omap-opp-supply entry.
>
> Great!
>
> There are some subtleties for testing.
>
> * I have added turbo-mode; to OPP6 / OPP1G
> * which means they are available but not used by the ondemand govenor
> * to enable them one has to echo 1 >/sys/devices/system/cpu/cpufreq/boost

Will that be documented somewhere? If not, can we put a comment in the
device tree so people know how to enable it?

>
> Testing is also easily done through cpufreq-set -f 800m or-f 1g
>
> Then I can see the microvolts change and also registers
> PRM_LDO_ABB_CTRL 0x483072F4 bit 3:0 1=bypass 5=FBB
> PRM_LDO_ABB_SETUP 0x483072F0 0x00=bypass 0x11=FBB
>
> I have added both of this as descriptive notes to the commit messages.
>
> What I have to check is if it behaves as expected on a dm3730 without
> 1GHz rating.

I already tested it.  From what I can see, it's behaving normally.

>
> > I noticed the FIXME note on the omap36xx.dtsi file where you added the
> > vdd-supply reference.  For what its worth, I searched for a list of
> > all files that reference omap3630, then built a bunch of dtb's
> >
> > make `cat dtb-list` ARCH=arm CROSS_COMPILE="ccache arm-linux-" -j8
> >  DTC     arch/arm/boot/dts/omap3-beagle-xm.dtb
> >  DTC     arch/arm/boot/dts/omap3-cm-t3730.dtb
> >  DTC     arch/arm/boot/dts/omap3-evm-37xx.dtb
> >  DTC     arch/arm/boot/dts/omap3-igep0020.dtb
> >  DTC     arch/arm/boot/dts/omap3-igep0020-rev-f.dtb
> >  DTC     arch/arm/boot/dts/omap3-igep0030.dtb
> >  DTC     arch/arm/boot/dts/omap3-igep0030-rev-g.dtb
> >  DTC     arch/arm/boot/dts/omap3-lilly-dbb056.dtb
> >  DTC     arch/arm/boot/dts/omap3-n950.dtb
> >  DTC     arch/arm/boot/dts/omap3-n9.dtb
> >  DTC     arch/arm/boot/dts/omap3-overo-storm-alto35.dtb
> >  DTC     arch/arm/boot/dts/omap3-overo-storm-chestnut43.dtb
> >  DTC     arch/arm/boot/dts/omap3-overo-storm-gallop43.dtb
> >  DTC     arch/arm/boot/dts/omap3-overo-storm-palo35.dtb
> >  DTC     arch/arm/boot/dts/omap3-overo-storm-palo43.dtb
> >  DTC     arch/arm/boot/dts/omap3-overo-storm-summit.dtb
> >  DTC     arch/arm/boot/dts/omap3-overo-storm-tobi.dtb
> >  DTC     arch/arm/boot/dts/omap3-overo-storm-tobiduo.dtb
> >  DTC     arch/arm/boot/dts/omap3-pandora-1ghz.dtb
> >  DTC     arch/arm/boot/dts/omap3-sbc-t3730.dtb
> >  DTC     arch/arm/boot/dts/omap3-sniper.dtb
> >  DTC     arch/arm/boot/dts/omap3-zoom3.dtb
> >  DTC     arch/arm/boot/dts/omap3-gta04a5.dtb
> >  DTC     arch/arm/boot/dts/omap3-gta04a5one.dtb
> >  DTC     arch/arm/boot/dts/omap3-gta04a3.dtb
> >  DTC     arch/arm/boot/dts/omap3-gta04a4.dtb
>
> Quite a lot...
>
> > I think it's probably safe to leave the vcc-supply there, but you may
> > want to add a note that users who do not use twl4030 should add
> > something to their device tree to specify the vcc-supply.
> >
> > At this point, I doubt anyone will do new designs on omap3 SoC's.
>
> Well, I am not sure if there are out-of-tree boards with omap3
> where we could break everything. I know of at least one such
> board.
>
> Therefore I have looked where the cpu0-supply vs. vdd-supply
> is decoded. It turns out to be also the ti-cpufreq driver
> which we already tweak for omap3 support.
>
> So I just have to modify ca. 10 LOC to add this "cpu0" to the
> SoC description tables and process it once during probe time.
>
> Then, it works with unmodified board.dtb
> defining cpu0-supply = <&vcc> or whatever regulator.
>
> The only question that comes up is if this change is am3517
> compatible. I.e. can we still use the omap36xx_soc_data for
> it as well which now expects two regulators... So you
> might now see an error message that cpu0-supply and vbb-supply
> are missing or not the right number of regulators is given.
>
> We may have to add an am3517_soc_data table, but that would
> be straightforward now.

I will run some tests on the 3517 using the 3430 table instead of the
3630. I didn't look into great detail as to what the tables do, but it
might be sufficient.
Otherwise, I can copy-paste the 3630 table and change the
multi-regulator off and test it that way if you'd prefer.

Let me know your preference, and I can do it.

>
> >
> >> so that you can inspect/compare/test/check before I tidy up and add
> >> the patches for our OPP-v2 patch set.
> >
> > I think it looks good.  Maybe Tony and or TI people have some
> > comments, but it appears to set both regulators now.
> >
> > Nice job!
>
> With your kind help!
>
I am glad to help.

> Now it's time to wrap up and post a new PATCH set version for
> review.
>
> Best regards,
> Nikolaus
>

Re: [Letux-kernel] [RFC PATCH 0/3] Enable 1GHz support on omap36xx

[H. Nikolaus Schaller]

> Am 11.09.2019 um 17:56 schrieb Adam Ford <aford173@gmail.com>:
> 
> On Wed, Sep 11, 2019 at 10:46 AM H. Nikolaus Schaller <hns@goldelico.com> wrote:
>> 
>> Hi,
>> 
>>> Am 11.09.2019 um 14:43 schrieb Adam Ford <aford173@gmail.com>:
>>> 
>>>> 
>>>> I can also see that there are transitions on the voltages (reg.8 is vdd and reg.3 is abb).
>>> 
>>> I concur.  I have good results with the added ti,omap-opp-supply entry.
>> 
>> Great!
>> 
>> There are some subtleties for testing.
>> 
>> * I have added turbo-mode; to OPP6 / OPP1G
>> * which means they are available but not used by the ondemand govenor
>> * to enable them one has to echo 1 >/sys/devices/system/cpu/cpufreq/boost
> 
> Will that be documented somewhere? If not, can we put a comment in the
> device tree so people know how to enable it?

It seems to be pretty standard on i86 systems if you google for "turbo mode".
I have added it to the commit message which adds the vbb regulator.

> 
>> 
>> Testing is also easily done through cpufreq-set -f 800m or-f 1g
>> 
>> Then I can see the microvolts change and also registers
>> PRM_LDO_ABB_CTRL 0x483072F4 bit 3:0 1=bypass 5=FBB
>> PRM_LDO_ABB_SETUP 0x483072F0 0x00=bypass 0x11=FBB
>> 
>> I have added both of this as descriptive notes to the commit messages.
>> 
>> What I have to check is if it behaves as expected on a dm3730 without
>> 1GHz rating.
> 
> I already tested it.  From what I can see, it's behaving normally.
> 
>> 
>>> I noticed the FIXME note on the omap36xx.dtsi file where you added the
>>> vdd-supply reference.  For what its worth, I searched for a list of
>>> all files that reference omap3630, then built a bunch of dtb's
>>> 
>>> make `cat dtb-list` ARCH=arm CROSS_COMPILE="ccache arm-linux-" -j8
>>> DTC     arch/arm/boot/dts/omap3-beagle-xm.dtb
>>> DTC     arch/arm/boot/dts/omap3-cm-t3730.dtb
>>> DTC     arch/arm/boot/dts/omap3-evm-37xx.dtb
>>> DTC     arch/arm/boot/dts/omap3-igep0020.dtb
>>> DTC     arch/arm/boot/dts/omap3-igep0020-rev-f.dtb
>>> DTC     arch/arm/boot/dts/omap3-igep0030.dtb
>>> DTC     arch/arm/boot/dts/omap3-igep0030-rev-g.dtb
>>> DTC     arch/arm/boot/dts/omap3-lilly-dbb056.dtb
>>> DTC     arch/arm/boot/dts/omap3-n950.dtb
>>> DTC     arch/arm/boot/dts/omap3-n9.dtb
>>> DTC     arch/arm/boot/dts/omap3-overo-storm-alto35.dtb
>>> DTC     arch/arm/boot/dts/omap3-overo-storm-chestnut43.dtb
>>> DTC     arch/arm/boot/dts/omap3-overo-storm-gallop43.dtb
>>> DTC     arch/arm/boot/dts/omap3-overo-storm-palo35.dtb
>>> DTC     arch/arm/boot/dts/omap3-overo-storm-palo43.dtb
>>> DTC     arch/arm/boot/dts/omap3-overo-storm-summit.dtb
>>> DTC     arch/arm/boot/dts/omap3-overo-storm-tobi.dtb
>>> DTC     arch/arm/boot/dts/omap3-overo-storm-tobiduo.dtb
>>> DTC     arch/arm/boot/dts/omap3-pandora-1ghz.dtb
>>> DTC     arch/arm/boot/dts/omap3-sbc-t3730.dtb
>>> DTC     arch/arm/boot/dts/omap3-sniper.dtb
>>> DTC     arch/arm/boot/dts/omap3-zoom3.dtb
>>> DTC     arch/arm/boot/dts/omap3-gta04a5.dtb
>>> DTC     arch/arm/boot/dts/omap3-gta04a5one.dtb
>>> DTC     arch/arm/boot/dts/omap3-gta04a3.dtb
>>> DTC     arch/arm/boot/dts/omap3-gta04a4.dtb
>> 
>> Quite a lot...
>> 
>>> I think it's probably safe to leave the vcc-supply there, but you may
>>> want to add a note that users who do not use twl4030 should add
>>> something to their device tree to specify the vcc-supply.
>>> 
>>> At this point, I doubt anyone will do new designs on omap3 SoC's.
>> 
>> Well, I am not sure if there are out-of-tree boards with omap3
>> where we could break everything. I know of at least one such
>> board.
>> 
>> Therefore I have looked where the cpu0-supply vs. vdd-supply
>> is decoded. It turns out to be also the ti-cpufreq driver
>> which we already tweak for omap3 support.
>> 
>> So I just have to modify ca. 10 LOC to add this "cpu0" to the
>> SoC description tables and process it once during probe time.
>> 
>> Then, it works with unmodified board.dtb
>> defining cpu0-supply = <&vcc> or whatever regulator.
>> 
>> The only question that comes up is if this change is am3517
>> compatible. I.e. can we still use the omap36xx_soc_data for
>> it as well which now expects two regulators... So you
>> might now see an error message that cpu0-supply and vbb-supply
>> are missing or not the right number of regulators is given.
>> 
>> We may have to add an am3517_soc_data table, but that would
>> be straightforward now.
> 
> I will run some tests on the 3517 using the 3430 table instead of the
> 3630. I didn't look into great detail as to what the tables do, but it
> might be sufficient.

Yes, but it reads some undocumented register...

> Otherwise, I can copy-paste the 3630 table and change the
> multi-regulator off and test it that way if you'd prefer.

I'd prefer to copy-paste the old 3630 table (which was already
tested...).

> Let me know your preference, and I can do it.
> 
>> 
>>> 
>>>> so that you can inspect/compare/test/check before I tidy up and add
>>>> the patches for our OPP-v2 patch set.
>>> 
>>> I think it looks good.  Maybe Tony and or TI people have some
>>> comments, but it appears to set both regulators now.
>>> 
>>> Nice job!
>> 
>> With your kind help!
>> 
> I am glad to help.
> 
>> Now it's time to wrap up and post a new PATCH set version for
>> review.
>> 
>> Best regards,
>> Nikolaus
>> 

BR,
Nikolaus

Re: [Letux-kernel] [RFC PATCH 0/3] Enable 1GHz support on omap36xx

[H. Nikolaus Schaller]

Hi Adam,

> Am 11.09.2019 um 18:01 schrieb H. Nikolaus Schaller <hns@goldelico.com>:
> 
>> 
>> Am 11.09.2019 um 17:56 schrieb Adam Ford <aford173@gmail.com>:
>> 
>>> There are some subtleties for testing.
>>> 
>>> * I have added turbo-mode; to OPP6 / OPP1G
>>> * which means they are available but not used by the ondemand govenor
>>> * to enable them one has to echo 1 >/sys/devices/system/cpu/cpufreq/boost
>> 
>> Will that be documented somewhere? If not, can we put a comment in the
>> device tree so people know how to enable it?
> 
> It seems to be pretty standard on i86 systems if you google for "turbo mode".
> I have added it to the commit message which adds the vbb regulator.

And, I am not sure if DT maintainers will accept comments about the
Linux /sys implementation in device tree files or bindings. Those
should be independent of Linux.

Basically the turbo-mode property is a hint to the OPP system (which
may or may not use of it).

So I think it is indeed better to have it in the commit message and
not the code.

One more thought: as long as we do not have junction temperature monitoring
we should keep it off by default... We may even remove the turbo-mode
designator if we have the 90°C limit and smart reflex working.

BR,
Nikolaus

Re: [Letux-kernel] [RFC PATCH 0/3] Enable 1GHz support on omap36xx

[Adam Ford]

On Wed, Sep 11, 2019 at 12:43 PM H. Nikolaus Schaller <hns@goldelico.com> wrote:
>
> Hi Adam,
>
> > Am 11.09.2019 um 18:01 schrieb H. Nikolaus Schaller <hns@goldelico.com>:
> >
> >>
> >> Am 11.09.2019 um 17:56 schrieb Adam Ford <aford173@gmail.com>:
> >>
> >>> There are some subtleties for testing.
> >>>
> >>> * I have added turbo-mode; to OPP6 / OPP1G
> >>> * which means they are available but not used by the ondemand govenor
> >>> * to enable them one has to echo 1 >/sys/devices/system/cpu/cpufreq/boost
> >>
> >> Will that be documented somewhere? If not, can we put a comment in the
> >> device tree so people know how to enable it?
> >
> > It seems to be pretty standard on i86 systems if you google for "turbo mode".
> > I have added it to the commit message which adds the vbb regulator.
>
> And, I am not sure if DT maintainers will accept comments about the
> Linux /sys implementation in device tree files or bindings. Those
> should be independent of Linux.

OK.
>
> Basically the turbo-mode property is a hint to the OPP system (which
> may or may not use of it).
>
> So I think it is indeed better to have it in the commit message and
> not the code.

That makes sense.

>
> One more thought: as long as we do not have junction temperature monitoring
> we should keep it off by default... We may even remove the turbo-mode
> designator if we have the 90°C limit and smart reflex working.

We're almost there!

adam
>
> BR,
> Nikolaus
>

Re: [Letux-kernel] [RFC PATCH 0/3] Enable 1GHz support on omap36xx

[Adam Ford]

On Wed, Sep 11, 2019 at 12:49 PM Adam Ford <aford173@gmail.com> wrote:
>
> On Wed, Sep 11, 2019 at 12:43 PM H. Nikolaus Schaller <hns@goldelico.com> wrote:
> >
> > Hi Adam,
> >
> > > Am 11.09.2019 um 18:01 schrieb H. Nikolaus Schaller <hns@goldelico.com>:
> > >
> > >>
> > >> Am 11.09.2019 um 17:56 schrieb Adam Ford <aford173@gmail.com>:
> > >>
> > >>> There are some subtleties for testing.
> > >>>
> > >>> * I have added turbo-mode; to OPP6 / OPP1G
> > >>> * which means they are available but not used by the ondemand govenor
> > >>> * to enable them one has to echo 1 >/sys/devices/system/cpu/cpufreq/boost
> > >>
> > >> Will that be documented somewhere? If not, can we put a comment in the
> > >> device tree so people know how to enable it?
> > >
> > > It seems to be pretty standard on i86 systems if you google for "turbo mode".
> > > I have added it to the commit message which adds the vbb regulator.
> >
> > And, I am not sure if DT maintainers will accept comments about the
> > Linux /sys implementation in device tree files or bindings. Those
> > should be independent of Linux.
>
> OK.
> >
> > Basically the turbo-mode property is a hint to the OPP system (which
> > may or may not use of it).
> >
> > So I think it is indeed better to have it in the commit message and
> > not the code.
>
> That makes sense.
>
> >
> > One more thought: as long as we do not have junction temperature monitoring
> > we should keep it off by default... We may even remove the turbo-mode
> > designator if we have the 90°C limit and smart reflex working.

I found some info on the thermal framework [1].  It seems to show that
we can put placeholders in the device tree to help facilitate this.

An excerpt from [1]  shows:
     Cooling devices provide control on power dissipation.
     There are essentially two ways to provide control on power dissipation.
     First is by means of regulating device performance, which is
known as passive cooling (DVFS).
     ... [snip stuff about cooling fans]

It also shows there are ways to link the cpu node to the cooling info.
Since it shows DVFS can be used to regulate temperature, it seems like
the hooks might already be in place

[1] - https://blog.linuxplumbersconf.org/2015/ocw//system/presentations/2613/original/thermal-framework-status-no-transitioning.pdf

The downside is that the OMAP thermal sensor is unreliable.
>
> We're almost there!
>
> adam
> >
> > BR,
> > Nikolaus
> >

Re: [Letux-kernel] [RFC PATCH 0/3] Enable 1GHz support on omap36xx

[Adam Ford]

On Thu, Sep 12, 2019 at 8:58 AM Adam Ford <aford173@gmail.com> wrote:
>
> On Wed, Sep 11, 2019 at 12:49 PM Adam Ford <aford173@gmail.com> wrote:
> >
> > On Wed, Sep 11, 2019 at 12:43 PM H. Nikolaus Schaller <hns@goldelico.com> wrote:
> > >
> > > Hi Adam,
> > >
> > > > Am 11.09.2019 um 18:01 schrieb H. Nikolaus Schaller <hns@goldelico.com>:
> > > >
> > > >>
> > > >> Am 11.09.2019 um 17:56 schrieb Adam Ford <aford173@gmail.com>:
> > > >>
> > > >>> There are some subtleties for testing.
> > > >>>
> > > >>> * I have added turbo-mode; to OPP6 / OPP1G
> > > >>> * which means they are available but not used by the ondemand govenor
> > > >>> * to enable them one has to echo 1 >/sys/devices/system/cpu/cpufreq/boost
> > > >>
> > > >> Will that be documented somewhere? If not, can we put a comment in the
> > > >> device tree so people know how to enable it?
> > > >
> > > > It seems to be pretty standard on i86 systems if you google for "turbo mode".
> > > > I have added it to the commit message which adds the vbb regulator.
> > >
> > > And, I am not sure if DT maintainers will accept comments about the
> > > Linux /sys implementation in device tree files or bindings. Those
> > > should be independent of Linux.
> >
> > OK.
> > >
> > > Basically the turbo-mode property is a hint to the OPP system (which
> > > may or may not use of it).
> > >
> > > So I think it is indeed better to have it in the commit message and
> > > not the code.
> >
> > That makes sense.
> >
> > >
> > > One more thought: as long as we do not have junction temperature monitoring
> > > we should keep it off by default... We may even remove the turbo-mode
> > > designator if we have the 90°C limit and smart reflex working.
>
> I found some info on the thermal framework [1].  It seems to show that
> we can put placeholders in the device tree to help facilitate this.
>
> An excerpt from [1]  shows:
>      Cooling devices provide control on power dissipation.
>      There are essentially two ways to provide control on power dissipation.
>      First is by means of regulating device performance, which is
> known as passive cooling (DVFS).
>      ... [snip stuff about cooling fans]
>
> It also shows there are ways to link the cpu node to the cooling info.
> Since it shows DVFS can be used to regulate temperature, it seems like
> the hooks might already be in place
>
> [1] - https://blog.linuxplumbersconf.org/2015/ocw//system/presentations/2613/original/thermal-framework-status-no-transitioning.pdf
>
> The downside is that the OMAP thermal sensor is unreliable.

I submitted an RFC for thermal throttling.  I am not sure if I did it
right, and I don't have a good way to test it.

adam


> >
> > We're almost there!
> >
> > adam
> > >
> > > BR,
> > > Nikolaus
> > >