All of lore.kernel.org
 help / color / mirror / Atom feed
* [patch v5] i2c: mux: mellanox: add driver
@ 2016-09-13 20:37 vadimp
  2016-09-14  7:49   ` Peter Rosin
  2016-09-23  9:36   ` Peter Rosin
  0 siblings, 2 replies; 13+ messages in thread
From: vadimp @ 2016-09-13 20:37 UTC (permalink / raw)
  To: wsa, peda; +Cc: linux-i2c, linux-kernel, jiri, Vadim Pasternak, Michael Shych

From: Vadim Pasternak <vadimp@mellanox.com>

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)

i2c-mux-mlxpcld does not necessarily require i2c-mlxcpld. It can be used
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 <michaelsh@mellanox.com>
Signed-off-by: Vadim Pasternak <vadimp@mellanox.com>
Reviewed-by: Jiri Pirko <jiri@mellanox.com>
---
v4->v5
 Fixes issues pointed out by Peter:
  - Fix text is submit message;
  - Fix text in Kconfig;
  - Arrange Makefile in alphabetic order;
  - Fix several comments;
  - Drop assignment for force to zero in probe;
  - Remove error message in probe - error message from i2c_mux_add_adapter
    is sufficient;
v3->v4
 Fixes issues pointed out by Peter:
  - Remove mlxcpld_mux_type;
  - Remove member type from mux control structure;
  - Remove variable muxes;
  - mlxcpld_mux_reg_write - set address from client;
  - mlxcpld_mux_reg_write - try smbus_xfer
  - mlxcpld_mux_select_chan - remove compare with for mux type;
  - mlxcpld_mux_probe - use I2C_FUNC_SMBUS_WRITE_BYTE_DATA instead of
    I2C_FUNC_SMBUS_BYTE
  - mlxcpld_mux_probe - remove switch statement
  - mlxcpld_mux_probe - update comment;
  - mlxcpld_mux_probe -change logic in testing num_adaps;
  - remove members first_channel and addr from mlxcpld_mux_plat_data
    structure;
v2->v3
 Fixes issues pointed out by Peter:
  - Fix several comments;
  - In MAINTAINERS file use linux-i2c instead of linux-kernel;
  - Place include files in alphabetic order;
  - When channel select fail, set last_chan to zero for cleaning last value;
  - Return with error from probe if there is no platform data;
  - Use i2c_mux_reg driver for the lpc_access cases:
    it fit well for Mellanox system with LPC access - has been tested and it
	works good.
  - Limit this driver to i2c_access only one device (mlxcpld_mux_module).
v1->v2
 Fixes issues pointed out by Peter:
  - changes in commit cover massage;
  - change leg to channel in comments;
  - missed return err;
  - use platform data mux array rather the offset from 1st channel in probe
    routine;
  - remove owner field from driver structure;
  - use module_i2c_driver macros, instead if __init and __exit;
  - remove deselect on exit flag;
  - add record to MAINTAINER file;
---
 MAINTAINERS                         |   8 ++
 drivers/i2c/muxes/Kconfig           |  11 ++
 drivers/i2c/muxes/Makefile          |   1 +
 drivers/i2c/muxes/i2c-mux-mlxcpld.c | 220 ++++++++++++++++++++++++++++++++++++
 include/linux/i2c/mlxcpld.h         |  52 +++++++++
 5 files changed, 292 insertions(+)
 create mode 100644 drivers/i2c/muxes/i2c-mux-mlxcpld.c
 create mode 100644 include/linux/i2c/mlxcpld.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 0bbe4b1..be83d69 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7655,6 +7655,14 @@ W:	http://www.mellanox.com
 Q:	http://patchwork.ozlabs.org/project/netdev/list/
 F:	drivers/net/ethernet/mellanox/mlxsw/
 
+MELLANOX MLXCPLD I2C MUX DRIVER
+M:	Vadim Pasternak <vadimp@mellanox.com>
+M:	Michael Shych <michaelsh@mellanox.com>
+L:	linux-i2c@vger.kernel.org
+S:	Supported
+W:	http://www.mellanox.com
+F:	drivers/i2c/muxes/i2c-mux-mlxcpld.c
+
 SOFT-ROCE DRIVER (rxe)
 M:	Moni Shoua <monis@mellanox.com>
 L:	linux-rdma@vger.kernel.org
