* [PATCH 1/3] i2c: uniphier: avoid WARN_ON() of clk_disable() in failure path
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 ` [PATCH 2/3] i2c: uniphier-f: " Masahiro Yamada
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Masahiro Yamada @ 2016-09-01 11:46 UTC (permalink / raw)
To: linux-i2c; +Cc: Masahiro Yamada, linux-arm-kernel, linux-kernel, Wolfram Sang
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
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH 2/3] i2c: uniphier-f: avoid WARN_ON() of clk_disable() in failure path
2016-09-01 11:46 [PATCH 0/3] i2c: uniphier: updates for v4.9 Masahiro Yamada
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 3/3] i2c: uniphier-f: set the adapter to master mode when probing Masahiro Yamada
2016-09-08 20:41 ` [PATCH 0/3] i2c: uniphier: updates for v4.9 Wolfram Sang
3 siblings, 0 replies; 5+ messages in thread
From: Masahiro Yamada @ 2016-09-01 11:46 UTC (permalink / raw)
To: linux-i2c; +Cc: Masahiro Yamada, linux-arm-kernel, linux-kernel, Wolfram Sang
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-f.c | 73 ++++++++++++++++---------------------
1 file changed, 32 insertions(+), 41 deletions(-)
diff --git a/drivers/i2c/busses/i2c-uniphier-f.c b/drivers/i2c/busses/i2c-uniphier-f.c
index 3560853..2886685 100644
--- a/drivers/i2c/busses/i2c-uniphier-f.c
+++ b/drivers/i2c/busses/i2c-uniphier-f.c
@@ -455,41 +455,10 @@ static struct i2c_bus_recovery_info uniphier_fi2c_bus_recovery_info = {
.unprepare_recovery = uniphier_fi2c_unprepare_recovery,
};
-static int uniphier_fi2c_clk_init(struct device *dev,
- struct uniphier_fi2c_priv *priv)
+static void uniphier_fi2c_hw_init(struct uniphier_fi2c_priv *priv,
+ u32 bus_speed, unsigned long clk_rate)
{
- struct device_node *np = dev->of_node;
- unsigned long clk_rate;
- u32 bus_speed, clk_count;
- int ret;
-
- if (of_property_read_u32(np, "clock-frequency", &bus_speed))
- bus_speed = UNIPHIER_FI2C_DEFAULT_SPEED;
-
- if (!bus_speed) {
- dev_err(dev, "clock-frequency should not be zero\n");
- return -EINVAL;
- }
-
- if (bus_speed > UNIPHIER_FI2C_MAX_SPEED)
- bus_speed = UNIPHIER_FI2C_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;
- }
+ u32 clk_count;
uniphier_fi2c_reset(priv);
@@ -501,8 +470,6 @@ static int uniphier_fi2c_clk_init(struct device *dev,
writel(clk_count / 16, priv->membase + UNIPHIER_FI2C_DSUT);
uniphier_fi2c_prepare_operation(priv);
-
- return 0;
}
static int uniphier_fi2c_probe(struct platform_device *pdev)
@@ -510,8 +477,9 @@ static int uniphier_fi2c_probe(struct platform_device *pdev)
struct device *dev = &pdev->dev;
struct uniphier_fi2c_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)
@@ -528,6 +496,31 @@ static int uniphier_fi2c_probe(struct platform_device *pdev)
return irq;
}
+ if (of_property_read_u32(dev->of_node, "clock-frequency", &bus_speed))
+ bus_speed = UNIPHIER_FI2C_DEFAULT_SPEED;
+
+ if (!bus_speed || bus_speed > UNIPHIER_FI2C_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_fi2c_algo;
@@ -538,9 +531,7 @@ static int uniphier_fi2c_probe(struct platform_device *pdev)
i2c_set_adapdata(&priv->adap, priv);
platform_set_drvdata(pdev, priv);
- ret = uniphier_fi2c_clk_init(dev, priv);
- if (ret)
- goto err;
+ uniphier_fi2c_hw_init(priv, bus_speed, clk_rate);
ret = devm_request_irq(dev, irq, uniphier_fi2c_interrupt, 0,
pdev->name, priv);
--
1.9.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH 3/3] i2c: uniphier-f: set the adapter to master mode when probing
2016-09-01 11:46 [PATCH 0/3] i2c: uniphier: updates for v4.9 Masahiro Yamada
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 ` [PATCH 2/3] i2c: uniphier-f: " 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
3 siblings, 0 replies; 5+ messages in thread
From: Masahiro Yamada @ 2016-09-01 11:46 UTC (permalink / raw)
To: linux-i2c; +Cc: Masahiro Yamada, linux-arm-kernel, linux-kernel, Wolfram Sang
Currently, the adapter is set to the master mode at the first use.
Since then, it is kept in the slave mode, so unexpected glitch
signals on the I2C lines may cause the adapter into insane state.
Setting it to the master mode along with initialization solves the
problem.
Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
Reported-by: Akio Noda <noda.akio@socionext.com>
---
drivers/i2c/busses/i2c-uniphier-f.c | 16 ++++++++++------
1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/drivers/i2c/busses/i2c-uniphier-f.c b/drivers/i2c/busses/i2c-uniphier-f.c
index 2886685..829df91 100644
--- a/drivers/i2c/busses/i2c-uniphier-f.c
+++ b/drivers/i2c/busses/i2c-uniphier-f.c
@@ -458,16 +458,20 @@ static struct i2c_bus_recovery_info uniphier_fi2c_bus_recovery_info = {
static void uniphier_fi2c_hw_init(struct uniphier_fi2c_priv *priv,
u32 bus_speed, unsigned long clk_rate)
{
- u32 clk_count;
+ u32 tmp;
+
+ tmp = readl(priv->membase + UNIPHIER_FI2C_CR);
+ tmp |= UNIPHIER_FI2C_CR_MST;
+ writel(tmp, priv->membase + UNIPHIER_FI2C_CR);
uniphier_fi2c_reset(priv);
- clk_count = clk_rate / bus_speed;
+ tmp = clk_rate / bus_speed;
- writel(clk_count, priv->membase + UNIPHIER_FI2C_CYC);
- writel(clk_count / 2, priv->membase + UNIPHIER_FI2C_LCTL);
- writel(clk_count / 2, priv->membase + UNIPHIER_FI2C_SSUT);
- writel(clk_count / 16, priv->membase + UNIPHIER_FI2C_DSUT);
+ writel(tmp, priv->membase + UNIPHIER_FI2C_CYC);
+ writel(tmp / 2, priv->membase + UNIPHIER_FI2C_LCTL);
+ writel(tmp / 2, priv->membase + UNIPHIER_FI2C_SSUT);
+ writel(tmp / 16, priv->membase + UNIPHIER_FI2C_DSUT);
uniphier_fi2c_prepare_operation(priv);
}
--
1.9.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH 0/3] i2c: uniphier: updates for v4.9
2016-09-01 11:46 [PATCH 0/3] i2c: uniphier: updates for v4.9 Masahiro Yamada
` (2 preceding siblings ...)
2016-09-01 11:46 ` [PATCH 3/3] i2c: uniphier-f: set the adapter to master mode when probing Masahiro Yamada
@ 2016-09-08 20:41 ` Wolfram Sang
3 siblings, 0 replies; 5+ messages in thread
From: Wolfram Sang @ 2016-09-08 20:41 UTC (permalink / raw)
To: Masahiro Yamada; +Cc: linux-i2c, linux-arm-kernel, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 346 bytes --]
On Thu, Sep 01, 2016 at 08:46:27PM +0900, Masahiro Yamada wrote:
>
>
>
> Masahiro Yamada (3):
> i2c: uniphier: avoid WARN_ON() of clk_disable() in failure path
> i2c: uniphier-f: avoid WARN_ON() of clk_disable() in failure path
> i2c: uniphier-f: set the adapter to master mode when probing
>
Applied to for-next, thanks!
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread