linux-mips.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Add I2C support for the Ingenic X1000 SoC v2.
@ 2019-12-16 10:06 周琰杰 (Zhou Yanjie)
  2019-12-16 10:06 ` [PATCH v2 0/2] Add I2C support for the Ingenic X1000 SoC 周琰杰 (Zhou Yanjie)
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: 周琰杰 (Zhou Yanjie) @ 2019-12-16 10:06 UTC (permalink / raw)
  To: linux-mips
  Cc: linux-kernel, linux-i2c, devicetree, robh+dt, paul.burton,
	paulburton, mark.rutland, paul, sernia.zhou, zhenwenjin,
	2374286503

v1->v2:
Add code to check device_get_match_data(), if it return a NULL ptr,
then print an error message and return -ENODEV.


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

* [PATCH v2 0/2] Add I2C support for the Ingenic X1000 SoC
  2019-12-16 10:06 Add I2C support for the Ingenic X1000 SoC v2 周琰杰 (Zhou Yanjie)
@ 2019-12-16 10:06 ` 周琰杰 (Zhou Yanjie)
  2019-12-16 10:06 ` [PATCH v2 1/2] dt-bindings: I2C: Add X1000 bindings 周琰杰 (Zhou Yanjie)
  2019-12-16 10:06 ` [PATCH v2 2/2] I2C: JZ4780: Add support for the X1000 周琰杰 (Zhou Yanjie)
  2 siblings, 0 replies; 6+ messages in thread
From: 周琰杰 (Zhou Yanjie) @ 2019-12-16 10:06 UTC (permalink / raw)
  To: linux-mips
  Cc: linux-kernel, linux-i2c, devicetree, robh+dt, paul.burton,
	paulburton, mark.rutland, paul, sernia.zhou, zhenwenjin,
	2374286503

v1->v2:
Add code to check device_get_match_data(), if it return a NULL ptr,
then print an error message and return -ENODEV.

周琰杰 (Zhou Yanjie) (2):
  dt-bindings: I2C: Add X1000 bindings.
  I2C: JZ4780: Add support for the X1000.

 .../devicetree/bindings/i2c/i2c-jz4780.txt         |   4 +-
 drivers/i2c/busses/i2c-jz4780.c                    | 155 +++++++++++++++------
 2 files changed, 118 insertions(+), 41 deletions(-)

-- 
2.7.4


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

