On Tue, Sep 10, 2019 at 11:59 AM H. Nikolaus Schaller <hns@xxxxxxxxxxxxx> wrote: > > Hi Adam, > > > Am 09.09.2019 um 21:13 schrieb Adam Ford <aford173@xxxxxxxxx>: > > > > On Mon, Sep 9, 2019 at 1:11 PM H. Nikolaus Schaller <hns@xxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxx, 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 >