Re: [i2c] [PATCH] i2c driver for Maxim max9485 audio clock generator chip

2008-10-22 Thread Jon Smirl
What you want me to do? Put in a bare .h file for the part? Bare .h
files are much harder to find and reuse. It is also going to force
anyone reusing this part into reading the datasheet and determining
how the part works.


On Wed, Oct 22, 2008 at 2:38 AM, Jean Delvare [EMAIL PROTECTED] wrote:
 On Tue, 21 Oct 2008 19:57:09 -0400, Jon Smirl wrote:
 On Tue, Oct 21, 2008 at 4:36 AM, Jean Delvare [EMAIL PROTECTED] wrote:
  Honestly I don't see any value in this driver. There's nothing you can
  do with it that you couldn't already do without it.

 I need a driver so that my device tree will bind and tell me the i2c
 address of the device.

 This is rubbish. It's perfectly fine to let the I2C device be
 instantiated from the device tree without having a driver that binds to
 it. The device tree and the instantiated device, not the driver, tell
 you the I2C address of the device.

 We may also build a device in the future with two of these chips.

 I fail to see how it matters.

 --
 Jean Delvare




-- 
Jon Smirl
[EMAIL PROTECTED]

___
i2c mailing list
i2c@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/i2c


Re: [i2c] [PATCH] i2c driver for Maxim max9485 audio clock generator chip

2008-10-21 Thread Jon Smirl
On Tue, Oct 21, 2008 at 4:36 AM, Jean Delvare [EMAIL PROTECTED] wrote:
 Honestly I don't see any value in this driver. There's nothing you can
 do with it that you couldn't already do without it.

I need a driver so that my device tree will bind and tell me the i2c
address of the device.

We may also build a device in the future with two of these chips.

-- 
Jon Smirl
[EMAIL PROTECTED]

___
i2c mailing list
i2c@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/i2c


[i2c] [V2 PATCH] i2c driver for Maxim max9485 audio clock generator chip

2008-10-21 Thread Jon Smirl
Signed-off-by: Jon Smirl [EMAIL PROTECTED]
---
 drivers/misc/Kconfig|9 
 drivers/misc/Makefile   |1 
 drivers/misc/max9485.c  |  105 +++
 include/linux/i2c/max9485.h |   39 
 4 files changed, 154 insertions(+), 0 deletions(-)
 create mode 100644 drivers/misc/max9485.c
 create mode 100644 include/linux/i2c/max9485.h

diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
index efd3aa0..010baa6 100644
--- a/drivers/misc/Kconfig
+++ b/drivers/misc/Kconfig
@@ -391,6 +391,15 @@ config ATMEL_SSC
 
  If unsure, say N.
 
+config MAX9485
+   tristate Maxim MAX9485 Programmable Audio Clock Generator
+   help
+ If you say yes here you get support for Maxim MAX9485 
+ Programmable Audio Clock Generator.
+
+ This driver can also be built as a module.  If so, the module
+ will be called max9485.
+
 config INTEL_MENLOW
tristate Thermal Management driver for Intel menlow platform
depends on ACPI_THERMAL
diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
index c6c13f6..6133434 100644
--- a/drivers/misc/Makefile
+++ b/drivers/misc/Makefile
@@ -24,6 +24,7 @@ obj-$(CONFIG_SONY_LAPTOP) += sony-laptop.o
 obj-$(CONFIG_THINKPAD_ACPI)+= thinkpad_acpi.o
 obj-$(CONFIG_FUJITSU_LAPTOP)   += fujitsu-laptop.o
 obj-$(CONFIG_EEPROM_93CX6) += eeprom_93cx6.o
+obj-$(CONFIG_MAX9485)  += max9485.o
 obj-$(CONFIG_INTEL_MENLOW) += intel_menlow.o
 obj-$(CONFIG_ENCLOSURE_SERVICES) += enclosure.o
 obj-$(CONFIG_KGDB_TESTS)   += kgdbts.o
diff --git a/drivers/misc/max9485.c b/drivers/misc/max9485.c
new file mode 100644
index 000..c1316f2
--- /dev/null
+++ b/drivers/misc/max9485.c
@@ -0,0 +1,105 @@
+/*
+ * Maxim max9485 Programmable Audio Clock Generator driver
+ *
+ * Written by: Jon Smirl [EMAIL PROTECTED]
+ *
+ * Copyright (C) 2008 Digispeaker.com
+ * Copyright (C) 2008 Jon Smirl [EMAIL PROTECTED]
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+/*
+ * This device is usually under the control of ALSA and should not be changed
+ * from userspace. The main purpose of this driver is to locate the i2c address
+ * of where the chip is located.
+ */
+
+#include linux/module.h
+#include linux/i2c.h
+#include linux/sysfs.h
+#include linux/i2c/max9485.h
+
+int max9485_set(struct i2c_client *max9485, u8 value)
+{
+   return i2c_smbus_write_byte(max9485, value);
+}
+EXPORT_SYMBOL_GPL(max9485_set);
+
+/*
+ * Display the only register
+ */
+static ssize_t max9485_show(struct device *dev, struct device_attribute *attr,
+  char *buf)
+{
+   struct i2c_client *client = to_i2c_client(dev);
+   int rc;
+
+   rc = i2c_smbus_read_byte(client);
+   if (rc  0)
+   return rc;
+
+   return sprintf(buf, %s%s%s%s%s%s,
+   (rc  MAX9485_MCLK ? MCLK+ : ),
+   (rc  MAX9485_CLK_OUT_2 ? CLK2+ : ),
+   (rc  MAX9485_CLK_OUT_2 ? CLK1+ : ),
+   (rc  MAX9485_DOUBLED ? DOUBLED+ : ),
+   (rc  MAX9485_SCALE_768 ? 768x+ : (rc  
MAX9485_SCALE_384 ? 384x+ : 256x+)),
+   ((rc  3) == MAX9485_FREQUENCY_48 ? 48Khz :
+   ((rc  3) == MAX9485_FREQUENCY_441 ? 44.1Khz :
+   ((rc  3) == MAX9485_FREQUENCY_32 ? 32Khz : 
12Khz;
+}
+static DEVICE_ATTR(max9485, S_IRUGO, max9485_show, NULL);
+
+/*
+ * Called when a max9485 device is matched with this driver
+ */
+static int max9485_probe(struct i2c_client *client,
+   const struct i2c_device_id *id)
+{
+   if (!i2c_check_functionality(client-adapter, I2C_FUNC_SMBUS_BYTE)) {
+   dev_err(client-dev, i2c bus does not support the max9485\n);
+   return -ENODEV;
+   }
+   return sysfs_create_file(client-dev.kobj, dev_attr_max9485.attr);
+}
+
+static int max9485_remove(struct i2c_client *client)
+{
+   sysfs_remove_file(client-dev.kobj, dev_attr_max9485.attr);
+   return 0;
+}
+
+static const struct i2c_device_id max9485_id[] = {
+   { max9485, 0 },
+   { }
+};
+MODULE_DEVICE_TABLE(i2c, max9485_id);
+
+static struct i2c_driver max9485_driver = {
+   .driver = {
+   .name = max9485,
+   },
+   .probe = max9485_probe,
+   .remove = max9485_remove,
+   .id_table = max9485_id,
+};
+
+static int __init max9485_init(void)
+{
+   return i2c_add_driver(max9485_driver);
+}
+
+static void __exit max9485_exit(void)
+{
+   i2c_del_driver(max9485_driver);
+}
+
+MODULE_AUTHOR(Jon Smirl [EMAIL PROTECTED]);
+MODULE_DESCRIPTION(Maxim MAX9485 Programmable Audio Clock Generator driver);
+MODULE_LICENSE(GPL);
+
+module_init(max9485_init);
+module_exit(max9485_exit);
diff --git a/include/linux/i2c/max9485.h b/include

[i2c] [PATCH] i2c driver for Maxim max9485 audio clock generator chip

2008-10-20 Thread Jon Smirl
Signed-off-by: Jon Smirl [EMAIL PROTECTED]
---
 drivers/i2c/chips/Kconfig   |9 
 drivers/i2c/chips/Makefile  |1 
 drivers/i2c/chips/max9485.c |  106 +++
 include/linux/i2c/max9485.h |   38 +++
 4 files changed, 154 insertions(+), 0 deletions(-)
 create mode 100644 drivers/i2c/chips/max9485.c
 create mode 100644 include/linux/i2c/max9485.h

diff --git a/drivers/i2c/chips/Kconfig b/drivers/i2c/chips/Kconfig
index 1735682..bc2a6d3 100644
--- a/drivers/i2c/chips/Kconfig
+++ b/drivers/i2c/chips/Kconfig
@@ -40,6 +40,15 @@ config AT24
  This driver can also be built as a module.  If so, the module
  will be called at24.
 
+config MAX9485
+   tristate Maxim MAX9485 Programmable Audio Clock Generator
+   help
+ If you say yes here you get support for Maxim MAX9485 
+ Programmable Audio Clock Generator.
+
+ This driver can also be built as a module.  If so, the module
+ will be called max9485.
+
 config SENSORS_EEPROM
tristate EEPROM reader
depends on EXPERIMENTAL
diff --git a/drivers/i2c/chips/Makefile b/drivers/i2c/chips/Makefile
index ca520fa..1560baf 100644
--- a/drivers/i2c/chips/Makefile
+++ b/drivers/i2c/chips/Makefile
@@ -11,6 +11,7 @@
 
 obj-$(CONFIG_DS1682)   += ds1682.o
 obj-$(CONFIG_AT24) += at24.o
+obj-$(CONFIG_MAX9485)  += max9485.o
 obj-$(CONFIG_SENSORS_EEPROM)   += eeprom.o
 obj-$(CONFIG_SENSORS_MAX6875)  += max6875.o
 obj-$(CONFIG_SENSORS_PCA9539)  += pca9539.o
diff --git a/drivers/i2c/chips/max9485.c b/drivers/i2c/chips/max9485.c
new file mode 100644
index 000..65058b4
--- /dev/null
+++ b/drivers/i2c/chips/max9485.c
@@ -0,0 +1,106 @@
+/*
+ * Maxim max9485 Programmable Audio Clock Generator driver
+ *
+ * Written by: Jon Smirl [EMAIL PROTECTED]
+ *
+ * Copyright (C) Digispeaker.com
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+/*
+ * This device is under the control of ALSA and can not be changed from
+ * userspace. The main purpose of this driver is to locate the i2c address
+ * of where the chip is located.
+ */
+
+#include linux/module.h
+#include linux/i2c.h
+#include linux/sysfs.h
+#include linux/i2c/max9485.h
+
+int max9485_set(struct i2c_client *max9485, u8 value)
+{
+   int rc;
+
+   if (!max9485)
+   return -ENODEV;
+
+   rc = i2c_smbus_write_byte(max9485, value);
+   if (rc  0)
+   return -EIO;
+
+   return 0;
+}
+EXPORT_SYMBOL_GPL(max9485_set);
+
+/*
+ * Display the only register
+ */
+static ssize_t max9485_show(struct device *dev, struct device_attribute *attr,
+  char *buf)
+{
+   struct i2c_client *client = to_i2c_client(dev);
+   int rc;
+
+   rc = i2c_smbus_read_byte(client);
+   if (rc  0)
+   return -EIO;
+
+   return sprintf(buf, 0x%02X\n, rc);
+}
+static DEVICE_ATTR(max9485, S_IRUGO, max9485_show, NULL);
+
+/*
+ * Called when a max9485 device is matched with this driver
+ */
+static int max9485_probe(struct i2c_client *client,
+   const struct i2c_device_id *id)
+{
+   if (!i2c_check_functionality(client-adapter,
+   I2C_FUNC_SMBUS_READ_BYTE | 
I2C_FUNC_SMBUS_WRITE_BYTE)) {
+   dev_err(client-dev, i2c bus does not support the max9485\n);
+   return -ENODEV;
+   }
+   return sysfs_create_file(client-dev.kobj, dev_attr_max9485.attr);
+}
+
+static int max9485_remove(struct i2c_client *client)
+{
+   sysfs_remove_file(client-dev.kobj, dev_attr_max9485.attr);
+   return 0;
+}
+
+static const struct i2c_device_id max9485_id[] = {
+   { max9485, 0 },
+   { }
+};
+MODULE_DEVICE_TABLE(i2c, max9485_id);
+
+static struct i2c_driver max9485_driver = {
+   .driver = {
+   .name = max9485,
+   },
+   .probe = max9485_probe,
+   .remove = max9485_remove,
+   .id_table = max9485_id,
+};
+
+static int __init max9485_init(void)
+{
+   return i2c_add_driver(max9485_driver);
+}
+
+static void __exit max9485_exit(void)
+{
+   i2c_del_driver(max9485_driver);
+}
+
+MODULE_AUTHOR(Jon Smirl [EMAIL PROTECTED]);
+MODULE_DESCRIPTION(Maxim MAX9485 Programmable Audio Clock Generator driver);
+MODULE_LICENSE(GPL);
+
+module_init(max9485_init);
+module_exit(max9485_exit);
diff --git a/include/linux/i2c/max9485.h b/include/linux/i2c/max9485.h
new file mode 100644
index 000..0c97450
--- /dev/null
+++ b/include/linux/i2c/max9485.h
@@ -0,0 +1,38 @@
+/*
+ * Maxim max9485 Programmable Audio Clock Generator driver
+ *
+ * Written by: Jon Smirl [EMAIL PROTECTED]
+ *
+ * Copyright (C) Digispeaker.com
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published

[i2c] [PATCH] Add the of_find_i2c_device_by_node function

2008-10-19 Thread Jon Smirl
Add the of_find_i2c_device_by_node function. This allows you to follow
a reference in the device tree to an i2c device node and then locate
the linux device instantiated by the device tree. Example use, an i2s
codec controlled by i2c. Depends on patch exporting i2c root bus symbol.

Signed-off-by: Jon Smirl [EMAIL PROTECTED]
---
 drivers/of/of_i2c.c |   28 
 1 files changed, 24 insertions(+), 4 deletions(-)

diff --git a/drivers/of/of_i2c.c b/drivers/of/of_i2c.c
index 6a98dc8..ba7b394 100644
--- a/drivers/of/of_i2c.c
+++ b/drivers/of/of_i2c.c
@@ -19,7 +19,7 @@
 void of_register_i2c_devices(struct i2c_adapter *adap,
 struct device_node *adap_node)
 {
-   void *result;
+   struct i2c_client *i2c_dev;
struct device_node *node;
 
for_each_child_of_node(adap_node, node) {
@@ -41,18 +41,38 @@ void of_register_i2c_devices(struct i2c_adapter *adap,
 
info.addr = *addr;
 
-   request_module(info.type);
+   request_module(%s, info.type);
 
-   result = i2c_new_device(adap, info);
-   if (result == NULL) {
+   i2c_dev = i2c_new_device(adap, info);
+   if (i2c_dev == NULL) {
printk(KERN_ERR
   of-i2c: Failed to load driver for %s\n,
   info.type);
irq_dispose_mapping(info.irq);
continue;
}
+   
+   i2c_dev-dev.archdata.of_node = of_node_get(node);
}
 }
 EXPORT_SYMBOL(of_register_i2c_devices);
 
+static int of_dev_node_match(struct device *dev, void *data)
+{
+return dev-archdata.of_node == data;
+}
+
+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;
+   
+   return to_i2c_client(dev);
+}
+EXPORT_SYMBOL(of_find_i2c_device_by_node);
+
 MODULE_LICENSE(GPL);
diff --git a/include/linux/of_i2c.h b/include/linux/of_i2c.h
index bd2a870..17d5897 100644
--- a/include/linux/of_i2c.h
+++ b/include/linux/of_i2c.h
@@ -16,5 +16,7 @@
 
 void of_register_i2c_devices(struct i2c_adapter *adap,
 struct device_node *adap_node);
+struct i2c_client *of_find_i2c_device_by_node(struct device_node *node);
+
 
 #endif /* __LINUX_OF_I2C_H */


___
i2c mailing list
i2c@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/i2c


Re: [i2c] [PATCH] Add the of_find_i2c_device_by_node function

2008-10-19 Thread Jon Smirl
On Sun, Oct 19, 2008 at 5:20 PM, Anton Vorontsov
[EMAIL PROTECTED] wrote:
 Hi Jon,

 On Sun, Oct 19, 2008 at 10:00:40AM -0400, Jon Smirl wrote:
 Add the of_find_i2c_device_by_node function. This allows you to follow
 a reference in the device tree to an i2c device node and then locate
 the linux device instantiated by the device tree. Example use, an i2s
 codec controlled by i2c. Depends on patch exporting i2c root bus symbol.

 Signed-off-by: Jon Smirl [EMAIL PROTECTED]

 Few comments are below.

 ---
  drivers/of/of_i2c.c |   28 
  1 files changed, 24 insertions(+), 4 deletions(-)

 diff --git a/drivers/of/of_i2c.c b/drivers/of/of_i2c.c
 index 6a98dc8..ba7b394 100644
 --- a/drivers/of/of_i2c.c
 +++ b/drivers/of/of_i2c.c
 @@ -19,7 +19,7 @@
  void of_register_i2c_devices(struct i2c_adapter *adap,
struct device_node *adap_node)
  {
 - void *result;
 + struct i2c_client *i2c_dev;
   struct device_node *node;

   for_each_child_of_node(adap_node, node) {
 @@ -41,18 +41,38 @@ void of_register_i2c_devices(struct i2c_adapter *adap,

   info.addr = *addr;

 - request_module(info.type);
 + request_module(%s, info.type);

 Patch description doesn't mention this change.

Patches for this have been posted before by other people and they
aren't making it in.

This is the original mail
http://lkml.org/lkml/2008/6/13/290
http://lkml.org/lkml/2008/6/12/8
I can't find the ones patching i2c.




 - result = i2c_new_device(adap, info);
 - if (result == NULL) {
 + i2c_dev = i2c_new_device(adap, info);
 + if (i2c_dev == NULL) {
   printk(KERN_ERR
  of-i2c: Failed to load driver for %s\n,
  info.type);
   irq_dispose_mapping(info.irq);
   continue;
   }
 +
 + i2c_dev-dev.archdata.of_node = of_node_get(node);

 Would break sparc build. Plus setting this after i2c_new_device() isn't
 right... Recently I sent few patches to deal with the archdata, could
 you please rebase your patch against these three patches?

 http://lkml.org/lkml/2008/10/16/250
 http://lkml.org/lkml/2008/10/16/251
 http://lkml.org/lkml/2008/10/16/252

   }
  }
  EXPORT_SYMBOL(of_register_i2c_devices);

 +static int of_dev_node_match(struct device *dev, void *data)
 +{
 +return dev-archdata.of_node == data;
 +}
 +
 +struct i2c_client *of_find_i2c_device_by_node(struct device_node *node)

 This should be documented. Especially the fact that every time you
 call this function, you must call device_put() when you're done with
 the returned i2c_client.

 +{
 + struct device *dev;
 +
 + dev = bus_find_device(i2c_bus_type, NULL, node,
 +  of_dev_node_match);
 + if (!dev)
 + return NULL;
 +
 + return to_i2c_client(dev);
 +}
 +EXPORT_SYMBOL(of_find_i2c_device_by_node);
 +
  MODULE_LICENSE(GPL);
 diff --git a/include/linux/of_i2c.h b/include/linux/of_i2c.h
 index bd2a870..17d5897 100644
 --- a/include/linux/of_i2c.h
 +++ b/include/linux/of_i2c.h
 @@ -16,5 +16,7 @@

  void of_register_i2c_devices(struct i2c_adapter *adap,
struct device_node *adap_node);
 +struct i2c_client *of_find_i2c_device_by_node(struct device_node *node);
 +

  #endif /* __LINUX_OF_I2C_H */

 Thanks,

 --
 Anton Vorontsov
 email: [EMAIL PROTECTED]
 irc://irc.freenode.net/bd2




-- 
Jon Smirl
[EMAIL PROTECTED]

___
i2c mailing list
i2c@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/i2c


Re: [i2c] Fwd: [PATCH 2/3] I2C: at24: add kernel interface for reading/writing EEPROM

2008-08-28 Thread Jon Smirl
On 8/28/08, David Brownell [EMAIL PROTECTED] wrote:
 On Wednesday 27 August 2008, Jon Smirl wrote:
   On 8/27/08, David Brownell [EMAIL PROTECTED] wrote:
On Tuesday 26 August 2008, Trent Piepho wrote:
  Lots of tv cards have eeproms with configuration information.  They 
 use an
  I2C driver called tveeprom that exports this:
 
  int tveeprom_read(struct i2c_client *c, unsigned char *eedata, int 
 len)
   
   
That presumes the driver somehow has access to that I2C client.
  
   How about using bus_find_device? PowerPC uses it to locate the i2c
   device using pointers in the device tree.


 Still making too many assumptions for my taste ... like using
  I2C, and in this case also OF.  What I want is an approach that
  can work with all the drivers with EEPROM and NVRAM support
  that I've happenened on so far:


 drivers/i2c/chips/at24.c

 drivers/spi/chips/at25.c
 drivers/rtc/rtc-cmos.c
 drivers/rtc/rtc-ds1305.c
 drivers/rtc/rtc-ds1307.c
 drivers/rtc/rtc-ds1511.c
 drivers/rtc/rtc-ds1553.c
 drivers/rtc/rtc-ds1742.c
 drivers/rtc/rtc-m48t59.c
 drivers/rtc/rtc-stk17ta8.c

  All those could easily export a (renamed) struct at24_iface * so
  their persistent (and updatable) memory could be exported to kernel
  code using the very same (simple) interface.

  Discussion about how to use a different interface than what's shown
  in the $SUBJECT patch seems to want to promote (a) each of those ten
  drivers having a *different* interface exposed by EXPORT_SYMBOL(),
  or else don't support in-kernel access at all, and (b) a bunch of
  extra glue code to support it, like the OF-specific bits you showed
  below and then going to the code that knows to use those bits to
  get which device, and how to ask those devices for their data.

  Moreover, IMO the basic question is still whether there is a good
  reason not to build on the $SUBJECT patch.  (So far:  no.)

  Sure it *could* be done differently -- especially if think (a) is
  good but being able to reuse interfaces is bad -- but it's like
  spending so much time choosing what color to paint the bikeshed
  that the bikeshed itself never gets built...


There are two parts to the puzzle.
1) A common way to export the interface. That can be addressed by
applying the technique in the initial patch.

2) How do the drivers find each other. Device trees already have a way
for drivers to find each other. This is an example of a audio codec
that lives on both the i2c and i2s bus. The codec-handle is used to
retrieve the tas0 node pointer. of_find_i2c_device_by_node then
searches the archdata for that pointer.

struct i2c_client *of_find_i2c_device_by_node(struct device_node
*node) could be generalized by having it search multiple buses for the
device. dev-archdata.of_node is just a cookie, it is only used to
match against. The pointer could be a void*.

I didn't like setup call part of the proposal. It is building another
way for drivers to find each other. How can you generate an equivalent
to the archdata.of_node cookie for other platforms? Another problem
with the setup callback, how do you tell identical devices apart?

[EMAIL PROTECTED] { /* PSC2 in i2s mode */
compatible = 
fsl,mpc5200b-psc-i2s,fsl,mpc5200-psc-i2s;
cell-index = 1;
reg = 0x2200 0x100;
interrupts = 0x2 0x2 0x0;
interrupt-parent = mpc5200_pic;
codec-handle = tas0;
};

[EMAIL PROTECTED] {
#address-cells = 1;
#size-cells = 0;
compatible = 
fsl,mpc5200b-i2c,fsl,mpc5200-i2c,fsl-i2c;
cell-index = 0;
reg = 0x3d00 0x40;
interrupts = 0x2 0xf 0x0;
interrupt-parent = mpc5200_pic;
fsl5200-clocking;

tas0:[EMAIL PROTECTED] {
compatible = ti,tas5504;
reg = 0x1b;
};
clock0:[EMAIL PROTECTED] {
compatible = maxim,max9485;
reg = 0x68;
};
};





  - Dave

  p.s. The only nontrivial technical issue I have with $PATCH is that
  struct at24_iface needs to cope better with rmmod at24.
  An optional teardown(...) call would suffice.



   +static int of_dev_node_match(struct device *dev, void *data)
   +{
   +return dev-archdata.of_node == data;
   +}
   +
   +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

Re: [i2c] Fwd: [PATCH 2/3] I2C: at24: add kernel interface for reading/writing EEPROM

2008-08-27 Thread Jon Smirl
On 8/27/08, David Brownell [EMAIL PROTECTED] wrote:
 On Tuesday 26 August 2008, Trent Piepho wrote:
   Lots of tv cards have eeproms with configuration information.  They use an
   I2C driver called tveeprom that exports this:
  
   int tveeprom_read(struct i2c_client *c, unsigned char *eedata, int len)


 That presumes the driver somehow has access to that I2C client.

How about using bus_find_device? PowerPC uses it to locate the i2c
device using pointers in the device tree.

+static int of_dev_node_match(struct device *dev, void *data)
+{
+return dev-archdata.of_node == data;
+}
+
+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;
+   
+   return to_i2c_client(dev);
+}
+EXPORT_SYMBOL(of_find_i2c_device_by_node);
+





  (Also, that it's an I2C EEPROM.  I keep wanting to have something
  usable for SPI EEPROMs and NVRAM too.  But maybe nobody else cares.)



Last I heard, the U-Boot policy was not to touch controllers that are
not required during boot.  Despite relatively common cases like needing
to set up MAC addresses ... maybe this patch of Kevin's can make it a
bunch easier to cope with that U-Boot policy.  :)
  
   U-boot can put the MAC address into a property of the MAC's device node in
   the OF device tree (which u-boot passes to the kernel).


 Maybe with PowerPC it does.  Not on ARM.  ;)





-- 
Jon Smirl
[EMAIL PROTECTED]

___
i2c mailing list
i2c@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/i2c


Re: [i2c] [PATCH] powerpc: i2c-mpc: make speed registers configurable via FDT

2008-08-01 Thread Jon Smirl
On 8/1/08, Timur Tabi [EMAIL PROTECTED] wrote:
 On Thu, Jul 31, 2008 at 9:35 PM, Jon Smirl [EMAIL PROTECTED] wrote:

   I've having trouble with whether these clocks are a system resource or
   something that belongs to i2c. If they are a system resource then we
   should make nodes in the root and use a phandle in the i2c node to
   link to them.


 I can't speak for 52xx, but for 8[356]xx, I would say the clocks
  belong to the I2C devices.  The screwball determination of whether to
  divide by 1, 2, or 3 only applies to the I2C device only.  The divider
  itself is not used to drive a clock for any other device.  If you
  disable I2C support, then you don't need to care about the divider (or
  its output clock) at all.  That's why I think have the I2C clock in
  the parent node is wrong, because it would only be used if you had I2C
  child nodes.  If you didn't have any I2C nodes, then you would need to
  delete the property from the parent node as well.

I see this as being one of three ways...

The source clocks belong to the platform and the clock messiness
belongs in the platform code.

The source clocks belong to i2c. The i2c driver needs to use platform
specific bindings like Grant proposed.

I don't like the third choice. Keep a simple Linux driver for i2c and
the platform, and then move all of the messy code into uboot.  BTW,
the messy code is about 10 lines. It's going to take more than 10
lines to hide those 10 lines.

I'm also of the view that all clocks are system resources. Linux is
designed to have clock domains, we just aren't using them on PowerPC.
Check out arch/powerpc/kernel/clock.c. They are mostly used on ARM.
Could we define domains I2C1, I2C2,.. and then implement them? That
implies using the root node to communicate the clock speeds to Linux.

-- 
Jon Smirl
[EMAIL PROTECTED]

___
i2c mailing list
i2c@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/i2c


Re: [i2c] [PATCH] powerpc: i2c-mpc: make speed registers configurable via FDT

2008-08-01 Thread Jon Smirl
On 8/1/08, Timur Tabi [EMAIL PROTECTED] wrote:
 On Thu, Jul 31, 2008 at 9:35 PM, Jon Smirl [EMAIL PROTECTED] wrote:

   I've having trouble with whether these clocks are a system resource or
   something that belongs to i2c. If they are a system resource then we
   should make nodes in the root and use a phandle in the i2c node to
   link to them.


 I can't speak for 52xx, but for 8[356]xx, I would say the clocks
  belong to the I2C devices.  The screwball determination of whether to
  divide by 1, 2, or 3 only applies to the I2C device only.  The divider
  itself is not used to drive a clock for any other device.  If you
  disable I2C support, then you don't need to care about the divider (or
  its output clock) at all.  That's why I think have the I2C clock in
  the parent node is wrong, because it would only be used if you had I2C
  child nodes.  If you didn't have any I2C nodes, then you would need to
  delete the property from the parent node as well.

I see this as being one of three ways...

The source clocks belong to the platform and the clock messiness
belongs in the platform code.

The source clocks belong to i2c. The i2c driver needs to use platform
specific bindings like Grant proposed.

I don't like the third choice. Keep a simple Linux driver for i2c and
the platform, and then move all of the messy code into uboot.  BTW,
the messy code is about 10 lines. It's going to take more than 10
lines to hide those 10 lines.

I'm also of the view that all clocks are system resources. Linux is
designed to have clock domains, we just aren't using them on PowerPC.
Check out arch/powerpc/kernel/clock.c. They are mostly used on ARM.
Could we define domains I2C1, I2C2,.. and then implement them? That
implies using the root node to communicate the clock speeds to Linux.

-- 
Jon Smirl
[EMAIL PROTECTED]

___
i2c mailing list
i2c@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/i2c


Re: [i2c] [PATCH] powerpc: i2c-mpc: make speed registers configurable via FDT

2008-08-01 Thread Jon Smirl
On 8/1/08, Timur Tabi [EMAIL PROTECTED] wrote:
 Jon Smirl wrote:


  What about the Efika which is mpc5200 based and doesn't use uboot?
 

  How does the Efika handle the dozen other properties that U-Boot normally
 initializes in the device tree?

Efika is like the original OpenFirmware. It has a Forth interpreter
and builds the device tree itself. It doesn't use flat device trees
that get filled in by the boot firmware.


  --
  Timur Tabi
  Linux Kernel Developer @ Freescale



-- 
Jon Smirl
[EMAIL PROTECTED]

___
i2c mailing list
i2c@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/i2c


Re: [i2c] [PATCH] powerpc: i2c-mpc: make speed registers configurable via FDT

2008-07-31 Thread Jon Smirl
On 7/31/08, Scott Wood [EMAIL PROTECTED] wrote:
 Timur Tabi wrote:

  Grant Likely wrote:
 
 
   No it doesn't, it depends on the register interface to decide
   compatibility.  Clock interface is part of that.
  
 
  I don't think so.  The interface for programming the clock registers is
  identical on all 8[356]xx parts.  The only thing that matters is what
 specific
  values to put in the FDR and DFSR registers to get a desired I2C bus
 speed.
 

  If it affects the values you need to write to the registers to achieve a
 given result, how is it not a difference in the register interface?


  I propose the property clock-frequency, like this:
 
 [EMAIL PROTECTED] {
 #address-cells = 1;
 #size-cells = 0;
 cell-index = 0;
 compatible = fsl-i2c;
 reg = 0x3000 0x100;
 interrupts = 14 0x8;
 interrupt-parent = ipic;
 dfsrr;
 clock-frequency = 0xblablabla;  -- added by
 U-Boot
 };
 

  A clock-frequency property is OK, and is in line with what we do in other
 types of nodes.  However, in the long run it might be nice to introduce some
 sort of clock binding where, for example, the i2c node can point to a clock
 elsewhere in the device tree as an input clock.

  That way, less knowledge is required by the firmware to poke values all
 over the place, and it also allows one to describe situations where the
 frequency of the input clock can change (such as in low-power modes).

PowerPC,[EMAIL PROTECTED] {
timebase-frequency = 0;   /* From Bootloader  */
bus-frequency = 0;/* From Bootloader  */
clock-frequency = 0;  /* From Bootloader  */
};

The mpc5200 code already has mpc52xx_find_ipb_freq() to get it.

I should grep before suggesting things.

-- 
Jon Smirl
[EMAIL PROTECTED]

___
i2c mailing list
i2c@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/i2c


Re: [i2c] [PATCH] powerpc: i2c-mpc: make speed registers configurable via FDT

2008-07-31 Thread Jon Smirl
On 7/31/08, Wolfgang Grandegger [EMAIL PROTECTED] wrote:
 Timur Tabi wrote:

  Wolfgang Grandegger wrote:
 
 
   But clock-frequency, aka bus-frequency, is already used by
 fsl_get_sys_freq():
  
  
 http://lxr.linux.no/linux+v2.6.26/arch/powerpc/sysdev/fsl_soc.c#L80
  
 
  So?  clock-frequency is a per-node property.  I use it in the codec node
 on the
  8610 (mpc8610_hpcd.dts).  It does not mean platform clock frequency.
 
 
   U-Boot could then fixup that value like bus-frequency() and the i2c-mpc
 driver simply calls fsl_get_i2c_freq().
  
 
  This is just more complicated than it needs to be.  Why should the I2C
 driver
  fetch the platform clock and the divider from the parent node, and then do
  additional math, when it could just get the value it needs right from the
 node
  it's probing?
 

  I'm a bit confused. The frequency of the I2C source clock and the real I2C
 clock frequency are two different things. The first one is common for all
 I2C devices, the second can be different. What properties would you like to
 use for defining both?

For mpc5200 it is easy, mpc52xx_find_ipb_freq()  already exists to get
the source clock for the i2c devices. Each i2c node in the device tree
can then specify clock-frequency = 40; or let it default. You
have the input clock and the desired clock, do some math and you can
set FDR.

For the 8xxx chips there is no simple solution for obtain the input
clock for the i2c section. Maybe create something like i2c-frequency
and set it from uboot. The make another accessor function like
mpc8xxx_find_i2c_freq().

PowerPC,[EMAIL PROTECTED] {
timebase-frequency = 0;   /* From Bootloader  */
bus-frequency = 0;/* From Bootloader  */
clock-frequency = 0;  /* From Bootloader  */
i2c-frequency = 0;/* From Bootloader  */
};

Instead of creating i2c-frequency in the device tree, you could put
this piece of code in the the mpc8xxx platform driver and use it to
implement mpc8xxx_find_i2c_freq(). Read the PORDEVSR2_SEC_CFG bit back
from whatever uboot set it too.

#if defined(CONFIG_MPC8540) || defined(CONFIG_MPC8541) || \
   defined(CONFIG_MPC8560) || defined(CONFIG_MPC8555)
   gd-i2c1_clk = sys_info.freqSystemBus;
#elif defined(CONFIG_MPC8544)
   /*
* On the 8544, the I2C clock is the same as the SEC clock.  This can be
* either CCB/2 or CCB/3, depending on the value of cfg_sec_freq. See
* 4.4.3.3 of the 8544 RM.  Note that this might actually work for all
* 85xx, but only the 8544 has cfg_sec_freq, so it's unknown if the
* PORDEVSR2_SEC_CFG bit is 0 on all 85xx boards that are not an 8544.
*/
   if (gur-pordevsr2  MPC85xx_PORDEVSR2_SEC_CFG)
   gd-i2c1_clk = sys_info.freqSystemBus / 3;
   else
   gd-i2c1_clk = sys_info.freqSystemBus / 2;
#else
   /* Most 85xx SOCs use CCB/2, so this is the default behavior. */
   gd-i2c1_clk = sys_info.freqSystemBus / 2;
#endif
   gd-i2c2_clk = gd-i2c1_clk;


-- 
Jon Smirl
[EMAIL PROTECTED]

___
i2c mailing list
i2c@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/i2c


Re: [i2c] [PATCH] powerpc: i2c-mpc: make speed registers configurable via FDT

2008-07-31 Thread Jon Smirl
On 7/31/08, Timur Tabi [EMAIL PROTECTED] wrote:
 Jon Smirl wrote:

   For mpc5200 it is easy, mpc52xx_find_ipb_freq()  already exists to get
   the source clock for the i2c devices. Each i2c node in the device tree
   can then specify clock-frequency = 40; or let it default. You


 400KHz is the *speed* of the I2C bus, so let's be sure to use speed in the
  property name.  Reserve the word bus for the input clock to the I2C device.


   #if defined(CONFIG_MPC8540) || defined(CONFIG_MPC8541) || \
  defined(CONFIG_MPC8560) || defined(CONFIG_MPC8555)
  gd-i2c1_clk = sys_info.freqSystemBus;
   #elif defined(CONFIG_MPC8544)
  /*
   * On the 8544, the I2C clock is the same as the SEC clock.  This 
 can be
   * either CCB/2 or CCB/3, depending on the value of cfg_sec_freq. 
 See
   * 4.4.3.3 of the 8544 RM.  Note that this might actually work for 
 all
   * 85xx, but only the 8544 has cfg_sec_freq, so it's unknown if the
   * PORDEVSR2_SEC_CFG bit is 0 on all 85xx boards that are not an 
 8544.
   */
  if (gur-pordevsr2  MPC85xx_PORDEVSR2_SEC_CFG)
  gd-i2c1_clk = sys_info.freqSystemBus / 3;
  else
  gd-i2c1_clk = sys_info.freqSystemBus / 2;
   #else
  /* Most 85xx SOCs use CCB/2, so this is the default behavior. */
  gd-i2c1_clk = sys_info.freqSystemBus / 2;
   #endif
  gd-i2c2_clk = gd-i2c1_clk;


 I think the whole point is to eliminate duplicating this code in the Linux
  driver.  It's hideously ugly, so it should be limited as much as possible.

It wouldn't go into the i2c driver, it would go into the mpc8xxx
platform driver. Why is it bad to put it into the mpc8xxx platform
driver? It is an accurate description of the mpc8xxx platform isn't
it?


-- 
Jon Smirl
[EMAIL PROTECTED]

___
i2c mailing list
i2c@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/i2c


Re: [i2c] [PATCH] powerpc: i2c-mpc: make speed registers configurable via FDT

2008-07-31 Thread Jon Smirl
On 7/31/08, Grant Likely [EMAIL PROTECTED] wrote:
 On Thu, Jul 31, 2008 at 03:37:07PM -0500, Timur Tabi wrote:
   Grant Likely wrote:
  
How is the divider controlled?  Is it a fixed property of the SoC?
  
   Yes.  The divider is either 1, 2, or 3, and the only way to know which one
   it is is to look up the specific SOC model number.  And depending on the
   SOC model, there may also be a register that needs to be looked up.
  
a
shared register setting? or a register setting within the i2c device?
  
   The I2C device itself has no idea what the divider is.  It only sees the
   result of the divider.


 Then that absolutely suggests to me that either the final clock or the
  divider should be encoded in the i2c node; not in the soc node.

Isn't there a single global divider that generates all the i2c source
clocks? You don't want to copy a global value into each i2c node.
Aren't we talking about the /2 or /3 or /1 divider that appears to be
randomly implemented on various members of the mpc8xxx family?



  g.



-- 
Jon Smirl
[EMAIL PROTECTED]

___
i2c mailing list
i2c@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/i2c


Re: [i2c] [PATCH] powerpc: i2c-mpc: make speed registers configurable via FDT

2008-07-31 Thread Jon Smirl
On 7/31/08, Timur Tabi [EMAIL PROTECTED] wrote:
 Jon Smirl wrote:

   Isn't there a single global divider that generates all the i2c source
   clocks? You don't want to copy a global value into each i2c node.


 Why not?  There are only two I2C devices, and it's theoretically possible for
  them to have different input clock frequencies.   Keeping it in the I2C node
  allows the I2C driver to reference a property directly in the node that its 
 probing.

But that's the same as saying we should copy the system clock
frequency into all of the PSC nodes because we might implement
hardware where they aren't all clocked off from the same input clock
source.

   Aren't we talking about the /2 or /3 or /1 divider that appears to be
   randomly implemented on various members of the mpc8xxx family?

I don't this these dividers or clocks need to be exposed at all if
you'd just put that ugly code snippet into your platform driver.




 Yes.


  --
  Timur Tabi
  Linux kernel developer at Freescale



-- 
Jon Smirl
[EMAIL PROTECTED]

___
i2c mailing list
i2c@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/i2c


Re: [i2c] [PATCH] powerpc: i2c-mpc: make speed registers configurable via FDT

2008-07-31 Thread Jon Smirl
On 7/31/08, Trent Piepho [EMAIL PROTECTED] wrote:
 On Thu, 31 Jul 2008, Jon Smirl wrote:

Here's my idea:
   
[EMAIL PROTECTED] {
compatible = fsl-i2c;
bus-frequency = 10;
   
/* Either */
source-clock-frequency = 0;
/* OR */
source-clock = ccb;
};
  
   Can't we hide a lot of this on platforms where the source clock is not
   messed up? For example the mpc5200 doesn't need any of this, the
   needed frequency is already available in mpc52xx_find_ipb_freq().
   mpc5200 doesn't need any uboot change.
  
   Next would be normal mpc8xxx platforms where i2c is driven by a single
   clock, add a uboot filled in parameter in the root node (or I think it
   can be computed off of the ones uboot is already filling in). make a
   mpc8xxx_find_i2c_freq() function. May not need to change device tree
   and uboot.
  
   Finally use this for those days when the tea leaves were especially
   bad. Both a device tree and uboot change.


 If you have to support clock speed in the i2c node anyway, what's the point
  of the other options?

So that I don't have to change my existing mpc5200 systems. mpc5200
has no need for specifying the source clock in each i2c node, hardware
doesn't allow it.

Except the i2c clock isn't always a based on an integer divider of the 
 CCB
 frequency.  What's more, it's not always the same for both i2c 
 controllers.
 Suppose i2c #1 uses CCB times 2/3 and i2c #2 uses CCB/2, how would
 fsl_get_i2c_freq() figure that out from bus-frequency and
 i2c-clock-divider?
  
   If you get the CCB frequency from uboot and know the chip model, can't
   you compute these in the platform code? Then make a
   mpc8xxx_find_i2c_freq(cell_index).


 You need a bunch of random other device registers (SEC, ethernet, sdhc,
  etc.) too.


   I don't see why we want to go through the trouble of having uboot tell
   us things about a chip that are fixed in stone. Obviously something
   like the frequency of the external crystal needs to be passed up, but
   why pass up stuff that can be computed (or recovered by reading a
   register)?


 One could also say that if U-boot has to have the code and already
  calculated the value, why duplicate the code and the calculation in Linux?

What about the Efika which is mpc5200 based and doesn't use uboot?

-- 
Jon Smirl
[EMAIL PROTECTED]

___
i2c mailing list
i2c@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/i2c


Re: [i2c] [PATCH] powerpc: i2c-mpc: make speed registers configurable via FDT

2008-07-31 Thread Jon Smirl
On 7/31/08, Grant Likely [EMAIL PROTECTED] wrote:
 Your mixing up device tree layout with implementation details.  Device
  tree layout must come first.  mpc52xx_find_ipb_freq() is just a
  convenience function that walks up the device tree looking for a
  bus-frequency property.

  So, instead of making arguments based on available helper functions,
  make your arguments based on how data should be laid out in the device
  tree.  Currently mpc5200 bindings simply depend on finding a
  bus-frequency property in the parent node for determining the input
  clock and I don't see any pressing reason to change that (though it
  probably needs to be documented better).

  However, for the complex cases that Trent and Timur are talking about,
  it makes perfect sense to have an optional property in the i2c node
  itself that defines a different clock.  Once that decision has been made
  and documented, then the driver can be modified and the appropriate
  helper functions added to adapt the device tree data into something
  useful.

I've having trouble with whether these clocks are a system resource or
something that belongs to i2c. If they are a system resource then we
should make nodes in the root and use a phandle in the i2c node to
link to them.

The phandle in the mpc5200 case could be implicit since it is fixed in silicon.

Is this register in a system register bank or an i2c one?
gur-pordevsr2  MPC85xx_PORDEVSR2_SEC_CFG


  Remember (and chant this to yourself).  The device tree describes
  *hardware*.  It does not describe Linux internal implementation details.


  g.




-- 
Jon Smirl
[EMAIL PROTECTED]

___
i2c mailing list
i2c@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/i2c


Re: [i2c] at24.c, how to get kobject for sysfs/bin_attribute methods?

2008-07-28 Thread Jon Smirl
PowerPC is going to use this

Add the of_find_i2c_device_by_node function

From: Jon Smirl [EMAIL PROTECTED]

Add the of_find_i2c_device_by_node function. This allows you to follow
a reference in the device tree to an i2c device node and then locate
the linux device instantiated by the device tree. Example use, an i2s
codec controlled by i2c. Depends on patch exporting i2c root bus symbol.

Signed-off-by: Jon Smirl [EMAIL PROTECTED]
---

 drivers/of/of_i2c.c|   28 
 include/linux/of_i2c.h |2 ++
 2 files changed, 26 insertions(+), 4 deletions(-)