* [PATCH v2 1/2] dt-bindings: I2C: Add X1000 bindings.
  2019-12-16 10:06 Add I2C support for the Ingenic X1000 SoC v2 周琰杰 (Zhou Yanjie)
  2019-12-16 10:06 ` [PATCH v2 0/2] Add I2C support for the Ingenic X1000 SoC 周琰杰 (Zhou Yanjie)
@ 2019-12-16 10:06 ` 周琰杰 (Zhou Yanjie)
  2019-12-16 10:06 ` [PATCH v2 2/2] I2C: JZ4780: Add support for the X1000 周琰杰 (Zhou Yanjie)
  2 siblings, 0 replies; 6+ messages in thread
From: 周琰杰 (Zhou Yanjie) @ 2019-12-16 10:06 UTC (permalink / raw)
  To: linux-mips
  Cc: linux-kernel, linux-i2c, devicetree, robh+dt, paul.burton,
	paulburton, mark.rutland, paul, sernia.zhou, zhenwenjin,
	2374286503

Add the I2C bindings for the X1000 Soc from Ingenic.

Signed-off-by: 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com>
---

Notes:
    v1->v2:
    No change.

 Documentation/devicetree/bindings/i2c/i2c-jz4780.txt | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/i2c/i2c-jz4780.txt b/Documentation/devicetree/bindings/i2c/i2c-jz4780.txt
index 3738cfb..d229eff 100644
--- a/Documentation/devicetree/bindings/i2c/i2c-jz4780.txt
+++ b/Documentation/devicetree/bindings/i2c/i2c-jz4780.txt
@@ -1,7 +1,9 @@
 * Ingenic JZ4780 I2C Bus controller
 
 Required properties:
-- compatible: should be "ingenic,jz4780-i2c"
+- compatible: should be one of the following:
+  - "ingenic,jz4780-i2c" for the JZ4780
+  - "ingenic,x1000-i2c" for the X1000
 - reg: Should contain the address & size of the I2C controller registers.
 - interrupts: Should specify the interrupt provided by parent.
 - clocks: Should contain a single clock specifier for the JZ4780 I2C clock.
-- 
2.7.4


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

* [PATCH v2 2/2] I2C: JZ4780: Add support for the X1000.
  2019-12-16 10:06 Add I2C support for the Ingenic X1000 SoC v2 周琰杰 (Zhou Yanjie)
  2019-12-16 10:06 ` [PATCH v2 0/2] Add I2C support for the Ingenic X1000 SoC 周琰杰 (Zhou Yanjie)
  2019-12-16 10:06 ` [PATCH v2 1/2] dt-bindings: I2C: Add X1000 bindings 周琰杰 (Zhou Yanjie)
@ 2019-12-16 10:06 ` 周琰杰 (Zhou Yanjie)
  2019-12-16 13:57   ` Paul Cercueil
  2 siblings, 1 reply; 6+ messages in thread
From: 周琰杰 (Zhou Yanjie) @ 2019-12-16 10:06 UTC (permalink / raw)
  To: linux-mips
  Cc: linux-kernel, linux-i2c, devicetree, robh+dt, paul.burton,
	paulburton, mark.rutland, paul, sernia.zhou, zhenwenjin,
	2374286503

Add support for probing i2c driver on the X1000 Soc from Ingenic.
call the corresponding fifo parameter according to the device
model obtained from the devicetree.

Signed-off-by: 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com>
---

Notes:
    v1->v2:
    Add code to check device_get_match_data(), if it return a NULL ptr,
    then print an error message and return -ENODEV.

 drivers/i2c/busses/i2c-jz4780.c | 155 +++++++++++++++++++++++++++++-----------
 1 file changed, 115 insertions(+), 40 deletions(-)

diff --git a/drivers/i2c/busses/i2c-jz4780.c b/drivers/i2c/busses/i2c-jz4780.c
index 25dcd73..f07a07c 100644
--- a/drivers/i2c/busses/i2c-jz4780.c
+++ b/drivers/i2c/busses/i2c-jz4780.c
@@ -4,6 +4,7 @@
  *
  * Copyright (C) 2006 - 2009 Ingenic Semiconductor Inc.
  * Copyright (C) 2015 Imagination Technologies
+ * Copyright (C) 2019 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com>
  */
 
 #include <linux/bitops.h>
@@ -17,6 +18,7 @@
 #include <linux/io.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
+#include <linux/of_device.h>
 #include <linux/platform_device.h>
 #include <linux/sched.h>
 #include <linux/slab.h>
@@ -55,6 +57,7 @@
 #define JZ4780_I2C_ACKGC	0x98
 #define JZ4780_I2C_ENSTA	0x9C
 #define JZ4780_I2C_SDAHD	0xD0
+#define X1000_I2C_SDAHD		0x7C
 
 #define JZ4780_I2C_CTRL_STPHLD		BIT(7)
 #define JZ4780_I2C_CTRL_SLVDIS		BIT(6)
@@ -73,6 +76,8 @@
 #define JZ4780_I2C_STA_TFNF		BIT(1)
 #define JZ4780_I2C_STA_ACT		BIT(0)
 
+#define X1000_I2C_DC_STOP		BIT(9)
+
 static const char * const jz4780_i2c_abrt_src[] = {
 	"ABRT_7B_ADDR_NOACK",
 	"ABRT_10ADDR1_NOACK",
@@ -130,18 +135,33 @@ static const char * const jz4780_i2c_abrt_src[] = {
 #define JZ4780_I2CFLCNT_ADJUST(n)	(((n) - 1) < 8 ? 8 : ((n) - 1))
 
 #define JZ4780_I2C_FIFO_LEN	16
-#define TX_LEVEL		3
-#define RX_LEVEL		(JZ4780_I2C_FIFO_LEN - TX_LEVEL - 1)
+
+#define X1000_I2C_FIFO_LEN	64
 
 #define JZ4780_I2C_TIMEOUT	300
 
 #define BUFSIZE 200
 
+enum ingenic_i2c_version {
+	ID_JZ4780,
+	ID_X1000,
+};
+
+/* ingenic_i2c_config: SoC specific config data. */
+struct ingenic_i2c_config {
+	enum ingenic_i2c_version version;
+
+	int fifosize;
+	int tx_level;
+	int rx_level;
+};
+
 struct jz4780_i2c {
 	void __iomem		*iomem;
 	int			 irq;
 	struct clk		*clk;
 	struct i2c_adapter	 adap;
+	const struct ingenic_i2c_config *cdata;
 
 	/* lock to protect rbuf and wbuf between xfer_rd/wr and irq handler */
 	spinlock_t		lock;
@@ -340,11 +360,18 @@ static int jz4780_i2c_set_speed(struct jz4780_i2c *i2c)
 
 	if (hold_time >= 0) {
 		/*i2c hold time enable */
-		hold_time |= JZ4780_I2C_SDAHD_HDENB;
-		jz4780_i2c_writew(i2c, JZ4780_I2C_SDAHD, hold_time);
+		if (i2c->cdata->version >= ID_X1000)
+			jz4780_i2c_writew(i2c, X1000_I2C_SDAHD, hold_time);
+		else {
+			hold_time |= JZ4780_I2C_SDAHD_HDENB;
+			jz4780_i2c_writew(i2c, JZ4780_I2C_SDAHD, hold_time);
+		}
 	} else {
 		/* disable hold time */
-		jz4780_i2c_writew(i2c, JZ4780_I2C_SDAHD, 0);
+		if (i2c->cdata->version >= ID_X1000)
+			jz4780_i2c_writew(i2c, X1000_I2C_SDAHD, 0);
+		else
+			jz4780_i2c_writew(i2c, JZ4780_I2C_SDAHD, 0);
 	}
 
 	return 0;
@@ -359,9 +386,11 @@ static int jz4780_i2c_cleanup(struct jz4780_i2c *i2c)
 	spin_lock_irqsave(&i2c->lock, flags);
 
 	/* can send stop now if need */
-	tmp = jz4780_i2c_readw(i2c, JZ4780_I2C_CTRL);
-	tmp &= ~JZ4780_I2C_CTRL_STPHLD;
-	jz4780_i2c_writew(i2c, JZ4780_I2C_CTRL, tmp);
+	if (i2c->cdata->version < ID_X1000) {
+		tmp = jz4780_i2c_readw(i2c, JZ4780_I2C_CTRL);
+		tmp &= ~JZ4780_I2C_CTRL_STPHLD;
+		jz4780_i2c_writew(i2c, JZ4780_I2C_CTRL, tmp);
+	}
 
 	/* disable all interrupts first */
 	jz4780_i2c_writew(i2c, JZ4780_I2C_INTM, 0);
@@ -399,11 +428,18 @@ static int jz4780_i2c_prepare(struct jz4780_i2c *i2c)
 	return jz4780_i2c_enable(i2c);
 }
 
-static void jz4780_i2c_send_rcmd(struct jz4780_i2c *i2c, int cmd_count)
+static void jz4780_i2c_send_rcmd(struct jz4780_i2c *i2c,
+				       int cmd_count, int cmd_left)
 {
 	int i;
 
-	for (i = 0; i < cmd_count; i++)
+	for (i = 0; i < cmd_count - 1; i++)
+		jz4780_i2c_writew(i2c, JZ4780_I2C_DC, JZ4780_I2C_DC_READ);
+
+	if ((cmd_left == 0) && (i2c->cdata->version >= ID_X1000))
+		jz4780_i2c_writew(i2c, JZ4780_I2C_DC,
+				JZ4780_I2C_DC_READ | X1000_I2C_DC_STOP);
+	else
 		jz4780_i2c_writew(i2c, JZ4780_I2C_DC, JZ4780_I2C_DC_READ);
 }
 
@@ -458,37 +494,44 @@ static irqreturn_t jz4780_i2c_irq(int irqno, void *dev_id)
 
 		rd_left = i2c->rd_total_len - i2c->rd_data_xfered;
 
-		if (rd_left <= JZ4780_I2C_FIFO_LEN)
+		if (rd_left <= i2c->cdata->fifosize)
 			jz4780_i2c_writew(i2c, JZ4780_I2C_RXTL, rd_left - 1);
 	}
 
 	if (intst & JZ4780_I2C_INTST_TXEMP) {
 		if (i2c->is_write == 0) {
 			int cmd_left = i2c->rd_total_len - i2c->rd_cmd_xfered;
-			int max_send = (JZ4780_I2C_FIFO_LEN - 1)
+			int max_send = (i2c->cdata->fifosize - 1)
 					 - (i2c->rd_cmd_xfered
 					 - i2c->rd_data_xfered);
 			int cmd_to_send = min(cmd_left, max_send);
 
 			if (i2c->rd_cmd_xfered != 0)
 				cmd_to_send = min(cmd_to_send,
-						  JZ4780_I2C_FIFO_LEN
-						  - TX_LEVEL - 1);
+						  i2c->cdata->fifosize
+						  - i2c->cdata->tx_level - 1);
 
 			if (cmd_to_send) {
-				jz4780_i2c_send_rcmd(i2c, cmd_to_send);
 				i2c->rd_cmd_xfered += cmd_to_send;
+				cmd_left = i2c->rd_total_len -
+						i2c->rd_cmd_xfered;
+				jz4780_i2c_send_rcmd(i2c,
+						cmd_to_send, cmd_left);
+
 			}
 
-			cmd_left = i2c->rd_total_len - i2c->rd_cmd_xfered;
 			if (cmd_left == 0) {
 				intmsk = jz4780_i2c_readw(i2c, JZ4780_I2C_INTM);
 				intmsk &= ~JZ4780_I2C_INTM_MTXEMP;
 				jz4780_i2c_writew(i2c, JZ4780_I2C_INTM, intmsk);
 
-				tmp = jz4780_i2c_readw(i2c, JZ4780_I2C_CTRL);
-				tmp &= ~JZ4780_I2C_CTRL_STPHLD;
-				jz4780_i2c_writew(i2c, JZ4780_I2C_CTRL, tmp);
+				if (i2c->cdata->version < ID_X1000) {
+					tmp = jz4780_i2c_readw(i2c,
+							JZ4780_I2C_CTRL);
+					tmp &= ~JZ4780_I2C_CTRL_STPHLD;
+					jz4780_i2c_writew(i2c,
+							JZ4780_I2C_CTRL, tmp);
+				}
 			}
 		} else {
 			unsigned short data;
@@ -497,23 +540,26 @@ static irqreturn_t jz4780_i2c_irq(int irqno, void *dev_id)
 			i2c_sta = jz4780_i2c_readw(i2c, JZ4780_I2C_STA);
 
 			while ((i2c_sta & JZ4780_I2C_STA_TFNF) &&
-			       (i2c->wt_len > 0)) {
+					(i2c->wt_len > 0)) {
 				i2c_sta = jz4780_i2c_readw(i2c, JZ4780_I2C_STA);
 				data = *i2c->wbuf;
 				data &= ~JZ4780_I2C_DC_READ;
-				jz4780_i2c_writew(i2c, JZ4780_I2C_DC,
-						  data);
+				if ((!i2c->stop_hold) && (i2c->cdata->version >=
+						ID_X1000))
+					data |= X1000_I2C_DC_STOP;
+				jz4780_i2c_writew(i2c, JZ4780_I2C_DC, data);
 				i2c->wbuf++;
 				i2c->wt_len--;
 			}
 
 			if (i2c->wt_len == 0) {
-				if (!i2c->stop_hold) {
+				if ((!i2c->stop_hold) && (i2c->cdata->version <
+						ID_X1000)) {
 					tmp = jz4780_i2c_readw(i2c,
-							       JZ4780_I2C_CTRL);
+							JZ4780_I2C_CTRL);
 					tmp &= ~JZ4780_I2C_CTRL_STPHLD;
-					jz4780_i2c_writew(i2c, JZ4780_I2C_CTRL,
-							  tmp);
+					jz4780_i2c_writew(i2c,
+							JZ4780_I2C_CTRL, tmp);
 				}
 
 				jz4780_i2c_trans_done(i2c);
@@ -567,20 +613,22 @@ static inline int jz4780_i2c_xfer_read(struct jz4780_i2c *i2c,
 	i2c->rd_data_xfered = 0;
 	i2c->rd_cmd_xfered = 0;
 
-	if (len <= JZ4780_I2C_FIFO_LEN)
+	if (len <= i2c->cdata->fifosize)
 		jz4780_i2c_writew(i2c, JZ4780_I2C_RXTL, len - 1);
 	else
-		jz4780_i2c_writew(i2c, JZ4780_I2C_RXTL, RX_LEVEL);
+		jz4780_i2c_writew(i2c, JZ4780_I2C_RXTL, i2c->cdata->rx_level);
 
-	jz4780_i2c_writew(i2c, JZ4780_I2C_TXTL, TX_LEVEL);
+	jz4780_i2c_writew(i2c, JZ4780_I2C_TXTL, i2c->cdata->tx_level);
 
 	jz4780_i2c_writew(i2c, JZ4780_I2C_INTM,
 			  JZ4780_I2C_INTM_MRXFL | JZ4780_I2C_INTM_MTXEMP
 			  | JZ4780_I2C_INTM_MTXABT | JZ4780_I2C_INTM_MRXOF);
 
-	tmp = jz4780_i2c_readw(i2c, JZ4780_I2C_CTRL);
-	tmp |= JZ4780_I2C_CTRL_STPHLD;
-	jz4780_i2c_writew(i2c, JZ4780_I2C_CTRL, tmp);
+	if (i2c->cdata->version < ID_X1000) {
+		tmp = jz4780_i2c_readw(i2c, JZ4780_I2C_CTRL);
+		tmp |= JZ4780_I2C_CTRL_STPHLD;
+		jz4780_i2c_writew(i2c, JZ4780_I2C_CTRL, tmp);
+	}
 
 	spin_unlock_irqrestore(&i2c->lock, flags);
 
@@ -626,14 +674,16 @@ static inline int jz4780_i2c_xfer_write(struct jz4780_i2c *i2c,
 	i2c->wbuf = buf;
 	i2c->wt_len = len;
 
-	jz4780_i2c_writew(i2c, JZ4780_I2C_TXTL, TX_LEVEL);
+	jz4780_i2c_writew(i2c, JZ4780_I2C_TXTL, i2c->cdata->tx_level);
 
 	jz4780_i2c_writew(i2c, JZ4780_I2C_INTM, JZ4780_I2C_INTM_MTXEMP
 					| JZ4780_I2C_INTM_MTXABT);
 
-	tmp = jz4780_i2c_readw(i2c, JZ4780_I2C_CTRL);
-	tmp |= JZ4780_I2C_CTRL_STPHLD;
-	jz4780_i2c_writew(i2c, JZ4780_I2C_CTRL, tmp);
+	if (i2c->cdata->version < ID_X1000) {
+		tmp = jz4780_i2c_readw(i2c, JZ4780_I2C_CTRL);
+		tmp |= JZ4780_I2C_CTRL_STPHLD;
+		jz4780_i2c_writew(i2c, JZ4780_I2C_CTRL, tmp);
+	}
 
 	spin_unlock_irqrestore(&i2c->lock, flags);
 
@@ -716,8 +766,25 @@ static const struct i2c_algorithm jz4780_i2c_algorithm = {
 	.functionality	= jz4780_i2c_functionality,
 };
 
+static const struct ingenic_i2c_config jz4780_i2c_config = {
+	.version = ID_JZ4780,
+
+	.fifosize = JZ4780_I2C_FIFO_LEN,
+	.tx_level = JZ4780_I2C_FIFO_LEN / 2,
+	.rx_level = JZ4780_I2C_FIFO_LEN / 2 - 1,
+};
+
+static const struct ingenic_i2c_config x1000_i2c_config = {
+	.version = ID_X1000,
+
+	.fifosize = X1000_I2C_FIFO_LEN,
+	.tx_level = X1000_I2C_FIFO_LEN / 2,
+	.rx_level = X1000_I2C_FIFO_LEN / 2 - 1,
+};
+
 static const struct of_device_id jz4780_i2c_of_matches[] = {
-	{ .compatible = "ingenic,jz4780-i2c", },
+	{ .compatible = "ingenic,jz4780-i2c", .data = &jz4780_i2c_config },
+	{ .compatible = "ingenic,x1000-i2c", .data = &x1000_i2c_config },
 	{ /* sentinel */ }
 };
 MODULE_DEVICE_TABLE(of, jz4780_i2c_of_matches);
@@ -734,6 +801,12 @@ static int jz4780_i2c_probe(struct platform_device *pdev)
 	if (!i2c)
 		return -ENOMEM;
 
+	i2c->cdata = device_get_match_data(&pdev->dev);
+	if (!i2c->cdata) {
+		dev_err(&pdev->dev, "Error: No device match found\n");
+		return -ENODEV;
+	}
+
 	i2c->adap.owner		= THIS_MODULE;
 	i2c->adap.algo		= &jz4780_i2c_algorithm;
 	i2c->adap.algo_data	= i2c;
@@ -777,9 +850,11 @@ static int jz4780_i2c_probe(struct platform_device *pdev)
 
 	dev_info(&pdev->dev, "Bus frequency is %d KHz\n", i2c->speed);
 
-	tmp = jz4780_i2c_readw(i2c, JZ4780_I2C_CTRL);
-	tmp &= ~JZ4780_I2C_CTRL_STPHLD;
-	jz4780_i2c_writew(i2c, JZ4780_I2C_CTRL, tmp);
+	if (i2c->cdata->version < ID_X1000) {
+		tmp = jz4780_i2c_readw(i2c, JZ4780_I2C_CTRL);
+		tmp &= ~JZ4780_I2C_CTRL_STPHLD;
+		jz4780_i2c_writew(i2c, JZ4780_I2C_CTRL, tmp);
+	}
 
 	jz4780_i2c_writew(i2c, JZ4780_I2C_INTM, 0x0);
 
-- 
2.7.4


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

* Re: [PATCH v2 2/2] I2C: JZ4780: Add support for the X1000.
  2019-12-16 10:06 ` [PATCH v2 2/2] I2C: JZ4780: Add support for the X1000 周琰杰 (Zhou Yanjie)
