[PATCH 0/8] Fix struct clk pointer comparing

[PATCH 0/8] Fix struct clk pointer comparing

[Shawn Guo]

On the first day back from Chinese new year holiday, I got a regression
report from rmk, saying Ethernet stops working on HimmingBoard with
v4.0-rc1.

I read through the thread [1] and found a couple of i.MX audio drivers
are also affected per Stephen's Coccinelle report.  That's why I came up
with this series based on Quentin's structclk.cocci, Stephen's result
and Mike's input (thanks all).

The first two patches are run-time tested and all others are
compile-tested only.

[1] http://thread.gmane.org/gmane.linux.kernel/1872604/focus=1880862

Shawn Guo (8):
  clk: add helper function clk_is_match()
  ARM: imx: fix struct clk pointer comparing
  drm: armada: fix struct clk pointer comparing
  pwm: atmel-hlcdc: fix struct clk pointer comparing
  serial: samsung: fix struct clk pointer comparing
  ASoC: fsl_esai: fix struct clk pointer comparing
  ASoC: fsl_spdif: fix struct clk pointer comparing
  ASoC: kirkwood: fix struct clk pointer comparing

 arch/arm/mach-imx/mach-imx6q.c      |  5 +++--
 drivers/clk/clk.c                   |  6 ++++++
 drivers/gpu/drm/armada/armada_510.c |  2 +-
 drivers/pwm/pwm-atmel-hlcdc.c       |  2 +-
 drivers/tty/serial/samsung.c        |  2 +-
 include/linux/clk.h                 | 16 ++++++++++++++++
 sound/soc/fsl/fsl_esai.c            |  2 +-
 sound/soc/fsl/fsl_spdif.c           |  4 ++--
 sound/soc/kirkwood/kirkwood-i2s.c   |  2 +-
 9 files changed, 32 insertions(+), 9 deletions(-)

-- 
1.9.1

[PATCH 1/8] clk: add helper function clk_is_match()

[Shawn Guo]

Since commit 035a61c314eb ("clk: Make clk API return per-user struct clk
instances"), clk API users can no longer check if two struct clk
pointers are pointing to the same hardware clock, i.e. struct clk_hw, by
simply comparing two pointers.  That's because with the per-user clk
change, a brand new struct clk is created whenever clients try to look
up the clock by calling clk_get() or sister functions like clk_get_sys()
and of_clk_get().  This changes the original behavior where the struct
clk is only created for once when clock driver registers the clock to
CCF in the first place.  The net change here is before commit
035a61c314eb the struct clk pointer is unique for given hardware
clock, while after the commit the pointers returned by clk lookup calls
become different for the same hardware clock.

A number of client drivers detecting if two struct clk pointers point to
the same one hardware clock by comparing the pointers are broken now.
As a stop-gap solution, this patch adds a helper function clk_is_match()
to test if two struct clk pointers point to the same hardware clock, so
that these client drivers can use to fix the regression.

Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
---
 drivers/clk/clk.c   |  6 ++++++
 include/linux/clk.h | 16 ++++++++++++++++
 2 files changed, 22 insertions(+)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index eb0152961d3c..7df4826d9c7b 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -597,6 +597,12 @@ struct clk *__clk_get_parent(struct clk *clk)
 }
 EXPORT_SYMBOL_GPL(__clk_get_parent);
 
+bool clk_is_match(struct clk *clk1, struct clk *clk2)
+{
+	return __clk_get_hw(clk1) == __clk_get_hw(clk2) ? true : false;
+}
+EXPORT_SYMBOL_GPL(clk_is_match);
+
 static struct clk_core *clk_core_get_parent_by_index(struct clk_core *clk,
 							 u8 index)
 {
diff --git a/include/linux/clk.h b/include/linux/clk.h
index 8381bbfbc308..f8a8a6dd6a98 100644
--- a/include/linux/clk.h
+++ b/include/linux/clk.h
@@ -376,6 +376,17 @@ struct clk *clk_get_parent(struct clk *clk);
  */
 struct clk *clk_get_sys(const char *dev_id, const char *con_id);
 
+/**
+ * clk_is_match - test if two given struct clk pointers point to the same
+ * 		  hardware clock, i.e. struct clk_hw.
+ * @clk1: the first clk
+ * @clk2: the second clk
+ *
+ * Returns true if two clk pointers point to the same hardware clock,
+ * or false otherwise.
+ */
+bool clk_is_match(struct clk *clk1, struct clk *clk2);
+
 #else /* !CONFIG_HAVE_CLK */
 
 static inline struct clk *clk_get(struct device *dev, const char *id)
@@ -429,6 +440,11 @@ static inline struct clk *clk_get_parent(struct clk *clk)
 	return NULL;
 }
 
+static inline bool clk_is_match(struct clk *clk1, struct clk *clk2)
+{
+	return clk1 == clk2 ? true : false;
+}
+
 #endif
 
 /* clk_prepare_enable helps cases using clk_enable in non-atomic context. */
-- 
1.9.1

[PATCH 2/8] ARM: imx: fix struct clk pointer comparing

[Shawn Guo]

