Hi Robert,
I'm truly sorry for the trouble the patch has caused you and for my late reply!
On 2024-06-14 at 23:52:08 +0000, "Pafford, Robert J." pafford.9@buckeyemail.osu.edu wrote:
The Allwinner SoC's typically have an upper and lower limit for their clocks' rates. Up until now, support for that has been implemented separately for each clock type.
Implement that functionality in the sunxi-ng's common part making use of the CCF rate liming capabilities, so that it is available for all clock types.
Suggested-by: Maxime Ripard mripard@kernel.org Signed-off-by: Frank Oltmanns frank@oltmanns.dev Cc: stable@vger.kernel.org
drivers/clk/sunxi-ng/ccu_common.c | 19 +++++++++++++++++++ drivers/clk/sunxi-ng/ccu_common.h | 3 +++ 2 files changed, 22 insertions(+)
This patch appears to cause a buffer under-read bug due to the call to 'hw_to_ccu_common', which assumes all entries in the desc->hw_clocks->hws array are contained in ccu_common structs.
However, not all clocks in the array are contained in ccu_common structs. For example, as part of the "sun20i-d1-ccu" driver, the "pll-video0" clock holds the 'clk_hw' struct inside of a 'clk_fixed_factor' struct, as it is a fixed factor clock based on the "pll-video0-4x" clock, created with the CLK_FIXED_FACTOR_HWS macro. This results in undefined behavior as the hw_to_ccu_common returns an invalid pointer referencing memory before the 'clk_fixed_factor' struct.
Great catch! At first glance, it seems to me that calling clk_hw_set_rate_range() in sunxi_ccu_probe() should not have happenend in the loop that iterates over the hw_clks.
Instead we should add one more loop that iterates over the ccu_clks. Note, that there is already one such loop but, unfortunately, we can't use that as it happens before the hw_clks loop and we can only call clk_hw_set_rate_range() after the hw_clk has been registered.
Hence, I propose to move the offending code to a new loop: for (i = 0; i < desc->num_ccu_clks; i++) { struct ccu_common *cclk = desc->ccu_clks[i];
if (!cclk) continue;
if (cclk->max_rate) clk_hw_set_rate_range(&cclk->hw, common->min_rate, common->max_rate); else WARN(cclk->min_rate, "No max_rate, ignoring min_rate of clock %d - %s\n", i, cclk->hw.init->name); }
I haven't tested (or even compiled) the above, but I'll test and send a patch within the next few days for you to test.
Thanks again, Frank
I have attached kernel warnings from a system based on the "sun8i-t113s.dtsi" device tree, where the memory contains a non-zero value for the min-rate but a zero value for the max-rate, triggering the "No max_rate, ignoring min_rate" warning in the 'sunxi_ccu_probe' function.
[ 0.549013] ------------[ cut here ]------------ [ 0.553727] WARNING: CPU: 0 PID: 1 at drivers/clk/sunxi-ng/ccu_common.c:155 sunxi_ccu_probe+0x105/0x164 [ 0.563153] No max_rate, ignoring min_rate of clock 6 - pll-periph0-div3 [ 0.569846] Modules linked in: [ 0.572913] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 6.6.32-winglet #7 [ 0.579540] Hardware name: Generic DT based system [ 0.584350] unwind_backtrace from show_stack+0xb/0xc [ 0.589445] show_stack from dump_stack_lvl+0x2b/0x34 [ 0.594531] dump_stack_lvl from __warn+0x5d/0x92 [ 0.599275] __warn from warn_slowpath_fmt+0xd7/0x12c [ 0.604354] warn_slowpath_fmt from sunxi_ccu_probe+0x105/0x164 [ 0.610299] sunxi_ccu_probe from devm_sunxi_ccu_probe+0x3d/0x60 [ 0.616317] devm_sunxi_ccu_probe from sun20i_d1_ccu_probe+0xbf/0xec [ 0.622681] sun20i_d1_ccu_probe from platform_probe+0x3d/0x78 [ 0.628542] platform_probe from really_probe+0x81/0x1d0 [ 0.633862] really_probe from __driver_probe_device+0x59/0x130 [ 0.639813] __driver_probe_device from driver_probe_device+0x2d/0xc8 [ 0.646283] driver_probe_device from __driver_attach+0x4d/0xf0 [ 0.652216] __driver_attach from bus_for_each_dev+0x49/0x84 [ 0.657888] bus_for_each_dev from bus_add_driver+0x91/0x13c [ 0.663567] bus_add_driver from driver_register+0x37/0xa4 [ 0.669066] driver_register from do_one_initcall+0x41/0x1c4 [ 0.674740] do_one_initcall from kernel_init_freeable+0x13d/0x180 [ 0.680937] kernel_init_freeable from kernel_init+0x15/0xec [ 0.686607] kernel_init from ret_from_fork+0x11/0x1c [ 0.691674] Exception stack(0xc8815fb0 to 0xc8815ff8) [ 0.696739] 5fa0: 00000000 00000000 00000000 00000000 [ 0.704926] 5fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 [ 0.713111] 5fe0: 00000000 00000000 00000000 00000000 00000013 00000000 [ 0.719765] ---[ end trace 0000000000000000 ]--- [ 0.724452] ------------[ cut here ]------------ [ 0.729082] WARNING: CPU: 0 PID: 1 at drivers/clk/sunxi-ng/ccu_common.c:155 sunxi_ccu_probe+0x105/0x164 [ 0.738518] No max_rate, ignoring min_rate of clock 9 - pll-video0 [ 0.744730] Modules linked in: [ 0.747801] CPU: 0 PID: 1 Comm: swapper/0 Tainted: G W 6.6.32-winglet #7 [ 0.755911] Hardware name: Generic DT based system [ 0.760696] unwind_backtrace from show_stack+0xb/0xc [ 0.765768] show_stack from dump_stack_lvl+0x2b/0x34 [ 0.770859] dump_stack_lvl from __warn+0x5d/0x92 [ 0.775600] __warn from warn_slowpath_fmt+0xd7/0x12c [ 0.780668] warn_slowpath_fmt from sunxi_ccu_probe+0x105/0x164 [ 0.786620] sunxi_ccu_probe from devm_sunxi_ccu_probe+0x3d/0x60 [ 0.792664] devm_sunxi_ccu_probe from sun20i_d1_ccu_probe+0xbf/0xec [ 0.799035] sun20i_d1_ccu_probe from platform_probe+0x3d/0x78 [ 0.804901] platform_probe from really_probe+0x81/0x1d0 [ 0.810229] really_probe from __driver_probe_device+0x59/0x130 [ 0.816171] __driver_probe_device from driver_probe_device+0x2d/0xc8 [ 0.822624] driver_probe_device from __driver_attach+0x4d/0xf0 [ 0.828566] __driver_attach from bus_for_each_dev+0x49/0x84 [ 0.834237] bus_for_each_dev from bus_add_driver+0x91/0x13c [ 0.839925] bus_add_driver from driver_register+0x37/0xa4 [ 0.845441] driver_register from do_one_initcall+0x41/0x1c4 [ 0.851123] do_one_initcall from kernel_init_freeable+0x13d/0x180 [ 0.857335] kernel_init_freeable from kernel_init+0x15/0xec [ 0.863022] kernel_init from ret_from_fork+0x11/0x1c [ 0.868096] Exception stack(0xc8815fb0 to 0xc8815ff8) [ 0.873145] 5fa0: 00000000 00000000 00000000 00000000 [ 0.881332] 5fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 [ 0.889525] 5fe0: 00000000 00000000 00000000 00000000 00000013 00000000 [ 0.896165] ---[ end trace 0000000000000000 ]--- [ 0.900821] ------------[ cut here ]------------ [ 0.905471] WARNING: CPU: 0 PID: 1 at drivers/clk/sunxi-ng/ccu_common.c:155 sunxi_ccu_probe+0x105/0x164 [ 0.914885] No max_rate, ignoring min_rate of clock 12 - pll-video1 [ 0.921143] Modules linked in: [ 0.924208] CPU: 0 PID: 1 Comm: swapper/0 Tainted: G W 6.6.32-winglet #7 [ 0.932308] Hardware name: Generic DT based system [ 0.937102] unwind_backtrace from show_stack+0xb/0xc [ 0.942173] show_stack from dump_stack_lvl+0x2b/0x34 [ 0.947254] dump_stack_lvl from __warn+0x5d/0x92 [ 0.952004] __warn from warn_slowpath_fmt+0xd7/0x12c [ 0.957081] warn_slowpath_fmt from sunxi_ccu_probe+0x105/0x164 [ 0.963034] sunxi_ccu_probe from devm_sunxi_ccu_probe+0x3d/0x60 [ 0.969052] devm_sunxi_ccu_probe from sun20i_d1_ccu_probe+0xbf/0xec [ 0.975422] sun20i_d1_ccu_probe from platform_probe+0x3d/0x78 [ 0.981288] platform_probe from really_probe+0x81/0x1d0 [ 0.986607] really_probe from __driver_probe_device+0x59/0x130 [ 0.992540] __driver_probe_device from driver_probe_device+0x2d/0xc8 [ 0.999002] driver_probe_device from __driver_attach+0x4d/0xf0 [ 1.004944] __driver_attach from bus_for_each_dev+0x49/0x84 [ 1.010606] bus_for_each_dev from bus_add_driver+0x91/0x13c [ 1.016286] bus_add_driver from driver_register+0x37/0xa4 [ 1.021785] driver_register from do_one_initcall+0x41/0x1c4 [ 1.027467] do_one_initcall from kernel_init_freeable+0x13d/0x180 [ 1.033679] kernel_init_freeable from kernel_init+0x15/0xec [ 1.039356] kernel_init from ret_from_fork+0x11/0x1c [ 1.044440] Exception stack(0xc8815fb0 to 0xc8815ff8) [ 1.049496] 5fa0: 00000000 00000000 00000000 00000000 [ 1.057674] 5fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 [ 1.065850] 5fe0: 00000000 00000000 00000000 00000000 00000013 00000000 [ 1.072471] ---[ end trace 0000000000000000 ]--- [ 1.077106] ------------[ cut here ]------------ [ 1.081734] WARNING: CPU: 0 PID: 1 at drivers/clk/sunxi-ng/ccu_common.c:155 sunxi_ccu_probe+0x105/0x164 [ 1.091165] No max_rate, ignoring min_rate of clock 16 - pll-audio0 [ 1.097441] Modules linked in: [ 1.100503] CPU: 0 PID: 1 Comm: swapper/0 Tainted: G W 6.6.32-winglet #7 [ 1.108602] Hardware name: Generic DT based system [ 1.113404] unwind_backtrace from show_stack+0xb/0xc [ 1.118474] show_stack from dump_stack_lvl+0x2b/0x34 [ 1.123564] dump_stack_lvl from __warn+0x5d/0x92 [ 1.128288] __warn from warn_slowpath_fmt+0xd7/0x12c [ 1.133356] warn_slowpath_fmt from sunxi_ccu_probe+0x105/0x164 [ 1.139283] sunxi_ccu_probe from devm_sunxi_ccu_probe+0x3d/0x60 [ 1.145318] devm_sunxi_ccu_probe from sun20i_d1_ccu_probe+0xbf/0xec [ 1.151680] sun20i_d1_ccu_probe from platform_probe+0x3d/0x78 [ 1.157537] platform_probe from really_probe+0x81/0x1d0 [ 1.162857] really_probe from __driver_probe_device+0x59/0x130 [ 1.168816] __driver_probe_device from driver_probe_device+0x2d/0xc8 [ 1.175278] driver_probe_device from __driver_attach+0x4d/0xf0 [ 1.181219] __driver_attach from bus_for_each_dev+0x49/0x84 [ 1.186908] bus_for_each_dev from bus_add_driver+0x91/0x13c [ 1.192595] bus_add_driver from driver_register+0x37/0xa4 [ 1.198103] driver_register from do_one_initcall+0x41/0x1c4 [ 1.203803] do_one_initcall from kernel_init_freeable+0x13d/0x180 [ 1.210006] kernel_init_freeable from kernel_init+0x15/0xec [ 1.215684] kernel_init from ret_from_fork+0x11/0x1c [ 1.220759] Exception stack(0xc8815fb0 to 0xc8815ff8) [ 1.225806] 5fa0: 00000000 00000000 00000000 00000000 [ 1.233984] 5fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 [ 1.242169] 5fe0: 00000000 00000000 00000000 00000000 00000013 00000000 [ 1.248818] ---[ end trace 0000000000000000 ]---