@ 2019-12-16 13:57   ` Paul Cercueil
  2019-12-17  7:50     ` Zhou Yanjie
  0 siblings, 1 reply; 6+ messages in thread
From: Paul Cercueil @ 2019-12-16 13:57 UTC (permalink / raw)
  To: 周琰杰 (Zhou Yanjie)
  Cc: linux-mips, linux-kernel, linux-i2c, devicetree, robh+dt,
	paul.burton, paulburton, mark.rutland, sernia.zhou, zhenwenjin,
	2374286503

Hi Zhou,


Le lun., déc. 16, 2019 at 18:06, 周琰杰 (Zhou Yanjie) 
<zhouyanjie@wanyeetech.com> a écrit :
> Add support for probing i2c driver on the X1000 Soc from Ingenic.
> call the corresponding fifo parameter according to the device
> model obtained from the devicetree.
> 
> Signed-off-by: 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com>
> ---
> 
> Notes:
>     v1->v2:
>     Add code to check device_get_match_data(), if it return a NULL 
> ptr,
>     then print an error message and return -ENODEV.
> 
>  drivers/i2c/busses/i2c-jz4780.c | 155 
> +++++++++++++++++++++++++++++-----------
>  1 file changed, 115 insertions(+), 40 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-jz4780.c 
> b/drivers/i2c/busses/i2c-jz4780.c
> index 25dcd73..f07a07c 100644
> --- a/drivers/i2c/busses/i2c-jz4780.c
> +++ b/drivers/i2c/busses/i2c-jz4780.c
> @@ -4,6 +4,7 @@
>   *
>   * Copyright (C) 2006 - 2009 Ingenic Semiconductor Inc.
>   * Copyright (C) 2015 Imagination Technologies
> + * Copyright (C) 2019 周琰杰 (Zhou Yanjie) 
> <zhouyanjie@wanyeetech.com>
>   */
> 
>  #include <linux/bitops.h>
> @@ -17,6 +18,7 @@
>  #include <linux/io.h>
>  #include <linux/kernel.h>
>  #include <linux/module.h>
> +#include <linux/of_device.h>
>  #include <linux/platform_device.h>
>  #include <linux/sched.h>
>  #include <linux/slab.h>
> @@ -55,6 +57,7 @@
>  #define JZ4780_I2C_ACKGC	0x98
>  #define JZ4780_I2C_ENSTA	0x9C
>  #define JZ4780_I2C_SDAHD	0xD0
> +#define X1000_I2C_SDAHD		0x7C
> 
>  #define JZ4780_I2C_CTRL_STPHLD		BIT(7)
>  #define JZ4780_I2C_CTRL_SLVDIS		BIT(6)
> @@ -73,6 +76,8 @@
>  #define JZ4780_I2C_STA_TFNF		BIT(1)
>  #define JZ4780_I2C_STA_ACT		BIT(0)
> 
> +#define X1000_I2C_DC_STOP		BIT(9)
> +
>  static const char * const jz4780_i2c_abrt_src[] = {
>  	"ABRT_7B_ADDR_NOACK",
>  	"ABRT_10ADDR1_NOACK",
> @@ -130,18 +135,33 @@ static const char * const jz4780_i2c_abrt_src[] 
> = {
>  #define JZ4780_I2CFLCNT_ADJUST(n)	(((n) - 1) < 8 ? 8 : ((n) - 1))
> 
>  #define JZ4780_I2C_FIFO_LEN	16
> -#define TX_LEVEL		3
> -#define RX_LEVEL		(JZ4780_I2C_FIFO_LEN - TX_LEVEL - 1)
> +
> +#define X1000_I2C_FIFO_LEN	64
> 
>  #define JZ4780_I2C_TIMEOUT	300
> 
>  #define BUFSIZE 200
> 
> +enum ingenic_i2c_version {
> +	ID_JZ4780,
> +	ID_X1000,
> +};
> +
> +/* ingenic_i2c_config: SoC specific config data. */
> +struct ingenic_i2c_config {
> +	enum ingenic_i2c_version version;
> +
> +	int fifosize;
> +	int tx_level;
> +	int rx_level;
> +};
> +
>  struct jz4780_i2c {
>  	void __iomem		*iomem;
>  	int			 irq;
>  	struct clk		*clk;
>  	struct i2c_adapter	 adap;
> +	const struct ingenic_i2c_config *cdata;
> 
>  	/* lock to protect rbuf and wbuf between xfer_rd/wr and irq handler 
> */
>  	spinlock_t		lock;
> @@ -340,11 +360,18 @@ static int jz4780_i2c_set_speed(struct 
> jz4780_i2c *i2c)
> 
>  	if (hold_time >= 0) {
>  		/*i2c hold time enable */
> -		hold_time |= JZ4780_I2C_SDAHD_HDENB;
> -		jz4780_i2c_writew(i2c, JZ4780_I2C_SDAHD, hold_time);
> +		if (i2c->cdata->version >= ID_X1000)
> +			jz4780_i2c_writew(i2c, X1000_I2C_SDAHD, hold_time);
> +		else {

If only one branch of a conditional statement is a single statement, 
then you should use braces in both branches.

See: 
https://www.kernel.org/doc/html/v4.10/process/coding-style.html#placing-braces-and-spaces


> +			hold_time |= JZ4780_I2C_SDAHD_HDENB;
> +			jz4780_i2c_writew(i2c, JZ4780_I2C_SDAHD, hold_time);
> +		}
>  	} else {
>  		/* disable hold time */
> -		jz4780_i2c_writew(i2c, JZ4780_I2C_SDAHD, 0);
> +		if (i2c->cdata->version >= ID_X1000)
> +			jz4780_i2c_writew(i2c, X1000_I2C_SDAHD, 0);
> +		else
> +			jz4780_i2c_writew(i2c, JZ4780_I2C_SDAHD, 0);
>  	}
> 
>  	return 0;
> @@ -359,9 +386,11 @@ static int jz4780_i2c_cleanup(struct jz4780_i2c 
> *i2c)
>  	spin_lock_irqsave(&i2c->lock, flags);
> 
>  	/* can send stop now if need */
> -	tmp = jz4780_i2c_readw(i2c, JZ4780_I2C_CTRL);
> -	tmp &= ~JZ4780_I2C_CTRL_STPHLD;
> -	jz4780_i2c_writew(i2c, JZ4780_I2C_CTRL, tmp);
> +	if (i2c->cdata->version < ID_X1000) {
> +		tmp = jz4780_i2c_readw(i2c, JZ4780_I2C_CTRL);
> +		tmp &= ~JZ4780_I2C_CTRL_STPHLD;
> +		jz4780_i2c_writew(i2c, JZ4780_I2C_CTRL, tmp);
> +	}
> 
>  	/* disable all interrupts first */
>  	jz4780_i2c_writew(i2c, JZ4780_I2C_INTM, 0);
> @@ -399,11 +428,18 @@ static int jz4780_i2c_prepare(struct jz4780_i2c 
> *i2c)
>  	return jz4780_i2c_enable(i2c);
>  }
> 
> -static void jz4780_i2c_send_rcmd(struct jz4780_i2c *i2c, int 
> cmd_count)
> +static void jz4780_i2c_send_rcmd(struct jz4780_i2c *i2c,
> +				       int cmd_count, int cmd_left)

Sorry to be pedantic ;) but this line is not properly indented. You 
should indent with tab charaters (configure your IDE for one tab == 4 
spaces) as much as possible, then use spaces to align the first word.

With these two things fixed:
Acked-by: Paul Cercueil <paul@crapouillou.net>

Cheers,
-Paul


>  {
>  	int i;
> 
> -	for (i = 0; i < cmd_count; i++)
> +	for (i = 0; i < cmd_count - 1; i++)
> +		jz4780_i2c_writew(i2c, JZ4780_I2C_DC, JZ4780_I2C_DC_READ);
> +
> +	if ((cmd_left == 0) && (i2c->cdata->version >= ID_X1000))
> +		jz4780_i2c_writew(i2c, JZ4780_I2C_DC,
> +				JZ4780_I2C_DC_READ | X1000_I2C_DC_STOP);
> +	else
>  		jz4780_i2c_writew(i2c, JZ4780_I2C_DC, JZ4780_I2C_DC_READ);
>  }
> 
> @@ -458,37 +494,44 @@ static irqreturn_t jz4780_i2c_irq(int irqno, 
> void *dev_id)
> 
>  		rd_left = i2c->rd_total_len - i2c->rd_data_xfered;
> 
> -		if (rd_left <= JZ4780_I2C_FIFO_LEN)
> +		if (rd_left <= i2c->cdata->fifosize)
>  			jz4780_i2c_writew(i2c, JZ4780_I2C_RXTL, rd_left - 1);
>  	}
> 
>  	if (intst & JZ4780_I2C_INTST_TXEMP) {
>  		if (i2c->is_write == 0) {
>  			int cmd_left = i2c->rd_total_len - i2c->rd_cmd_xfered;
> -			int max_send = (JZ4780_I2C_FIFO_LEN - 1)
> +			int max_send = (i2c->cdata->fifosize - 1)
>  					 - (i2c->rd_cmd_xfered
>  					 - i2c->rd_data_xfered);
>  			int cmd_to_send = min(cmd_left, max_send);
> 
>  			if (i2c->rd_cmd_xfered != 0)
>  				cmd_to_send = min(cmd_to_send,
> -						  JZ4780_I2C_FIFO_LEN
> -						  - TX_LEVEL - 1);
> +						  i2c->cdata->fifosize
> +						  - i2c->cdata->tx_level - 1);
> 
>  			if (cmd_to_send) {
> -				jz4780_i2c_send_rcmd(i2c, cmd_to_send);
>  				i2c->rd_cmd_xfered += cmd_to_send;
> +				cmd_left = i2c->rd_total_len -
> +						i2c->rd_cmd_xfered;
> +				jz4780_i2c_send_rcmd(i2c,
> +						cmd_to_send, cmd_left);
> +
>  			}
> 
> -			cmd_left = i2c->rd_total_len - i2c->rd_cmd_xfered;
>  			if (cmd_left == 0) {
>  				intmsk = jz4780_i2c_readw(i2c, JZ4780_I2C_INTM);
>  				intmsk &= ~JZ4780_I2C_INTM_MTXEMP;
>  				jz4780_i2c_writew(i2c, JZ4780_I2C_INTM, intmsk);
> 
> -				tmp = jz4780_i2c_readw(i2c, JZ4780_I2C_CTRL);
> -				tmp &= ~JZ4780_I2C_CTRL_STPHLD;
> -				jz4780_i2c_writew(i2c, JZ4780_I2C_CTRL, tmp);
> +				if (i2c->cdata->version < ID_X1000) {
> +					tmp = jz4780_i2c_readw(i2c,
> +							JZ4780_I2C_CTRL);
> +					tmp &= ~JZ4780_I2C_CTRL_STPHLD;
> +					jz4780_i2c_writew(i2c,
> +							JZ4780_I2C_CTRL, tmp);
> +				}
>  			}
>  		} else {
>  			unsigned short data;
> @@ -497,23 +540,26 @@ static irqreturn_t jz4780_i2c_irq(int irqno, 
> void *dev_id)
>  			i2c_sta = jz4780_i2c_readw(i2c, JZ4780_I2C_STA);
> 
>  			while ((i2c_sta & JZ4780_I2C_STA_TFNF) &&
> -			       (i2c->wt_len > 0)) {
> +					(i2c->wt_len > 0)) {
>  				i2c_sta = jz4780_i2c_readw(i2c, JZ4780_I2C_STA);
>  				data = *i2c->wbuf;
>  				data &= ~JZ4780_I2C_DC_READ;
> -				jz4780_i2c_writew(i2c, JZ4780_I2C_DC,
> -						  data);
> +				if ((!i2c->stop_hold) && (i2c->cdata->version >=
> +						ID_X1000))
> +					data |= X1000_I2C_DC_STOP;
> +				jz4780_i2c_writew(i2c, JZ4780_I2C_DC, data);
>  				i2c->wbuf++;
>  				i2c->wt_len--;
>  			}
> 
>  			if (i2c->wt_len == 0) {
> -				if (!i2c->stop_hold) {
> +				if ((!i2c->stop_hold) && (i2c->cdata->version <
> +						ID_X1000)) {
>  					tmp = jz4780_i2c_readw(i2c,
> -							       JZ4780_I2C_CTRL);
> +							JZ4780_I2C_CTRL);
>  					tmp &= ~JZ4780_I2C_CTRL_STPHLD;
> -					jz4780_i2c_writew(i2c, JZ4780_I2C_CTRL,
> -							  tmp);
> +					jz4780_i2c_writew(i2c,
> +							JZ4780_I2C_CTRL, tmp);
>  				}
> 
>  				jz4780_i2c_trans_done(i2c);
> @@ -567,20 +613,22 @@ static inline int jz4780_i2c_xfer_read(struct 
> jz4780_i2c *i2c,
>  	i2c->rd_data_xfered = 0;
>  	i2c->rd_cmd_xfered = 0;
> 
> -	if (len <= JZ4780_I2C_FIFO_LEN)
> +	if (len <= i2c->cdata->fifosize)
>  		jz4780_i2c_writew(i2c, JZ4780_I2C_RXTL, len - 1);
>  	else
> -		jz4780_i2c_writew(i2c, JZ4780_I2C_RXTL, RX_LEVEL);
> +		jz4780_i2c_writew(i2c, JZ4780_I2C_RXTL, i2c->cdata->rx_level);
> 
> -	jz4780_i2c_writew(i2c, JZ4780_I2C_TXTL, TX_LEVEL);
> +	jz4780_i2c_writew(i2c, JZ4780_I2C_TXTL, i2c->cdata->tx_level);
> 
>  	jz4780_i2c_writew(i2c, JZ4780_I2C_INTM,
>  			  JZ4780_I2C_INTM_MRXFL | JZ4780_I2C_INTM_MTXEMP
>  			  | JZ4780_I2C_INTM_MTXABT | JZ4780_I2C_INTM_MRXOF);
> 
> -	tmp = jz4780_i2c_readw(i2c, JZ4780_I2C_CTRL);
> -	tmp |= JZ4780_I2C_CTRL_STPHLD;
> -	jz4780_i2c_writew(i2c, JZ4780_I2C_CTRL, tmp);
> +	if (i2c->cdata->version < ID_X1000) {
> +		tmp = jz4780_i2c_readw(i2c, JZ4780_I2C_CTRL);
> +		tmp |= JZ4780_I2C_CTRL_STPHLD;
> +		jz4780_i2c_writew(i2c, JZ4780_I2C_CTRL, tmp);
> +	}
> 
>  	spin_unlock_irqrestore(&i2c->lock, flags);
> 
> @@ -626,14 +674,16 @@ static inline int jz4780_i2c_xfer_write(struct 
> jz4780_i2c *i2c,
>  	i2c->wbuf = buf;
>  	i2c->wt_len = len;
> 
> -	jz4780_i2c_writew(i2c, JZ4780_I2C_TXTL, TX_LEVEL);
> +	jz4780_i2c_writew(i2c, JZ4780_I2C_TXTL, i2c->cdata->tx_level);
> 
>  	jz4780_i2c_writew(i2c, JZ4780_I2C_INTM, JZ4780_I2C_INTM_MTXEMP
>  					| JZ4780_I2C_INTM_MTXABT);
> 
> -	tmp = jz4780_i2c_readw(i2c, JZ4780_I2C_CTRL);
> -	tmp |= JZ4780_I2C_CTRL_STPHLD;
> -	jz4780_i2c_writew(i2c, JZ4780_I2C_CTRL, tmp);
> +	if (i2c->cdata->version < ID_X1000) {
> +		tmp = jz4780_i2c_readw(i2c, JZ4780_I2C_CTRL);
> +		tmp |= JZ4780_I2C_CTRL_STPHLD;
> +		jz4780_i2c_writew(i2c, JZ4780_I2C_CTRL, tmp);
> +	}
> 
>  	spin_unlock_irqrestore(&i2c->lock, flags);
> 
> @@ -716,8 +766,25 @@ static const struct i2c_algorithm 
> jz4780_i2c_algorithm = {
>  	.functionality	= jz4780_i2c_functionality,
>  };
> 
> +static const struct ingenic_i2c_config jz4780_i2c_config = {
> +	.version = ID_JZ4780,
> +
> +	.fifosize = JZ4780_I2C_FIFO_LEN,
> +	.tx_level = JZ4780_I2C_FIFO_LEN / 2,
> +	.rx_level = JZ4780_I2C_FIFO_LEN / 2 - 1,
> +};
> +
> +static const struct ingenic_i2c_config x1000_i2c_config = {
> +	.version = ID_X1000,
> +
> +	.fifosize = X1000_I2C_FIFO_LEN,
> +	.tx_level = X1000_I2C_FIFO_LEN / 2,
> +	.rx_level = X1000_I2C_FIFO_LEN / 2 - 1,
> +};
> +
>  static const struct of_device_id jz4780_i2c_of_matches[] = {
> -	{ .compatible = "ingenic,jz4780-i2c", },
> +	{ .compatible = "ingenic,jz4780-i2c", .data = &jz4780_i2c_config },
> +	{ .compatible = "ingenic,x1000-i2c", .data = &x1000_i2c_config },
>  	{ /* sentinel */ }
>  };
>  MODULE_DEVICE_TABLE(of, jz4780_i2c_of_matches);
> @@ -734,6 +801,12 @@ static int jz4780_i2c_probe(struct 
> platform_device *pdev)
>  	if (!i2c)
>  		return -ENOMEM;
> 
> +	i2c->cdata = device_get_match_data(&pdev->dev);
> +	if (!i2c->cdata) {
> +		dev_err(&pdev->dev, "Error: No device match found\n");
> +		return -ENODEV;
> +	}
> +
>  	i2c->adap.owner		= THIS_MODULE;
>  	i2c->adap.algo		= &jz4780_i2c_algorithm;
>  	i2c->adap.algo_data	= i2c;
> @@ -777,9 +850,11 @@ static int jz4780_i2c_probe(struct 
> platform_device *pdev)
> 
>  	dev_info(&pdev->dev, "Bus frequency is %d KHz\n", i2c->speed);
> 
> -	tmp = jz4780_i2c_readw(i2c, JZ4780_I2C_CTRL);
> -	tmp &= ~JZ4780_I2C_CTRL_STPHLD;
> -	jz4780_i2c_writew(i2c, JZ4780_I2C_CTRL, tmp);
> +	if (i2c->cdata->version < ID_X1000) {
> +		tmp = jz4780_i2c_readw(i2c, JZ4780_I2C_CTRL);
> +		tmp &= ~JZ4780_I2C_CTRL_STPHLD;
> +		jz4780_i2c_writew(i2c, JZ4780_I2C_CTRL, tmp);
> +	}
> 
>  	jz4780_i2c_writew(i2c, JZ4780_I2C_INTM, 0x0);
> 
> --
> 2.7.4
> 



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

* Re: [PATCH v2 2/2] I2C: JZ4780: Add support for the X1000.
  2019-12-16 13:57   ` Paul Cercueil
@ 2019-12-17  7:50     ` Zhou Yanjie
  0 siblings, 0 replies; 6+ messages in thread
From: Zhou Yanjie @ 2019-12-17  7:50 UTC (permalink / raw)
  To: Paul Cercueil
  Cc: linux-mips, linux-kernel, linux-i2c, devicetree, robh+dt,
	paul.burton, paulburton, mark.rutland, sernia.zhou, zhenwenjin,
	2374286503

Hi Paul,

On 2019年12月16日 21:57, Paul Cercueil wrote:
> Hi Zhou,
>
>
> Le lun., déc. 16, 2019 at 18:06, 周琰杰 (Zhou Yanjie) 
> <zhouyanjie@wanyeetech.com> a écrit :
>> Add support for probing i2c driver on the X1000 Soc from Ingenic.
>> call the corresponding fifo parameter according to the device
>> model obtained from the devicetree.
>>
>> Signed-off-by: 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com>
>> ---
>>
>> Notes:
>>     v1->v2:
>>     Add code to check device_get_match_data(), if it return a NULL ptr,
>>     then print an error message and return -ENODEV.
>>
>>  drivers/i2c/busses/i2c-jz4780.c | 155 
>> +++++++++++++++++++++++++++++-----------
>>  1 file changed, 115 insertions(+), 40 deletions(-)
>>
>> diff --git a/drivers/i2c/busses/i2c-jz4780.c 
>> b/drivers/i2c/busses/i2c-jz4780.c
>> index 25dcd73..f07a07c 100644
>> --- a/drivers/i2c/busses/i2c-jz4780.c
>> +++ b/drivers/i2c/busses/i2c-jz4780.c
>> @@ -4,6 +4,7 @@
>>   *
>>   * Copyright (C) 2006 - 2009 Ingenic Semiconductor Inc.
>>   * Copyright (C) 2015 Imagination Technologies
>> + * Copyright (C) 2019 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com>
>>   */
>>
>>  #include <linux/bitops.h>
>> @@ -17,6 +18,7 @@
>>  #include <linux/io.h>
>>  #include <linux/kernel.h>
>>  #include <linux/module.h>
>> +#include <linux/of_device.h>
>>  #include <linux/platform_device.h>
>>  #include <linux/sched.h>
>>  #include <linux/slab.h>
>> @@ -55,6 +57,7 @@
>>  #define JZ4780_I2C_ACKGC    0x98
>>  #define JZ4780_I2C_ENSTA    0x9C
>>  #define JZ4780_I2C_SDAHD    0xD0
>> +#define X1000_I2C_SDAHD        0x7C
>>
>>  #define JZ4780_I2C_CTRL_STPHLD        BIT(7)
>>  #define JZ4780_I2C_CTRL_SLVDIS        BIT(6)
>> @@ -73,6 +76,8 @@
>>  #define JZ4780_I2C_STA_TFNF        BIT(1)
>>  #define JZ4780_I2C_STA_ACT        BIT(0)
>>
>> +#define X1000_I2C_DC_STOP        BIT(9)
>> +
>>  static const char * const jz4780_i2c_abrt_src[] = {
>>      "ABRT_7B_ADDR_NOACK",
>>      "ABRT_10ADDR1_NOACK",
>> @@ -130,18 +135,33 @@ static const char * const jz4780_i2c_abrt_src[] 
>> = {
>>  #define JZ4780_I2CFLCNT_ADJUST(n)    (((n) - 1) < 8 ? 8 : ((n) - 1))
>>
>>  #define JZ4780_I2C_FIFO_LEN    16
>> -#define TX_LEVEL        3
>> -#define RX_LEVEL        (JZ4780_I2C_FIFO_LEN - TX_LEVEL - 1)
>> +
>> +#define X1000_I2C_FIFO_LEN    64
>>
>>  #define JZ4780_I2C_TIMEOUT    300
>>
>>  #define BUFSIZE 200
>>
>> +enum ingenic_i2c_version {
>> +    ID_JZ4780,
>> +    ID_X1000,
>> +};
>> +
>> +/* ingenic_i2c_config: SoC specific config data. */
>> +struct ingenic_i2c_config {
>> +    enum ingenic_i2c_version version;
>> +
>> +    int fifosize;
>> +    int tx_level;
>> +    int rx_level;
>> +};
>> +
>>  struct jz4780_i2c {
>>      void __iomem        *iomem;
>>      int             irq;
>>      struct clk        *clk;
>>      struct i2c_adapter     adap;
>> +    const struct ingenic_i2c_config *cdata;
>>
>>      /* lock to protect rbuf and wbuf between xfer_rd/wr and irq 
>> handler */
>>      spinlock_t        lock;
>> @@ -340,11 +360,18 @@ static int jz4780_i2c_set_speed(struct 
>> jz4780_i2c *i2c)
>>
>>      if (hold_time >= 0) {
>>          /*i2c hold time enable */
>> -        hold_time |= JZ4780_I2C_SDAHD_HDENB;
>> -        jz4780_i2c_writew(i2c, JZ4780_I2C_SDAHD, hold_time);
>> +        if (i2c->cdata->version >= ID_X1000)
>> +            jz4780_i2c_writew(i2c, X1000_I2C_SDAHD, hold_time);
>> +        else {
>
> If only one branch of a conditional statement is a single statement, 
> then you should use braces in both branches.
>

Sure, I will fix this in v3.

> See: 
> https://www.kernel.org/doc/html/v4.10/process/coding-style.html#placing-braces-and-spaces 
>
>
>
>> +            hold_time |= JZ4780_I2C_SDAHD_HDENB;
>> +            jz4780_i2c_writew(i2c, JZ4780_I2C_SDAHD, hold_time);
>> +        }
>>      } else {
>>          /* disable hold time */
>> -        jz4780_i2c_writew(i2c, JZ4780_I2C_SDAHD, 0);
>> +        if (i2c->cdata->version >= ID_X1000)
>> +            jz4780_i2c_writew(i2c, X1000_I2C_SDAHD, 0);
>> +        else
>> +            jz4780_i2c_writew(i2c, JZ4780_I2C_SDAHD, 0);
>>      }
>>
>>      return 0;
>> @@ -359,9 +386,11 @@ static int jz4780_i2c_cleanup(struct jz4780_i2c 
>> *i2c)
>>      spin_lock_irqsave(&i2c->lock, flags);
>>
>>      /* can send stop now if need */
>> -    tmp = jz4780_i2c_readw(i2c, JZ4780_I2C_CTRL);
>> -    tmp &= ~JZ4780_I2C_CTRL_STPHLD;
>> -    jz4780_i2c_writew(i2c, JZ4780_I2C_CTRL, tmp);
>> +    if (i2c->cdata->version < ID_X1000) {
>> +        tmp = jz4780_i2c_readw(i2c, JZ4780_I2C_CTRL);
>> +        tmp &= ~JZ4780_I2C_CTRL_STPHLD;
>> +        jz4780_i2c_writew(i2c, JZ4780_I2C_CTRL, tmp);
>> +    }
>>
>>      /* disable all interrupts first */
>>      jz4780_i2c_writew(i2c, JZ4780_I2C_INTM, 0);
>> @@ -399,11 +428,18 @@ static int jz4780_i2c_prepare(struct jz4780_i2c 
>> *i2c)
>>      return jz4780_i2c_enable(i2c);
>>  }
>>
>> -static void jz4780_i2c_send_rcmd(struct jz4780_i2c *i2c, int cmd_count)
>> +static void jz4780_i2c_send_rcmd(struct jz4780_i2c *i2c,
>> +                       int cmd_count, int cmd_left)
>
> Sorry to be pedantic ;) but this line is not properly indented. You 
> should indent with tab charaters (configure your IDE for one tab == 4 
> spaces) as much as possible, then use spaces to align the first word.
>

Sure, I will fix this in v3.

Thanks and best regards!

> With these two things fixed:
> Acked-by: Paul Cercueil <paul@crapouillou.net>
>
> Cheers,
> -Paul
>
>
>>  {
>>      int i;
>>
>> -    for (i = 0; i < cmd_count; i++)
>> +    for (i = 0; i < cmd_count - 1; i++)
>> +        jz4780_i2c_writew(i2c, JZ4780_I2C_DC, JZ4780_I2C_DC_READ);
>> +
>> +    if ((cmd_left == 0) && (i2c->cdata->version >= ID_X1000))
>> +        jz4780_i2c_writew(i2c, JZ4780_I2C_DC,
>> +                JZ4780_I2C_DC_READ | X1000_I2C_DC_STOP);
>> +    else
>>          jz4780_i2c_writew(i2c, JZ4780_I2C_DC, JZ4780_I2C_DC_READ);
>>  }
>>
>> @@ -458,37 +494,44 @@ static irqreturn_t jz4780_i2c_irq(int irqno, 
>> void *dev_id)
>>
>>          rd_left = i2c->rd_total_len - i2c->rd_data_xfered;
>>
>> -        if (rd_left <= JZ4780_I2C_FIFO_LEN)
>> +        if (rd_left <= i2c->cdata->fifosize)
>>              jz4780_i2c_writew(i2c, JZ4780_I2C_RXTL, rd_left - 1);
>>      }
>>
>>      if (intst & JZ4780_I2C_INTST_TXEMP) {
>>          if (i2c->is_write == 0) {
>>              int cmd_left = i2c->rd_total_len - i2c->rd_cmd_xfered;
>> -            int max_send = (JZ4780_I2C_FIFO_LEN - 1)
>> +            int max_send = (i2c->cdata->fifosize - 1)
>>                       - (i2c->rd_cmd_xfered
>>                       - i2c->rd_data_xfered);
>>              int cmd_to_send = min(cmd_left, max_send);
>>
>>              if (i2c->rd_cmd_xfered != 0)
>>                  cmd_to_send = min(cmd_to_send,
>> -                          JZ4780_I2C_FIFO_LEN
>> -                          - TX_LEVEL - 1);
>> +                          i2c->cdata->fifosize
>> +                          - i2c->cdata->tx_level - 1);
>>
>>              if (cmd_to_send) {
>> -                jz4780_i2c_send_rcmd(i2c, cmd_to_send);
>>                  i2c->rd_cmd_xfered += cmd_to_send;
>> +                cmd_left = i2c->rd_total_len -
>> +                        i2c->rd_cmd_xfered;
>> +                jz4780_i2c_send_rcmd(i2c,
>> +                        cmd_to_send, cmd_left);
>> +
>>              }
>>
>> -            cmd_left = i2c->rd_total_len - i2c->rd_cmd_xfered;
>>              if (cmd_left == 0) {
>>                  intmsk = jz4780_i2c_readw(i2c, JZ4780_I2C_INTM);
>>                  intmsk &= ~JZ4780_I2C_INTM_MTXEMP;
>>                  jz4780_i2c_writew(i2c, JZ4780_I2C_INTM, intmsk);
>>
>> -                tmp = jz4780_i2c_readw(i2c, JZ4780_I2C_CTRL);
>> -                tmp &= ~JZ4780_I2C_CTRL_STPHLD;
>> -                jz4780_i2c_writew(i2c, JZ4780_I2C_CTRL, tmp);
>> +                if (i2c->cdata->version < ID_X1000) {
>> +                    tmp = jz4780_i2c_readw(i2c,
>> +                            JZ4780_I2C_CTRL);
>> +                    tmp &= ~JZ4780_I2C_CTRL_STPHLD;
>> +                    jz4780_i2c_writew(i2c,
>> +                            JZ4780_I2C_CTRL, tmp);
>> +                }
>>              }
>>          } else {
>>              unsigned short data;
>> @@ -497,23 +540,26 @@ static irqreturn_t jz4780_i2c_irq(int irqno, 
>> void *dev_id)
>>              i2c_sta = jz4780_i2c_readw(i2c, JZ4780_I2C_STA);
>>
>>              while ((i2c_sta & JZ4780_I2C_STA_TFNF) &&
>> -                   (i2c->wt_len > 0)) {
>> +                    (i2c->wt_len > 0)) {
>>                  i2c_sta = jz4780_i2c_readw(i2c, JZ4780_I2C_STA);
>>                  data = *i2c->wbuf;
>>                  data &= ~JZ4780_I2C_DC_READ;
>> -                jz4780_i2c_writew(i2c, JZ4780_I2C_DC,
>> -                          data);
>> +                if ((!i2c->stop_hold) && (i2c->cdata->version >=
>> +                        ID_X1000))
>> +                    data |= X1000_I2C_DC_STOP;
>> +                jz4780_i2c_writew(i2c, JZ4780_I2C_DC, data);
>>                  i2c->wbuf++;
>>                  i2c->wt_len--;
>>              }
>>
>>              if (i2c->wt_len == 0) {
>> -                if (!i2c->stop_hold) {
>> +                if ((!i2c->stop_hold) && (i2c->cdata->version <
>> +                        ID_X1000)) {
>>                      tmp = jz4780_i2c_readw(i2c,
>> -                                   JZ4780_I2C_CTRL);
>> +                            JZ4780_I2C_CTRL);
>>                      tmp &= ~JZ4780_I2C_CTRL_STPHLD;
>> -                    jz4780_i2c_writew(i2c, JZ4780_I2C_CTRL,
>> -                              tmp);
>> +                    jz4780_i2c_writew(i2c,
>> +                            JZ4780_I2C_CTRL, tmp);
>>                  }
>>
>>                  jz4780_i2c_trans_done(i2c);
>> @@ -567,20 +613,22 @@ static inline int jz4780_i2c_xfer_read(struct 
>> jz4780_i2c *i2c,
>>      i2c->rd_data_xfered = 0;
>>      i2c->rd_cmd_xfered = 0;
>>
>> -    if (len <= JZ4780_I2C_FIFO_LEN)
>> +    if (len <= i2c->cdata->fifosize)
>>          jz4780_i2c_writew(i2c, JZ4780_I2C_RXTL, len - 1);
>>      else
>> -        jz4780_i2c_writew(i2c, JZ4780_I2C_RXTL, RX_LEVEL);
>> +        jz4780_i2c_writew(i2c, JZ4780_I2C_RXTL, i2c->cdata->rx_level);
>>
>> -    jz4780_i2c_writew(i2c, JZ4780_I2C_TXTL, TX_LEVEL);
>> +    jz4780_i2c_writew(i2c, JZ4780_I2C_TXTL, i2c->cdata->tx_level);
>>
>>      jz4780_i2c_writew(i2c, JZ4780_I2C_INTM,
>>                JZ4780_I2C_INTM_MRXFL | JZ4780_I2C_INTM_MTXEMP
>>                | JZ4780_I2C_INTM_MTXABT | JZ4780_I2C_INTM_MRXOF);
>>
>> -    tmp = jz4780_i2c_readw(i2c, JZ4780_I2C_CTRL);
>> -    tmp |= JZ4780_I2C_CTRL_STPHLD;
>> -    jz4780_i2c_writew(i2c, JZ4780_I2C_CTRL, tmp);
>> +    if (i2c->cdata->version < ID_X1000) {
>> +        tmp = jz4780_i2c_readw(i2c, JZ4780_I2C_CTRL);
>> +        tmp |= JZ4780_I2C_CTRL_STPHLD;
>> +        jz4780_i2c_writew(i2c, JZ4780_I2C_CTRL, tmp);
>> +    }
>>
>>      spin_unlock_irqrestore(&i2c->lock, flags);
>>
>> @@ -626,14 +674,16 @@ static inline int jz4780_i2c_xfer_write(struct 
>> jz4780_i2c *i2c,
>>      i2c->wbuf = buf;
>>      i2c->wt_len = len;
>>
>> -    jz4780_i2c_writew(i2c, JZ4780_I2C_TXTL, TX_LEVEL);
>> +    jz4780_i2c_writew(i2c, JZ4780_I2C_TXTL, i2c->cdata->tx_level);
>>
>>      jz4780_i2c_writew(i2c, JZ4780_I2C_INTM, JZ4780_I2C_INTM_MTXEMP
>>                      | JZ4780_I2C_INTM_MTXABT);
>>
>> -    tmp = jz4780_i2c_readw(i2c, JZ4780_I2C_CTRL);
>> -    tmp |= JZ4780_I2C_CTRL_STPHLD;
>> -    jz4780_i2c_writew(i2c, JZ4780_I2C_CTRL, tmp);
>> +    if (i2c->cdata->version < ID_X1000) {
>> +        tmp = jz4780_i2c_readw(i2c, JZ4780_I2C_CTRL);
>> +        tmp |= JZ4780_I2C_CTRL_STPHLD;
>> +        jz4780_i2c_writew(i2c, JZ4780_I2C_CTRL, tmp);
>> +    }
>>
>>      spin_unlock_irqrestore(&i2c->lock, flags);
>>
>> @@ -716,8 +766,25 @@ static const struct i2c_algorithm 
>> jz4780_i2c_algorithm = {
>>      .functionality    = jz4780_i2c_functionality,
>>  };
>>
>> +static const struct ingenic_i2c_config jz4780_i2c_config = {
>> +    .version = ID_JZ4780,
>> +
>> +    .fifosize = JZ4780_I2C_FIFO_LEN,
>> +    .tx_level = JZ4780_I2C_FIFO_LEN / 2,
>> +    .rx_level = JZ4780_I2C_FIFO_LEN / 2 - 1,
>> +};
>> +
>> +static const struct ingenic_i2c_config x1000_i2c_config = {
>> +    .version = ID_X1000,
>> +
>> +    .fifosize = X1000_I2C_FIFO_LEN,
>> +    .tx_level = X1000_I2C_FIFO_LEN / 2,
>> +    .rx_level = X1000_I2C_FIFO_LEN / 2 - 1,
>> +};
>> +
>>  static const struct of_device_id jz4780_i2c_of_matches[] = {
>> -    { .compatible = "ingenic,jz4780-i2c", },
>> +    { .compatible = "ingenic,jz4780-i2c", .data = &jz4780_i2c_config },
>> +    { .compatible = "ingenic,x1000-i2c", .data = &x1000_i2c_config },
>>      { /* sentinel */ }
>>  };
>>  MODULE_DEVICE_TABLE(of, jz4780_i2c_of_matches);
>> @@ -734,6 +801,12 @@ static int jz4780_i2c_probe(struct 
>> platform_device *pdev)
>>      if (!i2c)
>>          return -ENOMEM;
>>
>> +    i2c->cdata = device_get_match_data(&pdev->dev);
>> +    if (!i2c->cdata) {
>> +        dev_err(&pdev->dev, "Error: No device match found\n");
>> +        return -ENODEV;
>> +    }
>> +
>>      i2c->adap.owner        = THIS_MODULE;
>>      i2c->adap.algo        = &jz4780_i2c_algorithm;
>>      i2c->adap.algo_data    = i2c;
>> @@ -777,9 +850,11 @@ static int jz4780_i2c_probe(struct 
>> platform_device *pdev)
>>
>>      dev_info(&pdev->dev, "Bus frequency is %d KHz\n", i2c->speed);
>>
>> -    tmp = jz4780_i2c_readw(i2c, JZ4780_I2C_CTRL);
>> -    tmp &= ~JZ4780_I2C_CTRL_STPHLD;
>> -    jz4780_i2c_writew(i2c, JZ4780_I2C_CTRL, tmp);
>> +    if (i2c->cdata->version < ID_X1000) {
>> +        tmp = jz4780_i2c_readw(i2c, JZ4780_I2C_CTRL);
>> +        tmp &= ~JZ4780_I2C_CTRL_STPHLD;
>> +        jz4780_i2c_writew(i2c, JZ4780_I2C_CTRL, tmp);
>> +    }
>>
>>      jz4780_i2c_writew(i2c, JZ4780_I2C_INTM, 0x0);
>>
>> -- 
>> 2.7.4
>>
>


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

end of thread, other threads:[~2019-12-17  7:51 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-16 10:06 Add I2C support for the Ingenic X1000 SoC v2 周琰杰 (Zhou Yanjie)
2019-12-16 10:06 ` [PATCH v2 0/2] Add I2C support for the Ingenic X1000 SoC 周琰杰 (Zhou Yanjie)
2019-12-16 10:06 ` [PATCH v2 1/2] dt-bindings: I2C: Add X1000 bindings 周琰杰 (Zhou Yanjie)
2019-12-16 10:06 ` [PATCH v2 2/2] I2C: JZ4780: Add support for the X1000 周琰杰 (Zhou Yanjie)
2019-12-16 13:57   ` Paul Cercueil
2019-12-17  7:50     ` Zhou Yanjie

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).