[PATCH 0/4] i2c: i2c-mv64xxx: misc small improvements

[PATCH 0/4] i2c: i2c-mv64xxx: misc small improvements

[Thomas Petazzoni]

Hello,

Here are four small patches doing misc improvements to the i2c-mv64xxx
I2C controller driver. The first two patches are really needed in
order to be able to use this I2C controller driver on the ARM64
Marvell Armada 7K/8K platforms. The last two patches are additional
cleanups/improvements made along the way.

Thanks!

Thomas

Thomas Petazzoni (4):
  i2c: i2c-mv64xxx: enable the driver on ARCH_MVEBU
  i2c: i2c-mv64xxx: handle probe deferral for the clock
  i2c: i2c-mv64xxx: use clk_{prepare_enable,disable_unprepare}
  i2c: i2c-mv64xxx: remove CONFIG_HAVE_CLK conditionals

 drivers/i2c/busses/Kconfig       |  2 +-
 drivers/i2c/busses/i2c-mv64xxx.c | 45 +++++++++++++---------------------------
 2 files changed, 15 insertions(+), 32 deletions(-)

-- 
2.6.4

[PATCH 1/4] i2c: i2c-mv64xxx: enable the driver on ARCH_MVEBU

[Thomas Petazzoni]

The new ARM64 Marvell Armada 7K/8K SoC family is using the same I2C
controller as the 32-bits Marvell EBU SoCs, so this commit allows
mv64xxx to be enabled when ARCH_MVEBU=y.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 drivers/i2c/busses/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index faa8e68..2966433 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -663,7 +663,7 @@ config I2C_MT65XX
 
 config I2C_MV64XXX
 	tristate "Marvell mv64xxx I2C Controller"
-	depends on MV64X60 || PLAT_ORION || ARCH_SUNXI
+	depends on MV64X60 || PLAT_ORION || ARCH_SUNXI || ARCH_MVEBU
 	help
 	  If you say yes to this option, support will be included for the
 	  built-in I2C interface on the Marvell 64xxx line of host bridges.
-- 
2.6.4

[PATCH 2/4] i2c: i2c-mv64xxx: handle probe deferral for the clock

[Thomas Petazzoni]

If a clock is registered by a platform driver and not by the
OF_CLK_DECLARE() mechanism, it might show up after the first attempt
to probe i2c-mv64xxx. In order to solve this, we need to handle
-EPROBE_PREFER as a special return value of devm_clk_get(), and return
the same error code from probe().

This gives us three situations:

 - There is no reference to a clock in the DT. In this case,
   devm_clk_get() returns an error that is not -EPROBE_DEFER
   (something like -ENODEV), and we continue the probing without
   enabling the clock.

 - There is a reference to the clock in the DT, and the clock is
   ready. devm_clk_get() returns a valid reference to the clock, and
   we prepare/enable it.

 - There is a reference to the clock in the DT, but the clock is not
   ready. devm_clk_get() returns -EPROBE_DEFER, and we exit from
   probe() with the same error code so that probe() is tried again
   later.

This is needed for Marvell Armada 7K/8K, where the clock driver is a
platform driver.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 drivers/i2c/busses/i2c-mv64xxx.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/i2c/busses/i2c-mv64xxx.c b/drivers/i2c/busses/i2c-mv64xxx.c
index 43207f5..4c6282a 100644
--- a/drivers/i2c/busses/i2c-mv64xxx.c
+++ b/drivers/i2c/busses/i2c-mv64xxx.c
@@ -910,6 +910,8 @@ mv64xxx_i2c_probe(struct platform_device *pd)
 #if defined(CONFIG_HAVE_CLK)
 	/* Not all platforms have a clk */
 	drv_data->clk = devm_clk_get(&pd->dev, NULL);
