All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/3] i2c: xlr: add support for Sigma Designs controller variant
@ 2015-11-02  2:03 Mans Rullgard
  2015-11-02  2:03 ` [PATCH 2/3] i2c: xlr: fix extra read/write at end of rx transfer Mans Rullgard
                   ` (3 more replies)
  0 siblings, 4 replies; 8+ messages in thread
From: Mans Rullgard @ 2015-11-02  2:03 UTC (permalink / raw
  To: Wolfram Sang, linux-i2c, linux-kernel

Sigma Designs chips use a variant of this controller with the following
differences:

- The BUSY bit in the STATUS register is inverted
- Bit 8 of the CONFIG register must be set
- The controller can generate interrupts

This patch adds support for the first two of these.  It also calculates
and sets the correct clock divisor if a clk is provided.  The bus
frequency is optionally speficied in the device tree node.

Signed-off-by: Mans Rullgard <mans@mansr.com>
---
 drivers/i2c/busses/Kconfig   |  6 ++--
 drivers/i2c/busses/i2c-xlr.c | 81 +++++++++++++++++++++++++++++++++++++++++---
 2 files changed, 80 insertions(+), 7 deletions(-)

diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index f764e3a..fdedf59 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -944,11 +944,11 @@ config I2C_XILINX
 	  will be called xilinx_i2c.
 
 config I2C_XLR
-	tristate "XLR I2C support"
-	depends on CPU_XLR
+	tristate "Netlogic XLR and Sigma Designs I2C support"
+	depends on CPU_XLR || ARCH_TANGOX
 	help
 	  This driver enables support for the on-chip I2C interface of
-	  the Netlogic XLR/XLS MIPS processors.
+	  the Netlogic XLR/XLS MIPS processors and Sigma Designs SOCs.
 
 	  This driver can also be built as a module.  If so, the module
 	  will be called i2c-xlr.
diff --git a/drivers/i2c/busses/i2c-xlr.c b/drivers/i2c/busses/i2c-xlr.c
index 8b36bcf..10fb916 100644
--- a/drivers/i2c/busses/i2c-xlr.c
+++ b/drivers/i2c/busses/i2c-xlr.c
@@ -17,6 +17,8 @@
 #include <linux/i2c.h>
 #include <linux/io.h>
 #include <linux/platform_device.h>
+#include <linux/of_device.h>
+#include <linux/clk.h>
 
 /* XLR I2C REGISTERS */
 #define XLR_I2C_CFG		0x00
@@ -63,11 +65,23 @@ static inline u32 xlr_i2c_rdreg(u32 __iomem *base, unsigned int reg)
 	return __raw_readl(base + reg);
 }
 
+struct xlr_i2c_config {
+	u32 status_busy;	/* value of STATUS[0] when busy */
+	u32 cfg_extra;		/* extra CFG bits to set */
+};
+
 struct xlr_i2c_private {
 	struct i2c_adapter adap;
 	u32 __iomem *iobase;
+	const struct xlr_i2c_config *cfg;
+	struct clk *clk;
 };
 
+static int xlr_i2c_busy(struct xlr_i2c_private *priv, u32 status)
+{
+	return (status & XLR_I2C_BUS_BUSY) == priv->cfg->status_busy;
+}
+
 static int xlr_i2c_tx(struct xlr_i2c_private *priv,  u16 len,
 	u8 *buf, u16 addr)
 {
@@ -80,7 +94,8 @@ static int xlr_i2c_tx(struct xlr_i2c_private *priv,  u16 len,
 	offset = buf[0];
 	xlr_i2c_wreg(priv->iobase, XLR_I2C_ADDR, offset);
 	xlr_i2c_wreg(priv->iobase, XLR_I2C_DEVADDR, addr);
-	xlr_i2c_wreg(priv->iobase, XLR_I2C_CFG, XLR_I2C_CFG_ADDR);
+	xlr_i2c_wreg(priv->iobase, XLR_I2C_CFG,
+			XLR_I2C_CFG_ADDR | priv->cfg->cfg_extra);
 	xlr_i2c_wreg(priv->iobase, XLR_I2C_BYTECNT, len - 1);
 
 	timeout = msecs_to_jiffies(XLR_I2C_TIMEOUT);
@@ -121,7 +136,7 @@ retry:
 		if (i2c_status & XLR_I2C_ACK_ERR)
 			return -EIO;
 
-		if ((i2c_status & XLR_I2C_BUS_BUSY) == 0 && pos >= len)
+		if (!xlr_i2c_busy(priv, i2c_status) && pos >= len)
 			return 0;
 	}
 	dev_err(&adap->dev, "I2C transmit timeout\n");
@@ -136,7 +151,8 @@ static int xlr_i2c_rx(struct xlr_i2c_private *priv, u16 len, u8 *buf, u16 addr)
 	int nbytes, timedout;
 	u8 byte;
 
-	xlr_i2c_wreg(priv->iobase, XLR_I2C_CFG, XLR_I2C_CFG_NOADDR);
+	xlr_i2c_wreg(priv->iobase, XLR_I2C_CFG,
+			XLR_I2C_CFG_NOADDR | priv->cfg->cfg_extra);
 	xlr_i2c_wreg(priv->iobase, XLR_I2C_BYTECNT, len);
 	xlr_i2c_wreg(priv->iobase, XLR_I2C_DEVADDR, addr);
 
@@ -174,7 +190,7 @@ retry:
 		if (i2c_status & XLR_I2C_ACK_ERR)
 			return -EIO;
 
-		if ((i2c_status & XLR_I2C_BUS_BUSY) == 0)
+		if (!xlr_i2c_busy(priv, i2c_status))
 			return 0;
 	}
 
@@ -190,6 +206,10 @@ static int xlr_i2c_xfer(struct i2c_adapter *adap,
 	int ret = 0;
 	struct xlr_i2c_private *priv = i2c_get_adapdata(adap);
 
+	ret = clk_enable(priv->clk);
+	if (ret)
+		return ret;
+
 	for (i = 0; ret == 0 && i < num; i++) {
 		msg = &msgs[i];
 		if (msg->flags & I2C_M_RD)
@@ -200,6 +220,8 @@ static int xlr_i2c_xfer(struct i2c_adapter *adap,
 					msg->addr);
 	}
 
+	clk_disable(priv->clk);
+
 	return (ret != 0) ? ret : num;
 }
 
@@ -214,22 +236,70 @@ static struct i2c_algorithm xlr_i2c_algo = {
 	.functionality	= xlr_func,
 };
 
+static const struct xlr_i2c_config xlr_i2c_config_default = {
+	.status_busy	= XLR_I2C_BUS_BUSY,
+	.cfg_extra	= 0,
+};
+
+static const struct xlr_i2c_config xlr_i2c_config_tangox = {
+	.status_busy	= 0,
+	.cfg_extra	= 1 << 8,
+};
+
+static const struct of_device_id xlr_i2c_dt_ids[] = {
+	{
+		.compatible	= "sigma,smp8642-i2c",
+		.data		= &xlr_i2c_config_tangox,
+	},
+	{ }
+};
+
 static int xlr_i2c_probe(struct platform_device *pdev)
 {
+	const struct of_device_id *match;
 	struct xlr_i2c_private  *priv;
 	struct resource *res;
+	struct clk *clk;
+	unsigned long clk_rate;
+	unsigned long clk_div;
+	u32 busfreq;
 	int ret;
 
 	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
 	if (!priv)
 		return -ENOMEM;
 
+	match = of_match_device(xlr_i2c_dt_ids, &pdev->dev);
+	if (match)
+		priv->cfg = match->data;
+	else
+		priv->cfg = &xlr_i2c_config_default;
+
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	priv->iobase = devm_ioremap_resource(&pdev->dev, res);
 	if (IS_ERR(priv->iobase))
 		return PTR_ERR(priv->iobase);
 
+	if (of_property_read_u32(pdev->dev.of_node, "clock-frequency",
+				 &busfreq))
+		busfreq = 100000;
+
+	clk = devm_clk_get(&pdev->dev, NULL);
+	if (!IS_ERR(clk)) {
+		ret = clk_prepare_enable(clk);
+		if (ret)
+			return ret;
+
+		clk_rate = clk_get_rate(clk);
+		clk_div = DIV_ROUND_UP(clk_rate, 2 * busfreq);
+		xlr_i2c_wreg(priv->iobase, XLR_I2C_CLKDIV, clk_div);
+
+		clk_disable(clk);
+		priv->clk = clk;
+	}
+
 	priv->adap.dev.parent = &pdev->dev;
+	priv->adap.dev.of_node	= pdev->dev.of_node;
 	priv->adap.owner	= THIS_MODULE;
 	priv->adap.algo_data	= priv;
 	priv->adap.algo		= &xlr_i2c_algo;
@@ -255,6 +325,8 @@ static int xlr_i2c_remove(struct platform_device *pdev)
 
 	priv = platform_get_drvdata(pdev);
 	i2c_del_adapter(&priv->adap);
+	clk_unprepare(priv->clk);
+
 	return 0;
 }
 
