[RFC V2 00/12] i2c: describe adapter quirks in a generic way

[RFC V2 00/12] i2c: describe adapter quirks in a generic way

[Wolfram Sang]

From: Wolfram Sang <wsa+renesas@sang-engineering.com>

Here is the second version of the patch series to describe i2c adapter quirks
in a generic way. For the motivation, please read description of patch 1. This
is still RFC because I would like to do some more tests on my own, but I need
to write a tool for that. However, I'd really like to have the driver authors
to have a look already. Actual testing is very much appreciated. Thanks to the
Mediatek guys for rebasing their new driver to this framework. That helps, too!

The branch is also here:

git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git i2c/quirks

Thanks,

   Wolfram

Major changes since V1:

* more fine-grained options to describe modes with combined messages.
  This should also cover the Mediatek HW now as well as all other
  permutations I can think of.

* the core code and at91 driver had to be refactored to reflect the
  above change

* added the bcm-iproc driver which came to mainline recently

Wolfram Sang (12):
  i2c: add quirk structure to describe adapter flaws
  i2c: add quirk checks to core
  i2c: at91: make use of the new infrastructure for quirks
  i2c: opal: make use of the new infrastructure for quirks
  i2c: qup: make use of the new infrastructure for quirks
  i2c: cpm: make use of the new infrastructure for quirks
  i2c: axxia: make use of the new infrastructure for quirks
  i2c: dln2: make use of the new infrastructure for quirks
  i2c: powermac: make use of the new infrastructure for quirks
  i2c: viperboard: make use of the new infrastructure for quirks
  i2c: pmcmsp: make use of the new infrastructure for quirks
  i2c: bcm-iproc: make use of the new infrastructure for quirks

 drivers/i2c/busses/i2c-at91.c       | 32 +++++++------------
 drivers/i2c/busses/i2c-axxia.c      | 11 ++++---
 drivers/i2c/busses/i2c-bcm-iproc.c  | 15 +++++----
 drivers/i2c/busses/i2c-cpm.c        | 20 ++++++------
 drivers/i2c/busses/i2c-dln2.c       | 12 +++----
 drivers/i2c/busses/i2c-opal.c       | 22 ++++++-------
 drivers/i2c/busses/i2c-pmcmsp.c     | 42 ++++++++++---------------
 drivers/i2c/busses/i2c-powermac.c   | 10 +++---
 drivers/i2c/busses/i2c-qup.c        | 21 ++++++-------
 drivers/i2c/busses/i2c-viperboard.c | 10 +++---
 drivers/i2c/i2c-core.c              | 62 +++++++++++++++++++++++++++++++++++++
 include/linux/i2c.h                 | 43 +++++++++++++++++++++++++
 12 files changed, 191 insertions(+), 109 deletions(-)

-- 
2.1.4

[RFC V2 01/12] i2c: add quirk structure to describe adapter flaws

[Wolfram Sang]

From: Wolfram Sang <wsa+renesas@sang-engineering.com>

The number of I2C adapters which are not fully I2C compatible is rising,
sadly. Drivers usually do handle the flaws, still the user receives only
some errno for a transfer which normally can be expected to work. This
patch introduces a formal description of flaws. One advantage is that
the core can check before the actual transfer if the messages could be
transferred at all. This is done in the next patch. Another advantage is
that we can pass this information to the user so the restrictions are
exactly known and further actions can be based on that. This will be
done later after some stabilization period for this description.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 include/linux/i2c.h | 43 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 43 insertions(+)

diff --git a/include/linux/i2c.h b/include/linux/i2c.h
index f17da50402a4da..243d1a1d78b248 100644
--- a/include/linux/i2c.h
+++ b/include/linux/i2c.h
@@ -449,6 +449,48 @@ int i2c_recover_bus(struct i2c_adapter *adap);
 int i2c_generic_gpio_recovery(struct i2c_adapter *adap);
 int i2c_generic_scl_recovery(struct i2c_adapter *adap);
 
+/**
+ * struct i2c_adapter_quirks - describe flaws of an i2c adapter
+ * @flags: see I2C_AQ_* for possible flags and read below
+ * @max_num_msgs: maximum number of messages per transfer
+ * @max_write_len: maximum length of a write message
+ * @max_read_len: maximum length of a read message
+ * @max_comb_1st_msg_len: maximum length of the first msg in a combined message
+ * @max_comb_2nd_msg_len: maximum length of the second msg in a combined message
+ *
+ * Note about combined messages: Some I2C controllers can only send one message
+ * per transfer, plus something called combined message or write-then-read.
+ * This is (usually) a small write message followed by a read message and
+ * barely enough to access register based devices like EEPROMs. There is a flag
+ * to support this mode. It implies max_num_msg = 2 and does the length checks
+ * with max_comb_*_len because combined message mode usually has its own
+ * limitations. Because of HW implementations, some controllers can actually do
+ * write-then-anything or other variants. To support that, write-then-read has
+ * been broken out into smaller bits like write-first and read-second which can
+ * be combined as needed.
+ */
+
+struct i2c_adapter_quirks {
+	u64 flags;
+	int max_num_msgs;
+	u16 max_write_len;
+	u16 max_read_len;
+	u16 max_comb_1st_msg_len;
+	u16 max_comb_2nd_msg_len;
+};
+
+/* enforce max_num_msgs = 2 and use max_comb_*_len for length checks */
+#define I2C_AQ_COMB			BIT(0)
+/* first combined message must be write */
+#define I2C_AQ_COMB_WRITE_FIRST		BIT(1)
+/* second combined message must be read */
+#define I2C_AQ_COMB_READ_SECOND		BIT(2)
+/* both combined messages must have the same target address */
+#define I2C_AQ_COMB_SAME_ADDR		BIT(3)
+/* convenience macro for typical write-then read case */
+#define I2C_AQ_COMB_WRITE_THEN_READ	(I2C_AQ_COMB | I2C_AQ_COMB_WRITE_FIRST | \
+					 I2C_AQ_COMB_READ_SECOND | I2C_AQ_COMB_SAME_ADDR)
+
 /*
  * i2c_adapter is the structure used to identify a physical i2c bus along
  * with the access algorithms necessary to access it.
@@ -474,6 +516,7 @@ struct i2c_adapter {
 	struct list_head userspace_clients;
 
 	struct i2c_bus_recovery_info *bus_recovery_info;
+	const struct i2c_adapter_quirks *quirks;
 };
 #define to_i2c_adapter(d) container_of(d, struct i2c_adapter, dev)
 
-- 
2.1.4

[RFC V2 02/12] i2c: add quirk checks to core

[Wolfram Sang]

From: Wolfram Sang <wsa+renesas@sang-engineering.com>

Let the core do the checks if HW quirks prevent a transfer. Saves code
from drivers and adds consistency.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 drivers/i2c/i2c-core.c | 62 ++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 62 insertions(+)

diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index 210cf4874cb7ea..c5ca0e4006edb7 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -1929,6 +1929,65 @@ module_exit(i2c_exit);
  * ----------------------------------------------------
  */
 