diff --git a/drivers/i2c/muxes/Kconfig b/drivers/i2c/muxes/Kconfig
index e280c8e..d033402 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 register.
+
+          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..9948fa4 100644
--- a/drivers/i2c/muxes/Makefile
+++ b/drivers/i2c/muxes/Makefile
@@ -6,6 +6,7 @@ obj-$(CONFIG_I2C_ARB_GPIO_CHALLENGE)	+= i2c-arb-gpio-challenge.o
 obj-$(CONFIG_I2C_DEMUX_PINCTRL)		+= i2c-demux-pinctrl.o
 
 obj-$(CONFIG_I2C_MUX_GPIO)	+= i2c-mux-gpio.o
+obj-$(CONFIG_I2C_MUX_MLXCPLD)	+= i2c-mux-mlxcpld.o
 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
diff --git a/drivers/i2c/muxes/i2c-mux-mlxcpld.c b/drivers/i2c/muxes/i2c-mux-mlxcpld.c
new file mode 100644
index 0000000..20be25a
--- /dev/null
+++ b/drivers/i2c/muxes/i2c-mux-mlxcpld.c
@@ -0,0 +1,220 @@
+/*
+ * drivers/i2c/muxes/i2c-mux-mlxcpld.c
+ * Copyright (c) 2016 Mellanox Technologies. All rights reserved.
+ * Copyright (c) 2016 Michael Shych <michaels@mellanox.com>
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are met:
+ *
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in the
+ *    documentation and/or other materials provided with the distribution.
+ * 3. Neither the names of the copyright holders nor the names of its
+ *    contributors may be used to endorse or promote products derived from
+ *    this software without specific prior written permission.
+ *
+ * Alternatively, this software may be distributed under the terms of the
+ * GNU General Public License ("GPL") version 2 as published by the Free
+ * Software Foundation.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
+ * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE
+ * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
+ * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
+ * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
+ * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
+ * POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#include <linux/device.h>
+#include <linux/i2c.h>
+#include <linux/i2c-mux.h>
+#include <linux/io.h>
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <linux/version.h>
+#include <linux/i2c/mlxcpld.h>
+
+#define CPLD_MUX_MAX_NCHANS	8
+
+/* mlxcpld_mux - mux control structure:
+ * @last_chan - last register value
+ * @client - I2C device client
+ */
+struct mlxcpld_mux {
+	u8 last_chan;
+	struct i2c_client *client;
+};
+
+/* MUX logic description.
+ * Driver can support different mux control logic, according to CPLD
+ * implementation.
+ *
+ * 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)
+ *
+ */
+
+static const struct i2c_device_id mlxcpld_mux_id[] = {
+	{ "mlxcpld_mux_module", 0 },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, mlxcpld_mux_id);
+
+/* Write to mux register. Don't use i2c_transfer() and i2c_smbus_xfer()
+ * for this as they will try to lock adapter a second time.
+ */
+static int mlxcpld_mux_reg_write(struct i2c_adapter *adap,
+				 struct i2c_client *client, u8 val)
+{
+	struct mlxcpld_mux_plat_data *pdata = dev_get_platdata(&client->dev);
+
+	if (adap->algo->master_xfer) {
+		struct i2c_msg msg;
+		u8 msgbuf[] = {pdata->sel_reg_addr, val};
+
+		msg.addr = client->addr;
+		msg.flags = 0;
+		msg.len = 2;
+		msg.buf = msgbuf;
+		return __i2c_transfer(adap, &msg, 1);
+	} else if (adap->algo->smbus_xfer) {
+		union i2c_smbus_data data;
+
+		data.byte = val;
+		return adap->algo->smbus_xfer(adap, client->addr,
+					      client->flags, I2C_SMBUS_WRITE,
+					      pdata->sel_reg_addr,
+					      I2C_SMBUS_BYTE_DATA, &data);
+	} else
+		return -ENODEV;
+}
+
+static int mlxcpld_mux_select_chan(struct i2c_mux_core *muxc, u32 chan)
+{
+	struct mlxcpld_mux *data = i2c_mux_priv(muxc);
+	struct i2c_client *client = data->client;
+	u8 regval = chan + 1;
+	int err = 0;
+
+	/* Only select the channel if its different from the last channel */
+	if (data->last_chan != regval) {
+		err = mlxcpld_mux_reg_write(muxc->parent, client, regval);
+		if (err)
+			data->last_chan = 0;
+		else
+			data->last_chan = regval;
+	}
+
+	return err;
+}
+
+static int mlxcpld_mux_deselect(struct i2c_mux_core *muxc, u32 chan)
+{
+	struct mlxcpld_mux *data = i2c_mux_priv(muxc);
+	struct i2c_client *client = data->client;
+
+	/* Deselect active channel */
+	data->last_chan = 0;
+
+	return mlxcpld_mux_reg_write(muxc->parent, client, data->last_chan);
+}
+
+/* Probe/reomove functions */
+static int mlxcpld_mux_probe(struct i2c_client *client,
+			     const struct i2c_device_id *id)
+{
+	struct i2c_adapter *adap = to_i2c_adapter(client->dev.parent);
+	struct mlxcpld_mux_plat_data *pdata = dev_get_platdata(&client->dev);
+	struct i2c_mux_core *muxc;
+	int num, force;
+	struct mlxcpld_mux *data;
+	int err;
+
+	if (!pdata)
+		return -EINVAL;
+
+	if (!i2c_check_functionality(adap, I2C_FUNC_SMBUS_WRITE_BYTE_DATA))
+		return -ENODEV;
+
+	muxc = i2c_mux_alloc(adap, &client->dev, CPLD_MUX_MAX_NCHANS,
+			     sizeof(*data), 0, mlxcpld_mux_select_chan,
+			     mlxcpld_mux_deselect);
+	if (!muxc)
+		return -ENOMEM;
+
+	data = i2c_mux_priv(muxc);
+	i2c_set_clientdata(client, muxc);
+	data->client = client;
+	data->last_chan = 0; /* force the first selection */
+
+	/* Create an adapter for each channel. */
+	for (num = 0; num < CPLD_MUX_MAX_NCHANS; num++) {
+		if (num >= pdata->num_adaps)
+			/* discard unconfigured channels */
+			break;
+
+		force = pdata->adap_ids[num];
+
+		err = i2c_mux_add_adapter(muxc, force, num, 0);
+		if (err)
+			goto virt_reg_failed;
+	}
+
+	return 0;
+
+virt_reg_failed:
+	i2c_mux_del_adapters(muxc);
+	return err;
+}
+
+static int mlxcpld_mux_remove(struct i2c_client *client)
+{
+	struct i2c_mux_core *muxc = i2c_get_clientdata(client);
+
+	i2c_mux_del_adapters(muxc);
+	return 0;
+}
+
+static struct i2c_driver mlxcpld_mux_driver = {
+	.driver		= {
+		.name	= "mlxcpld-mux",
+	},
+	.probe		= mlxcpld_mux_probe,
+	.remove		= mlxcpld_mux_remove,
+	.id_table	= mlxcpld_mux_id,
+};
+
+module_i2c_driver(mlxcpld_mux_driver);
+
+MODULE_AUTHOR("Michael Shych (michaels@mellanox.com)");
+MODULE_DESCRIPTION("Mellanox I2C-CPLD-MUX driver");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform:i2c-mux-mlxcpld");
diff --git a/include/linux/i2c/mlxcpld.h b/include/linux/i2c/mlxcpld.h
new file mode 100644
index 0000000..b08dcb1
--- /dev/null
+++ b/include/linux/i2c/mlxcpld.h
@@ -0,0 +1,52 @@
+/*
+ * mlxcpld.h - Mellanox I2C multiplexer support in CPLD
+ *
+ * Copyright (c) 2016 Mellanox Technologies. All rights reserved.
+ * Copyright (c) 2016 Michael Shych <michaels@mellanox.com>
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are met:
+ *
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in the
+ *    documentation and/or other materials provided with the distribution.
+ * 3. Neither the names of the copyright holders nor the names of its
+ *    contributors may be used to endorse or promote products derived from
+ *    this software without specific prior written permission.
+ *
+ * Alternatively, this software may be distributed under the terms of the
+ * GNU General Public License ("GPL") version 2 as published by the Free
+ * Software Foundation.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
+ * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE
+ * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
+ * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
+ * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
+ * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
+ * POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#ifndef _LINUX_I2C_MLXCPLD_H
+#define _LINUX_I2C_MLXCPLD_H
+
+/* Platform data for the CPLD I2C multiplexers */
+
+/* mlxcpld_mux_plat_data - per mux data, used with i2c_register_board_info
+ * @adap_ids - adapter array
+ * @num_adaps - number of adapters
+ * @sel_reg_addr - mux select register offset in CPLD space
+ */
+struct mlxcpld_mux_plat_data {
+	int *adap_ids;
+	int num_adaps;
+	int sel_reg_addr;
+};
+
+#endif /* _LINUX_I2C_MLXCPLD_H */
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [patch v5] i2c: mux: mellanox: add driver
  2016-09-13 20:37 [patch v5] i2c: mux: mellanox: add driver vadimp
