linux-rtc.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4 00/68] clk: Make determine_rate mandatory for muxes
@ 2023-05-05 11:25 Maxime Ripard
  2023-05-05 11:25 ` [PATCH v4 03/68] clk: Move no reparent case into a separate function Maxime Ripard
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: Maxime Ripard @ 2023-05-05 11:25 UTC (permalink / raw)
  To: Michael Turquette, Stephen Boyd
  Cc: linux-clk, Maxime Ripard, Abel Vesa, Alessandro Zummo,
	Alexandre Belloni, Alexandre Torgue, Andreas Färber,
	AngeloGioacchino Del Regno, Baolin Wang, Charles Keepax,
	Chen-Yu Tsai, Chen-Yu Tsai, Chunyan Zhang, Claudiu Beznea,
	Daniel Vetter, David Airlie, David Lechner, Dinh Nguyen,
	Fabio Estevam, Geert Uytterhoeven, Jaroslav Kysela,
	Jernej Skrabec, Jonathan Hunter, Kishon Vijay Abraham I,
	Liam Girdwood, Linus Walleij, Luca Ceresoli,
	Manivannan Sadhasivam, Mark Brown, Markus Schneider-Pargmann,
	Max Filippov, Maxime Coquelin, Mikko Perttunen, Miles Chen,
	Nicolas Ferre, Orson Zhai, Paul Cercueil, Peng Fan,
	Peter De Schrijver, Prashant Gaikwad, Richard Fitzgerald,
	Samuel Holland, Sascha Hauer, Sekhar Nori, Shawn Guo,
	Takashi Iwai, Thierry Reding, Ulf Hansson, Vinod Koul, dri-devel,
	linux-actions, linux-arm-kernel, linux-mips, linux-phy,
	linux-renesas-soc, linux-rtc, linux-stm32, linux-sunxi,
	linux-tegra, NXP Linux Team, patches, Pengutronix Kernel Team,
	Liam Beguin, Matthias Brugger, linux-mediatek, Miquel Raynal,
	Pawel Moll, alsa-devel

Hi,

This is a follow-up to a previous series that was printing a warning
when a mux has a set_parent implementation but is missing
determine_rate().

The rationale is that set_parent() is very likely to be useful when
changing the rate, but it's determine_rate() that takes the parenting
decision. If we're missing it, then the current parent is always going
to be used, and thus set_parent() will not be used. The only exception
being a direct call to clk_set_parent(), but those are fairly rare
compared to clk_set_rate().

Stephen then asked to promote the warning to an error, and to fix up all
the muxes that are in that situation first. So here it is :)

It was build-tested on x86, arm and arm64.

Affected drivers have been tracked down by the following coccinelle
script:

virtual report 

@ clk_ops @
identifier ops;
position p;
@@

 struct clk_ops ops@p = {
   ...
 };

@ has_set_parent @
identifier clk_ops.ops;
identifier set_parent_f;
@@

  struct clk_ops ops = {
	.set_parent = set_parent_f,
  };

@ has_determine_rate @
identifier clk_ops.ops;
identifier determine_rate_f;
@@

  struct clk_ops ops = {
	.determine_rate = determine_rate_f,
  };

@ script:python depends on report && has_set_parent && !has_determine_rate @
ops << clk_ops.ops;
set_parent_f << has_set_parent.set_parent_f;
p << clk_ops.p;
@@

coccilib.report.print_report(p[0], "ERROR: %s has set_parent (%s)" % (ops, set_parent_f))

Berlin is the only user still matching after this series has been
applied, but it's because it uses a composite clock which throws the
script off. The driver has been converted and shouldn't be a problem. 

Let me know what you think,
Maxime

Signed-off-by: Maxime Ripard <maxime@cerno.tech>
---
Changes in v4:
- Switch from __clk_mux_determine_rate to a new helper
- Introduced unit tests for that new helper
- Fix kunit regression
- Reworded most of the commit logs
- Link to v3: https://lore.kernel.org/r/20221018-clk-range-checks-fixes-v3-0-9a1358472d52@cerno.tech

Changes in v3:
- Rebased on top of next-20230404
- Link to v2: https://lore.kernel.org/r/20221018-clk-range-checks-fixes-v2-0-f6736dec138e@cerno.tech

Changes in v2:
- Drop all the patches already applied
- Promote the clk registration warning to an error
- Make all muxes use determine_rate
- Link to v1: https://lore.kernel.org/r/20221018-clk-range-checks-fixes-v1-0-f3ef80518140@cerno.tech

---
Maxime Ripard (66):
      clk: Export clk_hw_forward_rate_request()
      clk: test: Fix type sign of rounded rate variables
      clk: lan966x: Remove unused round_rate hook
      clk: nodrv: Add a determine_rate hook
      clk: test: Add a determine_rate hook
      clk: actions: composite: Add a determine_rate hook for pass clk
      clk: at91: main: Add a determine_rate hook
      clk: at91: sckc: Add a determine_rate hook
      clk: berlin: div: Add a determine_rate hook
      clk: cdce706: Add a determine_rate hook
      clk: k210: pll: Add a determine_rate hook
      clk: k210: aclk: Add a determine_rate hook
      clk: k210: mux: Add a determine_rate hook
      clk: lmk04832: clkout: Add a determine_rate hook
      clk: lochnagar: Add a determine_rate hook
      clk: qoriq: Add a determine_rate hook
      clk: si5341: Add a determine_rate hook
      clk: stm32f4: mux: Add a determine_rate hook
      clk: vc5: mux: Add a determine_rate hook
      clk: vc5: clkout: Add a determine_rate hook
      clk: wm831x: clkout: Add a determine_rate hook
      clk: davinci: da8xx-cfgchip: Add a determine_rate hook
      clk: davinci: da8xx-cfgchip: Add a determine_rate hook
      clk: imx: busy: Add a determine_rate hook
      clk: imx: fixup-mux: Add a determine_rate hook
      clk: imx: scu: Add a determine_rate hook
      clk: mediatek: cpumux: Add a determine_rate hook
      clk: pxa: Add a determine_rate hook
      clk: renesas: r9a06g032: Add a determine_rate hook
      clk: socfpga: gate: Add a determine_rate hook
      clk: stm32: core: Add a determine_rate hook
      clk: tegra: bpmp: Add a determine_rate hook
      clk: tegra: super: Add a determine_rate hook
      clk: tegra: periph: Add a determine_rate hook
      clk: ux500: prcmu: Add a determine_rate hook
      clk: ux500: sysctrl: Add a determine_rate hook
      clk: versatile: sp810: Add a determine_rate hook
      drm/tegra: sor: Add a determine_rate hook
      phy: cadence: sierra: Add a determine_rate hook
      phy: cadence: torrent: Add a determine_rate hook
      phy: ti: am654-serdes: Add a determine_rate hook
      phy: ti: j721e-wiz: Add a determine_rate hook
      rtc: sun6i: Add a determine_rate hook
      ASoC: tlv320aic32x4: Add a determine_rate hook
      clk: actions: composite: div: Switch to determine_rate
      clk: actions: composite: fact: Switch to determine_rate
      clk: at91: smd: Switch to determine_rate
      clk: axi-clkgen: Switch to determine_rate
      clk: cdce706: divider: Switch to determine_rate
      clk: cdce706: clkout: Switch to determine_rate
      clk: si5341: Switch to determine_rate
      clk: si5351: pll: Switch to determine_rate
      clk: si5351: msynth: Switch to determine_rate
      clk: si5351: clkout: Switch to determine_rate
      clk: da8xx: clk48: Switch to determine_rate
      clk: imx: scu: Switch to determine_rate
      clk: ingenic: cgu: Switch to determine_rate
      clk: ingenic: tcu: Switch to determine_rate
      clk: sprd: composite: Switch to determine_rate
      clk: st: flexgen: Switch to determine_rate
      clk: stm32: composite: Switch to determine_rate
      clk: tegra: periph: Switch to determine_rate
      clk: tegra: super: Switch to determine_rate
      ASoC: tlv320aic32x4: pll: Switch to determine_rate
      ASoC: tlv320aic32x4: div: Switch to determine_rate
      clk: Forbid to register a mux without determine_rate

Stephen Boyd (2):
      clk: Move no reparent case into a separate function
      clk: Introduce clk_hw_determine_rate_no_reparent()

 drivers/clk/actions/owl-composite.c       |  35 ++++--
 drivers/clk/at91/clk-main.c               |   1 +
 drivers/clk/at91/clk-smd.c                |  29 +++--
 drivers/clk/at91/sckc.c                   |   1 +
 drivers/clk/berlin/berlin2-div.c          |   1 +
 drivers/clk/clk-axi-clkgen.c              |  14 ++-
 drivers/clk/clk-cdce706.c                 |  30 ++---
 drivers/clk/clk-k210.c                    |   3 +
 drivers/clk/clk-lan966x.c                 |  17 ---
 drivers/clk/clk-lmk04832.c                |   1 +
 drivers/clk/clk-lochnagar.c               |   1 +
 drivers/clk/clk-qoriq.c                   |   1 +
 drivers/clk/clk-si5341.c                  |  19 ++--
 drivers/clk/clk-si5351.c                  |  67 ++++++-----
 drivers/clk/clk-stm32f4.c                 |   1 +
 drivers/clk/clk-versaclock5.c             |   2 +
 drivers/clk/clk-wm831x.c                  |   1 +
 drivers/clk/clk.c                         | 108 ++++++++++++------
 drivers/clk/clk_test.c                    | 180 +++++++++++++++++++++++++++++-
 drivers/clk/davinci/da8xx-cfgchip.c       |  12 +-
 drivers/clk/imx/clk-busy.c                |   1 +
 drivers/clk/imx/clk-fixup-mux.c           |   1 +
 drivers/clk/imx/clk-scu.c                 |  20 +++-
 drivers/clk/ingenic/cgu.c                 |  15 +--
 drivers/clk/ingenic/tcu.c                 |  19 ++--
 drivers/clk/mediatek/clk-cpumux.c         |   1 +
 drivers/clk/pxa/clk-pxa.c                 |   1 +
 drivers/clk/renesas/r9a06g032-clocks.c    |   1 +
 drivers/clk/socfpga/clk-gate.c            |   1 +
 drivers/clk/sprd/composite.c              |  16 ++-
 drivers/clk/st/clk-flexgen.c              |  15 +--
 drivers/clk/stm32/clk-stm32-core.c        |  33 ++++--
 drivers/clk/tegra/clk-bpmp.c              |   1 +
 drivers/clk/tegra/clk-periph.c            |  17 ++-
 drivers/clk/tegra/clk-super.c             |  16 ++-
 drivers/clk/ux500/clk-prcmu.c             |   1 +
 drivers/clk/ux500/clk-sysctrl.c           |   1 +
 drivers/clk/versatile/clk-sp810.c         |   1 +
 drivers/gpu/drm/tegra/sor.c               |   1 +
 drivers/phy/cadence/phy-cadence-sierra.c  |   1 +
 drivers/phy/cadence/phy-cadence-torrent.c |   1 +
 drivers/phy/ti/phy-am654-serdes.c         |   1 +
 drivers/phy/ti/phy-j721e-wiz.c            |   1 +
 drivers/rtc/rtc-sun6i.c                   |   1 +
 include/linux/clk-provider.h              |   2 +
 sound/soc/codecs/tlv320aic32x4-clk.c      |  33 +++---
 46 files changed, 527 insertions(+), 199 deletions(-)
---
base-commit: 145e5cddfe8b4bf607510b2dcf630d95f4db420f
change-id: 20221018-clk-range-checks-fixes-2039f3523240

Best regards,
-- 
Maxime Ripard <maxime@cerno.tech>


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

* [PATCH v4 03/68] clk: Move no reparent case into a separate function
  2023-05-05 11:25 [PATCH v4 00/68] clk: Make determine_rate mandatory for muxes Maxime Ripard
@ 2023-05-05 11:25 ` Maxime Ripard
       [not found]   ` <CGME20230613111502eucas1p2644889c9de1abfe1a14a3b549772f247@eucas1p2.samsung.com>
  2023-05-05 11:25 ` [PATCH v4 04/68] clk: Introduce clk_hw_determine_rate_no_reparent() Maxime Ripard
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 9+ messages in thread
From: Maxime Ripard @ 2023-05-05 11:25 UTC (permalink / raw)
  To: Michael Turquette, Stephen Boyd
  Cc: linux-clk, Maxime Ripard, Abel Vesa, Alessandro Zummo,
	Alexandre Belloni, Alexandre Torgue, Andreas Färber,
	AngeloGioacchino Del Regno, Baolin Wang, Charles Keepax,
	Chen-Yu Tsai, Chen-Yu Tsai, Chunyan Zhang, Claudiu Beznea,
	Daniel Vetter, David Airlie, David Lechner, Dinh Nguyen,
	Fabio Estevam, Geert Uytterhoeven, Jaroslav Kysela,
	Jernej Skrabec, Jonathan Hunter, Kishon Vijay Abraham I,
	Liam Girdwood, Linus Walleij, Luca Ceresoli,
	Manivannan Sadhasivam, Mark Brown, Markus Schneider-Pargmann,
	Max Filippov, Maxime Coquelin, Mikko Perttunen, Miles Chen,
	Nicolas Ferre, Orson Zhai, Paul Cercueil, Peng Fan,
	Peter De Schrijver, Prashant Gaikwad, Richard Fitzgerald,
	Samuel Holland, Sascha Hauer, Sekhar Nori, Shawn Guo,
	Takashi Iwai, Thierry Reding, Ulf Hansson, Vinod Koul, dri-devel,
	linux-actions, linux-arm-kernel, linux-mips, linux-phy,
	linux-renesas-soc, linux-rtc, linux-stm32, linux-sunxi,
	linux-tegra, NXP Linux Team, patches, Pengutronix Kernel Team

