Re: [patch 2/2] i2c: mux: mellanox: add driver

2016-09-06 Thread Peter Rosin 
On 2016-08-29 19:36, vad...@mellanox.com wrote:
> From: Vadim Pasternak 
> 
> This driver allows I2C routing controlled through CPLD select registers on
> 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 (LPC bus)* select |
> |* registers for **
> |* mux selection * deselect
> |*---*
> | |
> <> <--->
> i2c cntrl  Board cntrl reg
> reg space  space (mux select,
> |  IO, LED, WD, info)
> | |  *-*   *-*
> *- LPC bus --| PCH |---| CPU |
>  *-*   *-*
> 
> i2c-mux-mlxpcld does not necessary required i2c-mlxcpld. It can be use
> along with another bus driver, and still control i2c routing through CPLD
> mux selection, in case the system is equipped with CPLD capable of mux
> selection control.
> 
> The Kconfig currently controlling compilation of this code is:
> drivers/i2c/muxes/Kconfig:config I2C_MUX_MLXCPLD
> 
> Signed-off-by: Michael Shych 
> Signed-off-by: Vadim Pasternak 
> Reviewed-by: Jiri Pirko 

BTW, it just occurred to me that you could perhaps use (and extend if
needed) the i2c_mux_reg driver for the lpc_access cases. Have you
explored that option? It looks like a nice fit, and limiting this new
driver to i2c_access would make it significantly simpler with only
one device (mux module) and one access method (i2c) to support.

Cheers,
Peter

Re: [patch 2/2] i2c: mux: mellanox: add driver

2016-09-06 Thread Peter Rosin 
On 2016-08-29 19:36, vad...@mellanox.com wrote:
> From: Vadim Pasternak 
> 
> This driver allows I2C routing controlled through CPLD select registers on
> 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 (LPC bus)* select |
> |* registers for **
> |* mux selection * deselect
> |*---*
> | |
> <> <--->
> i2c cntrl  Board cntrl reg
> reg space  space (mux select,
> |  IO, LED, WD, info)
> | |  *-*   *-*
> *- LPC bus --| PCH |---| CPU |
>  *-*   *-*
> 
> i2c-mux-mlxpcld does not necessary required i2c-mlxcpld. It can be use
> along with another bus driver, and still control i2c routing through CPLD
> mux selection, in case the system is equipped with CPLD capable of mux
> selection control.
> 
> The Kconfig currently controlling compilation of this code is:
> drivers/i2c/muxes/Kconfig:config I2C_MUX_MLXCPLD
> 
> Signed-off-by: Michael Shych 
> Signed-off-by: Vadim Pasternak 
> Reviewed-by: Jiri Pirko 

BTW, it just occurred to me that you could perhaps use (and extend if
needed) the i2c_mux_reg driver for the lpc_access cases. Have you
explored that option? It looks like a nice fit, and limiting this new
driver to i2c_access would make it significantly simpler with only
one device (mux module) and one access method (i2c) to support.

Cheers,
Peter

Re: [patch 2/2] i2c: mux: mellanox: add driver

2016-09-02 Thread Peter Rosin 
On 2016-08-29 19:36, vad...@mellanox.com wrote:
> From: Vadim Pasternak 
> 
> This driver allows I2C routing controlled through CPLD select registers on
> 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 (LPC bus)* select |
> |* registers for **
> |* mux selection * deselect
> |*---*
> | |
> <> <--->
> i2c cntrl  Board cntrl reg
> reg space  space (mux select,
> |  IO, LED, WD, info)
> | |  *-*   *-*
> *- LPC bus --| PCH |---| CPU |
>  *-*   *-*
> 
> i2c-mux-mlxpcld does not necessary required i2c-mlxcpld. It can be use

s/necessary required/necessarily require a/

> along with another bus driver, and still control i2c routing through CPLD
> mux selection, in case the system is equipped with CPLD capable of mux
> selection control.
> 
> The Kconfig currently controlling compilation of this code is:
> drivers/i2c/muxes/Kconfig:config I2C_MUX_MLXCPLD
> 
> Signed-off-by: Michael Shych 
> Signed-off-by: Vadim Pasternak 
> Reviewed-by: Jiri Pirko 
> ---
>  MAINTAINERS |   8 +
>  drivers/i2c/muxes/Kconfig   |  11 ++
>  drivers/i2c/muxes/Makefile  |   1 +
>  drivers/i2c/muxes/i2c-mux-mlxcpld.c | 326 
> 
>  include/linux/i2c/mlxcpld.h |  57 +++
>  5 files changed, 403 insertions(+)
>  create mode 100644 drivers/i2c/muxes/i2c-mux-mlxcpld.c
>  create mode 100644 include/linux/i2c/mlxcpld.h

It is customary to mark updated patches with v2, v3 etc in the subject
and to include a short changelog right here in this space (or perhaps
in a cover letter if it is a patch series). Please do so going
forward.

> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 8b3f8d7..a994455 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -7664,6 +7664,14 @@ W: http://www.mellanox.com
>  F:   drivers/i2c/busses/i2c-mlxcpld.c
>  F:   Documentation/i2c/busses/i2c-mlxcpld
>  
> +MELLANOX MLXCPLD I2C MUX DRIVER
> +M:   Vadim Pasternak 
> +M:   Michael Shych 
> +L:   linux-kernel@vger.kernel.org

Isn't the linux-i2c list a narrower and therefore better fit?

> +S:   Supported
> +W:   http://www.mellanox.com
> +F:   drivers/i2c/muxes/i2c-mux-mlxcpld.c
> +
>  SOFT-ROCE DRIVER (rxe)
>  M:   Moni Shoua 
>  L:   linux-r...@vger.kernel.org
> diff --git a/drivers/i2c/muxes/Kconfig b/drivers/i2c/muxes/Kconfig
> index e280c8e..b7ab144 100644
> --- a/drivers/i2c/muxes/Kconfig
> +++ b/drivers/i2c/muxes/Kconfig
> @@ -81,4 +81,15 @@ config I2C_DEMUX_PINCTRL
> demultiplexer that uses the pinctrl subsystem. This is useful if you
> want to change the I2C master at run-time depending on features.
>  
> +config I2C_MUX_MLXCPLD
> +tristate "Mellanox CPLD based I2C multiplexer"
> +help
> +  If you say yes to this option, support will be included for a
> +  CPLD based I2C multiplexer. This driver provides access to
> +  I2C busses connected through a MUX, which is controlled
> +  by a CPLD registers.
> +
> +  This driver can also be built as a module.  If so, the module
> +  will be called i2c-mux-mlxcpld.
> +
>  endmenu
> diff --git a/drivers/i2c/muxes/Makefile b/drivers/i2c/muxes/Makefile
> index 7c267c2..e5c990e 100644
> --- a/drivers/i2c/muxes/Makefile
> +++ b/drivers/i2c/muxes/Makefile
> @@ -10,5 +10,6 @@ obj-$(CONFIG_I2C_MUX_PCA9541)   += i2c-mux-pca9541.o
>  obj-$(CONFIG_I2C_MUX_PCA954x)+= i2c-mux-pca954x.o
>  obj-$(CONFIG_I2C_MUX_PINCTRL)+= i2c-mux-pinctrl.o
>  obj-$(CONFIG_I2C_MUX_REG)+= i2c-mux-reg.o
> +obj-$(CONFIG_I2C_MUX_MLXCPLD)+= i2c-mux-mlxcpld.o
>  
>  ccflags-$(CONFIG_I2C_DEBUG_BUS) := -DDEBUG
> diff --git a/drivers/i2c/muxes/i2c-mux-mlxcpld.c 
>

