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 >