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@xxxxxxxxx>:
> 
> On Wed, Sep 11, 2019 at 10:46 AM H. Nikolaus Schaller <hns@xxxxxxxxxxxxx> wrote:
>> 
>> Hi,
>> 
>>> Am 11.09.2019 um 14:43 schrieb Adam Ford <aford173@xxxxxxxxx>:
>>> 
>>>> 
>>>> 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