@ 2016-09-14  7:49   ` Peter Rosin
  2016-09-23  9:36   ` Peter Rosin
  1 sibling, 0 replies; 13+ messages in thread
From: Peter Rosin @ 2016-09-14  7:49 UTC (permalink / raw)
  To: vadimp, wsa; +Cc: linux-i2c, linux-kernel, jiri, Michael Shych

On 2016-09-13 22:37, vadimp@mellanox.com wrote:
> From: Vadim Pasternak <vadimp@mellanox.com>
> 
> 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).

*snip*

> +/* Platform data for the CPLD I2C multiplexers */
> +
> +/* mlxcpld_mux_plat_data - per mux data, used with i2c_register_board_info
> + * @adap_ids - adapter array
> + * @num_adaps - number of adapters
> + * @sel_reg_addr - mux select register offset in CPLD space
> + */
> +struct mlxcpld_mux_plat_data {
> +	int *adap_ids;
> +	int num_adaps;
> +	int sel_reg_addr;
> +};
> +
> +#endif /* _LINUX_I2C_MLXCPLD_H */
> 

Hmm, you never confirmed that you need to support different register
values in the CPLD with sel_reg_addr. I can see that there is a
possibility that you actually really need it, but I'd like to remove
that variable if at all possible.

If you can confirm that you need that, this is

