[PATCH 00/10] i2c: improve i2c client address spaces and their DT support

[PATCH 00/10] i2c: improve i2c client address spaces and their DT support

[Wolfram Sang]

Here is the updated RFC series ready for submission. This gives the i2c core
seperate address spaces for standard clients, 10 bit clients, and our own slave
clients. So, you can now have a 7 bit slave at 0x50 and a 10 bit slave at
0x050. Or, you can have a slave driver listening at some address and at the
same time have a client driver talking to this address. Note that this is only
the core support for that separation, I am still not sure if there is hardware
being able talking to its own slave address, but we will see. The RFC had DT
support, this series also has support for that when instantiating via sysfs at
runtime.

Changes since RFC:

* dropped the patches for Tegra slave support. I couldn't get them to work
  and we found out that they need further changes. Tests have now been
  performed with a Renesas Lager board
* the hardcoded arbitrary offsets have now been #defined. Other than that,
  patches 1-7 which also have been in the RFC stayed the same.
* patch 8 adds support for the new flags in sysfs
* patch 9 gives some extra warning for users in case of a misconfiguration
* patch 10 finally introduces a binding documentation for generic i2c
  bindings. Finally, at last, hooray!
* tags from Andrey and Stephen have been added, thanks a lot!

Please comment, review...

Thanks,

   Wolfram


Wolfram Sang (10):
  dt-bindings: add header for generic I2C flags in bindings
  i2c: add a flag to mark clients as slaves
  i2c: apply address offset for slaves, too
  i2c: rename address check functions
  i2c: make address check indpendent from client struct
  i2c: apply DT flags when probing
  i2c: take address space into account when checking for used addresses
  i2c: support 10 bit and slave addresses in sysfs 'new_device'
  i2c: slave: print warning if slave flag not set
  i2c: dt: describe generic bindings

 Documentation/devicetree/bindings/i2c/i2c.txt | 33 ++++++++++
 Documentation/i2c/slave-interface             |  9 ++-
 Documentation/i2c/ten-bit-addresses           |  4 ++
 drivers/i2c/i2c-core.c                        | 88 ++++++++++++++++++++-------
 include/dt-bindings/i2c/i2c.h                 | 18 ++++++
 include/linux/i2c.h                           |  9 +--
 6 files changed, 131 insertions(+), 30 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/i2c/i2c.txt
 create mode 100644 include/dt-bindings/i2c/i2c.h

-- 
2.1.4

[PATCH 01/10] dt-bindings: add header for generic I2C flags in bindings

[Wolfram Sang]

From: Wolfram Sang <wsa+renesas@sang-engineering.com>

Tested-by: Andrey Danin <danindrey@mail.ru>
Acked-by: Stephen Warren <swarren@nvidia.com>
Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 include/dt-bindings/i2c/i2c.h | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)
 create mode 100644 include/dt-bindings/i2c/i2c.h

diff --git a/include/dt-bindings/i2c/i2c.h b/include/dt-bindings/i2c/i2c.h
new file mode 100644
index 00000000000000..1d5da81d90f144
--- /dev/null
+++ b/include/dt-bindings/i2c/i2c.h
@@ -0,0 +1,18 @@
+/*
+ * This header provides constants for I2C bindings
+ *
+ * Copyright (C) 2015 by Sang Engineering
+ * Copyright (C) 2015 by Renesas Electronics Corporation
+ *
+ * Wolfram Sang <wsa@sang-engineering.com>
+ *
+ * GPLv2 only
+ */
+
+#ifndef _DT_BINDINGS_I2C_I2C_H
+#define _DT_BINDINGS_I2C_I2C_H
+
+#define I2C_TEN_BIT_ADDRESS	(1 << 31)
+#define I2C_OWN_SLAVE_ADDRESS	(1 << 30)
+
+#endif
-- 
2.1.4

[PATCH 02/10] i2c: add a flag to mark clients as slaves

[Wolfram Sang]

From: Wolfram Sang <wsa+renesas@sang-engineering.com>

And update indentation with one more tab, sigh...

Tested-by: Andrey Danin <danindrey@mail.ru>
Acked-by: Stephen Warren <swarren@nvidia.com>
Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 include/linux/i2c.h | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/include/linux/i2c.h b/include/linux/i2c.h
index e83a738a3b8741..51ed2e40932f3d 100644
--- a/include/linux/i2c.h
+++ b/include/linux/i2c.h
@@ -550,11 +550,12 @@ void i2c_lock_adapter(struct i2c_adapter *);
 void i2c_unlock_adapter(struct i2c_adapter *);
 
 /*flags for the client struct: */
