2016-11-18 05:53:17

by Vadim Pasternak

[permalink] [raw]
Subject: [patch v8 1/1] i2c: add master driver for mellanox systems

From: Vadim Pasternak <[email protected]>

Device driver for Mellanox I2C controller logic, implemented in Lattice
CPLD device.
Device supports:
- Master mode
- One physical bus
- Polling mode

The Kconfig currently controlling compilation of this code is:
drivers/i2c/busses/Kconfig:config I2C_MLXCPLD

Signed-off-by: Michael Shych <[email protected]>
Signed-off-by: Vadim Pasternak <[email protected]>
Reviewed-by: Jiri Pirko <[email protected]>
Reviewed-by: Vladimir Zapolskiy <[email protected]>
---
v7->v8
Comments pointed out by Wolfram:
- Remove descriptions for two structures, since the members have
self-explaining names;
- Populate the structure i2c_adapter_quirks, so core will make length
validation, as a result remove mlxcpld_i2c_invalid_len,
and move common length calculation to mlxcpld_i2c_xfer. Consider
removing of mlxcpld_i2c_check_msg_params after core quirks is able
to handle messages full validation;
- Use ENXIO error code for NACK;
v6->v7
Comments pointed out by Peter:
- Fix grammar in doc file;
- Fix description for CMD in doc file;
- Fix grammar for NUM_DATA, DATAx in doc file;
- Rename mlxcpld_i2c_curr_transf to mlxcpld_i2c_curr_xfer;
- Make code in mlxcpld_i2c_lpc_write_buf more readable and
mlxcpld_i2c_lpc_read_buf, compact and elegant;
- Fix multiline comments;
v5->v6:
Comments pointed out by Vladimir:
- Drop the line with module path from the header;
- In description of mlxcpld_i2c_priv remove lpc_gen_dec_reg asnd dev_id;
- In mlxcpld_i2c_priv change type of the filed base_addr to u16 for
the alignment with in/out and remove unused dev_id;
- Fix misspelling in comment for mlxcpld_i2c_invalid_len;
- Remove comment regarding EBUSY return in mlxcpld_i2c_check_busy;
- Use sizeof of the target storage in allocation in probe routine;
v4->v5:
Comments pointed out by Vladimir:
- Remove "default n" from Kconfig;
- Fix the comments for timeout and pool time;
- Optimize error flow in mlxcpld_i2c_probe;
v3->v4:
Comments pointed out by Vladimir:
- Set default to no in Kconfig;
- Make mlxcpld_i2c_plat_dev static and add empty line before the
declaration;
- In function mlxcpld_i2c_invalid_len remove (msg->len < 0), since len is
unsigned;
- Remove unused symbol mlxcpld_i2c_plat_dev;
- Remove extra spaces in comments to mlxcpld_i2c_check_msg_params;
- Remove unnecessary round braces in mlxcpld_i2c_set_transf_data;
- Remove the assignment of 'i' variable in mlxcpld_i2c_wait_for_tc;
- Add extra line in mlxcpld_i2c_xfer;
- Move assignment of the adapter's fields retries and nr inside
mlxcpld_i2c_adapter declaration;
v2->v3:
Comments pointed out by Vladimir:
- Use tab symbol as indentation in Kconfig
- Add the Kconfig section preserving the alphabetical order - added
within "Other I2C/SMBus bus drivers" after I2C_ELEKTOR (but after this
sections others are not follow alphabetical);
- Change license to dual;
- Replace ADRR with ADDR in macros;
- Remove unused macros: MLXCPLD_LPCI2C_LPF_DFLT,
MLXCPLD_LPCI2C_HALF_CYC_100, MLXCPLD_LPCI2C_I2C_HOLD_100,
MLXCPLD_LPCI2C_HALF_CYC_REG, MLXCPLD_LPCI2C_I2C_HOLD_REG;
- Fix checkpatch warnings (**/ and the end of comment);
- Add empty line before structures mlxcpld_i2c_regs,
mlxcpld_i2c_curr_transf, mlxcpld_i2c_priv;
- Remove unused structure mlxcpld_i2c_regs;
- Remove from mlxcpld_i2c_priv the next fields:
retr_num, poll_time, block_sz, xfer_to; use instead macros
respectively: MLXCPLD_I2C_RETR_NUM, MLXCPLD_I2C_POLL_TIME,
MLXCPLD_I2C_DATA_REG_SZ, MLXCPLD_I2C_XFER_TO;
- In mlxcpld_i2c_invalid_len remove unnecessary else;
- Optimize mlxcpld_i2c_set_transf_data;
- mlxcpld_i2c_reset - add empty lines after/before mutex
lock/unlock;
- mlxcpld_i2c_wait_for_free - cover case timeout is equal
MLXCPLD_I2C_XFER_TO;
- mlxcpld_i2c_wait_for_tc:
- Do not assign err in declaration (also err is removed);
- Insert empty line before case MLXCPLD_LPCI2C_ACK_IND;
- inside case MLXCPLD_LPCI2C_ACK_IND - avoid unnecessary
indentation;
- Remove case MLXCPLD_LPCI2C_ERR_IND and remove this macro;
- Add empty lines in mlxcpld_i2c_xfer before/after mutex_lock/
mutex_unlock;
- In mlxcpld_i2c_probe add emtpy line after platform_set_drvdata;
- Replace platfrom handle pdev in mlxcpld_i2c_priv with the pointer
to the structure device;
- Place assignment of base_addr near the others;
- Enclose e-mail with <>;
Fixes added by Vadim:
- Change structure description format according to
Documentation/kernel-documentation.rst guideline;
- mlxcpld_i2c_wait_for_tc: return error if status reaches default case;
v1->v2
Fixes added by Vadim:
- Put new record in Makefile in alphabetic order;
- Remove http://www.mellanox.com from MAINTAINERS record;
---
Documentation/i2c/busses/i2c-mlxcpld | 47 ++++
MAINTAINERS | 8 +
drivers/i2c/busses/Kconfig | 11 +
drivers/i2c/busses/Makefile | 1 +
drivers/i2c/busses/i2c-mlxcpld.c | 502 +++++++++++++++++++++++++++++++++++
5 files changed, 569 insertions(+)
create mode 100644 Documentation/i2c/busses/i2c-mlxcpld
create mode 100644 drivers/i2c/busses/i2c-mlxcpld.c