Acked-by: Peter Rosin <peda@axentia.se>

Otherwise, please just hard code the register address.

Cheers,
Peter

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [patch v5] i2c: mux: mellanox: add driver
@ 2016-09-14  7:49   ` Peter Rosin
  0 siblings, 0 replies; 13+ messages in thread
From: Peter Rosin @ 2016-09-14  7:49 UTC (permalink / raw)
  To: vadimp, wsa; +Cc: linux-i2c, linux-kernel, jiri, Michael Shych

On 2016-09-13 22:37, vadimp@mellanox.com wrote:
> From: Vadim Pasternak <vadimp@mellanox.com>
> 
> 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).

*snip*

> +/* Platform data for the CPLD I2C multiplexers */
> +
> +/* mlxcpld_mux_plat_data - per mux data, used with i2c_register_board_info
> + * @adap_ids - adapter array
> + * @num_adaps - number of adapters
> + * @sel_reg_addr - mux select register offset in CPLD space
> + */
> +struct mlxcpld_mux_plat_data {
> +	int *adap_ids;
> +	int num_adaps;
> +	int sel_reg_addr;
> +};
> +
> +#endif /* _LINUX_I2C_MLXCPLD_H */
> 

Hmm, you never confirmed that you need to support different register
values in the CPLD with sel_reg_addr. I can see that there is a
possibility that you actually really need it, but I'd like to remove
that variable if at all possible.

If you can confirm that you need that, this is

Acked-by: Peter Rosin <peda@axentia.se>

Otherwise, please just hard code the register address.

Cheers,
Peter

^ permalink raw reply	[flat|nested] 13+ messages in thread

* RE: [patch v5] i2c: mux: mellanox: add driver
  2016-09-14  7:49   ` Peter Rosin
  (?)
@ 2016-09-14  8:10   ` Vadim Pasternak
  2016-09-14  8:42     ` Peter Rosin
  -1 siblings, 1 reply; 13+ messages in thread
From: Vadim Pasternak @ 2016-09-14  8:10 UTC (permalink / raw)
  To: Peter Rosin, wsa; +Cc: linux-i2c, linux-kernel, jiri, Michael Shych



