| Message ID | 20220822081424.1310926-2-wenst@chromium.org (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Headers | show |
| Series | clk: Fix CLK_OPS_PARENT_ENABLE and runtime PM | expand |
Hi everybody, Am Montag, 22. August 2022, 10:14:23 CEST schrieb Chen-Yu Tsai: > In the previous commits that added CLK_OPS_PARENT_ENABLE, support for > this flag was only added to rate change operations (rate setting and > reparent) and disabling unused subtree. It was not added to the > clock gate related operations. Any hardware driver that needs it for > these operations will either see bogus results, or worse, hang. > > This has been seen on MT8192 and MT8195, where the imp_ii2_* clk > drivers set this, but dumping debugfs clk_summary would cause it > to hang. > > Fixes: fc8726a2c021 ("clk: core: support clocks which requires parents > enable (part 2)") Fixes: a4b3518d146f ("clk: core: support clocks which > requires parents enable (part 1)") Signed-off-by: Chen-Yu Tsai > <wenst@chromium.org> > Reviewed-by: Nícolas F. R. A. Prado <nfraprado@collabora.com> > Tested-by: Nícolas F. R. A. Prado <nfraprado@collabora.com> > --- > drivers/clk/clk.c | 28 ++++++++++++++++++++++++++++ > 1 file changed, 28 insertions(+) > > diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c > index 7fc191c15507..9b365cd6d14b 100644 > --- a/drivers/clk/clk.c > +++ b/drivers/clk/clk.c > @@ -196,6 +196,9 @@ static bool clk_core_rate_is_protected(struct clk_core > *core) return core->protect_count; > } > > +static int clk_core_prepare_enable(struct clk_core *core); > +static void clk_core_disable_unprepare(struct clk_core *core); > + > static bool clk_core_is_prepared(struct clk_core *core) > { > bool ret = false; > @@ -208,7 +211,11 @@ static bool clk_core_is_prepared(struct clk_core *core) > return core->prepare_count; > > if (!clk_pm_runtime_get(core)) { > + if (core->flags & CLK_OPS_PARENT_ENABLE) > + clk_core_prepare_enable(core->parent); > ret = core->ops->is_prepared(core->hw); > + if (core->flags & CLK_OPS_PARENT_ENABLE) > + clk_core_disable_unprepare(core->parent); > clk_pm_runtime_put(core); > } > > @@ -244,7 +251,13 @@ static bool clk_core_is_enabled(struct clk_core *core) > } > } > > + if (core->flags & CLK_OPS_PARENT_ENABLE) > + clk_core_prepare_enable(core->parent); > + > ret = core->ops->is_enabled(core->hw); > + > + if (core->flags & CLK_OPS_PARENT_ENABLE) > + clk_core_disable_unprepare(core->parent); > done: > if (core->rpm_enabled) > pm_runtime_put(core->dev); > @@ -812,6 +825,9 @@ int clk_rate_exclusive_get(struct clk *clk) > } > EXPORT_SYMBOL_GPL(clk_rate_exclusive_get); > > +static int clk_core_enable_lock(struct clk_core *core); > +static void clk_core_disable_lock(struct clk_core *core); > + > static void clk_core_unprepare(struct clk_core *core) > { > lockdep_assert_held(&prepare_lock); > @@ -835,6 +851,9 @@ static void clk_core_unprepare(struct clk_core *core) > > WARN(core->enable_count > 0, "Unpreparing enabled %s\n", core- >name); > > + if (core->flags & CLK_OPS_PARENT_ENABLE) > + clk_core_enable_lock(core->parent); > + > trace_clk_unprepare(core); > > if (core->ops->unprepare) > @@ -843,6 +862,9 @@ static void clk_core_unprepare(struct clk_core *core) > clk_pm_runtime_put(core); > > trace_clk_unprepare_complete(core); > + > + if (core->flags & CLK_OPS_PARENT_ENABLE) > + clk_core_disable_lock(core->parent); > clk_core_unprepare(core->parent); > } > > @@ -891,6 +913,9 @@ static int clk_core_prepare(struct clk_core *core) > if (ret) > goto runtime_put; > > + if (core->flags & CLK_OPS_PARENT_ENABLE) > + clk_core_enable_lock(core->parent); > + > trace_clk_prepare(core); > > if (core->ops->prepare) > @@ -898,6 +923,9 @@ static int clk_core_prepare(struct clk_core *core) > > trace_clk_prepare_complete(core); > > + if (core->flags & CLK_OPS_PARENT_ENABLE) > + clk_core_disable_lock(core->parent); > + > if (ret) > goto unprepare; > } Unfortunately this completely locks up my i.MX8M Plus based board during early boot. I'm currently running on next-20220826 using arch/arm64/boot/dts/freescale/ imx8mp-tqma8mpql-mba8mpxl.dts Reverting this patch gets my board booting again. dmesg until hard lockup below. Best regards, Alexander [ 0.000000] Booting Linux on physical CPU 0x0000000000 [0x410fd034] [ 0.000000] Linux version 6.0.0-rc2-next-20220826 (steina@steina-w) (aarch64-v8a-linux-gnu-gcc (OSELAS.Toolchain-2020.08.0 10-2020 0822) 10.2.1 20200822, GNU ld (GNU Binutils) 2.35) #603 SMP PREEMPT Fri Aug 26 14:25:05 CEST 2022 [ 0.000000] Machine model: TQ-Systems i.MX8MPlus TQMa8MPxL on MBa8MPxL [ 0.000000] earlycon: ec_imx6q0 at MMIO 0x0000000030a60000 (options '115200') [ 0.000000] printk: bootconsole [ec_imx6q0] enabled [ 0.000000] efi: UEFI not found. [ 0.000000] Reserved memory: created CMA memory pool at 0x000000005a400000, size 896 MiB [ 0.000000] OF: reserved mem: initialized node linux,cma, compatible id shared-dma-pool [ 0.000000] NUMA: No NUMA configuration found [ 0.000000] NUMA: Faking a node at [mem 0x0000000040000000-0x00000000bfffffff] [ 0.000000] NUMA: NODE_DATA [mem 0xbfbd6b00-0xbfbd8fff] [ 0.000000] Zone ranges: [ 0.000000] DMA [mem 0x0000000040000000-0x00000000bfffffff] [ 0.000000] DMA32 empty [ 0.000000] Normal empty [ 0.000000] Movable zone start for each node [ 0.000000] Early memory node ranges [ 0.000000] node 0: [mem 0x0000000040000000-0x00000000923fffff] [ 0.000000] node 0: [mem 0x0000000092400000-0x00000000943fffff] [ 0.000000] node 0: [mem 0x0000000094400000-0x00000000bfffffff] [ 0.000000] Initmem setup node 0 [mem 0x0000000040000000-0x00000000bfffffff] [ 0.000000] psci: probing for conduit method from DT. [ 0.000000] psci: PSCIv1.1 detected in firmware. [ 0.000000] psci: Using standard PSCI v0.2 function IDs [ 0.000000] psci: MIGRATE_INFO_TYPE not supported. [ 0.000000] psci: SMC Calling Convention v1.1 [ 0.000000] percpu: Embedded 19 pages/cpu s38376 r8192 d31256 u77824 [ 0.000000] Detected VIPT I-cache on CPU0 [ 0.000000] CPU features: detected: GIC system register CPU interface [ 0.000000] CPU features: detected: ARM erratum 845719 [ 0.000000] Fallback order for Node 0: 0 [ 0.000000] Built 1 zonelists, mobility grouping on. Total pages: 516096 [ 0.000000] Policy zone: DMA [ 0.000000] Kernel command line: root=/dev/nfs rw nfsroot=192.168.0.101:/ srv/tftp/imx8_mainline,v3,tcp ip=192.168.0.100:192.168.0. 101::::eth0:off console=ttymxc3,115200 earlycon=ec_imx6q,0x30A60000,115200 [ 0.000000] Dentry cache hash table entries: 262144 (order: 9, 2097152 bytes, linear) [ 0.000000] Inode-cache hash table entries: 131072 (order: 8, 1048576 bytes, linear) [ 0.000000] mem auto-init: stack:off, heap alloc:off, heap free:off [ 0.000000] Memory: 1077444K/2097152K available (14144K kernel code, 2206K rwdata, 6596K rodata, 5376K init, 519K bss, 102204K res erved, 917504K cma-reserved) [ 0.000000] SLUB: HWalign=64, Order=0-3, MinObjects=0, CPUs=4, Nodes=1 [ 0.000000] rcu: Preemptible hierarchical RCU implementation. [ 0.000000] rcu: RCU event tracing is enabled. [ 0.000000] rcu: RCU restricting CPUs from NR_CPUS=256 to nr_cpu_ids=4. [ 0.000000] Trampoline variant of Tasks RCU enabled. [ 0.000000] Tracing variant of Tasks RCU enabled. [ 0.000000] rcu: RCU calculated value of scheduler-enlistment delay is 25 jiffies. [ 0.000000] rcu: Adjusting geometry for rcu_fanout_leaf=16, nr_cpu_ids=4 [ 0.000000] NR_IRQS: 64, nr_irqs: 64, preallocated irqs: 0 [ 0.000000] GICv3: GIC: Using split EOI/Deactivate mode [ 0.000000] GICv3: 160 SPIs implemented [ 0.000000] GICv3: 0 Extended SPIs implemented [ 0.000000] Root IRQ handler: gic_handle_irq [ 0.000000] GICv3: GICv3 features: 16 PPIs [ 0.000000] GICv3: CPU0: found redistributor 0 region 0:0x0000000038880000 [ 0.000000] ITS: No ITS available, not enabling LPIs [ 0.000000] rcu: srcu_init: Setting srcu_struct sizes based on contention. [ 0.000000] arch_timer: cp15 timer(s) running at 8.00MHz (phys). [ 0.000000] clocksource: arch_sys_counter: mask: 0xffffffffffffff max_cycles: 0x1d854df40, max_idle_ns: 440795202120 ns [ 0.000000] sched_clock: 56 bits at 8MHz, resolution 125ns, wraps every 2199023255500ns [ 0.008368] Console: colour dummy device 80x25 [ 0.012575] Calibrating delay loop (skipped), value calculated using timer frequency.. 16.00 BogoMIPS (lpj=32000) [ 0.022844] pid_max: default: 32768 minimum: 301 [ 0.027538] LSM: Security Framework initializing [ 0.032209] Mount-cache hash table entries: 4096 (order: 3, 32768 bytes, linear) [ 0.039560] Mountpoint-cache hash table entries: 4096 (order: 3, 32768 bytes, linear) [ 0.048701] cblist_init_generic: Setting adjustable number of callback queues. [ 0.054720] cblist_init_generic: Setting shift to 2 and lim to 1. [ 0.060880] cblist_init_generic: Setting shift to 2 and lim to 1. [ 0.067086] rcu: Hierarchical SRCU implementation. [ 0.071761] rcu: Max phase no-delay instances is 1000. [ 0.078123] EFI services will not be available. [ 0.081907] smp: Bringing up secondary CPUs ... [ 0.086553] Detected VIPT I-cache on CPU1 [ 0.086639] GICv3: CPU1: found redistributor 1 region 0:0x00000000388a0000 [ 0.086675] CPU1: Booted secondary processor 0x0000000001 [0x410fd034] [ 0.087141] Detected VIPT I-cache on CPU2 [ 0.087209] GICv3: CPU2: found redistributor 2 region 0:0x00000000388c0000 [ 0.087229] CPU2: Booted secondary processor 0x0000000002 [0x410fd034] [ 0.087664] Detected VIPT I-cache on CPU3 [ 0.087735] GICv3: CPU3: found redistributor 3 region 0:0x00000000388e0000 [ 0.087754] CPU3: Booted secondary processor 0x0000000003 [0x410fd034] [ 0.087819] smp: Brought up 1 node, 4 CPUs [ 0.142729] SMP: Total of 4 processors activated. [ 0.147444] CPU features: detected: 32-bit EL0 Support [ 0.152623] CPU features: detected: CRC32 instructions [ 0.157983] CPU: All CPU(s) started at EL2 [ 0.161908] alternatives: patching kernel code [ 0.167362] devtmpfs: initialized [ 0.175779] clocksource: jiffies: mask: 0xffffffff max_cycles: 0xffffffff, max_idle_ns: 7645041785100000 ns [ 0.182756] futex hash table entries: 1024 (order: 4, 65536 bytes, linear) [ 0.213470] pinctrl core: initialized pinctrl subsystem [ 0.217757] DMI not present or invalid. [ 0.220335] NET: Registered PF_NETLINK/PF_ROUTE protocol family [ 0.226544] DMA: preallocated 256 KiB GFP_KERNEL pool for atomic allocations [ 0.232875] DMA: preallocated 256 KiB GFP_KERNEL|GFP_DMA pool for atomic allocations [ 0.240685] DMA: preallocated 256 KiB GFP_KERNEL|GFP_DMA32 pool for atomic allocations [ 0.248550] audit: initializing netlink subsys (disabled) [ 0.254096] audit: type=2000 audit(0.180:1): state=initialized audit_enabled=0 res=1 [ 0.254516] thermal_sys: Registered thermal governor 'bang_bang' [ 0.261730] thermal_sys: Registered thermal governor 'step_wise' [ 0.267761] thermal_sys: Registered thermal governor 'power_allocator' [ 0.273829] cpuidle: using governor menu [ 0.284458] hw-breakpoint: found 6 breakpoint and 4 watchpoint registers. [ 0.291197] ASID allocator initialised with 65536 entries [ 0.297274] Serial: AMBA PL011 UART driver [ 0.307172] imx8mp-pinctrl 30330000.pinctrl: initialized IMX pinctrl driver [ 0.323077] KASLR disabled due to lack of seed [ 0.333413] HugeTLB: registered 1.00 GiB page size, pre-allocated 0 pages [ 0.337396] HugeTLB: 16380 KiB vmemmap can be freed for a 1.00 GiB page [ 0.344066] HugeTLB: registered 32.0 MiB page size, pre-allocated 0 pages [ 0.350860] HugeTLB: 508 KiB vmemmap can be freed for a 32.0 MiB page [ 0.357335] HugeTLB: registered 2.00 MiB page size, pre-allocated 0 pages [ 0.364157] HugeTLB: 28 KiB vmemmap can be freed for a 2.00 MiB page [ 0.370546] HugeTLB: registered 64.0 KiB page size, pre-allocated 0 pages [ 0.377372] HugeTLB: 0 KiB vmemmap can be freed for a 64.0 KiB page [ 0.384224] cryptd: max_cpu_qlen set to 1000 [ 0.390062] iommu: Default domain type: Translated [ 0.392886] iommu: DMA domain TLB invalidation policy: strict mode [ 0.399407] SCSI subsystem initialized [ 0.403223] usbcore: registered new interface driver usbfs [ 0.408468] usbcore: registered new interface driver hub [ 0.413793] usbcore: registered new device driver usb [ 0.419379] mc: Linux media interface: v0.10 [ 0.423158] videodev: Linux video capture interface: v2.00 [ 0.428668] pps_core: LinuxPPS API ver. 1 registered [ 0.433635] pps_core: Software ver. 5.3.6 - Copyright 2005-2007 Rodolfo Giometti <giometti@linux.it> [ 0.442834] PTP clock support registered [ 0.446880] EDAC MC: Ver: 3.0.0 [ 0.450601] FPGA manager framework [ 0.453404] Advanced Linux Sound Architecture Driver Initialized. [ 0.460221] vgaarb: loaded [ 0.462422] clocksource: Switched to clocksource arch_sys_counter [ 0.468465] VFS: Disk quotas dquot_6.6.0 [ 0.472258] VFS: Dquot-cache hash table entries: 512 (order 0, 4096 bytes) [ 0.485904] NET: Registered PF_INET protocol family [ 0.488099] IP idents hash table entries: 32768 (order: 6, 262144 bytes, linear) [ 0.496889] tcp_listen_portaddr_hash hash table entries: 1024 (order: 2, 16384 bytes, linear) [ 0.504019] Table-perturb hash table entries: 65536 (order: 6, 262144 bytes, linear) [ 0.511764] TCP established hash table entries: 16384 (order: 5, 131072 bytes, linear) [ 0.519819] TCP bind hash table entries: 16384 (order: 7, 524288 bytes, linear) [ 0.527471] TCP: Hash tables configured (established 16384 bind 16384) [ 0.533730] UDP hash table entries: 1024 (order: 3, 32768 bytes, linear) [ 0.540415] UDP-Lite hash table entries: 1024 (order: 3, 32768 bytes, linear) [ 0.547696] NET: Registered PF_UNIX/PF_LOCAL protocol family [ 0.553577] RPC: Registered named UNIX socket transport module. [ 0.559190] RPC: Registered udp transport module. [ 0.563903] RPC: Registered tcp transport module. [ 0.568627] RPC: Registered tcp NFSv4.1 backchannel transport module. [ 0.575107] PCI: CLS 0 bytes, default 64 [ 0.579612] hw perfevents: enabled with armv8_cortex_a53 PMU driver, 7 counters available [ 0.588770] Initialise system trusted keyrings [ 0.591888] workingset: timestamp_bits=42 max_order=19 bucket_order=0 [ 0.605043] squashfs: version 4.0 (2009/01/31) Phillip Lougher [ 0.608686] NFS: Registering the id_resolver key type [ 0.613155] Key type id_resolver registered [ 0.617328] Key type id_legacy registered [ 0.621432] nfs4filelayout_init: NFSv4 File Layout Driver Registering... [ 0.628089] nfs4flexfilelayout_init: NFSv4 Flexfile Layout Driver Registering... [ 0.671152] Key type asymmetric registered [ 0.672397] Asymmetric key parser 'x509' registered [ 0.677336] Block layer SCSI generic (bsg) driver version 0.4 loaded (major 243) [ 0.684742] io scheduler mq-deadline registered [ 0.689290] io scheduler kyber registered
Hi, On Fri, Aug 26, 2022 at 8:28 PM Alexander Stein <alexander.stein@ew.tq-group.com> wrote: > > Hi everybody, > > Am Montag, 22. August 2022, 10:14:23 CEST schrieb Chen-Yu Tsai: > > In the previous commits that added CLK_OPS_PARENT_ENABLE, support for > > this flag was only added to rate change operations (rate setting and > > reparent) and disabling unused subtree. It was not added to the > > clock gate related operations. Any hardware driver that needs it for > > these operations will either see bogus results, or worse, hang. > > > > This has been seen on MT8192 and MT8195, where the imp_ii2_* clk > > drivers set this, but dumping debugfs clk_summary would cause it > > to hang. > > > > Fixes: fc8726a2c021 ("clk: core: support clocks which requires parents > > enable (part 2)") Fixes: a4b3518d146f ("clk: core: support clocks which > > requires parents enable (part 1)") Signed-off-by: Chen-Yu Tsai > > <wenst@chromium.org> > > Reviewed-by: Nícolas F. R. A. Prado <nfraprado@collabora.com> > > Tested-by: Nícolas F. R. A. Prado <nfraprado@collabora.com> > > --- > > drivers/clk/clk.c | 28 ++++++++++++++++++++++++++++ > > 1 file changed, 28 insertions(+) > > > > diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c > > index 7fc191c15507..9b365cd6d14b 100644 > > --- a/drivers/clk/clk.c > > +++ b/drivers/clk/clk.c > > @@ -196,6 +196,9 @@ static bool clk_core_rate_is_protected(struct clk_core > > *core) return core->protect_count; > > } > > > > +static int clk_core_prepare_enable(struct clk_core *core); > > +static void clk_core_disable_unprepare(struct clk_core *core); > > + > > static bool clk_core_is_prepared(struct clk_core *core) > > { > > bool ret = false; > > @@ -208,7 +211,11 @@ static bool clk_core_is_prepared(struct clk_core *core) > > return core->prepare_count; > > > > if (!clk_pm_runtime_get(core)) { > > + if (core->flags & CLK_OPS_PARENT_ENABLE) > > + clk_core_prepare_enable(core->parent); > > ret = core->ops->is_prepared(core->hw); > > + if (core->flags & CLK_OPS_PARENT_ENABLE) > > + clk_core_disable_unprepare(core->parent); > > clk_pm_runtime_put(core); > > } > > > > @@ -244,7 +251,13 @@ static bool clk_core_is_enabled(struct clk_core *core) > > } > > } > > > > + if (core->flags & CLK_OPS_PARENT_ENABLE) > > + clk_core_prepare_enable(core->parent); > > + > > ret = core->ops->is_enabled(core->hw); > > + > > + if (core->flags & CLK_OPS_PARENT_ENABLE) > > + clk_core_disable_unprepare(core->parent); > > done: > > if (core->rpm_enabled) > > pm_runtime_put(core->dev); > > @@ -812,6 +825,9 @@ int clk_rate_exclusive_get(struct clk *clk) > > } > > EXPORT_SYMBOL_GPL(clk_rate_exclusive_get); > > > > +static int clk_core_enable_lock(struct clk_core *core); > > +static void clk_core_disable_lock(struct clk_core *core); > > + > > static void clk_core_unprepare(struct clk_core *core) > > { > > lockdep_assert_held(&prepare_lock); > > @@ -835,6 +851,9 @@ static void clk_core_unprepare(struct clk_core *core) > > > > WARN(core->enable_count > 0, "Unpreparing enabled %s\n", core- > >name); > > > > + if (core->flags & CLK_OPS_PARENT_ENABLE) > > + clk_core_enable_lock(core->parent); > > + > > trace_clk_unprepare(core); > > > > if (core->ops->unprepare) > > @@ -843,6 +862,9 @@ static void clk_core_unprepare(struct clk_core *core) > > clk_pm_runtime_put(core); > > > > trace_clk_unprepare_complete(core); > > + > > + if (core->flags & CLK_OPS_PARENT_ENABLE) > > + clk_core_disable_lock(core->parent); > > clk_core_unprepare(core->parent); > > } > > > > @@ -891,6 +913,9 @@ static int clk_core_prepare(struct clk_core *core) > > if (ret) > > goto runtime_put; > > > > + if (core->flags & CLK_OPS_PARENT_ENABLE) > > + clk_core_enable_lock(core->parent); > > + > > trace_clk_prepare(core); > > > > if (core->ops->prepare) > > @@ -898,6 +923,9 @@ static int clk_core_prepare(struct clk_core *core) > > > > trace_clk_prepare_complete(core); > > > > + if (core->flags & CLK_OPS_PARENT_ENABLE) > > + clk_core_disable_lock(core->parent); > > + > > if (ret) > > goto unprepare; > > } > > > Unfortunately this completely locks up my i.MX8M Plus based board during early > boot. > I'm currently running on next-20220826 using arch/arm64/boot/dts/freescale/ > imx8mp-tqma8mpql-mba8mpxl.dts > Reverting this patch gets my board booting again. dmesg until hard lockup > below. The standard logs don't have anything to go on. Could you add some printk calls to the clk core around the areas this patch touchs? That would help. Could you also provide a dump of /sys/kernel/debug/clk/clk_summary? That would help to understand the clock tree. Thanks ChenYu
Hi, Am Montag, 29. August 2022, 11:22:16 CEST schrieb Chen-Yu Tsai: > Hi, > > On Fri, Aug 26, 2022 at 8:28 PM Alexander Stein > > <alexander.stein@ew.tq-group.com> wrote: > > Hi everybody, > > > > Am Montag, 22. August 2022, 10:14:23 CEST schrieb Chen-Yu Tsai: > > > In the previous commits that added CLK_OPS_PARENT_ENABLE, support for > > > this flag was only added to rate change operations (rate setting and > > > reparent) and disabling unused subtree. It was not added to the > > > clock gate related operations. Any hardware driver that needs it for > > > these operations will either see bogus results, or worse, hang. > > > > > > This has been seen on MT8192 and MT8195, where the imp_ii2_* clk > > > drivers set this, but dumping debugfs clk_summary would cause it > > > to hang. > > > > > > Fixes: fc8726a2c021 ("clk: core: support clocks which requires parents > > > enable (part 2)") Fixes: a4b3518d146f ("clk: core: support clocks which > > > requires parents enable (part 1)") Signed-off-by: Chen-Yu Tsai > > > <wenst@chromium.org> > > > Reviewed-by: Nícolas F. R. A. Prado <nfraprado@collabora.com> > > > Tested-by: Nícolas F. R. A. Prado <nfraprado@collabora.com> > > > --- > > > > > > drivers/clk/clk.c | 28 ++++++++++++++++++++++++++++ > > > 1 file changed, 28 insertions(+) > > > > > > diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c > > > index 7fc191c15507..9b365cd6d14b 100644 > > > --- a/drivers/clk/clk.c > > > +++ b/drivers/clk/clk.c > > > @@ -196,6 +196,9 @@ static bool clk_core_rate_is_protected(struct > > > clk_core > > > *core) return core->protect_count; > > > > > > } > > > > > > +static int clk_core_prepare_enable(struct clk_core *core); > > > +static void clk_core_disable_unprepare(struct clk_core *core); > > > + > > > > > > static bool clk_core_is_prepared(struct clk_core *core) > > > { > > > > > > bool ret = false; > > > > > > @@ -208,7 +211,11 @@ static bool clk_core_is_prepared(struct clk_core > > > *core) return core->prepare_count; > > > > > > if (!clk_pm_runtime_get(core)) { > > > > > > + if (core->flags & CLK_OPS_PARENT_ENABLE) > > > + clk_core_prepare_enable(core->parent); > > > > > > ret = core->ops->is_prepared(core->hw); > > > > > > + if (core->flags & CLK_OPS_PARENT_ENABLE) > > > + clk_core_disable_unprepare(core->parent); > > > > > > clk_pm_runtime_put(core); > > > > > > } > > > > > > @@ -244,7 +251,13 @@ static bool clk_core_is_enabled(struct clk_core > > > *core) > > > > > > } > > > > > > } > > > > > > + if (core->flags & CLK_OPS_PARENT_ENABLE) > > > + clk_core_prepare_enable(core->parent); > > > + > > > > > > ret = core->ops->is_enabled(core->hw); > > > > > > + > > > + if (core->flags & CLK_OPS_PARENT_ENABLE) > > > + clk_core_disable_unprepare(core->parent); > > > > > > done: > > > if (core->rpm_enabled) > > > > > > pm_runtime_put(core->dev); > > > > > > @@ -812,6 +825,9 @@ int clk_rate_exclusive_get(struct clk *clk) > > > > > > } > > > EXPORT_SYMBOL_GPL(clk_rate_exclusive_get); > > > > > > +static int clk_core_enable_lock(struct clk_core *core); > > > +static void clk_core_disable_lock(struct clk_core *core); > > > + > > > > > > static void clk_core_unprepare(struct clk_core *core) > > > { > > > > > > lockdep_assert_held(&prepare_lock); > > > > > > @@ -835,6 +851,9 @@ static void clk_core_unprepare(struct clk_core > > > *core) > > > > > > WARN(core->enable_count > 0, "Unpreparing enabled %s\n", core- > > > > > >name); > > > > > > + if (core->flags & CLK_OPS_PARENT_ENABLE) > > > + clk_core_enable_lock(core->parent); > > > + > > > > > > trace_clk_unprepare(core); > > > > > > if (core->ops->unprepare) > > > > > > @@ -843,6 +862,9 @@ static void clk_core_unprepare(struct clk_core > > > *core) > > > > > > clk_pm_runtime_put(core); > > > > > > trace_clk_unprepare_complete(core); > > > > > > + > > > + if (core->flags & CLK_OPS_PARENT_ENABLE) > > > + clk_core_disable_lock(core->parent); > > > > > > clk_core_unprepare(core->parent); > > > > > > } > > > > > > @@ -891,6 +913,9 @@ static int clk_core_prepare(struct clk_core *core) > > > > > > if (ret) > > > > > > goto runtime_put; > > > > > > + if (core->flags & CLK_OPS_PARENT_ENABLE) > > > + clk_core_enable_lock(core->parent); > > > + > > > > > > trace_clk_prepare(core); > > > > > > if (core->ops->prepare) > > > > > > @@ -898,6 +923,9 @@ static int clk_core_prepare(struct clk_core *core) > > > > > > trace_clk_prepare_complete(core); > > > > > > + if (core->flags & CLK_OPS_PARENT_ENABLE) > > > + clk_core_disable_lock(core->parent); > > > + > > > > > > if (ret) > > > > > > goto unprepare; > > > > > > } > > > > Unfortunately this completely locks up my i.MX8M Plus based board during > > early boot. > > I'm currently running on next-20220826 using > > arch/arm64/boot/dts/freescale/ > > imx8mp-tqma8mpql-mba8mpxl.dts > > Reverting this patch gets my board booting again. dmesg until hard lockup > > below. > > The standard logs don't have anything to go on. Could you add some printk > calls to the clk core around the areas this patch touchs? That would help. > > Could you also provide a dump of /sys/kernel/debug/clk/clk_summary? That > would help to understand the clock tree. Sure, These are the last kernel log lines before hard lockup: [ 0.686357] io scheduler mq-deadline registered [ 0.690907] io scheduler kyber registered [ 0.699275] clk_core_prepare: clk: main_axi CLK_OPS_PARENT_ENABLE main_axi is also the only debug output up to this point. This is the used patch for debugging: ---8<--- --- a/drivers/clk/clk.c +++ b/drivers/clk/clk.c @@ -211,8 +211,10 @@ static bool clk_core_is_prepared(struct clk_core *core) return core->prepare_count; if (!clk_pm_runtime_get(core)) { - if (core->flags & CLK_OPS_PARENT_ENABLE) + if (core->flags & CLK_OPS_PARENT_ENABLE) { + pr_info("%s: clk: %s CLK_OPS_PARENT_ENABLE\n", __func__, core->name); clk_core_prepare_enable(core->parent); + } ret = core->ops->is_prepared(core->hw); if (core->flags & CLK_OPS_PARENT_ENABLE) clk_core_disable_unprepare(core->parent); @@ -251,8 +253,10 @@ static bool clk_core_is_enabled(struct clk_core *core) } } - if (core->flags & CLK_OPS_PARENT_ENABLE) + if (core->flags & CLK_OPS_PARENT_ENABLE) { + pr_info("%s: clk: %s CLK_OPS_PARENT_ENABLE\n", __func__, core- >name); clk_core_prepare_enable(core->parent); + } ret = core->ops->is_enabled(core->hw); @@ -851,8 +855,10 @@ static void clk_core_unprepare(struct clk_core *core) WARN(core->enable_count > 0, "Unpreparing enabled %s\n", core->name); - if (core->flags & CLK_OPS_PARENT_ENABLE) + if (core->flags & CLK_OPS_PARENT_ENABLE) { + pr_info("%s: clk: %s CLK_OPS_PARENT_ENABLE\n", __func__, core- >name); clk_core_enable_lock(core->parent); + } trace_clk_unprepare(core); @@ -912,8 +918,10 @@ static int clk_core_prepare(struct clk_core *core) if (ret) goto runtime_put; - if (core->flags & CLK_OPS_PARENT_ENABLE) + if (core->flags & CLK_OPS_PARENT_ENABLE) { + pr_info("%s: clk: %s CLK_OPS_PARENT_ENABLE\n", __func__, core->name); clk_core_enable_lock(core->parent); + } trace_clk_prepare(core); ---8<--- And here is the output of clk_summary with 35b0fac808b9 reverted: enable prepare protect duty hardware clock count count count rate accuracy phase cycle enable ------------------------------------------------------------------------------------------------------- pcf85063-clkout 0 0 0 0 0 0 50000 Y dummy 0 0 0 0 0 0 50000 Y clk_ext4 0 0 0 133000000 0 0 50000 Y clk_ext3 0 0 0 133000000 0 0 50000 Y clk_ext2 0 0 0 133000000 0 0 50000 Y clk_ext1 0 0 0 133000000 0 0 50000 Y osc_24m 8 12 0 24000000 0 0 50000 Y sai7 0 0 0 24000000 0 0 50000 N vpu_vc8000e 0 0 0 24000000 0 0 50000 N vpu_vc8ke_root_clk 0 0 0 24000000 0 0 50000 N pdm 0 0 0 24000000 0 0 50000 N media_mipi_test_byte 0 0 0 24000000 0 0 50000 N mem_repair 1 1 0 24000000 0 0 50000 Y media_ldb 0 0 0 24000000 0 0 50000 N media_cam2_pix 0 0 0 24000000 0 0 50000 N media_cam2_pix_root_clk 0 0 0 24000000 0 0 50000 N media_disp1_pix 0 0 0 24000000 0 0 50000 N media_disp1_pix_root_clk 0 0 0 24000000 0 0 50000 N media_cam1_pix 0 0 0 24000000 0 0 50000 N media_cam1_pix_root_clk 0 0 0 24000000 0 0 50000 N hdmi_ref_266m 0 0 0 24000000 0 0 50000 N hdmi_24m 0 0 0 24000000 0 0 50000 N hdmi_fdcc_tst 0 0 0 24000000 0 0 50000 N ipp_do_clko2 0 0 0 24000000 0 0 50000 N ipp_do_clko1 0 0 0 24000000 0 0 50000 N wdog 1 1 0 24000000 0 0 50000 Y wdog3_root_clk 0 0 0 24000000 0 0 50000 N wdog2_root_clk 0 0 0 24000000 0 0 50000 N wdog1_root_clk 1 1 0 24000000 0 0 50000 Y gpt6 0 0 0 24000000 0 0 50000 N gpt6_root_clk 0 0 0 24000000 0 0 50000 N gpt5 0 0 0 24000000 0 0 50000 N gpt5_root_clk 0 0 0 24000000 0 0 50000 N gpt4 0 0 0 24000000 0 0 50000 N gpt4_root_clk 0 0 0 24000000 0 0 50000 N gpt3 0 0 0 24000000 0 0 50000 N gpt3_root_clk 0 0 0 24000000 0 0 50000 N gpt2 0 0 0 24000000 0 0 50000 N gpt2_root_clk 0 0 0 24000000 0 0 50000 N gpt1 0 0 0 24000000 0 0 50000 N gpt1_root_clk 0 0 0 24000000 0 0 50000 N pwm4 0 0 0 24000000 0 0 50000 N pwm4_root_clk 0 0 0 24000000 0 0 50000 N pwm3 0 0 0 24000000 0 0 50000 N pwm3_root_clk 0 0 0 24000000 0 0 50000 N pwm2 0 0 0 24000000 0 0 50000 N pwm2_root_clk 0 0 0 24000000 0 0 50000 N pwm1 0 0 0 24000000 0 0 50000 N pwm1_root_clk 0 0 0 24000000 0 0 50000 N usb_core_ref 0 0 0 24000000 0 0 50000 N uart4 1 1 0 24000000 0 0 50000 Y uart4_root_clk 4 4 0 24000000 0 0 50000 Y i2c4 0 1 0 24000000 0 0 50000 N i2c4_root_clk 0 1 0 24000000 0 0 50000 N i2c3 0 0 0 24000000 0 0 50000 N i2c3_root_clk 0 0 0 24000000 0 0 50000 N i2c2 0 1 0 24000000 0 0 50000 N i2c2_root_clk 0 1 0 24000000 0 0 50000 N i2c1 0 1 0 24000000 0 0 50000 N i2c1_root_clk 0 1 0 24000000 0 0 50000 N sai6 0 0 0 24000000 0 0 50000 N sai5 0 0 0 24000000 0 0 50000 N sai4 0 0 0 24000000 0 0 50000 N sai3 0 0 0 24000000 0 0 50000 N sai2 0 0 0 24000000 0 0 50000 N sai1 0 0 0 24000000 0 0 50000 N i2c6 0 1 0 24000000 0 0 50000 N i2c6_root_clk 0 1 0 24000000 0 0 50000 N i2c5 0 0 0 24000000 0 0 50000 N i2c5_root_clk 0 0 0 24000000 0 0 50000 N pcie_aux 0 0 0 24000000 0 0 50000 N pcie_root_clk 0 0 0 24000000 0 0 50000 N vpu_g2 0 0 0 24000000 0 0 50000 N vpu_g2_root_clk 0 0 0 24000000 0 0 50000 N vpu_g1 0 0 0 24000000 0 0 50000 N vpu_g1_root_clk 0 0 0 24000000 0 0 50000 N media_disp2_pix 0 0 0 24000000 0 0 50000 N media_disp2_pix_root_clk 0 0 0 24000000 0 0 50000 N mipi_dsi_esc_rx 0 0 0 24000000 0 0 50000 N ml_ahb 0 0 0 24000000 0 0 50000 N ml_axi 0 0 0 24000000 0 0 50000 N hdmi_axi 0 0 0 24000000 0 0 50000 N hdmi_root_clk 0 0 0 24000000 0 0 50000 N vpu_bus 0 0 0 24000000 0 0 50000 N vpu_root_clk 0 0 0 24000000 0 0 50000 N media_isp 0 0 0 24000000 0 0 50000 N media_isp_root_clk 0 0 0 24000000 0 0 50000 N ml_core 0 0 0 24000000 0 0 50000 N npu_root_clk 0 0 0 24000000 0 0 50000 N sys_pll3_ref_sel 0 0 0 24000000 0 0 50000 Y sys_pll3 0 0 0 600000000 0 0 50000 Y sys_pll3_bypass 0 0 0 600000000 0 0 50000 Y sys_pll3_out 0 0 0 600000000 0 0 50000 N sys_pll2_ref_sel 1 1 0 24000000 0 0 50000 Y sys_pll2 1 1 0 1000000000 0 0 50000 Y sys_pll2_bypass 1 1 0 1000000000 0 0 50000 Y sys_pll2_out 5 5 0 1000000000 0 0 50000 Y sys_pll2_1000m 1 1 0 1000000000 0 0 50000 Y noc 1 1 0 1000000000 0 0 50000 Y media_axi 0 0 0 500000000 0 0 50000 N media_axi_root_clk 0 0 0 500000000 0 0 50000 N sys_pll2_500m 1 1 0 500000000 0 0 50000 Y hsio_axi 0 0 0 500000000 0 0 50000 N usb_root_clk 0 0 0 500000000 0 0 50000 N gic 1 1 0 500000000 0 0 50000 Y nand 0 0 0 500000000 0 0 50000 N nand_root_clk 0 0 0 500000000 0 0 50000 N sys_pll2_333m 0 0 0 333333333 0 0 50000 Y sys_pll2_250m 0 0 0 250000000 0 0 50000 Y sys_pll2_200m 0 0 0 200000000 0 0 50000 Y ecspi3 0 0 0 50000000 0 0 50000 N ecspi3_root_clk 0 0 0 50000000 0 0 50000 N ecspi2 0 0 0 50000000 0 0 50000 N ecspi2_root_clk 0 0 0 50000000 0 0 50000 N ecspi1 0 0 0 50000000 0 0 50000 N ecspi1_root_clk 0 0 0 50000000 0 0 50000 N m7_core 0 0 0 200000000 0 0 50000 N sys_pll2_166m 0 0 0 166666666 0 0 50000 Y sys_pll2_125m 2 2 0 125000000 0 0 50000 Y enet_ref 1 1 0 125000000 0 0 50000 Y enet_qos 1 1 0 125000000 0 0 50000 Y hdmi_apb 0 0 0 125000000 0 0 50000 N sys_pll2_100m 2 2 0 100000000 0 0 50000 Y enet_timer 1 1 0 100000000 0 0 50000 Y enet_qos_timer 1 1 0 100000000 0 0 50000 Y sys_pll2_50m 1 1 0 50000000 0 0 50000 Y enet_phy_ref 1 1 0 50000000 0 0 50000 Y sys_pll1_ref_sel 1 1 0 24000000 0 0 50000 Y sys_pll1 1 1 0 800000000 0 0 50000 Y sys_pll1_bypass 1 1 0 800000000 0 0 50000 Y sys_pll1_out 5 5 0 800000000 0 0 50000 Y sys_pll1_800m 3 3 0 800000000 0 0 50000 Y gpu2d_core 0 0 0 800000000 0 0 50000 N gpu2d_root_clk 0 0 0 800000000 0 0 50000 N gpu3d_shader_core 0 0 0 800000000 0 0 50000 N gpu3d_core 0 0 0 800000000 0 0 50000 N gpu3d_root_clk 0 0 0 800000000 0 0 50000 N gpu_ahb 0 0 0 400000000 0 0 50000 N gpu_axi 0 0 0 800000000 0 0 50000 N gpu_root_clk 0 0 0 800000000 0 0 50000 N audio_axi 0 0 0 800000000 0 0 50000 N audio_ahb 0 0 0 400000000 0 0 50000 N audio_root_clk 0 0 0 400000000 0 0 50000 N noc_io 1 1 0 800000000 0 0 50000 Y arm_a53_div 0 0 0 800000000 0 0 50000 N dram_apb 1 1 0 160000000 0 0 50000 Y media_apb 0 0 0 200000000 0 0 50000 N media_apb_root_clk 0 0 0 200000000 0 0 50000 N main_axi 1 1 0 400000000 0 0 50000 Y sys_pll1_400m 0 0 0 400000000 0 0 50000 Y usdhc3 0 0 0 400000000 0 0 50000 N usdhc3_root_clk 0 0 0 400000000 0 0 50000 N usdhc2 0 0 0 400000000 0 0 50000 N usdhc2_root_clk 0 0 0 400000000 0 0 50000 N usdhc1 0 0 0 200000000 0 0 50000 N usdhc1_root_clk 0 0 0 200000000 0 0 50000 N sys_pll1_266m 2 2 0 266666666 0 0 50000 Y nand_usdhc_bus 1 1 0 266666666 0 0 50000 Y nand_usdhc_rawnand_clk 0 0 0 266666666 0 0 50000 N enet_axi 2 2 0 266666666 0 0 50000 Y sim_enet_root_clk 2 2 0 266666666 0 0 50000 Y enet_qos_root_clk 1 1 0 266666666 0 0 50000 Y enet1_root_clk 1 1 0 266666666 0 0 50000 Y sys_pll1_200m 0 0 0 200000000 0 0 50000 Y sys_pll1_160m 0 0 0 160000000 0 0 50000 Y sys_pll1_133m 1 1 0 133333333 0 0 50000 Y ahb_root 9 4 0 133333333 0 0 50000 Y ipg_root 11 11 0 66666667 0 0 50000 Y tsensor_root_clk 1 1 0 66666667 0 0 50000 Y hsio_root_clk 0 0 0 66666667 0 0 50000 N sdma1_root_clk 6 1 0 66666667 0 0 50000 Y qos_enet_root_clk 1 1 0 66666667 0 0 50000 Y qos_root_clk 0 0 0 66666667 0 0 50000 N ocotp_root_clk 0 0 0 66666667 0 0 50000 N mu_root_clk 0 0 0 66666667 0 0 50000 N gpio5_root_clk 1 1 0 66666667 0 0 50000 Y gpio4_root_clk 1 1 0 66666667 0 0 50000 Y gpio3_root_clk 1 1 0 66666667 0 0 50000 Y gpio2_root_clk 1 1 0 66666667 0 0 50000 Y gpio1_root_clk 1 1 0 66666667 0 0 50000 Y sys_pll1_100m 1 1 0 100000000 0 0 50000 Y usb_phy_ref 0 0 0 100000000 0 0 50000 N usb_phy_root_clk 0 0 0 100000000 0 0 50000 N qspi 1 1 0 100000000 0 0 50000 Y qspi_root_clk 2 2 0 100000000 0 0 50000 Y dram_alt 0 0 0 100000000 0 0 50000 N dram_alt_root 0 0 0 25000000 0 0 50000 Y sys_pll1_80m 0 0 0 80000000 0 0 50000 Y uart2 0 0 0 80000000 0 0 50000 N uart2_root_clk 0 0 0 80000000 0 0 50000 N uart3 0 0 0 80000000 0 0 50000 N uart3_root_clk 0 0 0 80000000 0 0 50000 N uart1 0 0 0 80000000 0 0 50000 N uart1_root_clk 0 0 0 80000000 0 0 50000 N sys_pll1_40m 2 2 0 40000000 0 0 50000 Y can2 1 1 0 40000000 0 0 50000 Y can2_root_clk 1 1 0 40000000 0 0 50000 Y can1 1 1 0 40000000 0 0 50000 Y can1_root_clk 1 1 0 40000000 0 0 50000 Y wrclk 0 0 0 40000000 0 0 50000 N arm_pll_ref_sel 1 1 0 24000000 0 0 50000 Y arm_pll 1 1 0 1200000000 0 0 50000 Y arm_pll_bypass 1 1 0 1200000000 0 0 50000 Y arm_pll_out 1 1 0 1200000000 0 0 50000 Y arm_a53_core 1 1 0 1200000000 0 0 50000 Y arm 1 1 0 1200000000 0 0 50000 Y vpu_pll_ref_sel 0 0 0 24000000 0 0 50000 Y vpu_pll 0 0 0 800000000 0 0 50000 Y vpu_pll_bypass 0 0 0 800000000 0 0 50000 Y vpu_pll_out 0 0 0 800000000 0 0 50000 N gpu_pll_ref_sel 0 0 0 24000000 0 0 50000 Y gpu_pll 0 0 0 1000000000 0 0 50000 Y gpu_pll_bypass 0 0 0 1000000000 0 0 50000 Y gpu_pll_out 0 0 0 1000000000 0 0 50000 N dram_pll_ref_sel 1 1 0 24000000 0 0 50000 Y dram_pll 1 1 0 1000000000 0 0 50000 Y dram_pll_bypass 1 1 0 1000000000 0 0 50000 Y dram_pll_out 1 1 0 1000000000 0 0 50000 Y dram_core_clk 2 2 0 1000000000 0 0 50000 Y dram1_root_clk 1 1 0 1000000000 0 0 50000 Y video_pll1_ref_sel 0 0 0 24000000 0 0 50000 Y video_pll1 0 0 0 594000000 0 0 50000 Y video_pll1_bypass 0 0 0 594000000 0 0 50000 Y video_pll1_out 0 0 0 594000000 0 0 50000 N media_mipi_phy1_ref 0 0 0 27000000 0 0 50000 N media_mipi_phy1_ref_root 0 0 0 27000000 0 0 50000 N audio_pll2_ref_sel 0 0 0 24000000 0 0 50000 Y audio_pll2 0 0 0 361267196 0 0 50000 Y audio_pll2_bypass 0 0 0 361267196 0 0 50000 Y audio_pll2_out 0 0 0 361267196 0 0 50000 N audio_pll1_ref_sel 0 0 0 24000000 0 0 50000 Y audio_pll1 0 0 0 393215995 0 0 50000 Y audio_pll1_bypass 0 0 0 393215995 0 0 50000 Y audio_pll1_out 0 0 0 393215995 0 0 50000 N clkout2_sel 0 0 0 393215995 0 0 50000 Y clkout2_div 0 0 0 393215995 0 0 50000 Y clkout2 0 0 0 393215995 0 0 50000 N clkout1_sel 0 0 0 393215995 0 0 50000 Y clkout1_div 0 0 0 393215995 0 0 50000 Y clkout1 0 0 0 393215995 0 0 50000 N osc_32k 0 0 0 32768 0 0 50000 Y Thanks for the work and best regards, Alexander
On Tue, Aug 30, 2022 at 8:37 PM Alexander Stein <alexander.stein@ew.tq-group.com> wrote: > > Hi, > > Am Montag, 29. August 2022, 11:22:16 CEST schrieb Chen-Yu Tsai: > > Hi, > > > > On Fri, Aug 26, 2022 at 8:28 PM Alexander Stein > > > > <alexander.stein@ew.tq-group.com> wrote: > > > Hi everybody, > > > > > > Am Montag, 22. August 2022, 10:14:23 CEST schrieb Chen-Yu Tsai: > > > > In the previous commits that added CLK_OPS_PARENT_ENABLE, support for > > > > this flag was only added to rate change operations (rate setting and > > > > reparent) and disabling unused subtree. It was not added to the > > > > clock gate related operations. Any hardware driver that needs it for > > > > these operations will either see bogus results, or worse, hang. > > > > > > > > This has been seen on MT8192 and MT8195, where the imp_ii2_* clk > > > > drivers set this, but dumping debugfs clk_summary would cause it > > > > to hang. > > > > > > > > Fixes: fc8726a2c021 ("clk: core: support clocks which requires parents > > > > enable (part 2)") Fixes: a4b3518d146f ("clk: core: support clocks which > > > > requires parents enable (part 1)") Signed-off-by: Chen-Yu Tsai > > > > <wenst@chromium.org> > > > > Reviewed-by: Nícolas F. R. A. Prado <nfraprado@collabora.com> > > > > Tested-by: Nícolas F. R. A. Prado <nfraprado@collabora.com> > > > > --- > > > > > > > > drivers/clk/clk.c | 28 ++++++++++++++++++++++++++++ > > > > 1 file changed, 28 insertions(+) > > > > > > > > diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c > > > > index 7fc191c15507..9b365cd6d14b 100644 > > > > --- a/drivers/clk/clk.c > > > > +++ b/drivers/clk/clk.c > > > > @@ -196,6 +196,9 @@ static bool clk_core_rate_is_protected(struct > > > > clk_core > > > > *core) return core->protect_count; > > > > > > > > } > > > > > > > > +static int clk_core_prepare_enable(struct clk_core *core); > > > > +static void clk_core_disable_unprepare(struct clk_core *core); > > > > + > > > > > > > > static bool clk_core_is_prepared(struct clk_core *core) > > > > { > > > > > > > > bool ret = false; > > > > > > > > @@ -208,7 +211,11 @@ static bool clk_core_is_prepared(struct clk_core > > > > *core) return core->prepare_count; > > > > > > > > if (!clk_pm_runtime_get(core)) { > > > > > > > > + if (core->flags & CLK_OPS_PARENT_ENABLE) > > > > + clk_core_prepare_enable(core->parent); > > > > > > > > ret = core->ops->is_prepared(core->hw); > > > > > > > > + if (core->flags & CLK_OPS_PARENT_ENABLE) > > > > + clk_core_disable_unprepare(core->parent); > > > > > > > > clk_pm_runtime_put(core); > > > > > > > > } > > > > > > > > @@ -244,7 +251,13 @@ static bool clk_core_is_enabled(struct clk_core > > > > *core) > > > > > > > > } > > > > > > > > } > > > > > > > > + if (core->flags & CLK_OPS_PARENT_ENABLE) > > > > + clk_core_prepare_enable(core->parent); > > > > + > > > > > > > > ret = core->ops->is_enabled(core->hw); > > > > > > > > + > > > > + if (core->flags & CLK_OPS_PARENT_ENABLE) > > > > + clk_core_disable_unprepare(core->parent); > > > > > > > > done: > > > > if (core->rpm_enabled) > > > > > > > > pm_runtime_put(core->dev); > > > > > > > > @@ -812,6 +825,9 @@ int clk_rate_exclusive_get(struct clk *clk) > > > > > > > > } > > > > EXPORT_SYMBOL_GPL(clk_rate_exclusive_get); > > > > > > > > +static int clk_core_enable_lock(struct clk_core *core); > > > > +static void clk_core_disable_lock(struct clk_core *core); > > > > + > > > > > > > > static void clk_core_unprepare(struct clk_core *core) > > > > { > > > > > > > > lockdep_assert_held(&prepare_lock); > > > > > > > > @@ -835,6 +851,9 @@ static void clk_core_unprepare(struct clk_core > > > > *core) > > > > > > > > WARN(core->enable_count > 0, "Unpreparing enabled %s\n", core- > > > > > > > >name); > > > > > > > > + if (core->flags & CLK_OPS_PARENT_ENABLE) > > > > + clk_core_enable_lock(core->parent); > > > > + > > > > > > > > trace_clk_unprepare(core); > > > > > > > > if (core->ops->unprepare) > > > > > > > > @@ -843,6 +862,9 @@ static void clk_core_unprepare(struct clk_core > > > > *core) > > > > > > > > clk_pm_runtime_put(core); > > > > > > > > trace_clk_unprepare_complete(core); > > > > > > > > + > > > > + if (core->flags & CLK_OPS_PARENT_ENABLE) > > > > + clk_core_disable_lock(core->parent); > > > > > > > > clk_core_unprepare(core->parent); > > > > > > > > } > > > > > > > > @@ -891,6 +913,9 @@ static int clk_core_prepare(struct clk_core *core) > > > > > > > > if (ret) > > > > > > > > goto runtime_put; > > > > > > > > + if (core->flags & CLK_OPS_PARENT_ENABLE) > > > > + clk_core_enable_lock(core->parent); > > > > + > > > > > > > > trace_clk_prepare(core); > > > > > > > > if (core->ops->prepare) > > > > > > > > @@ -898,6 +923,9 @@ static int clk_core_prepare(struct clk_core *core) > > > > > > > > trace_clk_prepare_complete(core); > > > > > > > > + if (core->flags & CLK_OPS_PARENT_ENABLE) > > > > + clk_core_disable_lock(core->parent); > > > > + > > > > > > > > if (ret) > > > > > > > > goto unprepare; > > > > > > > > } > > > > > > Unfortunately this completely locks up my i.MX8M Plus based board during > > > early boot. > > > I'm currently running on next-20220826 using > > > arch/arm64/boot/dts/freescale/ > > > imx8mp-tqma8mpql-mba8mpxl.dts > > > Reverting this patch gets my board booting again. dmesg until hard lockup > > > below. > > > > The standard logs don't have anything to go on. Could you add some printk > > calls to the clk core around the areas this patch touchs? That would help. > > > > Could you also provide a dump of /sys/kernel/debug/clk/clk_summary? That > > would help to understand the clock tree. > > Sure, > > These are the last kernel log lines before hard lockup: > [ 0.686357] io scheduler mq-deadline registered > [ 0.690907] io scheduler kyber registered > [ 0.699275] clk_core_prepare: clk: main_axi CLK_OPS_PARENT_ENABLE > > main_axi is also the only debug output up to this point. This is the used > patch for debugging: > ---8<--- > --- a/drivers/clk/clk.c > +++ b/drivers/clk/clk.c > @@ -211,8 +211,10 @@ static bool clk_core_is_prepared(struct clk_core *core) > return core->prepare_count; > > if (!clk_pm_runtime_get(core)) { > - if (core->flags & CLK_OPS_PARENT_ENABLE) > + if (core->flags & CLK_OPS_PARENT_ENABLE) { > + pr_info("%s: clk: %s CLK_OPS_PARENT_ENABLE\n", > __func__, core->name); > clk_core_prepare_enable(core->parent); > + } > ret = core->ops->is_prepared(core->hw); > if (core->flags & CLK_OPS_PARENT_ENABLE) > clk_core_disable_unprepare(core->parent); > @@ -251,8 +253,10 @@ static bool clk_core_is_enabled(struct clk_core *core) > } > } > > - if (core->flags & CLK_OPS_PARENT_ENABLE) > + if (core->flags & CLK_OPS_PARENT_ENABLE) { > + pr_info("%s: clk: %s CLK_OPS_PARENT_ENABLE\n", __func__, core- > >name); > clk_core_prepare_enable(core->parent); > + } > > ret = core->ops->is_enabled(core->hw); > > @@ -851,8 +855,10 @@ static void clk_core_unprepare(struct clk_core *core) > > WARN(core->enable_count > 0, "Unpreparing enabled %s\n", core->name); > > - if (core->flags & CLK_OPS_PARENT_ENABLE) > + if (core->flags & CLK_OPS_PARENT_ENABLE) { > + pr_info("%s: clk: %s CLK_OPS_PARENT_ENABLE\n", __func__, core- > >name); > clk_core_enable_lock(core->parent); > + } > > trace_clk_unprepare(core); > > @@ -912,8 +918,10 @@ static int clk_core_prepare(struct clk_core *core) > if (ret) > goto runtime_put; > > - if (core->flags & CLK_OPS_PARENT_ENABLE) > + if (core->flags & CLK_OPS_PARENT_ENABLE) { > + pr_info("%s: clk: %s CLK_OPS_PARENT_ENABLE\n", > __func__, core->name); > clk_core_enable_lock(core->parent); > + } > > trace_clk_prepare(core); > > > ---8<--- Thanks. So the part of the clock tree that's problematic is this: osc_24m (fixed) sys_pll1_ref_sel (mux) sys_pll1 (imx pll14xx) sys_pll1_bypass (mux) sys_pll1_out (gate) sys_pll1_800m (fixed factor) main_axi (composite CLK_IS_CRITICAL) Would it be possible for you to produce a stack dump as well? And also enable lock debugging? This likely still won't catch anything if it's a spinlock deadlock though. Thanks ChenYu
Am Dienstag, 30. August 2022, 15:40:53 CEST schrieb Chen-Yu Tsai: > On Tue, Aug 30, 2022 at 8:37 PM Alexander Stein > > <alexander.stein@ew.tq-group.com> wrote: > > Hi, > > > > Am Montag, 29. August 2022, 11:22:16 CEST schrieb Chen-Yu Tsai: > > > Hi, > > > > > > On Fri, Aug 26, 2022 at 8:28 PM Alexander Stein > > > > > > <alexander.stein@ew.tq-group.com> wrote: > > > > Hi everybody, > > > > > > > > Am Montag, 22. August 2022, 10:14:23 CEST schrieb Chen-Yu Tsai: > > > > > In the previous commits that added CLK_OPS_PARENT_ENABLE, support > > > > > for > > > > > this flag was only added to rate change operations (rate setting and > > > > > reparent) and disabling unused subtree. It was not added to the > > > > > clock gate related operations. Any hardware driver that needs it for > > > > > these operations will either see bogus results, or worse, hang. > > > > > > > > > > This has been seen on MT8192 and MT8195, where the imp_ii2_* clk > > > > > drivers set this, but dumping debugfs clk_summary would cause it > > > > > to hang. > > > > > > > > > > Fixes: fc8726a2c021 ("clk: core: support clocks which requires > > > > > parents > > > > > enable (part 2)") Fixes: a4b3518d146f ("clk: core: support clocks > > > > > which > > > > > requires parents enable (part 1)") Signed-off-by: Chen-Yu Tsai > > > > > <wenst@chromium.org> > > > > > Reviewed-by: Nícolas F. R. A. Prado <nfraprado@collabora.com> > > > > > Tested-by: Nícolas F. R. A. Prado <nfraprado@collabora.com> > > > > > --- > > > > > > > > > > drivers/clk/clk.c | 28 ++++++++++++++++++++++++++++ > > > > > 1 file changed, 28 insertions(+) > > > > > > > > > > diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c > > > > > index 7fc191c15507..9b365cd6d14b 100644 > > > > > --- a/drivers/clk/clk.c > > > > > +++ b/drivers/clk/clk.c > > > > > @@ -196,6 +196,9 @@ static bool clk_core_rate_is_protected(struct > > > > > clk_core > > > > > *core) return core->protect_count; > > > > > > > > > > } > > > > > > > > > > +static int clk_core_prepare_enable(struct clk_core *core); > > > > > +static void clk_core_disable_unprepare(struct clk_core *core); > > > > > + > > > > > > > > > > static bool clk_core_is_prepared(struct clk_core *core) > > > > > { > > > > > > > > > > bool ret = false; > > > > > > > > > > @@ -208,7 +211,11 @@ static bool clk_core_is_prepared(struct > > > > > clk_core > > > > > *core) return core->prepare_count; > > > > > > > > > > if (!clk_pm_runtime_get(core)) { > > > > > > > > > > + if (core->flags & CLK_OPS_PARENT_ENABLE) > > > > > + clk_core_prepare_enable(core->parent); > > > > > > > > > > ret = core->ops->is_prepared(core->hw); > > > > > > > > > > + if (core->flags & CLK_OPS_PARENT_ENABLE) > > > > > + clk_core_disable_unprepare(core->parent); > > > > > > > > > > clk_pm_runtime_put(core); > > > > > > > > > > } > > > > > > > > > > @@ -244,7 +251,13 @@ static bool clk_core_is_enabled(struct clk_core > > > > > *core) > > > > > > > > > > } > > > > > > > > > > } > > > > > > > > > > + if (core->flags & CLK_OPS_PARENT_ENABLE) > > > > > + clk_core_prepare_enable(core->parent); > > > > > + > > > > > > > > > > ret = core->ops->is_enabled(core->hw); > > > > > > > > > > + > > > > > + if (core->flags & CLK_OPS_PARENT_ENABLE) > > > > > + clk_core_disable_unprepare(core->parent); > > > > > > > > > > done: > > > > > if (core->rpm_enabled) > > > > > > > > > > pm_runtime_put(core->dev); > > > > > > > > > > @@ -812,6 +825,9 @@ int clk_rate_exclusive_get(struct clk *clk) > > > > > > > > > > } > > > > > EXPORT_SYMBOL_GPL(clk_rate_exclusive_get); > > > > > > > > > > +static int clk_core_enable_lock(struct clk_core *core); > > > > > +static void clk_core_disable_lock(struct clk_core *core); > > > > > + > > > > > > > > > > static void clk_core_unprepare(struct clk_core *core) > > > > > { > > > > > > > > > > lockdep_assert_held(&prepare_lock); > > > > > > > > > > @@ -835,6 +851,9 @@ static void clk_core_unprepare(struct clk_core > > > > > *core) > > > > > > > > > > WARN(core->enable_count > 0, "Unpreparing enabled %s\n", core- > > > > > > > > > >name); > > > > > > > > > > + if (core->flags & CLK_OPS_PARENT_ENABLE) > > > > > + clk_core_enable_lock(core->parent); > > > > > + > > > > > > > > > > trace_clk_unprepare(core); > > > > > > > > > > if (core->ops->unprepare) > > > > > > > > > > @@ -843,6 +862,9 @@ static void clk_core_unprepare(struct clk_core > > > > > *core) > > > > > > > > > > clk_pm_runtime_put(core); > > > > > > > > > > trace_clk_unprepare_complete(core); > > > > > > > > > > + > > > > > + if (core->flags & CLK_OPS_PARENT_ENABLE) > > > > > + clk_core_disable_lock(core->parent); > > > > > > > > > > clk_core_unprepare(core->parent); > > > > > > > > > > } > > > > > > > > > > @@ -891,6 +913,9 @@ static int clk_core_prepare(struct clk_core > > > > > *core) > > > > > > > > > > if (ret) > > > > > > > > > > goto runtime_put; > > > > > > > > > > + if (core->flags & CLK_OPS_PARENT_ENABLE) > > > > > + clk_core_enable_lock(core->parent); > > > > > + > > > > > > > > > > trace_clk_prepare(core); > > > > > > > > > > if (core->ops->prepare) > > > > > > > > > > @@ -898,6 +923,9 @@ static int clk_core_prepare(struct clk_core > > > > > *core) > > > > > > > > > > trace_clk_prepare_complete(core); > > > > > > > > > > + if (core->flags & CLK_OPS_PARENT_ENABLE) > > > > > + clk_core_disable_lock(core->parent); > > > > > + > > > > > > > > > > if (ret) > > > > > > > > > > goto unprepare; > > > > > > > > > > } > > > > > > > > Unfortunately this completely locks up my i.MX8M Plus based board > > > > during > > > > early boot. > > > > I'm currently running on next-20220826 using > > > > arch/arm64/boot/dts/freescale/ > > > > imx8mp-tqma8mpql-mba8mpxl.dts > > > > Reverting this patch gets my board booting again. dmesg until hard > > > > lockup > > > > below. > > > > > > The standard logs don't have anything to go on. Could you add some > > > printk > > > calls to the clk core around the areas this patch touchs? That would > > > help. > > > > > > Could you also provide a dump of /sys/kernel/debug/clk/clk_summary? That > > > would help to understand the clock tree. > > > > Sure, > > > > These are the last kernel log lines before hard lockup: > > [ 0.686357] io scheduler mq-deadline registered > > [ 0.690907] io scheduler kyber registered > > [ 0.699275] clk_core_prepare: clk: main_axi CLK_OPS_PARENT_ENABLE > > > > main_axi is also the only debug output up to this point. This is the used > > patch for debugging: > > ---8<--- > > --- a/drivers/clk/clk.c > > +++ b/drivers/clk/clk.c > > @@ -211,8 +211,10 @@ static bool clk_core_is_prepared(struct clk_core > > *core)> > > return core->prepare_count; > > > > if (!clk_pm_runtime_get(core)) { > > > > - if (core->flags & CLK_OPS_PARENT_ENABLE) > > + if (core->flags & CLK_OPS_PARENT_ENABLE) { > > + pr_info("%s: clk: %s CLK_OPS_PARENT_ENABLE\n", > > __func__, core->name); > > > > clk_core_prepare_enable(core->parent); > > > > + } > > > > ret = core->ops->is_prepared(core->hw); > > if (core->flags & CLK_OPS_PARENT_ENABLE) > > > > clk_core_disable_unprepare(core->parent); > > > > @@ -251,8 +253,10 @@ static bool clk_core_is_enabled(struct clk_core > > *core) > > > > } > > > > } > > > > - if (core->flags & CLK_OPS_PARENT_ENABLE) > > + if (core->flags & CLK_OPS_PARENT_ENABLE) { > > + pr_info("%s: clk: %s CLK_OPS_PARENT_ENABLE\n", __func__, > > core-> > > >name); > > > > > clk_core_prepare_enable(core->parent); > > > > + } > > > > ret = core->ops->is_enabled(core->hw); > > > > @@ -851,8 +855,10 @@ static void clk_core_unprepare(struct clk_core *core) > > > > WARN(core->enable_count > 0, "Unpreparing enabled %s\n", > > core->name); > > > > - if (core->flags & CLK_OPS_PARENT_ENABLE) > > + if (core->flags & CLK_OPS_PARENT_ENABLE) { > > + pr_info("%s: clk: %s CLK_OPS_PARENT_ENABLE\n", __func__, > > core-> > > >name); > > > > > clk_core_enable_lock(core->parent); > > > > + } > > > > trace_clk_unprepare(core); > > > > @@ -912,8 +918,10 @@ static int clk_core_prepare(struct clk_core *core) > > > > if (ret) > > > > goto runtime_put; > > > > - if (core->flags & CLK_OPS_PARENT_ENABLE) > > + if (core->flags & CLK_OPS_PARENT_ENABLE) { > > + pr_info("%s: clk: %s CLK_OPS_PARENT_ENABLE\n", > > __func__, core->name); > > > > clk_core_enable_lock(core->parent); > > > > + } > > > > trace_clk_prepare(core); > > > > ---8<--- > > Thanks. So the part of the clock tree that's problematic is this: > > osc_24m (fixed) > sys_pll1_ref_sel (mux) > sys_pll1 (imx pll14xx) > sys_pll1_bypass (mux) > sys_pll1_out (gate) > sys_pll1_800m (fixed factor) > main_axi (composite CLK_IS_CRITICAL) > > Would it be possible for you to produce a stack dump as well? And also > enable lock debugging? This likely still won't catch anything if it's > a spinlock deadlock though. Sure thing, I added a dump_stack() call to all of the above debug outputs as well as the name of the parent clock, just to be sure, and here is the result output: [ 1.142386] io scheduler mq-deadline registered [ 1.146902] io scheduler kyber registered [ 1.177345] clk_core_prepare: clk: main_axi CLK_OPS_PARENT_ENABLE [ 1.180713] clk_core_prepare: clk->parent: sys_pll1_800m [ 1.186025] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 6.0.0-rc3- next-20220831+ #619 9c82e5b9075d735fa07e2c558603ffb720d72983 [ 1.197274] Hardware name: TQ-Systems i.MX8MPlus TQMa8MPxL on MBa8MPxL (DT) [ 1.204275] Call trace: [ 1.206723] dump_backtrace+0xd8/0x120 [ 1.210485] show_stack+0x14/0x50 [ 1.213811] dump_stack_lvl+0x88/0xb0 [ 1.217486] dump_stack+0x14/0x2c [ 1.220811] clk_core_prepare+0x1fc/0x240 [ 1.224834] __clk_core_init+0x208/0x4dc [ 1.228772] __clk_register+0x160/0x240 [ 1.232622] clk_hw_register+0x1c/0x5c [ 1.236384] __clk_hw_register_composite+0x1e8/0x2d0 [ 1.241372] clk_hw_register_composite+0x40/0x50 [ 1.246009] __imx8m_clk_hw_composite+0x130/0x210 [ 1.250734] imx8mp_clocks_probe+0x13ac/0x3750 [ 1.255199] platform_probe+0x64/0x100 [ 1.258959] call_driver_probe+0x28/0x140 [ 1.262984] really_probe+0xc0/0x334 [ 1.266572] __driver_probe_device+0x84/0x144 [ 1.270947] driver_probe_device+0x38/0x130 [ 1.275147] __driver_attach+0xd4/0x240 [ 1.278997] bus_for_each_dev+0x6c/0xbc [ 1.282847] driver_attach+0x20/0x30 [ 1.286436] bus_add_driver+0x178/0x250 [ 1.290284] driver_register+0x74/0x120 [ 1.294134] __platform_driver_register+0x24/0x30 [ 1.298859] imx8mp_clk_driver_init+0x18/0x20 [ 1.303234] do_one_initcall+0x48/0x180 [ 1.307084] do_initcalls+0x164/0x19c [ 1.310759] kernel_init_freeable+0xf4/0x138 [ 1.315047] kernel_init+0x2c/0x140 [ 1.318549] ret_from_fork+0x10/0x20 Enabling lock debugging results in no additional output. Best regards, Alexander
Hi there On Wed, 2022-08-31 at 09:40 +0200, Alexander Stein wrote: > Am Dienstag, 30. August 2022, 15:40:53 CEST schrieb Chen-Yu Tsai: [snip] > > Thanks. So the part of the clock tree that's problematic is this: > > > > osc_24m (fixed) > > sys_pll1_ref_sel (mux) > > sys_pll1 (imx pll14xx) > > sys_pll1_bypass (mux) > > sys_pll1_out (gate) > > sys_pll1_800m (fixed factor) > > main_axi (composite CLK_IS_CRITICAL) > > > > Would it be possible for you to produce a stack dump as well? And also > > enable lock debugging? This likely still won't catch anything if it's > > a spinlock deadlock though. > > Sure thing, I added a dump_stack() call to all of the above debug outputs as > well as the name of the parent clock, just to be sure, and here is the result > output: > [ 1.142386] io scheduler mq-deadline registered > [ 1.146902] io scheduler kyber registered > [ 1.177345] clk_core_prepare: clk: main_axi CLK_OPS_PARENT_ENABLE > [ 1.180713] clk_core_prepare: clk->parent: sys_pll1_800m > [ 1.186025] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 6.0.0-rc3- > next-20220831+ #619 9c82e5b9075d735fa07e2c558603ffb720d72983 > [ 1.197274] Hardware name: TQ-Systems i.MX8MPlus TQMa8MPxL on MBa8MPxL (DT) > [ 1.204275] Call trace: > [ 1.206723] dump_backtrace+0xd8/0x120 > [ 1.210485] show_stack+0x14/0x50 > [ 1.213811] dump_stack_lvl+0x88/0xb0 > [ 1.217486] dump_stack+0x14/0x2c > [ 1.220811] clk_core_prepare+0x1fc/0x240 > [ 1.224834] __clk_core_init+0x208/0x4dc > [ 1.228772] __clk_register+0x160/0x240 > [ 1.232622] clk_hw_register+0x1c/0x5c > [ 1.236384] __clk_hw_register_composite+0x1e8/0x2d0 > [ 1.241372] clk_hw_register_composite+0x40/0x50 > [ 1.246009] __imx8m_clk_hw_composite+0x130/0x210 > [ 1.250734] imx8mp_clocks_probe+0x13ac/0x3750 > [ 1.255199] platform_probe+0x64/0x100 > [ 1.258959] call_driver_probe+0x28/0x140 > [ 1.262984] really_probe+0xc0/0x334 > [ 1.266572] __driver_probe_device+0x84/0x144 > [ 1.270947] driver_probe_device+0x38/0x130 > [ 1.275147] __driver_attach+0xd4/0x240 > [ 1.278997] bus_for_each_dev+0x6c/0xbc > [ 1.282847] driver_attach+0x20/0x30 > [ 1.286436] bus_add_driver+0x178/0x250 > [ 1.290284] driver_register+0x74/0x120 > [ 1.294134] __platform_driver_register+0x24/0x30 > [ 1.298859] imx8mp_clk_driver_init+0x18/0x20 > [ 1.303234] do_one_initcall+0x48/0x180 > [ 1.307084] do_initcalls+0x164/0x19c > [ 1.310759] kernel_init_freeable+0xf4/0x138 > [ 1.315047] kernel_init+0x2c/0x140 > [ 1.318549] ret_from_fork+0x10/0x20 > > Enabling lock debugging results in no additional output. I also just bi-sected this to cause a similar hard lock-up on Verdin iMX8M Mini and Verdin iMX8M Plus running next-20220831. Let me know if you need any further information. Thanks! > Best regards, > Alexander Cheers Marcel
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c index 7fc191c15507..9b365cd6d14b 100644 --- a/drivers/clk/clk.c +++ b/drivers/clk/clk.c @@ -196,6 +196,9 @@ static bool clk_core_rate_is_protected(struct clk_core *core) return core->protect_count; } +static int clk_core_prepare_enable(struct clk_core *core); +static void clk_core_disable_unprepare(struct clk_core *core); + static bool clk_core_is_prepared(struct clk_core *core) { bool ret = false; @@ -208,7 +211,11 @@ static bool clk_core_is_prepared(struct clk_core *core) return core->prepare_count; if (!clk_pm_runtime_get(core)) { + if (core->flags & CLK_OPS_PARENT_ENABLE) + clk_core_prepare_enable(core->parent); ret = core->ops->is_prepared(core->hw); + if (core->flags & CLK_OPS_PARENT_ENABLE) + clk_core_disable_unprepare(core->parent); clk_pm_runtime_put(core); } @@ -244,7 +251,13 @@ static bool clk_core_is_enabled(struct clk_core *core) } } + if (core->flags & CLK_OPS_PARENT_ENABLE) + clk_core_prepare_enable(core->parent); + ret = core->ops->is_enabled(core->hw); + + if (core->flags & CLK_OPS_PARENT_ENABLE) + clk_core_disable_unprepare(core->parent); done: if (core->rpm_enabled) pm_runtime_put(core->dev); @@ -812,6 +825,9 @@ int clk_rate_exclusive_get(struct clk *clk) } EXPORT_SYMBOL_GPL(clk_rate_exclusive_get); +static int clk_core_enable_lock(struct clk_core *core); +static void clk_core_disable_lock(struct clk_core *core); + static void clk_core_unprepare(struct clk_core *core) { lockdep_assert_held(&prepare_lock); @@ -835,6 +851,9 @@ static void clk_core_unprepare(struct clk_core *core) WARN(core->enable_count > 0, "Unpreparing enabled %s\n", core->name); + if (core->flags & CLK_OPS_PARENT_ENABLE) + clk_core_enable_lock(core->parent); + trace_clk_unprepare(core); if (core->ops->unprepare) @@ -843,6 +862,9 @@ static void clk_core_unprepare(struct clk_core *core) clk_pm_runtime_put(core); trace_clk_unprepare_complete(core); + + if (core->flags & CLK_OPS_PARENT_ENABLE) + clk_core_disable_lock(core->parent); clk_core_unprepare(core->parent); } @@ -891,6 +913,9 @@ static int clk_core_prepare(struct clk_core *core) if (ret) goto runtime_put; + if (core->flags & CLK_OPS_PARENT_ENABLE) + clk_core_enable_lock(core->parent); + trace_clk_prepare(core); if (core->ops->prepare) @@ -898,6 +923,9 @@ static int clk_core_prepare(struct clk_core *core) trace_clk_prepare_complete(core); + if (core->flags & CLK_OPS_PARENT_ENABLE) + clk_core_disable_lock(core->parent); + if (ret) goto unprepare; }
In the previous commits that added CLK_OPS_PARENT_ENABLE, support for this flag was only added to rate change operations (rate setting and reparent) and disabling unused subtree. It was not added to the clock gate related operations. Any hardware driver that needs it for these operations will either see bogus results, or worse, hang. This has been seen on MT8192 and MT8195, where the imp_ii2_* clk drivers set this, but dumping debugfs clk_summary would cause it to hang. Fixes: fc8726a2c021 ("clk: core: support clocks which requires parents enable (part 2)") Fixes: a4b3518d146f ("clk: core: support clocks which requires parents enable (part 1)") Signed-off-by: Chen-Yu Tsai <wenst@chromium.org> Reviewed-by: Nícolas F. R. A. Prado <nfraprado@collabora.com> Tested-by: Nícolas F. R. A. Prado <nfraprado@collabora.com> --- drivers/clk/clk.c | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+)