-#define I2C_CLIENT_PEC	0x04		/* Use Packet Error Checking */
-#define I2C_CLIENT_TEN	0x10		/* we have a ten bit chip address */
+#define I2C_CLIENT_PEC		0x04	/* Use Packet Error Checking */
+#define I2C_CLIENT_TEN		0x10	/* we have a ten bit chip address */
 					/* Must equal I2C_M_TEN below */
-#define I2C_CLIENT_WAKE	0x80		/* for board_info; true iff can wake */
-#define I2C_CLIENT_SCCB	0x9000		/* Use Omnivision SCCB protocol */
+#define I2C_CLIENT_SLAVE	0x20	/* we are the slave */
+#define I2C_CLIENT_WAKE		0x80	/* for board_info; true iff can wake */
+#define I2C_CLIENT_SCCB		0x9000	/* Use Omnivision SCCB protocol */
 					/* Must match I2C_M_STOP|IGNORE_NAK */
 
 /* i2c adapter classes (bitmask) */
-- 
2.1.4

[PATCH 04/10] i2c: rename address check functions

[Wolfram Sang]

From: Wolfram Sang <wsa+renesas@sang-engineering.com>

The current naming is based on the arguments of the functions and not on
what they do. Even I as the maintainer find this confusing, so let's
rename them to something more descriptive.

Tested-by: Andrey Danin <danindrey@mail.ru>
Acked-by: Stephen Warren <swarren@nvidia.com>
Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 drivers/i2c/i2c-core.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index 150ee00b5ff5cb..28766b7208bcc2 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -796,7 +796,7 @@ static unsigned short i2c_encode_flags_to_addr(struct i2c_client *client)
 
 /* This is a permissive address validity check, I2C address map constraints
  * are purposely not enforced, except for the general call address. */
-static int i2c_check_client_addr_validity(const struct i2c_client *client)
+static int i2c_check_addr_validity(const struct i2c_client *client)
 {
 	if (client->flags & I2C_CLIENT_TEN) {
 		/* 10-bit address, all values are valid */
@@ -814,7 +814,7 @@ static int i2c_check_client_addr_validity(const struct i2c_client *client)
  * device uses a reserved address, then it shouldn't be probed. 7-bit
  * addressing is assumed, 10-bit address devices are rare and should be
  * explicitly enumerated. */
-static int i2c_check_addr_validity(unsigned short addr)
+static int i2c_check_7bit_addr_validity_strict(unsigned short addr)
 {
 	/*
 	 * Reserved addresses per I2C specification:
@@ -983,7 +983,7 @@ i2c_new_device(struct i2c_adapter *adap, struct i2c_board_info const *info)
 	strlcpy(client->name, info->type, sizeof(client->name));
 
 	/* Check for address validity */
-	status = i2c_check_client_addr_validity(client);
+	status = i2c_check_addr_validity(client);
 	if (status) {
 		dev_err(&adap->dev, "Invalid %d-bit I2C address 0x%02hx\n",
 			client->flags & I2C_CLIENT_TEN ? 10 : 7, client->addr);
@@ -2268,7 +2268,7 @@ static int i2c_detect_address(struct i2c_client *temp_client,
 	int err;
 
 	/* Make sure the address is valid */
-	err = i2c_check_addr_validity(addr);
+	err = i2c_check_7bit_addr_validity_strict(addr);
 	if (err) {
 		dev_warn(&adapter->dev, "Invalid probe address 0x%02x\n",
 			 addr);
@@ -2385,7 +2385,7 @@ i2c_new_probed_device(struct i2c_adapter *adap,
 
 	for (i = 0; addr_list[i] != I2C_CLIENT_END; i++) {
 		/* Check address validity */
-		if (i2c_check_addr_validity(addr_list[i]) < 0) {
+		if (i2c_check_7bit_addr_validity_strict(addr_list[i]) < 0) {
 			dev_warn(&adap->dev, "Invalid 7-bit address "
 				 "0x%02x\n", addr_list[i]);
 			continue;
@@ -2960,7 +2960,7 @@ int i2c_slave_register(struct i2c_client *client, i2c_slave_cb_t slave_cb)
 
 	if (!(client->flags & I2C_CLIENT_TEN)) {
 		/* Enforce stricter address checking */
-		ret = i2c_check_addr_validity(client->addr);
+		ret = i2c_check_7bit_addr_validity_strict(client->addr);
 		if (ret) {
 			dev_err(&client->dev, "%s: invalid address\n", __func__);
 			return ret;
-- 
2.1.4

[PATCH 05/10] i2c: make address check indpendent from client struct

[Wolfram Sang]

From: Wolfram Sang <wsa+renesas@sang-engineering.com>

We want to use this function with struct boardinfo soon, so let's just
pass the parameters really needed. We also extend the type of addr, so
more types can be input. Remove a superfluous dangling comment while
here.

Tested-by: Andrey Danin <danindrey@mail.ru>
Acked-by: Stephen Warren <swarren@nvidia.com>
Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 drivers/i2c/i2c-core.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index 28766b7208bcc2..4a83d39a5aabe2 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -796,15 +796,15 @@ static unsigned short i2c_encode_flags_to_addr(struct i2c_client *client)
 
 /* This is a permissive address validity check, I2C address map constraints
  * are purposely not enforced, except for the general call address. */
-static int i2c_check_addr_validity(const struct i2c_client *client)
+static int i2c_check_addr_validity(unsigned addr, unsigned short flags)
 {
-	if (client->flags & I2C_CLIENT_TEN) {
+	if (flags & I2C_CLIENT_TEN) {
 		/* 10-bit address, all values are valid */
-		if (client->addr > 0x3ff)
+		if (addr > 0x3ff)
 			return -EINVAL;
 	} else {
 		/* 7-bit address, reject the general call address */
-		if (client->addr == 0x00 || client->addr > 0x7f)
+		if (addr == 0x00 || addr > 0x7f)
 			return -EINVAL;
 	}
 	return 0;
@@ -982,8 +982,7 @@ i2c_new_device(struct i2c_adapter *adap, struct i2c_board_info const *info)
 
 	strlcpy(client->name, info->type, sizeof(client->name));
 
-	/* Check for address validity */
-	status = i2c_check_addr_validity(client);
+	status = i2c_check_addr_validity(client->addr, client->flags);
 	if (status) {
 		dev_err(&adap->dev, "Invalid %d-bit I2C address 0x%02hx\n",
 			client->flags & I2C_CLIENT_TEN ? 10 : 7, client->addr);
-- 
2.1.4

[PATCH 06/10] i2c: apply DT flags when probing

[Wolfram Sang]

From: Wolfram Sang <wsa+renesas@sang-engineering.com>

Check for slave and 10-bit flags when probing and mark the client when
found. Improve the address validity check, too

Tested-by: Andrey Danin <danindrey@mail.ru>
Acked-by: Stephen Warren <swarren@nvidia.com>
Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 drivers/i2c/i2c-core.c | 23 ++++++++++++++++++-----
 1 file changed, 18 insertions(+), 5 deletions(-)

diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index 4a83d39a5aabe2..7638eb1a5998f8 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -27,6 +27,7 @@
    I2C slave support (c) 2014 by Wolfram Sang <wsa@sang-engineering.com>
  */
 
+#include <dt-bindings/i2c/i2c.h>
 #include <linux/module.h>
 #include <linux/kernel.h>
 #include <linux/delay.h>
@@ -1286,7 +1287,8 @@ static struct i2c_client *of_i2c_register_device(struct i2c_adapter *adap,
 	struct i2c_client *result;
 	struct i2c_board_info info = {};
 	struct dev_archdata dev_ad = {};
-	const __be32 *addr;
+	const __be32 *addr_be;
+	u32 addr;
 	int len;
 
 	dev_dbg(&adap->dev, "of_i2c: register %s\n", node->full_name);
@@ -1297,20 +1299,31 @@ static struct i2c_client *of_i2c_register_device(struct i2c_adapter *adap,
 		return ERR_PTR(-EINVAL);
 	}
 
-	addr = of_get_property(node, "reg", &len);
-	if (!addr || (len < sizeof(*addr))) {
+	addr_be = of_get_property(node, "reg", &len);
+	if (!addr_be || (len < sizeof(*addr_be))) {
 		dev_err(&adap->dev, "of_i2c: invalid reg on %s\n",
 			node->full_name);
 		return ERR_PTR(-EINVAL);
 	}
 
-	info.addr = be32_to_cpup(addr);
-	if (info.addr > (1 << 10) - 1) {
+	addr = be32_to_cpup(addr_be);
+	if (addr & I2C_TEN_BIT_ADDRESS) {
+		addr &= ~I2C_TEN_BIT_ADDRESS;
+		info.flags |= I2C_CLIENT_TEN;
+	}
+
+	if (addr & I2C_OWN_SLAVE_ADDRESS) {
+		addr &= ~I2C_OWN_SLAVE_ADDRESS;
+		info.flags |= I2C_CLIENT_SLAVE;
+	}
+
+	if (i2c_check_addr_validity(addr, info.flags)) {
 		dev_err(&adap->dev, "of_i2c: invalid addr=%x on %s\n",
 			info.addr, node->full_name);
 		return ERR_PTR(-EINVAL);
 	}
 
+	info.addr = addr;
 	info.of_node = of_node_get(node);
 	info.archdata = &dev_ad;
 
-- 
2.1.4

[PATCH 07/10] i2c: take address space into account when checking for used addresses

[Wolfram Sang]

From: Wolfram Sang <wsa+renesas@sang-engineering.com>

It is not enough to compare the plain address value, we also need to
check the flags enabling a different address space. E.g. it is valid to
have address 0x50 as a 7-bit address and 0x050 as 10-bit address on the
same bus. Same for addresses when we are the slave.

Tested-by: Andrey Danin <danindrey@mail.ru>
Acked-by: Stephen Warren <swarren@nvidia.com>
Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 drivers/i2c/i2c-core.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index 7638eb1a5998f8..24c96dd0a5bee4 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -837,7 +837,7 @@ static int __i2c_check_addr_busy(struct device *dev, void *addrp)
 	struct i2c_client	*client = i2c_verify_client(dev);
 	int			addr = *(int *)addrp;
 
-	if (client && client->addr == addr)
+	if (client && i2c_encode_flags_to_addr(client) == addr)
 		return -EBUSY;
 	return 0;
 }
@@ -991,7 +991,7 @@ i2c_new_device(struct i2c_adapter *adap, struct i2c_board_info const *info)
 	}
 
 	/* Check for address business */
-	status = i2c_check_addr_busy(adap, client->addr);
+	status = i2c_check_addr_busy(adap, i2c_encode_flags_to_addr(client));
 	if (status)
 		goto out_err;
 
@@ -2287,7 +2287,7 @@ static int i2c_detect_address(struct i2c_client *temp_client,
 		return err;
 	}
 
-	/* Skip if already in use */
+	/* Skip if already in use (7 bit, no need to encode flags) */
 	if (i2c_check_addr_busy(adapter, addr))
 		return 0;
 
@@ -2403,7 +2403,7 @@ i2c_new_probed_device(struct i2c_adapter *adap,
 			continue;
 		}
 
-		/* Check address availability */
+		/* Check address availability (7 bit, no need to encode flags) */
 		if (i2c_check_addr_busy(adap, addr_list[i])) {
 			dev_dbg(&adap->dev, "Address 0x%02x already in "
 				"use, not probing\n", addr_list[i]);
-- 
2.1.4

[PATCH 08/10] i2c: support 10 bit and slave addresses in sysfs 'new_device'

[Wolfram Sang]

From: Wolfram Sang <wsa+renesas@sang-engineering.com>

We now have seperate address spaces for 10 bit and we-are-slave clients.
Update the sysfs device instantiation method to support these types by
accepting the address offsets that are assigned to the extra address
spaces. Update the documentation, too.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 Documentation/i2c/slave-interface   |  9 ++++++---
 Documentation/i2c/ten-bit-addresses |  4 ++++
 drivers/i2c/i2c-core.c              | 12 +++++++++++-
 3 files changed, 21 insertions(+), 4 deletions(-)

diff --git a/Documentation/i2c/slave-interface b/Documentation/i2c/slave-interface
index 2dee4e2d62df19..61ed05cd95317f 100644
--- a/Documentation/i2c/slave-interface
+++ b/Documentation/i2c/slave-interface
@@ -31,10 +31,13 @@ User manual
 ===========
 
 I2C slave backends behave like standard I2C clients. So, you can instantiate
-them as described in the document 'instantiating-devices'. A quick example for
-instantiating the slave-eeprom driver from userspace at address 0x64 on bus 1:
+them as described in the document 'instantiating-devices'. The only difference
+is that i2c slave backends have their own address space. So, you have to add
+0x1000 to the address you would originally request. An example for
+instantiating the slave-eeprom driver from userspace at the 7 bit address 0x64
+on bus 1:
 
-  # echo slave-24c02 0x64 > /sys/bus/i2c/devices/i2c-1/new_device
+  # echo slave-24c02 0x1064 > /sys/bus/i2c/devices/i2c-1/new_device
 
 Each backend should come with separate documentation to describe its specific
 behaviour and setup.
diff --git a/Documentation/i2c/ten-bit-addresses b/Documentation/i2c/ten-bit-addresses
index cdfe13901b99cb..7b2d11e53a49f1 100644
--- a/Documentation/i2c/ten-bit-addresses
+++ b/Documentation/i2c/ten-bit-addresses
@@ -2,6 +2,10 @@ The I2C protocol knows about two kinds of device addresses: normal 7 bit
 addresses, and an extended set of 10 bit addresses. The sets of addresses
 do not intersect: the 7 bit address 0x10 is not the same as the 10 bit
 address 0x10 (though a single device could respond to both of them).
+To avoid ambiguity, the user sees 10 bit addresses mapped to a different
+address space, namely 0xa000-0xa3ff. The leading 0xa (= 10) represents the
+10 bit mode. This is used for creating device names in sysfs. It is also
+needed when instantiating 10 bit devices via the new_device file in sysfs.
 
 I2C messages to and from 10-bit address devices have a different format.
 See the I2C specification for the details.
diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index 24c96dd0a5bee4..4c1f31f039964b 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -1156,6 +1156,16 @@ i2c_sysfs_new_device(struct device *dev, struct device_attribute *attr,
 		return -EINVAL;
 	}
 
+	if ((info.addr & I2C_ADDR_OFFSET_TEN_BIT) == I2C_ADDR_OFFSET_TEN_BIT) {
+		info.addr &= ~I2C_ADDR_OFFSET_TEN_BIT;
+		info.flags |= I2C_CLIENT_TEN;
+	}
+
+	if (info.addr & I2C_ADDR_OFFSET_SLAVE) {
+		info.addr &= ~I2C_ADDR_OFFSET_SLAVE;
+		info.flags |= I2C_CLIENT_SLAVE;
+	}
+
 	client = i2c_new_device(adap, &info);
 	if (!client)
 		return -EINVAL;
@@ -1207,7 +1217,7 @@ i2c_sysfs_delete_device(struct device *dev, struct device_attribute *attr,
 			  i2c_adapter_depth(adap));
 	list_for_each_entry_safe(client, next, &adap->userspace_clients,
 				 detected) {
-		if (client->addr == addr) {
+		if (i2c_encode_flags_to_addr(client) == addr) {
 			dev_info(dev, "%s: Deleting device %s at 0x%02hx\n",
 				 "delete_device", client->name, client->addr);
 
-- 
2.1.4

Re: [PATCH 08/10] i2c: support 10 bit and slave addresses in sysfs 'new_device'

[Geert Uytterhoeven]

Hi Wolfram,

On Sat, Aug 8, 2015 at 10:33 PM, Wolfram Sang <wsa@the-dreams.de> wrote:
> diff --git a/Documentation/i2c/slave-interface b/Documentation/i2c/slave-interface
> index 2dee4e2d62df19..61ed05cd95317f 100644
> --- a/Documentation/i2c/slave-interface
> +++ b/Documentation/i2c/slave-interface
> @@ -31,10 +31,13 @@ User manual
>  ===========
>
>  I2C slave backends behave like standard I2C clients. So, you can instantiate
> -them as described in the document 'instantiating-devices'. A quick example for
> -instantiating the slave-eeprom driver from userspace at address 0x64 on bus 1:
> +them as described in the document 'instantiating-devices'. The only difference
> +is that i2c slave backends have their own address space. So, you have to add
> +0x1000 to the address you would originally request. An example for
> +instantiating the slave-eeprom driver from userspace at the 7 bit address 0x64
> +on bus 1:
>
> -  # echo slave-24c02 0x64 > /sys/bus/i2c/devices/i2c-1/new_device
> +  # echo slave-24c02 0x1064 > /sys/bus/i2c/devices/i2c-1/new_device

Does 0x64 still work? It's an ABI since v4.1.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 08/10] i2c: support 10 bit and slave addresses in sysfs 'new_device'

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 1297 bytes --]

On Mon, Aug 10, 2015 at 03:17:03PM +0200, Geert Uytterhoeven wrote:
> Hi Wolfram,
> 
> On Sat, Aug 8, 2015 at 10:33 PM, Wolfram Sang <wsa@the-dreams.de> wrote:
> > diff --git a/Documentation/i2c/slave-interface b/Documentation/i2c/slave-interface
> > index 2dee4e2d62df19..61ed05cd95317f 100644
> > --- a/Documentation/i2c/slave-interface
> > +++ b/Documentation/i2c/slave-interface
> > @@ -31,10 +31,13 @@ User manual
> >  ===========
> >
> >  I2C slave backends behave like standard I2C clients. So, you can instantiate
> > -them as described in the document 'instantiating-devices'. A quick example for
> > -instantiating the slave-eeprom driver from userspace at address 0x64 on bus 1:
> > +them as described in the document 'instantiating-devices'. The only difference
> > +is that i2c slave backends have their own address space. So, you have to add
> > +0x1000 to the address you would originally request. An example for
> > +instantiating the slave-eeprom driver from userspace at the 7 bit address 0x64
> > +on bus 1:
> >
> > -  # echo slave-24c02 0x64 > /sys/bus/i2c/devices/i2c-1/new_device
> > +  # echo slave-24c02 0x1064 > /sys/bus/i2c/devices/i2c-1/new_device
> 
> Does 0x64 still work? It's an ABI since v4.1.

Yes, you'll see the warning from patch 9.


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[PATCH 03/10] i2c: apply address offset for slaves, too

[Wolfram Sang]

From: Wolfram Sang <wsa+renesas-jBu1N2QxHDJrcw3mvpCnnVaTQe2KTcn/@public.gmane.org>

We want a separate address range for being an I2C slave. Add an offset
of 0x1000, so it can be combined with ten bit addresses as well. Add a
separate function to create the address value, we will need it later in
other places.

Tested-by: Andrey Danin <danindrey-JGs/UdohzUI@public.gmane.org>
Acked-by: Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
Signed-off-by: Wolfram Sang <wsa+renesas-jBu1N2QxHDJrcw3mvpCnnVaTQe2KTcn/@public.gmane.org>
---
 drivers/i2c/i2c-core.c | 22 +++++++++++++++++++---
 1 file changed, 19 insertions(+), 3 deletions(-)

diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index e6d4935161e490..150ee00b5ff5cb 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -57,6 +57,9 @@
 #define CREATE_TRACE_POINTS
 #include <trace/events/i2c.h>
 
+#define I2C_ADDR_OFFSET_TEN_BIT	0xa000
+#define I2C_ADDR_OFFSET_SLAVE	0x1000
+
 /* core_lock protects i2c_adapter_idr, and guarantees
    that device detection, deletion of detected devices, and attach_adapter
    calls are serialized */
@@ -776,6 +779,21 @@ struct i2c_client *i2c_verify_client(struct device *dev)
 EXPORT_SYMBOL(i2c_verify_client);
 
 
+/* Return a unique address which takes the flags of the client into account */
+static unsigned short i2c_encode_flags_to_addr(struct i2c_client *client)
+{
+	unsigned short addr = client->addr;
+
+	/* For some client flags, add an arbitrary offset to avoid collisions */
+	if (client->flags & I2C_CLIENT_TEN)
+		addr |= I2C_ADDR_OFFSET_TEN_BIT;
+
+	if (client->flags & I2C_CLIENT_SLAVE)
+		addr |= I2C_ADDR_OFFSET_SLAVE;
+
+	return addr;
+}
+
 /* This is a permissive address validity check, I2C address map constraints
  * are purposely not enforced, except for the general call address. */
 static int i2c_check_client_addr_validity(const struct i2c_client *client)
@@ -921,10 +939,8 @@ static void i2c_dev_set_name(struct i2c_adapter *adap,
 		return;
 	}
 
-	/* For 10-bit clients, add an arbitrary offset to avoid collisions */
 	dev_set_name(&client->dev, "%d-%04x", i2c_adapter_id(adap),
-		     client->addr | ((client->flags & I2C_CLIENT_TEN)
-				     ? 0xa000 : 0));
+		     i2c_encode_flags_to_addr(client));
 }
 
 /**
-- 
2.1.4

[PATCH 09/10] i2c: slave: print warning if slave flag not set

[Wolfram Sang]

From: Wolfram Sang <wsa+renesas-jBu1N2QxHDJrcw3mvpCnnVaTQe2KTcn/@public.gmane.org>

Address collisions will be rare, but we should let the user know that
slaves have their own address space nonetheless.

Signed-off-by: Wolfram Sang <wsa+renesas-jBu1N2QxHDJrcw3mvpCnnVaTQe2KTcn/@public.gmane.org>
---
 drivers/i2c/i2c-core.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index 4c1f31f039964b..4d143fe7c3eab3 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -2980,6 +2980,10 @@ int i2c_slave_register(struct i2c_client *client, i2c_slave_cb_t slave_cb)
 		return -EINVAL;
 	}
 
+	if (!(client->flags & I2C_CLIENT_SLAVE))
+		dev_warn(&client->dev, "%s: client slave flag not set. You might see address collisions\n",
+			 __func__);
+
 	if (!(client->flags & I2C_CLIENT_TEN)) {
 		/* Enforce stricter address checking */
 		ret = i2c_check_7bit_addr_validity_strict(client->addr);
-- 
2.1.4

[PATCH 10/10] i2c: dt: describe generic bindings

[Wolfram Sang]

From: Wolfram Sang <wsa+renesas@sang-engineering.com>

Start a new file which describes the generic bindings used for I2C with
device tree. So we have a central place to look for them, increase
visibility of them, and hopefully reduce the amount of custom properties
introduced.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 Documentation/devicetree/bindings/i2c/i2c.txt | 33 +++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/i2c/i2c.txt

diff --git a/Documentation/devicetree/bindings/i2c/i2c.txt b/Documentation/devicetree/bindings/i2c/i2c.txt
new file mode 100644
index 00000000000000..1175efed4a41b5
--- /dev/null
+++ b/Documentation/devicetree/bindings/i2c/i2c.txt
@@ -0,0 +1,33 @@
+Generic device tree bindings for I2C busses
+===========================================
+
+This document describes generic bindings which can be used to describe I2C
+busses in a device tree.
+
+Required properties
+-------------------
+
+- #address-cells  - should be <1>. Read more about addresses below.
+- #size-cells     - should be <0>.
+- compatible      - name of I2C bus controller following generic names
+		    recommended practice.
+
+For other required properties e.g. to describe register sets, interrupts,
+clocks, etc. check the binding documentation of the specific driver.
+
+The cells properties above define that an address of children of an I2C bus
+are described by a single value. This is usually a 7 bit address. However,
+flags can be attached to the address. I2C_TEN_BIT_ADDRESS is used to mark a 10
+bit address. It is needed to avoid the ambiguity between e.g. a 7 bit address
+of 0x50 and a 10 bit address of 0x050 which, in theory, can be on the same bus.
+Another flag is I2C_OWN_SLAVE_ADDRESS to mark addresses on which we listen to
+be devices ourselves.
+
+Optional properties
+-------------------
+
+These properties may not be supported by all drivers. However, if a driver
+wants to support one of the below features, it should adapt the bindings below.
+
+- clock-frequency	- frequency of bus clock in Hz
+- wakeup-source		- device can be used as a wakeup source.
-- 
2.1.4

Re: [PATCH 10/10] i2c: dt: describe generic bindings

[Vaibhav Hiremath]

On Sunday 09 August 2015 02:03 AM, Wolfram Sang wrote:
> From: Wolfram Sang <wsa+renesas@sang-engineering.com>
>
> Start a new file which describes the generic bindings used for I2C with
> device tree. So we have a central place to look for them, increase
> visibility of them, and hopefully reduce the amount of custom properties
> introduced.
>
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> ---
>   Documentation/devicetree/bindings/i2c/i2c.txt | 33 +++++++++++++++++++++++++++
>   1 file changed, 33 insertions(+)
>   create mode 100644 Documentation/devicetree/bindings/i2c/i2c.txt
>
> diff --git a/Documentation/devicetree/bindings/i2c/i2c.txt b/Documentation/devicetree/bindings/i2c/i2c.txt
> new file mode 100644
> index 00000000000000..1175efed4a41b5
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/i2c/i2c.txt
> @@ -0,0 +1,33 @@
> +Generic device tree bindings for I2C busses
> +===========================================
> +
> +This document describes generic bindings which can be used to describe I2C
> +busses in a device tree.
> +
> +Required properties
> +-------------------
> +
> +- #address-cells  - should be <1>. Read more about addresses below.
> +- #size-cells     - should be <0>.
> +- compatible      - name of I2C bus controller following generic names
> +		    recommended practice.
> +
> +For other required properties e.g. to describe register sets, interrupts,
> +clocks, etc. check the binding documentation of the specific driver.
> +
> +The cells properties above define that an address of children of an I2C bus
> +are described by a single value. This is usually a 7 bit address. However,
> +flags can be attached to the address. I2C_TEN_BIT_ADDRESS is used to mark a 10
> +bit address. It is needed to avoid the ambiguity between e.g. a 7 bit address
> +of 0x50 and a 10 bit address of 0x050 which, in theory, can be on the same bus.
> +Another flag is I2C_OWN_SLAVE_ADDRESS to mark addresses on which we listen to
> +be devices ourselves.
> +
> +Optional properties
> +-------------------
> +
> +These properties may not be supported by all drivers. However, if a driver
> +wants to support one of the below features, it should adapt the bindings below.
> +
> +- clock-frequency	- frequency of bus clock in Hz
> +- wakeup-source		- device can be used as a wakeup source.
>

Thanks for the patch and it looks good to me.

Shouldn't we also update/add other generic optional properties?
For example,

  - i2c-sclk-low-time-ns
  - i2c-sclk-high-time-ns
  - etc...


We need to consolidate all such generic properties in here.

Thanks,
Vaibhav

Re: [PATCH 10/10] i2c: dt: describe generic bindings

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 393 bytes --]


> Thanks for the patch and it looks good to me.

Thanks, I interpret this as an Acked-by :)

> Shouldn't we also update/add other generic optional properties?
> For example,
> 
>  - i2c-sclk-low-time-ns
>  - i2c-sclk-high-time-ns
>  - etc...

Yes, I will do this as an incremental patch once people are happy with
this start. Review is usually easier if done in small chunks.


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 10/10] i2c: dt: describe generic bindings

[Vaibhav Hiremath]

On Sunday 09 August 2015 05:45 PM, Wolfram Sang wrote:
>
>> Thanks for the patch and it looks good to me.
>
> Thanks, I interpret this as an Acked-by :)
>
>> Shouldn't we also update/add other generic optional properties?
>> For example,
>>
>>   - i2c-sclk-low-time-ns
>>   - i2c-sclk-high-time-ns
>>   - etc...
>
> Yes, I will do this as an incremental patch once people are happy with
> this start. Review is usually easier if done in small chunks.
>

Agreed.

Feel free to add

Reviewed-by: Vaibhav Hiremath <vaibhav.hiremath@linaro.org>

Thanks,
Vaibhav

Re: [PATCH 00/10] i2c: improve i2c client address spaces and their DT support

[Wolfram Sang]

On Sat, Aug 08, 2015 at 10:33:17PM +0200, Wolfram Sang wrote:
> Here is the updated RFC series ready for submission. This gives the i2c core
> seperate address spaces for standard clients, 10 bit clients, and our own slave
> clients. So, you can now have a 7 bit slave at 0x50 and a 10 bit slave at
> 0x050. Or, you can have a slave driver listening at some address and at the
> same time have a client driver talking to this address. Note that this is only
> the core support for that separation, I am still not sure if there is hardware
> being able talking to its own slave address, but we will see. The RFC had DT
> support, this series also has support for that when instantiating via sysfs at
> runtime.
> 
> Changes since RFC:
> 
> * dropped the patches for Tegra slave support. I couldn't get them to work
>   and we found out that they need further changes. Tests have now been
>   performed with a Renesas Lager board
> * the hardcoded arbitrary offsets have now been #defined. Other than that,
>   patches 1-7 which also have been in the RFC stayed the same.
> * patch 8 adds support for the new flags in sysfs
> * patch 9 gives some extra warning for users in case of a misconfiguration
> * patch 10 finally introduces a binding documentation for generic i2c
>   bindings. Finally, at last, hooray!
> * tags from Andrey and Stephen have been added, thanks a lot!
> 
> Please comment, review...
> 
> Thanks,
> 
>    Wolfram
> 
> 
> Wolfram Sang (10):
>   dt-bindings: add header for generic I2C flags in bindings
>   i2c: add a flag to mark clients as slaves
>   i2c: apply address offset for slaves, too
>   i2c: rename address check functions
>   i2c: make address check indpendent from client struct
>   i2c: apply DT flags when probing
>   i2c: take address space into account when checking for used addresses
>   i2c: support 10 bit and slave addresses in sysfs 'new_device'
>   i2c: slave: print warning if slave flag not set
>   i2c: dt: describe generic bindings
> 
>  Documentation/devicetree/bindings/i2c/i2c.txt | 33 ++++++++++
>  Documentation/i2c/slave-interface             |  9 ++-
>  Documentation/i2c/ten-bit-addresses           |  4 ++

Applied to for-next, thanks!