10-bit address support

10-bit address support

[Jean Delvare]

Hi all,

After Sheng-Hui's fix to i2c-algo-bit, it would seem that we are almost
there with 10-bit slave address support. There's one remaining thing
that worries me though: the 7-bit and 10-bit address spaces overlap.
>From Documentation/i2c/ten-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)."

So for example, if on a given system you have two devices on I2C bus
segment 0, one responding to 7-bit address 0x2d and another responding
to 10-bit address 0x2d, they will have the same device name 0-002d,
meaning that instantiating the second one will fail.

Now I agree that this is extremely unlikely to happen, considering how
rare 10-bit addresses are in the first place. Still I think we should
fix this now, before the first 10-bit address I2C device gets supported
by the Linux kernel.

I can think of 2 different ways of addressing the problem.

First way is to use a different device name format for 10-bit address
devices, for example %d-10bit-%04x. This has the drawback that some
user-space applications and libraries may not recognize these as valid
i2c device names. libsensors and sensors-detect would be amongst these.

Second way is to add an offset to the 10-bit addresses, to avoid the
overlap. The minimum offset is 0x80, but that would translate for
example 0x2d to 0xad, which is not obviously "10-bit address 0x2d". So
I'd rather go with a larger offset such as 0x1000. This translates 0x2d
to 0x102d which is more obviously "10-bit address 0x2d". We have 16
bits to store the address so it shouldn't be an issue. Another possible
offset would be 0xa000 (as 0xa is 10.)

Given how rare 10-bit addresses are, the second option has my favors.
So my plan is to apply the following fix:

From: Jean Delvare <khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org>
Subject: i2c: Fix device name for 10-bit slave address

10-bit addresses overlap with traditional 7-bit addresses, leading in
device name collisions. Add an arbitrary offset to 10-bit addresses to
prevent this collision. The offset was chosen so that the address is
still easily recognizable.

Signed-off-by: Jean Delvare <khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org>
---
 drivers/i2c/i2c-core.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

