All of lore.kernel.org
 help / color / mirror / Atom feed
From: Masahiro Yamada <yamada.masahiro@socionext.com>
To: linux-i2c@vger.kernel.org
Cc: Masahiro Yamada <yamada.masahiro@socionext.com>,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, Wolfram Sang <wsa@the-dreams.de>
Subject: [PATCH 1/3] i2c: uniphier: avoid WARN_ON() of clk_disable() in failure path
Date: Thu,  1 Sep 2016 20:46:28 +0900	[thread overview]
Message-ID: <1472730390-8907-2-git-send-email-yamada.masahiro@socionext.com> (raw)
In-Reply-To: <1472730390-8907-1-git-send-email-yamada.masahiro@socionext.com>

If clk_prepare_enable() fails, clk_disable_unprepare() is called in
the failure path, where the enable_count is still zero, so it hits
WARN_ON(core->enable_count == 0) in the clk_core_disable().

To fix this, make the clock setting more linear in the probe function
so that it can exploit "goto err" in case of error.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

 drivers/i2c/busses/i2c-uniphier.c | 73 +++++++++++++++++----------------------
 1 file changed, 31 insertions(+), 42 deletions(-)

diff --git a/drivers/i2c/busses/i2c-uniphier.c b/drivers/i2c/busses/i2c-uniphier.c
index d6e612a..56e92af 100644
--- a/drivers/i2c/busses/i2c-uniphier.c
+++ b/drivers/i2c/busses/i2c-uniphier.c
@@ -316,50 +316,15 @@ static struct i2c_bus_recovery_info uniphier_i2c_bus_recovery_info = {
 	.unprepare_recovery = uniphier_i2c_unprepare_recovery,
 };
 
-static int uniphier_i2c_clk_init(struct device *dev,
-				 struct uniphier_i2c_priv *priv)
+static void uniphier_i2c_hw_init(struct uniphier_i2c_priv *priv,
+				 u32 bus_speed, unsigned long clk_rate)
 {
-	struct device_node *np = dev->of_node;
-	unsigned long clk_rate;
-	u32 bus_speed;
-	int ret;
-
-	if (of_property_read_u32(np, "clock-frequency", &bus_speed))
-		bus_speed = UNIPHIER_I2C_DEFAULT_SPEED;
-
-	if (!bus_speed) {
-		dev_err(dev, "clock-frequency should not be zero\n");
-		return -EINVAL;
-	}
-
-	if (bus_speed > UNIPHIER_I2C_MAX_SPEED)
-		bus_speed = UNIPHIER_I2C_MAX_SPEED;
-
-	/* Get input clk rate through clk driver */
-	priv->clk = devm_clk_get(dev, NULL);
-	if (IS_ERR(priv->clk)) {
-		dev_err(dev, "failed to get clock\n");
-		return PTR_ERR(priv->clk);
-	}
-
-	ret = clk_prepare_enable(priv->clk);
-	if (ret)
-		return ret;
-
-	clk_rate = clk_get_rate(priv->clk);
-	if (!clk_rate) {
-		dev_err(dev, "input clock rate should not be zero\n");
-		return -EINVAL;
-	}
-
 	uniphier_i2c_reset(priv, true);
 
 	writel((clk_rate / bus_speed / 2 << 16) | (clk_rate / bus_speed),
 	       priv->membase + UNIPHIER_I2C_CLK);
 
 	uniphier_i2c_reset(priv, false);
-
-	return 0;
 }
 
 static int uniphier_i2c_probe(struct platform_device *pdev)
@@ -367,8 +332,9 @@ static int uniphier_i2c_probe(struct platform_device *pdev)
 	struct device *dev = &pdev->dev;
 	struct uniphier_i2c_priv *priv;
 	struct resource *regs;
-	int irq;
-	int ret;
+	u32 bus_speed;
+	unsigned long clk_rate;
+	int irq, ret;
 
 	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
 	if (!priv)
@@ -385,6 +351,31 @@ static int uniphier_i2c_probe(struct platform_device *pdev)
 		return irq;
 	}
 
+	if (of_property_read_u32(dev->of_node, "clock-frequency", &bus_speed))
+		bus_speed = UNIPHIER_I2C_DEFAULT_SPEED;
+
+	if (!bus_speed || bus_speed > UNIPHIER_I2C_MAX_SPEED) {
+		dev_err(dev, "invalid clock-frequency %d\n", bus_speed);
+		return -EINVAL;
+	}
+
+	priv->clk = devm_clk_get(dev, NULL);
+	if (IS_ERR(priv->clk)) {
+		dev_err(dev, "failed to get clock\n");
+		return PTR_ERR(priv->clk);
+	}
+
+	ret = clk_prepare_enable(priv->clk);
+	if (ret)
+		return ret;
+
+	clk_rate = clk_get_rate(priv->clk);
+	if (!clk_rate) {
+		dev_err(dev, "input clock rate should not be zero\n");
+		ret = -EINVAL;
+		goto err;
+	}
+
 	init_completion(&priv->comp);
 	priv->adap.owner = THIS_MODULE;
 	priv->adap.algo = &uniphier_i2c_algo;
@@ -395,9 +386,7 @@ static int uniphier_i2c_probe(struct platform_device *pdev)
 	i2c_set_adapdata(&priv->adap, priv);
 	platform_set_drvdata(pdev, priv);
 
-	ret = uniphier_i2c_clk_init(dev, priv);
-	if (ret)
-		goto err;
+	uniphier_i2c_hw_init(priv, bus_speed, clk_rate);
 
 	ret = devm_request_irq(dev, irq, uniphier_i2c_interrupt, 0, pdev->name,
 			       priv);
-- 
1.9.1

WARNING: multiple messages have this Message-ID (diff)
From: Masahiro Yamada <yamada.masahiro@socionext.com>
To: linux-i2c@vger.kernel.org
Cc: Masahiro Yamada <yamada.masahiro@socionext.com>,
	linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	Wolfram Sang <wsa@the-dreams.de>
Subject: [PATCH 1/3] i2c: uniphier: avoid WARN_ON() of clk_disable() in failure path
Date: Thu,  1 Sep 2016 20:46:28 +0900	[thread overview]
Message-ID: <1472730390-8907-2-git-send-email-yamada.masahiro@socionext.com> (raw)
In-Reply-To: <1472730390-8907-1-git-send-email-yamada.masahiro@socionext.com>

If clk_prepare_enable() fails, clk_disable_unprepare() is called in
the failure path, where the enable_count is still zero, so it hits
WARN_ON(core->enable_count == 0) in the clk_core_disable().

To fix this, make the clock setting more linear in the probe function
so that it can exploit "goto err" in case of error.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

 drivers/i2c/busses/i2c-uniphier.c | 73 +++++++++++++++++----------------------
 1 file changed, 31 insertions(+), 42 deletions(-)

diff --git a/drivers/i2c/busses/i2c-uniphier.c b/drivers/i2c/busses/i2c-uniphier.c
index d6e612a..56e92af 100644
--- a/drivers/i2c/busses/i2c-uniphier.c
+++ b/drivers/i2c/busses/i2c-uniphier.c
@@ -316,50 +316,15 @@ static struct i2c_bus_recovery_info uniphier_i2c_bus_recovery_info = {
 	.unprepare_recovery = uniphier_i2c_unprepare_recovery,
 };
 
-static int uniphier_i2c_clk_init(struct device *dev,
-				 struct uniphier_i2c_priv *priv)
+static void uniphier_i2c_hw_init(struct uniphier_i2c_priv *priv,
+				 u32 bus_speed, unsigned long clk_rate)
 {
-	struct device_node *np = dev->of_node;
-	unsigned long clk_rate;
-	u32 bus_speed;
-	int ret;
-
-	if (of_property_read_u32(np, "clock-frequency", &bus_speed))
-		bus_speed = UNIPHIER_I2C_DEFAULT_SPEED;
-
-	if (!bus_speed) {
-		dev_err(dev, "clock-frequency should not be zero\n");
-		return -EINVAL;
-	}
-
-	if (bus_speed > UNIPHIER_I2C_MAX_SPEED)
-		bus_speed = UNIPHIER_I2C_MAX_SPEED;
-
-	/* Get input clk rate through clk driver */
-	priv->clk = devm_clk_get(dev, NULL);
-	if (IS_ERR(priv->clk)) {
-		dev_err(dev, "failed to get clock\n");
-		return PTR_ERR(priv->clk);
-	}
-
-	ret = clk_prepare_enable(priv->clk);
-	if (ret)
-		return ret;
-
-	clk_rate = clk_get_rate(priv->clk);
-	if (!clk_rate) {
-		dev_err(dev, "input clock rate should not be zero\n");
-		return -EINVAL;
-	}
-
 	uniphier_i2c_reset(priv, true);
 
 	writel((clk_rate / bus_speed / 2 << 16) | (clk_rate / bus_speed),
 	       priv->membase + UNIPHIER_I2C_CLK);
 
 	uniphier_i2c_reset(priv, false);
-
-	return 0;
 }
 
 static int uniphier_i2c_probe(struct platform_device *pdev)
