Message ID | cover.1568224032.git.hns@goldelico.com (mailing list archive) |
---|---|
Headers | show |
Series | OMAP3: convert opp-v1 to opp-v2 and read speed binned / 720MHz grade bits | expand |
* H. Nikolaus Schaller <hns@goldelico.com> [190911 17:48]: > CHANGES V3: > * make omap36xx control the abb-ldo and properly switch mode > (suggested by Adam Ford <aford173@gmail.com>) > * add a note about enabling the turbo-mode OPPs Looks good to me, when applying, please provide a minimal immutable branch maybe against v5.3 or v5.4-rc1, that I can also merge in if needed for the dts changes. Thanks, Tony
Hi Tony, > Am 16.09.2019 um 18:28 schrieb Tony Lindgren <tony@atomide.com>: > > * H. Nikolaus Schaller <hns@goldelico.com> [190911 17:48]: >> CHANGES V3: >> * make omap36xx control the abb-ldo and properly switch mode >> (suggested by Adam Ford <aford173@gmail.com>) >> * add a note about enabling the turbo-mode OPPs > > Looks good to me, when applying, please provide a > minimal immutable branch maybe against v5.3 or v5.4-rc1, > that I can also merge in if needed for the dts changes. Should I resend a v4 with your Acked-By added? BR and thanks, Nikolaus
Hi Tony, > Am 17.09.2019 um 16:35 schrieb H. Nikolaus Schaller <hns@goldelico.com>: > > Hi Tony, > >> Am 16.09.2019 um 18:28 schrieb Tony Lindgren <tony@atomide.com>: >> >> * H. Nikolaus Schaller <hns@goldelico.com> [190911 17:48]: >>> CHANGES V3: >>> * make omap36xx control the abb-ldo and properly switch mode >>> (suggested by Adam Ford <aford173@gmail.com>) >>> * add a note about enabling the turbo-mode OPPs >> >> Looks good to me, when applying, please provide a >> minimal immutable branch maybe against v5.3 or v5.4-rc1, >> that I can also merge in if needed for the dts changes. > > Should I resend a v4 with your Acked-By added? > > BR and thanks, > Nikolaus > what is the procedure to get this and Adam's thermal setup into linux-next? BR and thanks, Nikolaus
On Tue, 17 Sep 2019 at 16:35, H. Nikolaus Schaller <hns@goldelico.com> wrote: > > Hi Tony, > > > Am 16.09.2019 um 18:28 schrieb Tony Lindgren <tony@atomide.com>: > > > > * H. Nikolaus Schaller <hns@goldelico.com> [190911 17:48]: > >> CHANGES V3: > >> * make omap36xx control the abb-ldo and properly switch mode > >> (suggested by Adam Ford <aford173@gmail.com>) > >> * add a note about enabling the turbo-mode OPPs > > > > Looks good to me, when applying, please provide a > > minimal immutable branch maybe against v5.3 or v5.4-rc1, > > that I can also merge in if needed for the dts changes. > > Should I resend a v4 with your Acked-By added? I will pick them up in a few days. I was waiting for rc1 to get released and am on vacation right now. Tony already provided his Acks. -- viresh
On 11-09-19, 19:47, H. Nikolaus Schaller wrote: > CHANGES V3: > * make omap36xx control the abb-ldo and properly switch mode > (suggested by Adam Ford <aford173@gmail.com>) > * add a note about enabling the turbo-mode OPPs Applied the series to cpufreq/arm tree. Also shared a branch for you Tony: cpufreq/ti/oppv2.
On Wed, Sep 11, 2019 at 12:47 PM H. Nikolaus Schaller <hns@goldelico.com> wrote: > > CHANGES V3: > * make omap36xx control the abb-ldo and properly switch mode > (suggested by Adam Ford <aford173@gmail.com>) > * add a note about enabling the turbo-mode OPPs > > PATCH V2 2019-09-07 19:46:58: > * fix ti-cpufreq to properly describe which compatible string is legacy > * add some reviewed-by and acked-by Tony Lindgren <tony@atomide.com> > * include am3517 patches by Adam Ford <aford173@gmail.com> > * review opp-suspend; and add turbo-mode; opp properties > * add a note how to disable an OPP in a board.dts file > > PATCH V1 2019-09-07 08:56:19: > * fix typo in omap3-ldp.dts > (reported by Tony Lindgren <tony@atomide.com>) > * extend commit message to describe the bit patterns needed > for opp-supported-hw > * add error check to ioremap() > (suggested by Christ van Willegen <cvwillegen@gmail.com>) > * update Documentation/devicetree/bindings/arm/omap/omap.txt > * change bulk update to use "ti,omap3430" and "ti,omap3630" > * update OPP4 of omap3430 to 1275 mV since it was not a valid > voltage for the twl4030 driver (reported by Tony Lindgren > <tony@atomide.com>) > > RFC V2 2019-09-04 10:53:43: > * merge separate patch to remove opp-v1 table from n950-n9 into > the general omap3xxx.dtsi patch > (suggested by Viresh Kumar <viresh.kumar@linaro.org>) > * add legacy compatibility to ti,omap3430 and ti,omap3630 for > the ti-cpufreq driver > * make driver and omap3xxx.dtsi patches pass checkpatch > * add bulk patch to explicitly define compatibility to ti,omap3430 > and ti,omap36xx in addition to ti,omap3 of all in-tree boards > where it was missing > > RFC V1 2019-09-02 12:55:55: > > This patch set converts the omap3 opp tables to opp-v2 format > and extends the ti-cpufreq to support omap3. > > It adds 720 MHz (omap34xx) and 1 GHz (omap36xx) OPPs but > tells the ti-cpufreq driver to disable them if the speed > binned / 720MHz grade eFuse bits indicate that the chip > is not rated for that speed. > > It has been tested (for chip variant detection, not reliability > of the high speed OPPs) on: > > * BeagleBoard C2 (omap3530 600MHz) > * BeagleBoard XM B (dm3730 800MHz) > * GTA04A4 (dm3730 800MHz) > * GTA04A5 (dm3730 1GHz) > > > Adam Ford (2): > cpufreq: ti-cpufreq: Add support for AM3517 > ARM: dts: Add OPP-V2 table for AM3517 > Nikolaus, I am getting some notices sent to me when I apply your series. It works, but I want to clean up the notice. Can you suggest what might cause this: debugfs: Directory 'cpu0-cpu0' with parent '48070000.i2c:twl@48:regulator-vdd1-VDD1' already present! It wasn't present before your series. It's not a big deal, but I'd like to quiet down the messages if I can. Thanks. adam > H. Nikolaus Schaller (6): > cpufreq: ti-cpufreq: add support for omap34xx and omap36xx > ARM: dts: omap34xx & omap36xx: replace opp-v1 tables by opp-v2 for > DTS: bindings: omap: update bindings documentation > ARM: dts: omap3: bulk convert compatible to be explicitly ti,omap3430 > or ti,omap3630 or ti,am3517 > cpufreq: ti-cpufreq: omap36xx use "cpu0","vbb" if run in > multi_regulator mode > ARM: dts: omap36xx: using OPP1G needs to control the abb_ldo > > .../devicetree/bindings/arm/omap/omap.txt | 30 +++-- > .../bindings/cpufreq/ti-cpufreq.txt | 6 +- > arch/arm/boot/dts/am3517.dtsi | 31 +++++ > arch/arm/boot/dts/am3517_mt_ventoux.dts | 2 +- > .../boot/dts/logicpd-som-lv-35xx-devkit.dts | 2 +- > .../boot/dts/logicpd-torpedo-35xx-devkit.dts | 2 +- > arch/arm/boot/dts/omap3-beagle-xm.dts | 2 +- > arch/arm/boot/dts/omap3-beagle.dts | 2 +- > arch/arm/boot/dts/omap3-cm-t3530.dts | 2 +- > arch/arm/boot/dts/omap3-cm-t3730.dts | 2 +- > arch/arm/boot/dts/omap3-devkit8000-lcd43.dts | 2 +- > arch/arm/boot/dts/omap3-devkit8000-lcd70.dts | 2 +- > arch/arm/boot/dts/omap3-devkit8000.dts | 2 +- > arch/arm/boot/dts/omap3-gta04.dtsi | 2 +- > arch/arm/boot/dts/omap3-ha-lcd.dts | 2 +- > arch/arm/boot/dts/omap3-ha.dts | 2 +- > arch/arm/boot/dts/omap3-igep0020-rev-f.dts | 2 +- > arch/arm/boot/dts/omap3-igep0020.dts | 2 +- > arch/arm/boot/dts/omap3-igep0030-rev-g.dts | 2 +- > arch/arm/boot/dts/omap3-igep0030.dts | 2 +- > arch/arm/boot/dts/omap3-ldp.dts | 2 +- > arch/arm/boot/dts/omap3-lilly-a83x.dtsi | 2 +- > arch/arm/boot/dts/omap3-lilly-dbb056.dts | 2 +- > arch/arm/boot/dts/omap3-n9.dts | 2 +- > arch/arm/boot/dts/omap3-n950-n9.dtsi | 7 -- > arch/arm/boot/dts/omap3-n950.dts | 2 +- > .../arm/boot/dts/omap3-overo-storm-alto35.dts | 2 +- > .../boot/dts/omap3-overo-storm-chestnut43.dts | 2 +- > .../boot/dts/omap3-overo-storm-gallop43.dts | 2 +- > .../arm/boot/dts/omap3-overo-storm-palo35.dts | 2 +- > .../arm/boot/dts/omap3-overo-storm-palo43.dts | 2 +- > .../arm/boot/dts/omap3-overo-storm-summit.dts | 2 +- > arch/arm/boot/dts/omap3-overo-storm-tobi.dts | 2 +- > .../boot/dts/omap3-overo-storm-tobiduo.dts | 2 +- > arch/arm/boot/dts/omap3-pandora-1ghz.dts | 2 +- > arch/arm/boot/dts/omap3-sbc-t3530.dts | 2 +- > arch/arm/boot/dts/omap3-sbc-t3730.dts | 2 +- > arch/arm/boot/dts/omap3-sniper.dts | 2 +- > arch/arm/boot/dts/omap3-thunder.dts | 2 +- > arch/arm/boot/dts/omap3-zoom3.dts | 2 +- > arch/arm/boot/dts/omap3430-sdp.dts | 2 +- > arch/arm/boot/dts/omap34xx.dtsi | 66 ++++++++-- > arch/arm/boot/dts/omap36xx.dtsi | 65 ++++++++-- > drivers/cpufreq/cpufreq-dt-platdev.c | 2 +- > drivers/cpufreq/ti-cpufreq.c | 119 +++++++++++++++++- > 45 files changed, 320 insertions(+), 80 deletions(-) > > -- > 2.19.1 >
Hi Adam, > Am 08.11.2019 um 21:08 schrieb Adam Ford <aford173@gmail.com>: > > > Nikolaus, > > I am getting some notices sent to me when I apply your series. It > works, but I want to clean up the notice. Can you suggest what might > cause this: > > debugfs: Directory 'cpu0-cpu0' with parent > '48070000.i2c:twl@48:regulator-vdd1-VDD1' already present! > > It wasn't present before your series. It's not a big deal, but I'd > like to quiet down the messages if I can. > Thanks. I have checked and can also see this message - and it should be removed. There is a debugfs node at: /sys/kernel/debug/regulator/48070000.i2c:twl@48:regulator-vdd1-VDD1/cpu0-cpu0 OMAP5 also has a similar node but no such debugfs warning: /sys/kernel/debug/regulator/smps123/cpu0-cpu0 So what I suspect is some bug in the twl4030 regulator driver which is just revealed by my patch series for the first time. Could it be that the debugfs node is created and not cleaned up by deferred probing? But this is not explicitly done in drivers/regulator/twl-regulator.c BTW: twl6030 and palmas (twl6037) have separate driver, so that mentioning twl6030 in the comments in drivers/regulator/twl-regulator.c may be wrong. It also mentions some "TW5030" which I have never heard of. To find out the call sequence I added a dump_stack to start_creating() after the error message is printed: [ 2.289947] debugfs: Directory 'cpu0-cpu0' with parent '48070000.i2c:twl@48:regulator-vdd1-VDD1' already present! [ 2.301727] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.4.0-rc6-letux+ #1329 [ 2.309112] Hardware name: Generic OMAP36xx (Flattened Device Tree) [ 2.315734] [<c0110028>] (unwind_backtrace) from [<c010b60c>] (show_stack+0x10/0x14) [ 2.323852] [<c010b60c>] (show_stack) from [<c07b9b60>] (dump_stack+0x7c/0x9c) [ 2.331420] [<c07b9b60>] (dump_stack) from [<c0373588>] (start_creating+0xa8/0x104) [ 2.339477] [<c0373588>] (start_creating) from [<c0373fb0>] (debugfs_create_dir+0xc/0xc0) [ 2.348052] [<c0373fb0>] (debugfs_create_dir) from [<c04e96e0>] (create_regulator+0xd0/0x1c8) [ 2.356994] [<c04e96e0>] (create_regulator) from [<c04ec608>] (_regulator_get+0x190/0x224) [ 2.365661] [<c04ec608>] (_regulator_get) from [<c06653d0>] (dt_cpufreq_probe+0x80/0x108) [ 2.374237] [<c06653d0>] (dt_cpufreq_probe) from [<c053e018>] (platform_drv_probe+0x48/0x98) [ 2.383087] [<c053e018>] (platform_drv_probe) from [<c053c3a8>] (really_probe+0x164/0x324) [ 2.391754] [<c053c3a8>] (really_probe) from [<c053c7b8>] (driver_probe_device+0x10c/0x154) [ 2.400512] [<c053c7b8>] (driver_probe_device) from [<c053a9f4>] (bus_for_each_drv+0x90/0xb8) [ 2.409423] [<c053a9f4>] (bus_for_each_drv) from [<c053c5f8>] (__device_attach+0x90/0x120) [ 2.418090] [<c053c5f8>] (__device_attach) from [<c053b628>] (bus_probe_device+0x28/0x80) [ 2.426666] [<c053b628>] (bus_probe_device) from [<c05393ec>] (device_add+0x2f0/0x55c) [ 2.434967] [<c05393ec>] (device_add) from [<c053decc>] (platform_device_add+0x12c/0x1b8) [ 2.443542] [<c053decc>] (platform_device_add) from [<c053e954>] (platform_device_register_full+0xec/0x13c) [ 2.453765] [<c053e954>] (platform_device_register_full) from [<c0665ff4>] (ti_cpufreq_probe+0x298/0x2fc) [ 2.463775] [<c0665ff4>] (ti_cpufreq_probe) from [<c053e018>] (platform_drv_probe+0x48/0x98) [ 2.472625] [<c053e018>] (platform_drv_probe) from [<c053c3a8>] (really_probe+0x164/0x324) [ 2.481292] [<c053c3a8>] (really_probe) from [<c053c7b8>] (driver_probe_device+0x10c/0x154) [ 2.490051] [<c053c7b8>] (driver_probe_device) from [<c053a9f4>] (bus_for_each_drv+0x90/0xb8) [ 2.498992] [<c053a9f4>] (bus_for_each_drv) from [<c053c5f8>] (__device_attach+0x90/0x120) [ 2.507629] [<c053c5f8>] (__device_attach) from [<c053b628>] (bus_probe_device+0x28/0x80) [ 2.516204] [<c053b628>] (bus_probe_device) from [<c05393ec>] (device_add+0x2f0/0x55c) [ 2.524505] [<c05393ec>] (device_add) from [<c053decc>] (platform_device_add+0x12c/0x1b8) [ 2.533081] [<c053decc>] (platform_device_add) from [<c053e954>] (platform_device_register_full+0xec/0x13c) [ 2.543304] [<c053e954>] (platform_device_register_full) from [<c0665d2c>] (ti_cpufreq_init+0x78/0xa8) [ 2.553039] [<c0665d2c>] (ti_cpufreq_init) from [<c0102ed8>] (do_one_initcall+0xb4/0x268) [ 2.561645] [<c0102ed8>] (do_one_initcall) from [<c0b00fe4>] (kernel_init_freeable+0x11c/0x1ec) [ 2.570770] [<c0b00fe4>] (kernel_init_freeable) from [<c07cf1c8>] (kernel_init+0x8/0x110) [ 2.579345] [<c07cf1c8>] (kernel_init) from [<c01010e8>] (ret_from_fork+0x14/0x2c) [ 2.587249] Exception stack(0xde0b1fb0 to 0xde0b1ff8) [ 2.592559] 1fa0: 00000000 00000000 00000000 00000000 [ 2.601135] 1fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 [ 2.609680] 1fe0: 00000000 00000000 00000000 00000000 00000013 00000000 So the problem seems to be that ti_cpufreq_probe() tries to register the regulators "vdd" and "vbb" without properly checking if they have been registered elsewhere. The second attempt to create the debugfs directory seems to come from resources_available() which thinks that it has to create the regulator (again) [around line 1935 in drivers/regulator/core.c]. Hope this helps, although I have no idea why the vdd regulator already exists at that point. BR, Nikolaus
On Sat, Nov 9, 2019 at 12:12 AM H. Nikolaus Schaller <hns@goldelico.com> wrote: > > Hi Adam, > > > Am 08.11.2019 um 21:08 schrieb Adam Ford <aford173@gmail.com>: > > > > > > Nikolaus, > > > > I am getting some notices sent to me when I apply your series. It > > works, but I want to clean up the notice. Can you suggest what might > > cause this: > > > > debugfs: Directory 'cpu0-cpu0' with parent > > '48070000.i2c:twl@48:regulator-vdd1-VDD1' already present! > > > > It wasn't present before your series. It's not a big deal, but I'd > > like to quiet down the messages if I can. > > Thanks. > > I have checked and can also see this message - and it should be removed. > > There is a debugfs node at: > > /sys/kernel/debug/regulator/48070000.i2c:twl@48:regulator-vdd1-VDD1/cpu0-cpu0 > > OMAP5 also has a similar node but no such debugfs warning: > > /sys/kernel/debug/regulator/smps123/cpu0-cpu0 > > So what I suspect is some bug in the twl4030 regulator driver which is > just revealed by my patch series for the first time. I was wondering that too. > > Could it be that the debugfs node is created and not cleaned up by deferred > probing? That would make sense to me. > > But this is not explicitly done in drivers/regulator/twl-regulator.c > > BTW: twl6030 and palmas (twl6037) have separate driver, so that mentioning > twl6030 in the comments in drivers/regulator/twl-regulator.c may be wrong. > It also mentions some "TW5030" which I have never heard of. > > To find out the call sequence I added a dump_stack to start_creating() > after the error message is printed: > > > [ 2.289947] debugfs: Directory 'cpu0-cpu0' with parent '48070000.i2c:twl@48:regulator-vdd1-VDD1' already present! > [ 2.301727] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.4.0-rc6-letux+ #1329 > [ 2.309112] Hardware name: Generic OMAP36xx (Flattened Device Tree) > [ 2.315734] [<c0110028>] (unwind_backtrace) from [<c010b60c>] (show_stack+0x10/0x14) > [ 2.323852] [<c010b60c>] (show_stack) from [<c07b9b60>] (dump_stack+0x7c/0x9c) > [ 2.331420] [<c07b9b60>] (dump_stack) from [<c0373588>] (start_creating+0xa8/0x104) > [ 2.339477] [<c0373588>] (start_creating) from [<c0373fb0>] (debugfs_create_dir+0xc/0xc0) > [ 2.348052] [<c0373fb0>] (debugfs_create_dir) from [<c04e96e0>] (create_regulator+0xd0/0x1c8) > [ 2.356994] [<c04e96e0>] (create_regulator) from [<c04ec608>] (_regulator_get+0x190/0x224) > [ 2.365661] [<c04ec608>] (_regulator_get) from [<c06653d0>] (dt_cpufreq_probe+0x80/0x108) > [ 2.374237] [<c06653d0>] (dt_cpufreq_probe) from [<c053e018>] (platform_drv_probe+0x48/0x98) > [ 2.383087] [<c053e018>] (platform_drv_probe) from [<c053c3a8>] (really_probe+0x164/0x324) > [ 2.391754] [<c053c3a8>] (really_probe) from [<c053c7b8>] (driver_probe_device+0x10c/0x154) > [ 2.400512] [<c053c7b8>] (driver_probe_device) from [<c053a9f4>] (bus_for_each_drv+0x90/0xb8) > [ 2.409423] [<c053a9f4>] (bus_for_each_drv) from [<c053c5f8>] (__device_attach+0x90/0x120) > [ 2.418090] [<c053c5f8>] (__device_attach) from [<c053b628>] (bus_probe_device+0x28/0x80) > [ 2.426666] [<c053b628>] (bus_probe_device) from [<c05393ec>] (device_add+0x2f0/0x55c) > [ 2.434967] [<c05393ec>] (device_add) from [<c053decc>] (platform_device_add+0x12c/0x1b8) > [ 2.443542] [<c053decc>] (platform_device_add) from [<c053e954>] (platform_device_register_full+0xec/0x13c) > [ 2.453765] [<c053e954>] (platform_device_register_full) from [<c0665ff4>] (ti_cpufreq_probe+0x298/0x2fc) > [ 2.463775] [<c0665ff4>] (ti_cpufreq_probe) from [<c053e018>] (platform_drv_probe+0x48/0x98) > [ 2.472625] [<c053e018>] (platform_drv_probe) from [<c053c3a8>] (really_probe+0x164/0x324) > [ 2.481292] [<c053c3a8>] (really_probe) from [<c053c7b8>] (driver_probe_device+0x10c/0x154) > [ 2.490051] [<c053c7b8>] (driver_probe_device) from [<c053a9f4>] (bus_for_each_drv+0x90/0xb8) > [ 2.498992] [<c053a9f4>] (bus_for_each_drv) from [<c053c5f8>] (__device_attach+0x90/0x120) > [ 2.507629] [<c053c5f8>] (__device_attach) from [<c053b628>] (bus_probe_device+0x28/0x80) > [ 2.516204] [<c053b628>] (bus_probe_device) from [<c05393ec>] (device_add+0x2f0/0x55c) > [ 2.524505] [<c05393ec>] (device_add) from [<c053decc>] (platform_device_add+0x12c/0x1b8) > [ 2.533081] [<c053decc>] (platform_device_add) from [<c053e954>] (platform_device_register_full+0xec/0x13c) > [ 2.543304] [<c053e954>] (platform_device_register_full) from [<c0665d2c>] (ti_cpufreq_init+0x78/0xa8) > [ 2.553039] [<c0665d2c>] (ti_cpufreq_init) from [<c0102ed8>] (do_one_initcall+0xb4/0x268) > [ 2.561645] [<c0102ed8>] (do_one_initcall) from [<c0b00fe4>] (kernel_init_freeable+0x11c/0x1ec) > [ 2.570770] [<c0b00fe4>] (kernel_init_freeable) from [<c07cf1c8>] (kernel_init+0x8/0x110) > [ 2.579345] [<c07cf1c8>] (kernel_init) from [<c01010e8>] (ret_from_fork+0x14/0x2c) > [ 2.587249] Exception stack(0xde0b1fb0 to 0xde0b1ff8) > [ 2.592559] 1fa0: 00000000 00000000 00000000 00000000 > [ 2.601135] 1fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 > [ 2.609680] 1fe0: 00000000 00000000 00000000 00000000 00000013 00000000 > > > So the problem seems to be that ti_cpufreq_probe() tries to register the regulators > "vdd" and "vbb" without properly checking if they have been registered elsewhere. > > The second attempt to create the debugfs directory seems to come from resources_available() > which thinks that it has to create the regulator (again) [around line 1935 in drivers/regulator/core.c]. > > Hope this helps, although I have no idea why the vdd regulator already exists at that point. Thank you for the detailed analysis. > > BR, > Nikolaus > > > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel