[PATCH RESEND] i2c: move of helpers into the core

2013-08-19 Thread Wolfram Sang
I2C of helpers used to live in of_i2c.c but experience (from SPI) shows
that it is much cleaner to have this in the core. This also removes a
circular dependency between the helpers and the core, and so we can
finally register child nodes in the core instead of doing this manually
in each driver. So, fix the drivers and documentation, too.

Signed-off-by: Wolfram Sang w...@the-dreams.de
---

Sigh, hitting the CC threshold on vger again. So resending to the lists only.
BTW this patch is based on -rc4 and was tested on an AT91 board. More tests
very welcome. Thanks!


 Documentation/acpi/enumeration.txt  |1 -
 drivers/i2c/busses/i2c-at91.c   |3 -
 drivers/i2c/busses/i2c-cpm.c|6 --
 drivers/i2c/busses/i2c-davinci.c|2 -
 drivers/i2c/busses/i2c-designware-platdrv.c |2 -
 drivers/i2c/busses/i2c-gpio.c   |3 -
 drivers/i2c/busses/i2c-i801.c   |2 -
 drivers/i2c/busses/i2c-ibm_iic.c|4 -
 drivers/i2c/busses/i2c-imx.c|3 -
 drivers/i2c/busses/i2c-mpc.c|2 -
 drivers/i2c/busses/i2c-mv64xxx.c|3 -
 drivers/i2c/busses/i2c-mxs.c|3 -
 drivers/i2c/busses/i2c-nomadik.c|3 -
 drivers/i2c/busses/i2c-ocores.c |3 -
 drivers/i2c/busses/i2c-octeon.c |3 -
 drivers/i2c/busses/i2c-omap.c   |3 -
 drivers/i2c/busses/i2c-pnx.c|3 -
 drivers/i2c/busses/i2c-powermac.c   |9 +-
 drivers/i2c/busses/i2c-pxa.c|2 -
 drivers/i2c/busses/i2c-s3c2410.c|2 -
 drivers/i2c/busses/i2c-sh_mobile.c  |2 -
 drivers/i2c/busses/i2c-sirf.c   |3 -
 drivers/i2c/busses/i2c-stu300.c |2 -
 drivers/i2c/busses/i2c-tegra.c  |3 -
 drivers/i2c/busses/i2c-versatile.c  |2 -
 drivers/i2c/busses/i2c-wmt.c|3 -
 drivers/i2c/busses/i2c-xiic.c   |3 -
 drivers/i2c/i2c-core.c  |  107 -
 drivers/i2c/i2c-mux.c   |3 -
 drivers/i2c/muxes/i2c-arb-gpio-challenge.c  |1 -
 drivers/i2c/muxes/i2c-mux-gpio.c|1 -
 drivers/i2c/muxes/i2c-mux-pinctrl.c |1 -
 drivers/media/platform/exynos4-is/fimc-is-i2c.c |3 -
 drivers/of/Kconfig  |6 --
 drivers/of/Makefile |1 -
 drivers/of/of_i2c.c |  114 ---
 include/linux/i2c.h |   20 
 include/linux/of_i2c.h  |   46 -
 38 files changed, 130 insertions(+), 253 deletions(-)
 delete mode 100644 drivers/of/of_i2c.c
 delete mode 100644 include/linux/of_i2c.h

diff --git a/Documentation/acpi/enumeration.txt 
b/Documentation/acpi/enumeration.txt
index d9be7a9..958266e 100644
--- a/Documentation/acpi/enumeration.txt
+++ b/Documentation/acpi/enumeration.txt
@@ -238,7 +238,6 @@ An I2C bus (controller) driver does:
if (ret)
/* handle error */
 
-   of_i2c_register_devices(adapter);
/* Enumerate the slave devices behind this bus via ACPI */
acpi_i2c_register_devices(adapter);
 
diff --git a/drivers/i2c/busses/i2c-at91.c b/drivers/i2c/busses/i2c-at91.c
index 6bb839b..fd05930 100644
--- a/drivers/i2c/busses/i2c-at91.c
+++ b/drivers/i2c/busses/i2c-at91.c
@@ -28,7 +28,6 @@
 #include linux/module.h
 #include linux/of.h
 #include linux/of_device.h