+/* Check if val is exceeding the quirk IFF quirk is non 0 */
+#define i2c_quirk_exceeded(val, quirk) ((quirk) && ((val) > (quirk)))
+
+static int i2c_quirk_error(struct i2c_adapter *adap, struct i2c_msg *msg, char *err_msg)
+{
+	dev_err_ratelimited(&adap->dev, "quirk: %s (addr 0x%04x, size %u, %s)\n",
+			    err_msg, msg->addr, msg->len,
+			    msg->flags & I2C_M_RD ? "read" : "write");
+	return -EOPNOTSUPP;
+}
+
+static int i2c_check_for_quirks(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
+{
+	const struct i2c_adapter_quirks *q = adap->quirks;
+	int max_num = q->max_num_msgs, i;
+	bool do_len_check = true;
+
+	if (q->flags & I2C_AQ_COMB) {
+		max_num = 2;
+
+		/* special checks for combined messages */
+		if (num == 2) {
+			if (q->flags & I2C_AQ_COMB_WRITE_FIRST && msgs[0].flags & I2C_M_RD)
+				return i2c_quirk_error(adap, &msgs[0], "1st comb msg not write");
+
+			if (q->flags & I2C_AQ_COMB_READ_SECOND && !(msgs[1].flags & I2C_M_RD))
+				return i2c_quirk_error(adap, &msgs[1], "2nd comb msg not read");
+
+			if (q->flags & I2C_AQ_COMB_SAME_ADDR && msgs[0].addr != msgs[1].addr)
+				return i2c_quirk_error(adap, &msgs[0], "addresses do not match");
+
+			if (i2c_quirk_exceeded(msgs[0].len, q->max_comb_1st_msg_len))
+				return i2c_quirk_error(adap, &msgs[0], "msg too long");
+
+			if (i2c_quirk_exceeded(msgs[1].len, q->max_comb_2nd_msg_len))
+				return i2c_quirk_error(adap, &msgs[1], "msg too long");
+
+			do_len_check = false;
+		}
+	}
+
+	if (i2c_quirk_exceeded(num, max_num))
+		return i2c_quirk_error(adap, &msgs[0], "too many messages");
+
+	for (i = 0; i < num; i++) {
+		u16 len = msgs[i].len;
+
+		if (msgs[i].flags & I2C_M_RD) {
+			if (do_len_check && i2c_quirk_exceeded(len, q->max_read_len))
+				return i2c_quirk_error(adap, &msgs[i], "msg too long");
+		} else {
+			if (do_len_check && i2c_quirk_exceeded(len, q->max_write_len))
+				return i2c_quirk_error(adap, &msgs[i], "msg too long");
+		}
+	}
+
+	return 0;
+}
+
 /**
  * __i2c_transfer - unlocked flavor of i2c_transfer
  * @adap: Handle to I2C bus
@@ -1946,6 +2005,9 @@ int __i2c_transfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
 	unsigned long orig_jiffies;
 	int ret, try;
 
+	if (adap->quirks && i2c_check_for_quirks(adap, msgs, num))
+		return -EOPNOTSUPP;
+
 	/* i2c_trace_msg gets enabled when tracepoint i2c_transfer gets
 	 * enabled.  This is an efficient way of keeping the for-loop from
 	 * being executed when not needed.
-- 
2.1.4

[RFC V2 03/12] i2c: at91: make use of the new infrastructure for quirks

[Wolfram Sang]

From: Wolfram Sang <wsa+renesas@sang-engineering.com>

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 drivers/i2c/busses/i2c-at91.c | 32 +++++++++++---------------------
 1 file changed, 11 insertions(+), 21 deletions(-)

diff --git a/drivers/i2c/busses/i2c-at91.c b/drivers/i2c/busses/i2c-at91.c
index 636fd2efad8850..b3a70e8fc653c5 100644
--- a/drivers/i2c/busses/i2c-at91.c
+++ b/drivers/i2c/busses/i2c-at91.c
@@ -487,30 +487,10 @@ static int at91_twi_xfer(struct i2c_adapter *adap, struct i2c_msg *msg, int num)
 	if (ret < 0)
 		goto out;
 
-	/*
-	 * The hardware can handle@most two messages concatenated by a
-	 * repeated start via it's internal address feature.
-	 */
-	if (num > 2) {
-		dev_err(dev->dev,
-			"cannot handle more than two concatenated messages.\n");
-		ret = 0;
-		goto out;
-	} else if (num == 2) {
+	if (num == 2) {
 		int internal_address = 0;
 		int i;
 
-		if (msg->flags & I2C_M_RD) {
-			dev_err(dev->dev, "first transfer must be write.\n");
-			ret = -EINVAL;
-			goto out;
-		}
-		if (msg->len > 3) {
-			dev_err(dev->dev, "first message size must be <= 3.\n");
-			ret = -EINVAL;
-			goto out;
-		}
-
 		/* 1st msg is put into the internal address, start with 2nd */
 		m_start = &msg[1];
 		for (i = 0; i < msg->len; ++i) {
@@ -540,6 +520,15 @@ out:
 	return ret;
 }
 
+/*
+ * The hardware can handle at most two messages concatenated by a
+ * repeated start via it's internal address feature.
+ */
+static struct i2c_adapter_quirks at91_twi_quirks = {
+	.flags = I2C_AQ_COMB | I2C_AQ_COMB_WRITE_FIRST | I2C_AQ_COMB_SAME_ADDR,
+	.max_comb_1st_msg_len = 3,
+};
+
 static u32 at91_twi_func(struct i2c_adapter *adapter)
 {
 	return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL
@@ -777,6 +766,7 @@ static int at91_twi_probe(struct platform_device *pdev)
 	dev->adapter.owner = THIS_MODULE;
 	dev->adapter.class = I2C_CLASS_DEPRECATED;
 	dev->adapter.algo = &at91_twi_algorithm;
+	dev->adapter.quirks = &at91_twi_quirks;
 	dev->adapter.dev.parent = dev->dev;
 	dev->adapter.nr = pdev->id;
 	dev->adapter.timeout = AT91_I2C_TIMEOUT;
-- 
2.1.4

[RFC V2 04/12] i2c: opal: make use of the new infrastructure for quirks

[Wolfram Sang]

From: Wolfram Sang <wsa+renesas@sang-engineering.com>

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 drivers/i2c/busses/i2c-opal.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/i2c/busses/i2c-opal.c b/drivers/i2c/busses/i2c-opal.c
index 16f90b1a750894..b2788ecad5b3cb 100644
--- a/drivers/i2c/busses/i2c-opal.c
+++ b/drivers/i2c/busses/i2c-opal.c
@@ -104,17 +104,6 @@ static int i2c_opal_master_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs,
 		req.buffer_ra = cpu_to_be64(__pa(msgs[0].buf));
 		break;
 	case 2:
-		/* For two messages, we basically support only simple
-		 * smbus transactions of a write plus a read. We might
-		 * want to allow also two writes but we'd have to bounce
-		 * the data into a single buffer.
-		 */
-		if ((msgs[0].flags & I2C_M_RD) || !(msgs[1].flags & I2C_M_RD))
-			return -EOPNOTSUPP;
-		if (msgs[0].len > 4)
-			return -EOPNOTSUPP;
-		if (msgs[0].addr != msgs[1].addr)
-			return -EOPNOTSUPP;
 		req.type = OPAL_I2C_SM_READ;
 		req.addr = cpu_to_be16(msgs[0].addr);
 		req.subaddr_sz = msgs[0].len;
@@ -210,6 +199,16 @@ static const struct i2c_algorithm i2c_opal_algo = {
 	.functionality	= i2c_opal_func,
 };
 
+/* For two messages, we basically support only simple
+ * smbus transactions of a write plus a read. We might
+ * want to allow also two writes but we'd have to bounce
+ * the data into a single buffer.
+ */
+static struct i2c_adapter_quirks i2c_opal_quirks = {
+	.flags = I2C_AQ_COMB_WRITE_THEN_READ,
+	.max_comb_1st_msg_len = 4,
+};
+
 static int i2c_opal_probe(struct platform_device *pdev)
 {
 	struct i2c_adapter	*adapter;
@@ -232,6 +231,7 @@ static int i2c_opal_probe(struct platform_device *pdev)
 
 	adapter->algo = &i2c_opal_algo;
 	adapter->algo_data = (void *)(unsigned long)opal_id;
+	adapter->quirks = &i2c_opal_quirks;
 	adapter->dev.parent = &pdev->dev;
 	adapter->dev.of_node = of_node_get(pdev->dev.of_node);
 	pname = of_get_property(pdev->dev.of_node, "ibm,port-name", NULL);
-- 
2.1.4

[RFC V2 05/12] i2c: qup: make use of the new infrastructure for quirks

[Wolfram Sang]

From: Wolfram Sang <wsa+renesas@sang-engineering.com>

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 drivers/i2c/busses/i2c-qup.c | 21 ++++++++++-----------
 1 file changed, 10 insertions(+), 11 deletions(-)

diff --git a/drivers/i2c/busses/i2c-qup.c b/drivers/i2c/busses/i2c-qup.c
index 4dad23bdffbe90..fdcbdab808e9fc 100644
--- a/drivers/i2c/busses/i2c-qup.c
+++ b/drivers/i2c/busses/i2c-qup.c
@@ -412,17 +412,6 @@ static int qup_i2c_read_one(struct qup_i2c_dev *qup, struct i2c_msg *msg)
 	unsigned long left;
 	int ret;
 
-	/*
-	 * The QUP block will issue a NACK and STOP on the bus when reaching
-	 * the end of the read, the length of the read is specified as one byte
-	 * which limits the possible read to 256 (QUP_READ_LIMIT) bytes.
-	 */
-	if (msg->len > QUP_READ_LIMIT) {
-		dev_err(qup->dev, "HW not capable of reads over %d bytes\n",
-			QUP_READ_LIMIT);
-		return -EINVAL;
-	}
-
 	qup->msg = msg;
 	qup->pos  = 0;
 
@@ -534,6 +523,15 @@ static const struct i2c_algorithm qup_i2c_algo = {
 	.functionality	= qup_i2c_func,
 };
 
+/*
+ * The QUP block will issue a NACK and STOP on the bus when reaching
+ * the end of the read, the length of the read is specified as one byte
+ * which limits the possible read to 256 (QUP_READ_LIMIT) bytes.
+ */
+static struct i2c_adapter_quirks qup_i2c_quirks = {
+	.max_read_len = QUP_READ_LIMIT,
+};
+
 static void qup_i2c_enable_clocks(struct qup_i2c_dev *qup)
 {
 	clk_prepare_enable(qup->clk);
@@ -670,6 +668,7 @@ static int qup_i2c_probe(struct platform_device *pdev)
 
 	i2c_set_adapdata(&qup->adap, qup);
 	qup->adap.algo = &qup_i2c_algo;
+	qup->adap.quirks = &qup_i2c_quirks;
 	qup->adap.dev.parent = qup->dev;
 	qup->adap.dev.of_node = pdev->dev.of_node;
 	strlcpy(qup->adap.name, "QUP I2C adapter", sizeof(qup->adap.name));
-- 
2.1.4

[RFC V2 06/12] i2c: cpm: make use of the new infrastructure for quirks

[Wolfram Sang]

From: Wolfram Sang <wsa+renesas@sang-engineering.com>

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 drivers/i2c/busses/i2c-cpm.c | 20 +++++++++-----------
 1 file changed, 9 insertions(+), 11 deletions(-)

diff --git a/drivers/i2c/busses/i2c-cpm.c b/drivers/i2c/busses/i2c-cpm.c
index 2d466538b2e2c9..714bdc837769fd 100644
--- a/drivers/i2c/busses/i2c-cpm.c
+++ b/drivers/i2c/busses/i2c-cpm.c
@@ -308,22 +308,12 @@ static int cpm_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
 	struct i2c_reg __iomem *i2c_reg = cpm->i2c_reg;
 	struct i2c_ram __iomem *i2c_ram = cpm->i2c_ram;
 	struct i2c_msg *pmsg;
-	int ret, i;
+	int ret;
 	int tptr;
 	int rptr;
 	cbd_t __iomem *tbdf;
 	cbd_t __iomem *rbdf;
 
-	if (num > CPM_MAXBD)
-		return -EINVAL;
-
-	/* Check if we have any oversized READ requests */
-	for (i = 0; i < num; i++) {
-		pmsg = &msgs[i];
-		if (pmsg->len >= CPM_MAX_READ)
-			return -EINVAL;
-	}
-
 	/* Reset to use first buffer */
 	out_be16(&i2c_ram->rbptr, in_be16(&i2c_ram->rbase));
 	out_be16(&i2c_ram->tbptr, in_be16(&i2c_ram->tbase));
@@ -424,10 +414,18 @@ static const struct i2c_algorithm cpm_i2c_algo = {
 	.functionality = cpm_i2c_func,
 };
 
+/* CPM_MAX_READ is also limiting writes according to the code! */
+static struct i2c_adapter_quirks cpm_i2c_quirks = {
+	.max_num_msgs = CPM_MAXBD,
+	.max_read_len = CPM_MAX_READ,
+	.max_write_len = CPM_MAX_READ,
+};
+
 static const struct i2c_adapter cpm_ops = {
 	.owner		= THIS_MODULE,
 	.name		= "i2c-cpm",
 	.algo		= &cpm_i2c_algo,
+	.quirks		= &cpm_i2c_quirks,
 };
 
 static int cpm_i2c_setup(struct cpm_i2c *cpm)
-- 
2.1.4

[RFC V2 07/12] i2c: axxia: make use of the new infrastructure for quirks

[Wolfram Sang]

From: Wolfram Sang <wsa+renesas@sang-engineering.com>

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 drivers/i2c/busses/i2c-axxia.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/i2c/busses/i2c-axxia.c b/drivers/i2c/busses/i2c-axxia.c
index 768a598d8d03ad..488c5d3bf9dba7 100644
--- a/drivers/i2c/busses/i2c-axxia.c
+++ b/drivers/i2c/busses/i2c-axxia.c
@@ -336,11 +336,6 @@ static int axxia_i2c_xfer_msg(struct axxia_i2c_dev *idev, struct i2c_msg *msg)
 	u32 addr_1, addr_2;
 	int ret;
 
-	if (msg->len > 255) {
-		dev_warn(idev->dev, "unsupported length %u\n", msg->len);
-		return -EINVAL;
-	}
-
 	idev->msg = msg;
 	idev->msg_xfrd = 0;
 	idev->msg_err = 0;
@@ -454,6 +449,11 @@ static const struct i2c_algorithm axxia_i2c_algo = {
 	.functionality = axxia_i2c_func,
 };
 
+static struct i2c_adapter_quirks axxia_i2c_quirks = {
+	.max_read_len = 255,
+	.max_write_len = 255,
+};
+
 static int axxia_i2c_probe(struct platform_device *pdev)
 {
 	struct device_node *np = pdev->dev.of_node;
@@ -511,6 +511,7 @@ static int axxia_i2c_probe(struct platform_device *pdev)
 	strlcpy(idev->adapter.name, pdev->name, sizeof(idev->adapter.name));
 	idev->adapter.owner = THIS_MODULE;
 	idev->adapter.algo = &axxia_i2c_algo;
+	idev->adapter.quirks = &axxia_i2c_quirks;
 	idev->adapter.dev.parent = &pdev->dev;
 	idev->adapter.dev.of_node = pdev->dev.of_node;
 
-- 
2.1.4

[RFC V2 08/12] i2c: dln2: make use of the new infrastructure for quirks

[Wolfram Sang]

From: Wolfram Sang <wsa+renesas@sang-engineering.com>

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 drivers/i2c/busses/i2c-dln2.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/i2c/busses/i2c-dln2.c b/drivers/i2c/busses/i2c-dln2.c
index b3fb86af4cbb14..b6f9ba7eb17564 100644
--- a/drivers/i2c/busses/i2c-dln2.c
+++ b/drivers/i2c/busses/i2c-dln2.c
@@ -144,7 +144,6 @@ static int dln2_i2c_xfer(struct i2c_adapter *adapter,
 {
 	struct dln2_i2c *dln2 = i2c_get_adapdata(adapter);
 	struct i2c_msg *pmsg;
-	struct device *dev = &dln2->adapter.dev;
 	int i;
 
 	for (i = 0; i < num; i++) {
@@ -152,11 +151,6 @@ static int dln2_i2c_xfer(struct i2c_adapter *adapter,
 
 		pmsg = &msgs[i];
 
-		if (pmsg->len > DLN2_I2C_MAX_XFER_SIZE) {
-			dev_warn(dev, "maximum transfer size exceeded\n");
-			return -EOPNOTSUPP;
-		}
-
 		if (pmsg->flags & I2C_M_RD) {
 			ret = dln2_i2c_read(dln2, pmsg->addr, pmsg->buf,
 					    pmsg->len);
@@ -187,6 +181,11 @@ static const struct i2c_algorithm dln2_i2c_usb_algorithm = {
 	.functionality = dln2_i2c_func,
 };
 
+static struct i2c_adapter_quirks dln2_i2c_quirks = {
+	.max_read_len = DLN2_I2C_MAX_XFER_SIZE,
+	.max_write_len = DLN2_I2C_MAX_XFER_SIZE,
+};
+
 static int dln2_i2c_probe(struct platform_device *pdev)
 {
 	int ret;
@@ -209,6 +208,7 @@ static int dln2_i2c_probe(struct platform_device *pdev)
 	dln2->adapter.owner = THIS_MODULE;
 	dln2->adapter.class = I2C_CLASS_HWMON;
 	dln2->adapter.algo = &dln2_i2c_usb_algorithm;
+	dln2->adapter.quirks = &dln2_i2c_quirks;
 	dln2->adapter.dev.parent = dev;
 	i2c_set_adapdata(&dln2->adapter, dln2);
 	snprintf(dln2->adapter.name, sizeof(dln2->adapter.name), "%s-%s-%d",
-- 
2.1.4

[RFC V2 09/12] i2c: powermac: make use of the new infrastructure for quirks

[Wolfram Sang]

From: Wolfram Sang <wsa+renesas@sang-engineering.com>

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 drivers/i2c/busses/i2c-powermac.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/i2c/busses/i2c-powermac.c b/drivers/i2c/busses/i2c-powermac.c
index 60a53c169ed2b3..6abcf696e3594b 100644
--- a/drivers/i2c/busses/i2c-powermac.c
+++ b/drivers/i2c/busses/i2c-powermac.c
@@ -153,12 +153,6 @@ static int i2c_powermac_master_xfer(	struct i2c_adapter *adap,
 	int			read;
 	int			addrdir;
 
-	if (num != 1) {
-		dev_err(&adap->dev,
-			"Multi-message I2C transactions not supported\n");
-		return -EOPNOTSUPP;
-	}
-
 	if (msgs->flags & I2C_M_TEN)
 		return -EINVAL;
 	read = (msgs->flags & I2C_M_RD) != 0;
@@ -205,6 +199,9 @@ static const struct i2c_algorithm i2c_powermac_algorithm = {
 	.functionality	= i2c_powermac_func,
 };
 
+static struct i2c_adapter_quirks i2c_powermac_quirks = {
+	.max_num_msgs = 1,
+};
 
 static int i2c_powermac_remove(struct platform_device *dev)
 {
@@ -434,6 +431,7 @@ static int i2c_powermac_probe(struct platform_device *dev)
 
 	platform_set_drvdata(dev, adapter);
 	adapter->algo = &i2c_powermac_algorithm;
+	adapter->quirks = &i2c_powermac_quirks;
 	i2c_set_adapdata(adapter, bus);
 	adapter->dev.parent = &dev->dev;
 
-- 
2.1.4

[RFC V2 10/12] i2c: viperboard: make use of the new infrastructure for quirks

[Wolfram Sang]

From: Wolfram Sang <wsa+renesas@sang-engineering.com>

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 drivers/i2c/busses/i2c-viperboard.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/i2c/busses/i2c-viperboard.c b/drivers/i2c/busses/i2c-viperboard.c
index 7533fa34d73711..47e88adf2011e1 100644
--- a/drivers/i2c/busses/i2c-viperboard.c
+++ b/drivers/i2c/busses/i2c-viperboard.c
@@ -288,10 +288,6 @@ static int vprbrd_i2c_xfer(struct i2c_adapter *i2c, struct i2c_msg *msgs,
 			i, pmsg->flags & I2C_M_RD ? "read" : "write",
 			pmsg->flags, pmsg->len, pmsg->addr);
 
-		/* msgs longer than 2048 bytes are not supported by adapter */
-		if (pmsg->len > 2048)
-			return -EINVAL;
-
 		mutex_lock(&vb->lock);
 		/* directly send the message */
 		if (pmsg->flags & I2C_M_RD) {
@@ -358,6 +354,11 @@ static const struct i2c_algorithm vprbrd_algorithm = {
 	.functionality	= vprbrd_i2c_func,
 };
 
+static struct i2c_adapter_quirks vprbrd_quirks = {
+	.max_read_len = 2048,
+	.max_write_len = 2048,
+};
+
 static int vprbrd_i2c_probe(struct platform_device *pdev)
 {
 	struct vprbrd *vb = dev_get_drvdata(pdev->dev.parent);
@@ -373,6 +374,7 @@ static int vprbrd_i2c_probe(struct platform_device *pdev)
 	vb_i2c->i2c.owner = THIS_MODULE;
 	vb_i2c->i2c.class = I2C_CLASS_HWMON;
 	vb_i2c->i2c.algo = &vprbrd_algorithm;
+	vb_i2c->i2c.quirks = &vprbrd_quirks;
 	vb_i2c->i2c.algo_data = vb;
 	/* save the param in usb capabable memory */
 	vb_i2c->bus_freq_param = i2c_bus_param;
-- 
2.1.4

[RFC V2 11/12] i2c: pmcmsp: make use of the new infrastructure for quirks

[Wolfram Sang]

From: Wolfram Sang <wsa+renesas@sang-engineering.com>

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 drivers/i2c/busses/i2c-pmcmsp.c | 42 ++++++++++++++++-------------------------
 1 file changed, 16 insertions(+), 26 deletions(-)

diff --git a/drivers/i2c/busses/i2c-pmcmsp.c b/drivers/i2c/busses/i2c-pmcmsp.c
index d37d9db6681e7b..2c40edbf6224eb 100644
--- a/drivers/i2c/busses/i2c-pmcmsp.c
+++ b/drivers/i2c/busses/i2c-pmcmsp.c
@@ -456,14 +456,6 @@ static enum pmcmsptwi_xfer_result pmcmsptwi_xfer_cmd(
 		return -EINVAL;
 	}
 
-	if (cmd->read_len > MSP_MAX_BYTES_PER_RW ||
-	    cmd->write_len > MSP_MAX_BYTES_PER_RW) {
-		dev_err(&pmcmsptwi_adapter.dev,
-			"%s: Cannot transfer more than %d bytes\n",
-			__func__, MSP_MAX_BYTES_PER_RW);
-		return -EINVAL;
-	}
-
 	mutex_lock(&data->lock);
 	dev_dbg(&pmcmsptwi_adapter.dev,
 		"Setting address to 0x%04x\n", cmd->addr);
@@ -520,25 +512,14 @@ static int pmcmsptwi_master_xfer(struct i2c_adapter *adap,
 	struct pmcmsptwi_cfg oldcfg, newcfg;
 	int ret;
 
-	if (num > 2) {
-		dev_dbg(&adap->dev, "%d messages unsupported\n", num);
-		return -EINVAL;
-	} else if (num == 2) {
-		/* Check for a dual write-then-read command */
+	if (num == 2) {
 		struct i2c_msg *nextmsg = msg + 1;
-		if (!(msg->flags & I2C_M_RD) &&
-		    (nextmsg->flags & I2C_M_RD) &&
-		    msg->addr == nextmsg->addr) {
-			cmd.type = MSP_TWI_CMD_WRITE_READ;
-			cmd.write_len = msg->len;
-			cmd.write_data = msg->buf;
-			cmd.read_len = nextmsg->len;
-			cmd.read_data = nextmsg->buf;
-		} else {
-			dev_dbg(&adap->dev,
-				"Non write-read dual messages unsupported\n");
-			return -EINVAL;
-		}
+
+		cmd.type = MSP_TWI_CMD_WRITE_READ;
+		cmd.write_len = msg->len;
+		cmd.write_data = msg->buf;
+		cmd.read_len = nextmsg->len;
+		cmd.read_data = nextmsg->buf;
 	} else if (msg->flags & I2C_M_RD) {
 		cmd.type = MSP_TWI_CMD_READ;
 		cmd.read_len = msg->len;
@@ -598,6 +579,14 @@ static u32 pmcmsptwi_i2c_func(struct i2c_adapter *adapter)
 		I2C_FUNC_SMBUS_WORD_DATA | I2C_FUNC_SMBUS_PROC_CALL;
 }
 
+static struct i2c_adapter_quirks pmcmsptwi_i2c_quirks = {
+	.flags = I2C_AQ_COMB_WRITE_THEN_READ,
+	.max_write_len = MSP_MAX_BYTES_PER_RW,
+	.max_read_len = MSP_MAX_BYTES_PER_RW,
+	.max_comb_1st_msg_len = MSP_MAX_BYTES_PER_RW,
+	.max_comb_2nd_msg_len = MSP_MAX_BYTES_PER_RW,
+};
+
 /* -- Initialization -- */
 
 static struct i2c_algorithm pmcmsptwi_algo = {
@@ -609,6 +598,7 @@ static struct i2c_adapter pmcmsptwi_adapter = {
 	.owner		= THIS_MODULE,
 	.class		= I2C_CLASS_HWMON | I2C_CLASS_SPD,
 	.algo		= &pmcmsptwi_algo,
+	.quirks		= &pmcmsptwi_i2c_quirks,
 	.name		= DRV_NAME,
 };
 
-- 
2.1.4

[RFC V2 12/12] i2c: bcm-iproc: make use of the new infrastructure for quirks

[Wolfram Sang]

From: Wolfram Sang <wsa+renesas@sang-engineering.com>

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 drivers/i2c/busses/i2c-bcm-iproc.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/drivers/i2c/busses/i2c-bcm-iproc.c b/drivers/i2c/busses/i2c-bcm-iproc.c
index d3c89157b33774..f9f2c2082151e2 100644
--- a/drivers/i2c/busses/i2c-bcm-iproc.c
+++ b/drivers/i2c/busses/i2c-bcm-iproc.c
@@ -160,14 +160,6 @@ static int bcm_iproc_i2c_xfer_single_msg(struct bcm_iproc_i2c_dev *iproc_i2c,
 	u32 val;
 	unsigned long time_left = msecs_to_jiffies(I2C_TIMEOUT_MESC);
 
-	/* need to reserve one byte in the FIFO for the slave address */
-	if (msg->len > M_TX_RX_FIFO_SIZE - 1) {
-		dev_err(iproc_i2c->device,
-			"only support data length up to %u bytes\n",
-			M_TX_RX_FIFO_SIZE - 1);
-		return -EOPNOTSUPP;
-	}
-
 	/* check if bus is busy */
 	if (!!(readl(iproc_i2c->base + M_CMD_OFFSET) &
 	       BIT(M_CMD_START_BUSY_SHIFT))) {
@@ -287,6 +279,12 @@ static const struct i2c_algorithm bcm_iproc_algo = {
 	.functionality = bcm_iproc_i2c_functionality,
 };
 
+static struct i2c_adapter_quirks bcm_iproc_i2c_quirks = {
+	/* need to reserve one byte in the FIFO for the slave address */
+	.max_read_len = M_TX_RX_FIFO_SIZE - 1,
+	.max_write_len = M_TX_RX_FIFO_SIZE - 1,
+};
+
 static int bcm_iproc_i2c_cfg_speed(struct bcm_iproc_i2c_dev *iproc_i2c)
 {
 	unsigned int bus_speed;
@@ -413,6 +411,7 @@ static int bcm_iproc_i2c_probe(struct platform_device *pdev)
 	i2c_set_adapdata(adap, iproc_i2c);
 	strlcpy(adap->name, "Broadcom iProc I2C adapter", sizeof(adap->name));
 	adap->algo = &bcm_iproc_algo;
+	adap->quirks = &bcm_iproc_i2c_quirks;
 	adap->dev.parent = &pdev->dev;
 	adap->dev.of_node = pdev->dev.of_node;
 
-- 
2.1.4

[RFC V2 12/12] i2c: bcm-iproc: make use of the new infrastructure for quirks

[Ray Jui]

On 2/25/2015 8:02 AM, Wolfram Sang wrote:
> From: Wolfram Sang <wsa+renesas@sang-engineering.com>
> 
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> ---
>  drivers/i2c/busses/i2c-bcm-iproc.c | 15 +++++++--------
>  1 file changed, 7 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-bcm-iproc.c b/drivers/i2c/busses/i2c-bcm-iproc.c
> index d3c89157b33774..f9f2c2082151e2 100644
> --- a/drivers/i2c/busses/i2c-bcm-iproc.c
> +++ b/drivers/i2c/busses/i2c-bcm-iproc.c
> @@ -160,14 +160,6 @@ static int bcm_iproc_i2c_xfer_single_msg(struct bcm_iproc_i2c_dev *iproc_i2c,
>  	u32 val;
>  	unsigned long time_left = msecs_to_jiffies(I2C_TIMEOUT_MESC);
>  
> -	/* need to reserve one byte in the FIFO for the slave address */
> -	if (msg->len > M_TX_RX_FIFO_SIZE - 1) {
> -		dev_err(iproc_i2c->device,
> -			"only support data length up to %u bytes\n",
> -			M_TX_RX_FIFO_SIZE - 1);
> -		return -EOPNOTSUPP;
> -	}
> -
>  	/* check if bus is busy */
>  	if (!!(readl(iproc_i2c->base + M_CMD_OFFSET) &
>  	       BIT(M_CMD_START_BUSY_SHIFT))) {
> @@ -287,6 +279,12 @@ static const struct i2c_algorithm bcm_iproc_algo = {
>  	.functionality = bcm_iproc_i2c_functionality,
>  };
>  
> +static struct i2c_adapter_quirks bcm_iproc_i2c_quirks = {
> +	/* need to reserve one byte in the FIFO for the slave address */
> +	.max_read_len = M_TX_RX_FIFO_SIZE - 1,
> +	.max_write_len = M_TX_RX_FIFO_SIZE - 1,
> +};
> +
>  static int bcm_iproc_i2c_cfg_speed(struct bcm_iproc_i2c_dev *iproc_i2c)
>  {
>  	unsigned int bus_speed;
> @@ -413,6 +411,7 @@ static int bcm_iproc_i2c_probe(struct platform_device *pdev)
>  	i2c_set_adapdata(adap, iproc_i2c);
>  	strlcpy(adap->name, "Broadcom iProc I2C adapter", sizeof(adap->name));
>  	adap->algo = &bcm_iproc_algo;
> +	adap->quirks = &bcm_iproc_i2c_quirks;
>  	adap->dev.parent = &pdev->dev;
>  	adap->dev.of_node = pdev->dev.of_node;
>  
> 

Change on the iproc i2c driver looks good to me. Sanity tested the
change from Wolfram's i2c/quirks branch on Cygnus 958300K combo board.
Sanity tested with an attempt to transfer large amount of I2C data to
ensure the transfer is denied by the i2c-core:

/ # cat /dev/i2c-0
[  657.310261] i2c i2c-0: quirk: msg too long (addr 0x0000, size 4096, read)

Reviewed-by: Ray Jui <rjui@broadcom.com>
Tested-by: Ray Jui <rjui@broadcom.com>

Thanks,

Ray

[RFC V2 00/12] i2c: describe adapter quirks in a generic way

[Ivan T. Ivanov]

On Wed, 2015-02-25 at 17:01 +0100, Wolfram Sang wrote:
> From: Wolfram Sang <wsa+renesas@sang-engineering.com>
> 
> Here is the second version of the patch series to describe i2c adapter quirks
> in a generic way. For the motivation, please read description of patch 1. This
> is still RFC because I would like to do some more tests on my own, but I need
> to write a tool for that. However, I'd really like to have the driver authors
> to have a look already. Actual testing is very much appreciated. Thanks to the
> Mediatek guys for rebasing their new driver to this framework. That helps, too!
> 
> The branch is also here:
> 
> git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git i2c/quirks
> 
> Thanks,
> 
>    Wolfram
> 
> Major changes since V1:
> 
> * more fine-grained options to describe modes with combined messages.
>   This should also cover the Mediatek HW now as well as all other
>   permutations I can think of.
> 
> * the core code and at91 driver had to be refactored to reflect the
>   above change
> 
> * added the bcm-iproc driver which came to mainline recently
> 
> Wolfram Sang (12):
>   i2c: add quirk structure to describe adapter flaws
>   i2c: add quirk checks to core
>   i2c: at91: make use of the new infrastructure for quirks
>   i2c: opal: make use of the new infrastructure for quirks
>   i2c: qup: make use of the new infrastructure for quirks
> 

For QUP driver. 

Reviewed-by: Ivan T. Ivanov <iivanov@mm-sol.com>
Tested-by: Ivan T. Ivanov <iivanov@mm-sol.com>

Thanks,
Ivan

[RFC V2 03/12] i2c: at91: make use of the new infrastructure for quirks

[Wolfram Sang]

On Wed, Feb 25, 2015 at 05:01:54PM +0100, Wolfram Sang wrote:
> From: Wolfram Sang <wsa+renesas@sang-engineering.com>
> 
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

Hi Ludovic,

if you have a few minutes, could you please test this series? I'd like to
include it in 4.1. and because at91 is using the quirk infrastructure in
a more complex way, it is a really good test candidate.

Thanks,

   Wolfram

> ---
>  drivers/i2c/busses/i2c-at91.c | 32 +++++++++++---------------------
>  1 file changed, 11 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-at91.c b/drivers/i2c/busses/i2c-at91.c
> index 636fd2efad8850..b3a70e8fc653c5 100644
> --- a/drivers/i2c/busses/i2c-at91.c
> +++ b/drivers/i2c/busses/i2c-at91.c
> @@ -487,30 +487,10 @@ static int at91_twi_xfer(struct i2c_adapter *adap, struct i2c_msg *msg, int num)
>  	if (ret < 0)
>  		goto out;
>  
> -	/*
> -	 * The hardware can handle at most two messages concatenated by a
> -	 * repeated start via it's internal address feature.
> -	 */
> -	if (num > 2) {
> -		dev_err(dev->dev,
> -			"cannot handle more than two concatenated messages.\n");
> -		ret = 0;
> -		goto out;
> -	} else if (num == 2) {
> +	if (num == 2) {
>  		int internal_address = 0;
>  		int i;
>  
> -		if (msg->flags & I2C_M_RD) {
> -			dev_err(dev->dev, "first transfer must be write.\n");
> -			ret = -EINVAL;
> -			goto out;
> -		}
> -		if (msg->len > 3) {
> -			dev_err(dev->dev, "first message size must be <= 3.\n");
> -			ret = -EINVAL;
> -			goto out;
> -		}
> -
>  		/* 1st msg is put into the internal address, start with 2nd */
>  		m_start = &msg[1];
>  		for (i = 0; i < msg->len; ++i) {
> @@ -540,6 +520,15 @@ out:
>  	return ret;
>  }
>  
> +/*
> + * The hardware can handle at most two messages concatenated by a
> + * repeated start via it's internal address feature.
> + */
> +static struct i2c_adapter_quirks at91_twi_quirks = {
> +	.flags = I2C_AQ_COMB | I2C_AQ_COMB_WRITE_FIRST | I2C_AQ_COMB_SAME_ADDR,
> +	.max_comb_1st_msg_len = 3,
> +};
> +
>  static u32 at91_twi_func(struct i2c_adapter *adapter)
>  {
>  	return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL
> @@ -777,6 +766,7 @@ static int at91_twi_probe(struct platform_device *pdev)
>  	dev->adapter.owner = THIS_MODULE;
>  	dev->adapter.class = I2C_CLASS_DEPRECATED;
>  	dev->adapter.algo = &at91_twi_algorithm;
> +	dev->adapter.quirks = &at91_twi_quirks;
>  	dev->adapter.dev.parent = dev->dev;
>  	dev->adapter.nr = pdev->id;
>  	dev->adapter.timeout = AT91_I2C_TIMEOUT;
> -- 
> 2.1.4
> 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150308/eb77cf37/attachment.sig>

[RFC V2 03/12] i2c: at91: make use of the new infrastructure for quirks

[Ludovic Desroches]

Hi Wolfram,

On Sun, Mar 08, 2015 at 09:28:45AM +0100, Wolfram Sang wrote:
> On Wed, Feb 25, 2015 at 05:01:54PM +0100, Wolfram Sang wrote:
> > From: Wolfram Sang <wsa+renesas@sang-engineering.com>
> > 
> > Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> 
> Hi Ludovic,
> 
> if you have a few minutes, could you please test this series? I'd like to
> include it in 4.1. and because at91 is using the quirk infrastructure in
> a more complex way, it is a really good test candidate.

It was in the pipe. I have reviewed it, this second version seems to be
good. I am just waiting a bit more to give you my ack since I have some
issues to read an i2c eeprom (it works with a temperature sensor).
I am investigating if it doesn't come from a previous regression.

> 
> Thanks,
> 
>    Wolfram
> 

Regards

Ludovic

[RFC V2 03/12] i2c: at91: make use of the new infrastructure for quirks

[Ludovic Desroches]

Hi Wolfram,

You can add my 

Acked-by and Tested-By: Ludovic Desroches <ludovic.desroches@atmel.com>

Tested on sama5d3, some problems with at24 eeprom on sama5d4 but it
doesn't come from the i2c quirks patch series.

Regards

Ludovic

On Sun, Mar 08, 2015 at 09:28:45AM +0100, Wolfram Sang wrote:
> On Wed, Feb 25, 2015 at 05:01:54PM +0100, Wolfram Sang wrote:
> > From: Wolfram Sang <wsa+renesas@sang-engineering.com>
> > 
> > Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> 
> Hi Ludovic,
> 
> if you have a few minutes, could you please test this series? I'd like to
> include it in 4.1. and because at91 is using the quirk infrastructure in
> a more complex way, it is a really good test candidate.
> 
> Thanks,
> 
>    Wolfram
> 
> > ---
> >  drivers/i2c/busses/i2c-at91.c | 32 +++++++++++---------------------
> >  1 file changed, 11 insertions(+), 21 deletions(-)
> > 
> > diff --git a/drivers/i2c/busses/i2c-at91.c b/drivers/i2c/busses/i2c-at91.c
> > index 636fd2efad8850..b3a70e8fc653c5 100644
> > --- a/drivers/i2c/busses/i2c-at91.c
> > +++ b/drivers/i2c/busses/i2c-at91.c
> > @@ -487,30 +487,10 @@ static int at91_twi_xfer(struct i2c_adapter *adap, struct i2c_msg *msg, int num)
> >  	if (ret < 0)
> >  		goto out;
> >  
> > -	/*
> > -	 * The hardware can handle at most two messages concatenated by a
> > -	 * repeated start via it's internal address feature.
> > -	 */
> > -	if (num > 2) {
> > -		dev_err(dev->dev,
> > -			"cannot handle more than two concatenated messages.\n");
> > -		ret = 0;
> > -		goto out;
> > -	} else if (num == 2) {
> > +	if (num == 2) {
> >  		int internal_address = 0;
> >  		int i;
> >  
> > -		if (msg->flags & I2C_M_RD) {
> > -			dev_err(dev->dev, "first transfer must be write.\n");
> > -			ret = -EINVAL;
> > -			goto out;
> > -		}
> > -		if (msg->len > 3) {
> > -			dev_err(dev->dev, "first message size must be <= 3.\n");
> > -			ret = -EINVAL;
> > -			goto out;
> > -		}
> > -
> >  		/* 1st msg is put into the internal address, start with 2nd */
> >  		m_start = &msg[1];
> >  		for (i = 0; i < msg->len; ++i) {
> > @@ -540,6 +520,15 @@ out:
> >  	return ret;
> >  }
> >  
> > +/*
> > + * The hardware can handle at most two messages concatenated by a
> > + * repeated start via it's internal address feature.
> > + */
> > +static struct i2c_adapter_quirks at91_twi_quirks = {
> > +	.flags = I2C_AQ_COMB | I2C_AQ_COMB_WRITE_FIRST | I2C_AQ_COMB_SAME_ADDR,
> > +	.max_comb_1st_msg_len = 3,
> > +};
> > +
> >  static u32 at91_twi_func(struct i2c_adapter *adapter)
> >  {
> >  	return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL
> > @@ -777,6 +766,7 @@ static int at91_twi_probe(struct platform_device *pdev)
> >  	dev->adapter.owner = THIS_MODULE;
> >  	dev->adapter.class = I2C_CLASS_DEPRECATED;
> >  	dev->adapter.algo = &at91_twi_algorithm;
> > +	dev->adapter.quirks = &at91_twi_quirks;
> >  	dev->adapter.dev.parent = dev->dev;
> >  	dev->adapter.nr = pdev->id;
> >  	dev->adapter.timeout = AT91_I2C_TIMEOUT;
> > -- 
> > 2.1.4
> > 

[RFC V2 04/12] i2c: opal: make use of the new infrastructure for quirks

[Neelesh Gupta]

I tested the i2c opal driver after updating the patch as below.
Basically I think we can also support write-then-{read/write}
for the number of messages = 2.
Ben, any issues if we support both write plus read/write in the
opal driver ?

Regards,
Neelesh


  drivers/i2c/busses/i2c-opal.c |   20 ++++++++------------
  1 file changed, 8 insertions(+), 12 deletions(-)

diff --git a/drivers/i2c/busses/i2c-opal.c b/drivers/i2c/busses/i2c-opal.c
index 16f90b1..85412ba 100644
--- a/drivers/i2c/busses/i2c-opal.c
+++ b/drivers/i2c/busses/i2c-opal.c
@@ -104,18 +104,8 @@ static int i2c_opal_master_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs,
  		req.buffer_ra = cpu_to_be64(__pa(msgs[0].buf));
  		break;
  	case 2:
-		/* For two messages, we basically support only simple
-		 * smbus transactions of a write plus a read. We might
-		 * want to allow also two writes but we'd have to bounce
-		 * the data into a single buffer.
-		 */
-		if ((msgs[0].flags & I2C_M_RD) || !(msgs[1].flags & I2C_M_RD))
-			return -EOPNOTSUPP;
-		if (msgs[0].len > 4)
-			return -EOPNOTSUPP;
-		if (msgs[0].addr != msgs[1].addr)
-			return -EOPNOTSUPP;
-		req.type = OPAL_I2C_SM_READ;
+		req.type = (msgs[1].flags & I2C_M_RD) ?
+			OPAL_I2C_SM_READ : OPAL_I2C_SM_WRITE;
  		req.addr = cpu_to_be16(msgs[0].addr);
  		req.subaddr_sz = msgs[0].len;
  		for (i = 0; i < msgs[0].len; i++)
@@ -210,6 +200,11 @@ static const struct i2c_algorithm i2c_opal_algo = {
  	.functionality	= i2c_opal_func,
  };

+static struct i2c_adapter_quirks i2c_opal_quirks = {
+	.flags = I2C_AQ_COMB | I2C_AQ_COMB_WRITE_FIRST | I2C_AQ_COMB_SAME_ADDR,
+	.max_comb_1st_msg_len = 4,
+};
+
  static int i2c_opal_probe(struct platform_device *pdev)
  {
  	struct i2c_adapter	*adapter;
@@ -232,6 +227,7 @@ static int i2c_opal_probe(struct platform_device *pdev)

  	adapter->algo = &i2c_opal_algo;
  	adapter->algo_data = (void *)(unsigned long)opal_id;
+	adapter->quirks = &i2c_opal_quirks;
  	adapter->dev.parent = &pdev->dev;
  	adapter->dev.of_node = of_node_get(pdev->dev.of_node);
  	pname = of_get_property(pdev->dev.of_node, "ibm,port-name", NULL);



On 02/25/2015 09:31 PM, Wolfram Sang wrote:
> From: Wolfram Sang <wsa+renesas@sang-engineering.com>
>
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> ---
>   drivers/i2c/busses/i2c-opal.c | 22 +++++++++++-----------
>   1 file changed, 11 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-opal.c b/drivers/i2c/busses/i2c-opal.c
> index 16f90b1a750894..b2788ecad5b3cb 100644
> --- a/drivers/i2c/busses/i2c-opal.c
> +++ b/drivers/i2c/busses/i2c-opal.c
> @@ -104,17 +104,6 @@ static int i2c_opal_master_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs,
>   		req.buffer_ra = cpu_to_be64(__pa(msgs[0].buf));
>   		break;
>   	case 2:
> -		/* For two messages, we basically support only simple
> -		 * smbus transactions of a write plus a read. We might
> -		 * want to allow also two writes but we'd have to bounce
> -		 * the data into a single buffer.
> -		 */
> -		if ((msgs[0].flags & I2C_M_RD) || !(msgs[1].flags & I2C_M_RD))
> -			return -EOPNOTSUPP;
> -		if (msgs[0].len > 4)
> -			return -EOPNOTSUPP;
> -		if (msgs[0].addr != msgs[1].addr)
> -			return -EOPNOTSUPP;
>   		req.type = OPAL_I2C_SM_READ;
>   		req.addr = cpu_to_be16(msgs[0].addr);
>   		req.subaddr_sz = msgs[0].len;
> @@ -210,6 +199,16 @@ static const struct i2c_algorithm i2c_opal_algo = {
>   	.functionality	= i2c_opal_func,
>   };
>   
> +/* For two messages, we basically support only simple
> + * smbus transactions of a write plus a read. We might
> + * want to allow also two writes but we'd have to bounce
> + * the data into a single buffer.
> + */
> +static struct i2c_adapter_quirks i2c_opal_quirks = {
> +	.flags = I2C_AQ_COMB_WRITE_THEN_READ,
> +	.max_comb_1st_msg_len = 4,
> +};
> +
>   static int i2c_opal_probe(struct platform_device *pdev)
>   {
>   	struct i2c_adapter	*adapter;
> @@ -232,6 +231,7 @@ static int i2c_opal_probe(struct platform_device *pdev)
>   
>   	adapter->algo = &i2c_opal_algo;
>   	adapter->algo_data = (void *)(unsigned long)opal_id;
> +	adapter->quirks = &i2c_opal_quirks;
>   	adapter->dev.parent = &pdev->dev;
>   	adapter->dev.of_node = of_node_get(pdev->dev.of_node);
>   	pname = of_get_property(pdev->dev.of_node, "ibm,port-name", NULL);

[RFC V2 04/12] i2c: opal: make use of the new infrastructure for quirks

[Benjamin Herrenschmidt]

On Tue, 2015-03-10 at 22:43 +0530, Neelesh Gupta wrote:
> I tested the i2c opal driver after updating the patch as below.
> Basically I think we can also support write-then-{read/write}
> for the number of messages = 2.
> Ben, any issues if we support both write plus read/write in the
> opal driver ?

Nope, in fact it's a good idea, I found myself having to expoes such
an interface to some userspace tool of ours.

However...

> diff --git a/drivers/i2c/busses/i2c-opal.c b/drivers/i2c/busses/i2c-opal.c
> index 16f90b1..85412ba 100644
> --- a/drivers/i2c/busses/i2c-opal.c
> +++ b/drivers/i2c/busses/i2c-opal.c
> @@ -104,18 +104,8 @@ static int i2c_opal_master_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs,
>   		req.buffer_ra = cpu_to_be64(__pa(msgs[0].buf));
>   		break;
>   	case 2:
> -		/* For two messages, we basically support only simple
> -		 * smbus transactions of a write plus a read. We might
> -		 * want to allow also two writes but we'd have to bounce
> -		 * the data into a single buffer.
> -		 */
> -		if ((msgs[0].flags & I2C_M_RD) || !(msgs[1].flags & I2C_M_RD))
> -			return -EOPNOTSUPP;

Don't we still want to enforce that the first message is a write ?
Somebody may not be looking at the quirks...

> -		if (msgs[0].len > 4)
> -			return -EOPNOTSUPP;

And that the len is supported...

> -		if (msgs[0].addr != msgs[1].addr)
> -			return -EOPNOTSUPP;

Same...

Ie, the quirk indicates to the callers what we support, but we should
still check that we aren't called with something that doesn't match.

> -		req.type = OPAL_I2C_SM_READ;
> +		req.type = (msgs[1].flags & I2C_M_RD) ?
> +			OPAL_I2C_SM_READ : OPAL_I2C_SM_WRITE;
>   		req.addr = cpu_to_be16(msgs[0].addr);
>   		req.subaddr_sz = msgs[0].len;
>   		for (i = 0; i < msgs[0].len; i++)
> @@ -210,6 +200,11 @@ static const struct i2c_algorithm i2c_opal_algo = {
>   	.functionality	= i2c_opal_func,
>   };
> 
> +static struct i2c_adapter_quirks i2c_opal_quirks = {
> +	.flags = I2C_AQ_COMB | I2C_AQ_COMB_WRITE_FIRST | I2C_AQ_COMB_SAME_ADDR,
> +	.max_comb_1st_msg_len = 4,
> +};
> +
>   static int i2c_opal_probe(struct platform_device *pdev)
>   {
>   	struct i2c_adapter	*adapter;
> @@ -232,6 +227,7 @@ static int i2c_opal_probe(struct platform_device *pdev)
> 
>   	adapter->algo = &i2c_opal_algo;
>   	adapter->algo_data = (void *)(unsigned long)opal_id;
> +	adapter->quirks = &i2c_opal_quirks;
>   	adapter->dev.parent = &pdev->dev;
>   	adapter->dev.of_node = of_node_get(pdev->dev.of_node);
>   	pname = of_get_property(pdev->dev.of_node, "ibm,port-name", NULL);
> 
> 
> 
> On 02/25/2015 09:31 PM, Wolfram Sang wrote:
> > From: Wolfram Sang <wsa+renesas@sang-engineering.com>
> >
> > Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> > ---
> >   drivers/i2c/busses/i2c-opal.c | 22 +++++++++++-----------
> >   1 file changed, 11 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/i2c/busses/i2c-opal.c b/drivers/i2c/busses/i2c-opal.c
> > index 16f90b1a750894..b2788ecad5b3cb 100644
> > --- a/drivers/i2c/busses/i2c-opal.c
> > +++ b/drivers/i2c/busses/i2c-opal.c
> > @@ -104,17 +104,6 @@ static int i2c_opal_master_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs,
> >   		req.buffer_ra = cpu_to_be64(__pa(msgs[0].buf));
> >   		break;
> >   	case 2:
> > -		/* For two messages, we basically support only simple
> > -		 * smbus transactions of a write plus a read. We might
> > -		 * want to allow also two writes but we'd have to bounce
> > -		 * the data into a single buffer.
> > -		 */
> > -		if ((msgs[0].flags & I2C_M_RD) || !(msgs[1].flags & I2C_M_RD))
> > -			return -EOPNOTSUPP;
> > -		if (msgs[0].len > 4)
> > -			return -EOPNOTSUPP;
> > -		if (msgs[0].addr != msgs[1].addr)
> > -			return -EOPNOTSUPP;
> >   		req.type = OPAL_I2C_SM_READ;
> >   		req.addr = cpu_to_be16(msgs[0].addr);
> >   		req.subaddr_sz = msgs[0].len;
> > @@ -210,6 +199,16 @@ static const struct i2c_algorithm i2c_opal_algo = {
> >   	.functionality	= i2c_opal_func,
> >   };
> >   
> > +/* For two messages, we basically support only simple
> > + * smbus transactions of a write plus a read. We might
> > + * want to allow also two writes but we'd have to bounce
> > + * the data into a single buffer.
> > + */
> > +static struct i2c_adapter_quirks i2c_opal_quirks = {
> > +	.flags = I2C_AQ_COMB_WRITE_THEN_READ,
> > +	.max_comb_1st_msg_len = 4,
> > +};
> > +
> >   static int i2c_opal_probe(struct platform_device *pdev)
> >   {
> >   	struct i2c_adapter	*adapter;
> > @@ -232,6 +231,7 @@ static int i2c_opal_probe(struct platform_device *pdev)
> >   
> >   	adapter->algo = &i2c_opal_algo;
> >   	adapter->algo_data = (void *)(unsigned long)opal_id;
> > +	adapter->quirks = &i2c_opal_quirks;
> >   	adapter->dev.parent = &pdev->dev;
> >   	adapter->dev.of_node = of_node_get(pdev->dev.of_node);
> >   	pname = of_get_property(pdev->dev.of_node, "ibm,port-name", NULL);

[RFC V2 04/12] i2c: opal: make use of the new infrastructure for quirks

[Neelesh Gupta]

On 03/11/2015 04:42 AM, Benjamin Herrenschmidt wrote:
> On Tue, 2015-03-10 at 22:43 +0530, Neelesh Gupta wrote:
>> I tested the i2c opal driver after updating the patch as below.
>> Basically I think we can also support write-then-{read/write}
>> for the number of messages = 2.
>> Ben, any issues if we support both write plus read/write in the
>> opal driver ?
> Nope, in fact it's a good idea, I found myself having to expoes such
> an interface to some userspace tool of ours.
>
> However...
>
>> diff --git a/drivers/i2c/busses/i2c-opal.c b/drivers/i2c/busses/i2c-opal.c
>> index 16f90b1..85412ba 100644
>> --- a/drivers/i2c/busses/i2c-opal.c
>> +++ b/drivers/i2c/busses/i2c-opal.c
>> @@ -104,18 +104,8 @@ static int i2c_opal_master_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs,
>>    		req.buffer_ra = cpu_to_be64(__pa(msgs[0].buf));
>>    		break;
>>    	case 2:
>> -		/* For two messages, we basically support only simple
>> -		 * smbus transactions of a write plus a read. We might
>> -		 * want to allow also two writes but we'd have to bounce
>> -		 * the data into a single buffer.
>> -		 */
>> -		if ((msgs[0].flags & I2C_M_RD) || !(msgs[1].flags & I2C_M_RD))
>> -			return -EOPNOTSUPP;
> Don't we still want to enforce that the first message is a write ?
> Somebody may not be looking at the quirks...
>
>> -		if (msgs[0].len > 4)
>> -			return -EOPNOTSUPP;
> And that the len is supported...
>
>> -		if (msgs[0].addr != msgs[1].addr)
>> -			return -EOPNOTSUPP;
> Same...
>
> Ie, the quirk indicates to the callers what we support, but we should
> still check that we aren't called with something that doesn't match.

Quirk *also* return error to the user if any of the conditions mismatch with
what we have indicated through the quriks structure...

I think we can't land up here by-passing the check for quirks so above 
checks
are duplicated here..

Neelesh.

>
>> -		req.type = OPAL_I2C_SM_READ;
>> +		req.type = (msgs[1].flags & I2C_M_RD) ?
>> +			OPAL_I2C_SM_READ : OPAL_I2C_SM_WRITE;
>>    		req.addr = cpu_to_be16(msgs[0].addr);
>>    		req.subaddr_sz = msgs[0].len;
>>    		for (i = 0; i < msgs[0].len; i++)
>> @@ -210,6 +200,11 @@ static const struct i2c_algorithm i2c_opal_algo = {
>>    	.functionality	= i2c_opal_func,
>>    };
>>
>> +static struct i2c_adapter_quirks i2c_opal_quirks = {
>> +	.flags = I2C_AQ_COMB | I2C_AQ_COMB_WRITE_FIRST | I2C_AQ_COMB_SAME_ADDR,
>> +	.max_comb_1st_msg_len = 4,
>> +};
>> +
>>    static int i2c_opal_probe(struct platform_device *pdev)
>>    {
>>    	struct i2c_adapter	*adapter;
>> @@ -232,6 +227,7 @@ static int i2c_opal_probe(struct platform_device *pdev)
>>
>>    	adapter->algo = &i2c_opal_algo;
>>    	adapter->algo_data = (void *)(unsigned long)opal_id;
>> +	adapter->quirks = &i2c_opal_quirks;
>>    	adapter->dev.parent = &pdev->dev;
>>    	adapter->dev.of_node = of_node_get(pdev->dev.of_node);
>>    	pname = of_get_property(pdev->dev.of_node, "ibm,port-name", NULL);
>>
>>
>>
>> On 02/25/2015 09:31 PM, Wolfram Sang wrote:
>>> From: Wolfram Sang <wsa+renesas@sang-engineering.com>
>>>
>>> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
>>> ---
>>>    drivers/i2c/busses/i2c-opal.c | 22 +++++++++++-----------
>>>    1 file changed, 11 insertions(+), 11 deletions(-)
>>>
>>> diff --git a/drivers/i2c/busses/i2c-opal.c b/drivers/i2c/busses/i2c-opal.c
>>> index 16f90b1a750894..b2788ecad5b3cb 100644
>>> --- a/drivers/i2c/busses/i2c-opal.c
>>> +++ b/drivers/i2c/busses/i2c-opal.c
>>> @@ -104,17 +104,6 @@ static int i2c_opal_master_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs,
>>>    		req.buffer_ra = cpu_to_be64(__pa(msgs[0].buf));
>>>    		break;
>>>    	case 2:
>>> -		/* For two messages, we basically support only simple
>>> -		 * smbus transactions of a write plus a read. We might
>>> -		 * want to allow also two writes but we'd have to bounce
>>> -		 * the data into a single buffer.
>>> -		 */
>>> -		if ((msgs[0].flags & I2C_M_RD) || !(msgs[1].flags & I2C_M_RD))
>>> -			return -EOPNOTSUPP;
>>> -		if (msgs[0].len > 4)
>>> -			return -EOPNOTSUPP;
>>> -		if (msgs[0].addr != msgs[1].addr)
>>> -			return -EOPNOTSUPP;
>>>    		req.type = OPAL_I2C_SM_READ;
>>>    		req.addr = cpu_to_be16(msgs[0].addr);
>>>    		req.subaddr_sz = msgs[0].len;
>>> @@ -210,6 +199,16 @@ static const struct i2c_algorithm i2c_opal_algo = {
>>>    	.functionality	= i2c_opal_func,
>>>    };
>>>    
>>> +/* For two messages, we basically support only simple
>>> + * smbus transactions of a write plus a read. We might
>>> + * want to allow also two writes but we'd have to bounce
>>> + * the data into a single buffer.
>>> + */
>>> +static struct i2c_adapter_quirks i2c_opal_quirks = {
>>> +	.flags = I2C_AQ_COMB_WRITE_THEN_READ,
>>> +	.max_comb_1st_msg_len = 4,
>>> +};
>>> +
>>>    static int i2c_opal_probe(struct platform_device *pdev)
>>>    {
>>>    	struct i2c_adapter	*adapter;
>>> @@ -232,6 +231,7 @@ static int i2c_opal_probe(struct platform_device *pdev)
>>>    
>>>    	adapter->algo = &i2c_opal_algo;
>>>    	adapter->algo_data = (void *)(unsigned long)opal_id;
>>> +	adapter->quirks = &i2c_opal_quirks;
>>>    	adapter->dev.parent = &pdev->dev;
>>>    	adapter->dev.of_node = of_node_get(pdev->dev.of_node);
>>>    	pname = of_get_property(pdev->dev.of_node, "ibm,port-name", NULL);
>

[RFC V2 03/12] i2c: at91: make use of the new infrastructure for quirks

[Wolfram Sang]

> You can add my 
> 
> Acked-by and Tested-By: Ludovic Desroches <ludovic.desroches@atmel.com>
> 
> Tested on sama5d3, some problems with at24 eeprom on sama5d4 but it
> doesn't come from the i2c quirks patch series.

Thanks for testing! Are the eeprom problems something which needs fixing
upstream?

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150312/89ca0cda/attachment.sig>

[RFC V2 04/12] i2c: opal: make use of the new infrastructure for quirks

[Wolfram Sang]

> I think we can't land up here by-passing the check for quirks so above
> checks are duplicated here..

True.

So, as Ben seems OK with write-then-anything, can you send me your
changes as an incremental patch to mine with your Signed-off, please?

Thanks,

   Wolfram

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150312/0817d7c0/attachment-0001.sig>

[RFC V2 12/12] i2c: bcm-iproc: make use of the new infrastructure for quirks

[Wolfram Sang]

> Change on the iproc i2c driver looks good to me. Sanity tested the
> change from Wolfram's i2c/quirks branch on Cygnus 958300K combo board.
> Sanity tested with an attempt to transfer large amount of I2C data to
> ensure the transfer is denied by the i2c-core:
> 
> / # cat /dev/i2c-0
> [  657.310261] i2c i2c-0: quirk: msg too long (addr 0x0000, size 4096, read)
> 
> Reviewed-by: Ray Jui <rjui@broadcom.com>
> Tested-by: Ray Jui <rjui@broadcom.com>

Thanks for testing, and especially describing your test! Much
appreciated.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150312/3063923f/attachment.sig>

[RFC V2 00/12] i2c: describe adapter quirks in a generic way

[Wolfram Sang]

> For QUP driver. 
> 
> Reviewed-by: Ivan T. Ivanov <iivanov@mm-sol.com>
> Tested-by: Ivan T. Ivanov <iivanov@mm-sol.com>

Great, thanks for testing!

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150312/6907a6bf/attachment.sig>

[RFC V2 00/12] i2c: describe adapter quirks in a generic way

[Wolfram Sang]

On Wed, Feb 25, 2015 at 05:01:51PM +0100, Wolfram Sang wrote:
> From: Wolfram Sang <wsa+renesas@sang-engineering.com>
> 
> Here is the second version of the patch series to describe i2c adapter quirks
> in a generic way. For the motivation, please read description of patch 1. This
> is still RFC because I would like to do some more tests on my own, but I need
> to write a tool for that. However, I'd really like to have the driver authors
> to have a look already. Actual testing is very much appreciated. Thanks to the
> Mediatek guys for rebasing their new driver to this framework. That helps, too!

Thanks for all the testing. Merged this branch now into for-next!

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150314/f24f3abe/attachment.sig>