Patchwork [v2,1/5] i2c: mv64xxx: Add reset deassert call

login
register
mail settings
Submitter Maxime Ripard
Date Jan. 13, 2014, 10:34 a.m.
Message ID <1389609293-2824-2-git-send-email-maxime.ripard@free-electrons.com>
Download mbox | patch
Permalink /patch/309790/
State Superseded
Headers show

Comments

Maxime Ripard - Jan. 13, 2014, 10:34 a.m.
The Allwinner A31 SoC using that IP has a reset controller maintaining
it reset unless told otherwise.

Add some optional reset support to the driver.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 .../devicetree/bindings/i2c/i2c-mv64xxx.txt         |  1 +
 drivers/i2c/busses/Kconfig                          |  1 +
 drivers/i2c/busses/i2c-mv64xxx.c                    | 21 +++++++++++++++++++--
 3 files changed, 21 insertions(+), 2 deletions(-)
Gregory CLEMENT - Feb. 26, 2014, 9:44 a.m.
Hi Maxime,

On 13/01/2014 11:34, Maxime Ripard wrote:
> The Allwinner A31 SoC using that IP has a reset controller maintaining
> it reset unless told otherwise.
> 
> Add some optional reset support to the driver.
> 
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> ---
>  .../devicetree/bindings/i2c/i2c-mv64xxx.txt         |  1 +
>  drivers/i2c/busses/Kconfig                          |  1 +
>  drivers/i2c/busses/i2c-mv64xxx.c                    | 21 +++++++++++++++++++--
>  3 files changed, 21 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/i2c/i2c-mv64xxx.txt b/Documentation/devicetree/bindings/i2c/i2c-mv64xxx.txt
> index 82e8f6f..603003a 100644
> --- a/Documentation/devicetree/bindings/i2c/i2c-mv64xxx.txt
> +++ b/Documentation/devicetree/bindings/i2c/i2c-mv64xxx.txt
> @@ -12,6 +12,7 @@ Optional properties :
>  
>   - clock-frequency : Desired I2C bus clock frequency in Hz. If not set the
>  default frequency is 100kHz
> + - resets          : phandle to the parent reset controller
>  
>  Examples:
>  
> diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
> index 3b26129..69aa599 100644
> --- a/drivers/i2c/busses/Kconfig
> +++ b/drivers/i2c/busses/Kconfig
> @@ -528,6 +528,7 @@ config I2C_MPC
>  config I2C_MV64XXX
>  	tristate "Marvell mv64xxx I2C Controller"
>  	depends on (MV64X60 || PLAT_ORION || ARCH_SUNXI)
> +	select RESET_CONTROLLER

This one could maybe just depend on ARCH_SUNXI with something like:

select RESET_CONTROLLER if ARCH_SUNXI