> -----Original Message-----
> From: Peter Rosin [mailto:peda@axentia.se]
> Sent: Wednesday, September 14, 2016 10:50 AM
> To: Vadim Pasternak <vadimp@mellanox.com>; wsa@the-dreams.de
> Cc: linux-i2c@vger.kernel.org; linux-kernel@vger.kernel.org; jiri@resnulli.us;
> Michael Shych <michaelsh@mellanox.com>
> Subject: Re: [patch v5] i2c: mux: mellanox: add driver
> 
> On 2016-09-13 22:37, vadimp@mellanox.com wrote:
> > From: Vadim Pasternak <vadimp@mellanox.com>
> >
> > 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).
> 
> *snip*
> 
> > +/* Platform data for the CPLD I2C multiplexers */
> > +
> > +/* mlxcpld_mux_plat_data - per mux data, used with
> > +i2c_register_board_info
> > + * @adap_ids - adapter array
> > + * @num_adaps - number of adapters
> > + * @sel_reg_addr - mux select register offset in CPLD space  */
> > +struct mlxcpld_mux_plat_data {
> > +	int *adap_ids;
> > +	int num_adaps;
> > +	int sel_reg_addr;
> > +};
> > +
> > +#endif /* _LINUX_I2C_MLXCPLD_H */
> >
> 
> Hmm, you never confirmed that you need to support different register values in
> the CPLD with sel_reg_addr. I can see that there is a possibility that you actually
> really need it, but I'd like to remove that variable if at all possible.

Yes, it could be different register values.
Actually CPLD can also be programmed in different way.
So, I really need it.

> 
> If you can confirm that you need that, this is
> 
> Acked-by: Peter Rosin <peda@axentia.se>
> 
> Otherwise, please just hard code the register address.
> 
> Cheers,
> Peter

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [patch v5] i2c: mux: mellanox: add driver
  2016-09-14  8:10   ` Vadim Pasternak
@ 2016-09-14  8:42     ` Peter Rosin
  0 siblings, 0 replies; 13+ messages in thread
From: Peter Rosin @ 2016-09-14  8:42 UTC (permalink / raw)
  To: Vadim Pasternak, wsa; +Cc: linux-i2c, linux-kernel, jiri, Michael Shych

Wolfram, please pick up v5 for i2c-next whenever timing is right so that
it gets more testing etc. I didn't do any recent compile-testing myself,
but if you think that is required on the final version I can of course
spend some time on it. (But isn't that what all the infrastructure is
there to help with?)   Thanks!

On 2016-09-14 10:10, Vadim Pasternak wrote:
> Peter Rosin wrote:
>> On 2016-09-13 22:37, vadimp@mellanox.com wrote:
>>> From: Vadim Pasternak <vadimp@mellanox.com>
>>>
>>> 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).
>>
>> *snip*
>>
>>> +/* Platform data for the CPLD I2C multiplexers */
>>> +
>>> +/* mlxcpld_mux_plat_data - per mux data, used with
>>> +i2c_register_board_info
>>> + * @adap_ids - adapter array
>>> + * @num_adaps - number of adapters
>>> + * @sel_reg_addr - mux select register offset in CPLD space  */
>>> +struct mlxcpld_mux_plat_data {
>>> +	int *adap_ids;
>>> +	int num_adaps;
>>> +	int sel_reg_addr;
>>> +};
>>> +
>>> +#endif /* _LINUX_I2C_MLXCPLD_H */
>>>
>>
>> Hmm, you never confirmed that you need to support different register values in
>> the CPLD with sel_reg_addr. I can see that there is a possibility that you actually
>> really need it, but I'd like to remove that variable if at all possible.
> 
> Yes, it could be different register values.
> Actually CPLD can also be programmed in different way.
> So, I really need it.

Oh well :-)

Cheers,
Peter

>>
>> If you can confirm that you need that, this is
>>
>> Acked-by: Peter Rosin <peda@axentia.se>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [patch v5] i2c: mux: mellanox: add driver
  2016-09-13 20:37 [patch v5] i2c: mux: mellanox: add driver vadimp
@ 2016-09-23  9:36   ` Peter Rosin
  2016-09-23  9:36   ` Peter Rosin
  1 sibling, 0 replies; 13+ messages in thread
From: Peter Rosin @ 2016-09-23  9:36 UTC (permalink / raw)
  To: vadimp, wsa; +Cc: linux-i2c, linux-kernel, jiri, Michael Shych

On 2016-09-13 22:37, vadimp@mellanox.com wrote:
> From: Vadim Pasternak <vadimp@mellanox.com>
> 
> 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 |          |         |
  '---'          '---------'