diff --git a/drivers/of/of_i2c.c b/drivers/of/of_i2c.c
index 6a98dc8..ba7b394 100644
--- a/drivers/of/of_i2c.c
+++ b/drivers/of/of_i2c.c
@@ -19,7 +19,7 @@
 void of_register_i2c_devices(struct i2c_adapter *adap,
 struct device_node *adap_node)
 {
-   void *result;
+   struct i2c_client *i2c_dev;
struct device_node *node;

for_each_child_of_node(adap_node, node) {
@@ -41,18 +41,38 @@ void of_register_i2c_devices(struct i2c_adapter *adap,

info.addr = *addr;

-   request_module(info.type);
+   request_module(%s, info.type);

-   result = i2c_new_device(adap, info);
-   if (result == NULL) {
+   i2c_dev = i2c_new_device(adap, info);
+   if (i2c_dev == NULL) {
printk(KERN_ERR
   of-i2c: Failed to load driver for %s\n,
   info.type);
irq_dispose_mapping(info.irq);
continue;
}
+   
+   i2c_dev-dev.archdata.of_node = of_node_get(node);
}
 }
 EXPORT_SYMBOL(of_register_i2c_devices);

+static int of_dev_node_match(struct device *dev, void *data)
+{
+return dev-archdata.of_node == data;
+}
+
+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;
+   
+   return to_i2c_client(dev);
+}
+EXPORT_SYMBOL(of_find_i2c_device_by_node);
+
 MODULE_LICENSE(GPL);
diff --git a/include/linux/of_i2c.h b/include/linux/of_i2c.h
index bd2a870..17d5897 100644
--- a/include/linux/of_i2c.h
+++ b/include/linux/of_i2c.h
@@ -16,5 +16,7 @@

 void of_register_i2c_devices(struct i2c_adapter *adap,
 struct device_node *adap_node);
+struct i2c_client *of_find_i2c_device_by_node(struct device_node *node);
+   

 #endif /* __LINUX_OF_I2C_H */


-- 
Jon Smirl
[EMAIL PROTECTED]

___
i2c mailing list
i2c@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/i2c


Re: [i2c] Problem with restricted I2C algorithms in kernel 2.6.26!

2008-07-26 Thread Jon Smirl
On 7/26/08, Andrew Morton [EMAIL PROTECTED] wrote:
 (cc's added)


  On Wed, 16 Jul 2008 12:33:57 -0700 D. Kelly [EMAIL PROTECTED] wrote:

   
 http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=3845de25c5f83cd52729570f7b501679d37ca8de
  
   The patch at the preceeding url disables the users ability to select
   I2C algorithms.  Specifically the reason stated was:
  
   The algorithm drivers are
   helper drivers that are selected automatically
   as needed. There's no point in listing them in the config menu, it can
   only confuse users and waste their time.

I support Jean's decision on this. Very few people know how to
correctly enable those algorithms and most people get them wrong. Now
they are set automatically.

What about merging a placeholder driver for dvb-s2 that selects the
needed i2c algorithm? Then merge a real driver as soon as possible.

My out of tree drivers are written as a patch against the kernel. The
patch contains a select for the algorithm in my Kconfig additions.
When the drivers work, I'll submit them.



  
   The algorithm drivers will not be 'selected automatically as needed'
   if the user is compiling something outside of the kernel that requires
   them!  Just one example, there are drivers found in the V4L dvb driver
   tree that require i2c bit-banging be enabled.  The drivers are now
   broken because the user is not allowed to enable bit-banging himself.
   The only way around this is to revert the patch manually or enable
   something else in the kernel, that he doesn't need, just to get
   bit-banging.
  
   It's a very bad idea to assume that nothing built outside of the
   kernel may need i2c algorithms.  Furthermore, the whole point of being
   able to customize your kernel is so you can select only the things
   which you need.  It makes no good sense to intentionally
   disable/restrict the users ability to do so.  Additionally, assuming
   the ability to select i2c algorithms will only confuse the user and
   waste their time is ridiculous.  The user should be allowed to decide
   for himself what he needs regarding this!
  
   One of the biggest reasons people choose to compile things from
   cvs/svn/mercurial/etc. is because it gives them access to newer bug
   fixes and support for things not yet present in the kernel source.  A
   perfect example, the multiproto dvb driver tree.  Users wanting
   support for dvb-s2 devices have to compile drivers outside of the
   kernel because it's simply not available in the kernel and won't be
   for some time.
  
   I've contacted one of the i2c subsystem maintainers, Jean Delvare, but
   unfortunately he doesn't seem to care about this problem and his
   advice in dealing with it is to Just get these drivers merged in the
   kernel. Ah ah ah!...
  
   Clearly the more sane and reasonable solution is to not cripple the
   menu options in the first place, especially when it creates no benefit
   and only serves to limit/restrict the users ability to select what he
   needs.  I'm asking that the patch be reverted and anyone in agreement
   to please voice their opinion here in public.
  
   Best regards,
   -Derek



 ___
  i2c mailing list
  i2c@lm-sensors.org
  http://lists.lm-sensors.org/mailman/listinfo/i2c



-- 
Jon Smirl
[EMAIL PROTECTED]

___
i2c mailing list
i2c@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/i2c


Re: [i2c] [PATCH] rtc-pcf8563: Remove client validation

2008-07-22 Thread Jon Smirl
On 7/22/08, Laurent Pinchart [EMAIL PROTECTED] wrote:
 Hi Alexander,


  On Wednesday 02 July 2008, Jean Delvare wrote:
   On Wed, 2 Jul 2008 13:32:14 +0200, Laurent Pinchart wrote:
Validating clients with black magic register checks doesn't make much 
 sense
for new-style i2c driver and has been known to fail on valid NXP pcf8563 
 chips.
This patch removes the client validation code.

I've hit this same problem with the rtc8564. When the chips are first
installed the registers haven't been initialized. This check makes the
driver load fail which prevents the user space clock app from running
and initializing the registers. The work around was to set the initial
time from uboot.

-- 
Jon Smirl
[EMAIL PROTECTED]

___
i2c mailing list
i2c@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/i2c


Re: [i2c] int vs uint

2008-07-21 Thread Jon Smirl
On 7/21/08, Ben Dooks [EMAIL PROTECTED] wrote:
 On Sun, Jul 20, 2008 at 12:47:38PM -0400, Jon Smirl wrote:
   On 7/20/08, Jon Smirl [EMAIL PROTECTED] wrote:
On 7/20/08, Ben Dooks [EMAIL PROTECTED] wrote:
  On Sat, Jul 19, 2008 at 08:46:41PM -0400, Jon Smirl wrote:
There are a lot places in the i2c API where int is used when the
parameter can't be negative. For example, there are more
   
/*
 * The master routines are the ones normally used to transmit data 
 to devices
 * on a bus (or read from them). Apart from two basic transfer 
 functions to
 * transmit one message at a time, a more complex version can be 
 used to
 * transmit an arbitrary number of messages without interruption.
 */
extern int i2c_master_send(struct i2c_client *,const char* ,int);
extern int i2c_master_recv(struct i2c_client *,char* ,int);
   
/* Transfer num messages.
 */
extern int i2c_transfer(struct i2c_adapter *adap, struct i2c_msg
*msgs, int num);
   
  u8 level;   /* nesting level for lockdep 
 */
   
   
Wouldn't these generate more efficient code if switched to uints?
 
 
  I'm not sure, most of the time there's not a lot of difference
   between the signed and unsigned case. If you can provide an example
   of where this is actually true then I would be interested to see...
   
   
Assigning between lengths causes sign extended instructions. Mixing
 signed and unsigned in expressions can cause them too.


 Entirely dependant on the architecture, so very rarely see anything
  like this happening on ARM.


 You get the best code generation by using int or unsigned int as
 appropriate and supplying the compiler with the correct description of
 the variable.
  
   The compiler is also able to catch more errors. If the math can only
   yield a negative result and it is being compared to a unsigned int,
   you'll get compiler errors like  expression is constant.


 The compiler checking argument is the most compelling of these, are
  you offering to do the necessary changes, or is this fishing to find
  someone else to do it?

Fishing. Changes like these go a lot smoother if one of the
maintainers do them. Doing them as an outside patch will a generate a
pile of merge conflicts with other patches made during the cycle.

I'd just like to see the i2c interface be as accurate as possible. It
is clear that some of those ints should be uints.




  --

 Ben ([EMAIL PROTECTED], http://www.fluff.org/)

   'a smiley only costs 4 bytes'



-- 
Jon Smirl
[EMAIL PROTECTED]

___
i2c mailing list
i2c@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/i2c


Re: [i2c] int vs uint

2008-07-20 Thread Jon Smirl
On 7/20/08, Ben Dooks [EMAIL PROTECTED] wrote:
 On Sat, Jul 19, 2008 at 08:46:41PM -0400, Jon Smirl wrote:
   There are a lot places in the i2c API where int is used when the
   parameter can't be negative. For example, there are more
  
   /*
* The master routines are the ones normally used to transmit data to 
 devices
* on a bus (or read from them). Apart from two basic transfer functions to
* transmit one message at a time, a more complex version can be used to
* transmit an arbitrary number of messages without interruption.
*/
   extern int i2c_master_send(struct i2c_client *,const char* ,int);
   extern int i2c_master_recv(struct i2c_client *,char* ,int);
  
   /* Transfer num messages.
*/
   extern int i2c_transfer(struct i2c_adapter *adap, struct i2c_msg
   *msgs, int num);
  
 u8 level;   /* nesting level for lockdep */
  
  
   Wouldn't these generate more efficient code if switched to uints?


 I'm not sure, most of the time there's not a lot of difference
  between the signed and unsigned case. If you can provide an example
  of where this is actually true then I would be interested to see...

Assigning between lengths causes sign extended instructions. Mixing
signed and unsigned in expressions can cause them too.

You get the best code generation by using int or unsigned int as
appropriate and supplying the compiler with the correct description of
the variable.


  Technically, an unsigned int or simply an unsigned would be a
  reasonable change given that you can't really have a minus number
  of transfers.


  --
  Ben ([EMAIL PROTECTED], http://www.fluff.org/)

   'a smiley only costs 4 bytes'



-- 
Jon Smirl
[EMAIL PROTECTED]

___
i2c mailing list
i2c@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/i2c


Re: [i2c] int vs uint

2008-07-20 Thread Jon Smirl
On 7/20/08, Jon Smirl [EMAIL PROTECTED] wrote:
 On 7/20/08, Ben Dooks [EMAIL PROTECTED] wrote:
   On Sat, Jul 19, 2008 at 08:46:41PM -0400, Jon Smirl wrote:
 There are a lot places in the i2c API where int is used when the
 parameter can't be negative. For example, there are more

 /*
  * The master routines are the ones normally used to transmit data to 
 devices
  * on a bus (or read from them). Apart from two basic transfer 
 functions to
  * transmit one message at a time, a more complex version can be used to
  * transmit an arbitrary number of messages without interruption.
  */
 extern int i2c_master_send(struct i2c_client *,const char* ,int);
 extern int i2c_master_recv(struct i2c_client *,char* ,int);

 /* Transfer num messages.
  */
 extern int i2c_transfer(struct i2c_adapter *adap, struct i2c_msg
 *msgs, int num);

   u8 level;   /* nesting level for lockdep */


 Wouldn't these generate more efficient code if switched to uints?
  
  
   I'm not sure, most of the time there's not a lot of difference
between the signed and unsigned case. If you can provide an example
of where this is actually true then I would be interested to see...


 Assigning between lengths causes sign extended instructions. Mixing
  signed and unsigned in expressions can cause them too.

  You get the best code generation by using int or unsigned int as
  appropriate and supplying the compiler with the correct description of
  the variable.

The compiler is also able to catch more errors. If the math can only
yield a negative result and it is being compared to a unsigned int,
you'll get compiler errors like  expression is constant.



  
Technically, an unsigned int or simply an unsigned would be a
reasonable change given that you can't really have a minus number
of transfers.
  
  
--
Ben ([EMAIL PROTECTED], http://www.fluff.org/)
  
 'a smiley only costs 4 bytes'
  



 --
  Jon Smirl
  [EMAIL PROTECTED]



-- 
Jon Smirl
[EMAIL PROTECTED]

___
i2c mailing list
i2c@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/i2c


[i2c] int vs uint

2008-07-19 Thread Jon Smirl
There are a lot places in the i2c API where int is used when the
parameter can't be negative. For example, there are more

/*
 * The master routines are the ones normally used to transmit data to devices
 * on a bus (or read from them). Apart from two basic transfer functions to
 * transmit one message at a time, a more complex version can be used to
 * transmit an arbitrary number of messages without interruption.
 */
extern int i2c_master_send(struct i2c_client *,const char* ,int);
extern int i2c_master_recv(struct i2c_client *,char* ,int);

/* Transfer num messages.
 */
extern int i2c_transfer(struct i2c_adapter *adap, struct i2c_msg
*msgs, int num);

u8 level;   /* nesting level for lockdep */


Wouldn't these generate more efficient code if switched to uints?

-- 
Jon Smirl
[EMAIL PROTECTED]

___
i2c mailing list
i2c@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/i2c


Re: [i2c] i2c: cdev lock_kernel() pushdown

2008-07-15 Thread Jon Smirl
On 7/15/08, Jean Delvare [EMAIL PROTECTED] wrote:
 On Tue, 15 Jul 2008 10:14:06 -0600, Jonathan Corbet wrote:
   Hi, Jean,
  
I am looking at this patch of yours:

 http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=3db633ee352bfe20d4a2b0c3c8a46ce31a6c7149
   
I believe that no locking is needed in i2cdev_open(). Do you have any
reason to think it does? If not, can I simply revert this patch?
  
   Before now, i2cdev_open() has always had the protection of the BKL.
   When I pushed that locking down into the individual open() functions, I
   really had to take a pretty conservative approach and assume that the
   BKL was needed unless that was really obviously not the case.  In
   i2cdev_open(), for example, there's:
  
 i2c_dev = i2c_dev_get_by_minor(minor);
  
   and I really don't know what keeps *i2c_dev from going away during the
   rest of the call.  I'm *not* saying there is a problem here; I just
   don't know.  So the only thing I could really do is to push the BKL
   down and let the maintainers sort it out.
  
   ...all of which is my long-winded way of saying that, if you're
   convinced that i2cdev_open() is safe in the absence of the BKL, feel
   free to take it out.


 Good point. i2c_dev is guaranteed to stay thanks to the call to
  i2c_get_adapter(), however it happens _after_ the call to
  i2c_dev_get_by_minor(), so without additional locking, this is racy.
  That being said, I'm not sure how lock_kernel() is supposed to help. Is
  the BKL held when i2cdev_detach_adapter() is called? If not, then I
  suspect that the current code is already racy.

  I'll look into this, thanks for the hint.

Is i2c-dev vulnerable to the i2c device disappearing, for example
rmmod it? Would combining i2c-dev into i2c core and integrating it
with the core's lock protection make things easier to lock? You could
make a compile time option to remove it for small systems. If it's in
the core is attach/detach adapter still needed? I haven't looked at
any of this in detail, but i2c-dev is only 6K of code. Half of the 6K
might disappear if integrated into the core.

What happens if user space and an in-kernel user both try using the
device? I've never tried doing that. Should the presence of an
in-kernel user make the user space device disappear?

-- 
Jon Smirl
[EMAIL PROTECTED]

___
i2c mailing list
i2c@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/i2c


Re: [i2c] i2c: cdev lock_kernel() pushdown

2008-07-15 Thread Jon Smirl
On 7/15/08, Jean Delvare [EMAIL PROTECTED] wrote:
 Hi Jon,


  On Tue, 15 Jul 2008 13:01:07 -0400, Jon Smirl wrote:
   On 7/15/08, Jean Delvare [EMAIL PROTECTED] wrote:
On Tue, 15 Jul 2008 10:14:06 -0600, Jonathan Corbet wrote:
  Hi, Jean,
 
   I am looking at this patch of yours:
   
 http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=3db633ee352bfe20d4a2b0c3c8a46ce31a6c7149
  
   I believe that no locking is needed in i2cdev_open(). Do you have 
 any
   reason to think it does? If not, can I simply revert this patch?
 
  Before now, i2cdev_open() has always had the protection of the BKL.
  When I pushed that locking down into the individual open() functions, 
 I
  really had to take a pretty conservative approach and assume that the
  BKL was needed unless that was really obviously not the case.  In
  i2cdev_open(), for example, there's:
 
i2c_dev = i2c_dev_get_by_minor(minor);
 
  and I really don't know what keeps *i2c_dev from going away during the
  rest of the call.  I'm *not* saying there is a problem here; I just
  don't know.  So the only thing I could really do is to push the BKL
  down and let the maintainers sort it out.
 
  ...all of which is my long-winded way of saying that, if you're
  convinced that i2cdev_open() is safe in the absence of the BKL, feel
  free to take it out.
   
   
Good point. i2c_dev is guaranteed to stay thanks to the call to
 i2c_get_adapter(), however it happens _after_ the call to
 i2c_dev_get_by_minor(), so without additional locking, this is racy.
 That being said, I'm not sure how lock_kernel() is supposed to help. Is
 the BKL held when i2cdev_detach_adapter() is called? If not, then I
 suspect that the current code is already racy.
   
 I'll look into this, thanks for the hint.
  
   Is i2c-dev vulnerable to the i2c device disappearing, for example
   rmmod it?


 In general no, but there is a race where this can actually happen.


   Would combining i2c-dev into i2c core and integrating it
   with the core's lock protection make things easier to lock?


 Definitely. That's something I want to do for other reasons anyway.


   You could
   make a compile time option to remove it for small systems. If it's in
   the core is attach/detach adapter still needed?


 i2c-dev will probably soon be the last user of
  i2c_adapter.attach_adapter and i2c_adapter.detach_adapter, and this is
  indeed one of my incentives to look into how things could be rearranged.


   I haven't looked at
   any of this in detail, but i2c-dev is only 6K of code. Half of the 6K
   might disappear if integrated into the core.


 I doubt we can remove 3 kB just by moving things around. The
  book-keeping is relatively small. But this isn't my goal anyway. My
  goal is to make things cleaner and possibly safer.

  One thing we may do if size is a concern, is keep the functional part
  of i2c-dev in its separate module, and only merge the book-keeping and
  char device creation into i2c-core.


   What happens if user space and an in-kernel user both try using the
   device? I've never tried doing that. Should the presence of an
   in-kernel user make the user space device disappear?


 What do you mean by device? The i2c bus, or a specific i2c device
  (client)? The i2c bus can be used by any amount of users, in-kernel or
  user-space. For i2c devices, it's more complex, depending on whether an
  i2c_client is registered or not. Only one i2c_client can be registered
  at a given address, but nothing prevents raw accesses to the devices.
  i2c-dev makes such raw accesses, even though it has an initial check
  whether the i2c address is in used by a registered client or not. This
  needs to be cleaned up from the ground up, the current approach is not
  satisfactory at all and things are getting worse with the advent of
  new-style i2c devices, because the have a different notion of business.


My idea was that when a client registers it would indicate whether or
not it wants a user space device. Most in-kernel clients would not
want a user space device. A sysfs attribute could force the creation
of a user space device for debugging.

You might also want to make a global i2c sysfs attribute that you
would echo an i2c address into. Doing that would cause a user space
device to be created for that address. Now you can manipulate devices
with no in-kernel driver.

Of course we can't stop raw access but life is simpler if everyone
avoids doing it.

--
Jon Smirl
[EMAIL PROTECTED]

___
i2c mailing list
i2c@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/i2c


Re: [i2c] [PATCH] of/i2c: don't pass -1 to irq_dispose_mapping, otherwise kernel will oops

2008-07-12 Thread Jon Smirl
On 7/12/08, Wolfram Sang [EMAIL PROTECTED] wrote:
 On Fri, Jul 11, 2008 at 12:23:23PM -0600, Grant Likely wrote:

  On Fri, Jul 11, 2008 at 09:48:59PM +0400, Anton Vorontsov wrote:
Firstly kernel warns at set_irq_chip, and then dies completely:
   
Trying to install chip for IRQ-1
   
diff --git a/drivers/of/of_i2c.c b/drivers/of/of_i2c.c
index b2ccdcb..95a24de 100644
--- a/drivers/of/of_i2c.c
+++ b/drivers/of/of_i2c.c
@@ -93,10 +93,8 @@ void of_register_i2c_devices(struct i2c_adapter *adap,
if (info.irq == NO_IRQ)
info.irq = -1;
  
   What is the reason that info.irq is set to -1 in the first place?  This
   looks like another bug to me.  Does something in the i2c layer depend on
   the -1 value?
  


 You already acked a fix to this :) I wasn't sure if my patch would
  make it on its own, as Jon Smirl was also working on fixes to of_i2c.c
  and he seemed to pick up this issue, too.

I did another patch for the mpc-i2c driver changing all of the
comparisons to NO_IRQ. My understanding of this is the ppc has NO_IRQ
set to -1, and powerpc has NO_IRQ = 0. So to make all of this work
right you have to use the NO_IRQ symbol and you can't check against 0
or -1 directly. I also believe work is underway to get all platforms
to NO_IRQ = 0 but I don't know if it is completed yet.



  (Original patch is here:
  http://ozlabs.org/pipermail/linuxppc-dev/2008-June/058801.html
  )

  All the best,

Wolfram


  --
   Dipl.-Ing. Wolfram Sang | http://www.pengutronix.de
   Pengutronix - Linux Solutions for Science and Industry

 -BEGIN PGP SIGNATURE-
  Version: GnuPG v1.4.6 (GNU/Linux)

  iD8DBQFIeGSED27XaX1/VRsRAr+EAJ948UwobnY7WSSR4i/ywjof1+8dJACfWzPN
  bhW6NXgBCnwqITIC5rSXeAI=
  =W3sj
  -END PGP SIGNATURE-

 ___
  Linuxppc-dev mailing list
  [EMAIL PROTECTED]
  https://ozlabs.org/mailman/listinfo/linuxppc-dev




-- 
Jon Smirl
[EMAIL PROTECTED]

___
i2c mailing list
i2c@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/i2c


[i2c] IEEE 1275, Device tree standard

2008-07-04 Thread Jon Smirl
This is the IEEE standard that specifies the devices names we are using.
ftp://playground.sun.com/pub/p1275/coredoc/1275-1994/

-- 
Jon Smirl
[EMAIL PROTECTED]

___
i2c mailing list
i2c@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/i2c


Re: [i2c] [PATCH 2/2] Add the of_find_i2c_device_by_node function, V4

2008-07-01 Thread Jon Smirl
On 7/1/08, Jean Delvare [EMAIL PROTECTED] wrote:
 I'm fine with this patch. In particular, exporting i2c_bus_type is OK.
  It was un-exported only because it had no user left, but it can be
  exported again if needed.

Another solution would be to move drivers/of/of_i2c into the i2c
directory and make it part of i2c core on powerpc builds.

  I'm not the one to push this upstream though, as the patch is
  essentially an openfirmware patch. That would be something for Jochen
  Friedrich and Paul Mackerras I guess. Would be nice to have a
  MAINTAINERS entry for OF...

  --

 Jean Delvare



-- 
Jon Smirl
[EMAIL PROTECTED]

___
i2c mailing list
i2c@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/i2c


Re: [i2c] [PATCH 2/2] Add the of_find_i2c_device_by_node function, V4

2008-07-01 Thread Jon Smirl
On 7/1/08, Jean Delvare [EMAIL PROTECTED] wrote:
 Hi Jon,


  On Mon, 30 Jun 2008 19:01:28 -0400, Jon Smirl wrote:
   Add the of_find_i2c_device_by_node function. This allows you to
   follow a reference in the device to an i2c device node and then
   locate the linux device instantiated by the device tree. Example
   use, an i2s codec controlled by i2c.
   ---
  
drivers/i2c/i2c-core.c |2 +-
drivers/of/of_i2c.c|   37 ++---
include/linux/i2c.h|3 +++
include/linux/of_i2c.h |2 ++
4 files changed, 32 insertions(+), 12 deletions(-)
  
   diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
   index d0175f4..e3abe1b 100644
   --- a/drivers/i2c/i2c-core.c
   +++ b/drivers/i2c/i2c-core.c
   @@ -208,7 +208,7 @@ static struct device_attribute i2c_dev_attrs[] = {
 { },
};
  
   -static struct bus_type i2c_bus_type = {
   +struct bus_type i2c_bus_type = {
 .name   = i2c,
 .dev_attrs  = i2c_dev_attrs,
 .match  = i2c_device_match,


 What if i2c-core is built as a module? Don't you need to export the
  symbol then?

Jean, can you re-export i2c_bus_type and then I'll drop that part from
the patch? That way the patch won't hit multiple maintainers.

   diff --git a/drivers/of/of_i2c.c b/drivers/of/of_i2c.c
   index 715a444..ca69a16 100644
   --- a/drivers/of/of_i2c.c
   +++ b/drivers/of/of_i2c.c
   @@ -75,7 +75,7 @@ static int of_find_i2c_driver(struct device_node *node,
void of_register_i2c_devices(struct i2c_adapter *adap,
  struct device_node *adap_node)
{
   - void *result;
   + struct i2c_client *i2c_dev;
 struct device_node *node;
  
 for_each_child_of_node(adap_node, node) {
   @@ -90,29 +90,44 @@ void of_register_i2c_devices(struct i2c_adapter *adap,
 continue;
 }
  
   - info.irq = irq_of_parse_and_map(node, 0);
   - if (info.irq == NO_IRQ)
   - info.irq = -1;
   -
   - if (of_find_i2c_driver(node, info)  0) {
   - irq_dispose_mapping(info.irq);
   + if (of_find_i2c_driver(node, info)  0)
 continue;
   - }
  
   + info.irq = irq_of_parse_and_map(node, 0);
 info.addr = *addr;
  
   - request_module(info.type);
   + request_module(%s, info.type);


 A separate fix for this issue was already sent by Maximilian Attems a
  few days go:
  http://lists.lm-sensors.org/pipermail/i2c/2008-June/004030.html


  
   - result = i2c_new_device(adap, info);
   - if (result == NULL) {
   + i2c_dev = i2c_new_device(adap, info);
   + if (i2c_dev == NULL) {
 printk(KERN_ERR
of-i2c: Failed to load driver for %s\n,
info.type);
 irq_dispose_mapping(info.irq);
 continue;
 }
   +
   + i2c_dev-dev.archdata.of_node = of_node_get(node);
 }
}
EXPORT_SYMBOL(of_register_i2c_devices);
  
   +static int of_dev_node_match(struct device *dev, void *data)
   +{
   +return dev-archdata.of_node == data;
   +}
   +
   +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;
   +
   + return to_i2c_client(dev);
   +}
   +EXPORT_SYMBOL(of_find_i2c_device_by_node);
   +
MODULE_LICENSE(GPL);
   diff --git a/include/linux/i2c.h b/include/linux/i2c.h
   index fb9af6a..186b22d 100644
   --- a/include/linux/i2c.h
   +++ b/include/linux/i2c.h
   @@ -92,6 +92,9 @@ extern s32 i2c_smbus_read_i2c_block_data(struct 
 i2c_client * client,
extern s32 i2c_smbus_write_i2c_block_data(struct i2c_client * client,
   u8 command, u8 length,
   const u8 *values);
   +
   +/* Base of the i2c bus */
   +extern struct bus_type i2c_bus_type;
  
/*
 * A driver is capable of handling one or more physical devices present on
   diff --git a/include/linux/of_i2c.h b/include/linux/of_i2c.h
   index bd2a870..17d5897 100644
   --- a/include/linux/of_i2c.h
   +++ b/include/linux/of_i2c.h
   @@ -16,5 +16,7 @@
  
void of_register_i2c_devices(struct i2c_adapter *adap,
  struct device_node *adap_node);
   +struct i2c_client *of_find_i2c_device_by_node(struct device_node *node);
   +
  
#endif /* __LINUX_OF_I2C_H */


 I'm fine with this patch. In particular, exporting i2c_bus_type is OK.
  It was un-exported only because it had no user left, but it can be
  exported again if needed.

  I'm not the one

Re: [i2c] [PATCH 2/2] Add the of_find_i2c_device_by_node function, V4

2008-07-01 Thread Jon Smirl
On 7/1/08, Jean Delvare [EMAIL PROTECTED] wrote:
 On Tue, 1 Jul 2008 11:12:58 -0400, Jon Smirl wrote:
   On 7/1/08, Jean Delvare [EMAIL PROTECTED] wrote:
I'm fine with this patch. In particular, exporting i2c_bus_type is OK.
 It was un-exported only because it had no user left, but it can be
 exported again if needed.
  
   Another solution would be to move drivers/of/of_i2c into the i2c
   directory and make it part of i2c core on powerpc builds.


 I don't think this is a good idea. Merging arch-specific code (or
  half-arch-specific code in this case) into arch-neutral drivers ends up
  being a pain to maintain. People will keep sending me patches for stuff
  I don't know anything about and can't help with. Having of-specific
  stuff in just one directory as is the case now sounds much better to
  me. All it's missing is a MAINTAINERS entry.

A side effect of this is that the small pieces of code in drivers/of
have to be compiled into stand alone modules and they may need access
to internal symbols from the subsystem. If they were directly linked
into the subsystems you wouldn't need to make the internal symbols
visible. Now the subsystems have to be careful about breaking the
in-kernel, external users of the symbols and we've made it possible
for out of tree drivers to get to internal structures.





  --

 Jean Delvare



-- 
Jon Smirl
[EMAIL PROTECTED]

___
i2c mailing list
i2c@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/i2c


Re: [i2c] [PATCH 2/2] Add the of_find_i2c_device_by_node function, V4

2008-07-01 Thread Jon Smirl
On 7/1/08, Grant Likely [EMAIL PROTECTED] wrote:
 On Tue, Jul 01, 2008 at 11:12:58AM -0400, Jon Smirl wrote:
   On 7/1/08, Jean Delvare [EMAIL PROTECTED] wrote:
I'm fine with this patch. In particular, exporting i2c_bus_type is OK.
 It was un-exported only because it had no user left, but it can be
 exported again if needed.
  
   Another solution would be to move drivers/of/of_i2c into the i2c
   directory and make it part of i2c core on powerpc builds.


 My preference is for things like of_spi and of_i2c to go with the
  related busses; I think it makes more sense to keep all the I2C stuff
  together, but I've already lost that battle once.


This is a similar problem to adding aliases to the i2c driver drivers
for the device tree names of the i2c devices. Instead we have code in
drivers/of/of_i2c.c that tries to guess the translation from device
tree to linux names. Adding aliases to the drivers would eliminate the
need for of_find_i2c_driver().

I've previously posted patches implementing device tree names in the
drivers that used ifdef to only instantiate on powerpc builds. For
example

diff --git a/drivers/i2c/chips/tps65010.c b/drivers/i2c/chips/tps65010.c
index e07274d..9cd1770 100644
--- a/drivers/i2c/chips/tps65010.c
+++ b/drivers/i2c/chips/tps65010.c
@@ -571,6 +571,10 @@ static const struct i2c_device_id tps65010_id[] = {
{ tps65011, TPS65011 },
{ tps65012, TPS65012 },
{ tps65013, TPS65013 },
+   OF_ID(ti,tps65010, TPS65010)
+   OF_ID(ti,tps65011, TPS65011)
+   OF_ID(ti,tps65012, TPS65012)
+   OF_ID(ti,tps65013, TPS65013)
{ },
 };
 MODULE_DEVICE_TABLE(i2c, tps65010_id);


-- 
Jon Smirl
[EMAIL PROTECTED]

___
i2c mailing list
i2c@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/i2c


Re: [i2c] [PATCH 2/2] Add the of_find_i2c_device_by_node function, V4

2008-07-01 Thread Jon Smirl
On 7/1/08, Jean Delvare [EMAIL PROTECTED] wrote:
 On Tue, 1 Jul 2008 10:45:18 -0600, Grant Likely wrote:
   On Tue, Jul 01, 2008 at 06:29:49PM +0200, Jean Delvare wrote:
On Tue, 1 Jul 2008 11:12:58 -0400, Jon Smirl wrote:
 On 7/1/08, Jean Delvare [EMAIL PROTECTED] wrote:
  I'm fine with this patch. In particular, exporting i2c_bus_type is 
 OK.
   It was un-exported only because it had no user left, but it can be
   exported again if needed.

 Another solution would be to move drivers/of/of_i2c into the i2c
 directory and make it part of i2c core on powerpc builds.
   
I don't think this is a good idea. Merging arch-specific code (or
half-arch-specific code in this case) into arch-neutral drivers ends up
being a pain to maintain. People will keep sending me patches for stuff
I don't know anything about and can't help with. Having of-specific
stuff in just one directory as is the case now sounds much better to
me. All it's missing is a MAINTAINERS entry.
  
   But the other side of the coin is that each driver must have
   driver-specific OF code to translate data in the device tree to data
   usable by the driver.  It doesn't make any sense to me for that stuff to
   live anywhere other that with the driver that it supports.


 This code is glue between OF and subsystems. As with any glue code, you
  can argue forever on which side you want to push the code to. Both
  answers are valid.

  All I see on my personal side is that I don't have any system using OF
  and no knowledge about it either, so I can't maintain of_i2c. So having
  that file in drivers/of rather than drivers/i2c will make my life
  easier for sure. While I'd guess that most (all?) OF-based systems have
  an I2C bus, so whoever is responsible for drivers/of should be able to
  maintain of_i2c.

We could modify the Makefile for i2c core to get the source out of
drivers/of and link it into i2c-core. That would remove the need to
export symbols.

Or you could move the file into the i2c directory and just put a note
on it that Grant is the maintainer.



  --

 Jean Delvare



-- 
Jon Smirl
[EMAIL PROTECTED]

___
i2c mailing list
i2c@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/i2c


Re: [i2c] [PATCH 2/2] Add the of_find_i2c_device_by_node function, V4

2008-07-01 Thread Jon Smirl
On 7/1/08, Jean Delvare [EMAIL PROTECTED] wrote:
 On Tue, 1 Jul 2008 13:00:08 -0400, Jon Smirl wrote:
   On 7/1/08, Grant Likely [EMAIL PROTECTED] wrote:

   My preference is for things like of_spi and of_i2c to go with the
 related busses; I think it makes more sense to keep all the I2C stuff
 together, but I've already lost that battle once.
  
   This is a similar problem to adding aliases to the i2c driver drivers
   for the device tree names of the i2c devices. Instead we have code in
   drivers/of/of_i2c.c that tries to guess the translation from device
   tree to linux names. Adding aliases to the drivers would eliminate the
   need for of_find_i2c_driver().
  
   I've previously posted patches implementing device tree names in the
   drivers that used ifdef to only instantiate on powerpc builds. For
   example
  
   diff --git a/drivers/i2c/chips/tps65010.c b/drivers/i2c/chips/tps65010.c
   index e07274d..9cd1770 100644
   --- a/drivers/i2c/chips/tps65010.c
   +++ b/drivers/i2c/chips/tps65010.c
   @@ -571,6 +571,10 @@ static const struct i2c_device_id tps65010_id[] = {
   { tps65011, TPS65011 },
   { tps65012, TPS65012 },
   { tps65013, TPS65013 },
   +   OF_ID(ti,tps65010, TPS65010)
   +   OF_ID(ti,tps65011, TPS65011)
   +   OF_ID(ti,tps65012, TPS65012)
   +   OF_ID(ti,tps65013, TPS65013)
   { },
};
MODULE_DEVICE_TABLE(i2c, tps65010_id);


 Yeah, yeah, you've been asking for this for months already, but it's
  just not going to happen, sorry. You want to abuse the standard Linux
  alias mechanism for your personal (i.e. openfirmware) use, but that's
  bad. Linux drivers shouldn't have to know whether they are used in
  openfirmware trees and what device names are used there. And device
  names as seen by user-space shouldn't vary depending on whether the
  device comes from an openfirmware tree or not - otherwise all
  user-space apps need to learn about both naming conversions.

  Unsurprisingly, no other subsystem does what you propose.

Then what are all of the PCI aliases doing?

The only difference is that you are recognizing the PCI group as a
naming authority and not recognizing the PowerPC device tree group.
But on the PowerPC platform that is our naming authority. That's why I
proposed adding the names on ifdefs so that they disappear on non
PowerPC platforms.

PS - adding an alias to a driver does not change the name of the
driver. My PCI e1000 module has about 100 aliases but it is always
e1000.

-- 
Jon Smirl
[EMAIL PROTECTED]

___
i2c mailing list
i2c@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/i2c


Re: [i2c] [PATCH 2/2] Add the of_find_i2c_device_by_node function, V4

2008-07-01 Thread Jon Smirl
On 7/1/08, Jon Smirl [EMAIL PROTECTED] wrote:
 On 7/1/08, Jean Delvare [EMAIL PROTECTED] wrote:

  On Tue, 1 Jul 2008 13:00:08 -0400, Jon Smirl wrote:
 On 7/1/08, Grant Likely [EMAIL PROTECTED] wrote:
  
 My preference is for things like of_spi and of_i2c to go with the
   related busses; I think it makes more sense to keep all the I2C stuff
   together, but I've already lost that battle once.

 This is a similar problem to adding aliases to the i2c driver drivers
 for the device tree names of the i2c devices. Instead we have code in
 drivers/of/of_i2c.c that tries to guess the translation from device
 tree to linux names. Adding aliases to the drivers would eliminate the
 need for of_find_i2c_driver().

 I've previously posted patches implementing device tree names in the
 drivers that used ifdef to only instantiate on powerpc builds. For
 example

 diff --git a/drivers/i2c/chips/tps65010.c b/drivers/i2c/chips/tps65010.c
 index e07274d..9cd1770 100644
 --- a/drivers/i2c/chips/tps65010.c
 +++ b/drivers/i2c/chips/tps65010.c
 @@ -571,6 +571,10 @@ static const struct i2c_device_id tps65010_id[] = {
 { tps65011, TPS65011 },
 { tps65012, TPS65012 },
 { tps65013, TPS65013 },
 +   OF_ID(ti,tps65010, TPS65010)
 +   OF_ID(ti,tps65011, TPS65011)
 +   OF_ID(ti,tps65012, TPS65012)
 +   OF_ID(ti,tps65013, TPS65013)
 { },
  };
  MODULE_DEVICE_TABLE(i2c, tps65010_id);
  
  
   Yeah, yeah, you've been asking for this for months already, but it's
just not going to happen, sorry. You want to abuse the standard Linux
alias mechanism for your personal (i.e. openfirmware) use, but that's
bad. Linux drivers shouldn't have to know whether they are used in
openfirmware trees and what device names are used there. And device
names as seen by user-space shouldn't vary depending on whether the
device comes from an openfirmware tree or not - otherwise all
user-space apps need to learn about both naming conversions.
  
Unsurprisingly, no other subsystem does what you propose.


 Then what are all of the PCI aliases doing?

  The only difference is that you are recognizing the PCI group as a
  naming authority and not recognizing the PowerPC device tree group.
  But on the PowerPC platform that is our naming authority. That's why I
  proposed adding the names on ifdefs so that they disappear on non
  PowerPC platforms.

  PS - adding an alias to a driver does not change the name of the
  driver. My PCI e1000 module has about 100 aliases but it is always
  e1000.

Here's my e1000e sysfs entry:

[EMAIL PROTECTED]:/sys/bus/pci/devices/:00:19.0$ ls
broken_parity_status  device  local_cpus  power  resource2 uevent
bus   driver  modaliasresource   subsystem vendor
class enable  msi_bus resource0  subsystem_device
configirq net:eth0resource1  subsystem_vendor

[EMAIL PROTECTED]:/sys/bus/pci/devices/:00:19.0$ cat modalias
pci:v8086d104Bsv1028sd01DBbc02sc00i00

  This is the module alias that was used to load the driver.

[EMAIL PROTECTED]:/sys/bus/pci/devices/:00:19.0$ ls -l driver
lrwxrwxrwx 1 root root 0 2008-07-01 08:52 driver -
../../../bus/pci/drivers/e1000e

  The driver is always e1000e no matter which alias was used to load it. 
 e1000e  is controled by the name field of the driver structure.  That's 
 the publicly visible name for the driver.

  Adding the OF aliases would change the modalias entry, not the driver 
 name.

The i2c implementation is adding a field to a device entry that
contains the driver name. No other device drivers I could find do
this.

[EMAIL PROTECTED]:/sys/bus/i2c/devices/1-0050$ ls
bus  driver  eeprom  modalias  name  power  subsystem  uevent
[EMAIL PROTECTED]:/sys/bus/i2c/devices/1-0050$ cat name
eeprom
[EMAIL PROTECTED]:/sys/bus/i2c/devices/1-0050$ ls -l driver
lrwxrwxrwx 1 root root 0 2008-07-01 14:05 driver -
../../../../bus/i2c/drivers/eeprom
[EMAIL PROTECTED]:/sys/bus/i2c/devices/1-0050$ cat modalias

[EMAIL PROTECTED]:/sys/bus/i2c/devices/1-0050$

I believe the correct way to get the driver name from sysfs is to
follow the driver link. The name field is probably legacy.  Other
drivers in the system don't have a name entry on the device node.

Is the user space i2c code looking at the modalias entry?

-- 
Jon Smirl
[EMAIL PROTECTED]

___
i2c mailing list
i2c@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/i2c


[i2c] [PATCH] Export the i2c_bus_type symbol, standalone V1

2008-07-01 Thread Jon Smirl
Export the root of the i2c bus so that PowerPC device tree code can iterate 
over devices on the i2c bus.

Signed-off-by: Jon Smirl [EMAIL PROTECTED]
---

 drivers/i2c/i2c-core.c |3 ++-
 include/linux/i2c.h|3 +++
 2 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index d0175f4..05866ef 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -208,7 +208,7 @@ static struct device_attribute i2c_dev_attrs[] = {
{ },
 };
 
-static struct bus_type i2c_bus_type = {
+struct bus_type i2c_bus_type = {
.name   = i2c,
.dev_attrs  = i2c_dev_attrs,
.match  = i2c_device_match,
@@ -219,6 +219,7 @@ static struct bus_type i2c_bus_type = {
.suspend= i2c_device_suspend,
.resume = i2c_device_resume,
 };
+EXPORT_SYMBOL_GPL(i2c_bus_type);
 
 
 /**
diff --git a/include/linux/i2c.h b/include/linux/i2c.h
index fb9af6a..186b22d 100644
--- a/include/linux/i2c.h
+++ b/include/linux/i2c.h
@@ -92,6 +92,9 @@ extern s32 i2c_smbus_read_i2c_block_data(struct i2c_client * 
client,
 extern s32 i2c_smbus_write_i2c_block_data(struct i2c_client * client,
  u8 command, u8 length,
  const u8 *values);
+ 
+/* Base of the i2c bus */
+extern struct bus_type i2c_bus_type; 
 
 /*
  * A driver is capable of handling one or more physical devices present on


___
i2c mailing list
i2c@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/i2c


[i2c] Aliases and device names for i2c

2008-07-01 Thread Jon Smirl
Is the problem is stemming from having the i2c drivers support
multiple similar but not exactly the same chips?  These similar chips
are being exposed as aliases which they really aren't; they are
actually different chips.

A solution to this is for the drivers to register themselves multiple
times, one for each different chip. Doing that frees up aliases for
true aliases which is what device trees need.

That also resolve the issue of needing a name attribute in the sysfs
device node. We shouldn't need that attribute. By registering multiple
times, the link from the device to the driver will contain the correct
chip id. Of course we can still make a legacy name attribute until
user space is converted.

With this structure modalias will contain the alias used to load the driver.
The driver link will have pfc856?.name.name
pfc856?.name.name can also be use to create the name attribute.

static const struct i2c_device_id pcf8563_id[] = {
{ pcf8563, 0 },
{ samsung,pcf8563, 0 },
{ }
};
MODULE_DEVICE_TABLE(i2c, pcf8563_id);

static struct i2c_driver pcf8563_driver = {
.driver = {
.name   = pcf8563,
},
.probe  = pcf8563_probe,
.remove = pcf8563_remove,
.id_table   = pcf8563_id,
};

static int __init pcf8563_init(void)
{
return i2c_add_driver(pcf8563_driver);
}

static void __exit pcf8563_exit(void)
{
i2c_del_driver(pcf8563_driver);
}

static const struct i2c_device_id pcf8563_id[] = {
{ rtc8564, 0 },
{ dallas,rtc8564, 0 },
{ }
};
MODULE_DEVICE_TABLE(i2c, pcf8563_id);

static struct i2c_driver pcf8563_driver = {
.driver = {
.name   = pcf8564,
},
.probe  = pcf8563_probe,
.remove = pcf8563_remove,
.id_table   = pcf8564_id,
};

static int __init pcf8564_init(void)
{
return i2c_add_driver(pcf8564_driver);
}

static void __exit pcf8564_exit(void)
{
i2c_del_driver(pcf8564_driver);
}

-- 
Jon Smirl
[EMAIL PROTECTED]

___
i2c mailing list
i2c@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/i2c


[i2c] [PATCH] Convert i2c-mpc from a platform driver into a of_platform driver, V2

2008-06-29 Thread Jon Smirl
This version adds of_find_i2c_device_by_node() which is needed by ASOC drivers. 
To make it work I had to make the i2c bus struct visible. It also picks up a 
couple of other minor fixes that have been posted to the lists.

Signed-off-by: Jon Smirl [EMAIL PROTECTED]
---

 arch/powerpc/sysdev/fsl_soc.c |  122 -
 drivers/i2c/busses/i2c-mpc.c  |  104 ---
 drivers/i2c/i2c-core.c|2 -
 drivers/of/of_i2c.c   |   37 +---
 include/linux/i2c.h   |3 +
 include/linux/of_i2c.h|2 +
 6 files changed, 92 insertions(+), 178 deletions(-)

diff --git a/arch/powerpc/sysdev/fsl_soc.c b/arch/powerpc/sysdev/fsl_soc.c
index 019657c..ebcec73 100644
--- a/arch/powerpc/sysdev/fsl_soc.c
+++ b/arch/powerpc/sysdev/fsl_soc.c
@@ -414,128 +414,6 @@ err:
 
 arch_initcall(gfar_of_init);
 
-#ifdef CONFIG_I2C_BOARDINFO
-#include linux/i2c.h
-struct i2c_driver_device {
-   char*of_device;
-   char*i2c_type;
-};
-
-static struct i2c_driver_device i2c_devices[] __initdata = {
-   {ricoh,rs5c372a, rs5c372a},
-   {ricoh,rs5c372b, rs5c372b},
-   {ricoh,rv5c386,  rv5c386},
-   {ricoh,rv5c387a, rv5c387a},
-   {dallas,ds1307,  ds1307},
-   {dallas,ds1337,  ds1337},
-   {dallas,ds1338,  ds1338},
-   {dallas,ds1339,  ds1339},
-   {dallas,ds1340,  ds1340},
-   {stm,m41t00, m41t00},
-   {dallas,ds1374,  ds1374},
-};
-
-static int __init of_find_i2c_driver(struct device_node *node,
-struct i2c_board_info *info)
-{
-   int i;
-
-   for (i = 0; i  ARRAY_SIZE(i2c_devices); i++) {
-   if (!of_device_is_compatible(node, i2c_devices[i].of_device))
-   continue;
-   if (strlcpy(info-type, i2c_devices[i].i2c_type,
-   I2C_NAME_SIZE) = I2C_NAME_SIZE)
-   return -ENOMEM;
-   return 0;
-   }
-   return -ENODEV;
-}
-
-static void __init of_register_i2c_devices(struct device_node *adap_node,
-  int bus_num)
-{
-   struct device_node *node = NULL;
-
-   while ((node = of_get_next_child(adap_node, node))) {
-   struct i2c_board_info info = {};
-   const u32 *addr;
-   int len;
-
-   addr = of_get_property(node, reg, len);
-   if (!addr || len  sizeof(int) || *addr  (1  10) - 1) {
-   printk(KERN_WARNING fsl_soc.c: invalid i2c device 
entry\n);
-   continue;
-   }
-
-   info.irq = irq_of_parse_and_map(node, 0);
-   if (info.irq == NO_IRQ)
-   info.irq = -1;
-
-   if (of_find_i2c_driver(node, info)  0)
-   continue;
-
-   info.addr = *addr;
-
-   i2c_register_board_info(bus_num, info, 1);
-   }
-}
-
-static int __init fsl_i2c_of_init(void)
-{
-   struct device_node *np;
-   unsigned int i = 0;
-   struct platform_device *i2c_dev;
-   int ret;
-
-   for_each_compatible_node(np, NULL, fsl-i2c) {
-   struct resource r[2];
-   struct fsl_i2c_platform_data i2c_data;
-   const unsigned char *flags = NULL;
-
-   memset(r, 0, sizeof(r));
-   memset(i2c_data, 0, sizeof(i2c_data));
-
-   ret = of_address_to_resource(np, 0, r[0]);
-   if (ret)
-   goto err;
-
-   of_irq_to_resource(np, 0, r[1]);
-
-   i2c_dev = platform_device_register_simple(fsl-i2c, i, r, 2);
-   if (IS_ERR(i2c_dev)) {
-   ret = PTR_ERR(i2c_dev);
-   goto err;
-   }
-
-   i2c_data.device_flags = 0;
-   flags = of_get_property(np, dfsrr, NULL);
-   if (flags)
-   i2c_data.device_flags |= FSL_I2C_DEV_SEPARATE_DFSRR;
-
-   flags = of_get_property(np, fsl5200-clocking, NULL);
-   if (flags)
-   i2c_data.device_flags |= FSL_I2C_DEV_CLOCK_5200;
-
-   ret =
-   platform_device_add_data(i2c_dev, i2c_data,
-sizeof(struct
-   fsl_i2c_platform_data));
-   if (ret)
-   goto unreg;
-
-   of_register_i2c_devices(np, i++);
-   }
-
-   return 0;
-
-unreg:
-   platform_device_unregister(i2c_dev);
-err:
-   return ret;
-}
-
-arch_initcall(fsl_i2c_of_init);
-#endif
 
 #ifdef CONFIG_PPC_83xx
 static int __init mpc83xx_wdt_init(void)
diff --git a/drivers/i2c/busses/i2c-mpc.c b/drivers/i2c/busses/i2c-mpc.c
index a076129..36bea49 100644
--- a/drivers/i2c/busses/i2c-mpc.c
+++ b/drivers/i2c/busses/i2c-mpc.c
@@ -17,7 +17,8 @@
 #include linux/module.h
 #include

[i2c] FInding the right hwmon driver on x86

2008-06-12 Thread Jon Smirl
Does ACPI or something like that have a field containing the name of
the hardware monitor chip? If so, we can do the same thing the powerpc
code is doing and dynamically load the right module.

-- 
Jon Smirl
[EMAIL PROTECTED]

___
i2c mailing list
i2c@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/i2c


[i2c] Fwd: rtc pfc8563 driver lost a chip id

2008-06-11 Thread Jon Smirl
I think this got dropped during the conversion to the new probe signature.

From the Kconfig...

config RTC_DRV_PCF8563
tristate Philips PCF8563/Epson RTC8564
help
  If you say yes here you get support for the
  Philips PCF8563 RTC chip. The Epson RTC8564
  should work as well.

  This driver can also be built as a module. If so, the module
  will be called rtc-pcf8563.



-- Forwarded message --
From: Jean Delvare [EMAIL PROTECTED]
Date: Jun 11, 2008 3:04 AM
Subject: Re: [i2c] rtc pfc8563 driver lost a chip id
To: Jon Smirl [EMAIL PROTECTED]
Cc: Linux I2C i2c@lm-sensors.org


Hi Jon,


 On Tue, 10 Jun 2008 18:22:50 -0400, Jon Smirl wrote:
  It does the rtc8564 too.
 
  diff --git a/drivers/rtc/rtc-pcf8563.c b/drivers/rtc/rtc-pcf8563.c
  index 0fc4c36..748a502 100644
  --- a/drivers/rtc/rtc-pcf8563.c
  +++ b/drivers/rtc/rtc-pcf8563.c
  @@ -302,6 +302,7 @@ static int pcf8563_remove(struct i2c_client *client)
 
   static const struct i2c_device_id pcf8563_id[] = {
  { pcf8563, 0 },
  +   { rtc8564, 0 },
  { }
   };
   MODULE_DEVICE_TABLE(i2c, pcf8563_id);


Please check MAINTAINERS for the right person and list to send this
 patch to.

 --

Jean Delvare


-- 
Jon Smirl
[EMAIL PROTECTED]

___
i2c mailing list
i2c@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/i2c


Re: [i2c] [PATCH] Convert i2c-mpc from a platform driver to an of_platform one

2008-06-11 Thread Jon Smirl
On 6/11/08, Wolfram Sang [EMAIL PROTECTED] wrote:
 On Tue, Jun 10, 2008 at 10:40:45PM -0400, Jon Smirl wrote:
   Convert i2c-mpc from a platform driver into an of_platform driver.
   This patch is much smaller since Jochen already added
   of_find_i2c_driver(). Versions of this have been posted before.
  
   Signed-ff-by: Jon Smirl [EMAIL PROTECTED]


 Typo: Signed-off... (I'm curious, do such typos enforce resending the
  patch?)


I just cut and pasted this version to get the comments. Next pass I
will send it using stgit which will add the right signed-off line and
fix the wrapping.



  Tested-by: Wolfram Sang [EMAIL PROTECTED]


  
   --
  
drivers/i2c/busses/i2c-mpc.c |  105 
 +-
1 files changed, 63 insertions(+), 42 deletions(-)
  
  
   diff --git a/drivers/i2c/busses/i2c-mpc.c b/drivers/i2c/busses/i2c-mpc.c
   index a076129..76d8091 100644
   --- a/drivers/i2c/busses/i2c-mpc.c
   +++ b/drivers/i2c/busses/i2c-mpc.c
   @@ -18,6 +18,8 @@
#include linux/sched.h
#include linux/init.h
#include linux/platform_device.h
   +#include linux/of_platform.h
   +#include linux/of_i2c.h
  
#include asm/io.h
#include linux/fsl_devices.h
   @@ -25,13 +27,13 @@
#include linux/interrupt.h
#include linux/delay.h
  
   -#define MPC_I2C_ADDR  0x00
   +#define DRV_NAME mpc-i2c
   +
#define MPC_I2C_FDR  0x04
#define MPC_I2C_CR   0x08
#define MPC_I2C_SR   0x0c
#define MPC_I2C_DR   0x10
#define MPC_I2C_DFSRR 0x14
   -#define MPC_I2C_REGION 0x20
  
#define CCR_MEN  0x80
#define CCR_MIEN 0x40
   @@ -315,102 +317,121 @@ static struct i2c_adapter mpc_ops = {
 .timeout = 1,
};
  
   -static int fsl_i2c_probe(struct platform_device *pdev)
   +static int fsl_i2c_probe(struct of_device *op, const struct
   of_device_id *match)


 The above two lines have to be concatenated, otherwise the patch will
  not apply. Also, __devinit?


{
 int result = 0;
 struct mpc_i2c *i2c;
   - struct fsl_i2c_platform_data *pdata;
   - struct resource *r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
   -
   - pdata = (struct fsl_i2c_platform_data *) pdev-dev.platform_data;
  
 i2c = kzalloc(sizeof(*i2c), GFP_KERNEL);
 if (!i2c)
 return -ENOMEM;
  
   - i2c-irq = platform_get_irq(pdev, 0);
   - if (i2c-irq  0)
   - i2c-irq = NO_IRQ; /* Use polling */
   + if (of_get_property(op-node, dfsrr, NULL))
   + i2c-flags |= FSL_I2C_DEV_SEPARATE_DFSRR;
  
   - i2c-flags = pdata-device_flags;
   - init_waitqueue_head(i2c-queue);
   + if (of_device_is_compatible(op-node, mpc5200-i2c))
   + i2c-flags |= FSL_I2C_DEV_CLOCK_5200;
  
   - i2c-base = ioremap((phys_addr_t)r-start, MPC_I2C_REGION);
   + init_waitqueue_head(i2c-queue);
  
   + i2c-base = of_iomap(op-node, 0);
 if (!i2c-base) {
 printk(KERN_ERR i2c-mpc - failed to map controller\n);
 result = -ENOMEM;
 goto fail_map;
 }
  
   - if (i2c-irq != NO_IRQ)
   - if ((result = request_irq(i2c-irq, mpc_i2c_isr,
   -   IRQF_SHARED, i2c-mpc, i2c))  0) 
 {
   - printk(KERN_ERR
   -i2c-mpc - failed to attach interrupt\n);
   - goto fail_irq;
   - }
   + i2c-irq = irq_of_parse_and_map(op-node, 0);
   + if (i2c-irq == NO_IRQ) {
   + result = -ENXIO;


 Minor thing, but I wonder if -EINVAL is more appropriate?


   + goto fail_irq;
   + }
   +
   + result = request_irq(i2c-irq, mpc_i2c_isr,
   + IRQF_SHARED, i2c-mpc, i2c);
   + if (result  0) {
   + printk(KERN_ERR i2c-mpc - failed to attach interrupt\n);
   + goto fail_request;
   + }
  
 mpc_i2c_setclock(i2c);
   - platform_set_drvdata(pdev, i2c);
   +
   + dev_set_drvdata(op-dev, i2c);
  
 i2c-adap = mpc_ops;
   - i2c-adap.nr = pdev-id;
 i2c_set_adapdata(i2c-adap, i2c);
   - i2c-adap.dev.parent = pdev-dev;
   - if ((result = i2c_add_numbered_adapter(i2c-adap))  0) {
   + i2c-adap.dev.parent = op-dev;
   +
   + result = i2c_add_adapter(i2c-adap);
   + if (result  0) {
 printk(KERN_ERR i2c-mpc - failed to add adapter\n);
 goto fail_add;
 }
   + of_register_i2c_devices(i2c-adap, op-node);
  
 return result;
  
   -  fail_add:
   - if (i2c-irq != NO_IRQ)
   - free_irq(i2c-irq, i2c);
   -  fail_irq:
   + fail_add:
   + dev_set_drvdata(op-dev, NULL);
   + free_irq(i2c-irq, i2c);
   + fail_request:
   + irq_dispose_mapping(i2c-irq);
   + fail_irq:
 iounmap(i2c-base);
   -  fail_map:
   + fail_map:
 kfree(i2c);
 return result;
};
  
   -static int

[i2c] [PATCH] Convert i2c-mpc from a platform driver to an of_platform one

2008-06-10 Thread Jon Smirl
Convert i2c-mpc from a platform driver into an of_platform driver.
This patch is much smaller since Jochen already added
of_find_i2c_driver(). Versions of this have been posted before.

Signed-ff-by: Jon Smirl [EMAIL PROTECTED]

-- 

 drivers/i2c/busses/i2c-mpc.c |  105 +-
 1 files changed, 63 insertions(+), 42 deletions(-)


diff --git a/drivers/i2c/busses/i2c-mpc.c b/drivers/i2c/busses/i2c-mpc.c
index a076129..76d8091 100644
--- a/drivers/i2c/busses/i2c-mpc.c
+++ b/drivers/i2c/busses/i2c-mpc.c
@@ -18,6 +18,8 @@
 #include linux/sched.h
 #include linux/init.h
 #include linux/platform_device.h
+#include linux/of_platform.h
+#include linux/of_i2c.h

 #include asm/io.h
 #include linux/fsl_devices.h
@@ -25,13 +27,13 @@
 #include linux/interrupt.h
 #include linux/delay.h

-#define MPC_I2C_ADDR  0x00
+#define DRV_NAME mpc-i2c
+
 #define MPC_I2C_FDR0x04
 #define MPC_I2C_CR 0x08
 #define MPC_I2C_SR 0x0c
 #define MPC_I2C_DR 0x10
 #define MPC_I2C_DFSRR 0x14
-#define MPC_I2C_REGION 0x20

 #define CCR_MEN  0x80
 #define CCR_MIEN 0x40
@@ -315,102 +317,121 @@ static struct i2c_adapter mpc_ops = {
.timeout = 1,
 };

-static int fsl_i2c_probe(struct platform_device *pdev)
+static int fsl_i2c_probe(struct of_device *op, const struct
of_device_id *match)
 {
int result = 0;
struct mpc_i2c *i2c;
-   struct fsl_i2c_platform_data *pdata;
-   struct resource *r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-
-   pdata = (struct fsl_i2c_platform_data *) pdev-dev.platform_data;

i2c = kzalloc(sizeof(*i2c), GFP_KERNEL);
if (!i2c)
return -ENOMEM;

-   i2c-irq = platform_get_irq(pdev, 0);
-   if (i2c-irq  0)
-   i2c-irq = NO_IRQ; /* Use polling */
+   if (of_get_property(op-node, dfsrr, NULL))
+   i2c-flags |= FSL_I2C_DEV_SEPARATE_DFSRR;

-   i2c-flags = pdata-device_flags;
-   init_waitqueue_head(i2c-queue);
+   if (of_device_is_compatible(op-node, mpc5200-i2c))
+   i2c-flags |= FSL_I2C_DEV_CLOCK_5200;

-   i2c-base = ioremap((phys_addr_t)r-start, MPC_I2C_REGION);
+   init_waitqueue_head(i2c-queue);

+   i2c-base = of_iomap(op-node, 0);
if (!i2c-base) {
printk(KERN_ERR i2c-mpc - failed to map controller\n);
result = -ENOMEM;
goto fail_map;
}

-   if (i2c-irq != NO_IRQ)
-   if ((result = request_irq(i2c-irq, mpc_i2c_isr,
- IRQF_SHARED, i2c-mpc, i2c))  0) {
-   printk(KERN_ERR
-  i2c-mpc - failed to attach interrupt\n);
-   goto fail_irq;
-   }
+   i2c-irq = irq_of_parse_and_map(op-node, 0);
+   if (i2c-irq == NO_IRQ) {
+   result = -ENXIO;
+   goto fail_irq;
+   }
+
+   result = request_irq(i2c-irq, mpc_i2c_isr,
+   IRQF_SHARED, i2c-mpc, i2c);
+   if (result  0) {
+   printk(KERN_ERR i2c-mpc - failed to attach interrupt\n);
+   goto fail_request;
+   }

mpc_i2c_setclock(i2c);
-   platform_set_drvdata(pdev, i2c);
+
+   dev_set_drvdata(op-dev, i2c);

i2c-adap = mpc_ops;
-   i2c-adap.nr = pdev-id;
i2c_set_adapdata(i2c-adap, i2c);
-   i2c-adap.dev.parent = pdev-dev;
-   if ((result = i2c_add_numbered_adapter(i2c-adap))  0) {
+   i2c-adap.dev.parent = op-dev;
+
+   result = i2c_add_adapter(i2c-adap);
+   if (result  0) {
printk(KERN_ERR i2c-mpc - failed to add adapter\n);
goto fail_add;
}
+   of_register_i2c_devices(i2c-adap, op-node);

return result;

-  fail_add:
-   if (i2c-irq != NO_IRQ)
-   free_irq(i2c-irq, i2c);
-  fail_irq:
+ fail_add:
+   dev_set_drvdata(op-dev, NULL);
+   free_irq(i2c-irq, i2c);
+ fail_request:
+   irq_dispose_mapping(i2c-irq);
+ fail_irq:
iounmap(i2c-base);
-  fail_map:
+ fail_map:
kfree(i2c);
return result;
 };

-static int fsl_i2c_remove(struct platform_device *pdev)
+static int fsl_i2c_remove(struct of_device *op)
 {
-   struct mpc_i2c *i2c = platform_get_drvdata(pdev);
+   struct mpc_i2c *i2c = dev_get_drvdata(op-dev);

i2c_del_adapter(i2c-adap);
-   platform_set_drvdata(pdev, NULL);
+   dev_set_drvdata(op-dev, NULL);

if (i2c-irq != NO_IRQ)
free_irq(i2c-irq, i2c);

+   irq_dispose_mapping(i2c-irq);
iounmap(i2c-base);
kfree(i2c);
return 0;
 };

-/* work with hotplug and coldplug */
-MODULE_ALIAS(platform:fsl-i2c);
+static const struct of_device_id mpc_i2c_of_match[] = {
+   {
+   .compatible = fsl-i2c,
+   },
+   {},
+};
+MODULE_DEVICE_TABLE(of, mpc_i2c_of_match);
+

 /* Structure for a device driver */
-static struct

Re: [i2c] [PATCH 0/4] i2c: Introduce i2c listeners

2008-06-05 Thread Jon Smirl
On 6/5/08, Jean Delvare [EMAIL PROTECTED] wrote:
   Doesn't something like this work? (obviously this isn't complete)
  
   for (address = start; address = stop;) {
 make a new client
 for (; address =stop; address++) {
 client-address = address;
 res = device_register(client-dev);
 if (res != -ENODEV)
 break;
 }
   }
   delete the extra client


 You don't seriously propose to scan all addresses on every I2C bus for
  every i2c chip driver that we load, do you? This would be awfully slow
  and dangerous. No way.

This is just a small part of the code, you need to add in all the
other parts from the patch.
Exactly the same addresses would be scanned  under each scheme. .My
question is, does the driver need two entry points (detect and probe)
or could only one (probe) be used? I think detect and probe can be the
same function.

With the detect() function it would look somthing like this:

for (address = start; address = stop;) {
  for (; address =stop; address++) {
 do a generic detect
  if (driver-detect(address) {
 make a new client
 client-address = address;
 res = device_register(client-dev);
 if (!res)
delete the extra client
 }
   }
}





-- 
Jon Smirl
[EMAIL PROTECTED]

___
i2c mailing list
i2c@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/i2c


Re: [i2c] [PATCH 0/4] i2c: Introduce i2c listeners

2008-06-05 Thread Jon Smirl
On 6/5/08, David Brownell [EMAIL PROTECTED] wrote:
 On Wednesday 04 June 2008, Jon Smirl wrote:

  Now that we've had this discussion it looks to me that the scan
   capability should be added to struct i2c_driver instead of being
   standalone.
  
   I guess what is a little strange to me is that the pre-probe()
   function is like a static driver function and probe() is an instance
   function.


 The problem there is:  how do you know which pre-probe()
  function(s) to try?  If you could answer that reliably
  you'd be able to probe() directly.  If you can't, you'd
  need to load all I2C driver modules until one works...
  not the desired system model.


I see now that the adapter class should be added to the driver name
alias string. The we could load all modules of a given class.

But I don't think we need to go searching for the right module to load
since i2c is not generally a dynamic bus. If you install a video card
with an i2c bus, the driver for the video card can just load_module
the right i2c driver modules.

I do wonder if we could get hardware monitors to autoload. When the
adapter driver for the motherboard loads, it could load module all
drivers of class hardware_monitor. Then each one could do its
detection thing. Unload the ones that weren't found. That would really
help people who can figure out the right driver module to load. You
could even get fancy and generate a uevent after the right module was
found. A script in the uevent could save the info so you don't have to
repeat on each boot. Of course this assume that the hardware monitor
modules can reliably autodetect.


-- 
Jon Smirl
[EMAIL PROTECTED]

___
i2c mailing list
i2c@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/i2c


Re: [i2c] [PATCH 0/4] i2c: Introduce i2c listeners

2008-06-05 Thread Jon Smirl
On 6/5/08, Jean Delvare [EMAIL PROTECTED] wrote:
 On Thu, 5 Jun 2008 10:55:28 -0400, Jon Smirl wrote:
   I see now that the adapter class should be added to the driver name
   alias string. The we could load all modules of a given class.


 Ah ah. Oh wait, you were serious? Come on, why would anyone on earth
  want to load all modules of a given class?


   But I don't think we need to go searching for the right module to load
   since i2c is not generally a dynamic bus. If you install a video card
   with an i2c bus, the driver for the video card can just load_module
   the right i2c driver modules.


 Sometimes yes, sometimes no. For most V4L/DVB, the main driver should
  know which i2c chip drivers it needs, but unfortunately for old drivers
  such as bttv this knowledge has never been recorded, so there is no
  alternative to (at least partial) probing in practice. For video
  adapters, they could load the eeprom driver as they all give access to
  EDID EEPROMs, but if the card also has a hardware monitoring chip, we
  simply don't know what it is. We just can't record in the kernel
  drivers what hardware monitoring chip exists on thousands of cards out
  there (same problem as for motherboards.)


   I do wonder if we could get hardware monitors to autoload.


 Only if they don't do the device detection/identification themselves.
  I've considered it for a moment but now I'm fairly certain that it
  would be a dangerous and unmaintainable mess. Better let each driver
  include its detection/identification routine, and delegate the
  responsibility of loading the right driver to user-space (as is already
  the case.)


   When the
   adapter driver for the motherboard loads, it could load module all
   drivers of class hardware_monitor.


 No, really, forget about this, that's simply never going to happen.
  That's way too costly and dangerous.


   Then each one could do its
   detection thing. Unload the ones that weren't found.


 And do it all over again when the graphics adapter driver is loaded?
  And when an parport-to-I2C or USB-to-I2C adapter is plugged?


   That would really
   help people who can figure out the right driver module to load.


 If people can't run sensors-detect there's not much we can do for
  them ;)

What about live CDs and things like that? I do agree that there is no
immediate need to do something like this. But in the long run a scheme
like this could eliminate the need for sensors-detect.

It doesn't take as long to load/unload the modules as you might think,
they would all be on an initrd. The probing time should be the same as
the sensors-detect script. Maintenance would be better since the
detection code would only exist in one place instead of two.

File this away to think about for the future. After the drivers are
converted to the new model and have the ability to detect this can be
experimented with.




   You
   could even get fancy and generate a uevent after the right module was
   found. A script in the uevent could save the info so you don't have to
   repeat on each boot. Of course this assume that the hardware monitor
   modules can reliably autodetect.


 You mean, instead of just using the sensors-detect script which we
  already have and has worked fine for (almost) everyone for 8 years now?
  And the few cases where sensors-detect didn't work are a very good
  reason to _not_ attempt to do the same in the kernel. SMBus probing is
  _very_ tricky and we really want to control everything that happens.

  --

 Jean Delvare



-- 
Jon Smirl
[EMAIL PROTECTED]

___
i2c mailing list
i2c@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/i2c


Re: [i2c] [PATCH 0/4] i2c: Introduce i2c listeners

2008-06-04 Thread Jon Smirl
On 6/4/08, Jean Delvare [EMAIL PROTECTED] wrote:
 Hi all,

  This patch set demonstrates a new concept which I would like to add to
  the i2c subsystem. It is named i2c listeners and is based on the
  following structure:

How does this fit in with the existing driver bus structure in kernel?
This sounds a lot like hot plug and buses already implement hot plug
events.  This isn't exactly hot plug but it may be close enough that
the existing bus APIs will work. Note that both devices and entire
buses can be hot plugged with existing code.

-- 
Jon Smirl
[EMAIL PROTECTED]

___
i2c mailing list
i2c@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/i2c


Re: [i2c] [PATCH 0/4] i2c: Introduce i2c listeners

2008-06-04 Thread Jon Smirl
On 6/4/08, Jean Delvare [EMAIL PROTECTED] wrote:
 On Wed, 4 Jun 2008 15:28:54 -0400, Jon Smirl wrote:
   On 6/4/08, Jon Smirl [EMAIL PROTECTED] wrote:
On 6/4/08, Jean Delvare [EMAIL PROTECTED] wrote:
  Hi all,
 
   This patch set demonstrates a new concept which I would like to add 
 to
   the i2c subsystem. It is named i2c listeners and is based on the
   following structure:
  
   Could the existing API be used something like this?
  
   Drivers register with i2c core using standard driver registration.
   Class and address ranges are exposed in a structure like PCI IDs. I2C
   can make up it's own ID structure equivalent to the PCI IDs one.


 No, no, no. I've been repeating it many times already: I2C device
  addresses are NOT IDs. If you're even thinking of going in this
  direction, you're plain wrong, sorry. Many devices share the same
  address, and many devices can use different addresses. The address is
  really only one attribute of an I2C device, amongst many others. It's
  not an identifier.

I know these aren't identifiers, I'm proposing that they be ranges of
address to scan. The driver model doesn't force these to be IDs, but
in most cases they are IDs. To the driver model it's just a pointer to
a struct, no interpretation is done of the structure. It's the PCI bus
driver that interprets these structs as identifiers. In our case the
bus driver is inside i2c core and it interprets the structs as address
ranges and then calls the probe function sequentially with the
different value.

I shouldn't have compared this to PCI IDs, I was just thinking of it
as data that gets processed by i2c core. PCI IDs are an example of
data that is processed by the PCI core. Their meaning as ids is
supplied by PCI core, not the kernel driver framework.

  Please also realize that we _do_ have IDs for I2C devices by now. As
  there are no standard numbers as PCI has, we use arbitrary strings
  (typically the device name). That's what new-style drivers use for
  device binding since kernel 2.6.26. And you can't have more than one ID
  format for a given subsystem, and we don't want to break new-style
  drivers, so thinking about a radically different form of IDs for the
  i2c subsystem is a waste of time: it's simply not going to happen.


   When a new adapter/bus is created i2c core gets the event. Core then


 Side question: why would i2c-core want events when adapters are
  created, given that it does create them itself?

I used 'event' as a generic term for meaning the driver core was aware
that the adapter was created.

   scans the registered driver structures looking for class matches. When
   a match is encountered it repeatedly calls the driver's probe function
   with the addresses from the range. Driver returns true/false on each
   probe which triggers creation of the device.


 The driver's probe function is, by definition of the device driver
  model, called on an existing device. So the probe function cannot
  decide whether the device should be created or not - it's too late at
  this point.

I don't believe this is has to be true although usually it is. A
driver is passed in information from the bus core on the probe
function. It then looks at that information and decides if it wants to
bind. Drivers return true/false if they want to bind. If they return
false the device structure is destroyed (or reused for the next probe
call).

There is nothing in the model that requires an actual device to be
present for a probe call to happen. A perfectly valid reason for
refusing to bind would be not having hardware present at the address.
This is how older ISA drivers are loaded that predate PNP. They search
for the hardware in the probe function. That's probably why it is
called probe but that predates my involvement in Linux.

  One thing that we could consider though is merging i2c_listener into
  i2c_driver. For some reason I had not considered it - but I somehow
  expect funny locking issues if we do that.

We are not far apart on this proposal if you combine the
class/address_data fields into i2c_driver and then use the existing
probe function instead of detect(). The i2c_client struct already has
an address field. A driver would then check to see if there was
hardware at the i2c_client.address and return false if it is not
there. i2c core would just reuse the i2c_client struct with the next
address and do another probe.

Operations on an adapter need to be serialized but I thought the
adapters already supported that. If they don't they need to be fixed
anyway because of the BKL removal.



   Why do drivers need the bus creation (attach/detach adapter) events?


 Originally, to probe the adapters in search of devices to support. I
  seem to remember that some V4L/DVB drivers do more than that in their
  attach_adapter callback, but I need to take a closer look. And i2c-dev
  deserves particular attention, as it needs to do bookkeeping of all
  the adapters that are present

Re: [i2c] [PATCH 0/4] i2c: Introduce i2c listeners

2008-06-04 Thread Jon Smirl
On 6/4/08, David Brownell [EMAIL PROTECTED] wrote:
   It then looks at that information and decides if it wants to
   bind. Drivers return true/false if they want to bind.


 Wrong.  The probe returns zero if it bound, else negative errno.
  Those functions don't return booleans.

You guys are so literal. I should have said return 0 or -ENODEV or
some other error.

   If they return
   false the device structure is destroyed (or reused for the next probe
   call).


 No.  The struct device is not destroyed at that time for ANY
  driver model code I've seen in Linux.  Destroying it would
  prevent driver modules from binding to that device when they
  get loaded later.

Don't they get destroyed when hotplug devices are removed from the
system? Could this be considered a hotplug, create struct device,
probe to see if it is claimed, unhotplug if not claimed, destroy
struct device.

But I agree with what you meant, that they never get destroyed when
there is known hardware backing them in the system.

   They search
   for the hardware in the probe function. That's probably why it is
   called probe but that predates my involvement in Linux.


 And that legacy model is something that's getting removed
  everywhere practical.  Such legacy drivers don't support
  hotplugging or most of the other mechanisms defined by the
  driver model, since they have the driver creating the
  device structs (instead of bus infrastructure).

Those drivers are getting removed because the hardware is disappearing
and we have ACPI/PNP now to tell if if is present, not because they
misused the driver code. We just don't need the old style probing code
anymore and it was unreliable.

There is is a parallel here. With PowerPC device trees I can put the
exact address of the i2c device into the board specific device tree.
Now there's no need to probe addresses. That's equivalent to what's
happening with ACPI and serial ports. Maybe we'll implement device
trees on all archs and then we can get rid of i2c device probing.

-- 
Jon Smirl
[EMAIL PROTECTED]

___
i2c mailing list
i2c@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/i2c


Re: [i2c] [PATCH 0/4] i2c: Introduce i2c listeners

2008-06-04 Thread Jon Smirl
On 6/4/08, David Brownell [EMAIL PROTECTED] wrote:
 On Wednesday 04 June 2008, Trent Piepho wrote:
   Couldn't you say the probe function is called on a potential device?  The
   probe function can return -ENODEV, in which can other driver's probes get
   called, and it's perfectly ok if no driver binds to it.
  
   The way PCI works, is that when a new pci bus is created, each address is
   probed


 ... by config space accessors which all PCI devices support.

I made a mistake comparing this to PCI IDs. I should not have used PCI
IDs as an example and gotten us off on the tangent of not having a
global i2c device namespace.

Adding the address range to search is an unrelated problem to the name
space issue.





   and a device is created if anything responds.  The generic bus code
   tries to match each device to a driver or drivers


 ... using a formally managed set of product identifiers.



  and calls those drivers'
   probe functions.  The drivers don't have to claim the device in the probe
   function.  The bus code handles all the cases of a driver or bus getting
   added or removed in various orders.
  
   So why can't I2C do this too?


 No such product identifiers, and in general no way to tell
  what's sitting at a given address.  And in fact, there's no
  sure way to tell if a device is present there, since when
  an I2C device is busy, it's not required to ack its address.





-- 
Jon Smirl
[EMAIL PROTECTED]

___
i2c mailing list
i2c@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/i2c


Re: [i2c] [PATCH 2/3] i2c: Let bus drivers add SPD to their class

2008-06-03 Thread Jon Smirl
On 6/3/08, David Brownell [EMAIL PROTECTED] wrote:
 On Tuesday 03 June 2008, Jean Delvare wrote:
  
   Note that I took a conservative approach here, adding I2C_CLASS_SPD to
   all drivers that had I2C_CLASS_HWMON before. This is to make sure that
   the eeprom driver doesn't stop probing buses where SPD EEPROMs live.
   But the truth is that, for most of these, I simply have no idea whether
   they can host SPD EEPROMs or not.


 Few embedded platforms use discrete sticks of DRAM.
  My two cents:  use the opposite default in those cases.

  I'll highlight a few below, where I happen to have more
  specific knowledge.



   So, bus driver maintainers and users
   should feel free to remove the SPD class from drivers those buses never
   have SPD EEPROMs or they don't want the eeprom driver to bind to them.
   Likewise, feel free to add the SPD class to any bus driver I might have
   missed.
  
   Signed-off-by: Jean Delvare [EMAIL PROTECTED]
   ---
drivers/i2c/busses/i2c-ali1535.c   |2 +-
drivers/i2c/busses/i2c-ali1563.c   |2 +-
drivers/i2c/busses/i2c-ali15x3.c   |2 +-
drivers/i2c/busses/i2c-amd756.c|2 +-
drivers/i2c/busses/i2c-amd8111.c   |2 +-
drivers/i2c/busses/i2c-at91.c  |2 +-


 I've never heard of an AT91 board using DRAM sticks ...


drivers/i2c/busses/i2c-cpm.c   |2 +-
drivers/i2c/busses/i2c-davinci.c   |2 +-


 ... or a DaVinci one ...



drivers/i2c/busses/i2c-elektor.c   |2 +-
drivers/i2c/busses/i2c-gpio.c  |2 +-
drivers/i2c/busses/i2c-i801.c  |2 +-
drivers/i2c/busses/i2c-ibm_iic.c   |4 ++--
drivers/i2c/busses/i2c-iop3xx.c|2 +-
drivers/i2c/busses/i2c-isch.c  |2 +-
drivers/i2c/busses/i2c-mpc.c   |2 +-

Freescale embedded, no sticks

drivers/i2c/busses/i2c-mv64xxx.c   |2 +-
drivers/i2c/busses/i2c-nforce2.c   |2 +-
drivers/i2c/busses/i2c-ocores.c|2 +-
drivers/i2c/busses/i2c-omap.c  |2 +-


 ... or an OMAP one ...


drivers/i2c/busses/i2c-parport-light.c |2 +-
drivers/i2c/busses/i2c-parport.c   |2 +-
drivers/i2c/busses/i2c-pasemi.c|2 +-
drivers/i2c/busses/i2c-piix4.c |2 +-
drivers/i2c/busses/i2c-pmcmsp.c|2 +-
drivers/i2c/busses/i2c-s3c2410.c   |2 +-
drivers/i2c/busses/i2c-sibyte.c|4 ++--
drivers/i2c/busses/i2c-sis5595.c   |2 +-
drivers/i2c/busses/i2c-sis630.c|2 +-
drivers/i2c/busses/i2c-sis96x.c|2 +-
drivers/i2c/busses/i2c-stub.c  |2 +-
drivers/i2c/busses/i2c-tiny-usb.c  |2 +-


 ... DRAM-over-USB would be bizarre too ...


drivers/i2c/busses/i2c-via.c   |2 +-
drivers/i2c/busses/i2c-viapro.c|2 +-
drivers/i2c/busses/scx200_acb.c|2 +-
include/linux/i2c.h|1 +
35 files changed, 37 insertions(+), 36 deletions(-)




 ___
  i2c mailing list
  i2c@lm-sensors.org
  http://lists.lm-sensors.org/mailman/listinfo/i2c



-- 
Jon Smirl
[EMAIL PROTECTED]

___
i2c mailing list
i2c@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/i2c


Re: [i2c] [PATCH 2/3] i2c: Let bus drivers add SPD to their class

2008-06-03 Thread Jon Smirl
On 6/3/08, Trent Piepho [EMAIL PROTECTED] wrote:
 On Tue, 3 Jun 2008, Jon Smirl wrote:
   drivers/i2c/busses/i2c-elektor.c   |2 +-
   drivers/i2c/busses/i2c-gpio.c  |2 +-
   drivers/i2c/busses/i2c-i801.c  |2 +-
   drivers/i2c/busses/i2c-ibm_iic.c   |4 ++--
   drivers/i2c/busses/i2c-iop3xx.c|2 +-
   drivers/i2c/busses/i2c-isch.c  |2 +-
   drivers/i2c/busses/i2c-mpc.c   |2 +-
  
   Freescale embedded, no sticks


 I know you're wrong about this one, because I have a Freescale board with
  an mpc i2c adapter with SPD SODIMMs on the I2C bus sitting right in front
  of me.  The whole reason I added hexdump support the I2C tools' SPD parser
  was so I could parse the SPD data by hexdumping the eeproms with busybox.

Which CPU are you using? Maybe we should change to a socket.

  Using SPD with embedded boards isn't uncommon.  It's much easier to design
  a board with a SO-DIMM slot than actual chips.  In small quantities at
  least, it's cheaper too.  Using SPD is much more flexiable when you want to
  change memory size or speed, even if the memory isn't socketed.  If you
  look at U-Boot, most platforms are using SPD for DRAM controller setup and
  not hard coded values.



-- 
Jon Smirl
[EMAIL PROTECTED]

___
i2c mailing list
i2c@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/i2c


Re: [i2c] [PATCH] i2c: Push ioctl BKL down into the i2c code

2008-05-23 Thread Jon Smirl
On 5/23/08, Alan Cox [EMAIL PROTECTED] wrote:
 On Fri, 23 May 2008 09:35:45 +0200
  Jean Delvare [EMAIL PROTECTED] wrote:

   Hi Alan,
  
   On Thu, 22 May 2008 22:23:27 +0100, Alan Cox wrote:
Signed-off-by: Alan Cox [EMAIL PROTECTED]
   
  
   Description of what the patch does and why it is needed, please. I
   can't apply it without that. My first impression is a patch making the
   code bigger and more complex with no obvious benefit ;)


Alan, for people not familiar with the BKL attaching a write up or
pointer to the patches on the recommended ways to convert these locks
to something else would help. Some embedded developers won't know what
to do with these patches and they don't follow lkml.


 It pushes the BKL down into the i2c driver. The intention is to remove
  the big kernel lock ioctl method from the file_operations structure so
  that we can work on getting rid of the big kernel lock for good. It's one
  of a series of patches that give me an x86-32 tree with no -ioctl method
  at all.

  Similar activity is going on for the other calls made under the BKL the
  goal being to push it down into drivers and then eliminate it.



  ___
  i2c mailing list
  i2c@lm-sensors.org
  http://lists.lm-sensors.org/mailman/listinfo/i2c



-- 
Jon Smirl
[EMAIL PROTECTED]

___
i2c mailing list
i2c@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/i2c


Re: [i2c] [PATCH] mpc i2c driver, compare to NO_IRQ instead of zero

2008-05-02 Thread Jon Smirl
On 2/19/08, Jean Delvare [EMAIL PROTECTED] wrote:
 i2c-irq = platform_get_irq(pdev, 0);
   - if (i2c-irq  0) {
   + if (i2c-irq  NO_IRQ) {


 I am skeptical about this one. Can platform_get_irq() really return
  NO_IRQ? I thought that the IRQ resource would be plain missing if the
  device has no IRQ, so I would expect:


 i2c-irq = platform_get_irq(pdev, 0);
 if (i2c-irq  0)

 i2c-irq = NO_IRQ; /* Use polling */

  Testing against NO_IRQ suggests that devices with no IRQ would still
  have an IRQ resource defined and explicitly set to NO_IRQ. Sounds weird
  to me. Can you please clarify this point?

Your fix is correct. I'm not sure polling worked in the original driver.

  For what it's worth, no other kernel driver checks for irq  NO_IRQ.
  They all check for irq  0 after calling platform_get_irq().


 result = -ENXIO;
 goto fail_get_irq;
 }
   @@ -344,7 +344,7 @@ static int fsl_i2c_probe(struct platform_device *pdev)
 goto fail_map;
 }
  
   - if (i2c-irq != 0)
   + if (i2c-irq != NO_IRQ)
 if ((result = request_irq(i2c-irq, mpc_i2c_isr,
   IRQF_SHARED, i2c-mpc, i2c))  0) 
 {
 printk(KERN_ERR
   @@ -367,7 +367,7 @@ static int fsl_i2c_probe(struct platform_device *pdev)
 return result;
  
  fail_add:
   - if (i2c-irq != 0)
   + if (i2c-irq != NO_IRQ)
 free_irq(i2c-irq, i2c);
  fail_irq:
 iounmap(i2c-base);
   @@ -384,7 +384,7 @@ static int fsl_i2c_remove(struct platform_device *pdev)
 i2c_del_adapter(i2c-adap);
 platform_set_drvdata(pdev, NULL);
  
   - if (i2c-irq != 0)
   + if (i2c-irq != NO_IRQ)
 free_irq(i2c-irq, i2c);
  
 iounmap(i2c-base);


 The rest looks good.

  --

 Jean Delvare



-- 
Jon Smirl
[EMAIL PROTECTED]

___
i2c mailing list
i2c@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/i2c


Re: [i2c] [PATCH] mpc i2c driver, compare to NO_IRQ instead of zero

2008-05-02 Thread Jon Smirl
New version with your fix.

diff --git a/drivers/i2c/busses/i2c-mpc.c b/drivers/i2c/busses/i2c-mpc.c
index bbe787b..b141057 100644
--- a/drivers/i2c/busses/i2c-mpc.c
+++ b/drivers/i2c/busses/i2c-mpc.c
@@ -99,7 +99,7 @@ static int i2c_wait(struct mpc_i2c *i2c, unsigned
timeout, int writing)
u32 x;
int result = 0;

-   if (i2c-irq == 0)
+   if (i2c-irq == NO_IRQ)
{
while (!(readb(i2c-base + MPC_I2C_SR)  CSR_MIF)) {
schedule();
@@ -329,10 +329,9 @@ static int fsl_i2c_probe(struct platform_device *pdev)
return -ENOMEM;

i2c-irq = platform_get_irq(pdev, 0);
-   if (i2c-irq  0) {
-   result = -ENXIO;
-   goto fail_get_irq;
-   }
+   if (i2c-irq  0)
+   i2c-irq = NO_IRQ; /* Use polling */
+
i2c-flags = pdata-device_flags;
init_waitqueue_head(i2c-queue);

@@ -344,7 +343,7 @@ static int fsl_i2c_probe(struct platform_device *pdev)
goto fail_map;
}

-   if (i2c-irq != 0)
+   if (i2c-irq != NO_IRQ)
if ((result = request_irq(i2c-irq, mpc_i2c_isr,
  IRQF_SHARED, i2c-mpc, i2c))  0) {
printk(KERN_ERR
@@ -367,7 +366,7 @@ static int fsl_i2c_probe(struct platform_device *pdev)
return result;

   fail_add:
-   if (i2c-irq != 0)
+   if (i2c-irq != NO_IRQ)
free_irq(i2c-irq, i2c);
   fail_irq:
iounmap(i2c-base);
@@ -384,7 +383,7 @@ static int fsl_i2c_remove(struct platform_device *pdev)
i2c_del_adapter(i2c-adap);
platform_set_drvdata(pdev, NULL);

-   if (i2c-irq != 0)
+   if (i2c-irq != NO_IRQ)
free_irq(i2c-irq, i2c);

iounmap(i2c-base);


-- 
Jon Smirl
[EMAIL PROTECTED]

___
i2c mailing list
i2c@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/i2c


Re: [i2c] [PATCH] mpc i2c driver, compare to NO_IRQ instead of zero

2008-05-02 Thread Jon Smirl
I attached the diff file. I had forgot that I renamed the file so it
wasn't getting compiled. I compiled it this time. I've made too many
other changes to it to test this version on my current hardware.

-- 
Jon Smirl
[EMAIL PROTECTED]
diff --git a/drivers/i2c/busses/i2c-mpc.c b/drivers/i2c/busses/i2c-mpc.c
index bbe787b..d73edef 100644
--- a/drivers/i2c/busses/i2c-mpc.c
+++ b/drivers/i2c/busses/i2c-mpc.c
@@ -99,7 +99,7 @@ static int i2c_wait(struct mpc_i2c *i2c, unsigned timeout, int writing)
 	u32 x;
 	int result = 0;
 
-	if (i2c-irq == 0)
+	if (i2c-irq == NO_IRQ)
 	{
 		while (!(readb(i2c-base + MPC_I2C_SR)  CSR_MIF)) {
 			schedule();
@@ -329,10 +329,9 @@ static int fsl_i2c_probe(struct platform_device *pdev)
 		return -ENOMEM;
 
 	i2c-irq = platform_get_irq(pdev, 0);
-	if (i2c-irq  0) {
-		result = -ENXIO;
-		goto fail_get_irq;
-	}
+	if (i2c-irq  0)
+		i2c-irq = NO_IRQ; /* Use polling */
+
 	i2c-flags = pdata-device_flags;
 	init_waitqueue_head(i2c-queue);
 
@@ -344,7 +343,7 @@ static int fsl_i2c_probe(struct platform_device *pdev)
 		goto fail_map;
 	}
 
-	if (i2c-irq != 0)
+	if (i2c-irq != NO_IRQ)
 		if ((result = request_irq(i2c-irq, mpc_i2c_isr,
 	  IRQF_SHARED, i2c-mpc, i2c))  0) {
 			printk(KERN_ERR
@@ -367,12 +366,11 @@ static int fsl_i2c_probe(struct platform_device *pdev)
 	return result;
 
   fail_add:
-	if (i2c-irq != 0)
+	if (i2c-irq != NO_IRQ)
 		free_irq(i2c-irq, i2c);
   fail_irq:
 	iounmap(i2c-base);
   fail_map:
-  fail_get_irq:
 	kfree(i2c);
 	return result;
 };
@@ -384,7 +382,7 @@ static int fsl_i2c_remove(struct platform_device *pdev)
 	i2c_del_adapter(i2c-adap);
 	platform_set_drvdata(pdev, NULL);
 
-	if (i2c-irq != 0)
+	if (i2c-irq != NO_IRQ)
 		free_irq(i2c-irq, i2c);
 
 	iounmap(i2c-base);
___
i2c mailing list
i2c@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/i2c

Re: [i2c] [PATCH 1/2 v2] i2c: Add support for device alias names

2008-04-29 Thread Jon Smirl
On 4/29/08, Jean Delvare [EMAIL PROTECTED] wrote:
 Based on earlier work by Jon Smirl and Jochen Friedrich.
  +/* Looks like: i2c:S */
  +static int do_i2c_entry(const char *filename, struct i2c_device_id *id,
  +   char *alias)
  +{
  +   sprintf(alias, I2C_MODULE_PREFIX %s, id-name);

Do you want to add the colon?

  +
  +   return 1;
  +}

-- 
Jon Smirl
[EMAIL PROTECTED]

___
i2c mailing list
i2c@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/i2c


Re: [i2c] [PATCHv4 2.6.25] i2c: adds support for i2c bus on Freescale CPM1/CPM2 controllers

2008-02-24 Thread Jon Smirl
On 2/22/08, Jochen Friedrich [EMAIL PROTECTED] wrote:
 Hi Jean,


   +/*
   + * Wait for patch from Jon Smirl
   + * #include powerpc-common.h
   + */
  
   It doesn't make sense to merge this comment upstream.


 I know you don't like the patch from Jon Smirl and you also explained your 
 reasons.
  Fortunately, I2c no longer uses numeric device IDs but names. So what are 
 the alternatives?

  1. modify the I2c subsystem to accept OF names additionally to I2c names 
 (proposed by Jon smirl).

The correct statement is: modify the i2c subsystem to support the
standard kernel driver aliasing mechanism. Leaving powerpc and OF out
of the argument for the moment, i2c has a custom aliasing scheme on
the x86 too.

So as a first step, can we remove the custom i2c aliasing scheme and
change i2c to use standard module aliases on the x86? Patches for this
already exist.

On 2/23/08, Jean Delvare [EMAIL PROTECTED] wrote:
 The problem I have with this is that it breaks compatibility. The chip
  name is not only used for device/driver matching, it is also exported
  to userspace as a sysfs attribute (name). Applications might rely on
  it. At least libsensors does.

I think there is some confusion here. The OF aliases are only used by
the kernel to load the correct driver. Would doing something like this
help?

static struct i2c_device_id pcf8563_id[] = {
{pcf8563, 0, sysfs_legacy_name},
{rtc8564, 0, sysfs_legacy_name},
OF_ID(phillips,pcf8563, pcf8563_id[0], 0)
OF_ID(epson,rtc8564, pcf8563_id[1], 0)
{},
};
MODULE_DEVICE_TABLE(i2c, pcf8563_id);

Then in the probe function you can use the pointer to find the base id
entry and i2c never has to be aware that the OF alias exists.

  2. record the I2c name in the dts tree, either as separate tag (like 
 linux,i2c-name=i2c-name)

Not really practical for the millions of machines (all PowerPC Macs)
already shipped.

or as additional compatible entry (like compatible=..., 
 linux,i2c-name).
  3. use a glue layer with a translation map.

Audio codecs have exactly the same problem. There are probably other
devices that also need mapping.

This mapping table will need to contain a map from the OF names to the
kernel driver names.  It will need to stored permanently in RAM and
contain all possible mappings. This table will only grow in size.

The kernel has a widely used mechanism for mapping -- aliases in the
device drivers. Why do we want to build a new, parallel one?

What we are doing now is option 4.

4. Use kconfig to build custom kernels for each target system. Don't
load drivers automatically based on what the BIOS tells us.

-- 
Jon Smirl
[EMAIL PROTECTED]

___
i2c mailing list
i2c@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/i2c


Re: [i2c] [PATCH] Convert pfc8563 i2c driver from old style to new style

2008-02-19 Thread Jon Smirl
On 2/19/08, Jean Delvare [EMAIL PROTECTED] wrote:
 Hi Jon,

 On Mon, 21 Jan 2008 15:09:01 -0500, Jon Smirl wrote:
  Convert pfc8563 i2c driver from old style to new style.

Let's just forget about this patch until the dynamic module loading
support goes into the base.

The Phytec PCM030 mpc5200 board uses this chip. They are staying out
of tree because they've integrated real-time support which is not in
mainline yet.

-- 
Jon Smirl
[EMAIL PROTECTED]

___
i2c mailing list
i2c@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/i2c


[i2c] [PATCH 3/3] Add device tree compatible aliases to i2c drivers

2008-01-28 Thread Jon Smirl
PowerPC device trees use a different naming convention than the Linux kernel.  
Provide alias names for i2c drivers in order to allow them to be loaded by 
device tree name. The OF_ID macro ensures that the aliases are only present in 
powerpc builds and separated into their own namespace.

Signed-off-by: Jon Smirl [EMAIL PROTECTED]
---

 drivers/hwmon/f75375s.c  |2 ++
 drivers/i2c/chips/ds1682.c   |1 +
 drivers/i2c/chips/menelaus.c |1 +
 drivers/i2c/chips/tps65010.c |4 
 drivers/i2c/chips/tsl2550.c  |1 +
 drivers/rtc/rtc-ds1307.c |6 ++
 drivers/rtc/rtc-ds1374.c |1 +
 drivers/rtc/rtc-m41t80.c |8 
 drivers/rtc/rtc-pcf8563.c|2 ++
 drivers/rtc/rtc-rs5c372.c|4 
 10 files changed, 30 insertions(+), 0 deletions(-)


diff --git a/drivers/hwmon/f75375s.c b/drivers/hwmon/f75375s.c
index 4cb4db4..dd548e7 100644
--- a/drivers/hwmon/f75375s.c
+++ b/drivers/hwmon/f75375s.c
@@ -132,6 +132,8 @@ static struct i2c_driver f75375_legacy_driver = {
 static const struct i2c_device_id f75375_id[] = {
{ f75373, f75373 },
{ f75375, f75375 },
+   OF_ID(fintek,f75373, f75373)
+   OF_ID(fintek,f75375, f75375)
{ },
 };
 MODULE_DEVICE_TABLE(i2c, f75375_id);
diff --git a/drivers/i2c/chips/ds1682.c b/drivers/i2c/chips/ds1682.c
index 51ff518..817ad1f 100644
--- a/drivers/i2c/chips/ds1682.c
+++ b/drivers/i2c/chips/ds1682.c
@@ -237,6 +237,7 @@ static int ds1682_remove(struct i2c_client *client)
 
 static const struct i2c_device_id ds1682_id[] = {
{ ds1682, 0 },
+   OF_ID(dallas,ds1682, 0)
{ },
 };
 MODULE_DEVICE_TABLE(i2c, ds1682_id);
diff --git a/drivers/i2c/chips/menelaus.c b/drivers/i2c/chips/menelaus.c
index 1f9ac5e..ba7d619 100644
--- a/drivers/i2c/chips/menelaus.c
+++ b/drivers/i2c/chips/menelaus.c
@@ -1245,6 +1245,7 @@ static int __exit menelaus_remove(struct i2c_client 
*client)
 
 static const struct i2c_device_id menelaus_id[] = {
{ menelaus, 0 },
+   OF_ID(ti,twl92330, 0)
{ },
 };
 MODULE_DEVICE_TABLE(i2c, menelaus_id);
diff --git a/drivers/i2c/chips/tps65010.c b/drivers/i2c/chips/tps65010.c
index e07274d..9cd1770 100644
--- a/drivers/i2c/chips/tps65010.c
+++ b/drivers/i2c/chips/tps65010.c
@@ -571,6 +571,10 @@ static const struct i2c_device_id tps65010_id[] = {
{ tps65011, TPS65011 },
{ tps65012, TPS65012 },
{ tps65013, TPS65013 },
+   OF_ID(ti,tps65010, TPS65010)
+   OF_ID(ti,tps65011, TPS65011)
+   OF_ID(ti,tps65012, TPS65012)
+   OF_ID(ti,tps65013, TPS65013)
{ },
 };
 MODULE_DEVICE_TABLE(i2c, tps65010_id);
diff --git a/drivers/i2c/chips/tsl2550.c b/drivers/i2c/chips/tsl2550.c
index 2add8be..8071d6d 100644
--- a/drivers/i2c/chips/tsl2550.c
+++ b/drivers/i2c/chips/tsl2550.c
@@ -454,6 +454,7 @@ static int tsl2550_resume(struct i2c_client *client)
 
 static const struct i2c_device_id tsl2550_id[] = {
{ tsl2550, 0 },
+   OF_ID(taos,tsl2550, 0)
{ },
 };
 MODULE_DEVICE_TABLE(i2c, tsl2550_id);
diff --git a/drivers/rtc/rtc-ds1307.c b/drivers/rtc/rtc-ds1307.c
index a5614ab..e363a5f 100644
--- a/drivers/rtc/rtc-ds1307.c
+++ b/drivers/rtc/rtc-ds1307.c
@@ -129,6 +129,12 @@ static const struct i2c_device_id ds1307_id[] = {
{ ds1339, ds_1339 },
{ ds1340, ds_1340 },
{ m41t00, m41t00 },
+   OF_ID(dallas,ds1307, ds_1307)
+   OF_ID(dallas,ds1337, ds_1337)
+   OF_ID(dallas,ds1338, ds_1338)
+   OF_ID(dallas,ds1339, ds_1339)
+   OF_ID(dallas,ds1340, ds_1340)
+   OF_ID(stm,m41t00, m41t00)
{},
 };
 MODULE_DEVICE_TABLE(i2c, ds1307_id);
diff --git a/drivers/rtc/rtc-ds1374.c b/drivers/rtc/rtc-ds1374.c
index 376ceeb..e4f680a 100644
--- a/drivers/rtc/rtc-ds1374.c
+++ b/drivers/rtc/rtc-ds1374.c
@@ -43,6 +43,7 @@
 
 static const struct i2c_device_id ds1374_id[] = {
{ ds1374, 0 },
+   OF_ID(dallas,ds1374, 0)
{},
 };
 MODULE_DEVICE_TABLE(i2c, ds1374_id);
diff --git a/drivers/rtc/rtc-m41t80.c b/drivers/rtc/rtc-m41t80.c
index c672557..663f8b5 100644
--- a/drivers/rtc/rtc-m41t80.c
+++ b/drivers/rtc/rtc-m41t80.c
@@ -69,6 +69,14 @@ static const struct i2c_device_id m41t80_id[] = {
{ m41st84, M41T80_FEATURE_HT | M41T80_FEATURE_BL },
{ m41st85, M41T80_FEATURE_HT | M41T80_FEATURE_BL },
{ m41st87, M41T80_FEATURE_HT | M41T80_FEATURE_BL },
+   OF_ID(stm,m41t80, 0)
+   OF_ID(stm,m41t81, M41T80_FEATURE_HT)
+   OF_ID(stm,m41t81s, M41T80_FEATURE_HT | M41T80_FEATURE_BL)
+   OF_ID(stm,m41t82, M41T80_FEATURE_HT | M41T80_FEATURE_BL)
+   OF_ID(stm,m41t83, M41T80_FEATURE_HT | M41T80_FEATURE_BL)
+   OF_ID(stm,m41st84, M41T80_FEATURE_HT | M41T80_FEATURE_BL)
+   OF_ID(stm,m41st85, M41T80_FEATURE_HT | M41T80_FEATURE_BL)
+   OF_ID(stm,m41st87, M41T80_FEATURE_HT | M41T80_FEATURE_BL)
{},
 };
 MODULE_DEVICE_TABLE(i2c, m41t80_id);
diff --git a/drivers/rtc/rtc-pcf8563.c b/drivers/rtc/rtc-pcf8563.c
index 8eff549..51d7471 100644

[i2c] [PATCH 1/3] Rename i2c-mpc to i2c-mpc-drv in preparation for breaking out common code.

2008-01-28 Thread Jon Smirl
Rename i2c-mpc to i2c-mpc-drv in preparation for breaking out common code.

Signed-off-by: Jon Smirl [EMAIL PROTECTED]
---

 drivers/i2c/busses/Makefile  |2 
 drivers/i2c/busses/i2c-mpc-drv.c |  421 ++
 drivers/i2c/busses/i2c-mpc.c |  421 --
 3 files changed, 423 insertions(+), 421 deletions(-)
 create mode 100644 drivers/i2c/busses/i2c-mpc-drv.c
 delete mode 100644 drivers/i2c/busses/i2c-mpc.c


diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
index ea7068f..171800d 100644
--- a/drivers/i2c/busses/Makefile
+++ b/drivers/i2c/busses/Makefile
@@ -2,6 +2,8 @@
 # Makefile for the i2c bus drivers.
 #
 
+i2c-mpc-objs   := i2c-mpc-drv.o
+
 obj-$(CONFIG_I2C_ALI1535)  += i2c-ali1535.o
 obj-$(CONFIG_I2C_ALI1563)  += i2c-ali1563.o
 obj-$(CONFIG_I2C_ALI15X3)  += i2c-ali15x3.o
diff --git a/drivers/i2c/busses/i2c-mpc.c b/drivers/i2c/busses/i2c-mpc-drv.c
similarity index 100%
rename from drivers/i2c/busses/i2c-mpc.c
rename to drivers/i2c/busses/i2c-mpc-drv.c


___
i2c mailing list
i2c@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/i2c


Re: [i2c] [GIT PULL] i2c updates for 2.6.25

2008-01-28 Thread Jon Smirl
On 1/28/08, Jean Delvare [EMAIL PROTECTED] wrote:
 Hi Jon,

 On Sun, 27 Jan 2008 12:36:03 -0500, Jon Smirl wrote:
  On 1/27/08, Jean Delvare [EMAIL PROTECTED] wrote:
   Linus,
  
   Please pull the i2c subsystem updates for Linux 2.6.25 from:
  
   git://jdelvare.pck.nerim.net/jdelvare-2.6 i2c-for-linus
 
  The support for modalias loading of i2c modules isn't in here, what's
  the status?

 The status is that I want to give people some time to comment on my
 modalias patches before I merge them. I also didn't have the time to
 look into the latest patches you sent yet, and I'd rather merge
 everything at once. So the merge window for this is rather 2.6.26.

Delaying these until 2.6.26 which won't be final until 2009 forces us
to ship boxes using a private kernel. These patches have been around
since November. Does it really take over a year to get these changes
in?


 --
 Jean Delvare



-- 
Jon Smirl
[EMAIL PROTECTED]

___
i2c mailing list
i2c@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/i2c


Re: [i2c] [GIT PULL] i2c updates for 2.6.25

2008-01-27 Thread Jon Smirl
On 1/27/08, Jean Delvare [EMAIL PROTECTED] wrote:
 Linus,

 Please pull the i2c subsystem updates for Linux 2.6.25 from:

 git://jdelvare.pck.nerim.net/jdelvare-2.6 i2c-for-linus

The support for modalias loading of i2c modules isn't in here, what's
the status?

-- 
Jon Smirl
[EMAIL PROTECTED]

___
i2c mailing list
i2c@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/i2c


Re: [i2c] [PATCH] mpc i2c driver, compare to NO_IRQ instead of zero

2008-01-24 Thread Jon Smirl
Ben, do you approve of this? How should error be checked for, is
NO_IRQ right? The current code in the kernel looks to be broken
because of these checks, the ppc build is wrong and powerpc polled
mode doesn't work.

On 1/21/08, Jon Smirl [EMAIL PROTECTED] wrote:
 Alter the mpc i2c driver to use the NO_IRQ symbol instead of the constant 
 zero when checking for valid interrupts. NO_IRQ=-1 on ppc and NO_IRQ=0 on 
 powerpc so the checks against zero are not correct.

 Signed-off-by: Jon Smirl [EMAIL PROTECTED]
 ---

  drivers/i2c/busses/i2c-mpc.c |   10 +-
  1 files changed, 5 insertions(+), 5 deletions(-)


 diff --git a/drivers/i2c/busses/i2c-mpc.c b/drivers/i2c/busses/i2c-mpc.c
 index bbe787b..d20959d 100644
 --- a/drivers/i2c/busses/i2c-mpc.c
 +++ b/drivers/i2c/busses/i2c-mpc.c
 @@ -99,7 +99,7 @@ static int i2c_wait(struct mpc_i2c *i2c, unsigned timeout, 
 int writing)
 u32 x;
 int result = 0;

 -   if (i2c-irq == 0)
 +   if (i2c-irq == NO_IRQ)
 {
 while (!(readb(i2c-base + MPC_I2C_SR)  CSR_MIF)) {
 schedule();
 @@ -329,7 +329,7 @@ static int fsl_i2c_probe(struct platform_device *pdev)
 return -ENOMEM;

 i2c-irq = platform_get_irq(pdev, 0);
 -   if (i2c-irq  0) {
 +   if (i2c-irq  NO_IRQ) {
 result = -ENXIO;
 goto fail_get_irq;
 }
 @@ -344,7 +344,7 @@ static int fsl_i2c_probe(struct platform_device *pdev)
 goto fail_map;
 }

 -   if (i2c-irq != 0)
 +   if (i2c-irq != NO_IRQ)
 if ((result = request_irq(i2c-irq, mpc_i2c_isr,
   IRQF_SHARED, i2c-mpc, i2c))  0) {
 printk(KERN_ERR
 @@ -367,7 +367,7 @@ static int fsl_i2c_probe(struct platform_device *pdev)
 return result;

fail_add:
 -   if (i2c-irq != 0)
 +   if (i2c-irq != NO_IRQ)
 free_irq(i2c-irq, i2c);
fail_irq:
 iounmap(i2c-base);
 @@ -384,7 +384,7 @@ static int fsl_i2c_remove(struct platform_device *pdev)
 i2c_del_adapter(i2c-adap);
 platform_set_drvdata(pdev, NULL);

 -   if (i2c-irq != 0)
 +   if (i2c-irq != NO_IRQ)
 free_irq(i2c-irq, i2c);

 iounmap(i2c-base);


 ___
 i2c mailing list
 i2c@lm-sensors.org
 http://lists.lm-sensors.org/mailman/listinfo/i2c



-- 
Jon Smirl
[EMAIL PROTECTED]

___
i2c mailing list
i2c@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/i2c


Re: [i2c] [PATCH] mpc i2c driver, compare to NO_IRQ instead of zero

2008-01-24 Thread Jon Smirl
On 1/24/08, Benjamin Herrenschmidt [EMAIL PROTECTED] wrote:

 On Thu, 2008-01-24 at 17:32 -0500, Jon Smirl wrote:
  Ben, do you approve of this? How should error be checked for, is
  NO_IRQ right? The current code in the kernel looks to be broken
  because of these checks, the ppc build is wrong and powerpc polled
  mode doesn't work.

  == 0 should work on powerpc since NO_IRQ is defined to be 0 there no ?

The driver being patched is used in both the powerpc and ppc builds.


 Anyway, using the symbolic constant is always nicer I suppose.

 Ben.

  On 1/21/08, Jon Smirl [EMAIL PROTECTED] wrote:
   Alter the mpc i2c driver to use the NO_IRQ symbol instead of the constant 
   zero when checking for valid interrupts. NO_IRQ=-1 on ppc and NO_IRQ=0 on 
   powerpc so the checks against zero are not correct.
  
   Signed-off-by: Jon Smirl [EMAIL PROTECTED]
   ---
  
drivers/i2c/busses/i2c-mpc.c |   10 +-
1 files changed, 5 insertions(+), 5 deletions(-)
  
  
   diff --git a/drivers/i2c/busses/i2c-mpc.c b/drivers/i2c/busses/i2c-mpc.c
   index bbe787b..d20959d 100644
   --- a/drivers/i2c/busses/i2c-mpc.c
   +++ b/drivers/i2c/busses/i2c-mpc.c
   @@ -99,7 +99,7 @@ static int i2c_wait(struct mpc_i2c *i2c, unsigned 
   timeout, int writing)
   u32 x;
   int result = 0;
  
   -   if (i2c-irq == 0)
   +   if (i2c-irq == NO_IRQ)
   {
   while (!(readb(i2c-base + MPC_I2C_SR)  CSR_MIF)) {
   schedule();
   @@ -329,7 +329,7 @@ static int fsl_i2c_probe(struct platform_device *pdev)
   return -ENOMEM;
  
   i2c-irq = platform_get_irq(pdev, 0);
   -   if (i2c-irq  0) {
   +   if (i2c-irq  NO_IRQ) {
   result = -ENXIO;
   goto fail_get_irq;
   }
   @@ -344,7 +344,7 @@ static int fsl_i2c_probe(struct platform_device *pdev)
   goto fail_map;
   }
  
   -   if (i2c-irq != 0)
   +   if (i2c-irq != NO_IRQ)
   if ((result = request_irq(i2c-irq, mpc_i2c_isr,
 IRQF_SHARED, i2c-mpc, i2c))  
   0) {
   printk(KERN_ERR
   @@ -367,7 +367,7 @@ static int fsl_i2c_probe(struct platform_device *pdev)
   return result;
  
  fail_add:
   -   if (i2c-irq != 0)
   +   if (i2c-irq != NO_IRQ)
   free_irq(i2c-irq, i2c);
  fail_irq:
   iounmap(i2c-base);
   @@ -384,7 +384,7 @@ static int fsl_i2c_remove(struct platform_device 
   *pdev)
   i2c_del_adapter(i2c-adap);
   platform_set_drvdata(pdev, NULL);
  
   -   if (i2c-irq != 0)
   +   if (i2c-irq != NO_IRQ)
   free_irq(i2c-irq, i2c);
  
   iounmap(i2c-base);
  
  
   ___
   i2c mailing list
   i2c@lm-sensors.org
   http://lists.lm-sensors.org/mailman/listinfo/i2c
  
 
 




-- 
Jon Smirl
[EMAIL PROTECTED]

___
i2c mailing list
i2c@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/i2c


Re: [i2c] [PATCH 3/3] Add device tree compatible aliases to i2c drivers

2008-01-23 Thread Jon Smirl
On 1/23/08, Matt Sealey [EMAIL PROTECTED] wrote:


 Jon Smirl wrote:
  --- a/drivers/i2c/chips/tps65010.c
  +++ b/drivers/i2c/chips/tps65010.c
  @@ -571,6 +571,10 @@ static const struct i2c_device_id tps65010_id[] = {
{ tps65011, TPS65011 },
{ tps65012, TPS65012 },
{ tps65013, TPS65013 },
  + OF_ID(ti,tps65010, TPS65010)
  + OF_ID(ti,tps65011, TPS65011)
  + OF_ID(ti,tps65012, TPS65012)
  + OF_ID(ri,tps65013, TPS65013)
{ },

 ti, ti, ti, ri?

  --- a/drivers/rtc/rtc-ds1307.c
  +++ b/drivers/rtc/rtc-ds1307.c
  @@ -129,6 +129,12 @@ static const struct i2c_device_id ds1307_id[] = {
{ ds1339, ds_1339 },
{ ds1340, ds_1340 },
{ m41t00, m41t00 },
  + OF_ID(dallas,ds1307, ds_1307)
  + OF_ID(dallas,ds1337, ds_1337)
  + OF_ID(dallas,ds1338, ds_1338)
  + OF_ID(dallas,ds1339, ds_1339)
  + OF_ID(dallas,ds1340, ds_1340)
  + OF_ID(stm,m41t00, m41t00)
{},
   };

 The convention is to use the stock ticker, capitalized, if a company
 has one, I guess dallas is MXIM these days, but dallas is a good
 substitute based on the fact that most people still remember Dallas
 clock chips and so on from the Ancient Days. STMicroelectronics is STM.
 I couldn't care less about case sensitivity, but the stock ticker
 thing was always a good idea.. it gives a sort of grounding in reality
 for the manufacturer names.

I'll put in whatever the Open Firmware police tell me to. We just all
need to come to an agreement.


 Are we still following this convention or are the names of devices
 simply arbitrarily defined by Linux kernel developers now?

 --
 Matt Sealey [EMAIL PROTECTED]
 Genesi, Manager, Developer Relations



-- 
Jon Smirl
[EMAIL PROTECTED]

___
i2c mailing list
i2c@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/i2c


[i2c] [PATCH 2/3] Convert PowerPC MPC i2c to of_platform_driver from platform_driver

2008-01-22 Thread Jon Smirl
Convert MPC i2c driver from a platform_driver to a of_platform_driver. Add the 
ability to dynamically load i2c drivers based on device tree names. Routine 
names were changed from fsl_ to mpc_ to make them match the file name. Common 
code moved to powerpc-common.* Orginal ppc driver left intact for deletion when 
ppc arch is removed.

Signed-off-by: Jon Smirl [EMAIL PROTECTED]
---

 arch/powerpc/sysdev/fsl_soc.c   |  125 ---
 drivers/i2c/busses/Makefile |2 
 drivers/i2c/busses/i2c-mpc-drv.c|  165 ---
 drivers/i2c/busses/powerpc-common.c |   81 +
 drivers/i2c/busses/powerpc-common.h |   23 +
 include/linux/mod_devicetable.h |9 ++
 6 files changed, 263 insertions(+), 142 deletions(-)
 create mode 100644 drivers/i2c/busses/powerpc-common.c
 create mode 100644 drivers/i2c/busses/powerpc-common.h


diff --git a/arch/powerpc/sysdev/fsl_soc.c b/arch/powerpc/sysdev/fsl_soc.c
index e4b14a5..d6ef264 100644
--- a/arch/powerpc/sysdev/fsl_soc.c
+++ b/arch/powerpc/sysdev/fsl_soc.c
@@ -318,131 +318,6 @@ err:
 
 arch_initcall(gfar_of_init);
 
-#ifdef CONFIG_I2C_BOARDINFO
-#include linux/i2c.h
-struct i2c_driver_device {
-   char*of_device;
-   char*i2c_type;
-};
-
-static struct i2c_driver_device i2c_devices[] __initdata = {
-   {ricoh,rs5c372a, rs5c372a,},
-   {ricoh,rs5c372b, rs5c372b,},
-   {ricoh,rv5c386,  rv5c386,},
-   {ricoh,rv5c387a, rv5c387a,},
-   {dallas,ds1307,  ds1307,},
-   {dallas,ds1337,  ds1337,},
-   {dallas,ds1338,  ds1338,},
-   {dallas,ds1339,  ds1339,},
-   {dallas,ds1340,  ds1340,},
-   {stm,m41t00, m41t00},
-   {dallas,ds1374,  rtc-ds1374,},
-};
-
-static int __init of_find_i2c_driver(struct device_node *node,
-struct i2c_board_info *info)
-{
-   int i;
-
-   for (i = 0; i  ARRAY_SIZE(i2c_devices); i++) {
-   if (!of_device_is_compatible(node, i2c_devices[i].of_device))
-   continue;
-   if (strlcpy(info-type, i2c_devices[i].i2c_type,
-   I2C_NAME_SIZE) = I2C_NAME_SIZE)
-   return -ENOMEM;
-   return 0;
-   }
-   return -ENODEV;
-}
-
-static void __init of_register_i2c_devices(struct device_node *adap_node,
-  int bus_num)
-{
-   struct device_node *node = NULL;
-
-   while ((node = of_get_next_child(adap_node, node))) {
-   struct i2c_board_info info = {};
-   const u32 *addr;
-   int len;
-
-   addr = of_get_property(node, reg, len);
-   if (!addr || len  sizeof(int) || *addr  (1  10) - 1) {
-   printk(KERN_WARNING fsl_soc.c: invalid i2c device 
entry\n);
-   continue;
-   }
-
-   info.irq = irq_of_parse_and_map(node, 0);
-   if (info.irq == NO_IRQ)
-   info.irq = -1;
-
-   if (of_find_i2c_driver(node, info)  0)
-   continue;
-
-   info.addr = *addr;
-
-   i2c_register_board_info(bus_num, info, 1);
-   }
-}
-
-static int __init fsl_i2c_of_init(void)
-{
-   struct device_node *np;
-   unsigned int i;
-   struct platform_device *i2c_dev;
-   int ret;
-
-   for (np = NULL, i = 0;
-(np = of_find_compatible_node(np, i2c, fsl-i2c)) != NULL;
-i++) {
-   struct resource r[2];
-   struct fsl_i2c_platform_data i2c_data;
-   const unsigned char *flags = NULL;
-
-   memset(r, 0, sizeof(r));
-   memset(i2c_data, 0, sizeof(i2c_data));
-
-   ret = of_address_to_resource(np, 0, r[0]);
-   if (ret)
-   goto err;
-
-   of_irq_to_resource(np, 0, r[1]);
-
-   i2c_dev = platform_device_register_simple(fsl-i2c, i, r, 2);
-   if (IS_ERR(i2c_dev)) {
-   ret = PTR_ERR(i2c_dev);
-   goto err;
-   }
-
-   i2c_data.device_flags = 0;
-   flags = of_get_property(np, dfsrr, NULL);
-   if (flags)
-   i2c_data.device_flags |= FSL_I2C_DEV_SEPARATE_DFSRR;
-
-   flags = of_get_property(np, fsl5200-clocking, NULL);
-   if (flags)
-   i2c_data.device_flags |= FSL_I2C_DEV_CLOCK_5200;
-
-   ret =
-   platform_device_add_data(i2c_dev, i2c_data,
-sizeof(struct
-   fsl_i2c_platform_data));
-   if (ret)
-   goto unreg;
-
-   of_register_i2c_devices(np, i);
-   }
-
-   return 0;
-
-unreg:
-   platform_device_unregister(i2c_dev);
-err:
-   return ret

[i2c] i2c_adapter.class and new style drivers

2008-01-21 Thread Jon Smirl
Is i2c_adapter.class relevant anymore with new style drivers?

On 1/2/08, Jon Smirl [EMAIL PROTECTED] wrote:
 On 1/2/08, Jochen Friedrich [EMAIL PROTECTED] wrote:
  IMHO, there should be a node attribute to override i2c_adapter.class. 
  Legacy i2c drivers (in particular v4l and dvb drivers) use this class to 
  decide which adapter to bind to. dbox2 needs I2C_CLASS_TV_DIGITAL (4).

 I hadn't paid any attention to i2c_adapter.class. Now that you mention
 it I wonder why i2c has it.  It appears to be a holdover from the
 older mechanism of searching for devices and it is used as a filter to
 reduce the number of device being searched. I would think a new style
 driver could just ignore it.

 It is probably best to fix it in a successive patch.


-- 
Jon Smirl
[EMAIL PROTECTED]

___
i2c mailing list
i2c@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/i2c


[i2c] i2c_new_device is losing error codes

2008-01-21 Thread Jon Smirl
i2c_new_device() is not propagating error codes up. Callers will need
to be fixed to use PTR_ERR() to recover the errors.

struct i2c_client *
i2c_new_device(struct i2c_adapter *adap, struct i2c_board_info const *info)
{
struct i2c_client   *client;
int status;

client = kzalloc(sizeof *client, GFP_KERNEL);
if (!client)
--should return -ENOMEM;
return NULL;

client-adapter = adap;

client-dev.platform_data = info-platform_data;
device_init_wakeup(client-dev, info-flags  I2C_CLIENT_WAKE);

client-flags = info-flags  ~I2C_CLIENT_WAKE;
client-addr = info-addr;
client-irq = info-irq;

strlcpy(client-name, info-type, sizeof(client-name));

/* a new style driver may be bound to this device when we
 * return from this function, or any later moment (e.g. maybe
 * hotplugging will load the driver module).  and the device
 * refcount model is the standard driver model one.
 */
status = i2c_attach_client(client);
--error status is not propagated up
if (status  0) {
kfree(client);
client = NULL;
}
return client;
}
EXPORT_SYMBOL_GPL(i2c_new_device);


-- 
Jon Smirl
[EMAIL PROTECTED]

___
i2c mailing list
i2c@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/i2c


Re: [i2c] [PATCH 0/5] Version 17, series to add device tree naming to i2c

2008-01-11 Thread Jon Smirl
On 1/11/08, Jean Delvare [EMAIL PROTECTED] wrote:
 Hi Jon,

 On Wed, 19 Dec 2007 23:41:36 -0500, Jon Smirl wrote:
  Since copying i2c-mpc.c to maintain support for the ppc architecture seems 
  to be an issue; instead rework i2c-mpc.c to use CONFIG_PPC_MERGE #ifdefs to 
  support both the ppc and powerpc architecture. When ppc is deleted in six 
  months these #ifdefs will need to be removed.
 
  Another rework of the i2c for powerpc device tree patch. This version 
  implements standard alias naming only on the powerpc platform and only for 
  the device tree names. The old naming mechanism of 
  i2c_client.name,driver_name is left in place and not changed for 
  non-powerpc platforms. This patch is fully capable of dynamically loading 
  the i2c modules. You can modprobe in the i2c-mpc driver and the i2c modules 
  described in the device tree will be automatically loaded. Modules also 
  work if compiled in.
 
  The follow on patch to module-init-tools is also needed since the i2c 
  subsystem has never implemented dynamic loading.
 
  The following series implements standard linux module aliasing for i2c 
  modules on arch=powerpc. It then converts the mpc i2c driver from being a 
  platform driver to an open firmware one. I2C device names are picked up 
  from the device tree. Module aliasing is used to translate from device tree 
  names into to linux kernel names. Several i2c drivers are updated to use 
  the new aliasing.

 Now that I have read all the previous versions of this patch series
 and, more importantly, all objections that were raised on the way, I
 can start reviewing the latest iteration of your patches. I'll also do
 some testing, although I have no powerpc stuff here, but at least I
 want to make sure that there are no regressions introduced by your
 patches on x86.


Various people were worried about x86. Around version 15 I altered the
patches so that they only impacted PowerPC. If they impact x86 in
current form that is a bug.

When x86 is ready for it I do think dynamic module loading should be
implemented there also.

 --
 Jean Delvare



-- 
Jon Smirl
[EMAIL PROTECTED]

___
i2c mailing list
i2c@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/i2c


Re: [i2c] [PATCH 0/5] Version 17, series to add device tree naming to i2c

2008-01-11 Thread Jon Smirl
On 1/11/08, Jean Delvare [EMAIL PROTECTED] wrote:
 Secondly, it promotes OF device names as acceptable aliases. This I
 don't think I agree with. While I see some value in moving the OF name
 - Linux name translation to the drivers themselves (even though I
 don't see this as a mandatory move either), this doesn't imply that OF
 names should be used as aliases. I don't like the idea that different
 architectures will name the same device differently in a visible way.
 This could easily break user-space code that makes assumptions on the
 device names (libsensors comes to mind.) So, I think that this part
 will need some more discussion.

They're aliases.  On the x86 my e1000 Ethernet driver loads using this
alias name:
pci:v8086d1010sv*sd*bc*sc*i*
In fact, the e1000 driver has 63 alias names in addition to e1000

But it's still the e1000 driver after it is loaded.
[EMAIL PROTECTED]:/home/linux/drivers/net/e1000$ lsmod | grep e1000
e1000 115968  0

Loading a I2C driver with an OF alias name is not going to change the
module name after it is loaded. In fact, once the module is in memory
there's no way to tell what name was used to load it.

OF device names are set by the Open Firmware committee. It is not
reasonable to force the Linux names back into Open Firmware since this
would force the other operating systems using Open Firmware to adopt
the Linux names.

This issue hasn't been visible before since there was a global table
in the PowerPC code mapping all known Open Firmware names into linux
names. Keeping this as a global table doesn't scale. The mapping needs
to be done by each device individually.


 --
 Jean Delvare



-- 
Jon Smirl
[EMAIL PROTECTED]

___
i2c mailing list
i2c@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/i2c


[i2c] [PATCH 19 4/5] Convert PowerPC MPC i2c to of_platform_driver from platform_driver

2008-01-11 Thread Jon Smirl
Convert MPC i2c driver from being a platform_driver to an open firmware 
version. Error returns were improved. Routine names were changed from fsl_ to 
mpc_ to make them match the file name.

Signed-off-by: Jon Smirl [EMAIL PROTECTED]
---

 arch/powerpc/sysdev/fsl_soc.c |   96 --
 drivers/i2c/busses/i2c-mpc.c  |  183 -
 2 files changed, 180 insertions(+), 99 deletions(-)


diff --git a/arch/powerpc/sysdev/fsl_soc.c b/arch/powerpc/sysdev/fsl_soc.c
index 94e5c73..d6ef264 100644
--- a/arch/powerpc/sysdev/fsl_soc.c
+++ b/arch/powerpc/sysdev/fsl_soc.c
@@ -318,102 +318,6 @@ err:
 
 arch_initcall(gfar_of_init);
 
-#ifdef CONFIG_I2C_BOARDINFO
-#include linux/i2c.h
-
-static void __init of_register_i2c_devices(struct device_node *adap_node,
-  int bus_num)
-{
-   struct device_node *node = NULL;
-   const char *compatible;
-
-   while ((node = of_get_next_child(adap_node, node))) {
-   struct i2c_board_info info = {};
-   const u32 *addr;
-   int len;
-
-   addr = of_get_property(node, reg, len);
-   if (!addr || len  sizeof(int) || *addr  (1  10) - 1) {
-   printk(KERN_WARNING fsl_soc.c: invalid i2c device 
entry\n);
-   continue;
-   }
-
-   info.irq = irq_of_parse_and_map(node, 0);
-   if (info.irq == NO_IRQ)
-   info.irq = -1;
-
-   compatible = of_get_property(node, compatible, len);
-   if (!compatible) {
-   printk(KERN_WARNING i2c-mpc.c: invalid entry, missing 
compatible attribute\n);
-   continue;
-   }
-   strncpy(info.type, compatible, sizeof(info.type));
-
-   info.addr = *addr;
-
-   i2c_register_board_info(bus_num, info, 1);
-   }
-}
-
-static int __init fsl_i2c_of_init(void)
-{
-   struct device_node *np;
-   unsigned int i;
-   struct platform_device *i2c_dev;
-   int ret;
-
-   for (np = NULL, i = 0;
-(np = of_find_compatible_node(np, i2c, fsl-i2c)) != NULL;
-i++) {
-   struct resource r[2];
-   struct fsl_i2c_platform_data i2c_data;
-   const unsigned char *flags = NULL;
-
-   memset(r, 0, sizeof(r));
-   memset(i2c_data, 0, sizeof(i2c_data));
-
-   ret = of_address_to_resource(np, 0, r[0]);
-   if (ret)
-   goto err;
-
-   of_irq_to_resource(np, 0, r[1]);
-
-   i2c_dev = platform_device_register_simple(fsl-i2c, i, r, 2);
-   if (IS_ERR(i2c_dev)) {
-   ret = PTR_ERR(i2c_dev);
-   goto err;
-   }
-
-   i2c_data.device_flags = 0;
-   flags = of_get_property(np, dfsrr, NULL);
-   if (flags)
-   i2c_data.device_flags |= FSL_I2C_DEV_SEPARATE_DFSRR;
-
-   flags = of_get_property(np, fsl5200-clocking, NULL);
-   if (flags)
-   i2c_data.device_flags |= FSL_I2C_DEV_CLOCK_5200;
-
-   ret =
-   platform_device_add_data(i2c_dev, i2c_data,
-sizeof(struct
-   fsl_i2c_platform_data));
-   if (ret)
-   goto unreg;
-
-   of_register_i2c_devices(np, i);
-   }
-
-   return 0;
-
-unreg:
-   platform_device_unregister(i2c_dev);
-err:
-   return ret;
-}
-
-arch_initcall(fsl_i2c_of_init);
-#endif
-
 #ifdef CONFIG_PPC_83xx
 static int __init mpc83xx_wdt_init(void)
 {
diff --git a/drivers/i2c/busses/i2c-mpc.c b/drivers/i2c/busses/i2c-mpc.c
index 7c35a8f..91fa73c 100644
--- a/drivers/i2c/busses/i2c-mpc.c
+++ b/drivers/i2c/busses/i2c-mpc.c
@@ -17,7 +17,7 @@
 #include linux/module.h
 #include linux/sched.h
 #include linux/init.h
-#include linux/platform_device.h
+#include linux/of_platform.h
 
 #include asm/io.h
 #include linux/fsl_devices.h
@@ -25,13 +25,13 @@
 #include linux/interrupt.h
 #include linux/delay.h
 
-#define MPC_I2C_ADDR  0x00
+#define DRV_NAME mpc-i2c
+
 #define MPC_I2C_FDR0x04
 #define MPC_I2C_CR 0x08
 #define MPC_I2C_SR 0x0c
 #define MPC_I2C_DR 0x10
 #define MPC_I2C_DFSRR 0x14
-#define MPC_I2C_REGION 0x20
 
 #define CCR_MEN  0x80
 #define CCR_MIEN 0x40
@@ -316,6 +316,181 @@ static struct i2c_adapter mpc_ops = {
.retries = 1
 };
 
+struct i2c_driver_device {
+   char*of_device;
+   char*i2c_driver;
+   char*i2c_type;
+};
+
+#ifdef CONFIG_PPC_MERGE
+
+static void of_register_i2c_devices(struct i2c_adapter *adap, struct 
device_node *adap_node)
+{
+   struct device_node *node = NULL;
+
+   while ((node = of_get_next_child(adap_node, node))) {
+   struct

Re: [i2c] [PATCH 19 1/5] Implement module aliasing for i2c to translate from device tree names

2008-01-11 Thread Jon Smirl
Comment was wrong, I2C_OF_MODULE_PREFIX was needed. Add it back.

Implement module aliasing for i2c to translate from device tree names

This patch allows new style i2c chip drivers to have alias names using
the official kernel aliasing system and MODULE_DEVICE_TABLE(). I've
tested it on PowerPC and x86. This change is required for PowerPC
device tree support.

Signed-off-by: Jon Smirl [EMAIL PROTECTED]
---

 drivers/hwmon/f75375s.c |4 ++--
 drivers/i2c/chips/ds1682.c  |2 +-
 drivers/i2c/chips/menelaus.c|2 +-
 drivers/i2c/chips/tps65010.c|2 +-
 drivers/i2c/chips/tsl2550.c |2 +-
 drivers/i2c/i2c-core.c  |   24 +++-
 drivers/rtc/rtc-ds1307.c|2 +-
 drivers/rtc/rtc-ds1374.c|2 +-
 drivers/rtc/rtc-m41t80.c|2 +-
 drivers/rtc/rtc-rs5c372.c   |2 +-
 include/linux/i2c.h |5 ++---
 include/linux/mod_devicetable.h |   20 
 scripts/mod/file2alias.c|   12 
 13 files changed, 67 insertions(+), 14 deletions(-)


diff --git a/drivers/hwmon/f75375s.c b/drivers/hwmon/f75375s.c
index 6892f76..2247de6 100644
--- a/drivers/hwmon/f75375s.c
+++ b/drivers/hwmon/f75375s.c
@@ -117,7 +117,7 @@ struct f75375_data {
 static int f75375_attach_adapter(struct i2c_adapter *adapter);
 static int f75375_detect(struct i2c_adapter *adapter, int address, int kind);
 static int f75375_detach_client(struct i2c_client *client);
-static int f75375_probe(struct i2c_client *client);
+static int f75375_probe(struct i2c_client *client, const struct
i2c_device_id *id);
 static int f75375_remove(struct i2c_client *client);

 static struct i2c_driver f75375_legacy_driver = {
@@ -628,7 +628,7 @@ static void f75375_init(struct i2c_client *client,
struct f75375_data *data,

 }

-static int f75375_probe(struct i2c_client *client)
+static int f75375_probe(struct i2c_client *client, const struct
i2c_device_id *id)
 {
struct f75375_data *data = i2c_get_clientdata(client);
struct f75375s_platform_data *f75375s_pdata = client-dev.platform_data;
diff --git a/drivers/i2c/chips/ds1682.c b/drivers/i2c/chips/ds1682.c
index 9e94542..93c0441 100644
--- a/drivers/i2c/chips/ds1682.c
+++ b/drivers/i2c/chips/ds1682.c
@@ -200,7 +200,7 @@ static struct bin_attribute ds1682_eeprom_attr = {
 /*
  * Called when a ds1682 device is matched with this driver
  */
-static int ds1682_probe(struct i2c_client *client)
+static int ds1682_probe(struct i2c_client *client, const struct
i2c_device_id *id)
 {
int rc;

diff --git a/drivers/i2c/chips/menelaus.c b/drivers/i2c/chips/menelaus.c
index 2dea012..89ef9b6 100644
--- a/drivers/i2c/chips/menelaus.c
+++ b/drivers/i2c/chips/menelaus.c
@@ -1149,7 +1149,7 @@ static inline void menelaus_rtc_init(struct
menelaus_chip *m)

 static struct i2c_driver menelaus_i2c_driver;

-static int menelaus_probe(struct i2c_client *client)
+static int menelaus_probe(struct i2c_client *client, const struct
i2c_device_id *id)
 {
struct menelaus_chip*menelaus;
int rev = 0, val;
diff --git a/drivers/i2c/chips/tps65010.c b/drivers/i2c/chips/tps65010.c
index e320994..6b13642 100644
--- a/drivers/i2c/chips/tps65010.c
+++ b/drivers/i2c/chips/tps65010.c
@@ -465,7 +465,7 @@ static int __exit tps65010_remove(struct i2c_client *client)
return 0;
 }

-static int tps65010_probe(struct i2c_client *client)
+static int tps65010_probe(struct i2c_client *client, const struct
i2c_device_id *id)
 {
struct tps65010 *tps;
int status;
diff --git a/drivers/i2c/chips/tsl2550.c b/drivers/i2c/chips/tsl2550.c
index 3de4b19..27c553d 100644
--- a/drivers/i2c/chips/tsl2550.c
+++ b/drivers/i2c/chips/tsl2550.c
@@ -364,7 +364,7 @@ static int tsl2550_init_client(struct i2c_client *client)
  */

 static struct i2c_driver tsl2550_driver;
-static int __devinit tsl2550_probe(struct i2c_client *client)
+static int __devinit tsl2550_probe(struct i2c_client *client, const
struct i2c_device_id *id)
 {
struct i2c_adapter *adapter = to_i2c_adapter(client-dev.parent);
struct tsl2550_data *data;
diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index b5e13e4..5f62230 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -47,6 +47,19 @@ static DEFINE_IDR(i2c_adapter_idr);

 /* - */

+static const struct i2c_device_id *i2c_match_id(
+   const struct i2c_device_id *id, struct i2c_client *client)
+{
+   /* only powerpc drivers implement the id_table,
+* it is empty on other platforms */
+   while (id-name[0]) {
+   if (strcmp(client-name, id-name) == 0)
+   return id;
+   id++;
+   }
+   return NULL;
+}
+
 static int i2c_device_match(struct device *dev, struct device_driver *drv)
 {
struct i2c_client   *client = to_i2c_client(dev