linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v8 0/9] Couple improvements for Tegra clk driver
@ 2021-05-16 16:30 Dmitry Osipenko
  2021-05-16 16:30 ` [PATCH v8 1/9] clk: tegra30: Use 300MHz for video decoder by default Dmitry Osipenko
                   ` (10 more replies)
  0 siblings, 11 replies; 17+ messages in thread
From: Dmitry Osipenko @ 2021-05-16 16:30 UTC (permalink / raw)
  To: Thierry Reding, Jonathan Hunter, Peter De Schrijver,
	Prashant Gaikwad, Michael Turquette, Stephen Boyd,
	Michał Mirosław
  Cc: Rob Herring, linux-tegra, linux-clk, linux-kernel, devicetree

This series fixes couple minor standalone problems of the Tegra clk
driver and adds new features.

Changelog:

v8: - Replaced division with a shift, which was suggested by Michał Mirosław
      in a comment to "Handle thermal DIV2 CPU frequency throttling" v7 patch.
      Cortex A9 CPUs don't have hardware divider and shifting is a minor
      improvement here, nevertheless it's good to have it.

    - Added new patch "Don't deassert reset on enabling clocks", which I
      accidentally forgot to include in v7. Previously sound driver
      was a blocker for this patch, the sound is fixed now in v5.13. The
      patch itself is needed for maintaining proper clk/reset sequences
      by PMC and other drivers.

v7: - Added r-b from Rob Herring to the schema patch which he gave to v6.

    - Dropped the MAINTAINERS-update patch. Previously Peter said on IRC
      that he doesn't have time on the tegra-clk driver anymore and approved
      the patch, but then he refused to ack the v6 patch, saying that he
      is not reading mailing lists. So I don't feel comfortable with that
      patch. Peter could send it by himself if will be necessary.

    - Added these new patches:

        clk: tegra: cclk: Handle thermal DIV2 CPU frequency throttling
        clk: tegra: Mark external clocks as not having reset control

      I sent out the new Tegra30 thermal sensor driver and now CPU clock
      could be throttled by the sensor hardware [1]. The first patch adds
      support for reporting of the throttled frequency properly.

      [1] https://patchwork.ozlabs.org/project/linux-tegra/list/?series=243126

      During of debugging sound issues of Asus Transformer devices, I noticed
      that the external clocks are missing the no-reset flag. The second
      patch fixes it.

v6: - Made a small improvement and corrected a typo in patch
      "Fix refcounting of gate clocks" that were spotted by
      Michał Mirosław.

v5: - Corrected example in the schema binding to silence dt_binding_check
      warning.

    - The Tegra124 binding is factored out into standalone binding since
      Tegra124 has properties that aren't used by other SoCs and I couldn't
      figure out how to make them conditional in schema.

v4: - Added new patch that converts DT bindings to schema.

v3: - Added acks from Thierry Reding that he gave to v2.

    - Added new patch "clk: tegra: Don't allow zero clock rate for PLLs".

v2: - Added these new patches:

      clk: tegra: Halve SCLK rate on Tegra20
      MAINTAINERS: Hand Tegra clk driver to Jon and Thierry

v1: - Collected clk patches into a single series.

Dmitry Osipenko (9):
  clk: tegra30: Use 300MHz for video decoder by default
  clk: tegra: Fix refcounting of gate clocks
  clk: tegra: Ensure that PLLU configuration is applied properly
  clk: tegra: Halve SCLK rate on Tegra20
  clk: tegra: Don't allow zero clock rate for PLLs
  clk: tegra: cclk: Handle thermal DIV2 CPU frequency throttling
  clk: tegra: Mark external clocks as not having reset control
  clk: tegra: Don't deassert reset on enabling clocks
  dt-bindings: clock: tegra: Convert to schema

 .../bindings/clock/nvidia,tegra114-car.txt    |  63 ----------
 .../bindings/clock/nvidia,tegra124-car.txt    | 107 ----------------
 .../bindings/clock/nvidia,tegra124-car.yaml   | 115 ++++++++++++++++++
 .../bindings/clock/nvidia,tegra20-car.txt     |  63 ----------
 .../bindings/clock/nvidia,tegra20-car.yaml    |  69 +++++++++++
 .../bindings/clock/nvidia,tegra210-car.txt    |  56 ---------
 .../bindings/clock/nvidia,tegra30-car.txt     |  63 ----------
 drivers/clk/tegra/clk-periph-gate.c           |  80 +++++++-----
 drivers/clk/tegra/clk-periph.c                |  11 ++
 drivers/clk/tegra/clk-pll.c                   |  12 +-
 drivers/clk/tegra/clk-tegra-periph.c          |   6 +-
 drivers/clk/tegra/clk-tegra-super-cclk.c      |  16 ++-
 drivers/clk/tegra/clk-tegra20.c               |   6 +-
 drivers/clk/tegra/clk-tegra30.c               |   6 +-
 drivers/clk/tegra/clk.h                       |   4 -
 15 files changed, 272 insertions(+), 405 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/clock/nvidia,tegra114-car.txt
 delete mode 100644 Documentation/devicetree/bindings/clock/nvidia,tegra124-car.txt
 create mode 100644 Documentation/devicetree/bindings/clock/nvidia,tegra124-car.yaml
 delete mode 100644 Documentation/devicetree/bindings/clock/nvidia,tegra20-car.txt
 create mode 100644 Documentation/devicetree/bindings/clock/nvidia,tegra20-car.yaml
 delete mode 100644 Documentation/devicetree/bindings/clock/nvidia,tegra210-car.txt
 delete mode 100644 Documentation/devicetree/bindings/clock/nvidia,tegra30-car.txt

-- 
2.30.2


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH v8 1/9] clk: tegra30: Use 300MHz for video decoder by default
  2021-05-16 16:30 [PATCH v8 0/9] Couple improvements for Tegra clk driver Dmitry Osipenko
@ 2021-05-16 16:30 ` Dmitry Osipenko
  2021-05-16 16:30 ` [PATCH v8 2/9] clk: tegra: Fix refcounting of gate clocks Dmitry Osipenko
                   ` (9 subsequent siblings)
  10 siblings, 0 replies; 17+ messages in thread
From: Dmitry Osipenko @ 2021-05-16 16:30 UTC (permalink / raw)
  To: Thierry Reding, Jonathan Hunter, Peter De Schrijver,
	Prashant Gaikwad, Michael Turquette, Stephen Boyd,
	Michał Mirosław
  Cc: Rob Herring, linux-tegra, linux-clk, linux-kernel, devicetree

The 600MHz is a too high clock rate for some SoC versions for the video
decoder hardware and this may cause stability issues. Use 300MHz for the
video decoder by default, which is supported by all hardware versions.

Fixes: ed1a2459e20c ("clk: tegra: Add Tegra20/30 EMC clock implementation")
Acked-by: Thierry Reding <treding@nvidia.com>
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 drivers/clk/tegra/clk-tegra30.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/tegra/clk-tegra30.c b/drivers/clk/tegra/clk-tegra30.c
index 16dbf83d2f62..a33688b2359e 100644
--- a/drivers/clk/tegra/clk-tegra30.c
+++ b/drivers/clk/tegra/clk-tegra30.c
@@ -1245,7 +1245,7 @@ static struct tegra_clk_init_table init_table[] __initdata = {
 	{ TEGRA30_CLK_GR3D, TEGRA30_CLK_PLL_C, 300000000, 0 },
 	{ TEGRA30_CLK_GR3D2, TEGRA30_CLK_PLL_C, 300000000, 0 },
 	{ TEGRA30_CLK_PLL_U, TEGRA30_CLK_CLK_MAX, 480000000, 0 },
-	{ TEGRA30_CLK_VDE, TEGRA30_CLK_PLL_C, 600000000, 0 },
+	{ TEGRA30_CLK_VDE, TEGRA30_CLK_PLL_C, 300000000, 0 },
 	{ TEGRA30_CLK_SPDIF_IN_SYNC, TEGRA30_CLK_CLK_MAX, 24000000, 0 },
 	{ TEGRA30_CLK_I2S0_SYNC, TEGRA30_CLK_CLK_MAX, 24000000, 0 },
 	{ TEGRA30_CLK_I2S1_SYNC, TEGRA30_CLK_CLK_MAX, 24000000, 0 },
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v8 2/9] clk: tegra: Fix refcounting of gate clocks
  2021-05-16 16:30 [PATCH v8 0/9] Couple improvements for Tegra clk driver Dmitry Osipenko
  2021-05-16 16:30 ` [PATCH v8 1/9] clk: tegra30: Use 300MHz for video decoder by default Dmitry Osipenko
@ 2021-05-16 16:30 ` Dmitry Osipenko
  2021-07-14 11:48   ` Jon Hunter
  2021-05-16 16:30 ` [PATCH v8 3/9] clk: tegra: Ensure that PLLU configuration is applied properly Dmitry Osipenko
                   ` (8 subsequent siblings)
  10 siblings, 1 reply; 17+ messages in thread