From: Stephen Boyd <sboyd@kernel.org>

We'll need to turn the code in clk_mux_determine_rate_flags() to deal
with CLK_SET_RATE_NO_REPARENT into a helper clock drivers will be able
to use if they don't want to allow reparenting.

Cc: Abel Vesa <abelvesa@kernel.org>
Cc: Alessandro Zummo <a.zummo@towertech.it>
Cc: Alexandre Belloni <alexandre.belloni@bootlin.com>
Cc: Alexandre Torgue <alexandre.torgue@foss.st.com>
Cc: "Andreas Färber" <afaerber@suse.de>
Cc: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Cc: Baolin Wang <baolin.wang@linux.alibaba.com>
Cc: Charles Keepax <ckeepax@opensource.cirrus.com>
Cc: Chen-Yu Tsai <wens@csie.org>
Cc: Chen-Yu Tsai <wenst@chromium.org>
Cc: Chunyan Zhang <zhang.lyra@gmail.com>
Cc: Claudiu Beznea <claudiu.beznea@microchip.com>
Cc: Daniel Vetter <daniel@ffwll.ch>
Cc: David Airlie <airlied@gmail.com>
Cc: David Lechner <david@lechnology.com>
Cc: Dinh Nguyen <dinguyen@kernel.org>
Cc: Fabio Estevam <festevam@gmail.com>
Cc: Geert Uytterhoeven <geert+renesas@glider.be>
Cc: Jaroslav Kysela <perex@perex.cz>
Cc: Jernej Skrabec <jernej.skrabec@gmail.com>
Cc: Jonathan Hunter <jonathanh@nvidia.com>
Cc: Kishon Vijay Abraham I <kishon@kernel.org>
Cc: Liam Girdwood <lgirdwood@gmail.com>
Cc: Linus Walleij <linus.walleij@linaro.org>
Cc: Luca Ceresoli <luca.ceresoli@bootlin.com>
Cc: Manivannan Sadhasivam <mani@kernel.org>
Cc: Mark Brown <broonie@kernel.org>
Cc: Markus Schneider-Pargmann <msp@baylibre.com>
Cc: Max Filippov <jcmvbkbc@gmail.com>
Cc: Maxime Coquelin <mcoquelin.stm32@gmail.com>
Cc: Mikko Perttunen <mperttunen@nvidia.com>
Cc: Miles Chen <miles.chen@mediatek.com>
Cc: Nicolas Ferre <nicolas.ferre@microchip.com>
Cc: Orson Zhai <orsonzhai@gmail.com>
Cc: Paul Cercueil <paul@crapouillou.net>
Cc: Peng Fan <peng.fan@nxp.com>
Cc: Peter De Schrijver <pdeschrijver@nvidia.com>
Cc: Prashant Gaikwad <pgaikwad@nvidia.com>
Cc: Richard Fitzgerald <rf@opensource.cirrus.com>
Cc: Samuel Holland <samuel@sholland.org>
Cc: Sascha Hauer <s.hauer@pengutronix.de>
Cc: Sekhar Nori <nsekhar@ti.com>
Cc: Shawn Guo <shawnguo@kernel.org>
Cc: Takashi Iwai <tiwai@suse.com>
Cc: Thierry Reding <thierry.reding@gmail.com>
Cc: Ulf Hansson <ulf.hansson@linaro.org>
Cc: Vinod Koul <vkoul@kernel.org>
Cc: dri-devel@lists.freedesktop.org
Cc: linux-actions@lists.infradead.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-mips@vger.kernel.org
Cc: linux-phy@lists.infradead.org
Cc: linux-renesas-soc@vger.kernel.org
Cc: linux-rtc@vger.kernel.org
Cc: linux-stm32@st-md-mailman.stormreply.com
Cc: linux-sunxi@lists.linux.dev
Cc: linux-tegra@vger.kernel.org
Cc: NXP Linux Team <linux-imx@nxp.com>
Cc: patches@opensource.cirrus.com
Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
Signed-off-by: Stephen Boyd <sboyd@kernel.org>
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
---
 drivers/clk/clk.c | 75 +++++++++++++++++++++++++++++++------------------------
 1 file changed, 43 insertions(+), 32 deletions(-)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index e495dd7a1eae..f57f821a5e5a 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -594,6 +594,46 @@ clk_core_forward_rate_req(struct clk_core *core,
 		req->max_rate = old_req->max_rate;
 }
 
