Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1383732 > unrolled thread

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

Back to article view | Back to linux.kernel

Contents

  Re: [PATCH] iio: inv_mpu6050: Add support for auxiliary I2C master Peter Rosin <peda@axentia.se> - 2016-04-20 22:50 +0200
    Re: [PATCH] iio: inv_mpu6050: Add support for auxiliary I2C master Crestez Dan Leonard <leonard.crestez@intel.com> - 2016-04-21 14:10 +0200

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

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@intel.com>
> ---
> 
> This is based on earlier work by Daniel Baluta <daniel.baluta@intel.com>:
> https://www.spinics.net/lists/linux-iio/msg23573.html
> 
> Changes since that version:
> * Nest the adapter in inv_mpu6050_state instead of making it static
> * Explicitly forward of_node "i2c-aux-master" to allow describing aux devices
> via devicetree.
> 
> For bypass/mux mode devicetree works automatically. The forwarding is based on
> the "chan_id" parameter to i2c_add_mux_adapter and is implemented here:
> 
> http://lxr.free-electrons.com/source/drivers/i2c/i2c-mux.c#L158

With any luck [1], this reference will be stale when 4.6 is released. It
should be qulified with a version, something like .../i2c-mux.c?v=4.5#L158

> Perhaps it might be better for devices handled via master mode to be described
> via i2c@1? This would work by scanning the mpu node's children for something
> with reg == 1.

The 0 in i2c@0 (which is used by the mux mode) is the index of the mux slave
meaning that i2c@1 would be a second mux slave on the same mux, but this is
not a real mux as such, it is a gate which is piggybacking on the i2c mux infra.
So, this "mux" can't have a second slave which is why only 0 is valid.

Tl;dr i2c@1 is definitely wrong for something that is not related to i2c@0.

> Or maybe the two busses should be called i2c-aux-master and i2c-aux-mux? Not
> sure how to deal with that on the mux side.

Changing i2c to i2c-aux-mux would break existing device trees, that seems
like a bad thing, no?

> It is not clear how to properly handle this and suggestions are welcome. The
> way it currently works with this patch is documented immediately below.

I think the naming could be i2c-master0, i2c-master1 etc if it, with
future work, would be possible to add more than one master (you talked about
5 i2c slaves..).

Cheers,
Peter

[1] https://lkml.org/lkml/2016/4/20/467

[toc] | [next] | [standalone]

#1384134

On 04/20/2016 11:31 PM, Peter Rosin wrote:
> Crestez Dan Leonard wrote:
>> Changes since that version:
>> * Nest the adapter in inv_mpu6050_state instead of making it static
>> * Explicitly forward of_node "i2c-aux-master" to allow describing aux devices
>> via devicetree.
>>
>> For bypass/mux mode devicetree works automatically. The forwarding is based on
>> the "chan_id" parameter to i2c_add_mux_adapter and is implemented here:
>>
>> http://lxr.free-electrons.com/source/drivers/i2c/i2c-mux.c?v=4.5#L158
>>
>> Perhaps it might be better for devices handled via master mode to be described
>> via i2c@1? This would work by scanning the mpu node's children for something
>> with reg == 1.
> 
> The 0 in i2c@0 (which is used by the mux mode) is the index of the mux slave
> meaning that i2c@1 would be a second mux slave on the same mux, but this is
> not a real mux as such, it is a gate which is piggybacking on the i2c mux infra.
> So, this "mux" can't have a second slave which is why only 0 is valid.

This behavior is automatic in i2c mux code and seems to assume that all
the children of mux_dev are i2c muxes. This might be obviously correct
and useful for dedicated i2c mux devices but in my case mux_dev is just
the i2c_client for a sensor.

From Documentation/devicetree/bindings/i2c/i2c-mux.txt:

> An i2c bus multiplexer/switch will have several child busses that are
> numbered uniquely in a device dependent manner.  The nodes for an i2c
> bus multiplexer/switch will have one child node for each child bus.

This seems to be written in a way that would allow me to define the
"auxiliary i2c master" as bus "1". After all, the numbering is device
dependent and it's not clear that all the child busses need to be
accessible through muxing rather than indirect access through device
registers.

>> Or maybe the two busses should be called i2c-aux-master and i2c-aux-mux? Not
>> sure how to deal with that on the mux side.
> 
> Changing i2c to i2c-aux-mux would break existing device trees, that seems
> like a bad thing, no?

That support was not documented in mpu6050's bindings and might not be
actually used.

>> It is not clear how to properly handle this and suggestions are welcome. The
>> way it currently works with this patch is documented immediately below.
> 
> I think the naming could be i2c-master0, i2c-master1 etc if it, with
> future work, would be possible to add more than one master (you talked about
> 5 i2c slaves..).

The device has 5 sets of registers for controlling i2c slaves but only
one physical auxiliary i2c bus. As far as I can tell slaves 0-3 are
intended to be used for gathering readings for slaved sensors
periodically without external intervention. Slave 4 can generate an
interrupt on completion and is more suitable for general-purpose
communication with any number of devices.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web