From: Dmitry Osipenko @ 2021-05-16 16:30 UTC (permalink / raw)
  To: Thierry Reding, Jonathan Hunter, Peter De Schrijver,
	Prashant Gaikwad, Michael Turquette, Stephen Boyd,
	Michał Mirosław
  Cc: Rob Herring, linux-tegra, linux-clk, linux-kernel, devicetree

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>
---
 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


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v8 3/9] clk: tegra: Ensure that PLLU configuration is applied properly
  2021-05-16 16:30 [PATCH v8 0/9] Couple improvements for Tegra clk driver Dmitry Osipenko
  2021-05-16 16:30 ` [PATCH v8 1/9] clk: tegra30: Use 300MHz for video decoder by default Dmitry Osipenko
  2021-05-16 16:30 ` [PATCH v8 2/9] clk: tegra: Fix refcounting of gate clocks Dmitry Osipenko
@ 2021-05-16 16:30 ` Dmitry Osipenko
  2021-05-16 16:30 ` [PATCH v8 4/9] clk: tegra: Halve SCLK rate on Tegra20 Dmitry Osipenko
                   ` (7 subsequent siblings)
  10 siblings, 0 replies; 17+ messages in thread
From: Dmitry Osipenko @ 2021-05-16 16:30 UTC (permalink / raw)
  To: Thierry Reding, Jonathan Hunter, Peter De Schrijver,
	Prashant Gaikwad, Michael Turquette, Stephen Boyd,
	Michał Mirosław
  Cc: Rob Herring, linux-tegra, linux-clk, linux-kernel, devicetree

The PLLU (USB) consists of the PLL configuration itself and configuration
of the PLLU outputs. The PLLU programming is inconsistent on T30 vs T114,
where T114 immediately bails out if PLLU is enabled and T30 re-enables
a potentially already enabled PLL (left after bootloader) and then fully
reprograms it, which could be unsafe to do. The correct way should be to
skip enabling of the PLL if it's already enabled and then apply
configuration to the outputs. This patch doesn't fix any known problems,
it's a minor improvement.

Acked-by: Thierry Reding <treding@nvidia.com>
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 drivers/clk/tegra/clk-pll.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/clk/tegra/clk-pll.c b/drivers/clk/tegra/clk-pll.c
index 0193cebe8c5a..823a567f2adc 100644
--- a/drivers/clk/tegra/clk-pll.c
+++ b/drivers/clk/tegra/clk-pll.c
@@ -1131,7 +1131,8 @@ static int clk_pllu_enable(struct clk_hw *hw)
 	if (pll->lock)
 		spin_lock_irqsave(pll->lock, flags);
 
-	_clk_pll_enable(hw);
+	if (!clk_pll_is_enabled(hw))
+		_clk_pll_enable(hw);
 
 	ret = clk_pll_wait_for_lock(pll);
 	if (ret < 0)
@@ -1748,15 +1749,13 @@ static int clk_pllu_tegra114_enable(struct clk_hw *hw)
 		return -EINVAL;
 	}
 
-	if (clk_pll_is_enabled(hw))
-		return 0;
-
 	input_rate = clk_hw_get_rate(__clk_get_hw(osc));
 
 	if (pll->lock)
 		spin_lock_irqsave(pll->lock, flags);
 
-	_clk_pll_enable(hw);
+	if (!clk_pll_is_enabled(hw))
+		_clk_pll_enable(hw);
 
 	ret = clk_pll_wait_for_lock(pll);
 	if (ret < 0)
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v8 4/9] clk: tegra: Halve SCLK rate on Tegra20
  2021-05-16 16:30 [PATCH v8 0/9] Couple improvements for Tegra clk driver Dmitry Osipenko
                   ` (2 preceding siblings ...)
  2021-05-16 16:30 ` [PATCH v8 3/9] clk: tegra: Ensure that PLLU configuration is applied properly Dmitry Osipenko
@ 2021-05-16 16:30 ` Dmitry Osipenko
  2021-05-16 16:30 ` [PATCH v8 5/9] clk: tegra: Don't allow zero clock rate for PLLs Dmitry Osipenko
                   ` (6 subsequent siblings)
  10 siblings, 0 replies; 17+ messages in thread
From: Dmitry Osipenko @ 2021-05-16 16:30 UTC (permalink / raw)
  To: Thierry Reding, Jonathan Hunter, Peter De Schrijver,
	Prashant Gaikwad, Michael Turquette, Stephen Boyd,
	Michał Mirosław
  Cc: Rob Herring, linux-tegra, linux-clk, linux-kernel, devicetree

Higher SCLK rates on Tegra20 require high core voltage. The higher
clock rate may have a positive performance effect only for AHB DMA
transfers and AVP CPU, but both aren't used by upstream kernel at all.
Halve SCLK rate on Tegra20 in order to remove the high core voltage
requirement.

Acked-by: Thierry Reding <treding@nvidia.com>
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 drivers/clk/tegra/clk-tegra20.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/clk/tegra/clk-tegra20.c b/drivers/clk/tegra/clk-tegra20.c
index 3efc651b42e3..3664593a5ba4 100644
--- a/drivers/clk/tegra/clk-tegra20.c
+++ b/drivers/clk/tegra/clk-tegra20.c
@@ -1021,9 +1021,9 @@ static struct tegra_clk_init_table init_table[] __initdata = {
 	{ TEGRA20_CLK_PLL_P_OUT3, TEGRA20_CLK_CLK_MAX, 72000000, 1 },
 	{ TEGRA20_CLK_PLL_P_OUT4, TEGRA20_CLK_CLK_MAX, 24000000, 1 },
 	{ TEGRA20_CLK_PLL_C, TEGRA20_CLK_CLK_MAX, 600000000, 0 },
-	{ TEGRA20_CLK_PLL_C_OUT1, TEGRA20_CLK_CLK_MAX, 240000000, 0 },
-	{ TEGRA20_CLK_SCLK, TEGRA20_CLK_PLL_C_OUT1, 240000000, 0 },
-	{ TEGRA20_CLK_HCLK, TEGRA20_CLK_CLK_MAX, 240000000, 0 },
+	{ TEGRA20_CLK_PLL_C_OUT1, TEGRA20_CLK_CLK_MAX, 120000000, 0 },
+	{ TEGRA20_CLK_SCLK, TEGRA20_CLK_PLL_C_OUT1, 120000000, 0 },
+	{ TEGRA20_CLK_HCLK, TEGRA20_CLK_CLK_MAX, 120000000, 0 },
 	{ TEGRA20_CLK_PCLK, TEGRA20_CLK_CLK_MAX, 60000000, 0 },
 	{ TEGRA20_CLK_CSITE, TEGRA20_CLK_CLK_MAX, 0, 1 },
 	{ TEGRA20_CLK_CCLK, TEGRA20_CLK_CLK_MAX, 0, 1 },
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v8 5/9] clk: tegra: Don't allow zero clock rate for PLLs
  2021-05-16 16:30 [PATCH v8 0/9] Couple improvements for Tegra clk driver Dmitry Osipenko
                   ` (3 preceding siblings ...)
  2021-05-16 16:30 ` [PATCH v8 4/9] clk: tegra: Halve SCLK rate on Tegra20 Dmitry Osipenko
@ 2021-05-16 16:30 ` Dmitry Osipenko
  2021-05-16 16:30 ` [PATCH v8 6/9] clk: tegra: cclk: Handle thermal DIV2 CPU frequency throttling Dmitry Osipenko
                   ` (5 subsequent siblings)
  10 siblings, 0 replies; 17+ messages in thread
From: Dmitry Osipenko @ 2021-05-16 16:30 UTC (permalink / raw)
  To: Thierry Reding, Jonathan Hunter, Peter De Schrijver,
	Prashant Gaikwad, Michael Turquette, Stephen Boyd,
	Michał Mirosław
  Cc: Rob Herring, linux-tegra, linux-clk, linux-kernel, devicetree

Zero clock rate doesn't make sense for PLLs and tegra-clk driver enters
into infinite loop on trying to calculate PLL parameters for zero rate.
Make code to error out if requested rate is zero.

Originally this trouble was found by Robert Yang while he was trying to
bring up upstream kernel on Samsung Galaxy Tab, which happened due to a
bug in Tegra DRM driver that erroneously sets PLL rate to zero. This
issues came over again recently during of kernel bring up on ASUS TF700T.

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 drivers/clk/tegra/clk-pll.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/clk/tegra/clk-pll.c b/drivers/clk/tegra/clk-pll.c
index 823a567f2adc..eaa079c177c3 100644
--- a/drivers/clk/tegra/clk-pll.c
+++ b/drivers/clk/tegra/clk-pll.c
@@ -558,6 +558,9 @@ static int _calc_rate(struct clk_hw *hw, struct tegra_clk_pll_freq_table *cfg,
 	u32 p_div = 0;
 	int ret;
 
+	if (!rate)
+		return -EINVAL;
+
 	switch (parent_rate) {
 	case 12000000:
 	case 26000000:
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v8 6/9] clk: tegra: cclk: Handle thermal DIV2 CPU frequency throttling
  2021-05-16 16:30 [PATCH v8 0/9] Couple improvements for Tegra clk driver Dmitry Osipenko
                   ` (4 preceding siblings ...)
  2021-05-16 16:30 ` [PATCH v8 5/9] clk: tegra: Don't allow zero clock rate for PLLs Dmitry Osipenko
@ 2021-05-16 16:30 ` Dmitry Osipenko
  2021-05-16 16:30 ` [PATCH v8 7/9] clk: tegra: Mark external clocks as not having reset control Dmitry Osipenko
                   ` (4 subsequent siblings)
  10 siblings, 0 replies; 17+ messages in thread