diff --git a/Documentation/i2c/busses/i2c-mlxcpld b/Documentation/i2c/busses/i2c-mlxcpld
new file mode 100644
index 0000000..4e46c44
--- /dev/null
+++ b/Documentation/i2c/busses/i2c-mlxcpld
@@ -0,0 +1,47 @@
+Driver i2c-mlxcpld
+
+Author: Michael Shych <[email protected]>
+
+This is the Mellanox I2C controller logic, implemented in Lattice CPLD
+device.
+Device supports:
+ - Master mode.
+ - One physical bus.
+ - Polling mode.
+
+This controller is equipped within the next Mellanox systems:
+"msx6710", "msx6720", "msb7700", "msn2700", "msx1410", "msn2410", "msb7800",
+"msn2740", "msn2100".
+
+The next transaction types are supported:
+ - Receive Byte/Block.
+ - Send Byte/Block.
+ - Read Byte/Block.
+ - Write Byte/Block.
+
+Registers:
+CTRL 0x1 - control reg.
+ Resets all the registers.
+HALF_CYC 0x4 - cycle reg.
+ Configure the width of I2C SCL half clock cycle (in 4 LPC_CLK
+ units).
+I2C_HOLD 0x5 - hold reg.
+ OE (output enable) is delayed by value set to this register
+ (in LPC_CLK units)
+CMD 0x6 - command reg.
+ Bit 0, 0 = write, 1 = read.
+ Bits [7:1] - the 7bit Address of the I2C device.
+ It should be written last as it triggers an I2C transaction.
+NUM_DATA 0x7 - data size reg.
+ Number of data bytes to write in read transaction
+NUM_ADDR 0x8 - address reg.
+ Number of address bytes to write in read transaction.
+STATUS 0x9 - status reg.
+ Bit 0 - transaction is completed.
+ Bit 4 - ACK/NACK.
+DATAx 0xa - 0x54 - 68 bytes data buffer regs.
+ For write transaction address is specified in four first bytes
+ (DATA1 - DATA4), data starting from DATA4.
+ For read transactions address is sent in a separate transaction and
+ specified in the four first bytes (DATA0 - DATA3). Data is read
+ starting from DATA0.
diff --git a/MAINTAINERS b/MAINTAINERS
index 411e3b8..26d05f8 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7881,6 +7881,14 @@ W: http://www.mellanox.com
Q: http://patchwork.ozlabs.org/project/netdev/list/
F: drivers/net/ethernet/mellanox/mlxsw/