+static int
+clk_core_determine_rate_no_reparent(struct clk_hw *hw,
+				    struct clk_rate_request *req)
+{
+	struct clk_core *core = hw->core;
+	struct clk_core *parent = core->parent;
+	unsigned long best;
+	int ret;
+
+	if (core->flags & CLK_SET_RATE_PARENT) {
+		struct clk_rate_request parent_req;
+
+		if (!parent) {
+			req->rate = 0;
+			return 0;
+		}
+
+		clk_core_forward_rate_req(core, req, parent, &parent_req,
+					  req->rate);
+
+		trace_clk_rate_request_start(&parent_req);
+
+		ret = clk_core_round_rate_nolock(parent, &parent_req);
+		if (ret)
+			return ret;
+
+		trace_clk_rate_request_done(&parent_req);
+
+		best = parent_req.rate;
+	} else if (parent) {
+		best = clk_core_get_rate_nolock(parent);
+	} else {
+		best = clk_core_get_rate_nolock(core);
+	}
+
+	req->rate = best;
+
+	return 0;
+}
+
 int clk_mux_determine_rate_flags(struct clk_hw *hw,
 				 struct clk_rate_request *req,
 				 unsigned long flags)
@@ -603,35 +643,8 @@ int clk_mux_determine_rate_flags(struct clk_hw *hw,
 	unsigned long best = 0;
 
 	/* if NO_REPARENT flag set, pass through to current parent */
-	if (core->flags & CLK_SET_RATE_NO_REPARENT) {
-		parent = core->parent;
-		if (core->flags & CLK_SET_RATE_PARENT) {
-			struct clk_rate_request parent_req;
-
-			if (!parent) {
-				req->rate = 0;
-				return 0;
-			}
-
-			clk_core_forward_rate_req(core, req, parent, &parent_req, req->rate);
-
-			trace_clk_rate_request_start(&parent_req);
-
-			ret = clk_core_round_rate_nolock(parent, &parent_req);
-			if (ret)
-				return ret;
-
-			trace_clk_rate_request_done(&parent_req);
-
-			best = parent_req.rate;
-		} else if (parent) {
-			best = clk_core_get_rate_nolock(parent);
-		} else {
-			best = clk_core_get_rate_nolock(core);
-		}
-
-		goto out;
-	}
+	if (core->flags & CLK_SET_RATE_NO_REPARENT)
+		return clk_core_determine_rate_no_reparent(hw, req);
 
 	/* find the parent that can provide the fastest rate <= rate */
 	num_parents = core->num_parents;
@@ -670,9 +683,7 @@ int clk_mux_determine_rate_flags(struct clk_hw *hw,
 	if (!best_parent)
 		return -EINVAL;
 
-out:
-	if (best_parent)
-		req->best_parent_hw = best_parent->hw;
+	req->best_parent_hw = best_parent->hw;
 	req->best_parent_rate = best;
 	req->rate = best;
 

-- 
2.40.0


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

* [PATCH v4 04/68] clk: Introduce clk_hw_determine_rate_no_reparent()
  2023-05-05 11:25 [PATCH v4 00/68] clk: Make determine_rate mandatory for muxes Maxime Ripard
  2023-05-05 11:25 ` [PATCH v4 03/68] clk: Move no reparent case into a separate function Maxime Ripard
@ 2023-05-05 11:25 ` Maxime Ripard
  2023-05-05 11:25 ` [PATCH v4 45/68] rtc: sun6i: Add a determine_rate hook Maxime Ripard
  2023-05-05 11:26 ` [PATCH v4 68/68] clk: Forbid to register a mux without determine_rate Maxime Ripard
  3 siblings, 0 replies; 9+ messages in thread
From: Maxime Ripard @ 2023-05-05 11:25 UTC (permalink / raw)
  To: Michael Turquette, Stephen Boyd
  Cc: linux-clk, Maxime Ripard, Abel Vesa, Alessandro Zummo,
	Alexandre Belloni, Alexandre Torgue, Andreas Färber,
	AngeloGioacchino Del Regno, Baolin Wang, Charles Keepax,
	Chen-Yu Tsai, Chen-Yu Tsai, Chunyan Zhang, Claudiu Beznea,
	Daniel Vetter, David Airlie, David Lechner, Dinh Nguyen,
	Fabio Estevam, Geert Uytterhoeven, Jaroslav Kysela,
	Jernej Skrabec, Jonathan Hunter, Kishon Vijay Abraham I,
	Liam Girdwood, Linus Walleij, Luca Ceresoli,
	Manivannan Sadhasivam, Mark Brown, Markus Schneider-Pargmann,
	Max Filippov, Maxime Coquelin, Mikko Perttunen, Miles Chen,
	Nicolas Ferre, Orson Zhai, Paul Cercueil, Peng Fan,
	Peter De Schrijver, Prashant Gaikwad, Richard Fitzgerald,
	Samuel Holland, Sascha Hauer, Sekhar Nori, Shawn Guo,
	Takashi Iwai, Thierry Reding, Ulf Hansson, Vinod Koul, dri-devel,
	linux-actions, linux-arm-kernel, linux-mips, linux-phy,
	linux-renesas-soc, linux-rtc, linux-stm32, linux-sunxi,
	linux-tegra, NXP Linux Team, patches, Pengutronix Kernel Team

From: Stephen Boyd <sboyd@kernel.org>

Some clock drivers do not want to allow any reparenting on a given
clock, but usually do so by not providing any determine_rate
implementation.

Whenever we call clk_round_rate() or clk_set_rate(), this leads to
clk_core_can_round() returning false and thus the rest of the function
either forwarding the rate request to its current parent if
CLK_SET_RATE_PARENT is set, or just returning the current clock rate.

This behaviour happens implicitly, and as we move forward to making a
determine_rate implementation required for muxes, we need some way to
explicitly opt-in for that behaviour.

Fortunately, this is exactly what the clk_core_determine_rate_no_reparent()
function is doing, so we can simply make it available to drivers.

Cc: Abel Vesa <abelvesa@kernel.org>
Cc: Alessandro Zummo <a.zummo@towertech.it>
Cc: Alexandre Belloni <alexandre.belloni@bootlin.com>
Cc: Alexandre Torgue <alexandre.torgue@foss.st.com>
Cc: "Andreas Färber" <afaerber@suse.de>
Cc: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Cc: Baolin Wang <baolin.wang@linux.alibaba.com>
Cc: Charles Keepax <ckeepax@opensource.cirrus.com>
Cc: Chen-Yu Tsai <wens@csie.org>
Cc: Chen-Yu Tsai <wenst@chromium.org>
Cc: Chunyan Zhang <zhang.lyra@gmail.com>
Cc: Claudiu Beznea <claudiu.beznea@microchip.com>
Cc: Daniel Vetter <daniel@ffwll.ch>
Cc: David Airlie <airlied@gmail.com>
Cc: David Lechner <david@lechnology.com>
Cc: Dinh Nguyen <dinguyen@kernel.org>
Cc: Fabio Estevam <festevam@gmail.com>
Cc: Geert Uytterhoeven <geert+renesas@glider.be>
Cc: Jaroslav Kysela <perex@perex.cz>
Cc: Jernej Skrabec <jernej.skrabec@gmail.com>
Cc: Jonathan Hunter <jonathanh@nvidia.com>
Cc: Kishon Vijay Abraham I <kishon@kernel.org>
Cc: Liam Girdwood <lgirdwood@gmail.com>
Cc: Linus Walleij <linus.walleij@linaro.org>
Cc: Luca Ceresoli <luca.ceresoli@bootlin.com>
Cc: Manivannan Sadhasivam <mani@kernel.org>
Cc: Mark Brown <broonie@kernel.org>
Cc: Markus Schneider-Pargmann <msp@baylibre.com>
Cc: Max Filippov <jcmvbkbc@gmail.com>
Cc: Maxime Coquelin <mcoquelin.stm32@gmail.com>
Cc: Mikko Perttunen <mperttunen@nvidia.com>
Cc: Miles Chen <miles.chen@mediatek.com>
Cc: Nicolas Ferre <nicolas.ferre@microchip.com>
Cc: Orson Zhai <orsonzhai@gmail.com>
Cc: Paul Cercueil <paul@crapouillou.net>
Cc: Peng Fan <peng.fan@nxp.com>
Cc: Peter De Schrijver <pdeschrijver@nvidia.com>
Cc: Prashant Gaikwad <pgaikwad@nvidia.com>
Cc: Richard Fitzgerald <rf@opensource.cirrus.com>
Cc: Samuel Holland <samuel@sholland.org>
Cc: Sascha Hauer <s.hauer@pengutronix.de>
Cc: Sekhar Nori <nsekhar@ti.com>
Cc: Shawn Guo <shawnguo@kernel.org>
Cc: Takashi Iwai <tiwai@suse.com>
Cc: Thierry Reding <thierry.reding@gmail.com>
Cc: Ulf Hansson <ulf.hansson@linaro.org>
Cc: Vinod Koul <vkoul@kernel.org>
Cc: dri-devel@lists.freedesktop.org
Cc: linux-actions@lists.infradead.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-mips@vger.kernel.org
Cc: linux-phy@lists.infradead.org
Cc: linux-renesas-soc@vger.kernel.org
Cc: linux-rtc@vger.kernel.org
Cc: linux-stm32@st-md-mailman.stormreply.com
Cc: linux-sunxi@lists.linux.dev
Cc: linux-tegra@vger.kernel.org
Cc: NXP Linux Team <linux-imx@nxp.com>
Cc: patches@opensource.cirrus.com
Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
Signed-off-by: Stephen Boyd <sboyd@kernel.org>
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
---
 drivers/clk/clk.c            |  18 +++++
 drivers/clk/clk_test.c       | 152 +++++++++++++++++++++++++++++++++++++++++++
 include/linux/clk-provider.h |   2 +
 3 files changed, 172 insertions(+)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index f57f821a5e5a..5365595433c8 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -783,6 +783,24 @@ int __clk_mux_determine_rate_closest(struct clk_hw *hw,
 }
 EXPORT_SYMBOL_GPL(__clk_mux_determine_rate_closest);
 
+/*
+ * clk_hw_determine_rate_no_reparent - clk_ops::determine_rate implementation for a clk that doesn't reparent
+ * @hw: mux type clk to determine rate on
+ * @req: rate request, also used to return preferred frequency
+ *
+ * Helper for finding best parent rate to provide a given frequency.
+ * This can be used directly as a determine_rate callback (e.g. for a
+ * mux), or from a more complex clock that may combine a mux with other
+ * operations.
+ *
+ * Returns: 0 on success, -EERROR value on error
+ */
+int clk_hw_determine_rate_no_reparent(struct clk_hw *hw,
+				      struct clk_rate_request *req)
+{
+	return clk_core_determine_rate_no_reparent(hw, req);
+}
+
 /***        clk api        ***/
 
 static void clk_core_rate_unprotect(struct clk_core *core)
diff --git a/drivers/clk/clk_test.c b/drivers/clk/clk_test.c
index 2cb51153750d..b3ed3b0e4c31 100644
--- a/drivers/clk/clk_test.c
+++ b/drivers/clk/clk_test.c
@@ -141,6 +141,12 @@ static const struct clk_ops clk_multiple_parents_mux_ops = {
 	.determine_rate = __clk_mux_determine_rate_closest,
 };
 
+static const struct clk_ops clk_multiple_parents_no_reparent_mux_ops = {
+	.determine_rate = clk_hw_determine_rate_no_reparent,
+	.get_parent = clk_multiple_parents_mux_get_parent,
+	.set_parent = clk_multiple_parents_mux_set_parent,
+};
+
 static int clk_test_init_with_ops(struct kunit *test, const struct clk_ops *ops)
 {
 	struct clk_dummy_context *ctx;
@@ -2395,10 +2401,156 @@ static struct kunit_suite clk_mux_notifier_test_suite = {
 	.test_cases = clk_mux_notifier_test_cases,
 };
 
+static int
+clk_mux_no_reparent_test_init(struct kunit *test)
+{
+	struct clk_multiple_parent_ctx *ctx;
+	const char *parents[2] = { "parent-0", "parent-1"};
+	int ret;
+
+	ctx = kunit_kzalloc(test, sizeof(*ctx), GFP_KERNEL);
+	if (!ctx)
+		return -ENOMEM;
+	test->priv = ctx;
+
+	ctx->parents_ctx[0].hw.init = CLK_HW_INIT_NO_PARENT("parent-0",
+							    &clk_dummy_rate_ops,
+							    0);
+	ctx->parents_ctx[0].rate = DUMMY_CLOCK_RATE_1;
+	ret = clk_hw_register(NULL, &ctx->parents_ctx[0].hw);
+	if (ret)
+		return ret;
+
+	ctx->parents_ctx[1].hw.init = CLK_HW_INIT_NO_PARENT("parent-1",
+							    &clk_dummy_rate_ops,
+							    0);
+	ctx->parents_ctx[1].rate = DUMMY_CLOCK_RATE_2;
+	ret = clk_hw_register(NULL, &ctx->parents_ctx[1].hw);
+	if (ret)
+		return ret;
+
+	ctx->current_parent = 0;
+	ctx->hw.init = CLK_HW_INIT_PARENTS("test-mux", parents,
+					   &clk_multiple_parents_no_reparent_mux_ops,
+					   0);
+	ret = clk_hw_register(NULL, &ctx->hw);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+static void
+clk_mux_no_reparent_test_exit(struct kunit *test)
+{
+	struct clk_multiple_parent_ctx *ctx = test->priv;
+
+	clk_hw_unregister(&ctx->hw);
+	clk_hw_unregister(&ctx->parents_ctx[0].hw);
+	clk_hw_unregister(&ctx->parents_ctx[1].hw);
+}
+
+/*
+ * Test that if the we have a mux that cannot change parent and we call
+ * clk_round_rate() on it with a rate that should cause it to change
+ * parent, it won't.
+ */
+static void clk_mux_no_reparent_round_rate(struct kunit *test)
+{
+	struct clk_multiple_parent_ctx *ctx = test->priv;
+	struct clk_hw *hw = &ctx->hw;
+	struct clk *clk = clk_hw_get_clk(hw, NULL);
+	struct clk *other_parent, *parent;
+	unsigned long other_parent_rate;
+	unsigned long parent_rate;
+	long rounded_rate;
+
+	parent = clk_get_parent(clk);
+	KUNIT_ASSERT_PTR_NE(test, parent, NULL);
+
+	parent_rate = clk_get_rate(parent);
+	KUNIT_ASSERT_GT(test, parent_rate, 0);
+
+	other_parent = clk_hw_get_clk(&ctx->parents_ctx[1].hw, NULL);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, other_parent);
+	KUNIT_ASSERT_FALSE(test, clk_is_match(parent, other_parent));
+
+	other_parent_rate = clk_get_rate(other_parent);
+	KUNIT_ASSERT_GT(test, other_parent_rate, 0);
+	clk_put(other_parent);
+
+	rounded_rate = clk_round_rate(clk, other_parent_rate);
+	KUNIT_ASSERT_GT(test, rounded_rate, 0);
+	KUNIT_EXPECT_EQ(test, rounded_rate, parent_rate);
+
+	clk_put(clk);
+}
+
+/*
+ * Test that if the we have a mux that cannot change parent and we call
+ * clk_set_rate() on it with a rate that should cause it to change
+ * parent, it won't.
+ */
+static void clk_mux_no_reparent_set_rate(struct kunit *test)
+{
+	struct clk_multiple_parent_ctx *ctx = test->priv;
+	struct clk_hw *hw = &ctx->hw;
+	struct clk *clk = clk_hw_get_clk(hw, NULL);
+	struct clk *other_parent, *parent;
+	unsigned long other_parent_rate;
+	unsigned long parent_rate;
+	unsigned long rate;
+	int ret;
+
+	parent = clk_get_parent(clk);
+	KUNIT_ASSERT_PTR_NE(test, parent, NULL);
+
+	parent_rate = clk_get_rate(parent);
+	KUNIT_ASSERT_GT(test, parent_rate, 0);
+
+	other_parent = clk_hw_get_clk(&ctx->parents_ctx[1].hw, NULL);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, other_parent);
+	KUNIT_ASSERT_FALSE(test, clk_is_match(parent, other_parent));
+
+	other_parent_rate = clk_get_rate(other_parent);
+	KUNIT_ASSERT_GT(test, other_parent_rate, 0);
+	clk_put(other_parent);
+
+	ret = clk_set_rate(clk, other_parent_rate);
+	KUNIT_ASSERT_EQ(test, ret, 0);
+
+	rate = clk_get_rate(clk);
+	KUNIT_ASSERT_GT(test, rate, 0);
+	KUNIT_EXPECT_EQ(test, rate, parent_rate);
+
+	clk_put(clk);
+}
+
+static struct kunit_case clk_mux_no_reparent_test_cases[] = {
+	KUNIT_CASE(clk_mux_no_reparent_round_rate),
+	KUNIT_CASE(clk_mux_no_reparent_set_rate),
+	{}
+};
+
+/*
+ * Test suite for a clock mux that isn't allowed to change parent, using
+ * the clk_hw_determine_rate_no_reparent() helper.
+ *
+ * These tests exercise that helper, and the proper selection of
+ * rates and parents.
+ */
+static struct kunit_suite clk_mux_no_reparent_test_suite = {
+	.name = "clk-mux-no-reparent",
+	.init = clk_mux_no_reparent_test_init,
+	.exit = clk_mux_no_reparent_test_exit,
+	.test_cases = clk_mux_no_reparent_test_cases,
+};
+
 kunit_test_suites(
 	&clk_leaf_mux_set_rate_parent_test_suite,
 	&clk_test_suite,
 	&clk_multiple_parents_mux_test_suite,
+	&clk_mux_no_reparent_test_suite,
 	&clk_mux_notifier_test_suite,
 	&clk_orphan_transparent_multiple_parent_mux_test_suite,
 	&clk_orphan_transparent_single_parent_test_suite,
diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
index 28ff6f1a6ada..f8f220fb5dab 100644
--- a/include/linux/clk-provider.h
+++ b/include/linux/clk-provider.h
@@ -1333,6 +1333,8 @@ int __clk_mux_determine_rate_closest(struct clk_hw *hw,
 int clk_mux_determine_rate_flags(struct clk_hw *hw,
 				 struct clk_rate_request *req,
 				 unsigned long flags);
+int clk_hw_determine_rate_no_reparent(struct clk_hw *hw,
+				      struct clk_rate_request *req);
 void clk_hw_reparent(struct clk_hw *hw, struct clk_hw *new_parent);
 void clk_hw_get_rate_range(struct clk_hw *hw, unsigned long *min_rate,
 			   unsigned long *max_rate);

-- 
2.40.0


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

* [PATCH v4 45/68] rtc: sun6i: Add a determine_rate hook
  2023-05-05 11:25 [PATCH v4 00/68] clk: Make determine_rate mandatory for muxes Maxime Ripard
  2023-05-05 11:25 ` [PATCH v4 03/68] clk: Move no reparent case into a separate function Maxime Ripard
  2023-05-05 11:25 ` [PATCH v4 04/68] clk: Introduce clk_hw_determine_rate_no_reparent() Maxime Ripard
@ 2023-05-05 11:25 ` Maxime Ripard
  2023-05-05 18:46   ` Jernej Škrabec
  2023-05-05 11:26 ` [PATCH v4 68/68] clk: Forbid to register a mux without determine_rate Maxime Ripard
  3 siblings, 1 reply; 9+ messages in thread
From: Maxime Ripard @ 2023-05-05 11:25 UTC (permalink / raw)
  To: Michael Turquette, Stephen Boyd
  Cc: linux-clk, Maxime Ripard, Alessandro Zummo, Alexandre Belloni,
	Chen-Yu Tsai, Jernej Skrabec, Samuel Holland, linux-arm-kernel,
	linux-rtc, linux-sunxi

The Allwinner sun6i RTC clock implements a mux with a set_parent hook,
but doesn't provide a determine_rate implementation.

This is a bit odd, since set_parent() is there to, as its name implies,
change the parent of a clock. However, the most likely candidates to
trigger that parent change are either the assigned-clock-parents device
tree property or a call to clk_set_rate(), with determine_rate()
figuring out which parent is the best suited for a given rate.

The other trigger would be a call to clk_set_parent(), but it's far less
used, and it doesn't look like there's any obvious user for that clock.

Similarly, it doesn't look like the device tree using that clock driver
uses any of the assigned-clock properties on that clock.

So, the set_parent hook is effectively unused, possibly because of an
oversight. However, it could also be an explicit decision by the
original author to avoid any reparenting but through an explicit call to
clk_set_parent().

The latter case would be equivalent to setting the determine_rate
implementation to clk_hw_determine_rate_no_reparent(). Indeed, if no
determine_rate implementation is provided, clk_round_rate() (through
clk_core_round_rate_nolock()) will call itself on the parent if
CLK_SET_RATE_PARENT is set, and will not change the clock rate
otherwise.

And if it was an oversight, then we are at least explicit about our
behavior now and it can be further refined down the line.

Cc: Alessandro Zummo <a.zummo@towertech.it>
Cc: Alexandre Belloni <alexandre.belloni@bootlin.com>
Cc: Chen-Yu Tsai <wens@csie.org>
Cc: Jernej Skrabec <jernej.skrabec@gmail.com>
Cc: Samuel Holland <samuel@sholland.org>
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-rtc@vger.kernel.org
Cc: linux-sunxi@lists.linux.dev
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
---
 drivers/rtc/rtc-sun6i.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/rtc/rtc-sun6i.c b/drivers/rtc/rtc-sun6i.c
index dc76537f1b62..71548dd59a3a 100644
--- a/drivers/rtc/rtc-sun6i.c
+++ b/drivers/rtc/rtc-sun6i.c
@@ -214,6 +214,7 @@ static int sun6i_rtc_osc_set_parent(struct clk_hw *hw, u8 index)
 
 static const struct clk_ops sun6i_rtc_osc_ops = {
 	.recalc_rate	= sun6i_rtc_osc_recalc_rate,
+	.determine_rate	= clk_hw_determine_rate_no_reparent,
 
 	.get_parent	= sun6i_rtc_osc_get_parent,
 	.set_parent	= sun6i_rtc_osc_set_parent,

-- 
2.40.0


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

* [PATCH v4 68/68] clk: Forbid to register a mux without determine_rate
  2023-05-05 11:25 [PATCH v4 00/68] clk: Make determine_rate mandatory for muxes Maxime Ripard
                   ` (2 preceding siblings ...)
  2023-05-05 11:25 ` [PATCH v4 45/68] rtc: sun6i: Add a determine_rate hook Maxime Ripard
@ 2023-05-05 11:26 ` Maxime Ripard
  3 siblings, 0 replies; 9+ messages in thread
From: Maxime Ripard @ 2023-05-05 11:26 UTC (permalink / raw)
  To: Michael Turquette, Stephen Boyd
  Cc: linux-clk, Maxime Ripard, Abel Vesa, Alessandro Zummo,
	Alexandre Belloni, Alexandre Torgue, Andreas Färber,
	AngeloGioacchino Del Regno, Baolin Wang, Charles Keepax,
	Chen-Yu Tsai, Chen-Yu Tsai, Chunyan Zhang, Claudiu Beznea,
	Daniel Vetter, David Airlie, David Lechner, Dinh Nguyen,
	Fabio Estevam, Geert Uytterhoeven, Jaroslav Kysela,
	Jernej Skrabec, Jonathan Hunter, Kishon Vijay Abraham I,
	Liam Girdwood, Linus Walleij, Luca Ceresoli,
	Manivannan Sadhasivam, Mark Brown, Markus Schneider-Pargmann,
	Max Filippov, Maxime Coquelin, Mikko Perttunen, Miles Chen,
	Nicolas Ferre, Orson Zhai, Paul Cercueil, Peng Fan,
	Peter De Schrijver, Prashant Gaikwad, Richard Fitzgerald,
	Samuel Holland, Sascha Hauer, Sekhar Nori, Shawn Guo,
	Takashi Iwai, Thierry Reding, Ulf Hansson, Vinod Koul, dri-devel,
	linux-actions, linux-arm-kernel, linux-mips, linux-phy,
	linux-renesas-soc, linux-rtc, linux-stm32, linux-sunxi,
	linux-tegra, NXP Linux Team, patches, Pengutronix Kernel Team

The determine_rate hook allows to select the proper parent and its rate
for a given clock configuration. On another hand, set_parent is there to
change the parent of a mux.

Some clocks provide a set_parent hook but don't implement
determine_rate. In such a case, set_parent is pretty much useless since
the clock framework will always assume the current parent is to be used,
and we will thus never change it.

This situation can be solved in two ways:
  - either we don't need to change the parent, and we thus shouldn't
    implement set_parent;
  - or we don't want to change the parent, in this case we should set
    CLK_SET_RATE_NO_REPARENT;
  - or we're missing a determine_rate implementation.

The latter is probably just an oversight from the driver's author, and
we should thus raise their awareness about the fact that the current
state of the driver is confusing.

All the drivers in-tree have been converted by now, so let's prevent any
clock with set_parent but without determine_rate to register so that it
can't sneak in again in the future.

Cc: Abel Vesa <abelvesa@kernel.org>
Cc: Alessandro Zummo <a.zummo@towertech.it>
Cc: Alexandre Belloni <alexandre.belloni@bootlin.com>
Cc: Alexandre Torgue <alexandre.torgue@foss.st.com>
Cc: "Andreas Färber" <afaerber@suse.de>
Cc: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Cc: Baolin Wang <baolin.wang@linux.alibaba.com>
Cc: Charles Keepax <ckeepax@opensource.cirrus.com>
Cc: Chen-Yu Tsai <wens@csie.org>
Cc: Chen-Yu Tsai <wenst@chromium.org>
Cc: Chunyan Zhang <zhang.lyra@gmail.com>
Cc: Claudiu Beznea <claudiu.beznea@microchip.com>
Cc: Daniel Vetter <daniel@ffwll.ch>
Cc: David Airlie <airlied@gmail.com>
Cc: David Lechner <david@lechnology.com>
Cc: Dinh Nguyen <dinguyen@kernel.org>
Cc: Fabio Estevam <festevam@gmail.com>
Cc: Geert Uytterhoeven <geert+renesas@glider.be>
Cc: Jaroslav Kysela <perex@perex.cz>
Cc: Jernej Skrabec <jernej.skrabec@gmail.com>
Cc: Jonathan Hunter <jonathanh@nvidia.com>
Cc: Kishon Vijay Abraham I <kishon@kernel.org>
Cc: Liam Girdwood <lgirdwood@gmail.com>
Cc: Linus Walleij <linus.walleij@linaro.org>
Cc: Luca Ceresoli <luca.ceresoli@bootlin.com>
Cc: Manivannan Sadhasivam <mani@kernel.org>
Cc: Mark Brown <broonie@kernel.org>
Cc: Markus Schneider-Pargmann <msp@baylibre.com>
Cc: Max Filippov <jcmvbkbc@gmail.com>
Cc: Maxime Coquelin <mcoquelin.stm32@gmail.com>
Cc: Mikko Perttunen <mperttunen@nvidia.com>
Cc: Miles Chen <miles.chen@mediatek.com>
Cc: Nicolas Ferre <nicolas.ferre@microchip.com>
Cc: Orson Zhai <orsonzhai@gmail.com>
Cc: Paul Cercueil <paul@crapouillou.net>
Cc: Peng Fan <peng.fan@nxp.com>
Cc: Peter De Schrijver <pdeschrijver@nvidia.com>
Cc: Prashant Gaikwad <pgaikwad@nvidia.com>
Cc: Richard Fitzgerald <rf@opensource.cirrus.com>
Cc: Samuel Holland <samuel@sholland.org>
Cc: Sascha Hauer <s.hauer@pengutronix.de>
Cc: Sekhar Nori <nsekhar@ti.com>
Cc: Shawn Guo <shawnguo@kernel.org>
Cc: Takashi Iwai <tiwai@suse.com>
Cc: Thierry Reding <thierry.reding@gmail.com>
Cc: Ulf Hansson <ulf.hansson@linaro.org>
Cc: Vinod Koul <vkoul@kernel.org>
Cc: dri-devel@lists.freedesktop.org
Cc: linux-actions@lists.infradead.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-mips@vger.kernel.org
Cc: linux-phy@lists.infradead.org
Cc: linux-renesas-soc@vger.kernel.org
Cc: linux-rtc@vger.kernel.org
Cc: linux-stm32@st-md-mailman.stormreply.com
Cc: linux-sunxi@lists.linux.dev
Cc: linux-tegra@vger.kernel.org
Cc: NXP Linux Team <linux-imx@nxp.com>
Cc: patches@opensource.cirrus.com
Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
---
 drivers/clk/clk.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index e4a1d5f9694c..c8f9227c29c9 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -3775,6 +3775,13 @@ static int __clk_core_init(struct clk_core *core)
 		goto out;
 	}
 
+	if (core->ops->set_parent && !core->ops->determine_rate) {
+		pr_err("%s: %s must implement .set_parent & .determine_rate\n",
+			__func__, core->name);
+		ret = -EINVAL;
+		goto out;
+	}
+
 	if (core->num_parents > 1 && !core->ops->get_parent) {
 		pr_err("%s: %s must implement .get_parent as it has multi parents\n",
 		       __func__, core->name);

-- 
2.40.0


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

* Re: [PATCH v4 45/68] rtc: sun6i: Add a determine_rate hook
  2023-05-05 11:25 ` [PATCH v4 45/68] rtc: sun6i: Add a determine_rate hook Maxime Ripard
@ 2023-05-05 18:46   ` Jernej Škrabec
  0 siblings, 0 replies; 9+ messages in thread
From: Jernej Škrabec @ 2023-05-05 18:46 UTC (permalink / raw)
  To: Michael Turquette, Stephen Boyd, Maxime Ripard
  Cc: linux-clk, Maxime Ripard, Alessandro Zummo, Alexandre Belloni,
	Chen-Yu Tsai, Samuel Holland, linux-arm-kernel, linux-rtc,
	linux-sunxi

Dne petek, 05. maj 2023 ob 13:25:47 CEST je Maxime Ripard napisal(a):
> The Allwinner sun6i RTC clock implements a mux with a set_parent hook,
> but doesn't provide a determine_rate implementation.
> 
> This is a bit odd, since set_parent() is there to, as its name implies,
> change the parent of a clock. However, the most likely candidates to
> trigger that parent change are either the assigned-clock-parents device
> tree property or a call to clk_set_rate(), with determine_rate()
> figuring out which parent is the best suited for a given rate.
> 
> The other trigger would be a call to clk_set_parent(), but it's far less
> used, and it doesn't look like there's any obvious user for that clock.
> 
> Similarly, it doesn't look like the device tree using that clock driver
> uses any of the assigned-clock properties on that clock.
> 
> So, the set_parent hook is effectively unused, possibly because of an
> oversight. However, it could also be an explicit decision by the
> original author to avoid any reparenting but through an explicit call to
> clk_set_parent().
> 
> The latter case would be equivalent to setting the determine_rate
> implementation to clk_hw_determine_rate_no_reparent(). Indeed, if no
> determine_rate implementation is provided, clk_round_rate() (through
> clk_core_round_rate_nolock()) will call itself on the parent if
> CLK_SET_RATE_PARENT is set, and will not change the clock rate
> otherwise.
> 
> And if it was an oversight, then we are at least explicit about our
> behavior now and it can be further refined down the line.
> 
> Cc: Alessandro Zummo <a.zummo@towertech.it>
> Cc: Alexandre Belloni <alexandre.belloni@bootlin.com>
> Cc: Chen-Yu Tsai <wens@csie.org>
> Cc: Jernej Skrabec <jernej.skrabec@gmail.com>
> Cc: Samuel Holland <samuel@sholland.org>
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-rtc@vger.kernel.org
> Cc: linux-sunxi@lists.linux.dev
> Signed-off-by: Maxime Ripard <maxime@cerno.tech>

Acked-by: Jernej Skrabec <jernej.skrabec@gmail.com>

Best regards,
Jernej

> ---
>  drivers/rtc/rtc-sun6i.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/rtc/rtc-sun6i.c b/drivers/rtc/rtc-sun6i.c
> index dc76537f1b62..71548dd59a3a 100644
> --- a/drivers/rtc/rtc-sun6i.c
> +++ b/drivers/rtc/rtc-sun6i.c
> @@ -214,6 +214,7 @@ static int sun6i_rtc_osc_set_parent(struct clk_hw *hw,
> u8 index)
> 
>  static const struct clk_ops sun6i_rtc_osc_ops = {
>  	.recalc_rate	= sun6i_rtc_osc_recalc_rate,
> +	.determine_rate	= clk_hw_determine_rate_no_reparent,
> 
>  	.get_parent	= sun6i_rtc_osc_get_parent,
>  	.set_parent	= sun6i_rtc_osc_set_parent,





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

* Re: [PATCH v4 03/68] clk: Move no reparent case into a separate function
       [not found]   ` <CGME20230613111502eucas1p2644889c9de1abfe1a14a3b549772f247@eucas1p2.samsung.com>
@ 2023-06-13 11:15     ` Marek Szyprowski
       [not found]       ` <CGME20230613121511eucas1p2595e0de21fadbafc1f6ffdc5636b9271@eucas1p2.samsung.com>
  0 siblings, 1 reply; 9+ messages in thread
From: Marek Szyprowski @ 2023-06-13 11:15 UTC (permalink / raw)
  To: Maxime Ripard, Michael Turquette, Stephen Boyd
  Cc: linux-clk, dri-devel, linux-actions, linux-arm-kernel,
	linux-mips, linux-phy, linux-renesas-soc, linux-rtc, linux-stm32,
	linux-sunxi, linux-tegra, NXP Linux Team,
	Pengutronix Kernel Team

On 05.05.2023 13:25, Maxime Ripard wrote:
> From: Stephen Boyd <sboyd@kernel.org>
>
> We'll need to turn the code in clk_mux_determine_rate_flags() to deal
> with CLK_SET_RATE_NO_REPARENT into a helper clock drivers will be able
> to use if they don't want to allow reparenting.
>
> Cc: Abel Vesa <abelvesa@kernel.org>
> Cc: Alessandro Zummo <a.zummo@towertech.it>
> Cc: Alexandre Belloni <alexandre.belloni@bootlin.com>
> Cc: Alexandre Torgue <alexandre.torgue@foss.st.com>
> Cc: "Andreas Färber" <afaerber@suse.de>
> Cc: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> Cc: Baolin Wang <baolin.wang@linux.alibaba.com>
> Cc: Charles Keepax <ckeepax@opensource.cirrus.com>
> Cc: Chen-Yu Tsai <wens@csie.org>
> Cc: Chen-Yu Tsai <wenst@chromium.org>
> Cc: Chunyan Zhang <zhang.lyra@gmail.com>
> Cc: Claudiu Beznea <claudiu.beznea@microchip.com>
> Cc: Daniel Vetter <daniel@ffwll.ch>
> Cc: David Airlie <airlied@gmail.com>
> Cc: David Lechner <david@lechnology.com>
> Cc: Dinh Nguyen <dinguyen@kernel.org>
> Cc: Fabio Estevam <festevam@gmail.com>
> Cc: Geert Uytterhoeven <geert+renesas@glider.be>
> Cc: Jaroslav Kysela <perex@perex.cz>
> Cc: Jernej Skrabec <jernej.skrabec@gmail.com>
> Cc: Jonathan Hunter <jonathanh@nvidia.com>
> Cc: Kishon Vijay Abraham I <kishon@kernel.org>
> Cc: Liam Girdwood <lgirdwood@gmail.com>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Cc: Luca Ceresoli <luca.ceresoli@bootlin.com>
> Cc: Manivannan Sadhasivam <mani@kernel.org>
> Cc: Mark Brown <broonie@kernel.org>
> Cc: Markus Schneider-Pargmann <msp@baylibre.com>
> Cc: Max Filippov <jcmvbkbc@gmail.com>
> Cc: Maxime Coquelin <mcoquelin.stm32@gmail.com>
> Cc: Mikko Perttunen <mperttunen@nvidia.com>
> Cc: Miles Chen <miles.chen@mediatek.com>
> Cc: Nicolas Ferre <nicolas.ferre@microchip.com>
> Cc: Orson Zhai <orsonzhai@gmail.com>
> Cc: Paul Cercueil <paul@crapouillou.net>
> Cc: Peng Fan <peng.fan@nxp.com>
> Cc: Peter De Schrijver <pdeschrijver@nvidia.com>
> Cc: Prashant Gaikwad <pgaikwad@nvidia.com>
> Cc: Richard Fitzgerald <rf@opensource.cirrus.com>
> Cc: Samuel Holland <samuel@sholland.org>
> Cc: Sascha Hauer <s.hauer@pengutronix.de>
> Cc: Sekhar Nori <nsekhar@ti.com>
> Cc: Shawn Guo <shawnguo@kernel.org>
> Cc: Takashi Iwai <tiwai@suse.com>
> Cc: Thierry Reding <thierry.reding@gmail.com>
> Cc: Ulf Hansson <ulf.hansson@linaro.org>
> Cc: Vinod Koul <vkoul@kernel.org>
> Cc: dri-devel@lists.freedesktop.org
> Cc: linux-actions@lists.infradead.org
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-mips@vger.kernel.org
> Cc: linux-phy@lists.infradead.org
> Cc: linux-renesas-soc@vger.kernel.org
> Cc: linux-rtc@vger.kernel.org
> Cc: linux-stm32@st-md-mailman.stormreply.com
> Cc: linux-sunxi@lists.linux.dev
> Cc: linux-tegra@vger.kernel.org
> Cc: NXP Linux Team <linux-imx@nxp.com>
> Cc: patches@opensource.cirrus.com
> Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
> Signed-off-by: Stephen Boyd <sboyd@kernel.org>
> Signed-off-by: Maxime Ripard <maxime@cerno.tech>
> ---

This patch landed in today's linux-next as commit 1b4e99fda73f ("clk: 
Move no reparent case into a separate function"). Unfortunately it 
causes serious regression of some of my test boards. Namely Exynos3250 
based boards are so slow after it, that my test scripts fail with a 
timeout waiting for them to finish booting. I will try to debug this 
later in the evening to check what has happened that some clocks got 
very low rate.


> drivers/clk/clk.c | 75 
> +++++++++++++++++++++++++++++++------------------------
> 1 file changed, 43 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> index e495dd7a1eae..f57f821a5e5a 100644
> --- a/drivers/clk/clk.c
> +++ b/drivers/clk/clk.c
> @@ -594,6 +594,46 @@ clk_core_forward_rate_req(struct clk_core *core,
> req->max_rate = old_req->max_rate;
> }
> +static int
> +clk_core_determine_rate_no_reparent(struct clk_hw *hw,
> + struct clk_rate_request *req)
> +{
> + struct clk_core *core = hw->core;
> + struct clk_core *parent = core->parent;
> + unsigned long best;
> + int ret;
> +
> + if (core->flags & CLK_SET_RATE_PARENT) {
> + struct clk_rate_request parent_req;
> +
> + if (!parent) {
> + req->rate = 0;
> + return 0;
> + }
> +
> + clk_core_forward_rate_req(core, req, parent, &parent_req,
> + req->rate);
> +
> + trace_clk_rate_request_start(&parent_req);
> +
> + ret = clk_core_round_rate_nolock(parent, &parent_req);
> + if (ret)
> + return ret;
> +
> + trace_clk_rate_request_done(&parent_req);
> +
> + best = parent_req.rate;
> + } else if (parent) {
> + best = clk_core_get_rate_nolock(parent);
> + } else {
> + best = clk_core_get_rate_nolock(core);
> + }
> +
> + req->rate = best;
> +
> + return 0;
> +}
> +
> int clk_mux_determine_rate_flags(struct clk_hw *hw,
> struct clk_rate_request *req,
> unsigned long flags)
> @@ -603,35 +643,8 @@ int clk_mux_determine_rate_flags(struct clk_hw *hw,
> unsigned long best = 0;
> /* if NO_REPARENT flag set, pass through to current parent */
> - if (core->flags & CLK_SET_RATE_NO_REPARENT) {
> - parent = core->parent;
> - if (core->flags & CLK_SET_RATE_PARENT) {
> - struct clk_rate_request parent_req;
> -
> - if (!parent) {
> - req->rate = 0;
> - return 0;
> - }
> -
> - clk_core_forward_rate_req(core, req, parent, &parent_req, req->rate);
> -
> - trace_clk_rate_request_start(&parent_req);
> -
> - ret = clk_core_round_rate_nolock(parent, &parent_req);
> - if (ret)
> - return ret;
> -
> - trace_clk_rate_request_done(&parent_req);
> -
> - best = parent_req.rate;
> - } else if (parent) {
> - best = clk_core_get_rate_nolock(parent);
> - } else {
> - best = clk_core_get_rate_nolock(core);
> - }
> -
> - goto out;
> - }
> + if (core->flags & CLK_SET_RATE_NO_REPARENT)
> + return clk_core_determine_rate_no_reparent(hw, req);
> /* find the parent that can provide the fastest rate <= rate */
> num_parents = core->num_parents;
> @@ -670,9 +683,7 @@ int clk_mux_determine_rate_flags(struct clk_hw *hw,
> if (!best_parent)
> return -EINVAL;
> -out:
> - if (best_parent)
> - req->best_parent_hw = best_parent->hw;
> + req->best_parent_hw = best_parent->hw;
> req->best_parent_rate = best;
> req->rate = best;
>
Best regards

-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland


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

* Re: [PATCH v4 03/68] clk: Move no reparent case into a separate function
       [not found]       ` <CGME20230613121511eucas1p2595e0de21fadbafc1f6ffdc5636b9271@eucas1p2.samsung.com>
@ 2023-06-13 12:15         ` Marek Szyprowski
  2023-06-13 12:29           ` Maxime Ripard
  0 siblings, 1 reply; 9+ messages in thread
From: Marek Szyprowski @ 2023-06-13 12:15 UTC (permalink / raw)
  To: Maxime Ripard, Michael Turquette, Stephen Boyd
  Cc: linux-clk, dri-devel, linux-actions, linux-arm-kernel,
	linux-mips, linux-phy, linux-renesas-soc, linux-rtc, linux-stm32,
	linux-sunxi, linux-tegra, NXP Linux Team,
	Pengutronix Kernel Team

On 13.06.2023 13:15, Marek Szyprowski wrote:
> On 05.05.2023 13:25, Maxime Ripard wrote:
>> From: Stephen Boyd <sboyd@kernel.org>
>>
>> We'll need to turn the code in clk_mux_determine_rate_flags() to deal
>> with CLK_SET_RATE_NO_REPARENT into a helper clock drivers will be able
>> to use if they don't want to allow reparenting.
>>
>> Cc: Abel Vesa <abelvesa@kernel.org>
>> Cc: Alessandro Zummo <a.zummo@towertech.it>
>> Cc: Alexandre Belloni <alexandre.belloni@bootlin.com>
>> Cc: Alexandre Torgue <alexandre.torgue@foss.st.com>
>> Cc: "Andreas Färber" <afaerber@suse.de>
>> Cc: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
>> Cc: Baolin Wang <baolin.wang@linux.alibaba.com>
>> Cc: Charles Keepax <ckeepax@opensource.cirrus.com>
>> Cc: Chen-Yu Tsai <wens@csie.org>
>> Cc: Chen-Yu Tsai <wenst@chromium.org>
>> Cc: Chunyan Zhang <zhang.lyra@gmail.com>
>> Cc: Claudiu Beznea <claudiu.beznea@microchip.com>
>> Cc: Daniel Vetter <daniel@ffwll.ch>
>> Cc: David Airlie <airlied@gmail.com>
>> Cc: David Lechner <david@lechnology.com>
>> Cc: Dinh Nguyen <dinguyen@kernel.org>
>> Cc: Fabio Estevam <festevam@gmail.com>
>> Cc: Geert Uytterhoeven <geert+renesas@glider.be>
>> Cc: Jaroslav Kysela <perex@perex.cz>
>> Cc: Jernej Skrabec <jernej.skrabec@gmail.com>
>> Cc: Jonathan Hunter <jonathanh@nvidia.com>
>> Cc: Kishon Vijay Abraham I <kishon@kernel.org>
>> Cc: Liam Girdwood <lgirdwood@gmail.com>
>> Cc: Linus Walleij <linus.walleij@linaro.org>
>> Cc: Luca Ceresoli <luca.ceresoli@bootlin.com>
>> Cc: Manivannan Sadhasivam <mani@kernel.org>
>> Cc: Mark Brown <broonie@kernel.org>
>> Cc: Markus Schneider-Pargmann <msp@baylibre.com>
>> Cc: Max Filippov <jcmvbkbc@gmail.com>
>> Cc: Maxime Coquelin <mcoquelin.stm32@gmail.com>
>> Cc: Mikko Perttunen <mperttunen@nvidia.com>
>> Cc: Miles Chen <miles.chen@mediatek.com>
>> Cc: Nicolas Ferre <nicolas.ferre@microchip.com>
>> Cc: Orson Zhai <orsonzhai@gmail.com>
>> Cc: Paul Cercueil <paul@crapouillou.net>
>> Cc: Peng Fan <peng.fan@nxp.com>
>> Cc: Peter De Schrijver <pdeschrijver@nvidia.com>
>> Cc: Prashant Gaikwad <pgaikwad@nvidia.com>
>> Cc: Richard Fitzgerald <rf@opensource.cirrus.com>
>> Cc: Samuel Holland <samuel@sholland.org>
>> Cc: Sascha Hauer <s.hauer@pengutronix.de>
>> Cc: Sekhar Nori <nsekhar@ti.com>
>> Cc: Shawn Guo <shawnguo@kernel.org>
>> Cc: Takashi Iwai <tiwai@suse.com>
>> Cc: Thierry Reding <thierry.reding@gmail.com>
>> Cc: Ulf Hansson <ulf.hansson@linaro.org>
>> Cc: Vinod Koul <vkoul@kernel.org>
>> Cc: dri-devel@lists.freedesktop.org
>> Cc: linux-actions@lists.infradead.org
>> Cc: linux-arm-kernel@lists.infradead.org
>> Cc: linux-mips@vger.kernel.org
>> Cc: linux-phy@lists.infradead.org
>> Cc: linux-renesas-soc@vger.kernel.org
>> Cc: linux-rtc@vger.kernel.org
>> Cc: linux-stm32@st-md-mailman.stormreply.com
>> Cc: linux-sunxi@lists.linux.dev
>> Cc: linux-tegra@vger.kernel.org
>> Cc: NXP Linux Team <linux-imx@nxp.com>
>> Cc: patches@opensource.cirrus.com
>> Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
>> Signed-off-by: Stephen Boyd <sboyd@kernel.org>
>> Signed-off-by: Maxime Ripard <maxime@cerno.tech>
>> ---
>
> This patch landed in today's linux-next as commit 1b4e99fda73f ("clk: 
> Move no reparent case into a separate function"). Unfortunately it 
> causes serious regression of some of my test boards. Namely Exynos3250 
> based boards are so slow after it, that my test scripts fail with a 
> timeout waiting for them to finish booting. I will try to debug this 
> later in the evening to check what has happened that some clocks got 
> very low rate.
>
I just got a few spare minutes, so I decided to take a look into this 
issue. The following change fixed my problem:

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index ffc9f03840b7..7ac9f7a8cb84 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -629,6 +629,7 @@ clk_core_determine_rate_no_reparent(struct clk_hw *hw,
                 best = clk_core_get_rate_nolock(core);
         }

+       req->best_parent_rate = best;
         req->rate = best;

         return 0;

best_parent_rate is still being used somewhere in the code and needs to 
be updated regardless of the CLK_SET_RATE_NO_REPARENT flag.

 > ...

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland


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

* Re: [PATCH v4 03/68] clk: Move no reparent case into a separate function
  2023-06-13 12:15         ` Marek Szyprowski
@ 2023-06-13 12:29           ` Maxime Ripard
  0 siblings, 0 replies; 9+ messages in thread
From: Maxime Ripard @ 2023-06-13 12:29 UTC (permalink / raw)
  To: Marek Szyprowski
  Cc: Michael Turquette, Stephen Boyd, linux-clk, dri-devel,
	linux-actions, linux-arm-kernel, linux-mips, linux-phy,
	linux-renesas-soc, linux-rtc, linux-stm32, linux-sunxi,
	linux-tegra, NXP Linux Team, Pengutronix Kernel Team

[-- Attachment #1: Type: text/plain, Size: 4646 bytes --]

Hi,

On Tue, Jun 13, 2023 at 02:15:10PM +0200, Marek Szyprowski wrote:
> On 13.06.2023 13:15, Marek Szyprowski wrote:
> > On 05.05.2023 13:25, Maxime Ripard wrote:
> >> From: Stephen Boyd <sboyd@kernel.org>
> >>
> >> We'll need to turn the code in clk_mux_determine_rate_flags() to deal
> >> with CLK_SET_RATE_NO_REPARENT into a helper clock drivers will be able
> >> to use if they don't want to allow reparenting.
> >>
> >> Cc: Abel Vesa <abelvesa@kernel.org>
> >> Cc: Alessandro Zummo <a.zummo@towertech.it>
> >> Cc: Alexandre Belloni <alexandre.belloni@bootlin.com>
> >> Cc: Alexandre Torgue <alexandre.torgue@foss.st.com>
> >> Cc: "Andreas Färber" <afaerber@suse.de>
> >> Cc: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> >> Cc: Baolin Wang <baolin.wang@linux.alibaba.com>
> >> Cc: Charles Keepax <ckeepax@opensource.cirrus.com>
> >> Cc: Chen-Yu Tsai <wens@csie.org>
> >> Cc: Chen-Yu Tsai <wenst@chromium.org>
> >> Cc: Chunyan Zhang <zhang.lyra@gmail.com>
> >> Cc: Claudiu Beznea <claudiu.beznea@microchip.com>
> >> Cc: Daniel Vetter <daniel@ffwll.ch>
> >> Cc: David Airlie <airlied@gmail.com>
> >> Cc: David Lechner <david@lechnology.com>
> >> Cc: Dinh Nguyen <dinguyen@kernel.org>
> >> Cc: Fabio Estevam <festevam@gmail.com>
> >> Cc: Geert Uytterhoeven <geert+renesas@glider.be>
> >> Cc: Jaroslav Kysela <perex@perex.cz>
> >> Cc: Jernej Skrabec <jernej.skrabec@gmail.com>
> >> Cc: Jonathan Hunter <jonathanh@nvidia.com>
> >> Cc: Kishon Vijay Abraham I <kishon@kernel.org>
> >> Cc: Liam Girdwood <lgirdwood@gmail.com>
> >> Cc: Linus Walleij <linus.walleij@linaro.org>
> >> Cc: Luca Ceresoli <luca.ceresoli@bootlin.com>
> >> Cc: Manivannan Sadhasivam <mani@kernel.org>
> >> Cc: Mark Brown <broonie@kernel.org>
> >> Cc: Markus Schneider-Pargmann <msp@baylibre.com>
> >> Cc: Max Filippov <jcmvbkbc@gmail.com>
> >> Cc: Maxime Coquelin <mcoquelin.stm32@gmail.com>
> >> Cc: Mikko Perttunen <mperttunen@nvidia.com>
> >> Cc: Miles Chen <miles.chen@mediatek.com>
> >> Cc: Nicolas Ferre <nicolas.ferre@microchip.com>
> >> Cc: Orson Zhai <orsonzhai@gmail.com>
> >> Cc: Paul Cercueil <paul@crapouillou.net>
> >> Cc: Peng Fan <peng.fan@nxp.com>
> >> Cc: Peter De Schrijver <pdeschrijver@nvidia.com>
> >> Cc: Prashant Gaikwad <pgaikwad@nvidia.com>
> >> Cc: Richard Fitzgerald <rf@opensource.cirrus.com>
> >> Cc: Samuel Holland <samuel@sholland.org>
> >> Cc: Sascha Hauer <s.hauer@pengutronix.de>
> >> Cc: Sekhar Nori <nsekhar@ti.com>
> >> Cc: Shawn Guo <shawnguo@kernel.org>
> >> Cc: Takashi Iwai <tiwai@suse.com>
> >> Cc: Thierry Reding <thierry.reding@gmail.com>
> >> Cc: Ulf Hansson <ulf.hansson@linaro.org>
> >> Cc: Vinod Koul <vkoul@kernel.org>
> >> Cc: dri-devel@lists.freedesktop.org
> >> Cc: linux-actions@lists.infradead.org
> >> Cc: linux-arm-kernel@lists.infradead.org
> >> Cc: linux-mips@vger.kernel.org
> >> Cc: linux-phy@lists.infradead.org
> >> Cc: linux-renesas-soc@vger.kernel.org
> >> Cc: linux-rtc@vger.kernel.org
> >> Cc: linux-stm32@st-md-mailman.stormreply.com
> >> Cc: linux-sunxi@lists.linux.dev
> >> Cc: linux-tegra@vger.kernel.org
> >> Cc: NXP Linux Team <linux-imx@nxp.com>
> >> Cc: patches@opensource.cirrus.com
> >> Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
> >> Signed-off-by: Stephen Boyd <sboyd@kernel.org>
> >> Signed-off-by: Maxime Ripard <maxime@cerno.tech>
> >> ---
> >
> > This patch landed in today's linux-next as commit 1b4e99fda73f ("clk: 
> > Move no reparent case into a separate function"). Unfortunately it 
> > causes serious regression of some of my test boards. Namely Exynos3250 
> > based boards are so slow after it, that my test scripts fail with a 
> > timeout waiting for them to finish booting. I will try to debug this 
> > later in the evening to check what has happened that some clocks got 
> > very low rate.
> >
> I just got a few spare minutes, so I decided to take a look into this 
> issue. The following change fixed my problem:
> 
> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> index ffc9f03840b7..7ac9f7a8cb84 100644
> --- a/drivers/clk/clk.c
> +++ b/drivers/clk/clk.c
> @@ -629,6 +629,7 @@ clk_core_determine_rate_no_reparent(struct clk_hw *hw,
>                  best = clk_core_get_rate_nolock(core);
>          }
> 
> +       req->best_parent_rate = best;
>          req->rate = best;
> 
>          return 0;
> 
> best_parent_rate is still being used somewhere in the code and needs to 
> be updated regardless of the CLK_SET_RATE_NO_REPARENT flag.

Yeah, that makes sense, could you send a patch?

Thanks for figuring it out!
Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

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

end of thread, other threads:[~2023-06-13 12:29 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-05-05 11:25 [PATCH v4 00/68] clk: Make determine_rate mandatory for muxes Maxime Ripard
2023-05-05 11:25 ` [PATCH v4 03/68] clk: Move no reparent case into a separate function Maxime Ripard
     [not found]   ` <CGME20230613111502eucas1p2644889c9de1abfe1a14a3b549772f247@eucas1p2.samsung.com>
2023-06-13 11:15     ` Marek Szyprowski
     [not found]       ` <CGME20230613121511eucas1p2595e0de21fadbafc1f6ffdc5636b9271@eucas1p2.samsung.com>
2023-06-13 12:15         ` Marek Szyprowski
2023-06-13 12:29           ` Maxime Ripard
2023-05-05 11:25 ` [PATCH v4 04/68] clk: Introduce clk_hw_determine_rate_no_reparent() Maxime Ripard
2023-05-05 11:25 ` [PATCH v4 45/68] rtc: sun6i: Add a determine_rate hook Maxime Ripard
2023-05-05 18:46   ` Jernej Škrabec
2023-05-05 11:26 ` [PATCH v4 68/68] clk: Forbid to register a mux without determine_rate Maxime Ripard

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