Re: [PATCH 0/6] Exynos5: cleanup clocks handling in power domains

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Anand,

On 2018-02-22 06:42, Anand Moon wrote:
On 21 February 2018 at 15:45, Marek Szyprowski <m.szyprowski@xxxxxxxxxxx> wrote:
This patchset performs cleanup of the clock handling during Exynos power
domain power on/off sequences. This has been achieved by moving all clock
related operations from Exynos power domain driver to respective Exynos
clock controller drivers. Such change is possible after introducinng
runtime power-management in common clock framework.

Another nice result of this cleanup is removal of deplock warning reported
in the following thread (the 'second issue'):
https://www.spinics.net/lists/linux-samsung-soc/msg61766.html

[    5.932966] ======================================================
[    5.937199] usb 5-1: new high-speed USB device number 2 using xhci-hcd
[    5.939073] WARNING: possible circular locking dependency detected
[    5.939110] 4.15.0-rc8-next-20180116 #1121 Tainted: G        W
[    5.958143] ------------------------------------------------------
[    5.964299] kworker/0:1/59 is trying to acquire lock:
[    5.969304]  (&genpd->mlock){+.+.}, at: [<6abc3872>] genpd_runtime_resume+0x104/0x260
[    5.977155]
[    5.977155] but task is already holding lock:
[    5.982926]  (prepare_lock){+.+.}, at: [<74cef905>] clk_prepare_lock+0x10/0xf8
[    5.990143]
[    5.990143] which lock already depends on the new lock.
[    5.990143]
[    5.998309]
[    5.998309] the existing dependency chain (in reverse order) is:
[    6.005739]
[    6.005739] -> #1 (prepare_lock){+.+.}:
[    6.011042]        mutex_lock_nested+0x1c/0x24
[    6.015419]        clk_prepare_lock+0x50/0xf8
[    6.019755]        clk_unprepare+0x1c/0x2c
[    6.023841]        exynos_pd_power+0x1a8/0x1e4
[    6.028246]        genpd_power_off+0x160/0x274
[    6.032664]        genpd_power_off_work_fn+0x2c/0x40
[    6.037630]        process_one_work+0x2d4/0x8f0
[    6.042104]        worker_thread+0x38/0x584
[    6.046268]        kthread+0x138/0x168
[    6.049981]        ret_from_fork+0x14/0x20
[    6.054044]          (null)
[    6.056794]
[    6.056794] -> #0 (&genpd->mlock){+.+.}:
[    6.062238]        __mutex_lock+0x7c/0xa68
[    6.066278]        mutex_lock_nested+0x1c/0x24
[    6.070703]        genpd_runtime_resume+0x104/0x260
[    6.075557]        __rpm_callback+0xc0/0x21c
[    6.079792]        rpm_callback+0x20/0x80
[    6.083774]        rpm_resume+0x558/0x7dc
[    6.087762]        __pm_runtime_resume+0x60/0x98
[    6.092367]        clk_core_prepare+0x44/0x490
[    6.096783]        clk_prepare+0x20/0x30
[    6.100674]        amba_get_enable_pclk+0x2c/0x60
[    6.105363]        amba_device_try_add+0x8c/0x20c
[    6.110041]        amba_deferred_retry_func+0x40/0xbc
[    6.115080]        process_one_work+0x2d4/0x8f0
[    6.119569]        worker_thread+0x38/0x584
[    6.123727]        kthread+0x138/0x168
[    6.127444]        ret_from_fork+0x14/0x20
[    6.131510]          (null)
[    6.134263]
[    6.134263] other info that might help us debug this:
[    6.134263]
[    6.142328]  Possible unsafe locking scenario:
[    6.142328]
[    6.148178]        CPU0                    CPU1
[    6.152656]        ----                    ----
[    6.157160]   lock(prepare_lock);
[    6.160439]                                lock(&genpd->mlock);
[    6.166365]                                lock(prepare_lock);
[    6.172168]   lock(&genpd->mlock);
[    6.175517]
[    6.175517]  *** DEADLOCK ***
[    6.175517]
[    6.181475] 4 locks held by kworker/0:1/59:
[    6.185580]  #0:  ((wq_completion)"events"){+.+.}, at: [<f71c19aa>] process_one_work+0x210/0x8f0
[    6.194407]  #1:  ((deferred_retry_work).work){+.+.}, at: [<f71c19aa>] process_one_work+0x210/0x8f0
[    6.203422]  #2:  (deferred_devices_lock){+.+.}, at: [<3e940c1f>] amba_deferred_retry_func+0x1c/0xbc
[    6.212522]  #3:  (prepare_lock){+.+.}, at: [<74cef905>] clk_prepare_lock+0x10/0xf8
[    6.220128]
[    6.220128] stack backtrace:
[    6.224438] CPU: 0 PID: 59 Comm: kworker/0:1 Tainted: G        W        4.15.0-rc8-next-20180116 #1121
[    6.233757] Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
[    6.239791] Workqueue: events amba_deferred_retry_func
[    6.244929] [<c0111f90>] (unwind_backtrace) from [<c010e360>] (show_stack+0x10/0x14)
[    6.252670] [<c010e360>] (show_stack) from [<c0a19860>] (dump_stack+0x98/0xc4)
[    6.259877] [<c0a19860>] (dump_stack) from [<c0181478>] (print_circular_bug.constprop.17+0x210/0x32c)
[    6.269077] [<c0181478>] (print_circular_bug.constprop.17) from [<c01848f8>] (__lock_acquire+0x155c/0x1ac8)
[    6.278786] [<c01848f8>] (__lock_acquire) from [<c0185884>] (lock_acquire+0xe0/0x2bc)
[    6.286558] [<c0185884>] (lock_acquire) from [<c0a31788>] (__mutex_lock+0x7c/0xa68)
[    6.294181] [<c0a31788>] (__mutex_lock) from [<c0a32190>] (mutex_lock_nested+0x1c/0x24)
[    6.302161] [<c0a32190>] (mutex_lock_nested) from [<c05885dc>] (genpd_runtime_resume+0x104/0x260)
[    6.311008] [<c05885dc>] (genpd_runtime_resume) from [<c057c6c4>] (__rpm_callback+0xc0/0x21c)
[    6.319484] [<c057c6c4>] (__rpm_callback) from [<c057c840>] (rpm_callback+0x20/0x80)
[    6.327185] [<c057c840>] (rpm_callback) from [<c057c2a8>] (rpm_resume+0x558/0x7dc)
[    6.334721] [<c057c2a8>] (rpm_resume) from [<c057c58c>] (__pm_runtime_resume+0x60/0x98)
[    6.342706] [<c057c58c>] (__pm_runtime_resume) from [<c04b7f6c>] (clk_core_prepare+0x44/0x490)
[    6.351297] [<c04b7f6c>] (clk_core_prepare) from [<c04ba304>] (clk_prepare+0x20/0x30)
[    6.359081] [<c04ba304>] (clk_prepare) from [<c04b52f4>] (amba_get_enable_pclk+0x2c/0x60)
[    6.367229] [<c04b52f4>] (amba_get_enable_pclk) from [<c04b5510>] (amba_device_try_add+0x8c/0x20c)
[    6.376164] [<c04b5510>] (amba_device_try_add) from [<c04b56f8>] (amba_deferred_retry_func+0x40/0xbc)
[    6.385361] [<c04b56f8>] (amba_deferred_retry_func) from [<c01465b4>] (process_one_work+0x2d4/0x8f0)
[    6.394457] [<c01465b4>] (process_one_work) from [<c0147860>] (worker_thread+0x38/0x584)
[    6.402497] [<c0147860>] (worker_thread) from [<c014d794>] (kthread+0x138/0x168)
[    6.409852] [<c014d794>] (kthread) from [<c01010b4>] (ret_from_fork+0x14/0x20)

