Re: [PATCH v8 2/9] clk: tegra: Fix refcounting of gate clocks
From: Jon Hunter <jonathanh@nvidia.com>
Date: 2021-07-14 17:59:08
Also in:
linux-clk, linux-tegra, lkml
On 14/07/2021 12:59, Dmitry Osipenko wrote:
14.07.2021 14:48, Jon Hunter пишет:quoted
On 16/05/2021 17:30, Dmitry Osipenko wrote:quoted
The refcounting of the gate clocks has a bug causing the enable_refcnt to underflow when unused clocks are disabled. This happens because clk provider erroneously bumps the refcount if clock is enabled at a boot time, which it shouldn't be doing, and it does this only for the gate clocks, while peripheral clocks are using the same gate ops and the peripheral clocks are missing the initial bump. Hence the refcount of the peripheral clocks is 0 when unused clocks are disabled and then the counter is decremented further by the gate ops, causing the integer underflow. Fix this problem by removing the erroneous bump and by implementing the disable_unused() callback, which disables the unused gates properly. The visible effect of the bug is such that the unused clocks are never gated if a loaded kernel module grabs the unused clocks and starts to use them. In practice this shouldn't cause any real problems for the drivers and boards supported by the kernel today. Acked-by: Thierry Reding <redacted> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
...
quoted hunk ↗ jump to hunk
Seems you'll need to implement the disable_unused() callback for the clk_sdmmc_mux to fix it. It's good that this problem has been caught.diff --git a/drivers/clk/tegra/clk-sdmmc-mux.cb/drivers/clk/tegra/clk-sdmmc-mux.c index 316912d3b1a4..4f2c3309eea4 100644--- a/drivers/clk/tegra/clk-sdmmc-mux.c +++ b/drivers/clk/tegra/clk-sdmmc-mux.c@@ -194,6 +194,15 @@ static void clk_sdmmc_mux_disable(struct clk_hw *hw) gate_ops->disable(gate_hw); } +static void clk_sdmmc_mux_disable_unused(struct clk_hw *hw) +{ + struct tegra_sdmmc_mux *sdmmc_mux = to_clk_sdmmc_mux(hw); + const struct clk_ops *gate_ops = sdmmc_mux->gate_ops; + struct clk_hw *gate_hw = &sdmmc_mux->gate.hw; + + gate_ops->disable_unused(gate_hw); +} + static void clk_sdmmc_mux_restore_context(struct clk_hw *hw) { struct clk_hw *parent = clk_hw_get_parent(hw);@@ -218,6 +227,7 @@ static const struct clk_ops tegra_clk_sdmmc_mux_ops = { .is_enabled = clk_sdmmc_mux_is_enabled, .enable = clk_sdmmc_mux_enable, .disable = clk_sdmmc_mux_disable, + .disable_unused = clk_sdmmc_mux_disable_unused, .restore_context = clk_sdmmc_mux_restore_context, };
Thanks, that fixes it! Please feel free to add my test-by and acked-by when you send out the patch. Acked-by: Jon Hunter <jonathanh@nvidia.com> Tested-by: Jon Hunter <jonathanh@nvidia.com> Cheers! Jon -- nvpublic