Messages in this thread Patch in this message | | | Subject | Re: [PATCH] clk: rockchip: mark pclk_uart2 as critical on rk3328 | From | Robin Murphy <> | Date | Wed, 8 Jul 2020 19:44:24 +0100 |
| |
On 2020-07-08 17:28, Johan Jonker wrote: > Hi Robin and others, > > Just a clarification. > Test is done on rk3318 A95X Z2 that uses rk3328.dtsi. > Almost everything works fine now except for uart2. > Front display also works now with ported custom vfd driver for the > SM1628 clone used. > > When I include a printk function for debugging in > clk_disable_unused_subtree I get this list of disabled clocks below. > Could you list the rk3328 clocks that are turned off? Is there a difference?
I couldn't be bothered to rebuild a modified kernel package, so I just rebooted with "trace_event=clk_disable,clk_enable":
[robin@nemulon-9 ~]$ sudo grep uart /sys/kernel/debug/tracing/trace <idle>-0 [000] d... 0.000000: clk_enable: clk_uart2_div <idle>-0 [000] d... 0.000000: clk_enable: clk_uart2_frac <idle>-0 [000] d... 0.000000: clk_disable: clk_uart2_frac <idle>-0 [000] d... 0.000000: clk_disable: clk_uart2_div <idle>-0 [000] d... 0.000000: clk_enable: clk_uart1_div <idle>-0 [000] d... 0.000000: clk_enable: clk_uart1_frac <idle>-0 [000] d... 0.000000: clk_disable: clk_uart1_frac <idle>-0 [000] d... 0.000000: clk_disable: clk_uart1_div <idle>-0 [000] d... 0.000000: clk_enable: clk_uart0_div <idle>-0 [000] d... 0.000000: clk_enable: clk_uart0_frac <idle>-0 [000] d... 0.000000: clk_disable: clk_uart0_frac <idle>-0 [000] d... 0.000000: clk_disable: clk_uart0_div swapper/0-1 [002] d... 5.397599: clk_enable: sclk_uart2 swapper/0-1 [002] d... 5.397619: clk_enable: pclk_uart2 swapper/0-1 [002] d... 5.398056: clk_disable: sclk_uart2 swapper/0-1 [002] d... 5.398081: clk_enable: sclk_uart2 swapper/0-1 [003] d... 5.716848: clk_disable: pclk_uart1 swapper/0-1 [003] d... 5.716850: clk_disable: pclk_uart0 swapper/0-1 [003] d... 5.718596: clk_disable: sclk_uart2 swapper/0-1 [003] d... 5.718612: clk_enable: sclk_uart2 (agetty)-554 [003] d... 12.662968: clk_disable: sclk_uart2 (agetty)-554 [003] d... 12.662995: clk_enable: sclk_uart2
This looks as I would expect - UART2 is the only one enabled in DT, so pclk_uart2 gets enabled when the driver loads and stays that way, while the events for pclk_uart{0,1} correlate with clk_disable_unused() running at the final round of initcalls:
[ 5.398003] ff130000.serial: ttyS2 at MMIO 0xff130000 (irq = 14, base_baud = 1500000) is a 16550A [ 5.470655] printk: console [ttyS2] enabled ... [ 5.756099] Run /init as init process
FWIW I have "earlycon=uart8250,mmio32,0xff130000" on my command line, but no explicit console argument - that's going to ttyS2 by virtue of the DT "stdout-path".
Robin.
> boot options including: > earlycon=uart8250,mmio32,0xff130000,keep > console=ttyS2,1500000n8 > > But the real console in which one can type ends up on ttyUSB0. > Without console defined in the command line it's only usb keyboard and > hdmi monitor. Output on ttyS2 stops right after: > > [ 1.309231] C:pclk_uart2 -> pclk_bus > > To see more I have to include clk_ignore_unused to the command line. > In clk-px30.c they also included pclk_uart2 to the critical list. > Could Elaine Zhang give more info on that? > > Kind regards, > > Johan Jonker > > static void __init clk_disable_unused_subtree(struct clk_core *core) > { > struct clk_core *child; > unsigned long flags; > > lockdep_assert_held(&prepare_lock); > > hlist_for_each_entry(child, &core->children, child_node) > clk_disable_unused_subtree(child); > > if (core->flags & CLK_OPS_PARENT_ENABLE) > clk_core_prepare_enable(core->parent); > > if (clk_pm_runtime_get(core)) > goto unprepare_out; > > flags = clk_enable_lock(); > > if (core->enable_count) > goto unlock_out; > > if (core->flags & CLK_IGNORE_UNUSED) > goto unlock_out; > > /* > * some gate clocks have special needs during the disable-unused > * sequence. call .disable_unused if available, otherwise fall > * back to .disable > */ > if (clk_core_is_enabled(core)) { > trace_clk_disable(core); > > //////////// > // >>> Include debug here <<< > > printk("C:%s -> %s\n", core->name, core->parent ? core->parent->name : > "*"); > > //////////// > if (core->ops->disable_unused) > core->ops->disable_unused(core->hw); > else if (core->ops->disable) > core->ops->disable(core->hw); > trace_clk_disable_complete(core); > } > > unlock_out: > clk_enable_unlock(flags); > clk_pm_runtime_put(core); > unprepare_out: > if (core->flags & CLK_OPS_PARENT_ENABLE) > clk_core_disable_unprepare(core->parent); > } > > > label kernel > kernel /Image-CLK > fdt /rk3318-a95x-z2-CYX-led.dtb > append root=/dev/mmcblk0p5 console=tty0 console=ttyUSB0,115200n8 > console=ttyS2,1500000n8 initcall_debug=1 debug drm.debug=0xe > earlycon=uart8250,mmio32,0xff130000,keep swiotlb=1 kpti=0 > no_console_suspend=1 consoleblank=0 rootwait > > > [ 1.282096] calling clk_disable_unused+0x0/0x110 @ 1 > [ 1.282705] C:sclk_timer5 -> xin24m > [ 1.283115] C:sclk_timer4 -> xin24m > [ 1.283524] C:sclk_timer3 -> xin24m > [ 1.283932] C:sclk_timer2 -> xin24m > [ 1.284340] C:sclk_timer1 -> xin24m > [ 1.284748] C:sclk_timer0 -> xin24m > [ 1.285158] C:clk_otp -> xin24m > [ 1.285536] C:aclk_mac2io -> aclk_gmac > [ 1.286035] C:pclk_mac2io -> pclk_gmac > [ 1.286483] C:clk_rga -> gpll > [ 1.286834] C:aclk_gpu -> aclk_gpu_pre > [ 1.287284] C:aclk_tsp -> aclk_bus_pre > [ 1.287723] C:aclk_dcf -> aclk_bus_pre > [ 1.288162] C:pclk_acodecphy -> pclk_phy_pre > [ 1.295372] C:pclk_dcf -> pclk_bus > [ 1.309231] C:pclk_uart2 -> pclk_bus > [ 1.322902] C:pclk_uart1 -> pclk_bus > [ 1.329133] C:pclk_uart0 -> pclk_bus > [ 1.335230] C:pclk_spi -> pclk_bus > [ 1.341538] C:pclk_stimer -> pclk_bus > [ 1.341543] C:pclk_i2c3 -> pclk_bus > [ 1.341547] C:pclk_i2c2 -> pclk_bus > [ 1.341551] C:pclk_i2c1 -> pclk_bus > [ 1.341554] C:pclk_i2c0 -> pclk_bus > [ 1.341562] C:hclk_pdm -> hclk_bus_pre > [ 1.341566] C:hclk_crypto_slv -> hclk_bus_pre > [ 1.341574] C:hclk_crypto_mst -> hclk_bus_pre > [ 1.395518] C:hclk_tsp -> hclk_bus_pre > [ 1.401131] C:hclk_spdif_8ch -> hclk_bus_pre > [ 1.406791] C:hclk_i2s2_2ch -> hclk_bus_pre > [ 1.421229] C:hclk_i2s1_8ch -> hclk_bus_pre > [ 1.426642] C:hclk_i2s0_8ch -> hclk_bus_pre > [ 1.431878] C:clk_wifi -> cpll > [ 1.436987] C:sclk_vdec_core -> cpll > [ 1.442045] C:sclk_vdec_cabac -> cpll > [ 1.446982] C:aclk_rga -> aclk_rga_pre > [ 1.451825] C:aclk_hdcp -> aclk_vio_pre > [ 1.456574] C:aclk_cif -> aclk_vio_pre > [ 1.461226] C:aclk_iep -> aclk_vio_pre > [ 1.465794] C:pclk_hdcp -> hclk_vio_pre > [ 1.470401] C:hclk_hdcp -> hclk_vio_pre > [ 1.470406] C:hclk_rga -> hclk_vio_pre > [ 1.470411] C:hclk_cif -> hclk_vio_pre > [ 1.470415] C:hclk_iep -> hclk_vio_pre > [ 1.470424] C:clk_mac2io_out -> cpll > [ 1.470432] C:clk_mac2io_ref -> clk_mac2io > [ 1.470440] C:clk_mac2io_rx -> clk_mac2io > [ 1.507448] C:clk_mac2io_tx -> clk_mac2io > [ 1.511778] C:clk_mac2io_refout -> clk_mac2io > [ 1.516094] C:clk_mac2io_src -> cpll > [ 1.520315] C:clk_ref_usb3otg_src -> cpll > [ 1.524560] C:clk_sdmmc_ext -> cpll > [ 1.528701] C:hclk_sdmmc_ext -> hclk_peri > [ 1.532850] C:clk_cif_src -> cpll > [ 1.536945] C:sclk_venc_dsp -> cpll > [ 1.541006] C:sclk_venc_core -> cpll > [ 1.544972] C:aclk_h264 -> aclk_rkvenc > [ 1.548936] C:aclk_h265 -> aclk_rkvenc > [ 1.552794] C:hclk_h264 -> hclk_rkvenc > [ 1.556580] C:pclk_h265 -> hclk_rkvenc > [ 1.560373] C:aclk_rkvdec -> aclk_rkvdec_pre > [ 1.564227] C:hclk_rkvdec -> hclk_rkvdec_pre > [ 1.568096] C:clk_spi -> cpll > [ 1.571981] C:clk_crypto -> cpll > [ 1.575897] C:clk_i2c3 -> cpll > [ 1.579795] C:clk_i2c2 -> cpll > [ 1.583657] C:clk_i2c1 -> cpll > [ 1.587521] C:clk_i2c0 -> cpll > [ 1.591357] C:i2s2_out -> clk_i2s2 > [ 1.595170] C:clk_i2s2 -> i2s2_pre > [ 1.599020] C:i2s1_out -> clk_i2s1 > [ 1.602893] C:clk_i2s1 -> i2s1_pre > [ 1.606749] C:clk_i2s0 -> i2s0_pre > [ 1.610519] C:clk_tsp -> cpll > [ 1.614239] C:clk_pdm -> apll > [ 1.617877] C:clk_hsadc_tsp -> * > [ 1.621718] initcall clk_disable_unused+0x0/0x110 returned 0 after > 331056 usecs > > > On 7/8/20 5:34 PM, Robin Murphy wrote: >> On 2020-07-08 15:45, Johan Jonker wrote: >>> The rk3328 uart2 port is used as boot console and to debug. >>> During the boot pclk_uart2 is disabled by a clk_disable_unused >>> initcall. Fix the uart2 function by marking pclk_uart2 >>> as critical on rk3328. Also add sclk_uart2 as that is needed >>> for the same DT node. >> >> Hmm, given that those are named in the DT as the "baudclk" and >> "apb_pclk" that dw8250_probe() explicitly claims, they really >> shouldn't be unused :/ >> >> On my RK3328 box they appear to be prepared and enabled OK: >> >> [robin@nemulon-9 ~]$ sudo grep uart2 /sys/kernel/debug/clk/clk_summary >> sclk_uart2 1 1 0 24000000 0 0 50000 >> pclk_uart2 1 1 0 75000000 0 0 50000 >> ... >> >> Robin. >> >>> Signed-off-by: Johan Jonker <jbx6244@gmail.com> >>> --- >>> drivers/clk/rockchip/clk-rk3328.c | 2 ++ >>> 1 file changed, 2 insertions(+) >>> >>> diff --git a/drivers/clk/rockchip/clk-rk3328.c b/drivers/clk/rockchip/clk-rk3328.c >>> index c186a1985..cb7749cb7 100644 >>> --- a/drivers/clk/rockchip/clk-rk3328.c >>> +++ b/drivers/clk/rockchip/clk-rk3328.c >>> @@ -875,6 +875,8 @@ static const char *const rk3328_critical_clocks[] __initconst = { >>> "aclk_gmac_niu", >>> "pclk_gmac_niu", >>> "pclk_phy_niu", >>> + "pclk_uart2", >>> + "sclk_uart2", >>> }; >>> >>> static void __init rk3328_clk_init(struct device_node *np) >>> >
| |