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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <5718C11A.6010001@intel.com>
Date:	Thu, 21 Apr 2016 15:01:30 +0300
From:	Crestez Dan Leonard <leonard.crestez@...el.com>
To:	Peter Rosin <peda@...ntia.se>, Jonathan Cameron <jic23@...nel.org>,
	"linux-iio@...r.kernel.org" <linux-iio@...r.kernel.org>
Cc:	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Hartmut Knaack <knaack.h@....de>,
	Lars-Peter Clausen <lars@...afoo.de>,
	Peter Meerwald-Stadler <pmeerw@...erw.net>,
	Daniel Baluta <daniel.baluta@...el.com>,
	"linux-i2c@...r.kernel.org" <linux-i2c@...r.kernel.org>,
	Wolfram Sang <wsa@...-dreams.de>,
	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
	Rob Herring <robh+dt@...nel.org>,
	Pawel Moll <pawel.moll@....com>,
	Mark Rutland <mark.rutland@....com>,
	Ian Campbell <ijc+devicetree@...lion.org.uk>,
	Kumar Gala <galak@...eaurora.org>
Subject: Re: [PATCH] iio: inv_mpu6050: Add support for auxiliary I2C master

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.