Re: [patch 2/2] i2c: mux: mellanox: add driver

2016-09-02 Thread Peter Rosin 
On 2016-08-29 19:36, vad...@mellanox.com wrote:
> From: Vadim Pasternak 
> 
> This driver allows I2C routing controlled through CPLD select registers on
> 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 (LPC bus)* select |
> |* registers for **
> |* mux selection * deselect
> |*---*
> | |
> <> <--->
> i2c cntrl  Board cntrl reg
> reg space  space (mux select,
> |  IO, LED, WD, info)
> | |  *-*   *-*
> *- LPC bus --| PCH |---| CPU |
>  *-*   *-*
> 
> i2c-mux-mlxpcld does not necessary required i2c-mlxcpld. It can be use

s/necessary required/necessarily require a/

> along with another bus driver, and still control i2c routing through CPLD
> mux selection, in case the system is equipped with CPLD capable of mux
> selection control.
> 
> The Kconfig currently controlling compilation of this code is:
> drivers/i2c/muxes/Kconfig:config I2C_MUX_MLXCPLD
> 
> Signed-off-by: Michael Shych 
> Signed-off-by: Vadim Pasternak 
> Reviewed-by: Jiri Pirko 
> ---
>  MAINTAINERS |   8 +
>  drivers/i2c/muxes/Kconfig   |  11 ++
>  drivers/i2c/muxes/Makefile  |   1 +
>  drivers/i2c/muxes/i2c-mux-mlxcpld.c | 326 
> 
>  include/linux/i2c/mlxcpld.h |  57 +++
>  5 files changed, 403 insertions(+)
>  create mode 100644 drivers/i2c/muxes/i2c-mux-mlxcpld.c
>  create mode 100644 include/linux/i2c/mlxcpld.h

It is customary to mark updated patches with v2, v3 etc in the subject
and to include a short changelog right here in this space (or perhaps
in a cover letter if it is a patch series). Please do so going
forward.

> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 8b3f8d7..a994455 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -7664,6 +7664,14 @@ W: http://www.mellanox.com
>  F:   drivers/i2c/busses/i2c-mlxcpld.c
>  F:   Documentation/i2c/busses/i2c-mlxcpld
>  
> +MELLANOX MLXCPLD I2C MUX DRIVER
> +M:   Vadim Pasternak 
> +M:   Michael Shych 
> +L:   linux-kernel@vger.kernel.org

Isn't the linux-i2c list a narrower and therefore better fit?

> +S:   Supported
> +W:   http://www.mellanox.com
> +F:   drivers/i2c/muxes/i2c-mux-mlxcpld.c
> +
>  SOFT-ROCE DRIVER (rxe)
>  M:   Moni Shoua 
>  L:   linux-r...@vger.kernel.org
> diff --git a/drivers/i2c/muxes/Kconfig b/drivers/i2c/muxes/Kconfig
> index e280c8e..b7ab144 100644
> --- a/drivers/i2c/muxes/Kconfig
> +++ b/drivers/i2c/muxes/Kconfig
> @@ -81,4 +81,15 @@ config I2C_DEMUX_PINCTRL
> demultiplexer that uses the pinctrl subsystem. This is useful if you
> want to change the I2C master at run-time depending on features.
>  
> +config I2C_MUX_MLXCPLD
> +tristate "Mellanox CPLD based I2C multiplexer"
> +help
> +  If you say yes to this option, support will be included for a
> +  CPLD based I2C multiplexer. This driver provides access to
> +  I2C busses connected through a MUX, which is controlled
> +  by a CPLD registers.
> +
> +  This driver can also be built as a module.  If so, the module
> +  will be called i2c-mux-mlxcpld.
> +
>  endmenu
> diff --git a/drivers/i2c/muxes/Makefile b/drivers/i2c/muxes/Makefile
> index 7c267c2..e5c990e 100644
> --- a/drivers/i2c/muxes/Makefile
> +++ b/drivers/i2c/muxes/Makefile
> @@ -10,5 +10,6 @@ obj-$(CONFIG_I2C_MUX_PCA9541)   += i2c-mux-pca9541.o
>  obj-$(CONFIG_I2C_MUX_PCA954x)+= i2c-mux-pca954x.o
>  obj-$(CONFIG_I2C_MUX_PINCTRL)+= i2c-mux-pinctrl.o
>  obj-$(CONFIG_I2C_MUX_REG)+= i2c-mux-reg.o
> +obj-$(CONFIG_I2C_MUX_MLXCPLD)+= i2c-mux-mlxcpld.o
>  
>  ccflags-$(CONFIG_I2C_DEBUG_BUS) := -DDEBUG
> diff --git a/drivers/i2c/muxes/i2c-mux-mlxcpld.c 
> b/drivers/i2c/muxes/i2c-mux-mlxcpld.c
> new file mode 100644
> index 000..9624613
> --- /dev/null
> +++ b/drivers/i2c/muxes/i2c-mux-mlxcpld.c
> @@ -0,0

Re: [patch 2/2] i2c: mux: mellanox: add driver

2016-08-28 Thread kbuild test robot 
Hi Vadim,

[auto build test WARNING on wsa/i2c/for-next]
[also build test WARNING on v4.8-rc3 next-20160825]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]
[Suggest to use git(>=2.9.0) format-patch --base= (or --base=auto for 
convenience) to record what (public, well-known) commit your patch series was 
built on]
[Check https://git-scm.com/docs/git-format-patch for more information]

url:
https://github.com/0day-ci/linux/commits/vadimp-mellanox-com/i2c-add-master-driver-for-mellanox-systems/20160828-225625
base:   https://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git 
i2c/for-next
config: m68k-allmodconfig (attached as .config)
compiler: m68k-linux-gcc (GCC) 4.9.0
reproduce:
wget 
https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross
 -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=m68k 

All warnings (new ones prefixed by >>):

   In file included from arch/m68k/include/asm/io_mm.h:26:0,
from arch/m68k/include/asm/io.h:4,
from include/linux/io.h:25,
from drivers/i2c/muxes/i2c-mux-mlxcpld.c:42:
   drivers/i2c/muxes/i2c-mux-mlxcpld.c: In function 'mlxcpld_mux_reg_write':
   arch/m68k/include/asm/raw_io.h:43:32: warning: cast to pointer from integer 
of different size [-Wint-to-pointer-cast]
#define out_8(addr,b) (void)((*(__force volatile u8 *) (addr)) = (b))
   ^
   arch/m68k/include/asm/io_mm.h:396:72: note: in expansion of macro 'out_8'
#define outb(val, port) ((port) < 1024 ? isa_rom_outb((val), (port)) : 
out_8((port), (val)))
   ^
