> 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