From: Dmitry Osipenko @ 2021-05-16 16:30 UTC (permalink / raw)
  To: Thierry Reding, Jonathan Hunter, Peter De Schrijver,
	Prashant Gaikwad, Michael Turquette, Stephen Boyd,
	Michał Mirosław
  Cc: Rob Herring, linux-tegra, linux-clk, linux-kernel, devicetree

Check whether thermal DIV2 throttle is active in order to report
the CPU frequency properly. This very useful for userspace tools
like cpufreq-info which show actual frequency asserted from hardware.

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 drivers/clk/tegra/clk-tegra-super-cclk.c | 16 ++++++++++++++--
 drivers/clk/tegra/clk-tegra30.c          |  2 +-
 2 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/drivers/clk/tegra/clk-tegra-super-cclk.c b/drivers/clk/tegra/clk-tegra-super-cclk.c
index a03119c30456..68d7bcd5fc8a 100644
--- a/drivers/clk/tegra/clk-tegra-super-cclk.c
+++ b/drivers/clk/tegra/clk-tegra-super-cclk.c
@@ -25,6 +25,8 @@
 
 #define SUPER_CDIV_ENB		BIT(31)
 
+#define TSENSOR_SLOWDOWN	BIT(23)
+
 static struct tegra_clk_super_mux *cclk_super;
 static bool cclk_on_pllx;
 