@@ -263,6 +335,7 @@ static struct platform_driver xlr_i2c_driver = {
 	.remove = xlr_i2c_remove,
 	.driver = {
 		.name   = "xlr-i2cbus",
+		.of_match_table	= xlr_i2c_dt_ids,
 	},
 };
 
-- 
2.6.2


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

* [PATCH 2/3] i2c: xlr: fix extra read/write at end of rx transfer
  2015-11-02  2:03 [PATCH 1/3] i2c: xlr: add support for Sigma Designs controller variant Mans Rullgard
@ 2015-11-02  2:03 ` Mans Rullgard
  2015-12-15 12:21   ` Wolfram Sang
  2015-11-02  2:03 ` [PATCH 3/3] i2c: xlr: add interrupt support for Sigma Designs chips Mans Rullgard
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 8+ messages in thread
From: Mans Rullgard @ 2015-11-02  2:03 UTC (permalink / raw
  To: Wolfram Sang, linux-i2c, linux-kernel

The BYTECNT register holds the transfer size minus one.  Setting it
to the correct value requires a dummy read/write only for zero-length
transfers as it is impossible to request one from the hardware.  If a
zero-length transfer is requested, changing the length to 1 and setting
"buf" to a dummy location allows making the main loops less convoluted.

In other words, this patch makes the driver transfer the number of bytes
requested unless this is zero, which is not supported by the hardware,
in which case one byte is transferred instead.

Signed-off-by: Mans Rullgard <mans@mansr.com>
---
 drivers/i2c/busses/i2c-xlr.c | 51 +++++++++++++++++++++++++++-----------------
 1 file changed, 31 insertions(+), 20 deletions(-)

diff --git a/drivers/i2c/busses/i2c-xlr.c b/drivers/i2c/busses/i2c-xlr.c
index 10fb916..c1afc3e 100644
--- a/drivers/i2c/busses/i2c-xlr.c
+++ b/drivers/i2c/busses/i2c-xlr.c
@@ -90,37 +90,45 @@ static int xlr_i2c_tx(struct xlr_i2c_private *priv,  u16 len,
 	u32 i2c_status;
 	int pos, timedout;
 	u8 offset, byte;
+	u32 xfer;
+
+	if (!len) {
+		byte = 0;
+		buf = &byte;
+		len = 1;
+	}
 
 	offset = buf[0];
 	xlr_i2c_wreg(priv->iobase, XLR_I2C_ADDR, offset);
 	xlr_i2c_wreg(priv->iobase, XLR_I2C_DEVADDR, addr);
 	xlr_i2c_wreg(priv->iobase, XLR_I2C_CFG,
 			XLR_I2C_CFG_ADDR | priv->cfg->cfg_extra);
-	xlr_i2c_wreg(priv->iobase, XLR_I2C_BYTECNT, len - 1);
 
 	timeout = msecs_to_jiffies(XLR_I2C_TIMEOUT);
 	stoptime = jiffies + timeout;
 	timedout = 0;
-	pos = 1;
-retry:
+
 	if (len == 1) {
-		xlr_i2c_wreg(priv->iobase, XLR_I2C_STARTXFR,
-				XLR_I2C_STARTXFR_ND);
+		xlr_i2c_wreg(priv->iobase, XLR_I2C_BYTECNT, len - 1);
+		xfer = XLR_I2C_STARTXFR_ND;
+		pos = 1;
 	} else {
-		xlr_i2c_wreg(priv->iobase, XLR_I2C_DATAOUT, buf[pos]);
-		xlr_i2c_wreg(priv->iobase, XLR_I2C_STARTXFR,
-				XLR_I2C_STARTXFR_WR);
+		xlr_i2c_wreg(priv->iobase, XLR_I2C_BYTECNT, len - 2);
+		xlr_i2c_wreg(priv->iobase, XLR_I2C_DATAOUT, buf[1]);
+		xfer = XLR_I2C_STARTXFR_WR;
+		pos = 2;
 	}
 
+retry:
+	/* retry can only happen on the first byte */
+	xlr_i2c_wreg(priv->iobase, XLR_I2C_STARTXFR, xfer);
+
 	while (!timedout) {
 		checktime = jiffies;
 		i2c_status = xlr_i2c_rdreg(priv->iobase, XLR_I2C_STATUS);
 
-		if (i2c_status & XLR_I2C_SDOEMPTY) {
-			pos++;
-			/* need to do a empty dataout after the last byte */
-			byte = (pos < len) ? buf[pos] : 0;
-			xlr_i2c_wreg(priv->iobase, XLR_I2C_DATAOUT, byte);
+		if ((i2c_status & XLR_I2C_SDOEMPTY) && pos < len) {
+			xlr_i2c_wreg(priv->iobase, XLR_I2C_DATAOUT, buf[pos++]);
 
 			/* reset timeout on successful xmit */
 			stoptime = jiffies + timeout;
@@ -151,9 +159,15 @@ static int xlr_i2c_rx(struct xlr_i2c_private *priv, u16 len, u8 *buf, u16 addr)
 	int nbytes, timedout;
 	u8 byte;
 
+	if (!len) {
+		byte = 0;
+		buf = &byte;
+		len = 1;
+	}
+
 	xlr_i2c_wreg(priv->iobase, XLR_I2C_CFG,
 			XLR_I2C_CFG_NOADDR | priv->cfg->cfg_extra);
-	xlr_i2c_wreg(priv->iobase, XLR_I2C_BYTECNT, len);
+	xlr_i2c_wreg(priv->iobase, XLR_I2C_BYTECNT, len - 1);
 	xlr_i2c_wreg(priv->iobase, XLR_I2C_DEVADDR, addr);
 
 	timeout = msecs_to_jiffies(XLR_I2C_TIMEOUT);
@@ -167,14 +181,11 @@ retry:
 		checktime = jiffies;
 		i2c_status = xlr_i2c_rdreg(priv->iobase, XLR_I2C_STATUS);
 		if (i2c_status & XLR_I2C_RXRDY) {
-			if (nbytes > len)
+			if (nbytes >= len)
 				return -EIO;	/* should not happen */
 
-			/* we need to do a dummy datain when nbytes == len */
-			byte = xlr_i2c_rdreg(priv->iobase, XLR_I2C_DATAIN);
-			if (nbytes < len)
-				buf[nbytes] = byte;
-			nbytes++;
+			buf[nbytes++] =
+				xlr_i2c_rdreg(priv->iobase, XLR_I2C_DATAIN);
 
 			/* reset timeout on successful read */
 			stoptime = jiffies + timeout;
-- 
2.6.2


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

* [PATCH 3/3] i2c: xlr: add interrupt support for Sigma Designs chips
  2015-11-02  2:03 [PATCH 1/3] i2c: xlr: add support for Sigma Designs controller variant Mans Rullgard
  2015-11-02  2:03 ` [PATCH 2/3] i2c: xlr: fix extra read/write at end of rx transfer Mans Rullgard
@ 2015-11-02  2:03 ` Mans Rullgard
  2015-12-15 12:23   ` Wolfram Sang
  2015-11-21 12:02 ` [PATCH 1/3] i2c: xlr: add support for Sigma Designs controller variant Måns Rullgård
  2015-12-15 12:14 ` Wolfram Sang
  3 siblings, 1 reply; 8+ messages in thread
From: Mans Rullgard @ 2015-11-02  2:03 UTC (permalink / raw
  To: Wolfram Sang, linux-i2c, linux-kernel

The Sigma Designs variant of this controller has the ability to generate
interrupts.  This is controlled using two additional registers, oddly
enough overlapping with the defined but unused HDSTATIM.

This patch adds support for using this feature instead of busy-looping
if an IRQ is specified.

Signed-off-by: Mans Rullgard <mans@mansr.com>
---
 drivers/i2c/busses/i2c-xlr.c | 119 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 119 insertions(+)

diff --git a/drivers/i2c/busses/i2c-xlr.c b/drivers/i2c/busses/i2c-xlr.c
index c1afc3e..dd48c1e 100644
--- a/drivers/i2c/busses/i2c-xlr.c
+++ b/drivers/i2c/busses/i2c-xlr.c
@@ -19,6 +19,8 @@
 #include <linux/platform_device.h>
 #include <linux/of_device.h>
 #include <linux/clk.h>
+#include <linux/interrupt.h>
+#include <linux/wait.h>
 
 /* XLR I2C REGISTERS */
 #define XLR_I2C_CFG		0x00
@@ -32,6 +34,10 @@
 #define XLR_I2C_BYTECNT		0x08
 #define XLR_I2C_HDSTATIM	0x09
 
+/* Sigma Designs additional registers */
+#define XLR_I2C_INT_EN		0x09
+#define XLR_I2C_INT_STAT	0x0a
+
 /* XLR I2C REGISTERS FLAGS */
 #define XLR_I2C_BUS_BUSY	0x01
 #define XLR_I2C_SDOEMPTY	0x02
@@ -65,7 +71,10 @@ static inline u32 xlr_i2c_rdreg(u32 __iomem *base, unsigned int reg)
 	return __raw_readl(base + reg);
 }
 
+#define XLR_I2C_FLAG_IRQ	1
+
 struct xlr_i2c_config {
+	u32 flags;		/* optional feature support */
 	u32 status_busy;	/* value of STATUS[0] when busy */
 	u32 cfg_extra;		/* extra CFG bits to set */
 };
@@ -73,7 +82,11 @@ struct xlr_i2c_config {
 struct xlr_i2c_private {
 	struct i2c_adapter adap;
 	u32 __iomem *iobase;
+	int irq;
+	int pos;
+	struct i2c_msg *msg;
 	const struct xlr_i2c_config *cfg;
+	wait_queue_head_t wait;
 	struct clk *clk;
 };
 
@@ -82,6 +95,74 @@ static int xlr_i2c_busy(struct xlr_i2c_private *priv, u32 status)
 	return (status & XLR_I2C_BUS_BUSY) == priv->cfg->status_busy;
 }
 
+static int xlr_i2c_idle(struct xlr_i2c_private *priv)
+{
+	return !xlr_i2c_busy(priv, xlr_i2c_rdreg(priv->iobase, XLR_I2C_STATUS));
+}
+
+static int xlr_i2c_wait(struct xlr_i2c_private *priv, unsigned long timeout)
+{
+	int status;
+	int t;
+
+	t = wait_event_timeout(priv->wait, xlr_i2c_idle(priv),
+				msecs_to_jiffies(timeout));
+	if (!t)
+		return -ETIMEDOUT;
+
+	status = xlr_i2c_rdreg(priv->iobase, XLR_I2C_STATUS);
+
+	return status & XLR_I2C_ACK_ERR ? -EIO : 0;
+}
+
+static void xlr_i2c_tx_irq(struct xlr_i2c_private *priv, u32 status)
+{
+	struct i2c_msg *msg = priv->msg;
+
+	if (status & XLR_I2C_SDOEMPTY)
+		xlr_i2c_wreg(priv->iobase, XLR_I2C_DATAOUT,
+				msg->buf[priv->pos++]);
+}
+
+static void xlr_i2c_rx_irq(struct xlr_i2c_private *priv, u32 status)
+{
+	struct i2c_msg *msg = priv->msg;
+
+	if (status & XLR_I2C_RXRDY)
+		msg->buf[priv->pos++] =
+			xlr_i2c_rdreg(priv->iobase, XLR_I2C_DATAIN);
+}
+
+static irqreturn_t xlr_i2c_irq(int irq, void *dev_id)
+{
+	struct xlr_i2c_private *priv = dev_id;
+	struct i2c_msg *msg = priv->msg;
+	u32 int_stat, status;
+
+	int_stat = xlr_i2c_rdreg(priv->iobase, XLR_I2C_INT_STAT);
+	if (!int_stat)
+		return IRQ_NONE;
+
+	xlr_i2c_wreg(priv->iobase, XLR_I2C_INT_STAT, int_stat);
+
+	if (!msg)
+		return IRQ_HANDLED;
+
+	status = xlr_i2c_rdreg(priv->iobase, XLR_I2C_STATUS);
+
+	if (priv->pos < msg->len) {
+		if (msg->flags & I2C_M_RD)
+			xlr_i2c_rx_irq(priv, status);
+		else
+			xlr_i2c_tx_irq(priv, status);
+	}
+
+	if (!xlr_i2c_busy(priv, status))
+		wake_up(&priv->wait);
+
+	return IRQ_HANDLED;
+}
+
 static int xlr_i2c_tx(struct xlr_i2c_private *priv,  u16 len,
 	u8 *buf, u16 addr)
 {
@@ -119,10 +200,15 @@ static int xlr_i2c_tx(struct xlr_i2c_private *priv,  u16 len,
 		pos = 2;
 	}
 
+	priv->pos = pos;
+
 retry:
 	/* retry can only happen on the first byte */
 	xlr_i2c_wreg(priv->iobase, XLR_I2C_STARTXFR, xfer);
 
+	if (priv->irq > 0)
+		return xlr_i2c_wait(priv, XLR_I2C_TIMEOUT * len);
+
 	while (!timedout) {
 		checktime = jiffies;
 		i2c_status = xlr_i2c_rdreg(priv->iobase, XLR_I2C_STATUS);
@@ -170,6 +256,8 @@ static int xlr_i2c_rx(struct xlr_i2c_private *priv, u16 len, u8 *buf, u16 addr)
 	xlr_i2c_wreg(priv->iobase, XLR_I2C_BYTECNT, len - 1);
 	xlr_i2c_wreg(priv->iobase, XLR_I2C_DEVADDR, addr);
 
+	priv->pos = 0;
+
 	timeout = msecs_to_jiffies(XLR_I2C_TIMEOUT);
 	stoptime = jiffies + timeout;
 	timedout = 0;
@@ -177,6 +265,9 @@ static int xlr_i2c_rx(struct xlr_i2c_private *priv, u16 len, u8 *buf, u16 addr)
 retry:
 	xlr_i2c_wreg(priv->iobase, XLR_I2C_STARTXFR, XLR_I2C_STARTXFR_RD);
 
+	if (priv->irq > 0)
+		return xlr_i2c_wait(priv, XLR_I2C_TIMEOUT * len);
+
 	while (!timedout) {
 		checktime = jiffies;
 		i2c_status = xlr_i2c_rdreg(priv->iobase, XLR_I2C_STATUS);
@@ -221,8 +312,13 @@ static int xlr_i2c_xfer(struct i2c_adapter *adap,
 	if (ret)
 		return ret;
 
+	if (priv->irq)
+		xlr_i2c_wreg(priv->iobase, XLR_I2C_INT_EN, 0xf);
+
+
 	for (i = 0; ret == 0 && i < num; i++) {
 		msg = &msgs[i];
+		priv->msg = msg;
 		if (msg->flags & I2C_M_RD)
 			ret = xlr_i2c_rx(priv, msg->len, &msg->buf[0],
 					msg->addr);
@@ -231,7 +327,11 @@ static int xlr_i2c_xfer(struct i2c_adapter *adap,
 					msg->addr);
 	}
 
+	if (priv->irq)
+		xlr_i2c_wreg(priv->iobase, XLR_I2C_INT_EN, 0);
+
 	clk_disable(priv->clk);
+	priv->msg = NULL;
 
 	return (ret != 0) ? ret : num;
 }
@@ -253,6 +353,7 @@ static const struct xlr_i2c_config xlr_i2c_config_default = {
 };
 
 static const struct xlr_i2c_config xlr_i2c_config_tangox = {
+	.flags		= XLR_I2C_FLAG_IRQ,
 	.status_busy	= 0,
 	.cfg_extra	= 1 << 8,
 };
@@ -274,6 +375,7 @@ static int xlr_i2c_probe(struct platform_device *pdev)
 	unsigned long clk_rate;
 	unsigned long clk_div;
 	u32 busfreq;
+	int irq;
 	int ret;
 
 	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
@@ -291,6 +393,23 @@ static int xlr_i2c_probe(struct platform_device *pdev)
 	if (IS_ERR(priv->iobase))
 		return PTR_ERR(priv->iobase);
 
+	irq = platform_get_irq(pdev, 0);
+
+	if (irq > 0 && (priv->cfg->flags & XLR_I2C_FLAG_IRQ)) {
+		priv->irq = irq;
+
+		xlr_i2c_wreg(priv->iobase, XLR_I2C_INT_EN, 0);
+		xlr_i2c_wreg(priv->iobase, XLR_I2C_INT_STAT, 0xf);
+
+		ret = devm_request_irq(&pdev->dev, priv->irq, xlr_i2c_irq,
+					IRQF_SHARED, dev_name(&pdev->dev),
+					priv);
+		if (ret)
+			return ret;
+
+		init_waitqueue_head(&priv->wait);
+	}
+
 	if (of_property_read_u32(pdev->dev.of_node, "clock-frequency",
 				 &busfreq))
 		busfreq = 100000;
-- 
2.6.2


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

* Re: [PATCH 1/3] i2c: xlr: add support for Sigma Designs controller variant
  2015-11-02  2:03 [PATCH 1/3] i2c: xlr: add support for Sigma Designs controller variant Mans Rullgard
  2015-11-02  2:03 ` [PATCH 2/3] i2c: xlr: fix extra read/write at end of rx transfer Mans Rullgard
  2015-11-02  2:03 ` [PATCH 3/3] i2c: xlr: add interrupt support for Sigma Designs chips Mans Rullgard
@ 2015-11-21 12:02 ` Måns Rullgård
  2015-12-15 12:14 ` Wolfram Sang
  3 siblings, 0 replies; 8+ messages in thread
From: Måns Rullgård @ 2015-11-21 12:02 UTC (permalink / raw
  To: Wolfram Sang; +Cc: linux-i2c, linux-kernel

Mans Rullgard <mans@mansr.com> writes:

> Sigma Designs chips use a variant of this controller with the following
> differences:
>
> - The BUSY bit in the STATUS register is inverted
> - Bit 8 of the CONFIG register must be set
> - The controller can generate interrupts
>
> This patch adds support for the first two of these.  It also calculates
> and sets the correct clock divisor if a clk is provided.  The bus
> frequency is optionally speficied in the device tree node.
>
> Signed-off-by: Mans Rullgard <mans@mansr.com>
> ---
>  drivers/i2c/busses/Kconfig   |  6 ++--
>  drivers/i2c/busses/i2c-xlr.c | 81 +++++++++++++++++++++++++++++++++++++++++---
>  2 files changed, 80 insertions(+), 7 deletions(-)

Any comments on these patches?

-- 
Måns Rullgård
mans@mansr.com

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

* Re: [PATCH 1/3] i2c: xlr: add support for Sigma Designs controller variant
  2015-11-02  2:03 [PATCH 1/3] i2c: xlr: add support for Sigma Designs controller variant Mans Rullgard
                   ` (2 preceding siblings ...)
  2015-11-21 12:02 ` [PATCH 1/3] i2c: xlr: add support for Sigma Designs controller variant Måns Rullgård
@ 2015-12-15 12:14 ` Wolfram Sang
  3 siblings, 0 replies; 8+ messages in thread
From: Wolfram Sang @ 2015-12-15 12:14 UTC (permalink / raw
  To: Mans Rullgard; +Cc: linux-i2c, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 642 bytes --]

On Mon, Nov 02, 2015 at 02:03:36AM +0000, Mans Rullgard wrote:
> Sigma Designs chips use a variant of this controller with the following
> differences:
> 
> - The BUSY bit in the STATUS register is inverted
> - Bit 8 of the CONFIG register must be set
> - The controller can generate interrupts
> 
> This patch adds support for the first two of these.  It also calculates
> and sets the correct clock divisor if a clk is provided.  The bus
> frequency is optionally speficied in the device tree node.

Fixed the "speficied" typo and...

> 
> Signed-off-by: Mans Rullgard <mans@mansr.com>

... applied to for-next, thanks!


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

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

* Re: [PATCH 2/3] i2c: xlr: fix extra read/write at end of rx transfer
  2015-11-02  2:03 ` [PATCH 2/3] i2c: xlr: fix extra read/write at end of rx transfer Mans Rullgard
@ 2015-12-15 12:21   ` Wolfram Sang
  2015-12-15 12:48     ` Måns Rullgård
  0 siblings, 1 reply; 8+ messages in thread
From: Wolfram Sang @ 2015-12-15 12:21 UTC (permalink / raw
  To: Mans Rullgard; +Cc: linux-i2c, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 1073 bytes --]

On Mon, Nov 02, 2015 at 02:03:37AM +0000, Mans Rullgard wrote:
> The BYTECNT register holds the transfer size minus one.  Setting it
> to the correct value requires a dummy read/write only for zero-length
> transfers as it is impossible to request one from the hardware.  If a
> zero-length transfer is requested, changing the length to 1 and setting
> "buf" to a dummy location allows making the main loops less convoluted.
> 
> In other words, this patch makes the driver transfer the number of bytes
> requested unless this is zero, which is not supported by the hardware,
> in which case one byte is transferred instead.

Uh, this is wrong, zero byte should really not transfer anything. We
need to fix that and bail out, so probably something like

	if (!len)
		return -EOPNOTSUPP;

Also, the xlr_func() should mask out I2C_FUNC_SMBUS_QUICK.

Other than that, the patch looks good to me.

Out of curiosity, your first driver had the registers 32bit apart. Now
you can deal with 8bit. Is this configurable on this SoC?

Thanks,

   Wolfram


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

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

* Re: [PATCH 3/3] i2c: xlr: add interrupt support for Sigma Designs chips
  2015-11-02  2:03 ` [PATCH 3/3] i2c: xlr: add interrupt support for Sigma Designs chips Mans Rullgard
@ 2015-12-15 12:23   ` Wolfram Sang
  0 siblings, 0 replies; 8+ messages in thread
From: Wolfram Sang @ 2015-12-15 12:23 UTC (permalink / raw
  To: Mans Rullgard; +Cc: linux-i2c, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 523 bytes --]

On Mon, Nov 02, 2015 at 02:03:38AM +0000, Mans Rullgard wrote:
> The Sigma Designs variant of this controller has the ability to generate
> interrupts.  This is controlled using two additional registers, oddly
> enough overlapping with the defined but unused HDSTATIM.
> 
> This patch adds support for using this feature instead of busy-looping
> if an IRQ is specified.
> 
> Signed-off-by: Mans Rullgard <mans@mansr.com>

Looks good but needs to be resend with the updated patch 2/3.

Thanks,

   Wolfram


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

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

* Re: [PATCH 2/3] i2c: xlr: fix extra read/write at end of rx transfer
  2015-12-15 12:21   ` Wolfram Sang
@ 2015-12-15 12:48     ` Måns Rullgård
  0 siblings, 0 replies; 8+ messages in thread
From: Måns Rullgård @ 2015-12-15 12:48 UTC (permalink / raw
  To: Wolfram Sang; +Cc: linux-i2c, linux-kernel

Wolfram Sang <wsa@the-dreams.de> writes:

> On Mon, Nov 02, 2015 at 02:03:37AM +0000, Mans Rullgard wrote:
>> The BYTECNT register holds the transfer size minus one.  Setting it
>> to the correct value requires a dummy read/write only for zero-length
>> transfers as it is impossible to request one from the hardware.  If a
>> zero-length transfer is requested, changing the length to 1 and setting
>> "buf" to a dummy location allows making the main loops less convoluted.
>> 
>> In other words, this patch makes the driver transfer the number of bytes
>> requested unless this is zero, which is not supported by the hardware,
>> in which case one byte is transferred instead.
>
> Uh, this is wrong, zero byte should really not transfer anything. We
> need to fix that and bail out, so probably something like
>
> 	if (!len)
> 		return -EOPNOTSUPP;

So the existing driver is wrong to allow it.  Makes sense to drop that.

> Also, the xlr_func() should mask out I2C_FUNC_SMBUS_QUICK.

OK.

> Other than that, the patch looks good to me.
>
> Out of curiosity, your first driver had the registers 32bit apart. Now
> you can deal with 8bit. Is this configurable on this SoC?

It's all 32 bits.  The XLR driver uses a u32 * to access the registers.

-- 
Måns Rullgård

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

end of thread, other threads:[~2015-12-15 12:48 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-11-02  2:03 [PATCH 1/3] i2c: xlr: add support for Sigma Designs controller variant Mans Rullgard
2015-11-02  2:03 ` [PATCH 2/3] i2c: xlr: fix extra read/write at end of rx transfer Mans Rullgard
2015-12-15 12:21   ` Wolfram Sang
2015-12-15 12:48     ` Måns Rullgård
2015-11-02  2:03 ` [PATCH 3/3] i2c: xlr: add interrupt support for Sigma Designs chips Mans Rullgard
2015-12-15 12:23   ` Wolfram Sang
2015-11-21 12:02 ` [PATCH 1/3] i2c: xlr: add support for Sigma Designs controller variant Måns Rullgård
2015-12-15 12:14 ` Wolfram Sang

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