--- linux-3.2-rc1.orig/drivers/i2c/i2c-core.c	2011-10-28 15:45:15.000000000 +0200
+++ linux-3.2-rc1/drivers/i2c/i2c-core.c	2011-11-10 14:45:05.000000000 +0100
@@ -539,8 +539,10 @@ i2c_new_device(struct i2c_adapter *adap,
 	client->dev.type = &i2c_client_type;
 	client->dev.of_node = info->of_node;
 
+	/* 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->addr | ((client->flags & I2C_CLIENT_TEN)
+				     ? 0x1000 : 0));
 	status = device_register(&client->dev);
 	if (status)
 		goto out_err;

Anyone please let me know quickly if you have any objection.

Individual bus drivers may still need to be fixed to properly support
10-bit addresses, and user-space tools as well as they currently don't
support them at all, but this can be added later on as needed.

-- 
Jean Delvare

RE: 10-bit address support

[Jeffrey (Sheng-Hui) Chu]

> -----Original Message-----
> From: Jean Delvare [mailto:khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org]
> Sent: Thursday, November 10, 2011 10:08 AM
> To: Linux I2C
> Cc: Jeffrey (Sheng-Hui) Chu
> Subject: 10-bit address support
> 
> Hi all,
> 
> After Sheng-Hui's fix to i2c-algo-bit, it would seem that we are almost
> there with 10-bit slave address support. There's one remaining thing
> that worries me though: the 7-bit and 10-bit address spaces overlap.
> From Documentation/i2c/ten-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)."

>From my understanding of the spec, there is no overlap.  
The 10-bit client at 0x10 will be addressed by the following address sequence:
0Xf0 0X10 <data-out>  or 0xf0 0x10 <restart> 0xf1<data-in>

The 7-bit client at 0x10 will be addressed by the following address sequence:
0x20 <data-out>  or 0x21 <data-in>

0xf0 is equivalent to 7-bit address of 0x78 which is a reserved address in 7-bit space.  A compliant 7-bit client should not use or respond to this address.

<snip> 
> --
> Jean Delvare

Don't need to do any workaround. IMHO

Regards,

-Jeffrey

Re: 10-bit address support

[Jean Delvare]

On Thu, 10 Nov 2011 07:58:35 -0800, Jeffrey (Sheng-Hui) Chu wrote:
> 
> 
> > -----Original Message-----
> > From: Jean Delvare [mailto:khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org]
> > Sent: Thursday, November 10, 2011 10:08 AM
> > To: Linux I2C
> > Cc: Jeffrey (Sheng-Hui) Chu
> > Subject: 10-bit address support
> > 
> > Hi all,
> > 
> > After Sheng-Hui's fix to i2c-algo-bit, it would seem that we are almost
> > there with 10-bit slave address support. There's one remaining thing
> > that worries me though: the 7-bit and 10-bit address spaces overlap.
> > From Documentation/i2c/ten-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)."
> 
> From my understanding of the spec, there is no overlap.  
> The 10-bit client at 0x10 will be addressed by the following address sequence:
> 0Xf0 0X10 <data-out>  or 0xf0 0x10 <restart> 0xf1<data-in>
> 
> The 7-bit client at 0x10 will be addressed by the following address sequence:
> 0x20 <data-out>  or 0x21 <data-in>
> 
> 0xf0 is equivalent to 7-bit address of 0x78 which is a reserved address in 7-bit space.  A compliant 7-bit client should not use or respond to this address.

what you wrote above is completely exact, but is also completely
unrelated to what I explained. The overlapping that currently exists is
in the device names inside the kernel. It's a pure software issue.

> Don't need to do any workaround. IMHO

I'm certain we do.

-- 
Jean Delvare

RE: 10-bit address support

[Jeffrey (Sheng-Hui) Chu]

> -----Original Message-----
> From: Jean Delvare [mailto:khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org]
> Sent: Thursday, November 10, 2011 11:33 AM
> To: Jeffrey (Sheng-Hui) Chu
> Cc: Linux I2C
> Subject: Re: 10-bit address support
> 
> On Thu, 10 Nov 2011 07:58:35 -0800, Jeffrey (Sheng-Hui) Chu wrote:
> >
> >
> > > -----Original Message-----
> > > From: Jean Delvare [mailto:khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org]
> > > Sent: Thursday, November 10, 2011 10:08 AM
> > > To: Linux I2C
> > > Cc: Jeffrey (Sheng-Hui) Chu
> > > Subject: 10-bit address support
> > >
> > > Hi all,
> > >
> > > After Sheng-Hui's fix to i2c-algo-bit, it would seem that we are almost
> > > there with 10-bit slave address support. There's one remaining thing
> > > that worries me though: the 7-bit and 10-bit address spaces overlap.
> > > From Documentation/i2c/ten-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)."
> >
> > From my understanding of the spec, there is no overlap.
> > The 10-bit client at 0x10 will be addressed by the following address
> sequence:
> > 0Xf0 0X10 <data-out>  or 0xf0 0x10 <restart> 0xf1<data-in>
> >
> > The 7-bit client at 0x10 will be addressed by the following address
> sequence:
> > 0x20 <data-out>  or 0x21 <data-in>
> >
> > 0xf0 is equivalent to 7-bit address of 0x78 which is a reserved address in 7-
> bit space.  A compliant 7-bit client should not use or respond to this address.
> 
> what you wrote above is completely exact, but is also completely
> unrelated to what I explained. The overlapping that currently exists is
> in the device names inside the kernel. It's a pure software issue.
> 
> > Don't need to do any workaround. IMHO
> 
> I'm certain we do.
> 
> --
> Jean Delvare
You're right.  It's my misunderstanding.

Regards,

-Jeffrey

Re: 10-bit address support

[Wolfram Sang]

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

Hi Jean,

> I can think of 2 different ways of addressing the problem.
> 
> First way is to use a different device name format for 10-bit address
> devices, for example %d-10bit-%04x. This has the drawback that some
> user-space applications and libraries may not recognize these as valid
> i2c device names. libsensors and sensors-detect would be amongst these.

Wouldn't the cleanest solution be

"%d-%02x" for 7 bit
"%d-%04x" for 10 bit?

Yeah, I know that would really break userspace.

> I'd rather go with a larger offset such as 0x1000. This translates 0x2d
> to 0x102d which is more obviously "10-bit address 0x2d". We have 16
> bits to store the address so it shouldn't be an issue. Another possible
> offset would be 0xa000 (as 0xa is 10.)

I like 0xa000 a tad better than 0x1000, but well... Then again, those
devices are probably rare enough to take a non-intrusive approach.

Regards,

   Wolfram

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

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

Re: 10-bit address support

[Jean Delvare]

Hi Wolfram,

On Fri, 11 Nov 2011 11:43:35 +0100, Wolfram Sang wrote:
> > I can think of 2 different ways of addressing the problem.
> > 
> > First way is to use a different device name format for 10-bit address
> > devices, for example %d-10bit-%04x. This has the drawback that some
> > user-space applications and libraries may not recognize these as valid
> > i2c device names. libsensors and sensors-detect would be amongst these.
> 
> Wouldn't the cleanest solution be
> 
> "%d-%02x" for 7 bit
> "%d-%04x" for 10 bit?

I'd rather use %03x for 10-bit then, for consistency.

> Yeah, I know that would really break userspace.

Yes indeed. While it wouldn't break libsensors (which uses sscanf
"%hd-%x" to parse the fields), it would break at least sensors-detect,
which really expects 4 hex digits in the sysfs name. This would
definitely be a change in the user-space interface anyway, who knows
which other tools would be affected.

Your solution was elegant (although it has its own drawbacks, such as
leaving it to every library and application to come up with a creative
way to handle and store 10-bit addresses separately from 7-bit addresses
internally), but unfortunately it would have had to be implemented in
the early days, not 8 years later.

> > I'd rather go with a larger offset such as 0x1000. This translates 0x2d
> > to 0x102d which is more obviously "10-bit address 0x2d". We have 16
> > bits to store the address so it shouldn't be an issue. Another possible
> > offset would be 0xa000 (as 0xa is 10.)
> 
> I like 0xa000 a tad better than 0x1000, but well... Then again, those
> devices are probably rare enough to take a non-intrusive approach.

Yes I agree, we really want to leave 7-bit addresses unaffected by the
change.

0xa000 is not more intrusive than 0x1000, so if the majority - i.e.
you ;) - is in favor of this, that's fine with me. I'll send a patch
later today.

Thanks,
-- 
Jean Delvare

Re: 10-bit address support

[Wolfram Sang]

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

Hi Jean,

> > Wouldn't the cleanest solution be
> > 
> > "%d-%02x" for 7 bit
> > "%d-%04x" for 10 bit?
> 
> I'd rather use %03x for 10-bit then, for consistency.

Yup, I realized this a few hours later, too. This would leave the
possibility to add true 16-bit addressing of the next to be i2c standard
;)

> internally), but unfortunately it would have had to be implemented in
> the early days, not 8 years later.

Yes, and hopefully we can live with this drawback well enough.

> 0xa000 is not more intrusive than 0x1000, so if the majority - i.e.
> you ;) - is in favor of this, that's fine with me. I'll send a patch
> later today.

You can already add my:

Acked-by: Wolfram Sang <w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

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

Re: 10-bit address support

[Jean Delvare]

On Wed, 16 Nov 2011 16:04:49 +0100, Wolfram Sang wrote:
> Hi Jean,
> 
> > > Wouldn't the cleanest solution be
> > > 
> > > "%d-%02x" for 7 bit
> > > "%d-%04x" for 10 bit?
> > 
> > I'd rather use %03x for 10-bit then, for consistency.
> 
> Yup, I realized this a few hours later, too. This would leave the
> possibility to add true 16-bit addressing of the next to be i2c standard
> ;)

Given the fiasco 10-bit address support was, I doubt we'll see any
attempt to further extend the address space. The protocol penalty of
10-bit addressing is heavy and 7-bit addresses are often sufficient in
practice. Where they are not, designers have resorted to using muxes
and switches rather than 10-bit addresses.

> > internally), but unfortunately it would have had to be implemented in
> > the early days, not 8 years later.
> 
> Yes, and hopefully we can live with this drawback well enough.
> 
> > 0xa000 is not more intrusive than 0x1000, so if the majority - i.e.
> > you ;) - is in favor of this, that's fine with me. I'll send a patch
> > later today.
> 
> You can already add my:
> 
> Acked-by: Wolfram Sang <w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>

Thanks,
-- 
Jean Delvare