@@ -47,10 +49,20 @@ static int cclk_super_set_rate(struct clk_hw *hw, unsigned long rate,
 static unsigned long cclk_super_recalc_rate(struct clk_hw *hw,
 					    unsigned long parent_rate)
 {
+	struct tegra_clk_super_mux *super = to_clk_super_mux(hw);
+	u32 val = readl_relaxed(super->reg);
+	unsigned int div2;
+
+	/* check whether thermal throttling is active */
+	if (val & TSENSOR_SLOWDOWN)
+		div2 = 1;
+	else
+		div2 = 0;
+
 	if (cclk_super_get_parent(hw) == PLLX_INDEX)
-		return parent_rate;
+		return parent_rate >> div2;
 
-	return tegra_clk_super_ops.recalc_rate(hw, parent_rate);
+	return tegra_clk_super_ops.recalc_rate(hw, parent_rate) >> div2;
 }
 
 static int cclk_super_determine_rate(struct clk_hw *hw,
diff --git a/drivers/clk/tegra/clk-tegra30.c b/drivers/clk/tegra/clk-tegra30.c
index a33688b2359e..5b6bd138be84 100644
--- a/drivers/clk/tegra/clk-tegra30.c
+++ b/drivers/clk/tegra/clk-tegra30.c
@@ -930,7 +930,7 @@ static void __init tegra30_super_clk_init(void)
 	/* CCLKG */
 	clk = tegra_clk_register_super_cclk("cclk_g", cclk_g_parents,
 				  ARRAY_SIZE(cclk_g_parents),
-				  CLK_SET_RATE_PARENT,
+				  CLK_SET_RATE_PARENT | CLK_GET_RATE_NOCACHE,
 				  clk_base + CCLKG_BURST_POLICY,
 				  0, NULL);
 	clks[TEGRA30_CLK_CCLK_G] = clk;
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v8 7/9] clk: tegra: Mark external clocks as not having reset control
  2021-05-16 16:30 [PATCH v8 0/9] Couple improvements for Tegra clk driver Dmitry Osipenko
                   ` (5 preceding siblings ...)
  2021-05-16 16:30 ` [PATCH v8 6/9] clk: tegra: cclk: Handle thermal DIV2 CPU frequency throttling Dmitry Osipenko
@ 2021-05-16 16:30 ` Dmitry Osipenko
  2021-05-16 16:30 ` [PATCH v8 8/9] clk: tegra: Don't deassert reset on enabling clocks Dmitry Osipenko
                   ` (3 subsequent siblings)
  10 siblings, 0 replies; 17+ messages in thread
From: Dmitry Osipenko @ 2021-05-16 16:30 UTC (permalink / raw)
  To: Thierry Reding, Jonathan Hunter, Peter De Schrijver,
	Prashant Gaikwad, Michael Turquette, Stephen Boyd,
	Michał Mirosław
  Cc: Rob Herring, linux-tegra, linux-clk, linux-kernel, devicetree

The external clocks don't have reset bits as they don't belong to any
specific hardware unit. Mark them as not having reset control for
consistency.

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 drivers/clk/tegra/clk-tegra-periph.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/clk/tegra/clk-tegra-periph.c b/drivers/clk/tegra/clk-tegra-periph.c
index 60cc34f90cb9..292d6269daf1 100644
--- a/drivers/clk/tegra/clk-tegra-periph.c
+++ b/drivers/clk/tegra/clk-tegra-periph.c
@@ -712,9 +712,9 @@ static struct tegra_periph_init_data periph_clks[] = {
 	MUX8("ndflash", mux_pllp_pllc2_c_c3_pllm_clkm, CLK_SOURCE_NDFLASH, 13, TEGRA_PERIPH_ON_APB, tegra_clk_ndflash_8),
 	MUX8("ndspeed", mux_pllp_pllc2_c_c3_pllm_clkm, CLK_SOURCE_NDSPEED, 80, TEGRA_PERIPH_ON_APB, tegra_clk_ndspeed_8),
 	MUX8("hdmi", mux_pllp_pllm_plld_plla_pllc_plld2_clkm, CLK_SOURCE_HDMI, 51, 0, tegra_clk_hdmi),
-	MUX8("extern1", mux_plla_clk32_pllp_clkm_plle, CLK_SOURCE_EXTERN1, 120, 0, tegra_clk_extern1),
-	MUX8("extern2", mux_plla_clk32_pllp_clkm_plle, CLK_SOURCE_EXTERN2, 121, 0, tegra_clk_extern2),
-	MUX8("extern3", mux_plla_clk32_pllp_clkm_plle, CLK_SOURCE_EXTERN3, 122, 0, tegra_clk_extern3),
+	MUX8("extern1", mux_plla_clk32_pllp_clkm_plle, CLK_SOURCE_EXTERN1, 120, TEGRA_PERIPH_NO_RESET, tegra_clk_extern1),
+	MUX8("extern2", mux_plla_clk32_pllp_clkm_plle, CLK_SOURCE_EXTERN2, 121, TEGRA_PERIPH_NO_RESET, tegra_clk_extern2),
+	MUX8("extern3", mux_plla_clk32_pllp_clkm_plle, CLK_SOURCE_EXTERN3, 122, TEGRA_PERIPH_NO_RESET, tegra_clk_extern3),
 	MUX8("soc_therm", mux_pllm_pllc_pllp_plla, CLK_SOURCE_SOC_THERM, 78, TEGRA_PERIPH_ON_APB, tegra_clk_soc_therm),
 	MUX8("soc_therm", mux_clkm_pllc_pllp_plla, CLK_SOURCE_SOC_THERM, 78, TEGRA_PERIPH_ON_APB, tegra_clk_soc_therm_8),
 	MUX8("vi_sensor", mux_pllm_pllc2_c_c3_pllp_plla, CLK_SOURCE_VI_SENSOR, 164, TEGRA_PERIPH_NO_RESET, tegra_clk_vi_sensor_8),
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v8 8/9] clk: tegra: Don't deassert reset on enabling clocks
  2021-05-16 16:30 [PATCH v8 0/9] Couple improvements for Tegra clk driver Dmitry Osipenko
                   ` (6 preceding siblings ...)
  2021-05-16 16:30 ` [PATCH v8 7/9] clk: tegra: Mark external clocks as not having reset control Dmitry Osipenko
@ 2021-05-16 16:30 ` Dmitry Osipenko
  2021-05-16 16:30 ` [PATCH v8 9/9] dt-bindings: clock: tegra: Convert to schema Dmitry Osipenko
                   ` (2 subsequent siblings)
  10 siblings, 0 replies; 17+ messages in thread
From: Dmitry Osipenko @ 2021-05-16 16:30 UTC (permalink / raw)
  To: Thierry Reding, Jonathan Hunter, Peter De Schrijver,
	Prashant Gaikwad, Michael Turquette, Stephen Boyd,
	Michał Mirosław
  Cc: Rob Herring, linux-tegra, linux-clk, linux-kernel, devicetree

The Tegra clock driver contains legacy code which deasserts hardware reset
when peripheral clocks are enabled. This behaviour comes from a pre-CCF
era of the Tegra drivers. This is unacceptable for modern kernel drivers
which use generic CCF and reset-control APIs because it breaks assumptions
of the drivers about clk/reset sequences and about reset-propagation
delays. Hence remove the awkward legacy behaviour from the clk driver.

In particular PMC driver assumes that hardware blocks remains in reset
while power domain is turning on, but the clk driver deasserts the reset
before power clamp is removed, hence breaking the driver's assumption.

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 drivers/clk/tegra/clk-periph-gate.c | 8 --------
 drivers/clk/tegra/clk-tegra30.c     | 2 +-
 drivers/clk/tegra/clk.h             | 4 ----
 3 files changed, 1 insertion(+), 13 deletions(-)

diff --git a/drivers/clk/tegra/clk-periph-gate.c b/drivers/clk/tegra/clk-periph-gate.c
index dc3f92678407..2091fc9b0ca9 100644
--- a/drivers/clk/tegra/clk-periph-gate.c
+++ b/drivers/clk/tegra/clk-periph-gate.c
@@ -55,14 +55,6 @@ static void clk_periph_enable_locked(struct clk_hw *hw)
 	write_enb_set(periph_clk_to_bit(gate), gate);
 	udelay(2);
 
-	if (!(gate->flags & TEGRA_PERIPH_NO_RESET) &&
-	    !(gate->flags & TEGRA_PERIPH_MANUAL_RESET)) {
-		if (read_rst(gate) & periph_clk_to_bit(gate)) {
-			udelay(5); /* reset propogation delay */
-			write_rst_clr(periph_clk_to_bit(gate), gate);
-		}
-	}
-
 	if (gate->flags & TEGRA_PERIPH_WAR_1005168) {
 		writel_relaxed(0, gate->clk_base + LVL2_CLK_GATE_OVRE);
 		writel_relaxed(BIT(22), gate->clk_base + LVL2_CLK_GATE_OVRE);
diff --git a/drivers/clk/tegra/clk-tegra30.c b/drivers/clk/tegra/clk-tegra30.c
index 5b6bd138be84..64121bc66d85 100644
--- a/drivers/clk/tegra/clk-tegra30.c
+++ b/drivers/clk/tegra/clk-tegra30.c
@@ -1006,7 +1006,7 @@ static struct tegra_periph_init_data tegra_periph_clk_list[] = {
 	TEGRA_INIT_DATA_MUX("dam0", mux_pllacp_clkm, CLK_SOURCE_DAM0, 108, 0, TEGRA30_CLK_DAM0),
 	TEGRA_INIT_DATA_MUX("dam1", mux_pllacp_clkm, CLK_SOURCE_DAM1, 109, 0, TEGRA30_CLK_DAM1),
 	TEGRA_INIT_DATA_MUX("dam2", mux_pllacp_clkm, CLK_SOURCE_DAM2, 110, 0, TEGRA30_CLK_DAM2),
-	TEGRA_INIT_DATA_INT("3d2", mux_pllmcpa, CLK_SOURCE_3D2, 98, TEGRA_PERIPH_MANUAL_RESET, TEGRA30_CLK_GR3D2),
+	TEGRA_INIT_DATA_INT("3d2", mux_pllmcpa, CLK_SOURCE_3D2, 98, 0, TEGRA30_CLK_GR3D2),
 	TEGRA_INIT_DATA_INT("se", mux_pllpcm_clkm, CLK_SOURCE_SE, 127, 0, TEGRA30_CLK_SE),
 	TEGRA_INIT_DATA_MUX8("hdmi", mux_pllpmdacd2_clkm, CLK_SOURCE_HDMI, 51, 0, TEGRA30_CLK_HDMI),
 	TEGRA_INIT_DATA("pwm", NULL, NULL, pwm_parents, CLK_SOURCE_PWM, 28, 2, 0, 0, 8, 1, 0, 17, TEGRA_PERIPH_ON_APB, TEGRA30_CLK_PWM),
diff --git a/drivers/clk/tegra/clk.h b/drivers/clk/tegra/clk.h
index c3e36b5dcc75..0c3ba0ccce1a 100644
--- a/drivers/clk/tegra/clk.h
+++ b/drivers/clk/tegra/clk.h
@@ -553,9 +553,6 @@ struct tegra_clk_periph_regs {
  * Flags:
  * TEGRA_PERIPH_NO_RESET - This flag indicates that reset is not allowed
  *     for this module.
- * TEGRA_PERIPH_MANUAL_RESET - This flag indicates not to reset module
- *     after clock enable and driver for the module is responsible for
- *     doing reset.
  * TEGRA_PERIPH_ON_APB - If peripheral is in the APB bus then read the
  *     bus to flush the write operation in apb bus. This flag indicates
  *     that this peripheral is in apb bus.
@@ -577,7 +574,6 @@ struct tegra_clk_periph_gate {
 #define TEGRA_CLK_PERIPH_GATE_MAGIC 0x17760309
 
 #define TEGRA_PERIPH_NO_RESET BIT(0)
-#define TEGRA_PERIPH_MANUAL_RESET BIT(1)
 #define TEGRA_PERIPH_ON_APB BIT(2)
 #define TEGRA_PERIPH_WAR_1005168 BIT(3)
 #define TEGRA_PERIPH_NO_DIV BIT(4)
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v8 9/9] dt-bindings: clock: tegra: Convert to schema
  2021-05-16 16:30 [PATCH v8 0/9] Couple improvements for Tegra clk driver Dmitry Osipenko
                   ` (7 preceding siblings ...)
  2021-05-16 16:30 ` [PATCH v8 8/9] clk: tegra: Don't deassert reset on enabling clocks Dmitry Osipenko
@ 2021-05-16 16:30 ` Dmitry Osipenko
  2021-05-17  2:27   ` Rob Herring
  2021-05-31 13:20 ` (subset) [PATCH v8 0/9] Couple improvements for Tegra clk driver Thierry Reding
  2021-05-31 13:20 ` Thierry Reding
  10 siblings, 1 reply; 17+ messages in thread
From: Dmitry Osipenko @ 2021-05-16 16:30 UTC (permalink / raw)
  To: Thierry Reding, Jonathan Hunter, Peter De Schrijver,
	Prashant Gaikwad, Michael Turquette, Stephen Boyd,
	Michał Mirosław
  Cc: Rob Herring, linux-tegra, linux-clk, linux-kernel, devicetree

Convert NVIDIA Tegra clock bindings to schema.

Reviewed-by: Rob Herring <robh@kernel.org>
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 .../bindings/clock/nvidia,tegra114-car.txt    |  63 ----------
 .../bindings/clock/nvidia,tegra124-car.txt    | 107 ----------------
 .../bindings/clock/nvidia,tegra124-car.yaml   | 115 ++++++++++++++++++
 .../bindings/clock/nvidia,tegra20-car.txt     |  63 ----------
 .../bindings/clock/nvidia,tegra20-car.yaml    |  69 +++++++++++
 .../bindings/clock/nvidia,tegra210-car.txt    |  56 ---------
 .../bindings/clock/nvidia,tegra30-car.txt     |  63 ----------
 7 files changed, 184 insertions(+), 352 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/clock/nvidia,tegra114-car.txt
 delete mode 100644 Documentation/devicetree/bindings/clock/nvidia,tegra124-car.txt
 create mode 100644 Documentation/devicetree/bindings/clock/nvidia,tegra124-car.yaml
 delete mode 100644 Documentation/devicetree/bindings/clock/nvidia,tegra20-car.txt
 create mode 100644 Documentation/devicetree/bindings/clock/nvidia,tegra20-car.yaml
 delete mode 100644 Documentation/devicetree/bindings/clock/nvidia,tegra210-car.txt
 delete mode 100644 Documentation/devicetree/bindings/clock/nvidia,tegra30-car.txt

diff --git a/Documentation/devicetree/bindings/clock/nvidia,tegra114-car.txt b/Documentation/devicetree/bindings/clock/nvidia,tegra114-car.txt
deleted file mode 100644
index 9acea9d93160..000000000000
--- a/Documentation/devicetree/bindings/clock/nvidia,tegra114-car.txt
+++ /dev/null
@@ -1,63 +0,0 @@
-NVIDIA Tegra114 Clock And Reset Controller
-
-This binding uses the common clock binding:
-Documentation/devicetree/bindings/clock/clock-bindings.txt
-
-The CAR (Clock And Reset) Controller on Tegra is the HW module responsible
-for muxing and gating Tegra's clocks, and setting their rates.
-
-Required properties :
-- compatible : Should be "nvidia,tegra114-car"
-- reg : Should contain CAR registers location and length
-- clocks : Should contain phandle and clock specifiers for two clocks:
-  the 32 KHz "32k_in", and the board-specific oscillator "osc".
-- #clock-cells : Should be 1.
-  In clock consumers, this cell represents the clock ID exposed by the
-  CAR. The assignments may be found in header file
-  <dt-bindings/clock/tegra114-car.h>.
-- #reset-cells : Should be 1.
-  In clock consumers, this cell represents the bit number in the CAR's
-  array of CLK_RST_CONTROLLER_RST_DEVICES_* registers.
-
-Example SoC include file:
-
-/ {
-	tegra_car: clock {
-		compatible = "nvidia,tegra114-car";
-		reg = <0x60006000 0x1000>;
-		#clock-cells = <1>;
-		#reset-cells = <1>;
-	};
-
-	usb@c5004000 {
-		clocks = <&tegra_car TEGRA114_CLK_USB2>;
-	};
-};
-
-Example board file:
-
-/ {
-	clocks {
-		compatible = "simple-bus";
-		#address-cells = <1>;
-		#size-cells = <0>;
-
-		osc: clock@0 {
-			compatible = "fixed-clock";
-			reg = <0>;
-			#clock-cells = <0>;
-			clock-frequency = <12000000>;
-		};
-
-		clk_32k: clock@1 {
-			compatible = "fixed-clock";
-			reg = <1>;
-			#clock-cells = <0>;
-			clock-frequency = <32768>;
-		};
-	};
-
-	&tegra_car {
-		clocks = <&clk_32k> <&osc>;
-	};
-};
diff --git a/Documentation/devicetree/bindings/clock/nvidia,tegra124-car.txt b/Documentation/devicetree/bindings/clock/nvidia,tegra124-car.txt
deleted file mode 100644
index 7f02fb4ca4ad..000000000000
--- a/Documentation/devicetree/bindings/clock/nvidia,tegra124-car.txt
+++ /dev/null
@@ -1,107 +0,0 @@
-NVIDIA Tegra124 and Tegra132 Clock And Reset Controller
-
-This binding uses the common clock binding:
-Documentation/devicetree/bindings/clock/clock-bindings.txt
-
-The CAR (Clock And Reset) Controller on Tegra is the HW module responsible
-for muxing and gating Tegra's clocks, and setting their rates.
-
-Required properties :
-- compatible : Should be "nvidia,tegra124-car" or "nvidia,tegra132-car"
-- reg : Should contain CAR registers location and length
-- clocks : Should contain phandle and clock specifiers for two clocks:
-  the 32 KHz "32k_in", and the board-specific oscillator "osc".
-- #clock-cells : Should be 1.
-  In clock consumers, this cell represents the clock ID exposed by the
-  CAR. The assignments may be found in the header files
-  <dt-bindings/clock/tegra124-car-common.h> (which covers IDs common
-  to Tegra124 and Tegra132) and <dt-bindings/clock/tegra124-car.h>
-  (for Tegra124-specific clocks).
-- #reset-cells : Should be 1.
-  In clock consumers, this cell represents the bit number in the CAR's
-  array of CLK_RST_CONTROLLER_RST_DEVICES_* registers.
-- nvidia,external-memory-controller : phandle of the EMC driver.
-
-The node should contain a "emc-timings" subnode for each supported RAM type (see
-field RAM_CODE in register PMC_STRAPPING_OPT_A).
-
-Required properties for "emc-timings" nodes :
-- nvidia,ram-code : Should contain the value of RAM_CODE this timing set
-  is used for.
-
-Each "emc-timings" node should contain a "timing" subnode for every supported
-EMC clock rate.
-
-Required properties for "timing" nodes :
-- clock-frequency : Should contain the memory clock rate to which this timing
-relates.
-- nvidia,parent-clock-frequency : Should contain the rate at which the current
-parent of the EMC clock should be running at this timing.
-- clocks : Must contain an entry for each entry in clock-names.
-  See ../clocks/clock-bindings.txt for details.
-- clock-names : Must include the following entries:
-  - emc-parent : the clock that should be the parent of the EMC clock at this
-timing.
-
-Example SoC include file:
-
-/ {
-	tegra_car: clock@60006000 {
-		compatible = "nvidia,tegra124-car";
-		reg = <0x60006000 0x1000>;
-		#clock-cells = <1>;
-		#reset-cells = <1>;
-		nvidia,external-memory-controller = <&emc>;
-	};
-
-	usb@c5004000 {
-		clocks = <&tegra_car TEGRA124_CLK_USB2>;
-	};
-};
-
-Example board file:
-
-/ {
-	clocks {
-		compatible = "simple-bus";
-		#address-cells = <1>;
-		#size-cells = <0>;
-
-		osc: clock@0 {
-			compatible = "fixed-clock";
-			reg = <0>;
-			#clock-cells = <0>;
-			clock-frequency = <112400000>;
-		};
-
-		clk_32k: clock@1 {
-			compatible = "fixed-clock";
-			reg = <1>;
-			#clock-cells = <0>;
-			clock-frequency = <32768>;
-		};
-	};
-
-	&tegra_car {
-		clocks = <&clk_32k> <&osc>;
-	};
-
-	clock@60006000 {
-		emc-timings-3 {
-			nvidia,ram-code = <3>;
-
-			timing-12750000 {
-				clock-frequency = <12750000>;
-				nvidia,parent-clock-frequency = <408000000>;
-				clocks = <&tegra_car TEGRA124_CLK_PLL_P>;
-				clock-names = "emc-parent";
-			};
-			timing-20400000 {
-				clock-frequency = <20400000>;
-				nvidia,parent-clock-frequency = <408000000>;
-				clocks = <&tegra_car TEGRA124_CLK_PLL_P>;
-				clock-names = "emc-parent";
-			};
-		};
-	};
-};
diff --git a/Documentation/devicetree/bindings/clock/nvidia,tegra124-car.yaml b/Documentation/devicetree/bindings/clock/nvidia,tegra124-car.yaml
new file mode 100644
index 000000000000..ec7ab1483652
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/nvidia,tegra124-car.yaml
@@ -0,0 +1,115 @@
+# SPDX-License-Identifier: (GPL-2.0+ OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/clock/nvidia,tegra124-car.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: NVIDIA Tegra Clock and Reset Controller
+
+maintainers:
+  - Jon Hunter <jonathanh@nvidia.com>
+  - Thierry Reding <thierry.reding@gmail.com>
+
+description: |
+  The Clock and Reset (CAR) is the HW module responsible for muxing and gating
+  Tegra's clocks, and setting their rates. It comprises CLKGEN and RSTGEN units.
+
+  CLKGEN provides the registers to program the PLLs. It controls most of
+  the clock source programming and most of the clock dividers.
+
+  CLKGEN input signals include the external clock for the reference frequency
+  (12 MHz, 26 MHz) and the external clock for the Real Time Clock (32.768 KHz).
+
+  Outputs from CLKGEN are inputs clock of the h/w blocks in the Tegra system.
+
+  RSTGEN provides the registers needed to control resetting of each block in
+  the Tegra system.
+
+properties:
+  compatible:
+    const: nvidia,tegra124-car
+
+  reg:
+    maxItems: 1
+
+  '#clock-cells':
+    const: 1
+
+  "#reset-cells":
+    const: 1
+
+  nvidia,external-memory-controller:
+    $ref: /schemas/types.yaml#/definitions/phandle
+    description:
+      phandle of the external memory controller node
+
+patternProperties:
+  "^emc-timings-[0-9]+$":
+    type: object
+    properties:
+      nvidia,ram-code:
+        $ref: /schemas/types.yaml#/definitions/uint32
+        description:
+          value of the RAM_CODE field in the PMC_STRAPPING_OPT_A register that
+          this timing set is used for
+
+    patternProperties:
+      "^timing-[0-9]+$":
+        type: object
+        properties:
+          clock-frequency:
+            description:
+              external memory clock rate in Hz
+            minimum: 1000000
+            maximum: 1000000000
+
+          nvidia,parent-clock-frequency:
+            $ref: /schemas/types.yaml#/definitions/uint32
+            description:
+              rate of parent clock in Hz
+            minimum: 1000000
+            maximum: 1000000000
+
+          clocks:
+            items:
+              - description: parent clock of EMC
+
+          clock-names:
+            items:
+              - const: emc-parent
+
+        required:
+          - clock-frequency
+          - nvidia,parent-clock-frequency
+          - clocks
+          - clock-names
+
+        additionalProperties: false
+
+    additionalProperties: false
+
+required:
+  - compatible
+  - reg
+  - '#clock-cells'
+  - "#reset-cells"
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/tegra124-car.h>
+
+    car: clock-controller@60006000 {
+        compatible = "nvidia,tegra124-car";
+        reg = <0x60006000 0x1000>;
+        #clock-cells = <1>;
+        #reset-cells = <1>;
+    };
+
+    usb-controller@c5004000 {
+        compatible = "nvidia,tegra20-ehci";
+        reg = <0xc5004000 0x4000>;
+        clocks = <&car TEGRA124_CLK_USB2>;
+        resets = <&car TEGRA124_CLK_USB2>;
+    };
diff --git a/Documentation/devicetree/bindings/clock/nvidia,tegra20-car.txt b/Documentation/devicetree/bindings/clock/nvidia,tegra20-car.txt
deleted file mode 100644
index 6c5901b503d0..000000000000
--- a/Documentation/devicetree/bindings/clock/nvidia,tegra20-car.txt
+++ /dev/null
@@ -1,63 +0,0 @@
-NVIDIA Tegra20 Clock And Reset Controller
-
-This binding uses the common clock binding:
-Documentation/devicetree/bindings/clock/clock-bindings.txt
-
-The CAR (Clock And Reset) Controller on Tegra is the HW module responsible
-for muxing and gating Tegra's clocks, and setting their rates.
-
-Required properties :
-- compatible : Should be "nvidia,tegra20-car"
-- reg : Should contain CAR registers location and length
-- clocks : Should contain phandle and clock specifiers for two clocks:
-  the 32 KHz "32k_in", and the board-specific oscillator "osc".
-- #clock-cells : Should be 1.
-  In clock consumers, this cell represents the clock ID exposed by the
-  CAR. The assignments may be found in header file
-  <dt-bindings/clock/tegra20-car.h>.
-- #reset-cells : Should be 1.
-  In clock consumers, this cell represents the bit number in the CAR's
-  array of CLK_RST_CONTROLLER_RST_DEVICES_* registers.
-
-Example SoC include file:
-
-/ {
-	tegra_car: clock {
-		compatible = "nvidia,tegra20-car";
-		reg = <0x60006000 0x1000>;
-		#clock-cells = <1>;
-		#reset-cells = <1>;
-	};
-
-	usb@c5004000 {
-		clocks = <&tegra_car TEGRA20_CLK_USB2>;
-	};
-};
-
-Example board file:
-
-/ {
-	clocks {
-		compatible = "simple-bus";
-		#address-cells = <1>;
-		#size-cells = <0>;
-
-		osc: clock@0 {
-			compatible = "fixed-clock";
-			reg = <0>;
-			#clock-cells = <0>;
-			clock-frequency = <12000000>;
-		};
-
-		clk_32k: clock@1 {
-			compatible = "fixed-clock";
-			reg = <1>;
-			#clock-cells = <0>;
-			clock-frequency = <32768>;
-		};
-	};
-
-	&tegra_car {
-		clocks = <&clk_32k> <&osc>;
-	};
-};
diff --git a/Documentation/devicetree/bindings/clock/nvidia,tegra20-car.yaml b/Documentation/devicetree/bindings/clock/nvidia,tegra20-car.yaml
new file mode 100644
index 000000000000..459d2a525393
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/nvidia,tegra20-car.yaml
@@ -0,0 +1,69 @@
+# SPDX-License-Identifier: (GPL-2.0+ OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/clock/nvidia,tegra20-car.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: NVIDIA Tegra Clock and Reset Controller
+
+maintainers:
+  - Jon Hunter <jonathanh@nvidia.com>
+  - Thierry Reding <thierry.reding@gmail.com>
+
+description: |
+  The Clock and Reset (CAR) is the HW module responsible for muxing and gating
+  Tegra's clocks, and setting their rates. It comprises CLKGEN and RSTGEN units.
+
+  CLKGEN provides the registers to program the PLLs. It controls most of
+  the clock source programming and most of the clock dividers.
+
+  CLKGEN input signals include the external clock for the reference frequency
+  (12 MHz, 26 MHz) and the external clock for the Real Time Clock (32.768 KHz).
+
+  Outputs from CLKGEN are inputs clock of the h/w blocks in the Tegra system.
+
+  RSTGEN provides the registers needed to control resetting of each block in
+  the Tegra system.
+
+properties:
+  compatible:
+    enum:
+      - nvidia,tegra20-car
+      - nvidia,tegra30-car
+      - nvidia,tegra114-car
+      - nvidia,tegra210-car
+
+  reg:
+    maxItems: 1
+
+  '#clock-cells':
+    const: 1
+
+  "#reset-cells":
+    const: 1
+
+required:
+  - compatible
+  - reg
+  - '#clock-cells'
+  - "#reset-cells"
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/tegra20-car.h>
+
+    car: clock-controller@60006000 {
+        compatible = "nvidia,tegra20-car";
+        reg = <0x60006000 0x1000>;
+        #clock-cells = <1>;
+        #reset-cells = <1>;
+    };
+
+    usb-controller@c5004000 {
+        compatible = "nvidia,tegra20-ehci";
+        reg = <0xc5004000 0x4000>;
+        clocks = <&car TEGRA20_CLK_USB2>;
+        resets = <&car TEGRA20_CLK_USB2>;
+    };
diff --git a/Documentation/devicetree/bindings/clock/nvidia,tegra210-car.txt b/Documentation/devicetree/bindings/clock/nvidia,tegra210-car.txt
deleted file mode 100644
index 26f237f641b7..000000000000
--- a/Documentation/devicetree/bindings/clock/nvidia,tegra210-car.txt
+++ /dev/null
@@ -1,56 +0,0 @@
-NVIDIA Tegra210 Clock And Reset Controller
-
-This binding uses the common clock binding:
-Documentation/devicetree/bindings/clock/clock-bindings.txt
-
-The CAR (Clock And Reset) Controller on Tegra is the HW module responsible
-for muxing and gating Tegra's clocks, and setting their rates.
-
-Required properties :
-- compatible : Should be "nvidia,tegra210-car"
-- reg : Should contain CAR registers location and length
-- clocks : Should contain phandle and clock specifiers for two clocks:
-  the 32 KHz "32k_in".
-- #clock-cells : Should be 1.
-  In clock consumers, this cell represents the clock ID exposed by the
-  CAR. The assignments may be found in header file
-  <dt-bindings/clock/tegra210-car.h>.
-- #reset-cells : Should be 1.
-  In clock consumers, this cell represents the bit number in the CAR's
-  array of CLK_RST_CONTROLLER_RST_DEVICES_* registers.
-
-Example SoC include file:
-
-/ {
-	tegra_car: clock {
-		compatible = "nvidia,tegra210-car";
-		reg = <0x60006000 0x1000>;
-		#clock-cells = <1>;
-		#reset-cells = <1>;
-	};
-
-	usb@c5004000 {
-		clocks = <&tegra_car TEGRA210_CLK_USB2>;
-	};
-};
-
-Example board file:
-
-/ {
-	clocks {
-		compatible = "simple-bus";
-		#address-cells = <1>;
-		#size-cells = <0>;
-
-		clk_32k: clock@1 {
-			compatible = "fixed-clock";
-			reg = <1>;
-			#clock-cells = <0>;
-			clock-frequency = <32768>;
-		};
-	};
-
-	&tegra_car {
-		clocks = <&clk_32k>;
-	};
-};
diff --git a/Documentation/devicetree/bindings/clock/nvidia,tegra30-car.txt b/Documentation/devicetree/bindings/clock/nvidia,tegra30-car.txt
deleted file mode 100644
index 63618cde12df..000000000000
--- a/Documentation/devicetree/bindings/clock/nvidia,tegra30-car.txt
+++ /dev/null
@@ -1,63 +0,0 @@
-NVIDIA Tegra30 Clock And Reset Controller
-
-This binding uses the common clock binding:
-Documentation/devicetree/bindings/clock/clock-bindings.txt
-
-The CAR (Clock And Reset) Controller on Tegra is the HW module responsible
-for muxing and gating Tegra's clocks, and setting their rates.
-
-Required properties :
-- compatible : Should be "nvidia,tegra30-car"
-- reg : Should contain CAR registers location and length
-- clocks : Should contain phandle and clock specifiers for two clocks:
-  the 32 KHz "32k_in", and the board-specific oscillator "osc".
-- #clock-cells : Should be 1.
-  In clock consumers, this cell represents the clock ID exposed by the
-  CAR. The assignments may be found in header file
-  <dt-bindings/clock/tegra30-car.h>.
-- #reset-cells : Should be 1.
-  In clock consumers, this cell represents the bit number in the CAR's
-  array of CLK_RST_CONTROLLER_RST_DEVICES_* registers.
-
-Example SoC include file:
-
-/ {
-	tegra_car: clock {
-		compatible = "nvidia,tegra30-car";
-		reg = <0x60006000 0x1000>;
-		#clock-cells = <1>;
-		#reset-cells = <1>;
-	};
-
-	usb@c5004000 {
-		clocks = <&tegra_car TEGRA30_CLK_USB2>;
-	};
-};
-
-Example board file:
-
-/ {
-	clocks {
-		compatible = "simple-bus";
-		#address-cells = <1>;
-		#size-cells = <0>;
-
-		osc: clock@0 {
-			compatible = "fixed-clock";
-			reg = <0>;
-			#clock-cells = <0>;
-			clock-frequency = <12000000>;
-		};
-
-		clk_32k: clock@1 {
-			compatible = "fixed-clock";
-			reg = <1>;
-			#clock-cells = <0>;
-			clock-frequency = <32768>;
-		};
-	};
-
-	&tegra_car {
-		clocks = <&clk_32k> <&osc>;
-	};
-};
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH v8 9/9] dt-bindings: clock: tegra: Convert to schema
  2021-05-16 16:30 ` [PATCH v8 9/9] dt-bindings: clock: tegra: Convert to schema Dmitry Osipenko
@ 2021-05-17  2:27   ` Rob Herring
  0 siblings, 0 replies; 17+ messages in thread
From: Rob Herring @ 2021-05-17  2:27 UTC (permalink / raw)
  To: Dmitry Osipenko
  Cc: linux-clk, Michael Turquette, Stephen Boyd, Prashant Gaikwad,
	Rob Herring, linux-kernel, Jonathan Hunter, devicetree,
	Thierry Reding, Peter De Schrijver, Michał Mirosław,
	linux-tegra

On Sun, 16 May 2021 19:30:41 +0300, Dmitry Osipenko wrote:
> Convert NVIDIA Tegra clock bindings to schema.
> 
> Reviewed-by: Rob Herring <robh@kernel.org>
> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
> ---
>  .../bindings/clock/nvidia,tegra114-car.txt    |  63 ----------
>  .../bindings/clock/nvidia,tegra124-car.txt    | 107 ----------------
>  .../bindings/clock/nvidia,tegra124-car.yaml   | 115 ++++++++++++++++++
>  .../bindings/clock/nvidia,tegra20-car.txt     |  63 ----------
>  .../bindings/clock/nvidia,tegra20-car.yaml    |  69 +++++++++++
>  .../bindings/clock/nvidia,tegra210-car.txt    |  56 ---------
>  .../bindings/clock/nvidia,tegra30-car.txt     |  63 ----------
>  7 files changed, 184 insertions(+), 352 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/clock/nvidia,tegra114-car.txt
>  delete mode 100644 Documentation/devicetree/bindings/clock/nvidia,tegra124-car.txt
>  create mode 100644 Documentation/devicetree/bindings/clock/nvidia,tegra124-car.yaml
>  delete mode 100644 Documentation/devicetree/bindings/clock/nvidia,tegra20-car.txt
>  create mode 100644 Documentation/devicetree/bindings/clock/nvidia,tegra20-car.yaml
>  delete mode 100644 Documentation/devicetree/bindings/clock/nvidia,tegra210-car.txt
>  delete mode 100644 Documentation/devicetree/bindings/clock/nvidia,tegra30-car.txt
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
Documentation/devicetree/bindings/clock/nvidia,tegra20-car.example.dt.yaml:0:0: /example-0/usb-controller@c5004000: failed to match any schema with compatible: ['nvidia,tegra20-ehci']
Documentation/devicetree/bindings/clock/nvidia,tegra124-car.example.dt.yaml:0:0: /example-0/usb-controller@c5004000: failed to match any schema with compatible: ['nvidia,tegra20-ehci']

See https://patchwork.ozlabs.org/patch/1479105

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit.


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: (subset) [PATCH v8 0/9] Couple improvements for Tegra clk driver
  2021-05-16 16:30 [PATCH v8 0/9] Couple improvements for Tegra clk driver Dmitry Osipenko
                   ` (8 preceding siblings ...)
  2021-05-16 16:30 ` [PATCH v8 9/9] dt-bindings: clock: tegra: Convert to schema Dmitry Osipenko
@ 2021-05-31 13:20 ` Thierry Reding
  2021-05-31 13:20 ` Thierry Reding
  10 siblings, 0 replies; 17+ messages in thread
From: Thierry Reding @ 2021-05-31 13:20 UTC (permalink / raw)
  To: Peter De Schrijver, Thierry Reding, Michael Turquette,
	Jonathan Hunter, Stephen Boyd, Dmitry Osipenko,
	Michał Mirosław, Prashant Gaikwad
  Cc: linux-tegra, Rob Herring, linux-kernel, linux-clk, devicetree

From: Thierry Reding <treding@nvidia.com>

On Sun, 16 May 2021 19:30:32 +0300, Dmitry Osipenko wrote:
> This series fixes couple minor standalone problems of the Tegra clk
> driver and adds new features.
> 
> Changelog:
> 
> v8: - Replaced division with a shift, which was suggested by Michał Mirosław
>       in a comment to "Handle thermal DIV2 CPU frequency throttling" v7 patch.
>       Cortex A9 CPUs don't have hardware divider and shifting is a minor
>       improvement here, nevertheless it's good to have it.
> 
> [...]

Applied, thanks!

[1/9] clk: tegra30: Use 300MHz for video decoder by default
      (no commit info)
[2/9] clk: tegra: Fix refcounting of gate clocks
      (no commit info)
[3/9] clk: tegra: Ensure that PLLU configuration is applied properly
      (no commit info)
[4/9] clk: tegra: Halve SCLK rate on Tegra20
      (no commit info)
[5/9] clk: tegra: Don't allow zero clock rate for PLLs
      (no commit info)
[6/9] clk: tegra: cclk: Handle thermal DIV2 CPU frequency throttling
      (no commit info)
[7/9] clk: tegra: Mark external clocks as not having reset control
      (no commit info)
[8/9] clk: tegra: Don't deassert reset on enabling clocks
      (no commit info)

Best regards,
-- 
Thierry Reding <treding@nvidia.com>

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: (subset) [PATCH v8 0/9] Couple improvements for Tegra clk driver
  2021-05-16 16:30 [PATCH v8 0/9] Couple improvements for Tegra clk driver Dmitry Osipenko
                   ` (9 preceding siblings ...)
  2021-05-31 13:20 ` (subset) [PATCH v8 0/9] Couple improvements for Tegra clk driver Thierry Reding
@ 2021-05-31 13:20 ` Thierry Reding
  10 siblings, 0 replies; 17+ messages in thread
From: Thierry Reding @ 2021-05-31 13:20 UTC (permalink / raw)
  To: Stephen Boyd, Michael Turquette, Thierry Reding,
	Prashant Gaikwad, Dmitry Osipenko, Peter De Schrijver,
	Jonathan Hunter, Michał Mirosław
  Cc: linux-clk, linux-kernel, linux-tegra, Rob Herring, devicetree

From: Thierry Reding <treding@nvidia.com>

On Sun, 16 May 2021 19:30:32 +0300, Dmitry Osipenko wrote:
> This series fixes couple minor standalone problems of the Tegra clk
> driver and adds new features.
> 
> Changelog:
> 
> v8: - Replaced division with a shift, which was suggested by Michał Mirosław
>       in a comment to "Handle thermal DIV2 CPU frequency throttling" v7 patch.
>       Cortex A9 CPUs don't have hardware divider and shifting is a minor
>       improvement here, nevertheless it's good to have it.
> 
> [...]

Applied, thanks!

[9/9] dt-bindings: clock: tegra: Convert to schema
      commit: c4a41429951890d0bf7c1ef49b1fa1c8dfb1a034

Best regards,
-- 
Thierry Reding <treding@nvidia.com>

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v8 2/9] clk: tegra: Fix refcounting of gate clocks
  2021-05-16 16:30 ` [PATCH v8 2/9] clk: tegra: Fix refcounting of gate clocks Dmitry Osipenko
@ 2021-07-14 11:48   ` Jon Hunter
  2021-07-14 11:59     ` Dmitry Osipenko
  0 siblings, 1 reply; 17+ messages in thread
From: Jon Hunter @ 2021-07-14 11:48 UTC (permalink / raw)
  To: Dmitry Osipenko, Thierry Reding, Peter De Schrijver,
	Prashant Gaikwad, Michael Turquette, Stephen Boyd,
	Michał Mirosław
  Cc: Rob Herring, linux-tegra, linux-clk, linux-kernel, devicetree


On 16/05/2021 17:30, Dmitry Osipenko wrote:
> 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>
> ---
>  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);
> +}


A consequence of this change is now I see the following on Tegra210
Jetson Nano ...

[    8.138810] ------------[ cut here ]------------
[    8.150529] WARNING: CPU: 3 PID: 1 at /dvs/git/dirty/git-master_l4t-upstream/kernel/drivers/clk/tegra/clk-periph-gate.c:103 clk_periph_disable+0x68/0x90
[    8.164279] Modules linked in:
[    8.167373] CPU: 3 PID: 1 Comm: swapper/0 Not tainted 5.14.0-rc1-gb34c0e9111d0 #1
[    8.174905] Hardware name: NVIDIA Jetson Nano Developer Kit (DT)
[    8.180934] pstate: 000000c5 (nzcv daIF -PAN -UAO -TCO BTYPE=--)
[    8.186983] pc : clk_periph_disable+0x68/0x90
[    8.191388] lr : clk_periph_disable+0x20/0x90
[    8.195788] sp : ffff8000120abca0
[    8.199123] x29: ffff8000120abca0 x28: 0000000000000000 x27: ffff800011791070
[    8.206331] x26: ffff8000116e0458 x25: ffff800011fe3000 x24: ffff800011fe3000
[    8.213527] x23: ffff000080138000 x22: 0000000000000000 x21: 00000000000000c0
[    8.220725] x20: ffff0000800391b8 x19: ffff800012047000 x18: 0000000000000068
[    8.227920] x17: 0000000000000007 x16: 0000000000000001 x15: ffff800011222000
[    8.235116] x14: ffff8000120ab940 x13: ffff800011f629b8 x12: ffff000000000001
[    8.242313] x11: 0000000000000000 x10: 0000000000000000 x9 : 0000000000000000
[    8.249507] x8 : 0000000000000000 x7 : 0000000000000000 x6 : 0000000000000004
[    8.256700] x5 : ffff800011040d10 x4 : 0000000000000000 x3 : 000000000000000f
[    8.263892] x2 : ffff00008000c400 x1 : 0000000000000000 x0 : 0000000000000000
[    8.271088] Call trace:
[    8.273557]  clk_periph_disable+0x68/0x90
[    8.277615]  clk_sdmmc_mux_disable+0x1c/0x28
[    8.281924]  clk_disable_unused_subtree+0xac/0xe4
[    8.286685]  clk_disable_unused_subtree+0x3c/0xe4
[    8.291433]  clk_disable_unused_subtree+0x3c/0xe4
[    8.296182]  clk_disable_unused_subtree+0x3c/0xe4
[    8.300931]  clk_disable_unused_subtree+0x3c/0xe4
[    8.305678]  clk_disable_unused+0x5c/0xe8
[    8.309730]  do_one_initcall+0x58/0x1b8
[    8.313607]  kernel_init_freeable+0x22c/0x29c
[    8.318002]  kernel_init+0x20/0x120
[    8.321536]  ret_from_fork+0x10/0x18
[    8.325150] ---[ end trace b5b8bc7cd88ff097 ]---


Any thoughts on how to resolve this?

I now see this has been picked up for stable, but I don't see where
this was tagged for stable and so I am not sure how that happened?

Jon

-- 
nvpublic

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v8 2/9] clk: tegra: Fix refcounting of gate clocks
  2021-07-14 11:48   ` Jon Hunter
@ 2021-07-14 11:59     ` Dmitry Osipenko
  2021-07-14 15:13       ` Dmitry Osipenko
  2021-07-14 17:58       ` Jon Hunter
  0 siblings, 2 replies; 17+ messages in thread