Cheers,
Peter

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [patch v5] i2c: mux: mellanox: add driver
@ 2016-09-23  9:36   ` Peter Rosin
  0 siblings, 0 replies; 13+ messages in thread
From: Peter Rosin @ 2016-09-23  9:36 UTC (permalink / raw)
  To: vadimp, wsa; +Cc: linux-i2c, linux-kernel, jiri, Michael Shych

On 2016-09-13 22:37, vadimp@mellanox.com wrote:
> From: Vadim Pasternak <vadimp@mellanox.com>
> 
> 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 |          |         |
  '---'          '---------'

Cheers,
Peter

^ permalink raw reply	[flat|nested] 13+ messages in thread

* RE: [patch v5] i2c: mux: mellanox: add driver
  2016-09-23  9:36   ` Peter Rosin
  (?)
@ 2016-09-23 11:57   ` Vadim Pasternak
  -1 siblings, 0 replies; 13+ messages in thread
From: Vadim Pasternak @ 2016-09-23 11:57 UTC (permalink / raw)
  To: Peter Rosin, wsa; +Cc: linux-i2c, linux-kernel, jiri, Michael Shych



> -----Original Message-----
> From: Peter Rosin [mailto:peda@axentia.se]
> Sent: Friday, September 23, 2016 12:36 PM
> To: Vadim Pasternak <vadimp@mellanox.com>; wsa@the-dreams.de
> Cc: linux-i2c@vger.kernel.org; linux-kernel@vger.kernel.org; jiri@resnulli.us;
> Michael Shych <michaelsh@mellanox.com>
> Subject: Re: [patch v5] i2c: mux: mellanox: add driver
> 
> On 2016-09-13 22:37, vadimp@mellanox.com wrote:
> > From: Vadim Pasternak <vadimp@mellanox.com>
> >
> > 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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* RE: [patch v5] i2c: mux: mellanox: add driver
  2016-09-23  9:36   ` Peter Rosin
  (?)
  (?)
@ 2016-11-03  5:20   ` Vadim Pasternak
  2016-11-10 11:13     ` Peter Rosin
  -1 siblings, 1 reply; 13+ messages in thread
From: Vadim Pasternak @ 2016-11-03  5:20 UTC (permalink / raw)
  To: Peter Rosin, wsa; +Cc: linux-i2c, linux-kernel, jiri, Michael Shych

Hi,

I see that this patch has not been picked-up yet for i2c-next.
Is it possible it was missed from some reason?

Thanks,
Vadim.

> -----Original Message-----
> From: Vadim Pasternak
> Sent: Friday, September 23, 2016 2:57 PM
> To: 'Peter Rosin' <peda@axentia.se>; wsa@the-dreams.de
> Cc: linux-i2c@vger.kernel.org; linux-kernel@vger.kernel.org; jiri@resnulli.us;
> Michael Shych <michaelsh@mellanox.com>
> Subject: RE: [patch v5] i2c: mux: mellanox: add driver
> 
> 
> 
> > -----Original Message-----
> > From: Peter Rosin [mailto:peda@axentia.se]
> > Sent: Friday, September 23, 2016 12:36 PM
> > To: Vadim Pasternak <vadimp@mellanox.com>; wsa@the-dreams.de
> > Cc: linux-i2c@vger.kernel.org; linux-kernel@vger.kernel.org;
> > jiri@resnulli.us; Michael Shych <michaelsh@mellanox.com>
> > Subject: Re: [patch v5] i2c: mux: mellanox: add driver
> >
> > On 2016-09-13 22:37, vadimp@mellanox.com wrote:
> > > From: Vadim Pasternak <vadimp@mellanox.com>
> > >
> > > 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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [patch v5] i2c: mux: mellanox: add driver
  2016-11-03  5:20   ` Vadim Pasternak
@ 2016-11-10 11:13     ` Peter Rosin
  2016-11-10 12:56       ` Vadim Pasternak
  0 siblings, 1 reply; 13+ messages in thread
From: Peter Rosin @ 2016-11-10 11:13 UTC (permalink / raw)
  To: Vadim Pasternak, wsa; +Cc: linux-i2c, linux-kernel, jiri, Michael Shych

[resend to all, got the wrong button, sorry]

On 2016-11-10 11:42, Peter Rosin wrote:
> On 2016-11-03 06:20, Vadim Pasternak wrote:
>> Hi,
>>
>> I see that this patch has not been picked-up yet for i2c-next.
>> Is it possible it was missed from some reason?
>
> Yes, apparently, really sorry about that!
>
> I'll put it in a branch and make a pull request for Wolfram (but
> that is a bit new for me, we'll see how it goes).

But now that I looked again, I noticed that the source is
dual licensed and yet your MODULE_LICENSE tag says only
"GPL v2". I.e. the same issue your i2c master driver had
that Vladimir Zapolskiy noticed.

Please fix this, and it might be a good idea to take a look
in your other drivers as well in case you have further
problems in this department...

Also, while at it, the patch doesn't apply cleanly anymore,
please rebase to something more current.

Cheers,
Peter

^ permalink raw reply	[flat|nested] 13+ messages in thread

* RE: [patch v5] i2c: mux: mellanox: add driver
  2016-11-10 11:13     ` Peter Rosin
