| Message ID | 20131117185624.GA23639@roeck-us.net |
|---|---|
| State | Superseded |
| Headers | show |
Hi Guenter, On Sun, 17 Nov 2013 10:56:24 -0800, Guenter Roeck wrote: > I managed to do it without changing the API; see patch below. > This results in > > i2c-0/name:MPC adapter at 0xfff703000 > i2c-1/name:MPC adapter at 0xfff703100 > i2c-10/name:i2c-2-mux-75 (chan_id 7) > i2c-11/name:i2c-2-mux-76 (chan_id 0) > i2c-12/name:i2c-2-mux-76 (chan_id 1) > i2c-13/name:i2c-2-mux-76 (chan_id 2) > i2c-14/name:i2c-2-mux-76 (chan_id 3) > i2c-15/name:i2c-2-mux-76 (chan_id 4) > i2c-16/name:i2c-2-mux-76 (chan_id 5) > i2c-17/name:i2c-2-mux-76 (chan_id 6) > i2c-18/name:i2c-2-mux-76 (chan_id 7) > i2c-19/name:i2c-0-mux (chan_id 1) > i2c-2/name:i2c-0-mux (chan_id 0) > i2c-20/name:i2c-0-mux (chan_id 2) > i2c-21/name:i2c-0-mux (chan_id 3) > i2c-22/name:i2c-0-mux (chan_id 4) > i2c-23/name:i2c-0-mux (chan_id 5) > i2c-24/name:i2c-0-mux (chan_id 6) > i2c-25/name:i2c-0-mux (chan_id 7) > i2c-3/name:i2c-2-mux-75 (chan_id 0) > i2c-4/name:i2c-2-mux-75 (chan_id 1) > i2c-5/name:i2c-2-mux-75 (chan_id 2) > i2c-6/name:i2c-2-mux-75 (chan_id 3) > i2c-7/name:i2c-2-mux-75 (chan_id 4) > i2c-8/name:i2c-2-mux-75 (chan_id 5) > i2c-9/name:i2c-2-mux-75 (chan_id 6) > > This retains the old mux name if the mux is not an i2c device, and adds > the i2c device address if it is. This solves the problem for me. > > Comments ? This "for me" worries me a bit. You basically restored the uniqueness for the multiple I2C-based multiplexer case. But for multiple GPIO-based multiplexers, for example, the name confusion still exists. If we are going to address this problem, I believe we should do so in a way which doesn't need revisiting in a couple months. You are changing adapter names people or tools may be relying on, we really don't want to do this too often. We could use the first GPIO pin number as the unique identifier for the GPIO case. However I2C addresses and GPIO pin numbers could collide... So we'd need separate name spaces, too. I agree this makes things even more complicated than they are right now, but I can't think of any other way that would be both robust and durable.
On 11/17/2013 12:41 PM, Jean Delvare wrote: > Hi Guenter, > > On Sun, 17 Nov 2013 10:56:24 -0800, Guenter Roeck wrote: >> I managed to do it without changing the API; see patch below. >> This results in >> >> i2c-0/name:MPC adapter at 0xfff703000 >> i2c-1/name:MPC adapter at 0xfff703100 >> i2c-10/name:i2c-2-mux-75 (chan_id 7) >> i2c-11/name:i2c-2-mux-76 (chan_id 0) >> i2c-12/name:i2c-2-mux-76 (chan_id 1) >> i2c-13/name:i2c-2-mux-76 (chan_id 2) >> i2c-14/name:i2c-2-mux-76 (chan_id 3) >> i2c-15/name:i2c-2-mux-76 (chan_id 4) >> i2c-16/name:i2c-2-mux-76 (chan_id 5) >> i2c-17/name:i2c-2-mux-76 (chan_id 6) >> i2c-18/name:i2c-2-mux-76 (chan_id 7) >> i2c-19/name:i2c-0-mux (chan_id 1) >> i2c-2/name:i2c-0-mux (chan_id 0) >> i2c-20/name:i2c-0-mux (chan_id 2) >> i2c-21/name:i2c-0-mux (chan_id 3) >> i2c-22/name:i2c-0-mux (chan_id 4) >> i2c-23/name:i2c-0-mux (chan_id 5) >> i2c-24/name:i2c-0-mux (chan_id 6) >> i2c-25/name:i2c-0-mux (chan_id 7) >> i2c-3/name:i2c-2-mux-75 (chan_id 0) >> i2c-4/name:i2c-2-mux-75 (chan_id 1) >> i2c-5/name:i2c-2-mux-75 (chan_id 2) >> i2c-6/name:i2c-2-mux-75 (chan_id 3) >> i2c-7/name:i2c-2-mux-75 (chan_id 4) >> i2c-8/name:i2c-2-mux-75 (chan_id 5) >> i2c-9/name:i2c-2-mux-75 (chan_id 6) >> >> This retains the old mux name if the mux is not an i2c device, and adds >> the i2c device address if it is. This solves the problem for me. >> >> Comments ? > > This "for me" worries me a bit. You basically restored the uniqueness > for the multiple I2C-based multiplexer case. But for multiple > GPIO-based multiplexers, for example, the name confusion still exists. Only if some genius connects multiple separate gpio based multiplexers to the same bus. In practice, though, that can be modeled as single gpio mux with more pins, so I didn't think that is that much of a problem. > If we are going to address this problem, I believe we should do so in a > way which doesn't need revisiting in a couple months. You are changing > adapter names people or tools may be relying on, we really don't want > to do this too often. > Good point. One possible solution might be to add the mux type into the name. i2c-2-mux-i2c-76 (chan_id 0) i2c-2-mux-gpio-77 (chan_id 0) We could add an additional parameter to the API, such as const char *qualifier to provide the "i2c-76" or "gpio-77" string. If the qualifier is NULL, the name would revert to the old naming scheme. Would that make sense ? Thanks, Guenter > We could use the first GPIO pin number as the unique identifier for the > GPIO case. However I2C addresses and GPIO pin numbers could collide... > So we'd need separate name spaces, too. > > I agree this makes things even more complicated than they are right > now, but I can't think of any other way that would be both robust and > durable. > -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Guenter, On Sun, 17 Nov 2013 13:13:43 -0800, Guenter Roeck wrote: > On 11/17/2013 12:41 PM, Jean Delvare wrote: > > On Sun, 17 Nov 2013 10:56:24 -0800, Guenter Roeck wrote: > >> This retains the old mux name if the mux is not an i2c device, and adds > >> the i2c device address if it is. This solves the problem for me. > >> > >> Comments ? > > > > This "for me" worries me a bit. You basically restored the uniqueness > > for the multiple I2C-based multiplexer case. But for multiple > > GPIO-based multiplexers, for example, the name confusion still exists. > > Only if some genius connects multiple separate gpio based multiplexers > to the same bus. This is totally possible, for a variety of reasons, including physical location of the GPIO pins and bus segment ends, but also economical (using the same chips on several boards.) > In practice, though, that can be modeled as single > gpio mux with more pins, so I didn't think that is that much of a problem. I admit I had not considered this. But would it work if the different GPIO pins are on different chips (say south bridge and Super-I/O for example)? The i2c-mux-gpio driver assumes a single chip, at least when passed by name. > > If we are going to address this problem, I believe we should do so in a > > way which doesn't need revisiting in a couple months. You are changing > > adapter names people or tools may be relying on, we really don't want > > to do this too often. > > Good point. One possible solution might be to add the mux type into the name. > i2c-2-mux-i2c-76 (chan_id 0) > i2c-2-mux-gpio-77 (chan_id 0) Yes, that's pretty much what I had in mind. > We could add an additional parameter to the API, such as > const char *qualifier > to provide the "i2c-76" or "gpio-77" string. If the qualifier is NULL, > the name would revert to the old naming scheme. Would that make sense ? Yes, although your first draft gave me some hope that i2c-mux could do some magic to guess the type and identifier automatically. But maybe it's not that clean to have to touch i2c-mux for every new i2c-mux-* driver. Not that I expect too many of them... so it's mostly a matter of principle. An explicit qualifier as you suggest above might be cleaner.
On Mon, Nov 18, 2013 at 03:14:41PM +0100, Jean Delvare wrote: > Hi Guenter, > > On Sun, 17 Nov 2013 13:13:43 -0800, Guenter Roeck wrote: > > On 11/17/2013 12:41 PM, Jean Delvare wrote: > > > On Sun, 17 Nov 2013 10:56:24 -0800, Guenter Roeck wrote: > > >> This retains the old mux name if the mux is not an i2c device, and adds > > >> the i2c device address if it is. This solves the problem for me. > > >> > > >> Comments ? > > > > > > This "for me" worries me a bit. You basically restored the uniqueness > > > for the multiple I2C-based multiplexer case. But for multiple > > > GPIO-based multiplexers, for example, the name confusion still exists. > > > > Only if some genius connects multiple separate gpio based multiplexers > > to the same bus. > > This is totally possible, for a variety of reasons, including physical > location of the GPIO pins and bus segment ends, but also economical > (using the same chips on several boards.) > Ok. > > In practice, though, that can be modeled as single > > gpio mux with more pins, so I didn't think that is that much of a problem. > > I admit I had not considered this. But would it work if the different > GPIO pins are on different chips (say south bridge and Super-I/O for > example)? The i2c-mux-gpio driver assumes a single chip, at least when > passed by name. > ... so you could or should not pass a name if that is the case. > > > If we are going to address this problem, I believe we should do so in a > > > way which doesn't need revisiting in a couple months. You are changing > > > adapter names people or tools may be relying on, we really don't want > > > to do this too often. > > > > Good point. One possible solution might be to add the mux type into the name. > > i2c-2-mux-i2c-76 (chan_id 0) > > i2c-2-mux-gpio-77 (chan_id 0) > > Yes, that's pretty much what I had in mind. > > > We could add an additional parameter to the API, such as > > const char *qualifier > > to provide the "i2c-76" or "gpio-77" string. If the qualifier is NULL, > > the name would revert to the old naming scheme. Would that make sense ? > > Yes, although your first draft gave me some hope that i2c-mux could do > some magic to guess the type and identifier automatically. But maybe > it's not that clean to have to touch i2c-mux for every new i2c-mux-* > driver. Not that I expect too many of them... so it's mostly a matter > of principle. An explicit qualifier as you suggest above might be > cleaner. > Another option would be to use the patch I provided, except to create names such as i2c-2-mux-i2c-76. The API change could then be added later if/as needed. But, yes, an API change would be cleaner. Before I submit a "final" patch, it would be great to get feedback and direcetion from Wolfram ... after all, he'll have to approve it at the end. Thanks, Guenter -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/i2c/i2c-mux.c b/drivers/i2c/i2c-mux.c index 797e311..9a49fe9 100644 --- a/drivers/i2c/i2c-mux.c +++ b/drivers/i2c/i2c-mux.c @@ -109,7 +109,9 @@ struct i2c_adapter *i2c_add_mux_adapter(struct i2c_adapter *parent, int (*deselect) (struct i2c_adapter *, void *, u32)) { + struct i2c_client *client = i2c_verify_client(mux_dev); struct i2c_mux_priv *priv; + char client_addr[8]; int ret; priv = kzalloc(sizeof(struct i2c_mux_priv), GFP_KERNEL); @@ -133,8 +135,15 @@ struct i2c_adapter *i2c_add_mux_adapter(struct i2c_adapter *parent, priv->algo.functionality = i2c_mux_functionality; /* Now fill out new adapter structure */ + if (client) + scnprintf(client_addr, sizeof(client_addr), "-%02x", + client->addr); + else + client_addr[0] = '\0'; + snprintf(priv->adap.name, sizeof(priv->adap.name), - "i2c-%d-mux (chan_id %d)", i2c_adapter_id(parent), chan_id); + "i2c-%d-mux%s (chan_id %d)", i2c_adapter_id(parent), + client_addr, chan_id); priv->adap.owner = THIS_MODULE; priv->adap.algo = &priv->algo; priv->adap.algo_data = priv;