From: Dmitry Osipenko @ 2021-07-14 11:59 UTC (permalink / raw)
  To: Jon Hunter, Thierry Reding, Peter De Schrijver, Prashant Gaikwad,
	Michael Turquette, Stephen Boyd, Michał Mirosław
  Cc: Rob Herring, linux-tegra, linux-clk, linux-kernel, devicetree

14.07.2021 14:48, Jon Hunter пишет:
> 
> On 16/05/2021 17:30, Dmitry Osipenko wrote:
>> 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>
>> ---
>>  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);
>> +}
> 
> 
> A consequence of this change is now I see the following on Tegra210
> Jetson Nano ...
> 
> [    8.138810] ------------[ cut here ]------------
> [    8.150529] WARNING: CPU: 3 PID: 1 at /dvs/git/dirty/git-master_l4t-upstream/kernel/drivers/clk/tegra/clk-periph-gate.c:103 clk_periph_disable+0x68/0x90
> [    8.164279] Modules linked in:
> [    8.167373] CPU: 3 PID: 1 Comm: swapper/0 Not tainted 5.14.0-rc1-gb34c0e9111d0 #1
> [    8.174905] Hardware name: NVIDIA Jetson Nano Developer Kit (DT)
> [    8.180934] pstate: 000000c5 (nzcv daIF -PAN -UAO -TCO BTYPE=--)
> [    8.186983] pc : clk_periph_disable+0x68/0x90
> [    8.191388] lr : clk_periph_disable+0x20/0x90
> [    8.195788] sp : ffff8000120abca0
> [    8.199123] x29: ffff8000120abca0 x28: 0000000000000000 x27: ffff800011791070
> [    8.206331] x26: ffff8000116e0458 x25: ffff800011fe3000 x24: ffff800011fe3000
> [    8.213527] x23: ffff000080138000 x22: 0000000000000000 x21: 00000000000000c0
> [    8.220725] x20: ffff0000800391b8 x19: ffff800012047000 x18: 0000000000000068
> [    8.227920] x17: 0000000000000007 x16: 0000000000000001 x15: ffff800011222000
> [    8.235116] x14: ffff8000120ab940 x13: ffff800011f629b8 x12: ffff000000000001
> [    8.242313] x11: 0000000000000000 x10: 0000000000000000 x9 : 0000000000000000
> [    8.249507] x8 : 0000000000000000 x7 : 0000000000000000 x6 : 0000000000000004
> [    8.256700] x5 : ffff800011040d10 x4 : 0000000000000000 x3 : 000000000000000f
> [    8.263892] x2 : ffff00008000c400 x1 : 0000000000000000 x0 : 0000000000000000
> [    8.271088] Call trace:
> [    8.273557]  clk_periph_disable+0x68/0x90
> [    8.277615]  clk_sdmmc_mux_disable+0x1c/0x28
> [    8.281924]  clk_disable_unused_subtree+0xac/0xe4
> [    8.286685]  clk_disable_unused_subtree+0x3c/0xe4
> [    8.291433]  clk_disable_unused_subtree+0x3c/0xe4
> [    8.296182]  clk_disable_unused_subtree+0x3c/0xe4
> [    8.300931]  clk_disable_unused_subtree+0x3c/0xe4
> [    8.305678]  clk_disable_unused+0x5c/0xe8
> [    8.309730]  do_one_initcall+0x58/0x1b8
> [    8.313607]  kernel_init_freeable+0x22c/0x29c
> [    8.318002]  kernel_init+0x20/0x120
> [    8.321536]  ret_from_fork+0x10/0x18
> [    8.325150] ---[ end trace b5b8bc7cd88ff097 ]---
> 
> 
> Any thoughts on how to resolve this?
> 
> I now see this has been picked up for stable, but I don't see where
> this was tagged for stable and so I am not sure how that happened?

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.c
b/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,
 };


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH v8 2/9] clk: tegra: Fix refcounting of gate clocks
  2021-07-14 11:59     ` Dmitry Osipenko
@ 2021-07-14 15:13       ` Dmitry Osipenko
  2021-07-14 17:58       ` Jon Hunter
  1 sibling, 0 replies; 17+ messages in thread
From: Dmitry Osipenko @ 2021-07-14 15:13 UTC (permalink / raw)
  To: Jon Hunter, Thierry Reding, Peter De Schrijver, Prashant Gaikwad,
	Michael Turquette, Stephen Boyd, Michał Mirosław
  Cc: Rob Herring, linux-tegra, linux-clk, linux-kernel, devicetree

14.07.2021 14:59, Dmitry Osipenko пишет:
> I now see this has been picked up for stable, but I don't see where
> this was tagged for stable and so I am not sure how that happened?

I don't know it was picked for stable. Maybe bot picks up all patches
that have a "fix" word in commit message.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v8 2/9] clk: tegra: Fix refcounting of gate clocks
  2021-07-14 11:59     ` Dmitry Osipenko
  2021-07-14 15:13       ` Dmitry Osipenko
@ 2021-07-14 17:58       ` Jon Hunter
  1 sibling, 0 replies; 17+ messages in thread
