• Jernej Skrabec's avatar
    clk: sunxi-ng: nkmp: Avoid GENMASK(-1, 0) · 2abc330c
    Jernej Skrabec authored
    Sometimes one of the nkmp factors is unused. This means that one of the
    factors shift and width values are set to 0. Current nkmp clock code
    generates a mask for each factor with GENMASK(width + shift - 1, shift).
    For unused factor this translates to GENMASK(-1, 0). This code is
    further expanded by C preprocessor to final version:
    (((~0UL) - (1UL << (0)) + 1) & (~0UL >> (BITS_PER_LONG - 1 - (-1))))
    or a bit simplified:
    (~0UL & (~0UL >> BITS_PER_LONG))
    
    It turns out that result of the second part (~0UL >> BITS_PER_LONG) is
    actually undefined by C standard, which clearly specifies:
    
    "If the value of the right operand is negative or is greater than or
    equal to the width of the promoted left operand, the behavior is
    undefined."
    
    Additionally, compiling kernel with aarch64-linux-gnu-gcc 8.3.0 gave
    different results whether literals or variables with same values as
    literals were used. GENMASK with literals -1 and 0 gives zero and with
    variables gives 0xFFFFFFFFFFFFFFF (~0UL). Because nkmp driver uses
    GENMASK with variables as parameter, expression calculates mask as ~0UL
    instead of 0. This has further consequences that LSB in register is
    always set to 1 (1 is neutral value for a factor and shift is 0).
    
    For example, H6 pll-de clock is set to 600 MHz by sun4i-drm driver, but
    due to this bug ends up being 300 MHz. Additionally, 300 MHz seems to be
    too low because following warning can be found in dmesg:
    
    [    1.752763] WARNING: CPU: 2 PID: 41 at drivers/clk/sunxi-ng/ccu_common.c:41 ccu_helper_wait_for_lock.part.0+0x6c/0x90
    [    1.763378] Modules linked in:
    [    1.766441] CPU: 2 PID: 41 Comm: kworker/2:1 Not tainted 5.1.0-rc2-next-20190401 #138
    [    1.774269] Hardware name: Pine H64 (DT)
    [    1.778200] Workqueue: events deferred_probe_work_func
    [    1.783341] pstate: 40000005 (nZcv daif -PAN -UAO)
    [    1.788135] pc : ccu_helper_wait_for_lock.part.0+0x6c/0x90
    [    1.793623] lr : ccu_helper_wait_for_lock.part.0+0x48/0x90
    [    1.799107] sp : ffff000010f93840
    [    1.802422] x29: ffff000010f93840 x28: 0000000000000000
    [    1.807735] x27: ffff800073ce9d80 x26: ffff000010afd1b8
    [    1.813049] x25: ffffffffffffffff x24: 00000000ffffffff
    [    1.818362] x23: 0000000000000001 x22: ffff000010abd5c8
    [    1.823675] x21: 0000000010000000 x20: 00000000685f367e
    [    1.828987] x19: 0000000000001801 x18: 0000000000000001
    [    1.834300] x17: 0000000000000001 x16: 0000000000000000
    [    1.839613] x15: 0000000000000000 x14: ffff000010789858
    [    1.844926] x13: 0000000000000000 x12: 0000000000000001
    [    1.850239] x11: 0000000000000000 x10: 0000000000000970
    [    1.855551] x9 : ffff000010f936c0 x8 : ffff800074cec0d0
    [    1.860864] x7 : 0000800067117000 x6 : 0000000115c30b41
    [    1.866177] x5 : 00ffffffffffffff x4 : 002c959300bfe500
    [    1.871490] x3 : 0000000000000018 x2 : 0000000029aaaaab
    [    1.876802] x1 : 00000000000002e6 x0 : 00000000686072bc
    [    1.882114] Call trace:
    [    1.884565]  ccu_helper_wait_for_lock.part.0+0x6c/0x90
    [    1.889705]  ccu_helper_wait_for_lock+0x10/0x20
    [    1.894236]  ccu_nkmp_set_rate+0x244/0x2a8
    [    1.898334]  clk_change_rate+0x144/0x290
    [    1.902258]  clk_core_set_rate_nolock+0x180/0x1b8
    [    1.906963]  clk_set_rate+0x34/0xa0
    [    1.910455]  sun8i_mixer_bind+0x484/0x558
    [    1.914466]  component_bind_all+0x10c/0x230
    [    1.918651]  sun4i_drv_bind+0xc4/0x1a0
    [    1.922401]  try_to_bring_up_master+0x164/0x1c0
    [    1.926932]  __component_add+0xa0/0x168
    [    1.930769]  component_add+0x10/0x18
    [    1.934346]  sun8i_dw_hdmi_probe+0x18/0x20
    [    1.938443]  platform_drv_probe+0x50/0xa0
    [    1.942455]  really_probe+0xcc/0x280
    [    1.946032]  driver_probe_device+0x54/0xe8
    [    1.950130]  __device_attach_driver+0x80/0xb8
    [    1.954488]  bus_for_each_drv+0x78/0xc8
    [    1.958326]  __device_attach+0xd4/0x130
    [    1.962163]  device_initial_probe+0x10/0x18
    [    1.966348]  bus_probe_device+0x90/0x98
    [    1.970185]  deferred_probe_work_func+0x6c/0xa0
    [    1.974720]  process_one_work+0x1e0/0x320
    [    1.978732]  worker_thread+0x228/0x428
    [    1.982484]  kthread+0x120/0x128
    [    1.985714]  ret_from_fork+0x10/0x18
    [    1.989290] ---[ end trace 9babd42e1ca4b84f ]---
    
    This commit solves the issue by first checking value of the factor
    width. If it is equal to 0 (unused factor), mask is set to 0, otherwise
    GENMASK() macro is used as before.
    
    Fixes: d897ef56 ("clk: sunxi-ng: Mask nkmp factors when setting register")
    Signed-off-by: default avatarJernej Skrabec <jernej.skrabec@siol.net>
    Signed-off-by: default avatarMaxime Ripard <maxime.ripard@bootlin.com>
    2abc330c
ccu_nkmp.c 5.69 KB