@ 2016-11-10 12:56       ` Vadim Pasternak
  2016-11-10 13:02         ` Peter Rosin
  0 siblings, 1 reply; 13+ messages in thread
From: Vadim Pasternak @ 2016-11-10 12:56 UTC (permalink / raw)
  To: Peter Rosin, wsa; +Cc: linux-i2c, linux-kernel, jiri, Michael Shych



> -----Original Message-----
> From: Peter Rosin [mailto:peda@axentia.se]
> Sent: Thursday, November 10, 2016 1:13 PM
> To: Vadim Pasternak <vadimp@mellanox.com>; wsa@the-dreams.de
> Cc: linux-i2c@vger.kernel.org; linux-kernel@vger.kernel.org; jiri@resnulli.us;
> Michael Shych <michaelsh@mellanox.com>
> Subject: Re: [patch v5] i2c: mux: mellanox: add driver
> 
> [resend to all, got the wrong button, sorry]
> 
> On 2016-11-10 11:42, Peter Rosin wrote:
> > On 2016-11-03 06:20, Vadim Pasternak wrote:
> >> Hi,
> >>
> >> I see that this patch has not been picked-up yet for i2c-next.
> >> Is it possible it was missed from some reason?
> >
> > Yes, apparently, really sorry about that!
> >
> > I'll put it in a branch and make a pull request for Wolfram (but that
> > is a bit new for me, we'll see how it goes).
> 
> But now that I looked again, I noticed that the source is dual licensed and yet
> your MODULE_LICENSE tag says only "GPL v2". I.e. the same issue your i2c
> master driver had that Vladimir Zapolskiy noticed.
> 
> Please fix this, and it might be a good idea to take a look in your other drivers as
> well in case you have further problems in this department...

Hi Peter,

Thank you for reply. 
I'll re-submit it from the current i2c next branch.
Since I'll fix the license, I guess I should issue it as "patch v6". Right?

Cheers,
Vadim.

> 
> Also, while at it, the patch doesn't apply cleanly anymore, please rebase to
> something more current.
> 
> Cheers,
> Peter

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [patch v5] i2c: mux: mellanox: add driver
  2016-11-10 12:56       ` Vadim Pasternak
@ 2016-11-10 13:02         ` Peter Rosin
  2016-11-10 13:04           ` Vadim Pasternak
  0 siblings, 1 reply; 13+ messages in thread
From: Peter Rosin @ 2016-11-10 13:02 UTC (permalink / raw)
  To: Vadim Pasternak, wsa; +Cc: linux-i2c, linux-kernel, jiri, Michael Shych

On 2016-11-10 13:56, Vadim Pasternak wrote:
> 
> 
>> -----Original Message-----
>> From: Peter Rosin [mailto:peda@axentia.se]
>> Sent: Thursday, November 10, 2016 1:13 PM
>> To: Vadim Pasternak <vadimp@mellanox.com>; wsa@the-dreams.de
>> Cc: linux-i2c@vger.kernel.org; linux-kernel@vger.kernel.org; jiri@resnulli.us;
>> Michael Shych <michaelsh@mellanox.com>
>> Subject: Re: [patch v5] i2c: mux: mellanox: add driver
>>
>> [resend to all, got the wrong button, sorry]
>>
>> On 2016-11-10 11:42, Peter Rosin wrote:
>>> On 2016-11-03 06:20, Vadim Pasternak wrote:
>>>> Hi,
>>>>
>>>> I see that this patch has not been picked-up yet for i2c-next.
>>>> Is it possible it was missed from some reason?
>>>
>>> Yes, apparently, really sorry about that!
>>>
>>> I'll put it in a branch and make a pull request for Wolfram (but that
>>> is a bit new for me, we'll see how it goes).
>>
>> But now that I looked again, I noticed that the source is dual licensed and yet
>> your MODULE_LICENSE tag says only "GPL v2". I.e. the same issue your i2c
>> master driver had that Vladimir Zapolskiy noticed.
>>
>> Please fix this, and it might be a good idea to take a look in your other drivers as
>> well in case you have further problems in this department...
> 
> Hi Peter,
> 
> Thank you for reply. 
> I'll re-submit it from the current i2c next branch.
> Since I'll fix the license, I guess I should issue it as "patch v6". Right?

Sounds right, yes.

Cheers,
Peter

^ permalink raw reply	[flat|nested] 13+ messages in thread

* RE: [patch v5] i2c: mux: mellanox: add driver
  2016-11-10 13:02         ` Peter Rosin