From: Jon Hunter @ 2021-07-14 17:58 UTC (permalink / raw)
  To: Dmitry Osipenko, Thierry Reding, Peter De Schrijver,
	Prashant Gaikwad, Michael Turquette, Stephen Boyd,
	Michał Mirosław
  Cc: Rob Herring, linux-tegra, linux-clk, linux-kernel, devicetree


On 14/07/2021 12:59, Dmitry Osipenko wrote:
> 14.07.2021 14:48, Jon Hunter пишет:
>>
>> On 16/05/2021 17:30, Dmitry Osipenko wrote:
>>> 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>

...

> 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.c
> b/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

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2021-07-14 17:59 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-16 16:30 [PATCH v8 0/9] Couple improvements for Tegra clk driver Dmitry Osipenko
2021-05-16 16:30 ` [PATCH v8 1/9] clk: tegra30: Use 300MHz for video decoder by default Dmitry Osipenko
2021-05-16 16:30 ` [PATCH v8 2/9] clk: tegra: Fix refcounting of gate clocks Dmitry Osipenko
2021-07-14 11:48   ` Jon Hunter
2021-07-14 11:59     ` Dmitry Osipenko
2021-07-14 15:13       ` Dmitry Osipenko
2021-07-14 17:58       ` Jon Hunter
2021-05-16 16:30 ` [PATCH v8 3/9] clk: tegra: Ensure that PLLU configuration is applied properly Dmitry Osipenko
2021-05-16 16:30 ` [PATCH v8 4/9] clk: tegra: Halve SCLK rate on Tegra20 Dmitry Osipenko
2021-05-16 16:30 ` [PATCH v8 5/9] clk: tegra: Don't allow zero clock rate for PLLs Dmitry Osipenko
2021-05-16 16:30 ` [PATCH v8 6/9] clk: tegra: cclk: Handle thermal DIV2 CPU frequency throttling Dmitry Osipenko
2021-05-16 16:30 ` [PATCH v8 7/9] clk: tegra: Mark external clocks as not having reset control Dmitry Osipenko
2021-05-16 16:30 ` [PATCH v8 8/9] clk: tegra: Don't deassert reset on enabling clocks Dmitry Osipenko
2021-05-16 16:30 ` [PATCH v8 9/9] dt-bindings: clock: tegra: Convert to schema Dmitry Osipenko
2021-05-17  2:27   ` Rob Herring
2021-05-31 13:20 ` (subset) [PATCH v8 0/9] Couple improvements for Tegra clk driver Thierry Reding
2021-05-31 13:20 ` Thierry Reding

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).