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@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?

>
> 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
>