This patchset affects Exynos5250 and Exynos5420/5422/5800 SoCs.

Changes has been tested on Snow Chromebook (Exynos5250), Peach-Pit
Chromebook2 (Exynos5420) and Odroid XU3/XU4 (Exynos5422) boards with
latest linux-next kernel.
I feel their is different CMU to support clk framework on Exynos platform.
below are the some of the address of the CMU from the respective user manual.

Exynos 4412:
The address map of Exynos 4412 clock controller consists of six CMUs.
They are, CMU_LEFTBUS, CMU_RIGHTBUS, CMU_TOP, CMU_DMC, CMU_CPU, and
CMU_ISP. Each CMU uses an address space of 16 KB for SFRs.
The internal structure of address space for each CMU is similar for all CMUs.

0x1003_4000         CMU_LEFTBUS
0x1003_8000         CMU_RIGHTBUS
0x1003_C000         CMU_TOP
0x1004_0000         CMU_DMC
0x1004_4000         CMU_CPU

Exynos 5250:
The address map of Exynos 5250 clock controller. There are nine CMUs
in Exynos 5250 and
each CMU uses 16 KB address space for SFRs. The nine CMUs are CMU_CPU,
CMU_CORE, CMU_ACP,
CMU_ISP, CMU_TOP, CMU_LEX, CMU_R0X, CMU_R1X, and CMU_CDREX. The
internal structure of the
address space for each CMU is similar to all CMUs.

0x1001_0000       CMU_CPU
0x1001_4000       CMU_CORE
0x1001_8000       CMU_ACP
0x1001_C000       CMU_ISP
0x1002_0000       CMU_TOP
0x1002_4000       CMU_LEX
0x1002_8000       CMU_R0X
0x1002_C000       CMU_R1X
0x1003_4000       CMU_CDREX

Exynos5422:
The address map of Exynos 5422 Clock Controller. Exynos 5422 provides
for seven CMUs:
CMU_CPU, CMU_KFC, CMU_CDREX, CMU_CPERI, CMU_G2D, CMU_ISP,  CMU_TOP
Each CMU uses 16 KB address space for SFRs. The internal structure of
the address space
for each CMU is similar for all CMUs.

0x1001_0000         CMU_CPU
0x1001_4000         CMU_CPERI
0x1001_8000         CMU_G2D
0x1001_C000        CMU_ISP
0x1002_4000         CMU_TOP
0x1003_0000         CMU_CDREX
0x1003_4000         CMU_KFC

I feel currently we have only one parent CMU for all the clock
so is it possible to restructure the clk to support these CMU
and also look into respective power-domain.

I have read few parts of the documentation but dont have in-dept
knowledge into this.
Please correct me if my understating is wrong. I have limited
knowledge on clk frame work.

Well, the main problem with 4412 and 5420/5422 is the fact that there is
no direct relation between each CMU and respective power domains.

For example display related clocks, which correspond to DISP power domain
are all mixed in CMU_TOP together with other non-display clocks. Same for
MFC, GSC, MSC and some ISP blocks. PERI, G2D, CPU and KFC all belong to
always-on power domains, so it is easier to have them handled together.

In short: there is no point in splitting clocks controller driver based on
the base address, as it simply gives no advantage at all.

> ...

Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland

--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux SoC Development]     [Linux Rockchip Development]     [Linux USB Development]     [Video for Linux]     [Linux Audio Users]     [Linux SCSI]     [Yosemite News]

  Powered by Linux