• Maxime Ripard's avatar
    clk: Set req_rate on reparenting · cb1b1dd9
    Maxime Ripard authored
    If a non-rate clock started by default with a parent that never
    registered, core->req_rate will be 0. The expectation is that whenever
    the parent will be registered, req_rate will be updated with the new
    value that has just been computed.
    
    However, if that clock is a mux, clk_set_parent() can also make that
    clock no longer orphan. In this case however, we never update req_rate.
    
    The natural solution to this would be to update core->rate and
    core->req_rate in clk_reparent() by calling clk_recalc().
    
    However, this doesn't work in all cases. Indeed, clk_recalc() is called
    by __clk_set_parent_before(), __clk_set_parent() and
    clk_core_reparent(). Both __clk_set_parent_before() and __clk_set_parent
    will call clk_recalc() with the enable_lock taken through a call to
    clk_enable_lock(), the underlying locking primitive being a spinlock.
    
    clk_recalc() calls the backing driver .recalc_rate hook, and that
    implementation might sleep if the underlying device uses a bus with
    accesses that might sleep, such as i2c.
    
    In such a situation, we would end up sleeping while holding a spinlock,
    and thus in an atomic section.
    
    In order to work around this, we can move the core->rate and
    core->req_rate update to the clk_recalc() calling sites, after the
    enable_lock has been released if it was taken.
    
    The only situation that could still be problematic is the
    clk_core_reparent() -> clk_reparent() case that doesn't have any
    locking. clk_core_reparent() is itself called by clk_hw_reparent(),
    which is then called by 4 drivers:
    
      * clk-stm32mp1.c, stm32/clk-stm32-core.c and tegra/clk-tegra210-emc.c
        use it in their set_parent implementation. The set_parent hook is
        only called by __clk_set_parent() and clk_change_rate(), both of
        them calling it without the enable_lock taken.
    
      * clk/tegra/clk-tegra124-emc.c calls it as part of its set_rate
        implementation. set_rate is only called by clk_change_rate(), again
        without the enable_lock taken.
    
    In both cases we can't end up in a situation where the clk_hw_reparent()
    caller would hold a spinlock, so it seems like this is a good
    workaround.
    
    Let's also add some unit tests to make sure we cover the original bug.
    
    Tested-by: Alexander Stein <alexander.stein@ew.tq-group.com> # imx8mp
    Tested-by: Marek Szyprowski <m.szyprowski@samsung.com> # exynos4210, meson g12b
    Signed-off-by: default avatarMaxime Ripard <maxime@cerno.tech>
    Link: https://lore.kernel.org/r/20220816112530.1837489-14-maxime@cerno.techTested-by: default avatarLinux Kernel Functional Testing <lkft@linaro.org>
    Tested-by: default avatarNaresh Kamboju <naresh.kamboju@linaro.org>
    Signed-off-by: default avatarStephen Boyd <sboyd@kernel.org>
    cb1b1dd9
clk.c 131 KB