+MELLANOX MLXCPLD I2C DRIVER
+M: Vadim Pasternak <[email protected]>
+M: Michael Shych <[email protected]>
+L: [email protected]
+S: Supported
+F: drivers/i2c/busses/i2c-mlxcpld.c
+F: Documentation/i2c/busses/i2c-mlxcpld
+
MELLANOX MLXCPLD LED DRIVER
M: Vadim Pasternak <[email protected]>
L: [email protected]
diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index d252276..6399cea 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -1150,6 +1150,17 @@ config I2C_ELEKTOR
This support is also available as a module. If so, the module
will be called i2c-elektor.

+config I2C_MLXCPLD
+ tristate "Mellanox I2C driver"
+ depends on X86_64
+ help
+ This exposes the Mellanox platform I2C busses to the linux I2C layer
+ for X86 based systems.
+ Controller is implemented as CPLD logic.
+
+ This driver can also be built as a module. If so, the module will be
+ called as i2c-mlxcpld.
+
config I2C_PCA_ISA
tristate "PCA9564/PCA9665 on an ISA bus"
depends on ISA
diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
index 29764cc..645bf08 100644
--- a/drivers/i2c/busses/Makefile
+++ b/drivers/i2c/busses/Makefile
@@ -116,6 +116,7 @@ obj-$(CONFIG_I2C_BCM_KONA) += i2c-bcm-kona.o
obj-$(CONFIG_I2C_BRCMSTB) += i2c-brcmstb.o
obj-$(CONFIG_I2C_CROS_EC_TUNNEL) += i2c-cros-ec-tunnel.o
obj-$(CONFIG_I2C_ELEKTOR) += i2c-elektor.o
+obj-$(CONFIG_I2C_MLXCPLD) += i2c-mlxcpld.o
obj-$(CONFIG_I2C_OPAL) += i2c-opal.o
obj-$(CONFIG_I2C_PCA_ISA) += i2c-pca-isa.o
obj-$(CONFIG_I2C_SIBYTE) += i2c-sibyte.o
diff --git a/drivers/i2c/busses/i2c-mlxcpld.c b/drivers/i2c/busses/i2c-mlxcpld.c
new file mode 100644
index 0000000..d0eaf93
--- /dev/null
+++ b/drivers/i2c/busses/i2c-mlxcpld.c
@@ -0,0 +1,502 @@
+/*
+ * Copyright (c) 2016 Mellanox Technologies. All rights reserved.
+ * Copyright (c) 2016 Michael Shych <[email protected]>
+ *
+ * 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/delay.h>
+#include <linux/i2c.h>
+#include <linux/init.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+
+/* General defines */
+#define MLXPLAT_CPLD_LPC_I2C_BASE_ADDR 0x2000
+#define MLXCPLD_I2C_DEVICE_NAME "i2c_mlxcpld"
+#define MLXCPLD_I2C_VALID_FLAG (I2C_M_RECV_LEN | I2C_M_RD)
+#define MLXCPLD_I2C_BUS_NUM 1
+#define MLXCPLD_I2C_DATA_REG_SZ 36
+#define MLXCPLD_I2C_MAX_ADDR_LEN 4
+#define MLXCPLD_I2C_RETR_NUM 2
+#define MLXCPLD_I2C_XFER_TO 500000 /* usec */
+#define MLXCPLD_I2C_POLL_TIME 2000 /* usec */
+
+/* LPC I2C registers */
+#define MLXCPLD_LPCI2C_LPF_REG 0x0
+#define MLXCPLD_LPCI2C_CTRL_REG 0x1
+#define MLXCPLD_LPCI2C_HALF_CYC_REG 0x4
+#define MLXCPLD_LPCI2C_I2C_HOLD_REG 0x5
+#define MLXCPLD_LPCI2C_CMD_REG 0x6
+#define MLXCPLD_LPCI2C_NUM_DAT_REG 0x7
+#define MLXCPLD_LPCI2C_NUM_ADDR_REG 0x8
+#define MLXCPLD_LPCI2C_STATUS_REG 0x9
+#define MLXCPLD_LPCI2C_DATA_REG 0xa
+
+/* LPC I2C masks and parametres */
+#define MLXCPLD_LPCI2C_RST_SEL_MASK 0x1
+#define MLXCPLD_LPCI2C_TRANS_END 0x1
+#define MLXCPLD_LPCI2C_STATUS_NACK 0x10
+#define MLXCPLD_LPCI2C_NO_IND 0
+#define MLXCPLD_LPCI2C_ACK_IND 1
+#define MLXCPLD_LPCI2C_NACK_IND 2
+
+struct mlxcpld_i2c_curr_xfer {
+ u8 cmd;
+ u8 addr_width;
+ u8 data_len;
+ u8 msg_num;
+ struct i2c_msg *msg;
+};
+
+struct mlxcpld_i2c_priv {
+ struct i2c_adapter adap;
+ u32 base_addr;
+ struct mutex lock;
+ struct mlxcpld_i2c_curr_xfer xfer;
+ struct device *dev;
+};
+
+static void mlxcpld_i2c_lpc_write_buf(u8 *data, u8 len, u32 addr)
+{
+ int i;
+
+ for (i = 0; i < len - len % 4; i += 4)
+ outl(*(u32 *)(data + i), addr + i);
+ for (; i < len; ++i)
+ outb(*(data + i), addr + i);
+}
+
+static void mlxcpld_i2c_lpc_read_buf(u8 *data, u8 len, u32 addr)
+{
+ int i;
+
+ for (i = 0; i < len - len % 4; i += 4)
+ *(u32 *)(data + i) = inl(addr + i);
+ for (; i < len; ++i)
+ *(data + i) = inb(addr + i);
+}
+
+static void mlxcpld_i2c_read_comm(struct mlxcpld_i2c_priv *priv, u8 offs,
+ u8 *data, u8 datalen)
+{
+ u32 addr = priv->base_addr + offs;
+
+ switch (datalen) {
+ case 1:
+ *(data) = inb(addr);
+ break;
+ case 2:
+ *((u16 *)data) = inw(addr);
+ break;
+ case 3:
+ *((u16 *)data) = inw(addr);
+ *(data + 2) = inb(addr + 2);
+ break;
+ case 4:
+ *((u32 *)data) = inl(addr);
+ break;
+ default:
+ mlxcpld_i2c_lpc_read_buf(data, datalen, addr);
+ break;
+ }
+}
+
+static void mlxcpld_i2c_write_comm(struct mlxcpld_i2c_priv *priv, u8 offs,
+ u8 *data, u8 datalen)
+{
+ u32 addr = priv->base_addr + offs;
+
+ switch (datalen) {
+ case 1:
+ outb(*(data), addr);
+ break;
+ case 2:
+ outw(*((u16 *)data), addr);
+ break;
+ case 3:
+ outw(*((u16 *)data), addr);
+ outb(*(data + 2), addr + 2);
+ break;
+ case 4:
+ outl(*((u32 *)data), addr);
+ break;
+ default:
+ mlxcpld_i2c_lpc_write_buf(data, datalen, addr);
+ break;
+ }
+}
+
+/*
+ * Check validity of received i2c messages parameters.
+ * Returns 0 if OK, other - in case of invalid parameters.
+ */
+static int mlxcpld_i2c_check_msg_params(struct mlxcpld_i2c_priv *priv,
+ struct i2c_msg *msgs, int num)
+{
+ int i;
+
+ if (!num) {
+ dev_err(priv->dev, "Incorrect 0 num of messages\n");
+ return -EINVAL;
+ }
+
+ if (unlikely(msgs[0].addr > 0x7f)) {
+ dev_err(priv->dev, "Invalid address 0x%03x\n",
+ msgs[0].addr);
+ return -EINVAL;
+ }
+
+ for (i = 0; i < num; ++i) {
+ if (unlikely(!msgs[i].buf)) {
+ dev_err(priv->dev, "Invalid buf in msg[%d]\n",
+ i);
+ return -EINVAL;
+ }
+ if (unlikely(msgs[0].addr != msgs[i].addr)) {
+ dev_err(priv->dev, "Invalid addr in msg[%d]\n",
+ i);
+ return -EINVAL;
+ }
+ }
+
+ return 0;
+}
+
+/*
+ * Check if transfer is completed and status of operation.
+ * Returns 0 - transfer completed (both ACK or NACK),
+ * negative - transfer isn't finished.
+ */
+static int mlxcpld_i2c_check_status(struct mlxcpld_i2c_priv *priv, int *status)
+{
+ u8 val;
+
+ mlxcpld_i2c_read_comm(priv, MLXCPLD_LPCI2C_STATUS_REG, &val, 1);
+
+ if (val & MLXCPLD_LPCI2C_TRANS_END) {
+ if (val & MLXCPLD_LPCI2C_STATUS_NACK)
+ /*
+ * The slave is unable to accept the data. No such
+ * slave, command not understood, or unable to accept
+ * any more data.
+ */
+ *status = MLXCPLD_LPCI2C_NACK_IND;
+ else
+ *status = MLXCPLD_LPCI2C_ACK_IND;
+ return 0;
+ }
+ *status = MLXCPLD_LPCI2C_NO_IND;
+
+ return -EIO;
+}
+
+static void mlxcpld_i2c_set_transf_data(struct mlxcpld_i2c_priv *priv,
+ struct i2c_msg *msgs, int num,
+ u8 comm_len)
+{
+ priv->xfer.msg = msgs;
+ priv->xfer.msg_num = num;
+
+ /*
+ * All upper layers currently are never use transfer with more than
+ * 2 messages. Actually, it's also not so relevant in Mellanox systems
+ * because of HW limitation. Max size of transfer is o more than 20B
+ * in current x86 LPCI2C bridge.
+ */
+ priv->xfer.cmd = msgs[num - 1].flags & I2C_M_RD;
+
+ if (priv->xfer.cmd == I2C_M_RD && comm_len != msgs[0].len) {
+ priv->xfer.addr_width = msgs[0].len;
+ priv->xfer.data_len = comm_len - priv->xfer.addr_width;
+ } else {
+ priv->xfer.addr_width = 0;
+ priv->xfer.data_len = comm_len;
+ }
+}
+
+/* Reset CPLD LPCI2C block */
+static void mlxcpld_i2c_reset(struct mlxcpld_i2c_priv *priv)
+{
+ u8 val;
+
+ mutex_lock(&priv->lock);
+
+ mlxcpld_i2c_read_comm(priv, MLXCPLD_LPCI2C_CTRL_REG, &val, 1);
+ val &= ~MLXCPLD_LPCI2C_RST_SEL_MASK;
+ mlxcpld_i2c_write_comm(priv, MLXCPLD_LPCI2C_CTRL_REG, &val, 1);
+
+ mutex_unlock(&priv->lock);
+}
+
+/* Make sure the CPLD is ready to start transmitting. */
+static int mlxcpld_i2c_check_busy(struct mlxcpld_i2c_priv *priv)
+{
+ u8 val;
+
+ mlxcpld_i2c_read_comm(priv, MLXCPLD_LPCI2C_STATUS_REG, &val, 1);
+
+ if (val & MLXCPLD_LPCI2C_TRANS_END)
+ return 0;
+
+ return -EIO;
+}
+
+static int mlxcpld_i2c_wait_for_free(struct mlxcpld_i2c_priv *priv)
+{
+ int timeout = 0;
+
+ do {
+ if (!mlxcpld_i2c_check_busy(priv))
+ break;
+ usleep_range(MLXCPLD_I2C_POLL_TIME / 2, MLXCPLD_I2C_POLL_TIME);
+ timeout += MLXCPLD_I2C_POLL_TIME;
+ } while (timeout <= MLXCPLD_I2C_XFER_TO);
+
+ if (timeout > MLXCPLD_I2C_XFER_TO)
+ return -ETIMEDOUT;
+
+ return 0;
+}
+
+/*
+ * Wait for master transfer to complete.
+ * It puts current process to sleep until we get interrupt or timeout expires.
+ * Returns the number of transferred or read bytes or error (<0).
+ */
+static int mlxcpld_i2c_wait_for_tc(struct mlxcpld_i2c_priv *priv)
+{
+ int status, i, timeout = 0;
+ u8 datalen;
+
+ do {
+ usleep_range(MLXCPLD_I2C_POLL_TIME / 2, MLXCPLD_I2C_POLL_TIME);
+ if (!mlxcpld_i2c_check_status(priv, &status))
+ break;
+ timeout += MLXCPLD_I2C_POLL_TIME;
+ } while (status == 0 && timeout < MLXCPLD_I2C_XFER_TO);
+
+ switch (status) {
+ case MLXCPLD_LPCI2C_NO_IND:
+ return -ETIMEDOUT;
+
+ case MLXCPLD_LPCI2C_ACK_IND:
+ if (priv->xfer.cmd != I2C_M_RD)
+ return (priv->xfer.addr_width + priv->xfer.data_len);
+
+ if (priv->xfer.msg_num == 1)
+ i = 0;
+ else
+ i = 1;
+
+ if (!priv->xfer.msg[i].buf)
+ return -EINVAL;
+
+ /*
+ * Actual read data len will be always the same as
+ * requested len. 0xff (line pull-up) will be returned
+ * if slave has no data to return. Thus don't read
+ * MLXCPLD_LPCI2C_NUM_DAT_REG reg from CPLD.
+ */
+ datalen = priv->xfer.data_len;
+
+ mlxcpld_i2c_read_comm(priv, MLXCPLD_LPCI2C_DATA_REG,
+ priv->xfer.msg[i].buf, datalen);
+
+ return datalen;
+
+ case MLXCPLD_LPCI2C_NACK_IND:
+ return -ENXIO;
+
+ default:
+ return -EINVAL;
+ }
+}
+
+static void mlxcpld_i2c_xfer_msg(struct mlxcpld_i2c_priv *priv)
+{
+ int i, len = 0;
+ u8 cmd;
+
+ mlxcpld_i2c_write_comm(priv, MLXCPLD_LPCI2C_NUM_DAT_REG,
+ &priv->xfer.data_len, 1);
+ mlxcpld_i2c_write_comm(priv, MLXCPLD_LPCI2C_NUM_ADDR_REG,
+ &priv->xfer.addr_width, 1);
+
+ for (i = 0; i < priv->xfer.msg_num; i++) {
+ if ((priv->xfer.msg[i].flags & I2C_M_RD) != I2C_M_RD) {
+ /* Don't write to CPLD buffer in read transaction */
+ mlxcpld_i2c_write_comm(priv, MLXCPLD_LPCI2C_DATA_REG +
+ len, priv->xfer.msg[i].buf,
+ priv->xfer.msg[i].len);
+ len += priv->xfer.msg[i].len;
+ }
+ }
+
+ /*
+ * Set target slave address with command for master transfer.
+ * It should be latest executed function before CPLD transaction.
+ */
+ cmd = (priv->xfer.msg[0].addr << 1) | priv->xfer.cmd;
+ mlxcpld_i2c_write_comm(priv, MLXCPLD_LPCI2C_CMD_REG, &cmd, 1);
+}
+
+/*
+ * Generic lpc-i2c transfer.
+ * Returns the number of processed messages or error (<0).
+ */
+static int mlxcpld_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs,
+ int num)
+{
+ struct mlxcpld_i2c_priv *priv = i2c_get_adapdata(adap);
+ u8 comm_len = 0;
+ int i, err;
+
+ err = mlxcpld_i2c_check_msg_params(priv, msgs, num);
+ if (err) {
+ dev_err(priv->dev, "Incorrect message\n");
+ return err;
+ }
+
+ for (i = 0; i < num; ++i)
+ comm_len += msgs[i].len;
+
+ /* Check bus state */
+ if (mlxcpld_i2c_wait_for_free(priv)) {
+ dev_err(priv->dev, "LPCI2C bridge is busy\n");
+
+ /*
+ * Usually it means something serious has happened.
+ * We can not have unfinished previous transfer
+ * so it doesn't make any sense to try to stop it.
+ * Probably we were not able to recover from the
+ * previous error.
+ * The only reasonable thing - is soft reset.
+ */
+ mlxcpld_i2c_reset(priv);
+ if (mlxcpld_i2c_check_busy(priv)) {
+ dev_err(priv->dev, "LPCI2C bridge is busy after reset\n");
+ return -EIO;
+ }
+ }
+
+ mlxcpld_i2c_set_transf_data(priv, msgs, num, comm_len);
+
+ mutex_lock(&priv->lock);
+
+ /* Do real transfer. Can't fail */
+ mlxcpld_i2c_xfer_msg(priv);
+
+ /* Wait for transaction complete */
+ err = mlxcpld_i2c_wait_for_tc(priv);
+
+ mutex_unlock(&priv->lock);
+
+ return err < 0 ? err : num;
+}
+
+static u32 mlxcpld_i2c_func(struct i2c_adapter *adap)
+{
+ return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL | I2C_FUNC_SMBUS_BLOCK_DATA;
+}
+
+static const struct i2c_algorithm mlxcpld_i2c_algo = {
+ .master_xfer = mlxcpld_i2c_xfer,
+ .functionality = mlxcpld_i2c_func
+};
+
+static struct i2c_adapter_quirks mlxcpld_i2c_quirks = {
+ .max_read_len = MLXCPLD_I2C_DATA_REG_SZ - MLXCPLD_I2C_MAX_ADDR_LEN,
+ .max_write_len = MLXCPLD_I2C_DATA_REG_SZ,
+};
+
+static struct i2c_adapter mlxcpld_i2c_adapter = {
+ .owner = THIS_MODULE,
+ .name = "i2c-mlxcpld",
+ .class = I2C_CLASS_HWMON | I2C_CLASS_SPD,
+ .algo = &mlxcpld_i2c_algo,
+ .quirks = &mlxcpld_i2c_quirks,
+ .retries = MLXCPLD_I2C_RETR_NUM,
+ .nr = MLXCPLD_I2C_BUS_NUM,
+};
+
+static int mlxcpld_i2c_probe(struct platform_device *pdev)
+{
+ struct mlxcpld_i2c_priv *priv;
+ int err;
+
+ priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
+ if (!priv)
+ return -ENOMEM;
+
+ mutex_init(&priv->lock);
+ platform_set_drvdata(pdev, priv);
+
+ priv->dev = &pdev->dev;
+
+ /* Register with i2c layer */
+ mlxcpld_i2c_adapter.timeout = usecs_to_jiffies(MLXCPLD_I2C_XFER_TO);
+ priv->adap = mlxcpld_i2c_adapter;
+ priv->adap.dev.parent = &pdev->dev;
+ priv->base_addr = MLXPLAT_CPLD_LPC_I2C_BASE_ADDR;
+ i2c_set_adapdata(&priv->adap, priv);
+
+ err = i2c_add_numbered_adapter(&priv->adap);
+ if (err)
+ mutex_destroy(&priv->lock);
+
+ return err;
+}
+
+static int mlxcpld_i2c_remove(struct platform_device *pdev)
+{
+ struct mlxcpld_i2c_priv *priv = platform_get_drvdata(pdev);
+
+ i2c_del_adapter(&priv->adap);
+ mutex_destroy(&priv->lock);
+
+ return 0;
+}
+
+static struct platform_driver mlxcpld_i2c_driver = {
+ .probe = mlxcpld_i2c_probe,
+ .remove = mlxcpld_i2c_remove,
+ .driver = {
+ .name = MLXCPLD_I2C_DEVICE_NAME,
+ },
+};
+
+module_platform_driver(mlxcpld_i2c_driver);
+
+MODULE_AUTHOR("Michael Shych <[email protected]>");
+MODULE_DESCRIPTION("Mellanox I2C-CPLD controller driver");
+MODULE_LICENSE("Dual BSD/GPL");
+MODULE_ALIAS("platform:i2c-mlxcpld");
--
2.1.4


