Message ID | 1389609293-2824-2-git-send-email-maxime.ripard@free-electrons.com |
---|---|
State | Superseded |
Headers | show |
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
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)) {
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(-)