-#include linux/of_i2c.h
 #include linux/platform_device.h
 #include linux/slab.h
 #include linux/platform_data/dma-atmel.h
@@ -775,8 +774,6 @@ static int at91_twi_probe(struct platform_device *pdev)
return rc;
}
 
-   of_i2c_register_devices(dev-adapter);
-
dev_info(dev-dev, AT91 i2c bus driver.\n);
return 0;
 }
diff --git a/drivers/i2c/busses/i2c-cpm.c b/drivers/i2c/busses/i2c-cpm.c
index 2e1f7eb..b2b8aa9 100644
--- a/drivers/i2c/busses/i2c-cpm.c
+++ b/drivers/i2c/busses/i2c-cpm.c
@@ -42,7 +42,6 @@
 #include linux/dma-mapping.h
 #include linux/of_device.h
 #include linux/of_platform.h
-#include linux/of_i2c.h
 #include sysdev/fsl_soc.h
 #include asm/cpm.h
 
@@ -681,11 +680,6 @@ static int cpm_i2c_probe(struct platform_device *ofdev)
dev_dbg(ofdev-dev, hw routines for %s registered.\n,
cpm-adap.name);
 
-   /*
-* register OF I2C devices
-*/
-   of_i2c_register_devices(cpm-adap);
-
return 0;
 out_shut:
cpm_i2c_shutdown(cpm);
diff --git a/drivers/i2c/busses/i2c-davinci.c b/drivers/i2c/busses/i2c-davinci.c
index fa55605..62be3b3 100644
--- a/drivers/i2c/busses/i2c-davinci.c
+++ b/drivers/i2c/busses/i2c-davinci.c
@@ -38,7 +38,6 @@
 #include linux/slab.h
 #include 

Re: [PATCH RESEND] i2c: move of helpers into the core

2013-08-19 Thread Sylwester Nawrocki
On 08/19/2013 07:59 PM, Wolfram Sang wrote:
 I2C of helpers used to live in of_i2c.c but experience (from SPI) shows
 that it is much cleaner to have this in the core. This also removes a
 circular dependency between the helpers and the core, and so we can
 finally register child nodes in the core instead of doing this manually
 in each driver. So, fix the drivers and documentation, too.
 
 Signed-off-by: Wolfram Sang w...@the-dreams.de
 ---
 
 Sigh, hitting the CC threshold on vger again. So resending to the lists only.
 BTW this patch is based on -rc4 and was tested on an AT91 board. More tests
 very welcome. Thanks!
 
 
  drivers/i2c/busses/i2c-s3c2410.c|2 -
  drivers/media/platform/exynos4-is/fimc-is-i2c.c |3 -

For these:

Acked-by: Sylwester Nawrocki s.nawro...@amsung.com

However this patch fails to apply onto either v3.11-rc4 or v3.11-rc6:

Applying: i2c: move of helpers into the core
fatal: sha1 information is lacking or useless 
(drivers/i2c/busses/i2c-powermac.c).
Repository lacks necessary blobs to fall back on 3-way merge.
Cannot fall back to three-way merge.
Patch failed at 0001 i2c: move of helpers into the core


One nitpick below..

[...]
 diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
 index f32ca29..321b7ca 100644
 --- a/drivers/i2c/i2c-core.c
 +++ b/drivers/i2c/i2c-core.c
 @@ -23,7 +23,11 @@
 SMBus 2.0 support by Mark Studebaker mdsxyz...@yahoo.com and
 Jean Delvare kh...@linux-fr.org
 Mux support by Rodolfo Giometti giome...@enneenne.com and
 -   Michael Lawnick michael.lawnick@nsn.com */
 +   Michael Lawnick michael.lawnick@nsn.com
 +   OF support is copyright (c) 2008 Jochen Friedrich joc...@scram.de
 +   (based on a previous patch from Jon Smirl jonsm...@gmail.com) and
 +   (c) 2013  Wolfram Sang w...@the-dreams.de
 + */
  
  #include linux/module.h
  #include linux/kernel.h
 @@ -35,7 +39,9 @@
  #include linux/init.h
  #include linux/idr.h
  #include linux/mutex.h
 +#include linux/of.h
  #include linux/of_device.h
 +#include linux/of_irq.h
  #include linux/completion.h
  #include linux/hardirq.h
  #include linux/irqflags.h
 @@ -954,6 +960,102 @@ static void i2c_scan_static_board_info(struct 
 i2c_adapter *adapter)
   up_read(__i2c_board_lock);
  }
  
 +/* of support code */

/* OF support code */

or

/*
 * Device Tree support code.
 */

?
 +#if IS_ENABLED(CONFIG_OF)
 +static void of_i2c_register_devices(struct i2c_adapter *adap)
 +{
 + void *result;
 + struct device_node *node;
 +
 + /* Only register child devices if the adapter has a node pointer set */
 + if (!adap-dev.of_node)
 + return;
 +
 + dev_dbg(adap-dev, of_i2c: walking child nodes\n);
 +
 + for_each_available_child_of_node(adap-dev.of_node, node) {
 + struct i2c_board_info info = {};
 + struct dev_archdata dev_ad = {};
 + const __be32 *addr;
 + int len;
 +
 + dev_dbg(adap-dev, of_i2c: register %s\n, node-full_name);
 +
 + if (of_modalias_node(node, info.type, sizeof(info.type))  0) {
 + dev_err(adap-dev, of_i2c: modalias failure on %s\n,
 + node-full_name);
 + continue;
 + }
 +
 + addr = of_get_property(node, reg, len);
 + if (!addr || (len  sizeof(int))) {
 + dev_err(adap-dev, of_i2c: invalid reg on %s\n,
 + node-full_name);
 + continue;
 + }
 +
 + info.addr = be32_to_cpup(addr);
 + if (info.addr  (1  10) - 1) {
 + dev_err(adap-dev, of_i2c: invalid addr=%x on %s\n,
 + info.addr, node-full_name);
 + continue;
 + }
 +
 + info.irq = irq_of_parse_and_map(node, 0);
 + info.of_node = of_node_get(node);
 + info.archdata = dev_ad;
 +
 + if (of_get_property(node, wakeup-source, NULL))
 + info.flags |= I2C_CLIENT_WAKE;
 +
 + request_module(%s%s, I2C_MODULE_PREFIX, info.type);
 +
 + result = i2c_new_device(adap, info);
 + if (result == NULL) {
 + dev_err(adap-dev, of_i2c: Failure registering %s\n,
 + node-full_name);
 + of_node_put(node);
 + irq_dispose_mapping(info.irq);
 + continue;
 + }
 + }
 +}
 +
 +static int of_dev_node_match(struct device *dev, void *data)
 +{
 + return dev-of_node == data;
 +}
 +
 +/* must call put_device() when done with returned i2c_client device */
 +struct i2c_client *of_find_i2c_device_by_node(struct device_node *node)
 +{
 + struct device *dev;
 +
 + dev = bus_find_device(i2c_bus_type, NULL, node,
 +  of_dev_node_match);
 + if (!dev)
 + return NULL;
 +
 + 

Re: [PATCH RESEND] i2c: move of helpers into the core

2013-08-19 Thread Wolfram Sang

 However this patch fails to apply onto either v3.11-rc4 or v3.11-rc6:

Argh, did not drop the MPC patch before rebasing :( So either pick the
patch i2c: powermac: fix return path on error before, pull the branch
[1], or force me to resend ;)

Thanks!

[1] git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git i2c/core-with-of


signature.asc
Description: Digital signature


Re: [PATCH RESEND] i2c: move of helpers into the core

2013-08-19 Thread Thierry Reding
On Mon, Aug 19, 2013 at 07:59:40PM +0200, Wolfram Sang wrote:
[...]
 diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
[...]
 +#if IS_ENABLED(CONFIG_OF)
 +static void of_i2c_register_devices(struct i2c_adapter *adap)
 +{
[...]
 +}
[...]
 +#endif /* CONFIG_OF */

Isn't this missing the dummy implementation for !OF.

  static int i2c_do_add_adapter(struct i2c_driver *driver,
 struct i2c_adapter *adap)
  {
 @@ -1058,6 +1160,8 @@ static int i2c_register_adapter(struct i2c_adapter 
 *adap)
  
  exit_recovery:
   /* create pre-declared device nodes */
 + of_i2c_register_devices(adap);

Alternatively you could remove the of_i2c_register_devices() from the
#ifdef CONFIG_OF block so that it will always be compiled. You could
turn the above into

if (IS_ENABLED(CONFIG_OF))
of_i2c_register_devices(adap);

and let the compiler throw the static function away if it sees that the
condition is always false.

Thierry


pgpw1yWn8EA6p.pgp
Description: PGP signature


Re: [PATCH RESEND] i2c: move of helpers into the core

2013-08-19 Thread Felipe Balbi
On Mon, Aug 19, 2013 at 07:59:40PM +0200, Wolfram Sang wrote:
 I2C of helpers used to live in of_i2c.c but experience (from SPI) shows
 that it is much cleaner to have this in the core. This also removes a
 circular dependency between the helpers and the core, and so we can
 finally register child nodes in the core instead of doing this manually
 in each driver. So, fix the drivers and documentation, too.
 
 Signed-off-by: Wolfram Sang w...@the-dreams.de

for i2c-omap.c:

Reviewed-by: Felipe Balbi ba...@ti.com

-- 
balbi


signature.asc
Description: Digital signature


Re: [PATCH RESEND] i2c: move of helpers into the core

2013-08-19 Thread Wolfram Sang
On Mon, Aug 19, 2013 at 09:46:04PM +0200, Thierry Reding wrote:
 On Mon, Aug 19, 2013 at 07:59:40PM +0200, Wolfram Sang wrote:
 [...]
  diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
 [...]
  +#if IS_ENABLED(CONFIG_OF)
  +static void of_i2c_register_devices(struct i2c_adapter *adap)
  +{
 [...]
  +}
 [...]
  +#endif /* CONFIG_OF */
 
 Isn't this missing the dummy implementation for !OF.

Argh, will fix...



signature.asc
Description: Digital signature


Re: [PATCH RESEND] i2c: move of helpers into the core

2013-08-19 Thread Rob Herring
On 08/19/2013 12:59 PM, Wolfram Sang wrote:
 I2C of helpers used to live in of_i2c.c but experience (from SPI) shows
 that it is much cleaner to have this in the core. This also removes a
 circular dependency between the helpers and the core, and so we can
 finally register child nodes in the core instead of doing this manually
 in each driver. So, fix the drivers and documentation, too.
 
 Signed-off-by: Wolfram Sang w...@the-dreams.de
 ---

Glad to see this.

Acked-by: Rob Herring rob.herr...@calxeda.com

 
 Sigh, hitting the CC threshold on vger again. So resending to the lists only.
 BTW this patch is based on -rc4 and was tested on an AT91 board. More tests
 very welcome. Thanks!
 
 
  Documentation/acpi/enumeration.txt  |1 -
  drivers/i2c/busses/i2c-at91.c   |3 -
  drivers/i2c/busses/i2c-cpm.c|6 --
  drivers/i2c/busses/i2c-davinci.c|2 -
  drivers/i2c/busses/i2c-designware-platdrv.c |2 -
  drivers/i2c/busses/i2c-gpio.c   |3 -
  drivers/i2c/busses/i2c-i801.c   |2 -
  drivers/i2c/busses/i2c-ibm_iic.c|4 -
  drivers/i2c/busses/i2c-imx.c|3 -
  drivers/i2c/busses/i2c-mpc.c|2 -
  drivers/i2c/busses/i2c-mv64xxx.c|3 -
  drivers/i2c/busses/i2c-mxs.c|3 -
  drivers/i2c/busses/i2c-nomadik.c|3 -
  drivers/i2c/busses/i2c-ocores.c |3 -
  drivers/i2c/busses/i2c-octeon.c |3 -
  drivers/i2c/busses/i2c-omap.c   |3 -
  drivers/i2c/busses/i2c-pnx.c|3 -
  drivers/i2c/busses/i2c-powermac.c   |9 +-
  drivers/i2c/busses/i2c-pxa.c|2 -
  drivers/i2c/busses/i2c-s3c2410.c|2 -
  drivers/i2c/busses/i2c-sh_mobile.c  |2 -
  drivers/i2c/busses/i2c-sirf.c   |3 -
  drivers/i2c/busses/i2c-stu300.c |2 -
  drivers/i2c/busses/i2c-tegra.c  |3 -
  drivers/i2c/busses/i2c-versatile.c  |2 -
  drivers/i2c/busses/i2c-wmt.c|3 -
  drivers/i2c/busses/i2c-xiic.c   |3 -
  drivers/i2c/i2c-core.c  |  107 -
  drivers/i2c/i2c-mux.c   |3 -
  drivers/i2c/muxes/i2c-arb-gpio-challenge.c  |1 -
  drivers/i2c/muxes/i2c-mux-gpio.c|1 -
  drivers/i2c/muxes/i2c-mux-pinctrl.c |1 -
  drivers/media/platform/exynos4-is/fimc-is-i2c.c |3 -
  drivers/of/Kconfig  |6 --
  drivers/of/Makefile |1 -
  drivers/of/of_i2c.c |  114 
 ---
  include/linux/i2c.h |   20 
  include/linux/of_i2c.h  |   46 -
  38 files changed, 130 insertions(+), 253 deletions(-)
  delete mode 100644 drivers/of/of_i2c.c
  delete mode 100644 include/linux/of_i2c.h
 
 diff --git a/Documentation/acpi/enumeration.txt 
 b/Documentation/acpi/enumeration.txt
 index d9be7a9..958266e 100644
 --- a/Documentation/acpi/enumeration.txt
 +++ b/Documentation/acpi/enumeration.txt
 @@ -238,7 +238,6 @@ An I2C bus (controller) driver does:
   if (ret)
   /* handle error */
  
 - of_i2c_register_devices(adapter);
   /* Enumerate the slave devices behind this bus via ACPI */
   acpi_i2c_register_devices(adapter);
  
 diff --git a/drivers/i2c/busses/i2c-at91.c b/drivers/i2c/busses/i2c-at91.c
 index 6bb839b..fd05930 100644
 --- a/drivers/i2c/busses/i2c-at91.c
 +++ b/drivers/i2c/busses/i2c-at91.c
 @@ -28,7 +28,6 @@
  #include linux/module.h
  #include linux/of.h
  #include linux/of_device.h
 -#include linux/of_i2c.h
  #include linux/platform_device.h
  #include linux/slab.h
  #include linux/platform_data/dma-atmel.h
 @@ -775,8 +774,6 @@ static int at91_twi_probe(struct platform_device *pdev)
   return rc;
   }
  
 - of_i2c_register_devices(dev-adapter);
 -
   dev_info(dev-dev, AT91 i2c bus driver.\n);
   return 0;
  }
 diff --git a/drivers/i2c/busses/i2c-cpm.c b/drivers/i2c/busses/i2c-cpm.c
 index 2e1f7eb..b2b8aa9 100644
 --- a/drivers/i2c/busses/i2c-cpm.c
 +++ b/drivers/i2c/busses/i2c-cpm.c
 @@ -42,7 +42,6 @@
  #include linux/dma-mapping.h
  #include linux/of_device.h
  #include linux/of_platform.h
 -#include linux/of_i2c.h
  #include sysdev/fsl_soc.h
  #include asm/cpm.h
  
 @@ -681,11 +680,6 @@ static int cpm_i2c_probe(struct platform_device *ofdev)
   dev_dbg(ofdev-dev, hw routines for %s registered.\n,
   cpm-adap.name);
  
 - /*
 -  * register OF I2C devices
 -  */
 - of_i2c_register_devices(cpm-adap);
 -
   return 0;
  out_shut:
   cpm_i2c_shutdown(cpm);
 diff --git a/drivers/i2c/busses/i2c-davinci.c