| Message ID | 20210601023119.22044-1-digetx@gmail.com |
|---|---|
| Headers | show |
| Series | NVIDIA Tegra memory and power management changes for 5.14 | expand |
01.06.2021 20:10, Thierry Reding пишет: > On Tue, Jun 01, 2021 at 06:51:33PM +0300, Dmitry Osipenko wrote: >> 01.06.2021 14:27, Thierry Reding пишет: >>> On Tue, Jun 01, 2021 at 05:31:05AM +0300, Dmitry Osipenko wrote: >>>> This series: >>>> >>>> 1. Adds CPU/core voltage bump before system is rebooted. >>>> 2. Adds new devm_tegra_core_dev_init_opp_table() helper and switches >>>> Tegra memory drivers to use it. >>>> 3. Adds compile-testing support to the Tegra memory drivers. >>>> 4. Adds Tegra SoC core power domain support. >>>> >>>> Changelog: >>>> >>>> v6: - Fixed another compile-test trouble reported for v5. I double checked >>>> the clk stubs this time and compiled them locally. >>> >>> Heh... I just fixed those locally on top of your v5. Let me see if I can >>> roll back the changes and apply this new set instead. >> >> Thank you! You probably saw already that Ulf Hansson suggested to remove >> the lockdep annotation for now from the core PD, I'll make a v7 with >> this small change. > > Can you perhaps post this change as a follow-up? That way I can just > merge it into the corresponding branch, which may be easier than backing > out all the changes spread over four branches and applying basically the > same thing again. Alright, I'll make a follow-up. Thank you.
On 01/06/2021 04:31, Dmitry Osipenko wrote: > Enable compile testing for all Tegra memory drivers. > > Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com> > Signed-off-by: Dmitry Osipenko <digetx@gmail.com> > --- > drivers/memory/tegra/Kconfig | 16 ++++++++++------ > 1 file changed, 10 insertions(+), 6 deletions(-) > Hi Dmitry, This fails on x86_64 and i386: https://krzk.eu/#/builders/38/builds/260 https://krzk.eu/#/builders/40/builds/261 /bin/ld: warning: orphan section `__reservedmem_of_table' from `drivers/memory/tegra/tegra210-emc-table.o' being placed in section `__reservedmem_of_table' /bin/ld: drivers/memory/tegra/mc.o: in function `tegra_mc_probe': mc.c:(.text+0x87a): undefined reference to `reset_controller_register' make[1]: *** [/home/buildbot/worker/builddir/build/Makefile:1191: vmlinux] Error 1 It's a defconfig with: scripts/config --file out/.config -e COMPILE_TEST -e OF -e SRAM -e MEMORY -e PM_DEVFREQ -e ARM_PL172_MPMC -e ATMEL_SDRAMC -e ATMEL_EBI -e BRCMSTB_DPFE -e BT1_L2_CTL -e TI_AEMIF -e TI_EMIF -e OMAP_GPMC -e TI_EMIF_SRAM -e MVEBU_DEVBUS -e FSL_CORENET_CF -e FSL_IFC -e JZ4780_NEMC -e MTK_SMI -e DA8XX_DDRCTL -e PL353_SMC -e RENESAS_RPCIF -e STM32_FMC2_EBI -e SAMSUNG_MC -e EXYNOS5422_DMC -e EXYNOS_SROM -e TEGRA_MC -e TEGRA20_EMC -e TEGRA30_EMC -e TEGRA124_EMC -e TEGRA210_EMC_TABLE -e TEGRA210_EMC Best regards, Krzysztof
On Mon, Jun 07, 2021 at 08:01:28AM +0200, Krzysztof Kozlowski wrote: > On 01/06/2021 04:31, Dmitry Osipenko wrote: > > Enable compile testing for all Tegra memory drivers. > > > > Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com> > > Signed-off-by: Dmitry Osipenko <digetx@gmail.com> > > --- > > drivers/memory/tegra/Kconfig | 16 ++++++++++------ > > 1 file changed, 10 insertions(+), 6 deletions(-) > > > > Hi Dmitry, > > This fails on x86_64 and i386: > https://krzk.eu/#/builders/38/builds/260 > https://krzk.eu/#/builders/40/builds/261 > > /bin/ld: warning: orphan section `__reservedmem_of_table' from `drivers/memory/tegra/tegra210-emc-table.o' being placed in section `__reservedmem_of_table' > /bin/ld: drivers/memory/tegra/mc.o: in function `tegra_mc_probe': > mc.c:(.text+0x87a): undefined reference to `reset_controller_register' > make[1]: *** [/home/buildbot/worker/builddir/build/Makefile:1191: vmlinux] Error 1 > > It's a defconfig with: > scripts/config --file out/.config -e COMPILE_TEST -e OF -e SRAM -e > MEMORY -e PM_DEVFREQ -e ARM_PL172_MPMC -e ATMEL_SDRAMC -e ATMEL_EBI -e > BRCMSTB_DPFE -e BT1_L2_CTL -e TI_AEMIF -e TI_EMIF -e OMAP_GPMC -e > TI_EMIF_SRAM -e MVEBU_DEVBUS -e FSL_CORENET_CF -e FSL_IFC -e JZ4780_NEMC > -e MTK_SMI -e DA8XX_DDRCTL -e PL353_SMC -e RENESAS_RPCIF -e > STM32_FMC2_EBI -e SAMSUNG_MC -e EXYNOS5422_DMC -e EXYNOS_SROM -e > TEGRA_MC -e TEGRA20_EMC -e TEGRA30_EMC -e TEGRA124_EMC -e > TEGRA210_EMC_TABLE -e TEGRA210_EMC Ugh... that's exactly one of the reasons why I dislike COMPILE_TEST... though admittedly it does point out a missing dependency here. I think we need to add && RESET_CONTROLLER to that || branch of the depends on to fix that. ARCH_TEGRA selects RESET_CONTROLLER explicitly, so the COMPILE_TEST branch needs to mirror that. Either that, or I suppose we could add the depends on RESET_CONTROLLER explicitly to TEGRA_MC, or perhaps even select it (although that could cause conflicts down the road, but should be fine right now because RESET_CONTROLLER doesn't have any other dependencies right now). Not sure what to do about that orphaned __reservedmem_of_table section. Maybe all we need to do is to select OF_RESERVED_MEM from TEGRA210_EMC_TABLE? Let me to a few test builds. Thierry
07.06.2021 16:36, Thierry Reding пишет: > On Mon, Jun 07, 2021 at 08:01:28AM +0200, Krzysztof Kozlowski wrote: >> On 01/06/2021 04:31, Dmitry Osipenko wrote: >>> Enable compile testing for all Tegra memory drivers. >>> >>> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com> >>> Signed-off-by: Dmitry Osipenko <digetx@gmail.com> >>> --- >>> drivers/memory/tegra/Kconfig | 16 ++++++++++------ >>> 1 file changed, 10 insertions(+), 6 deletions(-) >>> >> >> Hi Dmitry, >> >> This fails on x86_64 and i386: >> https://krzk.eu/#/builders/38/builds/260 >> https://krzk.eu/#/builders/40/builds/261 >> >> /bin/ld: warning: orphan section `__reservedmem_of_table' from `drivers/memory/tegra/tegra210-emc-table.o' being placed in section `__reservedmem_of_table' >> /bin/ld: drivers/memory/tegra/mc.o: in function `tegra_mc_probe': >> mc.c:(.text+0x87a): undefined reference to `reset_controller_register' >> make[1]: *** [/home/buildbot/worker/builddir/build/Makefile:1191: vmlinux] Error 1 >> >> It's a defconfig with: >> scripts/config --file out/.config -e COMPILE_TEST -e OF -e SRAM -e >> MEMORY -e PM_DEVFREQ -e ARM_PL172_MPMC -e ATMEL_SDRAMC -e ATMEL_EBI -e >> BRCMSTB_DPFE -e BT1_L2_CTL -e TI_AEMIF -e TI_EMIF -e OMAP_GPMC -e >> TI_EMIF_SRAM -e MVEBU_DEVBUS -e FSL_CORENET_CF -e FSL_IFC -e JZ4780_NEMC >> -e MTK_SMI -e DA8XX_DDRCTL -e PL353_SMC -e RENESAS_RPCIF -e >> STM32_FMC2_EBI -e SAMSUNG_MC -e EXYNOS5422_DMC -e EXYNOS_SROM -e >> TEGRA_MC -e TEGRA20_EMC -e TEGRA30_EMC -e TEGRA124_EMC -e >> TEGRA210_EMC_TABLE -e TEGRA210_EMC > > Ugh... that's exactly one of the reasons why I dislike COMPILE_TEST... > though admittedly it does point out a missing dependency here. I think > we need to add && RESET_CONTROLLER to that || branch of the depends on > to fix that. ARCH_TEGRA selects RESET_CONTROLLER explicitly, so the > COMPILE_TEST branch needs to mirror that. > > Either that, or I suppose we could add the depends on RESET_CONTROLLER > explicitly to TEGRA_MC, or perhaps even select it (although that could > cause conflicts down the road, but should be fine right now because > RESET_CONTROLLER doesn't have any other dependencies right now). The select will work. The other option is to add stubs for the reset controller API.
07.06.2021 16:36, Thierry Reding пишет: >> /bin/ld: warning: orphan section `__reservedmem_of_table' from `drivers/memory/tegra/tegra210-emc-table.o' being placed in section `__reservedmem_of_table' >> /bin/ld: drivers/memory/tegra/mc.o: in function `tegra_mc_probe': >> mc.c:(.text+0x87a): undefined reference to `reset_controller_register' >> make[1]: *** [/home/buildbot/worker/builddir/build/Makefile:1191: vmlinux] Error 1 ... > Not sure what to do about that orphaned __reservedmem_of_table section. > Maybe all we need to do is to select OF_RESERVED_MEM from > TEGRA210_EMC_TABLE? Select won't work easily, but the dependency for TEGRA210_EMC should.
On Mon, Jun 07, 2021 at 05:01:02PM +0300, Dmitry Osipenko wrote: > 07.06.2021 16:36, Thierry Reding пишет: > >> /bin/ld: warning: orphan section `__reservedmem_of_table' from `drivers/memory/tegra/tegra210-emc-table.o' being placed in section `__reservedmem_of_table' > >> /bin/ld: drivers/memory/tegra/mc.o: in function `tegra_mc_probe': > >> mc.c:(.text+0x87a): undefined reference to `reset_controller_register' > >> make[1]: *** [/home/buildbot/worker/builddir/build/Makefile:1191: vmlinux] Error 1 > ... > > > Not sure what to do about that orphaned __reservedmem_of_table section. > > Maybe all we need to do is to select OF_RESERVED_MEM from > > TEGRA210_EMC_TABLE? > > Select won't work easily, but the dependency for TEGRA210_EMC should. Select works if I also select OF_EARLY_FLATTREE. That's slightly odd because typically that's something that the platform would select, but there's precedent for doing this in drivers/clk/x86/Kconfig, so I think it'd be fine. The attached patch resolves both of the above issues for me. Krzysztof: do you want to squash that into the problematic patch or do you want me to send this as a follow-up patch for you to apply? I guess the latter since you've already sent out the PR for Will and ARM SoC? Thierry --- >8 --- diff --git a/drivers/memory/tegra/Kconfig b/drivers/memory/tegra/Kconfig index f9bae36c03a3..ae8c57b921e6 100644 --- a/drivers/memory/tegra/Kconfig +++ b/drivers/memory/tegra/Kconfig @@ -4,6 +4,7 @@ config TEGRA_MC default y depends on ARCH_TEGRA || (COMPILE_TEST && COMMON_CLK) select INTERCONNECT + select RESET_CONTROLLER help This driver supports the Memory Controller (MC) hardware found on NVIDIA Tegra SoCs. @@ -48,6 +49,8 @@ config TEGRA124_EMC config TEGRA210_EMC_TABLE bool depends on ARCH_TEGRA_210_SOC || COMPILE_TEST + select OF_EARLY_FLATTREE + select OF_RESERVED_MEM config TEGRA210_EMC tristate "NVIDIA Tegra210 External Memory Controller driver" --- >8 ---
On 07/06/2021 16:19, Thierry Reding wrote: > On Mon, Jun 07, 2021 at 05:01:02PM +0300, Dmitry Osipenko wrote: >> 07.06.2021 16:36, Thierry Reding пишет: >>>> /bin/ld: warning: orphan section `__reservedmem_of_table' from `drivers/memory/tegra/tegra210-emc-table.o' being placed in section `__reservedmem_of_table' >>>> /bin/ld: drivers/memory/tegra/mc.o: in function `tegra_mc_probe': >>>> mc.c:(.text+0x87a): undefined reference to `reset_controller_register' >>>> make[1]: *** [/home/buildbot/worker/builddir/build/Makefile:1191: vmlinux] Error 1 >> ... >> >>> Not sure what to do about that orphaned __reservedmem_of_table section. >>> Maybe all we need to do is to select OF_RESERVED_MEM from >>> TEGRA210_EMC_TABLE? >> >> Select won't work easily, but the dependency for TEGRA210_EMC should. > > Select works if I also select OF_EARLY_FLATTREE. That's slightly odd > because typically that's something that the platform would select, but > there's precedent for doing this in drivers/clk/x86/Kconfig, so I think > it'd be fine. > > The attached patch resolves both of the above issues for me. > > Krzysztof: do you want to squash that into the problematic patch or do > you want me to send this as a follow-up patch for you to apply? I guess > the latter since you've already sent out the PR for Will and ARM SoC? Follow up, please, but I am not sure about selecting reset controller.
07.06.2021 17:42, Krzysztof Kozlowski пишет: > On 07/06/2021 16:19, Thierry Reding wrote: >> On Mon, Jun 07, 2021 at 05:01:02PM +0300, Dmitry Osipenko wrote: >>> 07.06.2021 16:36, Thierry Reding пишет: >>>>> /bin/ld: warning: orphan section `__reservedmem_of_table' from `drivers/memory/tegra/tegra210-emc-table.o' being placed in section `__reservedmem_of_table' >>>>> /bin/ld: drivers/memory/tegra/mc.o: in function `tegra_mc_probe': >>>>> mc.c:(.text+0x87a): undefined reference to `reset_controller_register' >>>>> make[1]: *** [/home/buildbot/worker/builddir/build/Makefile:1191: vmlinux] Error 1 >>> ... >>> >>>> Not sure what to do about that orphaned __reservedmem_of_table section. >>>> Maybe all we need to do is to select OF_RESERVED_MEM from >>>> TEGRA210_EMC_TABLE? >>> >>> Select won't work easily, but the dependency for TEGRA210_EMC should. >> >> Select works if I also select OF_EARLY_FLATTREE. That's slightly odd >> because typically that's something that the platform would select, but >> there's precedent for doing this in drivers/clk/x86/Kconfig, so I think >> it'd be fine. >> >> The attached patch resolves both of the above issues for me. >> >> Krzysztof: do you want to squash that into the problematic patch or do >> you want me to send this as a follow-up patch for you to apply? I guess >> the latter since you've already sent out the PR for Will and ARM SoC? > > Follow up, please, but I am not sure about selecting reset controller. > From the tegra/mc.c code I see it can be optional - if "reset_ops" is > provided. Therefore I think: > 1. Reset controller should provide proper stubs. This will fix building > of mc.c when reset controller is not chosen (regardless of point #2 below). > > 2. Specific drivers should depend on it. Selecting user-visible symbols > is rather discourage because might lead to circular dependencies. Thierry, should I send the patches or you're willing to do it?
This series: 1. Adds CPU/core voltage bump before system is rebooted. 2. Adds new devm_tegra_core_dev_init_opp_table() helper and switches Tegra memory drivers to use it. 3. Adds compile-testing support to the Tegra memory drivers. 4. Adds Tegra SoC core power domain support. Changelog: v6: - Fixed another compile-test trouble reported for v5. I double checked the clk stubs this time and compiled them locally. - Moved the Kconfig properties required for the core power domain to the PMC Kconfig entry. This was suggested by Thierry Reding. v5: - Fixed compile-test warning reported for v3 which happened due to copy-paste typo around new tegra210_clk_emc_attach() stub. v4: - Fixed misplaced prototypes of tegra_pmc_core_domain_state_synced(), which I noticed only after sending out v3. This fixes building with !CONFIG_PM_SLEEP. v3: - Dropped "Detach coupled regulator before coupling count is dropped" patch that was added in v2 since it missed regulator locking and it should be more reasonable to add a new generic hook for syncing before detaching. For now it's optional to sync Tegra SoC regulators before detaching since it's not something that happens in practice, hence it's more optimal to simply drop that feature. - Added more stubs for T210 clk driver which should fix compile-testing problem reported for v2 by kernel robot. - Added COMMON_CLK for COMPILE_TEST of memory drivers since for today the problem of compile-testing of legacy platforms that use HAVE_LEGACY_CLK isn't solved, we will be able to remove it after fixing the legacy platforms. - Factored out new PMC driver state syncing feature into a separate patch "pmc: Add driver state syncing", which was requested by Ulf Hansson in a review comment to v2. - Added r-b from Ulf Hansson to the PMC binding-update patch that he gave to v2. v2: - Added more clk stubs that should fix build error reported by the kernel bot to v1 for the T210 memory driver. - Added r-b from Krzysztof Kozlowski to the memory patches. - Added back voltage restore on detaching of coupled regulators to the T20 regulator coupler which previously got missed by accident. - Added new patch: regulator: core: Detach coupled regulator before coupling count is dropped It fixes skipped voltage balancing on detaching of regulators which I happened to notice due to the recent regression of the MAX77620 driver that made driver to re-probe and detach coupled regulators. v1: - Merged previous patches into this single series. - Added ack from Rob Herring to the core domain DT binding patch. - Implemented suggestions that were given by Krzysztof Kozlowski: - Factored out soc_is_tegra() stub into standalone patch. - Updated tags of the "Fix compilation warnings on 64bit platforms" patch, added reported-by from lkp robot and removed suggested-by from Nathan Chancellor. - Switched to use use BIT() macro. - Added r-b from Krzysztof Kozlowski to "Enable compile testing for all drivers" patch. - Added r-b from Nathan Chancellor. - Dropped voltage floor/ceiling from devm_tegra_core_dev_init_opp_table() since only memory drivers now need to initialize voltage vote and they don't need floor/ceil. This was suggested by Viresh Kumar. Dmitry Osipenko (14): regulator: core: Add regulator_sync_voltage_rdev() soc/tegra: regulators: Bump voltages on system reboot soc/tegra: Add stub for soc_is_tegra() soc/tegra: Add devm_tegra_core_dev_init_opp_table() soc/tegra: fuse: Add stubs needed for compile-testing clk: tegra: Add stubs needed for compile-testing memory: tegra: Fix compilation warnings on 64bit platforms memory: tegra: Enable compile testing for all drivers memory: tegra20-emc: Use devm_tegra_core_dev_init_opp_table() memory: tegra30-emc: Use devm_tegra_core_dev_init_opp_table() dt-bindings: soc: tegra-pmc: Document core power domain soc/tegra: pmc: Add core power domain soc/tegra: pmc: Add driver state syncing soc/tegra: regulators: Support core domain state syncing .../arm/tegra/nvidia,tegra20-pmc.yaml | 35 +++++ drivers/memory/tegra/Kconfig | 16 +- drivers/memory/tegra/tegra124-emc.c | 4 +- drivers/memory/tegra/tegra20-emc.c | 48 +----- drivers/memory/tegra/tegra30-emc.c | 52 +----- drivers/regulator/core.c | 23 +++ drivers/soc/tegra/Kconfig | 2 + drivers/soc/tegra/common.c | 97 ++++++++++++ drivers/soc/tegra/pmc.c | 148 +++++++++++++++++- drivers/soc/tegra/regulators-tegra20.c | 94 ++++++++++- drivers/soc/tegra/regulators-tegra30.c | 93 ++++++++++- include/linux/clk/tegra.h | 100 +++++++++--- include/linux/regulator/driver.h | 1 + include/soc/tegra/common.h | 31 ++++ include/soc/tegra/fuse.h | 20 ++- include/soc/tegra/pmc.h | 7 + 16 files changed, 640 insertions(+), 131 deletions(-)