>  	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.
> diff --git a/drivers/i2c/busses/i2c-mv64xxx.c b/drivers/i2c/busses/i2c-mv64xxx.c
> index 8be7e42..0f6dde5 100644
> --- a/drivers/i2c/busses/i2c-mv64xxx.c
> +++ b/drivers/i2c/busses/i2c-mv64xxx.c
> @@ -17,6 +17,7 @@
>  #include <linux/interrupt.h>
>  #include <linux/mv643xx_i2c.h>
>  #include <linux/platform_device.h>
> +#include <linux/reset.h>
>  #include <linux/io.h>
>  #include <linux/of.h>
>  #include <linux/of_device.h>
> @@ -149,6 +150,7 @@ struct mv64xxx_i2c_data {
>  	bool			offload_enabled;
>  /* 5us delay in order to avoid repeated start timing violation */
>  	bool			errata_delay;
> +	struct reset_control	*rstc;
>  };
>  
>  static struct mv64xxx_i2c_regs mv64xxx_i2c_regs_mv64xxx = {
> @@ -763,6 +765,16 @@ mv64xxx_of_config(struct mv64xxx_i2c_data *drv_data,
>  	}
>  	drv_data->irq = irq_of_parse_and_map(np, 0);
>  
> +	drv_data->rstc = devm_reset_control_get(dev, NULL);

Hum ok, you need RESET_CONTROLLER in all case to use it here.
As most of the architecture also use RESET_CONTROLLER it is not
a big deal to unable it then.

> +	if (IS_ERR(drv_data->rstc)) {
> +		if (PTR_ERR(drv_data->rstc) == -EPROBE_DEFER) {
> +			rc = -EPROBE_DEFER;
> +			goto out;
> +		}
> +	} else {
> +		reset_control_deassert(drv_data->rstc);
> +	}
> +
>  	/* Its not yet defined how timeouts will be specified in device tree.
>  	 * So hard code the value to 1 second.
>  	 */
> @@ -845,7 +857,7 @@ mv64xxx_i2c_probe(struct platform_device *pd)
>  	}
>  	if (drv_data->irq < 0) {
>  		rc = -ENXIO;
> -		goto exit_clk;
> +		goto exit_reset;
>  	}
>  
>  	drv_data->adapter.dev.parent = &pd->dev;
> @@ -865,7 +877,7 @@ mv64xxx_i2c_probe(struct platform_device *pd)
>  		dev_err(&drv_data->adapter.dev,
>  			"mv64xxx: Can't register intr handler irq%d: %d\n",
>  			drv_data->irq, rc);
> -		goto exit_clk;
> +		goto exit_reset;
>  	} else if ((rc = i2c_add_numbered_adapter(&drv_data->adapter)) != 0) {
>  		dev_err(&drv_data->adapter.dev,
>  			"mv64xxx: Can't add i2c adapter, rc: %d\n", -rc);
> @@ -876,6 +888,9 @@ mv64xxx_i2c_probe(struct platform_device *pd)
>  
>  exit_free_irq:
>  	free_irq(drv_data->irq, drv_data);
> +exit_reset:
> +	if (pd->dev.of_node && !IS_ERR(drv_data->rstc))
> +		reset_control_assert(drv_data->rstc);
>  exit_clk:
>  #if defined(CONFIG_HAVE_CLK)
>  	/* Not all platforms have a clk */
> @@ -894,6 +909,8 @@ mv64xxx_i2c_remove(struct platform_device *dev)
>  
>  	i2c_del_adapter(&drv_data->adapter);
>  	free_irq(drv_data->irq, drv_data);
> +	if (dev->dev.of_node && !IS_ERR(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)) {
> 

Everything else looks sensible, I also tested in on an Armada 370.
You can add my

Reviewed-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
Tested-by: Gregory CLEMENT <gregory.clement@free-electrons.com>

Gregory

Patch

diff --git a/Documentation/devicetree/bindings/i2c/i2c-mv64xxx.txt b/Documentation/devicetree/bindings/i2c/i2c-mv64xxx.txt
index 82e8f6f..603003a 100644
--- a/Documentation/devicetree/bindings/i2c/i2c-mv64xxx.txt
+++ b/Documentation/devicetree/bindings/i2c/i2c-mv64xxx.txt
@@ -12,6 +12,7 @@  Optional properties :
 
  - clock-frequency : Desired I2C bus clock frequency in Hz. If not set the
 default frequency is 100kHz
+ - resets          : phandle to the parent reset controller
 
 Examples:
 
diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index 3b26129..69aa599 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -528,6 +528,7 @@  config I2C_MPC
 config I2C_MV64XXX
 	tristate "Marvell mv64xxx I2C Controller"
 	depends on (MV64X60 || PLAT_ORION || ARCH_SUNXI)
+	select RESET_CONTROLLER
 	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.
diff --git a/drivers/i2c/busses/i2c-mv64xxx.c b/drivers/i2c/busses/i2c-mv64xxx.c
index 8be7e42..0f6dde5 100644
--- a/drivers/i2c/busses/i2c-mv64xxx.c
+++ b/drivers/i2c/busses/i2c-mv64xxx.c
@@ -17,6 +17,7 @@ 
 #include <linux/interrupt.h>
 #include <linux/mv643xx_i2c.h>
 #include <linux/platform_device.h>
+#include <linux/reset.h>
 #include <linux/io.h>
 #include <linux/of.h>
 #include <linux/of_device.h>
@@ -149,6 +150,7 @@  struct mv64xxx_i2c_data {
 	bool			offload_enabled;
 /* 5us delay in order to avoid repeated start timing violation */
 	bool			errata_delay;
+	struct reset_control	*rstc;
 };
 
 static struct mv64xxx_i2c_regs mv64xxx_i2c_regs_mv64xxx = {
@@ -763,6 +765,16 @@  mv64xxx_of_config(struct mv64xxx_i2c_data *drv_data,
 	}
 	drv_data->irq = irq_of_parse_and_map(np, 0);
 
+	drv_data->rstc = devm_reset_control_get(dev, NULL);
+	if (IS_ERR(drv_data->rstc)) {
+		if (PTR_ERR(drv_data->rstc) == -EPROBE_DEFER) {
+			rc = -EPROBE_DEFER;
+			goto out;
+		}
+	} else {
+		reset_control_deassert(drv_data->rstc);
+	}
+
 	/* Its not yet defined how timeouts will be specified in device tree.
 	 * So hard code the value to 1 second.
 	 */
@@ -845,7 +857,7 @@  mv64xxx_i2c_probe(struct platform_device *pd)
 	}
 	if (drv_data->irq < 0) {
 		rc = -ENXIO;
-		goto exit_clk;
+		goto exit_reset;
 	}
 
 	drv_data->adapter.dev.parent = &pd->dev;
@@ -865,7 +877,7 @@  mv64xxx_i2c_probe(struct platform_device *pd)
 		dev_err(&drv_data->adapter.dev,
 			"mv64xxx: Can't register intr handler irq%d: %d\n",
 			drv_data->irq, rc);
-		goto exit_clk;
+		goto exit_reset;
 	} else if ((rc = i2c_add_numbered_adapter(&drv_data->adapter)) != 0) {
 		dev_err(&drv_data->adapter.dev,
 			"mv64xxx: Can't add i2c adapter, rc: %d\n", -rc);
@@ -876,6 +888,9 @@  mv64xxx_i2c_probe(struct platform_device *pd)
 
 exit_free_irq:
 	free_irq(drv_data->irq, drv_data);
+exit_reset:
+	if (pd->dev.of_node && !IS_ERR(drv_data->rstc))
+		reset_control_assert(drv_data->rstc);
 exit_clk:
 #if defined(CONFIG_HAVE_CLK)
 	/* Not all platforms have a clk */
@@ -894,6 +909,8 @@  mv64xxx_i2c_remove(struct platform_device *dev)
 
 	i2c_del_adapter(&drv_data->adapter);
 	free_irq(drv_data->irq, drv_data);
+	if (dev->dev.of_node && !IS_ERR(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)) {