+	if (IS_ERR(drv_data->clk) && PTR_ERR(drv_data->clk) == -EPROBE_DEFER)
+		return -EPROBE_DEFER;
 	if (!IS_ERR(drv_data->clk)) {
 		clk_prepare(drv_data->clk);
 		clk_enable(drv_data->clk);
-- 
2.6.4

[PATCH 3/4] i2c: i2c-mv64xxx: use clk_{prepare_enable,disable_unprepare}

[Thomas Petazzoni]

Instead of separately calling clk_prepare()/clk_enable(), use
clk_prepare_enable(), and instead of calling
clk_disable()/clk_unprepare(), use clk_disable_unprepare(). Those
handy shortcuts have been introduced specifically to simplify the
numerous call sites were both functions were called in sequence.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 drivers/i2c/busses/i2c-mv64xxx.c | 18 ++++++------------
 1 file changed, 6 insertions(+), 12 deletions(-)

diff --git a/drivers/i2c/busses/i2c-mv64xxx.c b/drivers/i2c/busses/i2c-mv64xxx.c
index 4c6282a..e87db03 100644
--- a/drivers/i2c/busses/i2c-mv64xxx.c
+++ b/drivers/i2c/busses/i2c-mv64xxx.c
@@ -912,10 +912,8 @@ mv64xxx_i2c_probe(struct platform_device *pd)
 	drv_data->clk = devm_clk_get(&pd->dev, NULL);
 	if (IS_ERR(drv_data->clk) && PTR_ERR(drv_data->clk) == -EPROBE_DEFER)
 		return -EPROBE_DEFER;
-	if (!IS_ERR(drv_data->clk)) {
-		clk_prepare(drv_data->clk);
-		clk_enable(drv_data->clk);
-	}
+	if (!IS_ERR(drv_data->clk))
+		clk_prepare_enable(drv_data->clk);
 #endif
 	if (pdata) {
 		drv_data->freq_m = pdata->freq_m;
@@ -968,10 +966,8 @@ exit_reset:
 exit_clk:
 #if defined(CONFIG_HAVE_CLK)
 	/* Not all platforms have a clk */
-	if (!IS_ERR(drv_data->clk)) {
-		clk_disable(drv_data->clk);
-		clk_unprepare(drv_data->clk);
-	}
+	if (!IS_ERR(drv_data->clk))
+		clk_disable_unprepare(drv_data->clk);
 #endif
 	return rc;
 }
@@ -987,10 +983,8 @@ mv64xxx_i2c_remove(struct platform_device *dev)
 		reset_control_assert(drv_data->rstc);
 #if defined(CONFIG_HAVE_CLK)
 	/* Not all platforms have a clk */
-	if (!IS_ERR(drv_data->clk)) {
-		clk_disable(drv_data->clk);
-		clk_unprepare(drv_data->clk);
-	}
+	if (!IS_ERR(drv_data->clk))
+		clk_disable_unprepare(drv_data->clk);
 #endif
 
 	return 0;
-- 
2.6.4

[PATCH 4/4] i2c: i2c-mv64xxx: remove CONFIG_HAVE_CLK conditionals

[Thomas Petazzoni]

When clock support was added to the i2c-mv64xxx, not all clk functions
had stubs when for !CONFIG_HAVE_CLK configurations. However, nowadays,
both "struct clk" and all the clock framework functions have stubs
when CONFIG_HAVE_CLK is not enabled, so it no longer makes sense to
carry such compile-time conditionals in the driver.

This commit was compile tested on both ARM64 (which has both
CONFIG_OF=y and CONFIG_HAVE_CLK=y) and PowerPC c2k_defconfig (which
has CONFIG_OF=y, CONFIG_HAVE_CLK disabled, and the i2c-mv64xxx driver
enabled).

The only non-trivial change is in the mv64xxx_of_config() function,
which was returning -ENODEV unconditionally if CONFIG_HAVE_CLK was
disabled. Simply removing this condition works fine because the first
test done by the function is to verify if drv_data->clk points to a
valid clock, and if it doesn't, we return -ENODEV. When
CONFIG_HAVE_CLK is disabled, devm_clk_get() unconditionally returns
NULL, so mv64xxx_of_config() will return -ENODEV when no clock is
provided, which is the intended behavior.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 drivers/i2c/busses/i2c-mv64xxx.c | 25 ++++++-------------------
 1 file changed, 6 insertions(+), 19 deletions(-)

diff --git a/drivers/i2c/busses/i2c-mv64xxx.c b/drivers/i2c/busses/i2c-mv64xxx.c
index e87db03..b4dec08 100644
--- a/drivers/i2c/busses/i2c-mv64xxx.c
+++ b/drivers/i2c/busses/i2c-mv64xxx.c
@@ -134,9 +134,7 @@ struct mv64xxx_i2c_data {
 	int			rc;
 	u32			freq_m;
 	u32			freq_n;
-#if defined(CONFIG_HAVE_CLK)
 	struct clk              *clk;
-#endif
 	wait_queue_head_t	waitq;
 	spinlock_t		lock;
 	struct i2c_msg		*msg;
@@ -757,7 +755,6 @@ static const struct of_device_id mv64xxx_i2c_of_match_table[] = {
 MODULE_DEVICE_TABLE(of, mv64xxx_i2c_of_match_table);
 
 #ifdef CONFIG_OF
-#ifdef CONFIG_HAVE_CLK
 static int
 mv64xxx_calc_freq(struct mv64xxx_i2c_data *drv_data,
 		  const int tclk, const int n, const int m)
@@ -791,25 +788,20 @@ mv64xxx_find_baud_factors(struct mv64xxx_i2c_data *drv_data,
 		return false;
 	return true;
 }
-#endif /* CONFIG_HAVE_CLK */
 
 static int
 mv64xxx_of_config(struct mv64xxx_i2c_data *drv_data,
 		  struct device *dev)
 {
-	/* CLK is mandatory when using DT to describe the i2c bus. We
-	 * need to know tclk in order to calculate bus clock
-	 * factors.
-	 */
-#if !defined(CONFIG_HAVE_CLK)
-	/* Have OF but no CLK */
-	return -ENODEV;
-#else
 	const struct of_device_id *device;
 	struct device_node *np = dev->of_node;
 	u32 bus_freq, tclk;
 	int rc = 0;
 
+	/* CLK is mandatory when using DT to describe the i2c bus. We
+	 * need to know tclk in order to calculate bus clock
+	 * factors.
+	 */
 	if (IS_ERR(drv_data->clk)) {
 		rc = -ENODEV;
 		goto out;
@@ -869,7 +861,6 @@ mv64xxx_of_config(struct mv64xxx_i2c_data *drv_data,
 
 out:
 	return rc;
-#endif
 }
 #else /* CONFIG_OF */
 static int
@@ -907,14 +898,13 @@ mv64xxx_i2c_probe(struct platform_device *pd)
 	init_waitqueue_head(&drv_data->waitq);
 	spin_lock_init(&drv_data->lock);
 
-#if defined(CONFIG_HAVE_CLK)
 	/* Not all platforms have a clk */
 	drv_data->clk = devm_clk_get(&pd->dev, NULL);
 	if (IS_ERR(drv_data->clk) && PTR_ERR(drv_data->clk) == -EPROBE_DEFER)
 		return -EPROBE_DEFER;
 	if (!IS_ERR(drv_data->clk))
 		clk_prepare_enable(drv_data->clk);
-#endif
+
 	if (pdata) {
 		drv_data->freq_m = pdata->freq_m;
 		drv_data->freq_n = pdata->freq_n;
@@ -964,11 +954,10 @@ exit_reset:
 	if (!IS_ERR_OR_NULL(drv_data->rstc))
 		reset_control_assert(drv_data->rstc);
 exit_clk:
-#if defined(CONFIG_HAVE_CLK)
 	/* Not all platforms have a clk */
 	if (!IS_ERR(drv_data->clk))
 		clk_disable_unprepare(drv_data->clk);
-#endif
+
 	return rc;
 }
 
@@ -981,11 +970,9 @@ mv64xxx_i2c_remove(struct platform_device *dev)
 	free_irq(drv_data->irq, drv_data);
 	if (!IS_ERR_OR_NULL(drv_data->rstc))
 		reset_control_assert(drv_data->rstc);
-#if defined(CONFIG_HAVE_CLK)
 	/* Not all platforms have a clk */
 	if (!IS_ERR(drv_data->clk))
 		clk_disable_unprepare(drv_data->clk);
-#endif
 
 	return 0;
 }
-- 
2.6.4

Re: [PATCH 0/4] i2c: i2c-mv64xxx: misc small improvements

[Wolfram Sang]

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

On Fri, Apr 22, 2016 at 03:19:50PM +0200, Thomas Petazzoni wrote:
> Hello,
> 
> Here are four small patches doing misc improvements to the i2c-mv64xxx
> I2C controller driver. The first two patches are really needed in
> order to be able to use this I2C controller driver on the ARM64
> Marvell Armada 7K/8K platforms. The last two patches are additional
> cleanups/improvements made along the way.

All applied to for-next, thanks!


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