Since commit 035a61c314eb ("clk: Make clk API return per-user struct clk
instances"), clk API users can no longer check if two struct clk
pointers are pointing to the same hardware clock, i.e. struct clk_hw, by
simply comparing two pointers.  That's because with the per-user clk
change, a brand new struct clk is created whenever clients try to look
up the clock by calling clk_get() or sister functions like clk_get_sys()
and of_clk_get().  This changes the original behavior where the struct
clk is only created for once when clock driver registers the clock to
CCF in the first place.  The net change here is before commit
035a61c314eb the struct clk pointer is unique for given hardware
clock, while after the commit the pointers returned by clk lookup calls
become different for the same hardware clock.

That said, the struct clk pointer comparing in the code doesn't work any
more.  Call helper function clk_is_match() instead to fix the problem.

Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
---
 arch/arm/mach-imx/mach-imx6q.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-imx/mach-imx6q.c b/arch/arm/mach-imx/mach-imx6q.c
index 4ad6e473cf83..9de3412af406 100644
--- a/arch/arm/mach-imx/mach-imx6q.c
+++ b/arch/arm/mach-imx/mach-imx6q.c
@@ -211,8 +211,9 @@ static void __init imx6q_1588_init(void)
 	 * set bit IOMUXC_GPR1[21].  Or the PTP clock must be from pad
 	 * (external OSC), and we need to clear the bit.
 	 */
-	clksel = ptp_clk == enet_ref ? IMX6Q_GPR1_ENET_CLK_SEL_ANATOP :
-				       IMX6Q_GPR1_ENET_CLK_SEL_PAD;
+	clksel = clk_is_match(ptp_clk, enet_ref) ?
+				IMX6Q_GPR1_ENET_CLK_SEL_ANATOP :
+				IMX6Q_GPR1_ENET_CLK_SEL_PAD;
 	gpr = syscon_regmap_lookup_by_compatible("fsl,imx6q-iomuxc-gpr");
 	if (!IS_ERR(gpr))
 		regmap_update_bits(gpr, IOMUXC_GPR1,
-- 
1.9.1

[PATCH 3/8] drm: armada: fix struct clk pointer comparing

[Shawn Guo]

Since commit 035a61c314eb ("clk: Make clk API return per-user struct clk
instances"), clk API users can no longer check if two struct clk
pointers are pointing to the same hardware clock, i.e. struct clk_hw, by
simply comparing two pointers.  That's because with the per-user clk
change, a brand new struct clk is created whenever clients try to look
up the clock by calling clk_get() or sister functions like clk_get_sys()
and of_clk_get().  This changes the original behavior where the struct
clk is only created for once when clock driver registers the clock to
CCF in the first place.  The net change here is before commit
035a61c314eb the struct clk pointer is unique for given hardware
clock, while after the commit the pointers returned by clk lookup calls
become different for the same hardware clock.

That said, the struct clk pointer comparing in the code doesn't work any
more.  Call helper function clk_is_match() instead to fix the problem.

Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
---
 drivers/gpu/drm/armada/armada_510.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/armada/armada_510.c b/drivers/gpu/drm/armada/armada_510.c
index ad3d2ebf95c9..862deafe8b24 100644
--- a/drivers/gpu/drm/armada/armada_510.c
+++ b/drivers/gpu/drm/armada/armada_510.c
@@ -53,7 +53,7 @@ static int armada510_crtc_compute_clock(struct armada_crtc *dcrtc,
 	if (IS_ERR(clk))
 		return PTR_ERR(clk);
 
-	if (dcrtc->clk != clk) {
+	if (!clk_is_match(dcrtc->clk, clk)) {
 		ret = clk_prepare_enable(clk);
 		if (ret)
 			return ret;
-- 
1.9.1

[PATCH 4/8] pwm: atmel-hlcdc: fix struct clk pointer comparing

[Shawn Guo]

Since commit 035a61c314eb ("clk: Make clk API return per-user struct clk
instances"), clk API users can no longer check if two struct clk
pointers are pointing to the same hardware clock, i.e. struct clk_hw, by
simply comparing two pointers.  That's because with the per-user clk
change, a brand new struct clk is created whenever clients try to look
up the clock by calling clk_get() or sister functions like clk_get_sys()
and of_clk_get().  This changes the original behavior where the struct
clk is only created for once when clock driver registers the clock to
CCF in the first place.  The net change here is before commit
035a61c314eb the struct clk pointer is unique for given hardware
clock, while after the commit the pointers returned by clk lookup calls
become different for the same hardware clock.

That said, the struct clk pointer comparing in the code doesn't work any
more.  Call helper function clk_is_match() instead to fix the problem.

Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
---
 drivers/pwm/pwm-atmel-hlcdc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pwm/pwm-atmel-hlcdc.c b/drivers/pwm/pwm-atmel-hlcdc.c
index 522f7075bb1a..36475949b829 100644
--- a/drivers/pwm/pwm-atmel-hlcdc.c
+++ b/drivers/pwm/pwm-atmel-hlcdc.c
@@ -97,7 +97,7 @@ static int atmel_hlcdc_pwm_config(struct pwm_chip *c,
 
 	pwmcfg = ATMEL_HLCDC_PWMPS(pres);
 
-	if (new_clk != chip->cur_clk) {
+	if (!clk_is_match(new_clk, chip->cur_clk)) {
 		u32 gencfg = 0;
 		int ret;
 
-- 
1.9.1

[PATCH 5/8] serial: samsung: fix struct clk pointer comparing

[Shawn Guo]

Since commit 035a61c314eb ("clk: Make clk API return per-user struct clk
instances"), clk API users can no longer check if two struct clk
pointers are pointing to the same hardware clock, i.e. struct clk_hw, by
simply comparing two pointers.  That's because with the per-user clk
change, a brand new struct clk is created whenever clients try to look
up the clock by calling clk_get() or sister functions like clk_get_sys()
and of_clk_get().  This changes the original behavior where the struct
clk is only created for once when clock driver registers the clock to
CCF in the first place.  The net change here is before commit
035a61c314eb the struct clk pointer is unique for given hardware
clock, while after the commit the pointers returned by clk lookup calls
become different for the same hardware clock.

That said, the struct clk pointer comparing in the code doesn't work any
more.  Call helper function clk_is_match() instead to fix the problem.

Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
---
 drivers/tty/serial/samsung.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/tty/serial/samsung.c b/drivers/tty/serial/samsung.c
index af821a908720..b747066247df 100644
--- a/drivers/tty/serial/samsung.c
+++ b/drivers/tty/serial/samsung.c
@@ -1277,7 +1277,7 @@ static void s3c24xx_serial_set_termios(struct uart_port *port,
 
 	/* check to see if we need  to change clock source */
 
-	if (ourport->baudclk != clk) {
+	if (!clk_is_match(ourport->baudclk, clk)) {
 		s3c24xx_serial_setsource(port, clk_sel);
 
 		if (!IS_ERR(ourport->baudclk)) {
-- 
1.9.1

[PATCH 6/8] ASoC: fsl_esai: fix struct clk pointer comparing

[Shawn Guo]

Since commit 035a61c314eb ("clk: Make clk API return per-user struct clk
instances"), clk API users can no longer check if two struct clk
pointers are pointing to the same hardware clock, i.e. struct clk_hw, by
simply comparing two pointers.  That's because with the per-user clk
change, a brand new struct clk is created whenever clients try to look
up the clock by calling clk_get() or sister functions like clk_get_sys()
and of_clk_get().  This changes the original behavior where the struct
clk is only created for once when clock driver registers the clock to
CCF in the first place.  The net change here is before commit
035a61c314eb the struct clk pointer is unique for given hardware
clock, while after the commit the pointers returned by clk lookup calls
become different for the same hardware clock.

That said, the struct clk pointer comparing in the code doesn't work any
more.  Call helper function clk_is_match() instead to fix the problem.

Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
---
 sound/soc/fsl/fsl_esai.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/soc/fsl/fsl_esai.c b/sound/soc/fsl/fsl_esai.c
index 5c7597191e3f..6e00e51b98c2 100644
--- a/sound/soc/fsl/fsl_esai.c
+++ b/sound/soc/fsl/fsl_esai.c
@@ -269,7 +269,7 @@ static int fsl_esai_set_dai_sysclk(struct snd_soc_dai *dai, int clk_id,
 	}
 
 	/* Only EXTAL source can be output directly without using PSR and PM */
-	if (ratio == 1 && clksrc == esai_priv->extalclk) {
+	if (ratio == 1 && clk_is_match(clksrc, esai_priv->extalclk)) {
 		/* Bypass all the dividers if not being needed */
 		ecr |= tx ? ESAI_ECR_ETO : ESAI_ECR_ERO;
 		goto out;
-- 
1.9.1

[PATCH 7/8] ASoC: fsl_spdif: fix struct clk pointer comparing

[Shawn Guo]

Since commit 035a61c314eb ("clk: Make clk API return per-user struct clk
instances"), clk API users can no longer check if two struct clk
pointers are pointing to the same hardware clock, i.e. struct clk_hw, by
simply comparing two pointers.  That's because with the per-user clk
change, a brand new struct clk is created whenever clients try to look
up the clock by calling clk_get() or sister functions like clk_get_sys()
and of_clk_get().  This changes the original behavior where the struct
clk is only created for once when clock driver registers the clock to
CCF in the first place.  The net change here is before commit
035a61c314eb the struct clk pointer is unique for given hardware
clock, while after the commit the pointers returned by clk lookup calls
become different for the same hardware clock.

That said, the struct clk pointer comparing in the code doesn't work any
more.  Call helper function clk_is_match() instead to fix the problem.

Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
---
 sound/soc/fsl/fsl_spdif.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/sound/soc/fsl/fsl_spdif.c b/sound/soc/fsl/fsl_spdif.c
index 75870c0ea2c9..91eb3aef7f02 100644
--- a/sound/soc/fsl/fsl_spdif.c
+++ b/sound/soc/fsl/fsl_spdif.c
@@ -1049,7 +1049,7 @@ static u32 fsl_spdif_txclk_caldiv(struct fsl_spdif_priv *spdif_priv,
 				enum spdif_txrate index, bool round)
 {
 	const u32 rate[] = { 32000, 44100, 48000, 96000, 192000 };
-	bool is_sysclk = clk == spdif_priv->sysclk;
+	bool is_sysclk = clk_is_match(clk, spdif_priv->sysclk);
 	u64 rate_ideal, rate_actual, sub;
 	u32 sysclk_dfmin, sysclk_dfmax;
 	u32 txclk_df, sysclk_df, arate;
@@ -1143,7 +1143,7 @@ static int fsl_spdif_probe_txclk(struct fsl_spdif_priv *spdif_priv,
 			spdif_priv->txclk_src[index], rate[index]);
 	dev_dbg(&pdev->dev, "use txclk df %d for %dHz sample rate\n",
 			spdif_priv->txclk_df[index], rate[index]);
-	if (spdif_priv->txclk[index] == spdif_priv->sysclk)
+	if (clk_is_match(spdif_priv->txclk[index], spdif_priv->sysclk))
 		dev_dbg(&pdev->dev, "use sysclk df %d for %dHz sample rate\n",
 				spdif_priv->sysclk_df[index], rate[index]);
 	dev_dbg(&pdev->dev, "the best rate for %dHz sample rate is %dHz\n",
-- 
1.9.1

[PATCH 8/8] ASoC: kirkwood: fix struct clk pointer comparing

[Shawn Guo]

Since commit 035a61c314eb ("clk: Make clk API return per-user struct clk
instances"), clk API users can no longer check if two struct clk
pointers are pointing to the same hardware clock, i.e. struct clk_hw, by
simply comparing two pointers.  That's because with the per-user clk
change, a brand new struct clk is created whenever clients try to look
up the clock by calling clk_get() or sister functions like clk_get_sys()
and of_clk_get().  This changes the original behavior where the struct
clk is only created for once when clock driver registers the clock to
CCF in the first place.  The net change here is before commit
035a61c314eb the struct clk pointer is unique for given hardware
clock, while after the commit the pointers returned by clk lookup calls
become different for the same hardware clock.

That said, the struct clk pointer comparing in the code doesn't work any
more.  Call helper function clk_is_match() instead to fix the problem.

Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
---
 sound/soc/kirkwood/kirkwood-i2s.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/soc/kirkwood/kirkwood-i2s.c b/sound/soc/kirkwood/kirkwood-i2s.c
index def7d8260c4e..d19483081f9b 100644
--- a/sound/soc/kirkwood/kirkwood-i2s.c
+++ b/sound/soc/kirkwood/kirkwood-i2s.c
@@ -579,7 +579,7 @@ static int kirkwood_i2s_dev_probe(struct platform_device *pdev)
 		if (PTR_ERR(priv->extclk) == -EPROBE_DEFER)
 			return -EPROBE_DEFER;
 	} else {
-		if (priv->extclk == priv->clk) {
+		if (clk_is_match(priv->extclk, priv->clk)) {
 			devm_clk_put(&pdev->dev, priv->extclk);
 			priv->extclk = ERR_PTR(-EINVAL);
 		} else {
-- 
1.9.1

[PATCH 0/8] Fix struct clk pointer comparing

[Russell King - ARM Linux]

On Wed, Feb 25, 2015 at 10:53:30PM +0800, Shawn Guo wrote:
> On the first day back from Chinese new year holiday, I got a regression
> report from rmk, saying Ethernet stops working on HimmingBoard with
> v4.0-rc1.
> 
> I read through the thread [1] and found a couple of i.MX audio drivers
> are also affected per Stephen's Coccinelle report.  That's why I came up
> with this series based on Quentin's structclk.cocci, Stephen's result
> and Mike's input (thanks all).

Not all places need to be fixed in this way.

There are two broad cases:

1. Where we are trying to determine if two clocks obtained from clk_get()
   are the same clock.  IOW:

	probe()
	{
		clk1 = clk_get(dev, ...);
		clk2 = clk_get(dev, ...);

		if (clk1 == clk2)
			do_something();
	}

2. Where we are trying to determine if a clock selected from a set of
   previously obtained clk_get()'d clocks is the same as a one of those
   clocks.  IOW:

	probe()
	{
		clk1 = clk_get(dev, ...);
		clk2 = clk_get(dev, ...);
	}
...
	some_fn()
	{
		clk = select_best_clock(clk1, clk2);
		if (clk == previously_selected_clk) {
			previously_selected_clk = clk;
			do_something();
		}
	}

Case 1 applies in places like the Kirkwood I2S driver, and the iMX6
ethernet code, and it's these cases which need to be fixed.

Case 2 applies in the Armada DRM driver, and these cases need not be
"fixed".

To put it a different way: case 1 is when you're testing to see whether
two clocks refer to the same clock.  case 2 is when you're testing
whether the cached clk cookie is the same.

-- 
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.

[PATCH 1/8] clk: add helper function clk_is_match()

[Mike Turquette]

Quoting Shawn Guo (2015-02-25 06:53:31)
> Since commit 035a61c314eb ("clk: Make clk API return per-user struct clk
> instances"), clk API users can no longer check if two struct clk
> pointers are pointing to the same hardware clock, i.e. struct clk_hw, by
> simply comparing two pointers.  That's because with the per-user clk
> change, a brand new struct clk is created whenever clients try to look
> up the clock by calling clk_get() or sister functions like clk_get_sys()
> and of_clk_get().  This changes the original behavior where the struct
> clk is only created for once when clock driver registers the clock to
> CCF in the first place.  The net change here is before commit
> 035a61c314eb the struct clk pointer is unique for given hardware
> clock, while after the commit the pointers returned by clk lookup calls
> become different for the same hardware clock.
> 
> A number of client drivers detecting if two struct clk pointers point to
> the same one hardware clock by comparing the pointers are broken now.
> As a stop-gap solution, this patch adds a helper function clk_is_match()
> to test if two struct clk pointers point to the same hardware clock, so
> that these client drivers can use to fix the regression.
> 
> Signed-off-by: Shawn Guo <shawn.guo@linaro.org>

Hi Shawn,

Thanks for the patch. I wrote a similar one last night but did not
finish fixing up the drivers (and thus did not post it). I prefer my
implementation below, and I'm happy to merge your driver fixes with it.

Regards,
Mike



From: Michael Turquette <mturquette@linaro.org>
Date: Wed, 25 Feb 2015 09:11:01 -0800
Subject: [PATCH] clk: introduce clk_is_match

Some drivers compare struct clk pointers as a means of knowing
if the two pointers reference the same clock hardware. This behavior is
dubious (drivers must not dereference struct clk), but did not cause any
regressions until the per-user struct clk patch was merged. Now the test
for matching clk's will always fail with per-user struct clk's.

clk_is_match is introduced to fix the regression and prevent drivers
from comparing the pointers manually.

Fixes: 035a61c314eb ("clk: Make clk API return per-user struct clk instances")
Cc: Russell King <linux@arm.linux.org.uk>
Cc: Stephen Boyd <sboyd@codeaurora.org>
Cc: Shawn Guo <shawn.guo@linaro.org>
Cc: Tomeu Vizoso <tomeu.vizoso@collabora.com>
Signed-off-by: Michael Turquette <mturquette@linaro.org>
---
 drivers/clk/clk.c   | 26 ++++++++++++++++++++++++++
 include/linux/clk.h | 18 ++++++++++++++++++
 2 files changed, 44 insertions(+)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index eb01529..09670de 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -2171,6 +2171,32 @@ int clk_get_phase(struct clk *clk)
 }
 
 /**
+ * clk_is_match - check if two clk's point to the same hardware clock
+ * @p: clk compared against q
+ * @q: clk compared against p
+ *
+ * Returns true if the two struct clk pointers both point to the same hardware
+ * clock node. Put differently, returns true if struct clk *p and struct clk *q
+ * share the same struct clk_core object.
+ *
+ * Returns false otherwise. Note that two NULL clks are treated as matching.
+ */
+bool clk_is_match(struct clk *p, struct clk *q)
+{
+	/* trivial case: identical struct clk's or both NULL */
+	if (p == q)
+		return true;
+
+	/* true if clk->core pointers match. Avoid derefing garbage */
+	if (!IS_ERR_OR_NULL(p) && !IS_ERR_OR_NULL(q))
+		if (p->core == q->core)
+			return true;
+
+	return false;
+}
+EXPORT_SYMBOL_GPL(clk_is_match);
+
+/**
  * __clk_init - initialize the data structures in a struct clk
  * @dev:	device initializing this clk, placeholder for now
  * @clk:	clk being initialized
diff --git a/include/linux/clk.h b/include/linux/clk.h
index 8381bbf..5c076e4 100644
--- a/include/linux/clk.h
+++ b/include/linux/clk.h
@@ -376,6 +376,19 @@ struct clk *clk_get_parent(struct clk *clk);
  */
 struct clk *clk_get_sys(const char *dev_id, const char *con_id);
 
+/**
+ * clk_is_match - check if two clk's point to the same hardware clock
+ * @p: clk compared against q
+ * @q: clk compared against p
+ *
+ * Returns true if the two struct clk pointers both point to the same hardware
+ * clock node. Put differently, returns true if struct clk *p and struct clk *q
+ * share the same struct clk_core object.
+ *
+ * Returns false otherwise. Note that two NULL clks are treated as matching.
+ */
+bool clk_is_match(struct clk *p, struct clk *q);
+
 #else /* !CONFIG_HAVE_CLK */
 
 static inline struct clk *clk_get(struct device *dev, const char *id)
@@ -429,6 +442,11 @@ static inline struct clk *clk_get_parent(struct clk *clk)
 	return NULL;
 }
 
+static inline bool clk_is_match(struct clk *p, struct clk *q)
+{
+	return p == q ? true : false;
+}
+
 #endif
 
 /* clk_prepare_enable helps cases using clk_enable in non-atomic context. */
-- 
1.9.1

[PATCH 0/8] Fix struct clk pointer comparing

[Mike Turquette]

Quoting Russell King - ARM Linux (2015-02-25 07:03:49)
> On Wed, Feb 25, 2015 at 10:53:30PM +0800, Shawn Guo wrote:
> > On the first day back from Chinese new year holiday, I got a regression
> > report from rmk, saying Ethernet stops working on HimmingBoard with
> > v4.0-rc1.
> > 
> > I read through the thread [1] and found a couple of i.MX audio drivers
> > are also affected per Stephen's Coccinelle report.  That's why I came up
> > with this series based on Quentin's structclk.cocci, Stephen's result
> > and Mike's input (thanks all).
> 
> Not all places need to be fixed in this way.
> 
> There are two broad cases:
> 
> 1. Where we are trying to determine if two clocks obtained from clk_get()
>    are the same clock.  IOW:
> 
>         probe()
>         {
>                 clk1 = clk_get(dev, ...);
>                 clk2 = clk_get(dev, ...);
> 
>                 if (clk1 == clk2)
>                         do_something();
>         }
> 
> 2. Where we are trying to determine if a clock selected from a set of
>    previously obtained clk_get()'d clocks is the same as a one of those
>    clocks.  IOW:
> 
>         probe()
>         {
>                 clk1 = clk_get(dev, ...);
>                 clk2 = clk_get(dev, ...);
>         }
> ...
>         some_fn()
>         {
>                 clk = select_best_clock(clk1, clk2);
>                 if (clk == previously_selected_clk) {
>                         previously_selected_clk = clk;
>                         do_something();
>                 }
>         }
> 
> Case 1 applies in places like the Kirkwood I2S driver, and the iMX6
> ethernet code, and it's these cases which need to be fixed.
> 
> Case 2 applies in the Armada DRM driver, and these cases need not be
> "fixed".
> 
> To put it a different way: case 1 is when you're testing to see whether
> two clocks refer to the same clock.  case 2 is when you're testing
> whether the cached clk cookie is the same.

It looks like patches 2, 7 and 8 are correct in this series. I'll apply
them towards -rc2 if nobody objects.

Regards,
Mike

> 
> -- 
> FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
> according to speedtest.net.

[PATCH 0/8] Fix struct clk pointer comparing

[Stephen Boyd]

On 02/25/15 09:55, Mike Turquette wrote:
> Quoting Russell King - ARM Linux (2015-02-25 07:03:49)
>> Case 1 applies in places like the Kirkwood I2S driver, and the iMX6
>> ethernet code, and it's these cases which need to be fixed.
>>
>> Case 2 applies in the Armada DRM driver, and these cases need not be
>> "fixed".
>>
>> To put it a different way: case 1 is when you're testing to see whether
>> two clocks refer to the same clock.  case 2 is when you're testing
>> whether the cached clk cookie is the same.
> It looks like patches 2, 7 and 8 are correct in this series. I'll apply
> them towards -rc2 if nobody objects.
>
>

For patch 8 can't we use the alternative patch that I came up with
before? I have a patch for sound/soc/fsl/fsl_esai.c and
drivers/pwm/pwm-atmel-hlcdc.c which I can send shortly.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH 7/8] ASoC: fsl_spdif: fix struct clk pointer comparing

[Stephen Boyd]

On 02/25/15 06:53, Shawn Guo wrote:
> Since commit 035a61c314eb ("clk: Make clk API return per-user struct clk
> instances"), clk API users can no longer check if two struct clk
> pointers are pointing to the same hardware clock, i.e. struct clk_hw, by
> simply comparing two pointers.  That's because with the per-user clk
> change, a brand new struct clk is created whenever clients try to look
> up the clock by calling clk_get() or sister functions like clk_get_sys()
> and of_clk_get().  This changes the original behavior where the struct
> clk is only created for once when clock driver registers the clock to
> CCF in the first place.  The net change here is before commit
> 035a61c314eb the struct clk pointer is unique for given hardware
> clock, while after the commit the pointers returned by clk lookup calls
> become different for the same hardware clock.
>
> That said, the struct clk pointer comparing in the code doesn't work any
> more.  Call helper function clk_is_match() instead to fix the problem.
>
> Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> ---
>  sound/soc/fsl/fsl_spdif.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/sound/soc/fsl/fsl_spdif.c b/sound/soc/fsl/fsl_spdif.c
> index 75870c0ea2c9..91eb3aef7f02 100644
> --- a/sound/soc/fsl/fsl_spdif.c
> +++ b/sound/soc/fsl/fsl_spdif.c
> @@ -1049,7 +1049,7 @@ static u32 fsl_spdif_txclk_caldiv(struct fsl_spdif_priv *spdif_priv,
>  				enum spdif_txrate index, bool round)
>  {
>  	const u32 rate[] = { 32000, 44100, 48000, 96000, 192000 };
> -	bool is_sysclk = clk == spdif_priv->sysclk;
> +	bool is_sysclk = clk_is_match(clk, spdif_priv->sysclk);
>  	u64 rate_ideal, rate_actual, sub;
>  	u32 sysclk_dfmin, sysclk_dfmax;
>  	u32 txclk_df, sysclk_df, arate;
> @@ -1143,7 +1143,7 @@ static int fsl_spdif_probe_txclk(struct fsl_spdif_priv *spdif_priv,
>  			spdif_priv->txclk_src[index], rate[index]);
>  	dev_dbg(&pdev->dev, "use txclk df %d for %dHz sample rate\n",
>  			spdif_priv->txclk_df[index], rate[index]);
> -	if (spdif_priv->txclk[index] == spdif_priv->sysclk)
> +	if (clk_is_match(spdif_priv->txclk[index], spdif_priv->sysclk))
>  		dev_dbg(&pdev->dev, "use sysclk df %d for %dHz sample rate\n",
>  				spdif_priv->sysclk_df[index], rate[index]);
>  	dev_dbg(&pdev->dev, "the best rate for %dHz sample rate is %dHz\n",

Couldn't this be fixed like this?

----8<----

diff --git a/sound/soc/fsl/fsl_spdif.c b/sound/soc/fsl/fsl_spdif.c
index af0429421fc8..1878dd62a247 100644
--- a/sound/soc/fsl/fsl_spdif.c
+++ b/sound/soc/fsl/fsl_spdif.c
@@ -20,6 +20,7 @@
 #include <linux/of_device.h>
 #include <linux/of_irq.h>
 #include <linux/regmap.h>
+#include <linux/stringify.h>
 
 #include <sound/asoundef.h>
 #include <sound/dmaengine_pcm.h>
@@ -46,6 +47,8 @@ static u8 srpc_dpll_locked[] = { 0x0, 0x1, 0x2, 0x3, 0x4, 0xa, 0xb };
 
 #define DEFAULT_RXCLK_SRC	1
 
+#define SYSCLK_NUM		5
+
 /*
  * SPDIF control structure
  * Defines channel status, subcode and Q sub
@@ -1051,10 +1054,10 @@ static const struct regmap_config fsl_spdif_regmap_config = {
 
 static u32 fsl_spdif_txclk_caldiv(struct fsl_spdif_priv *spdif_priv,
 				struct clk *clk, u64 savesub,
-				enum spdif_txrate index, bool round)
+				enum spdif_txrate index, bool round,
+				bool is_sysclk)
 {
 	const u32 rate[] = { 32000, 44100, 48000, 96000, 192000 };
-	bool is_sysclk = clk == spdif_priv->sysclk;
 	u64 rate_ideal, rate_actual, sub;
 	u32 sysclk_dfmin, sysclk_dfmax;
 	u32 txclk_df, sysclk_df, arate;
@@ -1131,7 +1134,8 @@ static int fsl_spdif_probe_txclk(struct fsl_spdif_priv *spdif_priv,
 			continue;
 
 		ret = fsl_spdif_txclk_caldiv(spdif_priv, clk, savesub, index,
-					     i == STC_TXCLK_SPDIF_ROOT);
+					     i == STC_TXCLK_SPDIF_ROOT,
+					     i == SYSCLK_NUM);
 		if (savesub == ret)
 			continue;
 
@@ -1210,7 +1214,8 @@ static int fsl_spdif_probe(struct platform_device *pdev)
 	}
 
 	/* Get system clock for rx clock rate calculation */
-	spdif_priv->sysclk = devm_clk_get(&pdev->dev, "rxtx5");
+	spdif_priv->sysclk = devm_clk_get(&pdev->dev,
+					  "rxtx" __stringify(SYSCLK_NUM));
 	if (IS_ERR(spdif_priv->sysclk)) {
 		dev_err(&pdev->dev, "no sys clock (rxtx5) in devicetree\n");
 		return PTR_ERR(spdif_priv->sysclk);


-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH 1/8] clk: add helper function clk_is_match()

[Shawn Guo]

On Wed, Feb 25, 2015 at 09:27:57AM -0800, Mike Turquette wrote:
> Hi Shawn,
> 
> Thanks for the patch. I wrote a similar one last night but did not
> finish fixing up the drivers (and thus did not post it). I prefer my
> implementation below, and I'm happy to merge your driver fixes with it.

Sure, no problem.  My intention was to get rmk's HummingBoard back to
work ASAP :)

> From: Michael Turquette <mturquette@linaro.org>
> Date: Wed, 25 Feb 2015 09:11:01 -0800
> Subject: [PATCH] clk: introduce clk_is_match
> 
> Some drivers compare struct clk pointers as a means of knowing
> if the two pointers reference the same clock hardware. This behavior is
> dubious (drivers must not dereference struct clk), but did not cause any
> regressions until the per-user struct clk patch was merged. Now the test
> for matching clk's will always fail with per-user struct clk's.
> 
> clk_is_match is introduced to fix the regression and prevent drivers
> from comparing the pointers manually.
> 
> Fixes: 035a61c314eb ("clk: Make clk API return per-user struct clk instances")
> Cc: Russell King <linux@arm.linux.org.uk>
> Cc: Stephen Boyd <sboyd@codeaurora.org>
> Cc: Shawn Guo <shawn.guo@linaro.org>

Tested-by: Shawn Guo <shawn.guo@linaro.org>

> Cc: Tomeu Vizoso <tomeu.vizoso@collabora.com>
> Signed-off-by: Michael Turquette <mturquette@linaro.org>

[PATCH 7/8] ASoC: fsl_spdif: fix struct clk pointer comparing

[Shawn Guo]

On Wed, Feb 25, 2015 at 01:04:41PM -0800, Stephen Boyd wrote:
> > diff --git a/sound/soc/fsl/fsl_spdif.c b/sound/soc/fsl/fsl_spdif.c
> > index 75870c0ea2c9..91eb3aef7f02 100644
> > --- a/sound/soc/fsl/fsl_spdif.c
> > +++ b/sound/soc/fsl/fsl_spdif.c
> > @@ -1049,7 +1049,7 @@ static u32 fsl_spdif_txclk_caldiv(struct fsl_spdif_priv *spdif_priv,
> >  				enum spdif_txrate index, bool round)
> >  {
> >  	const u32 rate[] = { 32000, 44100, 48000, 96000, 192000 };
> > -	bool is_sysclk = clk == spdif_priv->sysclk;
> > +	bool is_sysclk = clk_is_match(clk, spdif_priv->sysclk);
> >  	u64 rate_ideal, rate_actual, sub;
> >  	u32 sysclk_dfmin, sysclk_dfmax;
> >  	u32 txclk_df, sysclk_df, arate;
> > @@ -1143,7 +1143,7 @@ static int fsl_spdif_probe_txclk(struct fsl_spdif_priv *spdif_priv,
> >  			spdif_priv->txclk_src[index], rate[index]);
> >  	dev_dbg(&pdev->dev, "use txclk df %d for %dHz sample rate\n",
> >  			spdif_priv->txclk_df[index], rate[index]);
> > -	if (spdif_priv->txclk[index] == spdif_priv->sysclk)
> > +	if (clk_is_match(spdif_priv->txclk[index], spdif_priv->sysclk))
> >  		dev_dbg(&pdev->dev, "use sysclk df %d for %dHz sample rate\n",
> >  				spdif_priv->sysclk_df[index], rate[index]);
> >  	dev_dbg(&pdev->dev, "the best rate for %dHz sample rate is %dHz\n",
> 
> Couldn't this be fixed like this?

It could.  But this fix is less-intuitive and makes the code change
unnecessarily complex, considering we are introducing helper
clk_is_match() anyway.

Shawn

> 
> ----8<----
> 
> diff --git a/sound/soc/fsl/fsl_spdif.c b/sound/soc/fsl/fsl_spdif.c
> index af0429421fc8..1878dd62a247 100644
> --- a/sound/soc/fsl/fsl_spdif.c
> +++ b/sound/soc/fsl/fsl_spdif.c
> @@ -20,6 +20,7 @@
>  #include <linux/of_device.h>
>  #include <linux/of_irq.h>
>  #include <linux/regmap.h>
> +#include <linux/stringify.h>
>  
>  #include <sound/asoundef.h>
>  #include <sound/dmaengine_pcm.h>
> @@ -46,6 +47,8 @@ static u8 srpc_dpll_locked[] = { 0x0, 0x1, 0x2, 0x3, 0x4, 0xa, 0xb };
>  
>  #define DEFAULT_RXCLK_SRC	1
>  
> +#define SYSCLK_NUM		5
> +
>  /*
>   * SPDIF control structure
>   * Defines channel status, subcode and Q sub
> @@ -1051,10 +1054,10 @@ static const struct regmap_config fsl_spdif_regmap_config = {
>  
>  static u32 fsl_spdif_txclk_caldiv(struct fsl_spdif_priv *spdif_priv,
>  				struct clk *clk, u64 savesub,
> -				enum spdif_txrate index, bool round)
> +				enum spdif_txrate index, bool round,
> +				bool is_sysclk)
>  {
>  	const u32 rate[] = { 32000, 44100, 48000, 96000, 192000 };
> -	bool is_sysclk = clk == spdif_priv->sysclk;
>  	u64 rate_ideal, rate_actual, sub;
>  	u32 sysclk_dfmin, sysclk_dfmax;
>  	u32 txclk_df, sysclk_df, arate;
> @@ -1131,7 +1134,8 @@ static int fsl_spdif_probe_txclk(struct fsl_spdif_priv *spdif_priv,
>  			continue;
>  
>  		ret = fsl_spdif_txclk_caldiv(spdif_priv, clk, savesub, index,
> -					     i == STC_TXCLK_SPDIF_ROOT);
> +					     i == STC_TXCLK_SPDIF_ROOT,
> +					     i == SYSCLK_NUM);
>  		if (savesub == ret)
>  			continue;
>  
> @@ -1210,7 +1214,8 @@ static int fsl_spdif_probe(struct platform_device *pdev)
>  	}
>  
>  	/* Get system clock for rx clock rate calculation */
> -	spdif_priv->sysclk = devm_clk_get(&pdev->dev, "rxtx5");
> +	spdif_priv->sysclk = devm_clk_get(&pdev->dev,
> +					  "rxtx" __stringify(SYSCLK_NUM));
>  	if (IS_ERR(spdif_priv->sysclk)) {
>  		dev_err(&pdev->dev, "no sys clock (rxtx5) in devicetree\n");
>  		return PTR_ERR(spdif_priv->sysclk);
> 
> 
> -- 
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
> a Linux Foundation Collaborative Project
> 

[PATCH 0/8] Fix struct clk pointer comparing

[Shawn Guo]

On Wed, Feb 25, 2015 at 12:42:45PM -0800, Stephen Boyd wrote:
> On 02/25/15 09:55, Mike Turquette wrote:
> > Quoting Russell King - ARM Linux (2015-02-25 07:03:49)
> >> Case 1 applies in places like the Kirkwood I2S driver, and the iMX6
> >> ethernet code, and it's these cases which need to be fixed.
> >>
> >> Case 2 applies in the Armada DRM driver, and these cases need not be
> >> "fixed".
> >>
> >> To put it a different way: case 1 is when you're testing to see whether
> >> two clocks refer to the same clock.  case 2 is when you're testing
> >> whether the cached clk cookie is the same.
> > It looks like patches 2, 7 and 8 are correct in this series. I'll apply
> > them towards -rc2 if nobody objects.
> >
> >
> 
> For patch 8 can't we use the alternative patch that I came up with
> before? I have a patch for sound/soc/fsl/fsl_esai.c and
> drivers/pwm/pwm-atmel-hlcdc.c which I can send shortly.

This beats the idea of clk_is_match() helper.  I prefer to fix this
generic problem in a generic way, i.e. using clk_is_match(), which
should be much easier and safer during -rc cycle.

Shawn

[PATCH 0/8] Fix struct clk pointer comparing

[Mike Turquette]

Quoting Stephen Boyd (2015-02-25 12:42:45)
> On 02/25/15 09:55, Mike Turquette wrote:
> > Quoting Russell King - ARM Linux (2015-02-25 07:03:49)
> >> Case 1 applies in places like the Kirkwood I2S driver, and the iMX6
> >> ethernet code, and it's these cases which need to be fixed.
> >>
> >> Case 2 applies in the Armada DRM driver, and these cases need not be
> >> "fixed".
> >>
> >> To put it a different way: case 1 is when you're testing to see whether
> >> two clocks refer to the same clock.  case 2 is when you're testing
> >> whether the cached clk cookie is the same.
> > It looks like patches 2, 7 and 8 are correct in this series. I'll apply
> > them towards -rc2 if nobody objects.
> >
> >
> 
> For patch 8 can't we use the alternative patch that I came up with
> before? I have a patch for sound/soc/fsl/fsl_esai.c and
> drivers/pwm/pwm-atmel-hlcdc.c which I can send shortly.

Probably we can use them. I'll need to spend a few minutes with both
patches and make sure it looks good. clkdev stuff always spins my head
around.

Regards,
Mike

> 
> -- 
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
> a Linux Foundation Collaborative Project
> 

[PATCH 8/8] ASoC: kirkwood: fix struct clk pointer comparing

[Mark Brown]

On Wed, Feb 25, 2015 at 10:53:38PM +0800, Shawn Guo wrote:
> Since commit 035a61c314eb ("clk: Make clk API return per-user struct clk
> instances"), clk API users can no longer check if two struct clk
> pointers are pointing to the same hardware clock, i.e. struct clk_hw, by

Applied, thanks.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150226/130aa66d/attachment.sig>

[PATCH 7/8] ASoC: fsl_spdif: fix struct clk pointer comparing

[Mark Brown]

On Wed, Feb 25, 2015 at 10:53:37PM +0800, Shawn Guo wrote:
> Since commit 035a61c314eb ("clk: Make clk API return per-user struct clk
> instances"), clk API users can no longer check if two struct clk
> pointers are pointing to the same hardware clock, i.e. struct clk_hw, by

Applied, thanks.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150226/acd119b1/attachment.sig>

[PATCH 7/8] ASoC: fsl_spdif: fix struct clk pointer comparing

[Shawn Guo]

On Thu, Feb 26, 2015 at 11:12:50AM +0900, Mark Brown wrote:
> On Wed, Feb 25, 2015 at 10:53:37PM +0800, Shawn Guo wrote:
> > Since commit 035a61c314eb ("clk: Make clk API return per-user struct clk
> > instances"), clk API users can no longer check if two struct clk
> > pointers are pointing to the same hardware clock, i.e. struct clk_hw, by
> 
> Applied, thanks.

Mark,

Sorry that I did not make it clear in the cover-letter.  But the first
patch introduces a helper function clk_is_match(), on which all the
other patches in the series depend.  That said, the patch series should
probably be handled by Mike as a whole.

Shawn

[PATCH 7/8] ASoC: fsl_spdif: fix struct clk pointer comparing

[Mark Brown]

On Thu, Feb 26, 2015 at 10:20:00AM +0800, Shawn Guo wrote:

> Sorry that I did not make it clear in the cover-letter.  But the first
> patch introduces a helper function clk_is_match(), on which all the
> other patches in the series depend.  That said, the patch series should
> probably be handled by Mike as a whole.

Ugh, right.  I've dropped those patches then.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150226/0cf9f199/attachment.sig>

[PATCH 7/8] ASoC: fsl_spdif: fix struct clk pointer comparing

[Mark Brown]

On Wed, Feb 25, 2015 at 10:53:37PM +0800, Shawn Guo wrote:
> Since commit 035a61c314eb ("clk: Make clk API return per-user struct clk
> instances"), clk API users can no longer check if two struct clk
> pointers are pointing to the same hardware clock, i.e. struct clk_hw, by

Acked-by: Mark Brown <broonie@kernel.org>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150226/f40b117a/attachment.sig>

[PATCH 8/8] ASoC: kirkwood: fix struct clk pointer comparing

[Mark Brown]

On Wed, Feb 25, 2015 at 10:53:38PM +0800, Shawn Guo wrote:
> Since commit 035a61c314eb ("clk: Make clk API return per-user struct clk
> instances"), clk API users can no longer check if two struct clk
> pointers are pointing to the same hardware clock, i.e. struct clk_hw, by

Acked-by: Mark Brown <broonie@kernel.org>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150226/0555aca7/attachment.sig>

[PATCH 6/8] ASoC: fsl_esai: fix struct clk pointer comparing

[Mark Brown]

On Wed, Feb 25, 2015 at 10:53:36PM +0800, Shawn Guo wrote:
> Since commit 035a61c314eb ("clk: Make clk API return per-user struct clk
> instances"), clk API users can no longer check if two struct clk
> pointers are pointing to the same hardware clock, i.e. struct clk_hw, by

Acked-by: Mark Brown <broonie@kernel.org>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150226/a789c434/attachment.sig>

[alsa-devel] [PATCH 1/8] clk: add helper function clk_is_match()

[Ben Dooks]

On Wed, Feb 25, 2015 at 09:27:57AM -0800, Mike Turquette wrote:
> Quoting Shawn Guo (2015-02-25 06:53:31)
> > Since commit 035a61c314eb ("clk: Make clk API return per-user struct clk
> > instances"), clk API users can no longer check if two struct clk
> > pointers are pointing to the same hardware clock, i.e. struct clk_hw, by
> > simply comparing two pointers.  That's because with the per-user clk
> > change, a brand new struct clk is created whenever clients try to look
> > up the clock by calling clk_get() or sister functions like clk_get_sys()
> > and of_clk_get().  This changes the original behavior where the struct
> > clk is only created for once when clock driver registers the clock to
> > CCF in the first place.  The net change here is before commit
> > 035a61c314eb the struct clk pointer is unique for given hardware
> > clock, while after the commit the pointers returned by clk lookup calls
> > become different for the same hardware clock.
> > 
> > A number of client drivers detecting if two struct clk pointers point to
> > the same one hardware clock by comparing the pointers are broken now.
> > As a stop-gap solution, this patch adds a helper function clk_is_match()
> > to test if two struct clk pointers point to the same hardware clock, so
> > that these client drivers can use to fix the regression.
> > 
> > Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> 
> Hi Shawn,
> 
> Thanks for the patch. I wrote a similar one last night but did not
> finish fixing up the drivers (and thus did not post it). I prefer my
> implementation below, and I'm happy to merge your driver fixes with it.
> 
> Regards,
> Mike
> 
> 
> 
> From: Michael Turquette <mturquette@linaro.org>
> Date: Wed, 25 Feb 2015 09:11:01 -0800
> Subject: [PATCH] clk: introduce clk_is_match
> 
> Some drivers compare struct clk pointers as a means of knowing
> if the two pointers reference the same clock hardware. This behavior is
> dubious (drivers must not dereference struct clk), but did not cause any
> regressions until the per-user struct clk patch was merged. Now the test
> for matching clk's will always fail with per-user struct clk's.
> 
> clk_is_match is introduced to fix the regression and prevent drivers
> from comparing the pointers manually.

small observaton, clk_is_same() is linguistically nicer.

-- 
Ben Dooks, ben at fluff.org, http://www.fluff.org/ben/

Large Hadron Colada: A large Pina Colada that makes the universe disappear.

[PATCH 4/8] pwm: atmel-hlcdc: fix struct clk pointer comparing

[Nicolas Ferre]

Le 25/02/2015 15:53, Shawn Guo a ?crit :
> Since commit 035a61c314eb ("clk: Make clk API return per-user struct clk
> instances"), clk API users can no longer check if two struct clk
> pointers are pointing to the same hardware clock, i.e. struct clk_hw, by
> simply comparing two pointers.  That's because with the per-user clk
> change, a brand new struct clk is created whenever clients try to look
> up the clock by calling clk_get() or sister functions like clk_get_sys()
> and of_clk_get().  This changes the original behavior where the struct
> clk is only created for once when clock driver registers the clock to
> CCF in the first place.  The net change here is before commit
> 035a61c314eb the struct clk pointer is unique for given hardware
> clock, while after the commit the pointers returned by clk lookup calls
> become different for the same hardware clock.
> 
> That said, the struct clk pointer comparing in the code doesn't work any
> more.  Call helper function clk_is_match() instead to fix the problem.
> 
> Signed-off-by: Shawn Guo <shawn.guo@linaro.org>

I agree with the fix whichever name is chosen for the function in an
future version of this series. So you can add my:
Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com>

Maybe Boris can double check...

> ---
>  drivers/pwm/pwm-atmel-hlcdc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/pwm/pwm-atmel-hlcdc.c b/drivers/pwm/pwm-atmel-hlcdc.c
> index 522f7075bb1a..36475949b829 100644
> --- a/drivers/pwm/pwm-atmel-hlcdc.c
> +++ b/drivers/pwm/pwm-atmel-hlcdc.c
> @@ -97,7 +97,7 @@ static int atmel_hlcdc_pwm_config(struct pwm_chip *c,
>  
>  	pwmcfg = ATMEL_HLCDC_PWMPS(pres);
>  
> -	if (new_clk != chip->cur_clk) {
> +	if (!clk_is_match(new_clk, chip->cur_clk)) {
>  		u32 gencfg = 0;
>  		int ret;
>  
> 


-- 
Nicolas Ferre

[PATCH 4/8] pwm: atmel-hlcdc: fix struct clk pointer comparing

[Boris Brezillon]

Nicolas, Shawn,

On Thu, 26 Feb 2015 10:22:50 +0100
Nicolas Ferre <nicolas.ferre@atmel.com> wrote:

> Le 25/02/2015 15:53, Shawn Guo a ?crit :
> > Since commit 035a61c314eb ("clk: Make clk API return per-user struct clk
> > instances"), clk API users can no longer check if two struct clk
> > pointers are pointing to the same hardware clock, i.e. struct clk_hw, by
> > simply comparing two pointers.  That's because with the per-user clk
> > change, a brand new struct clk is created whenever clients try to look
> > up the clock by calling clk_get() or sister functions like clk_get_sys()
> > and of_clk_get().  This changes the original behavior where the struct
> > clk is only created for once when clock driver registers the clock to
> > CCF in the first place.  The net change here is before commit
> > 035a61c314eb the struct clk pointer is unique for given hardware
> > clock, while after the commit the pointers returned by clk lookup calls
> > become different for the same hardware clock.
> > 
> > That said, the struct clk pointer comparing in the code doesn't work any
> > more.  Call helper function clk_is_match() instead to fix the problem.
> > 
> > Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> 
> I agree with the fix whichever name is chosen for the function in an
> future version of this series. So you can add my:
> Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com>
> 
> Maybe Boris can double check...

Looks good to me.
Thanks for fixing that.

Acked-by: Boris Brezillon <boris.brezillon@free-electrons.com>

> 
> > ---
> >  drivers/pwm/pwm-atmel-hlcdc.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/pwm/pwm-atmel-hlcdc.c b/drivers/pwm/pwm-atmel-hlcdc.c
> > index 522f7075bb1a..36475949b829 100644
> > --- a/drivers/pwm/pwm-atmel-hlcdc.c
> > +++ b/drivers/pwm/pwm-atmel-hlcdc.c
> > @@ -97,7 +97,7 @@ static int atmel_hlcdc_pwm_config(struct pwm_chip *c,
> >  
> >  	pwmcfg = ATMEL_HLCDC_PWMPS(pres);
> >  
> > -	if (new_clk != chip->cur_clk) {
> > +	if (!clk_is_match(new_clk, chip->cur_clk)) {
> >  		u32 gencfg = 0;
> >  		int ret;
> >  
> > 
> 
> 



-- 
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[alsa-devel] [PATCH 1/8] clk: add helper function clk_is_match()

[Philipp Zabel]

Am Donnerstag, den 26.02.2015, 09:02 +0000 schrieb Ben Dooks:
> On Wed, Feb 25, 2015 at 09:27:57AM -0800, Mike Turquette wrote:
[...]
> > From: Michael Turquette <mturquette@linaro.org>
> > Date: Wed, 25 Feb 2015 09:11:01 -0800
> > Subject: [PATCH] clk: introduce clk_is_match
> > 
> > Some drivers compare struct clk pointers as a means of knowing
> > if the two pointers reference the same clock hardware. This behavior is
> > dubious (drivers must not dereference struct clk), but did not cause any
> > regressions until the per-user struct clk patch was merged. Now the test
> > for matching clk's will always fail with per-user struct clk's.
> > 
> > clk_is_match is introduced to fix the regression and prevent drivers
> > from comparing the pointers manually.
> 
> small observaton, clk_is_same() is linguistically nicer.

How about clk_equal() ?

regards
Philipp

[alsa-devel] [PATCH 1/8] clk: add helper function clk_is_match()

[Ben Dooks]

On Thu, Feb 26, 2015 at 10:56:58AM +0100, Philipp Zabel wrote:
> Am Donnerstag, den 26.02.2015, 09:02 +0000 schrieb Ben Dooks:
> > On Wed, Feb 25, 2015 at 09:27:57AM -0800, Mike Turquette wrote:
> [...]
> > > From: Michael Turquette <mturquette@linaro.org>
> > > Date: Wed, 25 Feb 2015 09:11:01 -0800
> > > Subject: [PATCH] clk: introduce clk_is_match
> > > 
> > > Some drivers compare struct clk pointers as a means of knowing
> > > if the two pointers reference the same clock hardware. This behavior is
> > > dubious (drivers must not dereference struct clk), but did not cause any
> > > regressions until the per-user struct clk patch was merged. Now the test
> > > for matching clk's will always fail with per-user struct clk's.
> > > 
> > > clk_is_match is introduced to fix the regression and prevent drivers
> > > from comparing the pointers manually.
> > 
> > small observaton, clk_is_same() is linguistically nicer.
> 
> How about clk_equal() ?

That's good, the only issue that's not clear in any of these names is
that does this mean "the same clock", a "clock of the same rate" or a
"clock that is equivalent to in the rate and phase but not subject to
the same gate".


-- 
Ben Dooks, ben at fluff.org, http://www.fluff.org/ben/

Large Hadron Colada: A large Pina Colada that makes the universe disappear.

[alsa-devel] [PATCH 1/8] clk: add helper function clk_is_match()

[Ricard Wanderlof]

On Thu, 26 Feb 2015, Ben Dooks wrote:

> > > small observaton, clk_is_same() is linguistically nicer.
> > 
> > How about clk_equal() ?
> 
> That's good, the only issue that's not clear in any of these names is
> that does this mean "the same clock", a "clock of the same rate" or a
> "clock that is equivalent to in the rate and phase but not subject to
> the same gate".

clk_identical() ?

/Ricard
-- 
Ricard Wolf Wanderl?f                           ricardw(at)axis.com
Axis Communications AB, Lund, Sweden            www.axis.com
Phone +46 46 272 2016                           Fax +46 46 13 61 30

[PATCH 1/8] clk: add helper function clk_is_match()

[Geert Uytterhoeven]

On Wed, Feb 25, 2015 at 6:27 PM, Mike Turquette <mturquette@linaro.org> wrote:
> From: Michael Turquette <mturquette@linaro.org>
> Date: Wed, 25 Feb 2015 09:11:01 -0800
> Subject: [PATCH] clk: introduce clk_is_match
>
> Some drivers compare struct clk pointers as a means of knowing
> if the two pointers reference the same clock hardware. This behavior is
> dubious (drivers must not dereference struct clk), but did not cause any
> regressions until the per-user struct clk patch was merged. Now the test
> for matching clk's will always fail with per-user struct clk's.
>
> clk_is_match is introduced to fix the regression and prevent drivers
> from comparing the pointers manually.
>
> Fixes: 035a61c314eb ("clk: Make clk API return per-user struct clk instances")
> Cc: Russell King <linux@arm.linux.org.uk>
> Cc: Stephen Boyd <sboyd@codeaurora.org>
> Cc: Shawn Guo <shawn.guo@linaro.org>
> Cc: Tomeu Vizoso <tomeu.vizoso@collabora.com>
> Signed-off-by: Michael Turquette <mturquette@linaro.org>
> ---
>  drivers/clk/clk.c   | 26 ++++++++++++++++++++++++++
>  include/linux/clk.h | 18 ++++++++++++++++++
>  2 files changed, 44 insertions(+)
>
> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> index eb01529..09670de 100644
> --- a/drivers/clk/clk.c
> +++ b/drivers/clk/clk.c
> @@ -2171,6 +2171,32 @@ int clk_get_phase(struct clk *clk)
>  }
>
>  /**
> + * clk_is_match - check if two clk's point to the same hardware clock
> + * @p: clk compared against q
> + * @q: clk compared against p
> + *
> + * Returns true if the two struct clk pointers both point to the same hardware
> + * clock node. Put differently, returns true if struct clk *p and struct clk *q
> + * share the same struct clk_core object.
> + *
> + * Returns false otherwise. Note that two NULL clks are treated as matching.
> + */
> +bool clk_is_match(struct clk *p, struct clk *q)

const struct clk *p, const struct clk *q

> --- a/include/linux/clk.h
> +++ b/include/linux/clk.h
> @@ -376,6 +376,19 @@ struct clk *clk_get_parent(struct clk *clk);
>   */
>  struct clk *clk_get_sys(const char *dev_id, const char *con_id);
>
> +/**
> + * clk_is_match - check if two clk's point to the same hardware clock
> + * @p: clk compared against q
> + * @q: clk compared against p
> + *
> + * Returns true if the two struct clk pointers both point to the same hardware
> + * clock node. Put differently, returns true if struct clk *p and struct clk *q
> + * share the same struct clk_core object.
> + *
> + * Returns false otherwise. Note that two NULL clks are treated as matching.
> + */
> +bool clk_is_match(struct clk *p, struct clk *q);

const struct clk *p, const struct clk *q

> +
>  #else /* !CONFIG_HAVE_CLK */
>
>  static inline struct clk *clk_get(struct device *dev, const char *id)
> @@ -429,6 +442,11 @@ static inline struct clk *clk_get_parent(struct clk *clk)
>         return NULL;
>  }
>
> +static inline bool clk_is_match(struct clk *p, struct clk *q)

const struct clk *p, const struct clk *q

> +{
> +       return p == q ? true : false;

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

[PATCH 1/8] clk: add helper function clk_is_match()

[Russell King - ARM Linux]

On Wed, Feb 25, 2015 at 09:27:57AM -0800, Mike Turquette wrote:
> +static inline bool clk_is_match(struct clk *p, struct clk *q)
> +{
> +	return p == q ? true : false;

When they created the C standard, they already taught the compiler that
p == q returns a true / false boolean value; there's no need for ?: here.

-- 
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.

[PATCH] clk: provide clk_is_match() dummy for non-common clk

[Arnd Bergmann]

ARM randconfig build tests found a new error for configurations
with COMMON_CLK disabled but HAS_CLK selected by the platform:

ERROR: "clk_is_match" [sound/soc/fsl/snd-soc-fsl-spdif.ko] undefined!

This moves the declaration around, so this case is covered
by the existing static inline helper function.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Fixes: c69e182e51d89 ("clk: introduce clk_is_match")
----
BTW, we have a preexisting problem in clk_get_parent,
clk_round_rate and clk_set_parent, which I've worked around in
my randconfig builds so far. Should we do that the same way?

diff --git a/include/linux/clk.h b/include/linux/clk.h
index 5c076e4d90f9..a9b91595d106 100644
--- a/include/linux/clk.h
+++ b/include/linux/clk.h
@@ -125,6 +125,19 @@ int clk_set_phase(struct clk *clk, int degrees);
  */
 int clk_get_phase(struct clk *clk);
 
+/**
+ * clk_is_match - check if two clk's point to the same hardware clock
+ * @p: clk compared against q
+ * @q: clk compared against p
+ *
+ * Returns true if the two struct clk pointers both point to the same hardware
+ * clock node. Put differently, returns true if struct clk *p and struct clk *q
+ * share the same struct clk_core object.
+ *
+ * Returns false otherwise. Note that two NULL clks are treated as matching.
+ */
+bool clk_is_match(struct clk *p, struct clk *q);
+
 #else
 
 static inline long clk_get_accuracy(struct clk *clk)
@@ -142,6 +155,11 @@ static inline long clk_get_phase(struct clk *clk)
 	return -ENOTSUPP;
 }
 
+static inline bool clk_is_match(struct clk *p, struct clk *q)
+{
+	return p == q ? true : false;
+}
+
 #endif
 
 /**
@@ -376,19 +394,6 @@ struct clk *clk_get_parent(struct clk *clk);
  */
 struct clk *clk_get_sys(const char *dev_id, const char *con_id);
 
-/**
- * clk_is_match - check if two clk's point to the same hardware clock
- * @p: clk compared against q
- * @q: clk compared against p
- *
- * Returns true if the two struct clk pointers both point to the same hardware
- * clock node. Put differently, returns true if struct clk *p and struct clk *q
- * share the same struct clk_core object.
- *
- * Returns false otherwise. Note that two NULL clks are treated as matching.
- */
-bool clk_is_match(struct clk *p, struct clk *q);
-
 #else /* !CONFIG_HAVE_CLK */
 
 static inline struct clk *clk_get(struct device *dev, const char *id)
@@ -442,11 +447,6 @@ static inline struct clk *clk_get_parent(struct clk *clk)
 	return NULL;
 }
 
-static inline bool clk_is_match(struct clk *p, struct clk *q)
-{
-	return p == q ? true : false;
-}
-
 #endif
 
 /* clk_prepare_enable helps cases using clk_enable in non-atomic context. */

[PATCH] clk: provide clk_is_match() dummy for non-common clk

[Geert Uytterhoeven]

On Sun, Mar 8, 2015 at 10:05 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> ARM randconfig build tests found a new error for configurations
> with COMMON_CLK disabled but HAS_CLK selected by the platform:
>
> ERROR: "clk_is_match" [sound/soc/fsl/snd-soc-fsl-spdif.ko] undefined!
>
> This moves the declaration around, so this case is covered
> by the existing static inline helper function.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Fixes: c69e182e51d89 ("clk: introduce clk_is_match")

FYI, this also happens for sh/allmodconfig
http://kisskb.ellerman.id.au/kisskb/buildresult/12379884/

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

[PATCH] clk: provide clk_is_match() dummy for non-common clk

[Uwe Kleine-König]

Hello, 

On Sun, Mar 08, 2015 at 10:05:29PM +0100, Arnd Bergmann wrote:
> +static inline bool clk_is_match(struct clk *p, struct clk *q)
> +{
> +	return p == q ? true : false;
OK, this is only a move, but I wonder why Russell's comment that this is
equivalent to the easier

	return p == q;

wasn't addressed?!

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[PATCH] clk: provide clk_is_match() dummy for non-common clk

[Russell King - ARM Linux]

On Sun, Mar 08, 2015 at 10:05:29PM +0100, Arnd Bergmann wrote:
> ARM randconfig build tests found a new error for configurations
> with COMMON_CLK disabled but HAS_CLK selected by the platform:
> 
> ERROR: "clk_is_match" [sound/soc/fsl/snd-soc-fsl-spdif.ko] undefined!
> 
> This moves the declaration around, so this case is covered
> by the existing static inline helper function.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Fixes: c69e182e51d89 ("clk: introduce clk_is_match")
> ----
> BTW, we have a preexisting problem in clk_get_parent,
> clk_round_rate and clk_set_parent, which I've worked around in
> my randconfig builds so far. Should we do that the same way?

NAK, as Uwe points out, you didn't address my comment.

-- 
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.

[PATCH 4/8] pwm: atmel-hlcdc: fix struct clk pointer comparing

[Thierry Reding]

On Wed, Feb 25, 2015 at 10:53:34PM +0800, Shawn Guo wrote:
> Since commit 035a61c314eb ("clk: Make clk API return per-user struct clk
> instances"), clk API users can no longer check if two struct clk
> pointers are pointing to the same hardware clock, i.e. struct clk_hw, by
> simply comparing two pointers.  That's because with the per-user clk
> change, a brand new struct clk is created whenever clients try to look
> up the clock by calling clk_get() or sister functions like clk_get_sys()
> and of_clk_get().  This changes the original behavior where the struct
> clk is only created for once when clock driver registers the clock to
> CCF in the first place.  The net change here is before commit
> 035a61c314eb the struct clk pointer is unique for given hardware
> clock, while after the commit the pointers returned by clk lookup calls
> become different for the same hardware clock.
> 
> That said, the struct clk pointer comparing in the code doesn't work any
> more.  Call helper function clk_is_match() instead to fix the problem.
> 
> Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> ---
>  drivers/pwm/pwm-atmel-hlcdc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Acked-by: Thierry Reding <thierry.reding@gmail.com>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150311/8ec93d5f/attachment.sig>

[PATCH] clk: provide clk_is_match() dummy for non-common clk

[Uwe Kleine-König]

Hey Russell,

On Wed, Mar 11, 2015 at 10:22:09AM +0000, Russell King - ARM Linux wrote:
> On Sun, Mar 08, 2015 at 10:05:29PM +0100, Arnd Bergmann wrote:
> > ARM randconfig build tests found a new error for configurations
> > with COMMON_CLK disabled but HAS_CLK selected by the platform:
> > 
> > ERROR: "clk_is_match" [sound/soc/fsl/snd-soc-fsl-spdif.ko] undefined!
> > 
> > This moves the declaration around, so this case is covered
> > by the existing static inline helper function.
> > 
> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> > Fixes: c69e182e51d89 ("clk: introduce clk_is_match")
> > ----
> > BTW, we have a preexisting problem in clk_get_parent,
> > clk_round_rate and clk_set_parent, which I've worked around in
> > my randconfig builds so far. Should we do that the same way?
> 
> NAK, as Uwe points out, you didn't address my comment.
You commented on the patch that is c69e182e51d8 ("clk: introduce
clk_is_match") now in next. Arnd just moved this around.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[PATCH] clk: provide clk_is_match() dummy for non-common clk

[Russell King - ARM Linux]

On Wed, Mar 11, 2015 at 12:17:55PM +0100, Uwe Kleine-K?nig wrote:
> Hey Russell,
> 
> On Wed, Mar 11, 2015 at 10:22:09AM +0000, Russell King - ARM Linux wrote:
> > On Sun, Mar 08, 2015 at 10:05:29PM +0100, Arnd Bergmann wrote:
> > > ARM randconfig build tests found a new error for configurations
> > > with COMMON_CLK disabled but HAS_CLK selected by the platform:
> > > 
> > > ERROR: "clk_is_match" [sound/soc/fsl/snd-soc-fsl-spdif.ko] undefined!
> > > 
> > > This moves the declaration around, so this case is covered
> > > by the existing static inline helper function.
> > > 
> > > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> > > Fixes: c69e182e51d89 ("clk: introduce clk_is_match")
> > > ----
> > > BTW, we have a preexisting problem in clk_get_parent,
> > > clk_round_rate and clk_set_parent, which I've worked around in
> > > my randconfig builds so far. Should we do that the same way?
> > 
> > NAK, as Uwe points out, you didn't address my comment.
> You commented on the patch that is c69e182e51d8 ("clk: introduce
> clk_is_match") now in next. Arnd just moved this around.

*Sigh*

Mike - please remove this commit until proper kernel patch process is
honoured.  We'll have some order here, and mutual respect of fellow
kernel developers, rather than people selectively ignoring people.
Yes, I realise that it fixes a bug, but it's utterly disgusting that
comments on a patch are ignored and it's just picked up irrespective
of comments being addressed.

If you don't like that, bloody well do a better job.

-- 
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.

[PATCH] clk: provide clk_is_match() dummy for non-common clk

[Stephen Boyd]

On 03/11/15 05:29, Russell King - ARM Linux wrote:
> On Wed, Mar 11, 2015 at 12:17:55PM +0100, Uwe Kleine-K?nig wrote:
>> Hey Russell,
>>
>> On Wed, Mar 11, 2015 at 10:22:09AM +0000, Russell King - ARM Linux wrote:
>>> On Sun, Mar 08, 2015 at 10:05:29PM +0100, Arnd Bergmann wrote:
>>>> ARM randconfig build tests found a new error for configurations
>>>> with COMMON_CLK disabled but HAS_CLK selected by the platform:
>>>>
>>>> ERROR: "clk_is_match" [sound/soc/fsl/snd-soc-fsl-spdif.ko] undefined!
>>>>
>>>> This moves the declaration around, so this case is covered
>>>> by the existing static inline helper function.
>>>>
>>>> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>>>> Fixes: c69e182e51d89 ("clk: introduce clk_is_match")
>>>> ----
>>>> BTW, we have a preexisting problem in clk_get_parent,
>>>> clk_round_rate and clk_set_parent, which I've worked around in
>>>> my randconfig builds so far. Should we do that the same way?
>>> NAK, as Uwe points out, you didn't address my comment.
>> You commented on the patch that is c69e182e51d8 ("clk: introduce
>> clk_is_match") now in next. Arnd just moved this around.
> *Sigh*
>
> Mike - please remove this commit until proper kernel patch process is
> honoured.  We'll have some order here, and mutual respect of fellow
> kernel developers, rather than people selectively ignoring people.
> Yes, I realise that it fixes a bug, but it's utterly disgusting that
> comments on a patch are ignored and it's just picked up irrespective
> of comments being addressed.
>
> If you don't like that, bloody well do a better job.
>

The patch was *not* picked up after your mail was sent. The patch was
picked up the same day it was posted to the list (Feb 25). You sent
review comments a week later (March 4). I don't see any selective
ignoring here.

I'll fold the const comments from Geert, your comments, and Arnd's fix
into the patch. Here's the new patch:

----8<----

From: Michael Turquette <mturquette@linaro.org>
Subject: [PATCH] clk: introduce clk_is_match

Some drivers compare struct clk pointers as a means of knowing
if the two pointers reference the same clock hardware. This behavior is
dubious (drivers must not dereference struct clk), but did not cause any
regressions until the per-user struct clk patch was merged. Now the test
for matching clk's will always fail with per-user struct clk's.

clk_is_match is introduced to fix the regression and prevent drivers
from comparing the pointers manually.

Fixes: 035a61c314eb ("clk: Make clk API return per-user struct clk instances")
Cc: Russell King <linux@arm.linux.org.uk>
Cc: Shawn Guo <shawn.guo@linaro.org>
Cc: Tomeu Vizoso <tomeu.vizoso@collabora.com>
Signed-off-by: Michael Turquette <mturquette@linaro.org>
[arnd at arndb.de: Fix COMMON_CLK=N && HAS_CLK=Y config]
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
[sboyd at codeaurora.org: const arguments to clk_is_match() and
remove unnecessary ternary operation]
Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
---
 drivers/clk/clk.c   | 26 ++++++++++++++++++++++++++
 include/linux/clk.h | 18 ++++++++++++++++++
 2 files changed, 44 insertions(+)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index b9f85fc2ce3f..237f23f68bfc 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -2170,6 +2170,32 @@ int clk_get_phase(struct clk *clk)
 }
 
 /**
+ * clk_is_match - check if two clk's point to the same hardware clock
+ * @p: clk compared against q
+ * @q: clk compared against p
+ *
+ * Returns true if the two struct clk pointers both point to the same hardware
+ * clock node. Put differently, returns true if struct clk *p and struct clk *q
+ * share the same struct clk_core object.
+ *
+ * Returns false otherwise. Note that two NULL clks are treated as matching.
+ */
+bool clk_is_match(const struct clk *p, const struct clk *q)
+{
+	/* trivial case: identical struct clk's or both NULL */
+	if (p == q)
+		return true;
+
+	/* true if clk->core pointers match. Avoid derefing garbage */
+	if (!IS_ERR_OR_NULL(p) && !IS_ERR_OR_NULL(q))
+		if (p->core == q->core)
+			return true;
+
+	return false;
+}
+EXPORT_SYMBOL_GPL(clk_is_match);
+
+/**
  * __clk_init - initialize the data structures in a struct clk
  * @dev:	device initializing this clk, placeholder for now
  * @clk:	clk being initialized
diff --git a/include/linux/clk.h b/include/linux/clk.h
index 8381bbfbc308..68c16a6bedb3 100644
--- a/include/linux/clk.h
+++ b/include/linux/clk.h
@@ -125,6 +125,19 @@ int clk_set_phase(struct clk *clk, int degrees);
  */
 int clk_get_phase(struct clk *clk);
 
+/**
+ * clk_is_match - check if two clk's point to the same hardware clock
+ * @p: clk compared against q
+ * @q: clk compared against p
+ *
+ * Returns true if the two struct clk pointers both point to the same hardware
+ * clock node. Put differently, returns true if struct clk *p and struct clk *q
+ * share the same struct clk_core object.
+ *
+ * Returns false otherwise. Note that two NULL clks are treated as matching.
+ */
+bool clk_is_match(const struct clk *p, const struct clk *q);
+
 #else
 
 static inline long clk_get_accuracy(struct clk *clk)
@@ -142,6 +155,11 @@ static inline long clk_get_phase(struct clk *clk)
 	return -ENOTSUPP;
 }
 
+static inline bool clk_is_match(const struct clk *p, const struct clk *q)
+{
+	return p == q;
+}
+
 #endif
 
 /**

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project