@ 2016-11-10 13:04           ` Vadim Pasternak
  0 siblings, 0 replies; 13+ messages in thread
From: Vadim Pasternak @ 2016-11-10 13:04 UTC (permalink / raw)
  To: Peter Rosin, wsa; +Cc: linux-i2c, linux-kernel, jiri, Michael Shych



> -----Original Message-----
> From: Peter Rosin [mailto:peda@axentia.se]
> Sent: Thursday, November 10, 2016 3:02 PM
> To: Vadim Pasternak <vadimp@mellanox.com>; wsa@the-dreams.de
> Cc: linux-i2c@vger.kernel.org; linux-kernel@vger.kernel.org; jiri@resnulli.us;
> Michael Shych <michaelsh@mellanox.com>
> Subject: Re: [patch v5] i2c: mux: mellanox: add driver
> 
> On 2016-11-10 13:56, Vadim Pasternak wrote:
> >
> >
> >> -----Original Message-----
> >> From: Peter Rosin [mailto:peda@axentia.se]
> >> Sent: Thursday, November 10, 2016 1:13 PM
> >> To: Vadim Pasternak <vadimp@mellanox.com>; wsa@the-dreams.de
> >> Cc: linux-i2c@vger.kernel.org; linux-kernel@vger.kernel.org;
> >> jiri@resnulli.us; Michael Shych <michaelsh@mellanox.com>
> >> Subject: Re: [patch v5] i2c: mux: mellanox: add driver
> >>
> >> [resend to all, got the wrong button, sorry]
> >>
> >> On 2016-11-10 11:42, Peter Rosin wrote:
> >>> On 2016-11-03 06:20, Vadim Pasternak wrote:
> >>>> Hi,
> >>>>
> >>>> I see that this patch has not been picked-up yet for i2c-next.
> >>>> Is it possible it was missed from some reason?
> >>>
> >>> Yes, apparently, really sorry about that!
> >>>
> >>> I'll put it in a branch and make a pull request for Wolfram (but
> >>> that is a bit new for me, we'll see how it goes).
> >>
> >> But now that I looked again, I noticed that the source is dual
> >> licensed and yet your MODULE_LICENSE tag says only "GPL v2". I.e. the
> >> same issue your i2c master driver had that Vladimir Zapolskiy noticed.
> >>
> >> Please fix this, and it might be a good idea to take a look in your
> >> other drivers as well in case you have further problems in this department...
> >
> > Hi Peter,
> >
> > Thank you for reply.
> > I'll re-submit it from the current i2c next branch.
> > Since I'll fix the license, I guess I should issue it as "patch v6". Right?
> 
> Sounds right, yes.

OK, will do it.
And I'll fix the comment according to your last recommendation.

Cheers,
Vadim.
> 
> Cheers,
> Peter

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2016-11-11  7:36 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-09-13 20:37 [patch v5] i2c: mux: mellanox: add driver vadimp
2016-09-14  7:49 ` Peter Rosin
2016-09-14  7:49   ` Peter Rosin
2016-09-14  8:10   ` Vadim Pasternak
2016-09-14  8:42     ` Peter Rosin
2016-09-23  9:36 ` Peter Rosin
2016-09-23  9:36   ` Peter Rosin
2016-09-23 11:57   ` Vadim Pasternak
2016-11-03  5:20   ` Vadim Pasternak
2016-11-10 11:13     ` Peter Rosin
2016-11-10 12:56       ` Vadim Pasternak
2016-11-10 13:02         ` Peter Rosin
2016-11-10 13:04           ` Vadim Pasternak

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.