2016-11-19 21:04:49

by Wolfram Sang

[permalink] [raw]
Subject: Re: [patch v8 1/1] i2c: add master driver for mellanox systems

Hi,

looks mostly good. I just found this comment which needs clarification:

> + /*
> + * All upper layers currently are never use transfer with more than
> + * 2 messages.

Have you really checked ALL of the upper layers? And even if so, there
is the dev-interface to userspace which allows for arbitrary I2C
transfers using I2C_RDWR.

> Actually, it's also not so relevant in Mellanox systems
> + * because of HW limitation.

What kind of HW limitation do you mean here? Can Mellanox send more than
two messages?

> Max size of transfer is o more than 20B

"is o more"? Typo?

> + * in current x86 LPCI2C bridge.

What does that mean in result?

Regards,

Wolfram


Attachments:
(No filename) (676.00 B)
signature.asc (819.00 B)
Download all attachments

2016-11-20 05:53:03

by Vadim Pasternak

[permalink] [raw]
Subject: RE: [patch v8 1/1] i2c: add master driver for mellanox systems

Hi Wolfram,

> -----Original Message-----
> From: Wolfram Sang [mailto:[email protected]]
> Sent: Saturday, November 19, 2016 11:05 PM
> To: Vadim Pasternak <[email protected]>
> Cc: [email protected]; [email protected]; linux-
> [email protected]; [email protected]; Michael Shych
> <[email protected]>
> Subject: Re: [patch v8 1/1] i2c: add master driver for mellanox systems
>
> Hi,
>
> looks mostly good. I just found this comment which needs clarification:
>
> > + /*
> > + * All upper layers currently are never use transfer with more than
> > + * 2 messages.
>
> Have you really checked ALL of the upper layers? And even if so, there is the dev-
> interface to userspace which allows for arbitrary I2C transfers using I2C_RDWR.
>

We are using the next drivers on our switches: max11603, lm75, pmbus, mlxsw_minimal (last one is new, I wrote it to support Mellanox ASIC and it's has 32 byte limitation for I2C transaction chunk) and at24.

I am planning to add some code in the next patch, for handling bufferization for message length greater then data buffer. Allowed by HW.

Possibly I should add validation for "num" parameter at the beginning of mlxcpld_i2c_xfer?

> > Actually, it's also not so relevant in Mellanox systems
> > + * because of HW limitation.
>
> What kind of HW limitation do you mean here? Can Mellanox send more than
> two messages?
>
> > Max size of transfer is o more than 20B
>
> "is o more"? Typo?

Typo.
Max size of transfer is not more than 32 bytes.

>
> > + * in current x86 LPCI2C bridge.
>
> What does that mean in result?

In case more than 32 bytes of data length will be set in transaction, such message will fail length validation.
The 32 is HW limitation - 32 registers of DATAx.
In new version of LPCI2C logic, there is 64 registers, which can be used for data, and I am going to support it in next patch for this driver.

>
> Regards,
>
> Wolfram

Thanks,

Vadim.

2016-11-20 09:02:24

by Wolfram Sang

[permalink] [raw]
Subject: Re: [patch v8 1/1] i2c: add master driver for mellanox systems


> Possibly I should add validation for "num" parameter at the beginning of mlxcpld_i2c_xfer?

There is 'max_num_msgs' in the quirk structure.

Thanks,

Wolfram


Attachments:
(No filename) (165.00 B)
signature.asc (819.00 B)
Download all attachments

2016-11-20 09:15:08

by Vadim Pasternak

[permalink] [raw]
Subject: RE: [patch v8 1/1] i2c: add master driver for mellanox systems



> -----Original Message-----
> From: Wolfram Sang [mailto:[email protected]]
> Sent: Sunday, November 20, 2016 11:02 AM
> To: Vadim Pasternak <[email protected]>
> Cc: [email protected]; [email protected]; linux-
> [email protected]; [email protected]; Michael Shych
> <[email protected]>
> Subject: Re: [patch v8 1/1] i2c: add master driver for mellanox systems
>
>
> > Possibly I should add validation for "num" parameter at the beginning of
> mlxcpld_i2c_xfer?
>
> There is 'max_num_msgs' in the quirk structure.

Yes, I see it.
So I should just set max_num_msgs to 1, right?

Thanks you,

Vadim.
/
>
> Thanks,
>
> Wolfram