* [PATCH RESEND v2 0/2] clk: Fix CLK_OPS_PARENT_ENABLE and runtime PM
@ 2022-08-22 8:14 Chen-Yu Tsai
2022-08-22 8:14 ` [PATCH RESEND v2 1/2] clk: core: Honor CLK_OPS_PARENT_ENABLE for clk gate ops Chen-Yu Tsai
2022-08-22 8:14 ` [PATCH RESEND v2 2/2] clk: core: Fix runtime PM sequence in clk_core_unprepare() Chen-Yu Tsai
0 siblings, 2 replies; 9+ messages in thread
From: Chen-Yu Tsai @ 2022-08-22 8:14 UTC (permalink / raw)
To: Michael Turquette, Stephen Boyd
Cc: Chen-Yu Tsai, AngeloGioacchino Del Regno,
Nícolas F . R . A . Prado, linux-clk, linux-arm-kernel,
linux-kernel
Hi Mike, Stephen,
Resending this series from last month. This is now based on
next-20220822, but should apply cleanly to v6.0-rc1.
Here are a couple fixes for the clk core. They are unrelated but overlap
in diff context, so I'm sending them together.
Patch 1 makes the clk core honor CLK_OPS_PARENT_ENABLE for clk gate ops.
Without this, dumping clk_summary on the MT8192 would cause the system
to hang.
Patch 2 reorders the runtime PM call in clk_core_unprepare() to match
the order described in its original commit, and the opposite of that
in clk_core_prepare().
Changes since v1
- Use clk_core_{enable,disable}_lock() instead of non-locking variant.
Reported by Nícolas
- Added coverage for clk_core_is_prepared()
- Correct sequencing in clk_core_is_enabled() so that runtime PM is
handled before parent clock is enabled, matching other functions.
Regards
ChenYu
Chen-Yu Tsai (2):
clk: core: Honor CLK_OPS_PARENT_ENABLE for clk gate ops
clk: core: Fix runtime PM sequence in clk_core_unprepare()
drivers/clk/clk.c | 31 +++++++++++++++++++++++++++++--
1 file changed, 29 insertions(+), 2 deletions(-)
--
2.37.1.595.g718a3a8f04-goog
^ permalink raw reply [flat|nested] 9+ messages in thread* [PATCH RESEND v2 1/2] clk: core: Honor CLK_OPS_PARENT_ENABLE for clk gate ops 2022-08-22 8:14 [PATCH RESEND v2 0/2] clk: Fix CLK_OPS_PARENT_ENABLE and runtime PM Chen-Yu Tsai @ 2022-08-22 8:14 ` Chen-Yu Tsai 2022-08-26 12:28 ` Alexander Stein 2022-08-22 8:14 ` [PATCH RESEND v2 2/2] clk: core: Fix runtime PM sequence in clk_core_unprepare() Chen-Yu Tsai 1 sibling, 1 reply; 9+ messages in thread From: Chen-Yu Tsai @ 2022-08-22 8:14 UTC (permalink / raw) To: Michael Turquette, Stephen Boyd Cc: Chen-Yu Tsai, AngeloGioacchino Del Regno, Nícolas F . R . A . Prado, linux-clk, linux-arm-kernel, linux-kernel 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; } -- 2.37.1.595.g718a3a8f04-goog ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH RESEND v2 1/2] clk: core: Honor CLK_OPS_PARENT_ENABLE for clk gate ops 2022-08-22 8:14 ` [PATCH RESEND v2 1/2] clk: core: Honor CLK_OPS_PARENT_ENABLE for clk gate ops Chen-Yu Tsai @ 2022-08-26 12:28 ` Alexander Stein 2022-08-29 9:22 ` Chen-Yu Tsai 0 siblings, 1 reply; 9+ messages in thread From: Alexander Stein @ 2022-08-26 12:28 UTC (permalink / raw) To: Michael Turquette, Stephen Boyd, Chen-Yu Tsai Cc: Chen-Yu Tsai, AngeloGioacchino Del Regno, Nícolas F . R . A . Prado, linux-clk, linux-arm-kernel, linux-kernel 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH RESEND v2 1/2] clk: core: Honor CLK_OPS_PARENT_ENABLE for clk gate ops 2022-08-26 12:28 ` Alexander Stein @ 2022-08-29 9:22 ` Chen-Yu Tsai 2022-08-30 12:37 ` Alexander Stein 0 siblings, 1 reply; 9+ messages in thread From: Chen-Yu Tsai @ 2022-08-29 9:22 UTC (permalink / raw) To: Alexander Stein Cc: Michael Turquette, Stephen Boyd, AngeloGioacchino Del Regno, Nícolas F . R . A . Prado, linux-clk, linux-arm-kernel, linux-kernel 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH RESEND v2 1/2] clk: core: Honor CLK_OPS_PARENT_ENABLE for clk gate ops 2022-08-29 9:22 ` Chen-Yu Tsai @ 2022-08-30 12:37 ` Alexander Stein 2022-08-30 13:40 ` Chen-Yu Tsai 0 siblings, 1 reply; 9+ messages in thread From: Alexander Stein @ 2022-08-30 12:37 UTC (permalink / raw) To: Chen-Yu Tsai Cc: Michael Turquette, Stephen Boyd, AngeloGioacchino Del Regno, Nícolas F . R . A . Prado, linux-clk, linux-arm-kernel, linux-kernel 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH RESEND v2 1/2] clk: core: Honor CLK_OPS_PARENT_ENABLE for clk gate ops 2022-08-30 12:37 ` Alexander Stein @ 2022-08-30 13:40 ` Chen-Yu Tsai 2022-08-31 7:40 ` Alexander Stein 0 siblings, 1 reply; 9+ messages in thread From: Chen-Yu Tsai @ 2022-08-30 13:40 UTC (permalink / raw) To: Alexander Stein Cc: Michael Turquette, Stephen Boyd, AngeloGioacchino Del Regno, Nícolas F . R . A . Prado, linux-clk, linux-arm-kernel, linux-kernel 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH RESEND v2 1/2] clk: core: Honor CLK_OPS_PARENT_ENABLE for clk gate ops 2022-08-30 13:40 ` Chen-Yu Tsai @ 2022-08-31 7:40 ` Alexander Stein 2022-08-31 13:51 ` Marcel Ziswiler 0 siblings, 1 reply; 9+ messages in thread From: Alexander Stein @ 2022-08-31 7:40 UTC (permalink / raw) To: Chen-Yu Tsai Cc: Michael Turquette, Stephen Boyd, AngeloGioacchino Del Regno, Nícolas F . R . A . Prado, linux-clk, linux-arm-kernel, linux-kernel 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH RESEND v2 1/2] clk: core: Honor CLK_OPS_PARENT_ENABLE for clk gate ops 2022-08-31 7:40 ` Alexander Stein @ 2022-08-31 13:51 ` Marcel Ziswiler 0 siblings, 0 replies; 9+ messages in thread From: Marcel Ziswiler @ 2022-08-31 13:51 UTC (permalink / raw) To: alexander.stein, wenst Cc: nfraprado, linux-arm-kernel, angelogioacchino.delregno, linux-clk, linux-kernel, mturquette, sboyd 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH RESEND v2 2/2] clk: core: Fix runtime PM sequence in clk_core_unprepare() 2022-08-22 8:14 [PATCH RESEND v2 0/2] clk: Fix CLK_OPS_PARENT_ENABLE and runtime PM Chen-Yu Tsai 2022-08-22 8:14 ` [PATCH RESEND v2 1/2] clk: core: Honor CLK_OPS_PARENT_ENABLE for clk gate ops Chen-Yu Tsai @ 2022-08-22 8:14 ` Chen-Yu Tsai 1 sibling, 0 replies; 9+ messages in thread From: Chen-Yu Tsai @ 2022-08-22 8:14 UTC (permalink / raw) To: Michael Turquette, Stephen Boyd Cc: Chen-Yu Tsai, AngeloGioacchino Del Regno, Nícolas F . R . A . Prado, linux-clk, linux-arm-kernel, linux-kernel In the original commit 9a34b45397e5 ("clk: Add support for runtime PM"), the commit message mentioned that pm_runtime_put_sync() would be done at the end of clk_core_unprepare(). This mirrors the operations in clk_core_prepare() in the opposite order. However, the actual code that was added wasn't in the order the commit message described. Move clk_pm_runtime_put() to the end of clk_core_unprepare() so that it is in the correct order. Fixes: 9a34b45397e5 ("clk: Add support for runtime PM") Signed-off-by: Chen-Yu Tsai <wenst@chromium.org> Reviewed-by: Nícolas F. R. A. Prado <nfraprado@collabora.com> --- drivers/clk/clk.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c index 9b365cd6d14b..2e29a72c68e1 100644 --- a/drivers/clk/clk.c +++ b/drivers/clk/clk.c @@ -859,13 +859,12 @@ static void clk_core_unprepare(struct clk_core *core) if (core->ops->unprepare) core->ops->unprepare(core->hw); - 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); + clk_pm_runtime_put(core); } static void clk_core_unprepare_lock(struct clk_core *core) -- 2.37.1.595.g718a3a8f04-goog ^ permalink raw reply related [flat|nested] 9+ messages in thread
end of thread, other threads:[~2022-08-31 13:51 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-08-22 8:14 [PATCH RESEND v2 0/2] clk: Fix CLK_OPS_PARENT_ENABLE and runtime PM Chen-Yu Tsai 2022-08-22 8:14 ` [PATCH RESEND v2 1/2] clk: core: Honor CLK_OPS_PARENT_ENABLE for clk gate ops Chen-Yu Tsai 2022-08-26 12:28 ` Alexander Stein 2022-08-29 9:22 ` Chen-Yu Tsai 2022-08-30 12:37 ` Alexander Stein 2022-08-30 13:40 ` Chen-Yu Tsai 2022-08-31 7:40 ` Alexander Stein 2022-08-31 13:51 ` Marcel Ziswiler 2022-08-22 8:14 ` [PATCH RESEND v2 2/2] clk: core: Fix runtime PM sequence in clk_core_unprepare() Chen-Yu Tsai
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).