* [PATCH 5/6]: i2c: Add bus addressing support.
@ 2008-08-21 9:43 David Miller
[not found] ` <20080821.024330.51639001.davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
0 siblings, 1 reply; 5+ messages in thread
From: David Miller @ 2008-08-21 9:43 UTC (permalink / raw)
To: i2c-GZX6beZjE8VD60Wz+7aTrA
i2c: Add bus addressing support.
Some I2C bus controllers support the driving of multiple I2C bus
segments (think PCI domains).
For example, the pcf8584 variants on some sparc64 boxes can do this.
They have an auxiliary 8-bit register that specifies the I2C bus each
I2C operation acts upon. In the openfirmware device tree, the I2C
client devices are described using an 8-bit bus address and a 10-bit
I2C device address.
In practice only really small numbers are used for the I2C bus
numbers, such as "0" and "1". So we don't need to really represent
the full 8-bits.
Adding this support is a little bit tricky, because we can't just add
a "bus" member to struct i2c_client, struct i2c_msg, etc. because
some of these structures are exported to userspace for the sake of the
i2c-dev driver interface.
Since we encode the I2C addresses in a 16-bit quantity, we steal the
top 6 bits for the bus number.
This works out because all current code will generate addresses with
those top 6-bits clear.
We add a new functionality bit, I2C_FUNC_BUS_ADDRESSING, that the
algorithm provider uses to indicate that it can support bus addressing.
If we see a non-zero bus address, we make sure the adapter can support
it.
Signed-off-by: David S. Miller <davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index 550853f..3c0b5af 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -807,6 +807,10 @@ static int __i2c_check_addr(struct device *dev, void *addrp)
static int i2c_check_addr(struct i2c_adapter *adapter, int addr)
{
+ if (I2C_ADDR_GET_BUS(addr) != 0 &&
+ !i2c_check_functionality(adapter, I2C_FUNC_BUS_ADDRESSING))
+ return -EOPNOTSUPP;
+
return device_for_each_child(&adapter->dev, &addr, __i2c_check_addr);
}
diff --git a/include/linux/i2c.h b/include/linux/i2c.h
index 08be0d2..c93561e 100644
--- a/include/linux/i2c.h
+++ b/include/linux/i2c.h
@@ -169,6 +169,31 @@ struct i2c_driver {
};
#define to_i2c_driver(d) container_of(d, struct i2c_driver, driver)
+/* In order to add bus addressing to the I2C layer without changing
+ * the layout of structures such as i2c_msg (for the sake of
+ * userspace), we encode the bus number into the top 6 bits of the
+ * address value.
+ *
+ * This allows existing userspace and drivers to keep working (the bus
+ * will be zero), and bus addressing support can be gradually added.
+ *
+ * Algorithm providers must indicate they support bus addressing via
+ * i2c_algorithm->functionality(). If they don't, the I2C layer will
+ * reject any attempt to attach, probe, or detect a device with a bus
+ * number other than zero.
+ */
+#define I2C_ADDR_ADDR_MASK 0x03ff
+#define I2C_ADDR_ADDR_SHIFT 0
+#define I2C_ADDR_BUS_MASK 0xfc00
+#define I2C_ADDR_BUS_SHIFT 10
+#define I2C_ADDR_GET_ADDR(addr) \
+ (((addr) & I2C_ADDR_ADDR_MASK) >> I2C_ADDR_ADDR_SHIFT)
+#define I2C_ADDR_GET_BUS(addr) \
+ (((addr) & I2C_ADDR_BUS_MASK) >> I2C_ADDR_BUS_SHIFT)
+#define I2C_ADDR_ENCODE(bus, addr) \
+ (((bus) << I2C_ADDR_BUS_SHIFT) | \
+ ((addr) << I2C_ADDR_ADDR_SHIFT))
+
/**
* struct i2c_client - represent an I2C slave device
* @flags: I2C_CLIENT_TEN indicates the device uses a ten bit chip address;
@@ -531,6 +556,7 @@ struct i2c_msg {
#define I2C_FUNC_SMBUS_WRITE_I2C_BLOCK 0x08000000 /* w/ 1-byte reg. addr. */
#define I2C_FUNC_SMBUS_READ_I2C_BLOCK_2 0x10000000 /* I2C-like block xfer */
#define I2C_FUNC_SMBUS_WRITE_I2C_BLOCK_2 0x20000000 /* w/ 2-byte reg. addr. */
+#define I2C_FUNC_BUS_ADDRESSING 0x40000000
#define I2C_FUNC_SMBUS_BYTE (I2C_FUNC_SMBUS_READ_BYTE | \
I2C_FUNC_SMBUS_WRITE_BYTE)
--
1.5.6.5.GIT
_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 5/6]: i2c: Add bus addressing support.
[not found] ` <20080821.024330.51639001.davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
@ 2008-08-21 12:56 ` Peter Korsgaard
2008-10-15 12:52 ` Jean Delvare
1 sibling, 0 replies; 5+ messages in thread
From: Peter Korsgaard @ 2008-08-21 12:56 UTC (permalink / raw)
To: David Miller; +Cc: i2c-GZX6beZjE8VD60Wz+7aTrA
>>>>> "David" == David Miller <davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org> writes:
David> i2c: Add bus addressing support.
David> Some I2C bus controllers support the driving of multiple I2C bus
David> segments (think PCI domains).
David> For example, the pcf8584 variants on some sparc64 boxes can do this.
David> They have an auxiliary 8-bit register that specifies the I2C bus each
David> I2C operation acts upon. In the openfirmware device tree, the I2C
David> client devices are described using an 8-bit bus address and a 10-bit
David> I2C device address.
So it's like a bus multiplexer on the SDA / SCL lines? Wouldn't that
be better modelled like multiple I2C adapters talking to the same
chip?
I do something similar on a few embedded platforms with a multiplexer
driver that attaches to an I2C adapter and registers virtual I2C
adapters for each MUX position. This way the multiplexing is
independent of the I2C adapter implementation, but you could ofcause
combine it in a single driver - E.G.:
/*
* i2c-bcccpmux.c: I2C multiplexer for Barco CCCP boards
*
* Peter Korsgaard <peter.korsgaard-ob4gmnvZ1/cAvxtiuMwx3w@public.gmane.org>
*
* This file is licensed under the terms of the GNU General Public License
* version 2. This program is licensed "as is" without any warranty of any
* kind, whether express or implied.
*/
#include <linux/errno.h>
#include <linux/init.h>
#include <linux/module.h>
#include <linux/platform_device.h>
#include <linux/i2c.h>
#include <linux/i2c-bcccpmux.h>
#include <asm/io.h>
#include <asm/uaccess.h>
struct bcccpmux_i2c {
u16 __iomem *base;
struct i2c_adapter *parent;
struct i2c_adapter *adap;
struct bcccpmux_i2c_platform_data data;
};
static int bcccpmux_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs,
int num)
{
struct bcccpmux_i2c *i2c = i2c_get_adapdata(adap);
int nr = adap->nr - i2c->data.base_nr;
u16 val;
int ret;
mutex_lock(&i2c->parent->bus_lock);
val = in_be16(i2c->base) & i2c->data.mask;
out_be16(i2c->base, val | i2c->data.values[nr]);
ret = i2c->parent->algo->master_xfer(i2c->parent, msgs, num);
out_be16(i2c->base, val | i2c->data.idle);
mutex_unlock(&i2c->parent->bus_lock);
return ret;
}
static u32 bcccpmux_func(struct i2c_adapter *adap)
{
struct bcccpmux_i2c *i2c = i2c_get_adapdata(adap);
return i2c->parent->algo->functionality(i2c->parent);
}
static struct i2c_algorithm bcccpmux_algorithm = {
.master_xfer = bcccpmux_xfer,
.functionality = bcccpmux_func,
};
static struct i2c_adapter bcccpmux_adapter = {
.owner = THIS_MODULE,
.name = "bcccpmux",
.class = I2C_CLASS_HWMON,
.algo = &bcccpmux_algorithm,
.timeout = 2,
.retries = 1
};
static int __devinit bcccpmux_probe(struct platform_device *pdev)
{
struct bcccpmux_i2c *i2c;
struct bcccpmux_i2c_platform_data *pdata;
struct i2c_adapter *adap;
struct resource *res;
u16 __iomem *base;
int i, j, ret;
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
if (!res)
return -ENODEV;
pdata = pdev->dev.platform_data;
if (!pdata) {
dev_err(&pdev->dev, "Missing platform data\n");
return -ENODEV;
}
adap = i2c_get_adapter(pdata->parent);
if (!adap) {
dev_err(&pdev->dev, "Parent adapter (%d) not found\n",
pdata->parent);
return -ENODEV;
}
base = ioremap(res->start, res->end - res->start + 1);
if (!base) {
dev_err(&pdev->dev, "Unable to map registers\n");
ret = -EIO;
goto map_failed;
}
i2c = kzalloc(sizeof(*i2c), GFP_KERNEL);
if (!i2c) {
ret = -ENOMEM;
goto alloc_failed;
}
i2c->adap = kzalloc(sizeof(struct i2c_adapter)*pdata->busses,
GFP_KERNEL);
if (!i2c->adap) {
ret = -ENOMEM;
goto alloc_failed2;
}
i2c->base = base;
i2c->parent = adap;
i2c->data = *pdata;
for (i=0; i<pdata->busses; i++) {
i2c->adap[i] = bcccpmux_adapter;
i2c->adap[i].dev.parent = &adap->dev;
i2c->adap[i].nr = i + pdata->base_nr;
snprintf(i2c->adap[i].name, I2C_NAME_SIZE, "%s.%d",
bcccpmux_adapter.name, i);
i2c_set_adapdata(&i2c->adap[i], i2c);
ret = i2c_add_numbered_adapter(&i2c->adap[i]);
if (ret) {
dev_err(&pdev->dev, "Failed to add adapter %d\n", i);
goto add_adapter_failed;
}
}
/* disable parent bus so probes won't find devices on it */
out_be16(base, pdata->idle);
dev_info(&pdev->dev, "%d port mux at 0x%lx on %s adapter\n",
pdata->busses, (unsigned long)res->start, adap->name);
platform_set_drvdata(pdev, i2c);
return 0;
add_adapter_failed:
for (j=0; j<i; j++)
i2c_del_adapter(&i2c->adap[j]);
kfree(i2c->adap);
alloc_failed2:
kfree(i2c);
alloc_failed:
iounmap(base);
map_failed:
i2c_put_adapter(adap);
return ret;
}
static int __devexit bcccpmux_remove(struct platform_device *pdev)
{
struct bcccpmux_i2c *i2c = platform_get_drvdata(pdev);
int i;
for (i=0; i<i2c->data.busses; i++)
i2c_del_adapter(&i2c->adap[i]);
iounmap(i2c->base);
platform_set_drvdata(pdev, NULL);
i2c_put_adapter(i2c->parent);
kfree(i2c->adap);
kfree(i2c);
return 0;
}
static struct platform_driver bcccpmux_driver = {
.probe = bcccpmux_probe,
.remove = __devexit_p(bcccpmux_remove),
.driver = {
.owner = THIS_MODULE,
.name = "bcccpi2cmux",
},
};
static int __init bcccpmux_init(void)
{
return platform_driver_register(&bcccpmux_driver);
}
static void __exit bcccpmux_exit(void)
{
platform_driver_unregister(&bcccpmux_driver);
}
module_init(bcccpmux_init);
module_exit(bcccpmux_exit);
MODULE_DESCRIPTION("Barco CCCP I2C multiplexer driver");
MODULE_AUTHOR("Peter Korsgaard <peter.korsgaard-ob4gmnvZ1/cAvxtiuMwx3w@public.gmane.org>");
MODULE_LICENSE("GPL");
--
Bye, Peter Korsgaard
_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 5/6]: i2c: Add bus addressing support.
[not found] ` <20080821.024330.51639001.davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
2008-08-21 12:56 ` Peter Korsgaard
@ 2008-10-15 12:52 ` Jean Delvare
[not found] ` <20081015145228.14b1ae77-ig7AzVSIIG7kN2dkZ6Wm7A@public.gmane.org>
1 sibling, 1 reply; 5+ messages in thread
From: Jean Delvare @ 2008-10-15 12:52 UTC (permalink / raw)
To: David Miller; +Cc: i2c-GZX6beZjE8VD60Wz+7aTrA
Hi David,
On Thu, 21 Aug 2008 02:43:30 -0700 (PDT), David Miller wrote:
>
> i2c: Add bus addressing support.
>
> Some I2C bus controllers support the driving of multiple I2C bus
> segments (think PCI domains).
>
> For example, the pcf8584 variants on some sparc64 boxes can do this.
> They have an auxiliary 8-bit register that specifies the I2C bus each
> I2C operation acts upon. In the openfirmware device tree, the I2C
> client devices are described using an 8-bit bus address and a 10-bit
> I2C device address.
Can you please point me to the part of the PCF8454 datasheet which
explains this? I can't find it. As I read the datasheet, the PCF8454
supports a single I2C bus.
>
> In practice only really small numbers are used for the I2C bus
> numbers, such as "0" and "1". So we don't need to really represent
> the full 8-bits.
>
> Adding this support is a little bit tricky, because we can't just add
> a "bus" member to struct i2c_client, struct i2c_msg, etc. because
> some of these structures are exported to userspace for the sake of the
> i2c-dev driver interface.
>
> Since we encode the I2C addresses in a 16-bit quantity, we steal the
> top 6 bits for the bus number.
>
> This works out because all current code will generate addresses with
> those top 6-bits clear.
Oh my, holy crap! I can't believe you dared to propose this.
(Note that we may actually need one of these 6 unused bits someday, to
properly support 10-bit addressing. Currently it isn't possible to have
a 7-bit address chip and a 10-bit address chip using the same address
number on a given bus, while this is physically allowed.)
>
> We add a new functionality bit, I2C_FUNC_BUS_ADDRESSING, that the
> algorithm provider uses to indicate that it can support bus addressing.
>
> If we see a non-zero bus address, we make sure the adapter can support
> it.
I don't like this at all. This really looks like a custom hack to solve
the problem at hand without paying too much attention to compatibility
issues which I am certain will arise. Just to mention the first one
that comes to mind: access from user-space through i2c-dev, and
specifically i2c-tools.
What makes your setup fundamentally different from a motherboard with
more than one I2C bus (which is a rather common case)? Why don't you
simply register one i2c_adapter for each i2c bus segment?
>
> Signed-off-by: David S. Miller <davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
>
> diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
> index 550853f..3c0b5af 100644
> --- a/drivers/i2c/i2c-core.c
> +++ b/drivers/i2c/i2c-core.c
> @@ -807,6 +807,10 @@ static int __i2c_check_addr(struct device *dev, void *addrp)
>
> static int i2c_check_addr(struct i2c_adapter *adapter, int addr)
> {
> + if (I2C_ADDR_GET_BUS(addr) != 0 &&
> + !i2c_check_functionality(adapter, I2C_FUNC_BUS_ADDRESSING))
> + return -EOPNOTSUPP;
> +
> return device_for_each_child(&adapter->dev, &addr, __i2c_check_addr);
> }
>
> diff --git a/include/linux/i2c.h b/include/linux/i2c.h
> index 08be0d2..c93561e 100644
> --- a/include/linux/i2c.h
> +++ b/include/linux/i2c.h
> @@ -169,6 +169,31 @@ struct i2c_driver {
> };
> #define to_i2c_driver(d) container_of(d, struct i2c_driver, driver)
>
> +/* In order to add bus addressing to the I2C layer without changing
> + * the layout of structures such as i2c_msg (for the sake of
> + * userspace), we encode the bus number into the top 6 bits of the
> + * address value.
> + *
> + * This allows existing userspace and drivers to keep working (the bus
> + * will be zero), and bus addressing support can be gradually added.
> + *
> + * Algorithm providers must indicate they support bus addressing via
> + * i2c_algorithm->functionality(). If they don't, the I2C layer will
> + * reject any attempt to attach, probe, or detect a device with a bus
> + * number other than zero.
> + */
> +#define I2C_ADDR_ADDR_MASK 0x03ff
> +#define I2C_ADDR_ADDR_SHIFT 0
> +#define I2C_ADDR_BUS_MASK 0xfc00
> +#define I2C_ADDR_BUS_SHIFT 10
> +#define I2C_ADDR_GET_ADDR(addr) \
> + (((addr) & I2C_ADDR_ADDR_MASK) >> I2C_ADDR_ADDR_SHIFT)
> +#define I2C_ADDR_GET_BUS(addr) \
> + (((addr) & I2C_ADDR_BUS_MASK) >> I2C_ADDR_BUS_SHIFT)
> +#define I2C_ADDR_ENCODE(bus, addr) \
> + (((bus) << I2C_ADDR_BUS_SHIFT) | \
> + ((addr) << I2C_ADDR_ADDR_SHIFT))
> +
> /**
> * struct i2c_client - represent an I2C slave device
> * @flags: I2C_CLIENT_TEN indicates the device uses a ten bit chip address;
> @@ -531,6 +556,7 @@ struct i2c_msg {
> #define I2C_FUNC_SMBUS_WRITE_I2C_BLOCK 0x08000000 /* w/ 1-byte reg. addr. */
> #define I2C_FUNC_SMBUS_READ_I2C_BLOCK_2 0x10000000 /* I2C-like block xfer */
> #define I2C_FUNC_SMBUS_WRITE_I2C_BLOCK_2 0x20000000 /* w/ 2-byte reg. addr. */
> +#define I2C_FUNC_BUS_ADDRESSING 0x40000000
>
> #define I2C_FUNC_SMBUS_BYTE (I2C_FUNC_SMBUS_READ_BYTE | \
> I2C_FUNC_SMBUS_WRITE_BYTE)
--
Jean Delvare
_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 5/6]: i2c: Add bus addressing support.
[not found] ` <20081015145228.14b1ae77-ig7AzVSIIG7kN2dkZ6Wm7A@public.gmane.org>
@ 2008-10-15 21:37 ` David Miller
[not found] ` <20081015.143722.260594637.davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
0 siblings, 1 reply; 5+ messages in thread
From: David Miller @ 2008-10-15 21:37 UTC (permalink / raw)
To: khali-PUYAD+kWke1g9hUCZPvPmw; +Cc: i2c-GZX6beZjE8VD60Wz+7aTrA
From: Jean Delvare <khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org>
Date: Wed, 15 Oct 2008 14:52:28 +0200
> Hi David,
>
> On Thu, 21 Aug 2008 02:43:30 -0700 (PDT), David Miller wrote:
> >
> > i2c: Add bus addressing support.
> >
> > Some I2C bus controllers support the driving of multiple I2C bus
> > segments (think PCI domains).
> >
> > For example, the pcf8584 variants on some sparc64 boxes can do this.
> > They have an auxiliary 8-bit register that specifies the I2C bus each
> > I2C operation acts upon. In the openfirmware device tree, the I2C
> > client devices are described using an 8-bit bus address and a 10-bit
> > I2C device address.
>
> Can you please point me to the part of the PCF8454 datasheet which
> explains this? I can't find it. As I read the datasheet, the PCF8454
> supports a single I2C bus.
It's a feature Sun added to their PCF chips. The register address
is forwarded to an internal MUX they implemented which implements
the register selecting the I2C bus.
> Oh my, holy crap! I can't believe you dared to propose this.
Propose an alternative solution that actually works and preserves
compatability, as mine does.
> I don't like this at all. This really looks like a custom hack to solve
> the problem at hand without paying too much attention to compatibility
> issues which I am certain will arise. Just to mention the first one
> that comes to mind: access from user-space through i2c-dev, and
> specifically i2c-tools.
I got that working, read the patch my patch works, I was careful
to keep things working.
> What makes your setup fundamentally different from a motherboard with
> more than one I2C bus (which is a rather common case)? Why don't you
> simply register one i2c_adapter for each i2c bus segment?
It doesn't work like that. All the bit banging goes through the
one I2C controller, the bus it is accessing is simply changed behind
it's back by that special register.
_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 5/6]: i2c: Add bus addressing support.
[not found] ` <20081015.143722.260594637.davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
@ 2008-10-17 15:24 ` Jean Delvare
0 siblings, 0 replies; 5+ messages in thread
From: Jean Delvare @ 2008-10-17 15:24 UTC (permalink / raw)
To: David Miller; +Cc: David Brownell, i2c-GZX6beZjE8VD60Wz+7aTrA
Hi David,
On Wed, 15 Oct 2008 14:37:22 -0700 (PDT), David Miller wrote:
> From: Jean Delvare <khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org>
> Date: Wed, 15 Oct 2008 14:52:28 +0200
>
> > Hi David,
> >
> > On Thu, 21 Aug 2008 02:43:30 -0700 (PDT), David Miller wrote:
> > >
> > > i2c: Add bus addressing support.
> > >
> > > Some I2C bus controllers support the driving of multiple I2C bus
> > > segments (think PCI domains).
> > >
> > > For example, the pcf8584 variants on some sparc64 boxes can do this.
> > > They have an auxiliary 8-bit register that specifies the I2C bus each
> > > I2C operation acts upon. In the openfirmware device tree, the I2C
> > > client devices are described using an 8-bit bus address and a 10-bit
> > > I2C device address.
> >
> > Can you please point me to the part of the PCF8454 datasheet which
> > explains this? I can't find it. As I read the datasheet, the PCF8454
> > supports a single I2C bus.
>
> It's a feature Sun added to their PCF chips. The register address
> is forwarded to an internal MUX they implemented which implements
> the register selecting the I2C bus.
I don't really follow you here. The PCF8584 is a chip manufactured by
Philips. I don't quite see how Sun could have added registers to a chip
after the fact. Or do they produce compatible chips for their own
systems, with extra features?
More likely, they combined the PCF8584 with another chip which is
multiplexing the I2C bus. Philips has many such chips, for example the
PCA9540 series. It probably doesn't really matter for the problem at
hand anyway, but I'm curious.
> > Oh my, holy crap! I can't believe you dared to propose this.
>
> Propose an alternative solution that actually works and preserves
> compatability, as mine does.
What you need is i2c bus multiplexing support. What you call "bus
addressing" is only the most simple form of multiplexing, i.e. a single
level of multiplexing and no trunk.
A generic form of i2c bus multiplexing is not upstream yet but a number
of people have been working on this in the last few years, in
particular Rodolfo Giometti (Cc'd.) I admit I did not pay too much
attention to the patches myself, due to a lack of time and personal
interest, as well as the feeling that it was preferable to first
cleanup i2c-core from its legacy binding model and redundant lists and
locks before adding something as complex as bus multiplexing support.
Multiplexing will require clever locking and it did not seem sane to
attempt to do that as long as legacy and new-style i2c devices coexist.
But the cleanup phase of i2c-core is hopefully coming to an end so it
will soon be time to look into i2c bus multiplexing support.
In the meantime, it is always possible to write hardware-specific
multiplexing drivers. See for example i2c-amd756-s4882 and
i2c-nforce2-s4985, two drivers that handle the multiplexing of SMBus on
Tyan motherboards. While these drivers aren't exactly elegant and we do
not want such drivers to proliferate, they at least fill the gap until
we have a good generic solution. And these two drivers did not require
any change to i2c-core as I recall, except maybe bug fixes which the
new use case revealed.
> > I don't like this at all. This really looks like a custom hack to solve
> > the problem at hand without paying too much attention to compatibility
> > issues which I am certain will arise. Just to mention the first one
> > that comes to mind: access from user-space through i2c-dev, and
> > specifically i2c-tools.
>
> I got that working, read the patch my patch works, I was careful
> to keep things working.
How do you access your "addressed buses" through i2c-dev? It will
reject any address larger than 0x3ff. And even if you did update
i2c-dev to no longer check for "wrong" addresses, i2c-tools would not
support them. So you'd also need to update i2c-tools.
Now maybe you don't plan to use i2c-dev and i2c-tools on your I2C
buses, but that's hardly the point. The point is that your proposal
requires changes to other drivers and user-space tools, while the
alternative doesn't require such changes.
Even i2c-core has checks on invalid addresses, in
i2c_new_probed_device(), i2c_detect_address() and i2c_probe_address().
While we don't care much about the latter (it's going away soon) the
former 2 would need to be updated. Then again you may say that you do
not plan to use these functions on your I2C buses, but this illustrates
how your proposal would require more changes to be universally usable.
> > What makes your setup fundamentally different from a motherboard with
> > more than one I2C bus (which is a rather common case)? Why don't you
> > simply register one i2c_adapter for each i2c bus segment?
>
> It doesn't work like that. All the bit banging goes through the
> one I2C controller, the bus it is accessing is simply changed behind
> it's back by that special register.
That's the definition of bus multiplexing. You apparently think that
struct i2c_adapter represents one I2C controller (and admittedly that's
what the name suggests for historical reasons) while what it really
represents is an I2C bus segment. In your case you have one controller
for several I2C bus segments. So you simply need as many i2c_adapter
structures registered, and then it's up to your driver to handle the
multiplexing in a way that is transparent for the users.
If you want to see generic i2c bus multiplexing support available soon,
then please help Rodolfo by reviewing and testing his patches, and help
me finish the conversion of all i2c chip drivers to the standard device
driver binding model. 87 drivers have been converted or deleted
already, most by myself, 27 are remaining. An up-to-date list is at:
http://jdelvare.pck.nerim.net/linux/legacy_i2c_driver.list
As you can see, most remaining drivers are in drivers/macintosh and
drivers/media/video. I can probably handle media/video myself with the
help of Hans Verkuil but I have no idea what to do with the macintosh
drivers.
Thanks,
--
Jean Delvare
_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2008-10-17 15:24 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-08-21 9:43 [PATCH 5/6]: i2c: Add bus addressing support David Miller
[not found] ` <20080821.024330.51639001.davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
2008-08-21 12:56 ` Peter Korsgaard
2008-10-15 12:52 ` Jean Delvare
[not found] ` <20081015145228.14b1ae77-ig7AzVSIIG7kN2dkZ6Wm7A@public.gmane.org>
2008-10-15 21:37 ` David Miller
[not found] ` <20081015.143722.260594637.davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
2008-10-17 15:24 ` Jean Delvare
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox