lkml.org 
[lkml]   [2021]   [Jul]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 5.13 053/266] clk: tegra: Fix refcounting of gate clocks
    Date
    From: Dmitry Osipenko <digetx@gmail.com>

    [ Upstream commit c592c8a28f5821e880ac6675781cd8a151b0737c ]

    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 <treding@nvidia.com>
    Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
    Signed-off-by: Thierry Reding <treding@nvidia.com>
    Signed-off-by: Sasha Levin <sashal@kernel.org>
    ---
    drivers/clk/tegra/clk-periph-gate.c | 72 +++++++++++++++++++----------
    drivers/clk/tegra/clk-periph.c | 11 +++++
    2 files changed, 58 insertions(+), 25 deletions(-)

    diff --git a/drivers/clk/tegra/clk-periph-gate.c b/drivers/clk/tegra/clk-periph-gate.c
    index 4b31beefc9fc..dc3f92678407 100644
    --- a/drivers/clk/tegra/clk-periph-gate.c
    +++ b/drivers/clk/tegra/clk-periph-gate.c
    @@ -48,18 +48,9 @@ static int clk_periph_is_enabled(struct clk_hw *hw)
    return state;
    }

    -static int clk_periph_enable(struct clk_hw *hw)
    +static void clk_periph_enable_locked(struct clk_hw *hw)
    {
    struct tegra_clk_periph_gate *gate = to_clk_periph_gate(hw);
    - unsigned long flags = 0;
    -
    - spin_lock_irqsave(&periph_ref_lock, flags);
    -
    - gate->enable_refcnt[gate->clk_num]++;
    - if (gate->enable_refcnt[gate->clk_num] > 1) {
    - spin_unlock_irqrestore(&periph_ref_lock, flags);
    - return 0;
    - }

    write_enb_set(periph_clk_to_bit(gate), gate);
    udelay(2);
    @@ -78,6 +69,32 @@ static int clk_periph_enable(struct clk_hw *hw)
    udelay(1);
    writel_relaxed(0, gate->clk_base + LVL2_CLK_GATE_OVRE);
    }
    +}
    +
    +static void clk_periph_disable_locked(struct clk_hw *hw)
    +{
    + struct tegra_clk_periph_gate *gate = to_clk_periph_gate(hw);
    +
    + /*
    + * If peripheral is in the APB bus then read the APB bus to
    + * flush the write operation in apb bus. This will avoid the
    + * peripheral access after disabling clock
    + */
    + if (gate->flags & TEGRA_PERIPH_ON_APB)
    + tegra_read_chipid();
    +
    + write_enb_clr(periph_clk_to_bit(gate), gate);
    +}
    +
    +static int clk_periph_enable(struct clk_hw *hw)
    +{
    + struct tegra_clk_periph_gate *gate = to_clk_periph_gate(hw);
    + unsigned long flags = 0;
    +
    + spin_lock_irqsave(&periph_ref_lock, flags);
    +
    + if (!gate->enable_refcnt[gate->clk_num]++)
    + clk_periph_enable_locked(hw);

    spin_unlock_irqrestore(&periph_ref_lock, flags);

    @@ -91,21 +108,28 @@ static void clk_periph_disable(struct clk_hw *hw)

    spin_lock_irqsave(&periph_ref_lock, flags);

    - gate->enable_refcnt[gate->clk_num]--;
    - if (gate->enable_refcnt[gate->clk_num] > 0) {
    - spin_unlock_irqrestore(&periph_ref_lock, flags);
    - return;
    - }
    + WARN_ON(!gate->enable_refcnt[gate->clk_num]);
    +
    + if (--gate->enable_refcnt[gate->clk_num] == 0)
    + clk_periph_disable_locked(hw);
    +
    + spin_unlock_irqrestore(&periph_ref_lock, flags);
    +}
    +
    +static void clk_periph_disable_unused(struct clk_hw *hw)
    +{
    + struct tegra_clk_periph_gate *gate = to_clk_periph_gate(hw);
    + unsigned long flags = 0;
    +
    + spin_lock_irqsave(&periph_ref_lock, flags);

    /*
    - * If peripheral is in the APB bus then read the APB bus to
    - * flush the write operation in apb bus. This will avoid the
    - * peripheral access after disabling clock
    + * Some clocks are duplicated and some of them are marked as critical,
    + * like fuse and fuse_burn for example, thus the enable_refcnt will
    + * be non-zero here if the "unused" duplicate is disabled by CCF.
    */
    - if (gate->flags & TEGRA_PERIPH_ON_APB)
    - tegra_read_chipid();
    -
    - write_enb_clr(periph_clk_to_bit(gate), gate);
    + if (!gate->enable_refcnt[gate->clk_num])
    + clk_periph_disable_locked(hw);

    spin_unlock_irqrestore(&periph_ref_lock, flags);
    }
    @@ -114,6 +138,7 @@ const struct clk_ops tegra_clk_periph_gate_ops = {
    .is_enabled = clk_periph_is_enabled,
    .enable = clk_periph_enable,
    .disable = clk_periph_disable,
    + .disable_unused = clk_periph_disable_unused,
    };

    struct clk *tegra_clk_register_periph_gate(const char *name,
    @@ -148,9 +173,6 @@ struct clk *tegra_clk_register_periph_gate(const char *name,
    gate->enable_refcnt = enable_refcnt;
    gate->regs = pregs;

    - if (read_enb(gate) & periph_clk_to_bit(gate))
    - enable_refcnt[clk_num]++;
    -
    /* Data in .init is copied by clk_register(), so stack variable OK */
    gate->hw.init = &init;

    diff --git a/drivers/clk/tegra/clk-periph.c b/drivers/clk/tegra/clk-periph.c
    index 67620c7ecd9e..79ca3aa072b7 100644
    --- a/drivers/clk/tegra/clk-periph.c
    +++ b/drivers/clk/tegra/clk-periph.c
    @@ -100,6 +100,15 @@ static void clk_periph_disable(struct clk_hw *hw)
    gate_ops->disable(gate_hw);
    }

    +static void clk_periph_disable_unused(struct clk_hw *hw)
    +{
    + struct tegra_clk_periph *periph = to_clk_periph(hw);
    + const struct clk_ops *gate_ops = periph->gate_ops;
    + struct clk_hw *gate_hw = &periph->gate.hw;
    +
    + gate_ops->disable_unused(gate_hw);
    +}
    +
    static void clk_periph_restore_context(struct clk_hw *hw)
    {
    struct tegra_clk_periph *periph = to_clk_periph(hw);
    @@ -126,6 +135,7 @@ const struct clk_ops tegra_clk_periph_ops = {
    .is_enabled = clk_periph_is_enabled,
    .enable = clk_periph_enable,
    .disable = clk_periph_disable,
    + .disable_unused = clk_periph_disable_unused,
    .restore_context = clk_periph_restore_context,
    };

    @@ -135,6 +145,7 @@ static const struct clk_ops tegra_clk_periph_nodiv_ops = {
    .is_enabled = clk_periph_is_enabled,
    .enable = clk_periph_enable,
    .disable = clk_periph_disable,
    + .disable_unused = clk_periph_disable_unused,
    .restore_context = clk_periph_restore_context,
    };

    --
    2.30.2


    \
     
     \ /
      Last update: 2021-07-15 21:28    [W:4.163 / U:0.364 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site