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@xxxxxxxxxxxxxxxxxx>
> ---
> .../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@xxxxxxxxxxxxxxxxxx>
Tested-by: Gregory CLEMENT <gregory.clement@xxxxxxxxxxxxxxxxxx>
Gregory
--
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
