RE: [patch v5] i2c: mux: mellanox: add driver

[Vadim Pasternak]

> -----Original Message-----
> From: Peter Rosin [mailto:peda@xxxxxxxxxx]
> Sent: Friday, September 23, 2016 12:36 PM
> To: Vadim Pasternak <vadimp@xxxxxxxxxxxx>; wsa@xxxxxxxxxxxxx
> Cc: linux-i2c@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; jiri@xxxxxxxxxxx;
> Michael Shych <michaelsh@xxxxxxxxxxxx>
> Subject: Re: [patch v5] i2c: mux: mellanox: add driver
> 
> On 2016-09-13 22:37, vadimp@xxxxxxxxxxxx wrote:
> > From: Vadim Pasternak <vadimp@xxxxxxxxxxxx>
> >
> > This driver allows I2C routing controlled through CPLD select
> > registers on a wide range of Mellanox systems (CPLD Lattice device).
> > MUX selection is provided by digital and analog HW. Analog part is not
> > under SW control.
> > Digital part is under CPLD control (channel selection/de-selection).
> >
> > Connectivity schema.
> > i2c-mlxcpld                                 Digital               Analog
> > driver
> > *--------*                                 * -> mux1 (virt bus2) -> mux ->|
> > | I2CLPC | i2c physical                    * -> mux2 (virt bus3) -> mux ->|
> > | bridge | bus 1                 *---------*                              |
> > | logic  |---------------------> * mux reg *                              |
> > | in CPLD|                       *---------*                              |
> > *--------*   i2c-mux-mlxpcld          ^    * -> muxn (virt busn) -> mux ->|
> >     |        driver                   |                                   |
> >     |        *---------------*        |                             Devices
> >     |        * CPLD (i2c bus)* select |
> >     |        * registers for *--------*
> >     |        * mux selection * deselect
> >     |        *---------------*
> >     |                 |
> > <-------->     <----------->
> > i2c cntrl      Board cntrl reg
> > reg space      space (mux select,
> >                IO, LED, WD, info)
> 
> Hmm, I'm wondering about the above schematics, which seems a bit overly
> complex. I mean, it's not relevant to this driver how the I2CLPC bridge logic in
> the CPLD is controlled. Particularly so since it does not need to be a this
> particular i2c adapter that is muxed.
> 
> But what I'm really wondering is if this mux can mux same other bus than is used
> to control the mux. I.e. is it possible to do something like this:
> 
>   .---.          .-------------.
>   |   |          |             |-- i2c2 --
>   | l |-- i2c0 --| mlxcpld mux |
>   | i |          |             |-- i2c3 --
>   | n |          '-------------'
>   | u |                 |
>   | x |-- i2c1 ---------'
>   |   |
>   '---'
> 
> Or is it always as below, with the mux being controlled from the same i2c bus it
> muxes?
> 
>   .---.             .-------------.
>   | l |             |             |-- i2c1 --
>   | i |-- i2c0 --+--| mlxcpld mux |
>   | n |          |  |             |-- i2c2 --
>   | u |          |  '-------------'
>   | x |          |         |
>   '---'          '---------'
> 

Hi Peter,

Thank you very much for this and all previous feedbacks.

Yes, it is not coupled with I2CLPC bridge logic.
When I sent my initial patch, it was marked as 1/2, while 1/1 was patch for i2c-mlxcpld controller.
So, maybe my comment here is redundant I can make it simpler.

Actual description you provided is better.

Would you like me to submit patch with this modification only?


Just as an example to show how we are going to use this driver.

This is x86 systems, when we do have LPCI2C as controller (we have on these systems management CPLD attached to LPC on carrier board and I2C attached CLPD on switch board:
.---.             .-------------.
| l |             |             |-- i2cx1 -- i2cy8
| i |-- i2c1 --+--| mlxcpld mux |
| n |          |  |             |-- i2y1 -- i2cy8
| u |          |  '-------------'
| x |          |         |
 '---'          '---------'

I2C CPLD is attached to bus 1 at 0x62, and has register 0x81 - 0x85 for access to 4 or 5 banks of QSFP ports (each bank of 8).

This is coming systems equipped with BMC Aspeed 2520 SoC ARM11 based (by the way this driver now also in review process): 
.---.             .-------------.
| l |             |             |-- i2cx1 -- i2cy8
| i |-- i2cn --+--| mlxcpld mux |
| n |          |  |             |-- i2y1 -- i2cy8
| u |          |  '-------------'
| x |          |         |
 '---'          '---------

I2C CPLD is attached to bus n (SoC equipped with 14 I2C busses) at 0x62, and has register 0xz1 - 0xz1 for access to 4 or 5 banks of QSFP ports (each bank of 8).

Cheers,
Vadim.

> Cheers,
> Peter