Re: [PATCH] iio: inv_mpu6050: Add support for auxiliary I2C master

[Peter Rosin]

On 2016-04-28 12:39, Crestez Dan Leonard wrote:
> On 04/27/2016 11:39 AM, Peter Rosin wrote:
>> On 2016-04-23 23:32, Jonathan Cameron wrote:
>>> On 20/04/16 18:17, Crestez Dan Leonard wrote:
>>>> The MPU has an auxiliary I2C bus for connecting external
>>>> sensors. This bus has two operating modes:
>>>> * pass-through, which connects the primary and auxiliary busses
>>>> together. This is already supported via an i2c mux.
>>>> * I2C master mode, where the mpu60x0 acts as a master to any external
>>>> connected sensors. This is implemented by this patch.
>>>>
>>>> This I2C master mode also works when the MPU itself is connected via
>>>> SPI.
>>>>
>>>> I2C master supports up to 5 slaves. Slaves 0-3 have a common operating
>>>> mode while slave 4 is different. This patch implements an i2c adapter
>>>> using slave 4 because it has a cleaner interface and it has an
>>>> interrupt that signals when data from slave to master arrived.
>>>>
>>>> Signed-off-by: Crestez Dan Leonard <leonard.crestez@xxxxxxxxx>
>>> This one needs acks from:
>>>
>>> Device tree maintainer (odd binding ;)
>>> Peter Rosin (odd binding interacting with the mux support)
>>> Wolfram (it has a whole i2c master driver in here).
>>>
>>> (just thought I'd list these for the avoidance of doubt).
>> I spot some overlap with the questions in "[RFC] i2c: device-tree:
>> Handling child nodes which are not i2c devices"
>> http://marc.info/?l=linux-i2c&m=146073452819116&w=2
>>
>> And I think I agree with Stephen Warren that an intermediate placeholder
>> node would make sense. I.e.
>>
>>     mpu6050@68 {
>>         compatible = "...";
>>         reg = <0x68>;
>>         ...
>>         i2c-aux-mux {
>>             i2c@0 {
>>                 #address-cells = <1>;
>>                 #size-cells = <0>;
>>                 reg = <0>;
>>
>>                 foo@44 {
>>                     compatible = "bar";
>>                     reg = <0x44>;
>>                     ...
>>                 }
>>             }
>>         }
>>     }
>>
>> Or
>>
>>     mpu6050@68 {
>>         compatible = "...";
>>         reg = <0x68>;
>>         ...
>>         i2c-aux-master {
>>             #address-cells = <1>;
>>             #size-cells = <0>;
>>
>>             gazonk@44 {
>>                 compatible = "baz";
>>                 reg = <0x44>;
>>                 ...
>>             }
>>         }
>>     }
>>
>> depending on if you want an aux-mux or an aux-master.
>>
>> But I don't know if that intermediate i2c-aux-mux node causes any
>> problems?
> It's not clear how that would be implemented. It seems to me that right
> now i2c_add_mux_adapter assumes that the parent device is a dedicated
> mux device and all it's children are mux branches. Would this require
> introducing a new "struct device" for the i2c-aux-master node?
>
> It might make sense to make the automatic processing of the parents
> node's of_node optional and let the caller assign the of_node describing
> the attached devices.
>
> I think the most natural solution would be to require child nodes named
> i2c-aux-mux and i2c-aux-master to describe aux devices. For backwards
> compatibility it would be easiest to go with i2c@0/i2c@1 (identified by
> reg=0/1).
>
> But I don't know much about devicetree and I'd rather accept an external
> suggestion.
>
I was thinking that with the new i2c_mux_core in place, it should be pretty simple
to add a hook to point to another node and only use dev->of_node as a default
value for where to look for the mux child adapters?

Cheers,
Peter