@@ -367,8 +332,9 @@ static int uniphier_i2c_probe(struct platform_device *pdev)
 	struct device *dev = &pdev->dev;
 	struct uniphier_i2c_priv *priv;
 	struct resource *regs;
-	int irq;
-	int ret;
+	u32 bus_speed;
+	unsigned long clk_rate;
+	int irq, ret;
 
 	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
 	if (!priv)
@@ -385,6 +351,31 @@ static int uniphier_i2c_probe(struct platform_device *pdev)
 		return irq;
 	}
 
+	if (of_property_read_u32(dev->of_node, "clock-frequency", &bus_speed))
+		bus_speed = UNIPHIER_I2C_DEFAULT_SPEED;
+
+	if (!bus_speed || bus_speed > UNIPHIER_I2C_MAX_SPEED) {
+		dev_err(dev, "invalid clock-frequency %d\n", bus_speed);
+		return -EINVAL;
+	}
+
+	priv->clk = devm_clk_get(dev, NULL);
+	if (IS_ERR(priv->clk)) {
+		dev_err(dev, "failed to get clock\n");
+		return PTR_ERR(priv->clk);
+	}
+
+	ret = clk_prepare_enable(priv->clk);
+	if (ret)
+		return ret;
+
+	clk_rate = clk_get_rate(priv->clk);
+	if (!clk_rate) {
+		dev_err(dev, "input clock rate should not be zero\n");
+		ret = -EINVAL;
+		goto err;
+	}
+
 	init_completion(&priv->comp);
 	priv->adap.owner = THIS_MODULE;
 	priv->adap.algo = &uniphier_i2c_algo;
@@ -395,9 +386,7 @@ static int uniphier_i2c_probe(struct platform_device *pdev)
 	i2c_set_adapdata(&priv->adap, priv);
 	platform_set_drvdata(pdev, priv);
 
-	ret = uniphier_i2c_clk_init(dev, priv);
-	if (ret)
-		goto err;
+	uniphier_i2c_hw_init(priv, bus_speed, clk_rate);
 
 	ret = devm_request_irq(dev, irq, uniphier_i2c_interrupt, 0, pdev->name,
 			       priv);
-- 
1.9.1

WARNING: multiple messages have this Message-ID (diff)
From: yamada.masahiro@socionext.com (Masahiro Yamada)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 1/3] i2c: uniphier: avoid WARN_ON() of clk_disable() in failure path
Date: Thu,  1 Sep 2016 20:46:28 +0900	[thread overview]
Message-ID: <1472730390-8907-2-git-send-email-yamada.masahiro@socionext.com> (raw)
In-Reply-To: <1472730390-8907-1-git-send-email-yamada.masahiro@socionext.com>

If clk_prepare_enable() fails, clk_disable_unprepare() is called in
the failure path, where the enable_count is still zero, so it hits
WARN_ON(core->enable_count == 0) in the clk_core_disable().

To fix this, make the clock setting more linear in the probe function
so that it can exploit "goto err" in case of error.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

 drivers/i2c/busses/i2c-uniphier.c | 73 +++++++++++++++++----------------------
 1 file changed, 31 insertions(+), 42 deletions(-)

diff --git a/drivers/i2c/busses/i2c-uniphier.c b/drivers/i2c/busses/i2c-uniphier.c
index d6e612a..56e92af 100644
--- a/drivers/i2c/busses/i2c-uniphier.c
+++ b/drivers/i2c/busses/i2c-uniphier.c
@@ -316,50 +316,15 @@ static struct i2c_bus_recovery_info uniphier_i2c_bus_recovery_info = {
 	.unprepare_recovery = uniphier_i2c_unprepare_recovery,
 };
 
-static int uniphier_i2c_clk_init(struct device *dev,
-				 struct uniphier_i2c_priv *priv)
+static void uniphier_i2c_hw_init(struct uniphier_i2c_priv *priv,
+				 u32 bus_speed, unsigned long clk_rate)
 {
-	struct device_node *np = dev->of_node;
-	unsigned long clk_rate;
-	u32 bus_speed;
-	int ret;
-
-	if (of_property_read_u32(np, "clock-frequency", &bus_speed))
-		bus_speed = UNIPHIER_I2C_DEFAULT_SPEED;
-
-	if (!bus_speed) {
-		dev_err(dev, "clock-frequency should not be zero\n");
-		return -EINVAL;
-	}
-
-	if (bus_speed > UNIPHIER_I2C_MAX_SPEED)
-		bus_speed = UNIPHIER_I2C_MAX_SPEED;
-
-	/* Get input clk rate through clk driver */
-	priv->clk = devm_clk_get(dev, NULL);
-	if (IS_ERR(priv->clk)) {
-		dev_err(dev, "failed to get clock\n");
-		return PTR_ERR(priv->clk);
-	}
-
-	ret = clk_prepare_enable(priv->clk);
-	if (ret)
-		return ret;
-
-	clk_rate = clk_get_rate(priv->clk);
-	if (!clk_rate) {
-		dev_err(dev, "input clock rate should not be zero\n");
-		return -EINVAL;
-	}
-
 	uniphier_i2c_reset(priv, true);
 
 	writel((clk_rate / bus_speed / 2 << 16) | (clk_rate / bus_speed),
 	       priv->membase + UNIPHIER_I2C_CLK);
 
 	uniphier_i2c_reset(priv, false);
-
-	return 0;
 }
 
 static int uniphier_i2c_probe(struct platform_device *pdev)
@@ -367,8 +332,9 @@ static int uniphier_i2c_probe(struct platform_device *pdev)
 	struct device *dev = &pdev->dev;
 	struct uniphier_i2c_priv *priv;
 	struct resource *regs;
-	int irq;
-	int ret;
+	u32 bus_speed;
+	unsigned long clk_rate;
+	int irq, ret;
 
 	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
 	if (!priv)
@@ -385,6 +351,31 @@ static int uniphier_i2c_probe(struct platform_device *pdev)
 		return irq;
 	}
 
+	if (of_property_read_u32(dev->of_node, "clock-frequency", &bus_speed))
+		bus_speed = UNIPHIER_I2C_DEFAULT_SPEED;
+
+	if (!bus_speed || bus_speed > UNIPHIER_I2C_MAX_SPEED) {
+		dev_err(dev, "invalid clock-frequency %d\n", bus_speed);
+		return -EINVAL;
+	}
+
+	priv->clk = devm_clk_get(dev, NULL);
+	if (IS_ERR(priv->clk)) {
+		dev_err(dev, "failed to get clock\n");
+		return PTR_ERR(priv->clk);
+	}
+
+	ret = clk_prepare_enable(priv->clk);
+	if (ret)
+		return ret;
+
+	clk_rate = clk_get_rate(priv->clk);
+	if (!clk_rate) {
+		dev_err(dev, "input clock rate should not be zero\n");
+		ret = -EINVAL;
+		goto err;
+	}
+
 	init_completion(&priv->comp);
 	priv->adap.owner = THIS_MODULE;
 	priv->adap.algo = &uniphier_i2c_algo;
@@ -395,9 +386,7 @@ static int uniphier_i2c_probe(struct platform_device *pdev)
 	i2c_set_adapdata(&priv->adap, priv);
 	platform_set_drvdata(pdev, priv);
 
-	ret = uniphier_i2c_clk_init(dev, priv);
-	if (ret)
-		goto err;
+	uniphier_i2c_hw_init(priv, bus_speed, clk_rate);
 
 	ret = devm_request_irq(dev, irq, uniphier_i2c_interrupt, 0, pdev->name,
 			       priv);
-- 
1.9.1

  reply	other threads:[~2016-09-01 11:45 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-09-01 11:46 [PATCH 0/3] i2c: uniphier: updates for v4.9 Masahiro Yamada
2016-09-01 11:46 ` Masahiro Yamada
2016-09-01 11:46 ` Masahiro Yamada [this message]
2016-09-01 11:46   ` [PATCH 1/3] i2c: uniphier: avoid WARN_ON() of clk_disable() in failure path Masahiro Yamada
2016-09-01 11:46   ` Masahiro Yamada
2016-09-01 11:46 ` [PATCH 2/3] i2c: uniphier-f: " Masahiro Yamada
2016-09-01 11:46   ` Masahiro Yamada
2016-09-01 11:46 ` [PATCH 3/3] i2c: uniphier-f: set the adapter to master mode when probing Masahiro Yamada
2016-09-01 11:46   ` Masahiro Yamada
2016-09-08 20:41 ` [PATCH 0/3] i2c: uniphier: updates for v4.9 Wolfram Sang
2016-09-08 20:41   ` Wolfram Sang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1472730390-8907-2-git-send-email-yamada.masahiro@socionext.com \
    --to=yamada.masahiro@socionext.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-i2c@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=wsa@the-dreams.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.