2022-08-22 08:26:06

by Chen-Yu Tsai

[permalink] [raw]
Subject: [PATCH RESEND v2 1/2] clk: core: Honor CLK_OPS_PARENT_ENABLE for clk gate ops

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 <[email protected]>
Reviewed-by: Nícolas F. R. A. Prado <[email protected]>
Tested-by: Nícolas F. R. A. Prado <[email protected]>
---
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


2022-08-26 12:38:08

by Alexander Stein

[permalink] [raw]
Subject: Re: [PATCH RESEND v2 1/2] clk: core: Honor CLK_OPS_PARENT_ENABLE for clk gate ops

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
> <[email protected]>
> Reviewed-by: N?colas F. R. A. Prado <[email protected]>
> Tested-by: N?colas F. R. A. Prado <[email protected]>
> ---
> 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 <[email protected]>
[ 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



2022-08-29 09:41:04

by Chen-Yu Tsai

[permalink] [raw]
Subject: Re: [PATCH RESEND v2 1/2] clk: core: Honor CLK_OPS_PARENT_ENABLE for clk gate ops

Hi,

On Fri, Aug 26, 2022 at 8:28 PM Alexander Stein
<[email protected]> 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
> > <[email protected]>
> > Reviewed-by: Nícolas F. R. A. Prado <[email protected]>
> > Tested-by: Nícolas F. R. A. Prado <[email protected]>
> > ---
> > 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

2022-08-30 12:54:13

by Alexander Stein

[permalink] [raw]
Subject: Re: [PATCH RESEND v2 1/2] clk: core: Honor CLK_OPS_PARENT_ENABLE for clk gate ops

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
>
> <[email protected]> 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
> > > <[email protected]>
> > > Reviewed-by: N?colas F. R. A. Prado <[email protected]>
> > > Tested-by: N?colas F. R. A. Prado <[email protected]>
> > > ---
> > >
> > > 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



2022-08-30 14:09:28

by Chen-Yu Tsai

[permalink] [raw]
Subject: Re: [PATCH RESEND v2 1/2] clk: core: Honor CLK_OPS_PARENT_ENABLE for clk gate ops

On Tue, Aug 30, 2022 at 8:37 PM Alexander Stein
<[email protected]> 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
> >
> > <[email protected]> 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
> > > > <[email protected]>
> > > > Reviewed-by: Nícolas F. R. A. Prado <[email protected]>
> > > > Tested-by: Nícolas F. R. A. Prado <[email protected]>
> > > > ---
> > > >
> > > > 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

2022-08-31 07:49:02

by Alexander Stein

[permalink] [raw]
Subject: Re: [PATCH RESEND v2 1/2] clk: core: Honor CLK_OPS_PARENT_ENABLE for clk gate ops

Am Dienstag, 30. August 2022, 15:40:53 CEST schrieb Chen-Yu Tsai:
> On Tue, Aug 30, 2022 at 8:37 PM Alexander Stein
>
> <[email protected]> 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
> > >
> > > <[email protected]> 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
> > > > > <[email protected]>
> > > > > Reviewed-by: N?colas F. R. A. Prado <[email protected]>
> > > > > Tested-by: N?colas F. R. A. Prado <[email protected]>
> > > > > ---
> > > > >
> > > > > 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



2022-08-31 14:04:06

by Marcel Ziswiler

[permalink] [raw]
Subject: Re: [PATCH RESEND v2 1/2] clk: core: Honor CLK_OPS_PARENT_ENABLE for clk gate ops

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