>> drivers/i2c/muxes/i2c-mux-mlxcpld.c:164:3: note: in expansion of macro 'outb'
  outb(val, pdata->addr); /* addr = CPLD base + offset */
  ^

vim +/outb +164 drivers/i2c/muxes/i2c-mux-mlxcpld.c

36  #include 
37  #include 
38  #include 
39  #include 
40  #include 
41  #include 
  > 42  #include 
43  #include 
44  #include 
45  
46  #define CPLD_MUX_MAX_NCHANS 8
47  #define CPLD_MUX_EXT_MAX_NCHANS 24
48  
49  /*
50   * mlxcpld_mux types - kind of mux supported by driver:
51   * @mlxcpld_mux_tor - LPC access; 8 channels.
52   * @mlxcpld_mux_mgmt - LPC access; 8 channels.
53   * @mlxcpld_mux_mgmt_ext - LPC access; 24 channels.
54   * @mlxcpld_mux_module - I2C access; 8 channels/legs.
55   */
56  enum mlxcpld_mux_type {
57  mlxcpld_mux_tor,
58  mlxcpld_mux_mgmt,
59  mlxcpld_mux_mgmt_ext,
60  mlxcpld_mux_module,
61  };
62  
63  /* mlxcpld_mux_type - underlying physical bus, to which device is 
connected:
64   * @lpc_access - LPC connected CPLD device
65   * @i2c_access - I2C connected CPLD device
66   */
67  enum mlxcpld_mux_access_type {
68  lpc_access,
69  i2c_access,
70  };
71  
72  /* mlxcpld_mux - mux control structure:
73   * @type - mux type
74   * @last_chan - last register value
75   * @client - I2C device client
76   */
77  struct mlxcpld_mux {
78  enum mlxcpld_mux_type type;
79  u8 last_chan;
80  struct i2c_client *client;
81  };
82  
83  /* mlxcpld_mux_desc - mux descriptor structure:
84   * @nchans - number of channels
85   * @muxtype - physical mux type (LPC or I2C)
86   */
87  struct mlxcpld_mux_desc {
88  u8 nchans;
89  enum mlxcpld_mux_access_type muxtype;
90  };
91  
92  /* MUX logic description.
93   * This logic can be applied for LPC attached CPLD and fro I2C attached 
CPLD.
94   * Driver can support different mux control logic, according to CPLD
95   * implementation.
96   *
97   * Connectivity schema.
98   *
99   * i2c-mlxcpld Digital   
Analog
   100   * driver
   101   * ** * -> mux1 (virt bus2) -> 
mux -> |
   102   * | I2CLPC | i2c physical* -> mux2 (virt bus3) -> 
mux -> |
   103   * | bridge | bus 1 *-* 
  |
   104   * | logic  |-> * mux reg * 
  |
   105   * | in CPLD|   *-* 
  |
   106   * **   i2c-mux-mlxpcld  ^* -> muxn (virt busn) -> 
mux -> |
   107   * |driver   |  
  |
   108   * |*---*|  
Devices
   109   * |* CPLD (LPC bus)* select |
   110   * |* registers for **
   111   * |* mux selection * deselect

Re: [patch 2/2] i2c: mux: mellanox: add driver

2016-08-28 Thread kbuild test robot 
Hi Vadim,

[auto build test WARNING on wsa/i2c/for-next]
[also build test WARNING on v4.8-rc3 next-20160825]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]
[Suggest to use git(>=2.9.0) format-patch --base= (or --base=auto for 
convenience) to record what (public, well-known) commit your patch series was 
built on]
[Check https://git-scm.com/docs/git-format-patch for more information]

url:
https://github.com/0day-ci/linux/commits/vadimp-mellanox-com/i2c-add-master-driver-for-mellanox-systems/20160828-225625
base:   https://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git 
i2c/for-next
config: m68k-allmodconfig (attached as .config)
compiler: m68k-linux-gcc (GCC) 4.9.0
reproduce:
wget 
https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross
 -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=m68k 

All warnings (new ones prefixed by >>):

   In file included from arch/m68k/include/asm/io_mm.h:26:0,
from arch/m68k/include/asm/io.h:4,
from include/linux/io.h:25,
from drivers/i2c/muxes/i2c-mux-mlxcpld.c:42:
   drivers/i2c/muxes/i2c-mux-mlxcpld.c: In function 'mlxcpld_mux_reg_write':
   arch/m68k/include/asm/raw_io.h:43:32: warning: cast to pointer from integer 
of different size [-Wint-to-pointer-cast]
#define out_8(addr,b) (void)((*(__force volatile u8 *) (addr)) = (b))
   ^
   arch/m68k/include/asm/io_mm.h:396:72: note: in expansion of macro 'out_8'
#define outb(val, port) ((port) < 1024 ? isa_rom_outb((val), (port)) : 
out_8((port), (val)))
   ^
>> drivers/i2c/muxes/i2c-mux-mlxcpld.c:164:3: note: in expansion of macro 'outb'
  outb(val, pdata->addr); /* addr = CPLD base + offset */
  ^

vim +/outb +164 drivers/i2c/muxes/i2c-mux-mlxcpld.c

36  #include 
37  #include 
38  #include 
39  #include 
40  #include 
41  #include 
  > 42  #include 
43  #include 
44  #include 
45  
46  #define CPLD_MUX_MAX_NCHANS 8
47  #define CPLD_MUX_EXT_MAX_NCHANS 24
48  
49  /*
50   * mlxcpld_mux types - kind of mux supported by driver:
51   * @mlxcpld_mux_tor - LPC access; 8 channels.
52   * @mlxcpld_mux_mgmt - LPC access; 8 channels.
53   * @mlxcpld_mux_mgmt_ext - LPC access; 24 channels.
54   * @mlxcpld_mux_module - I2C access; 8 channels/legs.
55   */
56  enum mlxcpld_mux_type {
57  mlxcpld_mux_tor,
58  mlxcpld_mux_mgmt,
59  mlxcpld_mux_mgmt_ext,
60  mlxcpld_mux_module,
61  };
62  
63  /* mlxcpld_mux_type - underlying physical bus, to which device is 
connected:
64   * @lpc_access - LPC connected CPLD device
65   * @i2c_access - I2C connected CPLD device
66   */
67  enum mlxcpld_mux_access_type {
68  lpc_access,
69  i2c_access,
70  };
71  
72  /* mlxcpld_mux - mux control structure:
73   * @type - mux type
74   * @last_chan - last register value
75   * @client - I2C device client
76   */
77  struct mlxcpld_mux {
78  enum mlxcpld_mux_type type;
79  u8 last_chan;
80  struct i2c_client *client;
81  };
82  
83  /* mlxcpld_mux_desc - mux descriptor structure:
84   * @nchans - number of channels
85   * @muxtype - physical mux type (LPC or I2C)
86   */
87  struct mlxcpld_mux_desc {
88  u8 nchans;
89  enum mlxcpld_mux_access_type muxtype;
90  };
91  
92  /* MUX logic description.
93   * This logic can be applied for LPC attached CPLD and fro I2C attached 
CPLD.
94   * Driver can support different mux control logic, according to CPLD
95   * implementation.
96   *
97   * Connectivity schema.
98   *
99   * i2c-mlxcpld Digital   
Analog
   100   * driver
   101   * ** * -> mux1 (virt bus2) -> 
mux -> |
   102   * | I2CLPC | i2c physical* -> mux2 (virt bus3) -> 
mux -> |
   103   * | bridge | bus 1 *-* 
  |
   104   * | logic  |-> * mux reg * 
  |
   105   * | in CPLD|   *-* 
  |
   106   * **   i2c-mux-mlxpcld  ^* -> muxn (virt busn) -> 
mux -> |
   107   * |driver   |  
  |
   108   * |*---*|  
Devices
   109   * |* CPLD (LPC bus)* select |
   110   * |* registers for **
   111   * |* mux selection * deselect

Re: [patch 2/2] i2c: mux: mellanox: add driver

2016-08-25 Thread Peter Rosin 
On 2016-08-24 18:20, Vadim Pasternak wrote:
> Hi Peter,
> 
> Thank you very much for your review.
> 
>> -Original Message-
>> From: Peter Rosin [mailto:p...@axentia.se]
>> Sent: Wednesday, August 24, 2016 4:55 PM
>> To: Vadim Pasternak <vad...@mellanox.com>; w...@the-dreams.de
>> Cc: linux-...@vger.kernel.org; linux-kernel@vger.kernel.org; 
>> j...@resnulli.us;
>> Michael Shych <michae...@mellanox.com>
>> Subject: Re: [patch 2/2] i2c: mux: mellanox: add driver
>>
>> On 2016-08-24 15:56, Vadim Pasternak wrote:
>>> From: Vadim Pasternak <vad...@mellanox.com>
>>>
>>> This driver allows I2C routing controlled through CPLD select
>>> registers on 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).
>>>
>>> MUX logic description.
>>> Mux selector can control 256 mux (channels), if utilized one CPLD
>>> register
>>> (8 bits) as select register - register value specifies mux id.
>>> Mux selector can control n*256 mux, if utilized n CPLD registers as
>>> select registers.
>>> The number of registers within the same CPLD can be combined to
>>> support mux hierarchy.
>>> This logic can be applied for LPC attached CPLD and fro I2C attached CPLD.
>>> Driver can support different mux control logic, according to CPLD
>>> implementation.
>>
>> This paragraph is strangely wrapped. And please limit to 75 chars per line 
>> as per
>> checkpatch.
>>
>> Also, I have a hard time getting the grips on the actual number of mux 
>> channels
>> that can be controlled. You talk about n * 256 channels if you have n 
>> registers.
>> But the code below appears to only deal with one register. So why complicate
>> things in the comments and the commit message? It is also not entirely clear 
>> to
>> me if you support 8 channels or if you really support 256 channels. That 
>> would
>> be huge number of pins.
> 
> In CPLD we have three channel selection registers, which one can support up 
> to 256 channels.
> CPLD could be programmed with the bigger number of selection registers.
> I tried to explain it in description.
> Since it's misleading, I'll remove it.

I'm a curious bastard though, and now I want to know how it works :-)
So, the CPLD supports up to 768 channels, using three registers, but there
is no HW that makes use of all of the options of this building block?
At least there's no such "big" system yet?

But then I wonder if that wouldn't be three parallel muxes, i.e. one mux
for each register? Otherwise you would only need two more bits to get
three times the number of channels...

>>
>> Because from the text above, I would have guessed 256, but below...
>>
>>> 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 (LPC bus)* select |
>>> |* registers for **
>>> |* mux selection * deselect
>>> |*---*
>>> | |
>>> <> <--->
>>> i2c cntrl  Board cntrl reg
>>> reg space  space (mux select,
>>> |  IO, LED, WD, info)
>>> | |  *-*   *-*
>>> *- LPC bus --| PCH |---| CPU |
>>>  *-*   *-*
>>>
>>> i2c-mux-mlxpcld does not necessary required i2c-mlxcpld. It can be use
>>> along with another bus driver, and still control i2c routing through
>>> CPLD mux selection, in case the system is equipped with CPLD capable
>>> of mux selection control.
>

Re: [patch 2/2] i2c: mux: mellanox: add driver

2016-08-25 Thread Peter Rosin 
On 2016-08-24 18:20, Vadim Pasternak wrote:
> Hi Peter,
> 
> Thank you very much for your review.
> 
>> -Original Message-
>> From: Peter Rosin [mailto:p...@axentia.se]
>> Sent: Wednesday, August 24, 2016 4:55 PM
>> To: Vadim Pasternak ; w...@the-dreams.de
>> Cc: linux-...@vger.kernel.org; linux-kernel@vger.kernel.org; 
>> j...@resnulli.us;
>> Michael Shych 
>> Subject: Re: [patch 2/2] i2c: mux: mellanox: add driver
>>
>> On 2016-08-24 15:56, Vadim Pasternak wrote:
>>> From: Vadim Pasternak 
>>>
>>> This driver allows I2C routing controlled through CPLD select
>>> registers on 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).
>>>
>>> MUX logic description.
>>> Mux selector can control 256 mux (channels), if utilized one CPLD
>>> register
>>> (8 bits) as select register - register value specifies mux id.
>>> Mux selector can control n*256 mux, if utilized n CPLD registers as
>>> select registers.
>>> The number of registers within the same CPLD can be combined to
>>> support mux hierarchy.
>>> This logic can be applied for LPC attached CPLD and fro I2C attached CPLD.
>>> Driver can support different mux control logic, according to CPLD
>>> implementation.
>>
>> This paragraph is strangely wrapped. And please limit to 75 chars per line 
>> as per
>> checkpatch.
>>
>> Also, I have a hard time getting the grips on the actual number of mux 
>> channels
>> that can be controlled. You talk about n * 256 channels if you have n 
>> registers.
>> But the code below appears to only deal with one register. So why complicate
>> things in the comments and the commit message? It is also not entirely clear 
>> to
>> me if you support 8 channels or if you really support 256 channels. That 
>> would
>> be huge number of pins.
> 
> In CPLD we have three channel selection registers, which one can support up 
> to 256 channels.
> CPLD could be programmed with the bigger number of selection registers.
> I tried to explain it in description.
> Since it's misleading, I'll remove it.

I'm a curious bastard though, and now I want to know how it works :-)
So, the CPLD supports up to 768 channels, using three registers, but there
is no HW that makes use of all of the options of this building block?
At least there's no such "big" system yet?

But then I wonder if that wouldn't be three parallel muxes, i.e. one mux
for each register? Otherwise you would only need two more bits to get
three times the number of channels...

>>
>> Because from the text above, I would have guessed 256, but below...
>>
>>> 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 (LPC bus)* select |
>>> |* registers for **
>>> |* mux selection * deselect
>>> |*---*
>>> | |
>>> <> <--->
>>> i2c cntrl  Board cntrl reg
>>> reg space  space (mux select,
>>> |  IO, LED, WD, info)
>>> | |  *-*   *-*
>>> *- LPC bus --| PCH |---| CPU |
>>>  *-*   *-*
>>>
>>> i2c-mux-mlxpcld does not necessary required i2c-mlxcpld. It can be use
>>> along with another bus driver, and still control i2c routing through
>>> CPLD mux selection, in case the system is equipped with CPLD capable
>>> of mux selection control.
>>>
>>> The Kconfig currently controlling compilation of this code is:

RE: [patch 2/2] i2c: mux: mellanox: add driver

2016-08-25 Thread Vadim Pasternak 


> -Original Message-
> From: Peter Rosin [mailto:p...@axentia.se]
> Sent: Thursday, August 25, 2016 11:53 AM
> To: Vadim Pasternak <vad...@mellanox.com>; w...@the-dreams.de
> Cc: linux-...@vger.kernel.org; linux-kernel@vger.kernel.org; j...@resnulli.us;
> Michael Shych <michae...@mellanox.com>
> Subject: Re: [patch 2/2] i2c: mux: mellanox: add driver
> 
> On 2016-08-24 18:20, Vadim Pasternak wrote:
> > Hi Peter,
> >
> > Thank you very much for your review.
> >
> >> -Original Message-
> >> From: Peter Rosin [mailto:p...@axentia.se]
> >> Sent: Wednesday, August 24, 2016 4:55 PM
> >> To: Vadim Pasternak <vad...@mellanox.com>; w...@the-dreams.de
> >> Cc: linux-...@vger.kernel.org; linux-kernel@vger.kernel.org;
> >> j...@resnulli.us; Michael Shych <michae...@mellanox.com>
> >> Subject: Re: [patch 2/2] i2c: mux: mellanox: add driver
> >>
> >> On 2016-08-24 15:56, Vadim Pasternak wrote:
> >>> From: Vadim Pasternak <vad...@mellanox.com>
> >>>
> >>> This driver allows I2C routing controlled through CPLD select
> >>> registers on 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).
> >>>
> >>> MUX logic description.
> >>> Mux selector can control 256 mux (channels), if utilized one CPLD
> >>> register
> >>> (8 bits) as select register - register value specifies mux id.
> >>> Mux selector can control n*256 mux, if utilized n CPLD registers as
> >>> select registers.
> >>> The number of registers within the same CPLD can be combined to
> >>> support mux hierarchy.
> >>> This logic can be applied for LPC attached CPLD and fro I2C attached CPLD.
> >>> Driver can support different mux control logic, according to CPLD
> >>> implementation.
> >>
> >> This paragraph is strangely wrapped. And please limit to 75 chars per
> >> line as per checkpatch.
> >>
> >> Also, I have a hard time getting the grips on the actual number of
> >> mux channels that can be controlled. You talk about n * 256 channels if you
> have n registers.
> >> But the code below appears to only deal with one register. So why
> >> complicate things in the comments and the commit message? It is also
> >> not entirely clear to me if you support 8 channels or if you really
> >> support 256 channels. That would be huge number of pins.
> >
> > In CPLD we have three channel selection registers, which one can support up
> to 256 channels.
> > CPLD could be programmed with the bigger number of selection registers.
> > I tried to explain it in description.
> > Since it's misleading, I'll remove it.
> 
> I'm a curious bastard though, and now I want to know how it works :-) So, the
> CPLD supports up to 768 channels, using three registers, but there is no HW 
> that
> makes use of all of the options of this building block?
> At least there's no such "big" system yet?
> 

Right.
On our TOR systems we are using only 16 channels, controlled by two registers - 
one for channels on main board, the other on main board.
We also have director systems, that we are using on management board up to 48 
channels (on such systems we have up-to 648 100G ports.
On these systems we also have such CPLD on modules (leafs, spines), which 
controls devices on modules (also on each modules we have 16 channels).
On management board of such system we have 113 channels (adapters from i2c1 to 
i2c-113), where 16 channels are on top of headachy:
i2c-1: has multiplexed buses from 1 to 16
i2c-3: has multiplexed buses bus 42 to 65
i2c-4: has multiplexed buses bus 66 to 89
i2c-4: has multiplexed buses bus 90 to 113
These channels go to the modules (PSU, FAN, leaf, spine). 
On such system we have 5 registers in CPLD for channel selection, two of top 
headachy (on main board and on switch board), each controls 8 channels. Three 
of 2-nd level of hierarchy, each controls 24 channels.

Each register of 8 bytes, so theoretically you can support 255 channels through 
1 register.
In CPLD you can program each register as channel selection (if necessary and if 
you have spares).

CPLD can be connected to LPC, I2C (we don't have other connections). We are 
using Lattice devices.

> But then I wonder if that wouldn't be three parallel muxes, i.e. one mux for 
> each
> register? Otherwise you would only need two more bits to get three times the
>

RE: [patch 2/2] i2c: mux: mellanox: add driver

2016-08-25 Thread Vadim Pasternak 


> -Original Message-
> From: Peter Rosin [mailto:p...@axentia.se]
> Sent: Thursday, August 25, 2016 11:53 AM
> To: Vadim Pasternak ; w...@the-dreams.de
> Cc: linux-...@vger.kernel.org; linux-kernel@vger.kernel.org; j...@resnulli.us;
> Michael Shych 
> Subject: Re: [patch 2/2] i2c: mux: mellanox: add driver
> 
> On 2016-08-24 18:20, Vadim Pasternak wrote:
> > Hi Peter,
> >
> > Thank you very much for your review.
> >
> >> -Original Message-
> >> From: Peter Rosin [mailto:p...@axentia.se]
> >> Sent: Wednesday, August 24, 2016 4:55 PM
> >> To: Vadim Pasternak ; w...@the-dreams.de
> >> Cc: linux-...@vger.kernel.org; linux-kernel@vger.kernel.org;
> >> j...@resnulli.us; Michael Shych 
> >> Subject: Re: [patch 2/2] i2c: mux: mellanox: add driver
> >>
> >> On 2016-08-24 15:56, Vadim Pasternak wrote:
> >>> From: Vadim Pasternak 
> >>>
> >>> This driver allows I2C routing controlled through CPLD select
> >>> registers on 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).
> >>>
> >>> MUX logic description.
> >>> Mux selector can control 256 mux (channels), if utilized one CPLD
> >>> register
> >>> (8 bits) as select register - register value specifies mux id.
> >>> Mux selector can control n*256 mux, if utilized n CPLD registers as
> >>> select registers.
> >>> The number of registers within the same CPLD can be combined to
> >>> support mux hierarchy.
> >>> This logic can be applied for LPC attached CPLD and fro I2C attached CPLD.
> >>> Driver can support different mux control logic, according to CPLD
> >>> implementation.
> >>
> >> This paragraph is strangely wrapped. And please limit to 75 chars per
> >> line as per checkpatch.
> >>
> >> Also, I have a hard time getting the grips on the actual number of
> >> mux channels that can be controlled. You talk about n * 256 channels if you
> have n registers.
> >> But the code below appears to only deal with one register. So why
> >> complicate things in the comments and the commit message? It is also
> >> not entirely clear to me if you support 8 channels or if you really
> >> support 256 channels. That would be huge number of pins.
> >
> > In CPLD we have three channel selection registers, which one can support up
> to 256 channels.
> > CPLD could be programmed with the bigger number of selection registers.
> > I tried to explain it in description.
> > Since it's misleading, I'll remove it.
> 
> I'm a curious bastard though, and now I want to know how it works :-) So, the
> CPLD supports up to 768 channels, using three registers, but there is no HW 
> that
> makes use of all of the options of this building block?
> At least there's no such "big" system yet?
> 

Right.
On our TOR systems we are using only 16 channels, controlled by two registers - 
one for channels on main board, the other on main board.
We also have director systems, that we are using on management board up to 48 
channels (on such systems we have up-to 648 100G ports.
On these systems we also have such CPLD on modules (leafs, spines), which 
controls devices on modules (also on each modules we have 16 channels).
On management board of such system we have 113 channels (adapters from i2c1 to 
i2c-113), where 16 channels are on top of headachy:
i2c-1: has multiplexed buses from 1 to 16
i2c-3: has multiplexed buses bus 42 to 65
i2c-4: has multiplexed buses bus 66 to 89
i2c-4: has multiplexed buses bus 90 to 113
These channels go to the modules (PSU, FAN, leaf, spine). 
On such system we have 5 registers in CPLD for channel selection, two of top 
headachy (on main board and on switch board), each controls 8 channels. Three 
of 2-nd level of hierarchy, each controls 24 channels.

Each register of 8 bytes, so theoretically you can support 255 channels through 
1 register.
In CPLD you can program each register as channel selection (if necessary and if 
you have spares).

CPLD can be connected to LPC, I2C (we don't have other connections). We are 
using Lattice devices.

> But then I wonder if that wouldn't be three parallel muxes, i.e. one mux for 
> each
> register? Otherwise you would only need two more bits to get three times the
> number of channels...

It's all depends on system topology.
Practically you can't control all channels from one register, because of system 
confi

Re: [patch 2/2] i2c: mux: mellanox: add driver

2016-08-25 Thread kbuild test robot 
Hi Vadim,

[auto build test WARNING on wsa/i2c/for-next]
[also build test WARNING on v4.8-rc3 next-20160824]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]
[Suggest to use git(>=2.9.0) format-patch --base= (or --base=auto for 
convenience) to record what (public, well-known) commit your patch series was 
built on]
[Check https://git-scm.com/docs/git-format-patch for more information]

url:
https://github.com/0day-ci/linux/commits/vadimp-mellanox-com/i2c-add-master-driver-for-mellanox-systems/20160824-200057
base:   https://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git 
i2c/for-next


coccinelle warnings: (new ones prefixed by >>)

>> drivers/i2c/muxes/i2c-mux-mlxcpld.c:329:3-8: No need to set .owner here. The 
>> core will do it.

Please review and possibly fold the followup patch.

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation

Re: [patch 2/2] i2c: mux: mellanox: add driver

2016-08-25 Thread kbuild test robot 
Hi Vadim,

[auto build test WARNING on wsa/i2c/for-next]
[also build test WARNING on v4.8-rc3 next-20160824]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]
[Suggest to use git(>=2.9.0) format-patch --base= (or --base=auto for 
convenience) to record what (public, well-known) commit your patch series was 
built on]
[Check https://git-scm.com/docs/git-format-patch for more information]

url:
https://github.com/0day-ci/linux/commits/vadimp-mellanox-com/i2c-add-master-driver-for-mellanox-systems/20160824-200057
base:   https://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git 
i2c/for-next


coccinelle warnings: (new ones prefixed by >>)

>> drivers/i2c/muxes/i2c-mux-mlxcpld.c:329:3-8: No need to set .owner here. The 
>> core will do it.

Please review and possibly fold the followup patch.

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation

RE: [patch 2/2] i2c: mux: mellanox: add driver

2016-08-24 Thread Vadim Pasternak 
Hi Peter,

Thank you very much for your review.

> -Original Message-
> From: Peter Rosin [mailto:p...@axentia.se]
> Sent: Wednesday, August 24, 2016 4:55 PM
> To: Vadim Pasternak <vad...@mellanox.com>; w...@the-dreams.de
> Cc: linux-...@vger.kernel.org; linux-kernel@vger.kernel.org; j...@resnulli.us;
> Michael Shych <michae...@mellanox.com>
> Subject: Re: [patch 2/2] i2c: mux: mellanox: add driver
> 
> On 2016-08-24 15:56, Vadim Pasternak wrote:
> > From: Vadim Pasternak <vad...@mellanox.com>
> >
> > This driver allows I2C routing controlled through CPLD select
> > registers on 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).
> >
> > MUX logic description.
> > Mux selector can control 256 mux (channels), if utilized one CPLD
> > register
> > (8 bits) as select register - register value specifies mux id.
> > Mux selector can control n*256 mux, if utilized n CPLD registers as
> > select registers.
> > The number of registers within the same CPLD can be combined to
> > support mux hierarchy.
> > This logic can be applied for LPC attached CPLD and fro I2C attached CPLD.
> > Driver can support different mux control logic, according to CPLD
> > implementation.
> 
> This paragraph is strangely wrapped. And please limit to 75 chars per line as 
> per
> checkpatch.
> 
> Also, I have a hard time getting the grips on the actual number of mux 
> channels
> that can be controlled. You talk about n * 256 channels if you have n 
> registers.
> But the code below appears to only deal with one register. So why complicate
> things in the comments and the commit message? It is also not entirely clear 
> to
> me if you support 8 channels or if you really support 256 channels. That would
> be huge number of pins.

In CPLD we have three channel selection registers, which one can support up to 
256 channels.
CPLD could be programmed with the bigger number of selection registers.
I tried to explain it in description.
Since it's misleading, I'll remove it.

> 
> Because from the text above, I would have guessed 256, but below...
> 
> > 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 (LPC bus)* select |
> > |* registers for **
> > |* mux selection * deselect
> > |*---*
> > | |
> > <> <--->
> > i2c cntrl  Board cntrl reg
> > reg space  space (mux select,
> > |  IO, LED, WD, info)
> > | |  *-*   *-*
> > *- LPC bus --| PCH |---| CPU |
> >  *-*   *-*
> >
> > i2c-mux-mlxpcld does not necessary required i2c-mlxcpld. It can be use
> > along with another bus driver, and still control i2c routing through
> > CPLD mux selection, in case the system is equipped with CPLD capable
> > of mux selection control.
> >
> > The Kconfig currently controlling compilation of this code is:
> > drivers/i2c/muxes/Kconfig:config I2C_MUX_MLXCPLD
> >
> > Signed-off-by: Michael Shych <michae...@mellanox.com>
> > Signed-off-by: Vadim Pasternak <vad...@mellanox.com>
> > Reviewed-by: Jiri Pirko <j...@mellanox.com>
> > ---
> >  drivers/i2c/muxes/Kconfig   |  11 ++
> >  drivers/i2c/muxes/Makefile  |   1 +
> >  drivers/i2c/muxes/i2c-mux-mlxcpld.c | 352
> 
> >  include/linux/i2c/mlxcpld.h |  67 +++
> >  4 files changed, 431 insertions(+)
> >  create mode 100644 drivers/i2c/muxes/i2c-mux-mlxcpld.c
> >  create mode 100644 include/linux/i2c/mlxcpld.h
> >

RE: [patch 2/2] i2c: mux: mellanox: add driver

2016-08-24 Thread Vadim Pasternak 
Hi Peter,

Thank you very much for your review.

> -Original Message-
> From: Peter Rosin [mailto:p...@axentia.se]
> Sent: Wednesday, August 24, 2016 4:55 PM
> To: Vadim Pasternak ; w...@the-dreams.de
> Cc: linux-...@vger.kernel.org; linux-kernel@vger.kernel.org; j...@resnulli.us;
> Michael Shych 
> Subject: Re: [patch 2/2] i2c: mux: mellanox: add driver
> 
> On 2016-08-24 15:56, Vadim Pasternak wrote:
> > From: Vadim Pasternak 
> >
> > This driver allows I2C routing controlled through CPLD select
> > registers on 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).
> >
> > MUX logic description.
> > Mux selector can control 256 mux (channels), if utilized one CPLD
> > register
> > (8 bits) as select register - register value specifies mux id.
> > Mux selector can control n*256 mux, if utilized n CPLD registers as
> > select registers.
> > The number of registers within the same CPLD can be combined to
> > support mux hierarchy.
> > This logic can be applied for LPC attached CPLD and fro I2C attached CPLD.
> > Driver can support different mux control logic, according to CPLD
> > implementation.
> 
> This paragraph is strangely wrapped. And please limit to 75 chars per line as 
> per
> checkpatch.
> 
> Also, I have a hard time getting the grips on the actual number of mux 
> channels
> that can be controlled. You talk about n * 256 channels if you have n 
> registers.
> But the code below appears to only deal with one register. So why complicate
> things in the comments and the commit message? It is also not entirely clear 
> to
> me if you support 8 channels or if you really support 256 channels. That would
> be huge number of pins.

In CPLD we have three channel selection registers, which one can support up to 
256 channels.
CPLD could be programmed with the bigger number of selection registers.
I tried to explain it in description.
Since it's misleading, I'll remove it.

> 
> Because from the text above, I would have guessed 256, but below...
> 
> > 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 (LPC bus)* select |
> > |* registers for **
> > |* mux selection * deselect
> > |*---*
> > | |
> > <> <--->
> > i2c cntrl  Board cntrl reg
> > reg space  space (mux select,
> > |  IO, LED, WD, info)
> > | |  *-*   *-*
> > *- LPC bus --| PCH |---| CPU |
> >  *-*   *-*
> >
> > i2c-mux-mlxpcld does not necessary required i2c-mlxcpld. It can be use
> > along with another bus driver, and still control i2c routing through
> > CPLD mux selection, in case the system is equipped with CPLD capable
> > of mux selection control.
> >
> > The Kconfig currently controlling compilation of this code is:
> > drivers/i2c/muxes/Kconfig:config I2C_MUX_MLXCPLD
> >
> > Signed-off-by: Michael Shych 
> > Signed-off-by: Vadim Pasternak 
> > Reviewed-by: Jiri Pirko 
> > ---
> >  drivers/i2c/muxes/Kconfig   |  11 ++
> >  drivers/i2c/muxes/Makefile  |   1 +
> >  drivers/i2c/muxes/i2c-mux-mlxcpld.c | 352
> 
> >  include/linux/i2c/mlxcpld.h |  67 +++
> >  4 files changed, 431 insertions(+)
> >  create mode 100644 drivers/i2c/muxes/i2c-mux-mlxcpld.c
> >  create mode 100644 include/linux/i2c/mlxcpld.h
> >
> > diff --git a/drivers/i2c/muxes/Kconfig b/drivers/i2c/muxes/Kconfig
> > index e280c8e..b7ab144 100644
> > --- a/drivers/i2c/muxes/Kconfig
&

Re: [patch 2/2] i2c: mux: mellanox: add driver

2016-08-24 Thread Peter Rosin 
On 2016-08-24 15:56, Vadim Pasternak wrote:
> From: Vadim Pasternak 
> 
> This driver allows I2C routing controlled through CPLD select registers on 
> 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).
> 
> MUX logic description.
> Mux selector can control 256 mux (channels), if utilized one CPLD register
> (8 bits) as select register - register value specifies mux id.
> Mux selector can control n*256 mux, if utilized n CPLD registers as select
> registers.
> The number of registers within the same CPLD can be combined to support mux
> hierarchy.
> This logic can be applied for LPC attached CPLD and fro I2C attached CPLD.
> Driver can support different mux control logic, according to CPLD
> implementation.

This paragraph is strangely wrapped. And please limit to 75 chars per line
as per checkpatch.

Also, I have a hard time getting the grips on the actual number of mux
channels that can be controlled. You talk about n * 256 channels if you
have n registers. But the code below appears to only deal with one
register. So why complicate things in the comments and the commit
message? It is also not entirely clear to me if you support 8 channels
or if you really support 256 channels. That would be huge number of
pins.

Because from the text above, I would have guessed 256, but below...

> 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 (LPC bus)* select |
> |* registers for **
> |* mux selection * deselect
> |*---*
> | |
> <> <--->
> i2c cntrl  Board cntrl reg
> reg space  space (mux select,
> |  IO, LED, WD, info)
> | |  *-*   *-*
> *- LPC bus --| PCH |---| CPU |
>  *-*   *-*
> 
> i2c-mux-mlxpcld does not necessary required i2c-mlxcpld. It can be use along
> with another bus driver, and still control i2c routing through CPLD mux
> selection, in case the system is equipped with CPLD capable of mux selection
> control.
> 
> The Kconfig currently controlling compilation of this code is:
> drivers/i2c/muxes/Kconfig:config I2C_MUX_MLXCPLD
> 
> Signed-off-by: Michael Shych 
> Signed-off-by: Vadim Pasternak 
> Reviewed-by: Jiri Pirko 
> ---
>  drivers/i2c/muxes/Kconfig   |  11 ++
>  drivers/i2c/muxes/Makefile  |   1 +
>  drivers/i2c/muxes/i2c-mux-mlxcpld.c | 352 
> 
>  include/linux/i2c/mlxcpld.h |  67 +++
>  4 files changed, 431 insertions(+)
>  create mode 100644 drivers/i2c/muxes/i2c-mux-mlxcpld.c
>  create mode 100644 include/linux/i2c/mlxcpld.h
> 
> diff --git a/drivers/i2c/muxes/Kconfig b/drivers/i2c/muxes/Kconfig
> index e280c8e..b7ab144 100644
> --- a/drivers/i2c/muxes/Kconfig
> +++ b/drivers/i2c/muxes/Kconfig
> @@ -81,4 +81,15 @@ config I2C_DEMUX_PINCTRL
> demultiplexer that uses the pinctrl subsystem. This is useful if you
> want to change the I2C master at run-time depending on features.
>  
> +config I2C_MUX_MLXCPLD
> +tristate "Mellanox CPLD based I2C multiplexer"
> +help
> +  If you say yes to this option, support will be included for a
> +  CPLD based I2C multiplexer. This driver provides access to
> +  I2C busses connected through a MUX, which is controlled
> +  by a CPLD registers.
> +
> +  This driver can also be built as a module.  If so, the module
> +  will be called i2c-mux-mlxcpld.
> +
>  endmenu
> diff --git a/drivers/i2c/muxes/Makefile b/drivers/i2c/muxes/Makefile
> index 7c267c2..e5c990e 100644
> --- a/drivers/i2c/muxes/Makefile
> +++ b/drivers/i2c/muxes/Makefile
> @@ -10,5 +10,6 @@ obj-$(CONFIG_I2C_MUX_PCA9541)   += i2c-mux-pca9541.o
>  obj-$(CONFIG_I2C_MUX_PCA954x)+= i2c-mux-pca954x.o
>  obj-$(CONFIG_I2C_MUX_PINCTRL)+= i2c-mux-pinctrl.o
>  obj-$(CONFIG_I2C_MUX_REG)+= i2c-mux-reg.o
> +obj-$(CONFIG_I2C_MUX_MLXCPLD)+= i2c-mux-mlxcpld.o
>

Re: [patch 2/2] i2c: mux: mellanox: add driver

2016-08-24 Thread Peter Rosin 
On 2016-08-24 15:56, Vadim Pasternak wrote:
> From: Vadim Pasternak 
> 
> This driver allows I2C routing controlled through CPLD select registers on 
> 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).
> 
> MUX logic description.
> Mux selector can control 256 mux (channels), if utilized one CPLD register
> (8 bits) as select register - register value specifies mux id.
> Mux selector can control n*256 mux, if utilized n CPLD registers as select
> registers.
> The number of registers within the same CPLD can be combined to support mux
> hierarchy.
> This logic can be applied for LPC attached CPLD and fro I2C attached CPLD.
> Driver can support different mux control logic, according to CPLD
> implementation.

This paragraph is strangely wrapped. And please limit to 75 chars per line
as per checkpatch.

Also, I have a hard time getting the grips on the actual number of mux
channels that can be controlled. You talk about n * 256 channels if you
have n registers. But the code below appears to only deal with one
register. So why complicate things in the comments and the commit
message? It is also not entirely clear to me if you support 8 channels
or if you really support 256 channels. That would be huge number of
pins.

Because from the text above, I would have guessed 256, but below...

> 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 (LPC bus)* select |
> |* registers for **
> |* mux selection * deselect
> |*---*
> | |
> <> <--->
> i2c cntrl  Board cntrl reg
> reg space  space (mux select,
> |  IO, LED, WD, info)
> | |  *-*   *-*
> *- LPC bus --| PCH |---| CPU |
>  *-*   *-*
> 
> i2c-mux-mlxpcld does not necessary required i2c-mlxcpld. It can be use along
> with another bus driver, and still control i2c routing through CPLD mux
> selection, in case the system is equipped with CPLD capable of mux selection
> control.
> 
> The Kconfig currently controlling compilation of this code is:
> drivers/i2c/muxes/Kconfig:config I2C_MUX_MLXCPLD
> 
> Signed-off-by: Michael Shych 
> Signed-off-by: Vadim Pasternak 
> Reviewed-by: Jiri Pirko 
> ---
>  drivers/i2c/muxes/Kconfig   |  11 ++
>  drivers/i2c/muxes/Makefile  |   1 +
>  drivers/i2c/muxes/i2c-mux-mlxcpld.c | 352 
> 
>  include/linux/i2c/mlxcpld.h |  67 +++
>  4 files changed, 431 insertions(+)
>  create mode 100644 drivers/i2c/muxes/i2c-mux-mlxcpld.c
>  create mode 100644 include/linux/i2c/mlxcpld.h
> 
> diff --git a/drivers/i2c/muxes/Kconfig b/drivers/i2c/muxes/Kconfig
> index e280c8e..b7ab144 100644
> --- a/drivers/i2c/muxes/Kconfig
> +++ b/drivers/i2c/muxes/Kconfig
> @@ -81,4 +81,15 @@ config I2C_DEMUX_PINCTRL
> demultiplexer that uses the pinctrl subsystem. This is useful if you
> want to change the I2C master at run-time depending on features.
>  
> +config I2C_MUX_MLXCPLD
> +tristate "Mellanox CPLD based I2C multiplexer"
> +help
> +  If you say yes to this option, support will be included for a
> +  CPLD based I2C multiplexer. This driver provides access to
> +  I2C busses connected through a MUX, which is controlled
> +  by a CPLD registers.
> +
> +  This driver can also be built as a module.  If so, the module
> +  will be called i2c-mux-mlxcpld.
> +
>  endmenu
> diff --git a/drivers/i2c/muxes/Makefile b/drivers/i2c/muxes/Makefile
> index 7c267c2..e5c990e 100644
> --- a/drivers/i2c/muxes/Makefile
> +++ b/drivers/i2c/muxes/Makefile
> @@ -10,5 +10,6 @@ obj-$(CONFIG_I2C_MUX_PCA9541)   += i2c-mux-pca9541.o
>  obj-$(CONFIG_I2C_MUX_PCA954x)+= i2c-mux-pca954x.o
>  obj-$(CONFIG_I2C_MUX_PINCTRL)+= i2c-mux-pinctrl.o
>  obj-$(CONFIG_I2C_MUX_REG)+= i2c-mux-reg.o
> +obj-$(CONFIG_I2C_MUX_MLXCPLD)+= i2c-mux-mlxcpld.o
>  
>  ccflags-$(CONFIG_I2C_DEBUG_BUS) := -DDEBUG
> diff --git