[PATCH v3 0/2] Qualcomm Universal Peripheral (QUP) I2C controller

[PATCH v3 0/2] Qualcomm Universal Peripheral (QUP) I2C controller

[Bjorn Andersson]

This third revision of the QUP I2C driver series comes with quite a bit of
cleanup and corrections. Most notably is the removal of the "magic" delay loops
and bug fixes related to larger than 32 byte reads and writes.

Special thanks to Andy Gross for helping answering questions regarding the
inner working of this block.

Regards,
Bjorn

Changes from v3:
 - Simplified interrupt handler
 - Corrected the state transition poll timeout
 - Refactored state transition code
 - Refactored the polling functions waiting for transfers to finish
 - Made the write fifo fill function care if there's space
 - Corrected programmed length on writes
 - Made block read and block write work
 - Removed data duplicates from qup_i2c_dev
 - Changed timeout to HZ, to give room for clock stretching
 - Properly reject reads over 256 bytes, as limited by HW
 - Dropped reinitialization of completions
 - Made sure to not re-initiate reads for every block read
 - Added QUP version number to compatible

Changes from v2:
 - Removed unused variables and includes
 - Corrected read logic in irq handler
 - Made the polling loop in qup_i2c_poll_state() less arbitrary
 - Only building suspend/resume if CONFIG_PM_SLEEP

Changes from v1:
 - Cleaned up device tree binding example.
 - Refrased device tree bindings.
 - Following changes in the i2c framework.
 - Use the core clock to calculate divider for the bus clock, instead of
   explicitly setting it.
 - Remove explicit pinctrl settting.
 - Split/renamed qup_i2c_enable(bool) into enable/disable functions.
 - Return value was overwritten on error in write_one/read_one.
 - Initialize the i2c core every time, so that we actually can execute
   more than 1 transmission per xfer.

Bjorn Andersson (1):
  i2c: New bus driver for the Qualcomm QUP I2C controller

Ivan T. Ivanov (1):
  i2c: qup: Add device tree bindings information

 .../devicetree/bindings/i2c/qcom,i2c-qup.txt       |   44 ++
 drivers/i2c/busses/Kconfig                         |   10 +
 drivers/i2c/busses/Makefile                        |    1 +
 drivers/i2c/busses/i2c-qup.c                       |  772 ++++++++++++++++++++
 4 files changed, 827 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/i2c/qcom,i2c-qup.txt
 create mode 100644 drivers/i2c/busses/i2c-qup.c

-- 
1.7.9.5

[PATCH v3 1/2] i2c: qup: Add device tree bindings information

[Bjorn Andersson]

From: "Ivan T. Ivanov" <iivanov-NEYub+7Iv8PQT0dZR+AlfA@public.gmane.org>

The Qualcomm Universal Peripherial (QUP) wraps I2C mini-core and
provide input and output FIFO's for it. I2C controller can operate
as master with supported bus speeds of 100Kbps and 400Kbps.

Signed-off-by: Ivan T. Ivanov <iivanov-NEYub+7Iv8PQT0dZR+AlfA@public.gmane.org>
[bjorn: reformulated part of binding description
        added version to compatible
        cleaned up example]
Signed-off-by: Bjorn Andersson <bjorn.andersson-/MT0OVThwyLZJqsBc5GL+g@public.gmane.org>
---
 .../devicetree/bindings/i2c/qcom,i2c-qup.txt       |   44 ++++++++++++++++++++
 1 file changed, 44 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/i2c/qcom,i2c-qup.txt

diff --git a/Documentation/devicetree/bindings/i2c/qcom,i2c-qup.txt b/Documentation/devicetree/bindings/i2c/qcom,i2c-qup.txt
new file mode 100644
index 0000000..38d505f
--- /dev/null
+++ b/Documentation/devicetree/bindings/i2c/qcom,i2c-qup.txt
@@ -0,0 +1,44 @@
+Qualcomm Universal Peripheral (QUP) I2C controller
+
+Required properties:
+ - compatible: Should be
+		"qcom,i2c-qup-v1.1.1" for 8660, 8960 and 8064.
+		"qcom,i2c-qup-v2.1.1" for 8974 v1.
+		"qcom,i2c-qup-v2.2.1" for 8974 v2 and later.
+ - reg: Should contain QUP register address and length.
+ - interrupts: Should contain I2C interrupt.
+
+ - clocks: Should contain the core clock and the AHB clock.
+ - clock-names: Should be "core" for the core clock and "iface" for the
+                AHB clock.
+
+ - #address-cells: Should be <1> Address cells for i2c device address
+ - #size-cells: Should be <0> as i2c addresses have no size component
+
+Optional properties:
+ - clock-frequency: Should specify the desired i2c bus clock frequency in Hz,
+                    defaults to 100kHz if omitted.
+
+Child nodes should conform to i2c bus binding.
+
+Example:
+
+ i2c@f9924000 {
+ 	compatible = "qcom,i2c-qup-v2.2.1";
+ 	reg = <0xf9924000 0x1000>;
+ 	interrupts = <0 96 0>;
+
+ 	clocks = <&gcc GCC_BLSP1_QUP2_I2C_APPS_CLK>, <&gcc GCC_BLSP1_AHB_CLK>;
+ 	clock-names = "core", "iface";
+
+ 	clock-frequency = <355000>;
+
+ 	#address-cells = <1>;
+ 	#size-cells = <0>;
+
+ 	dummy@60 {
+ 		compatible = "dummy";
+ 		reg = <0x60>;
+ 	};
+ };
+
-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 1/2] i2c: qup: Add device tree bindings information

[Mark Rutland]

On Fri, Feb 21, 2014 at 12:38:09AM +0000, Bjorn Andersson wrote:
> From: "Ivan T. Ivanov" <iivanov@mm-sol.com>
> 
> The Qualcomm Universal Peripherial (QUP) wraps I2C mini-core and
> provide input and output FIFO's for it. I2C controller can operate
> as master with supported bus speeds of 100Kbps and 400Kbps.
> 
> Signed-off-by: Ivan T. Ivanov <iivanov@mm-sol.com>
> [bjorn: reformulated part of binding description
>         added version to compatible
>         cleaned up example]
> Signed-off-by: Bjorn Andersson <bjorn.andersson@sonymobile.com>
> ---
>  .../devicetree/bindings/i2c/qcom,i2c-qup.txt       |   44 ++++++++++++++++++++
>  1 file changed, 44 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/i2c/qcom,i2c-qup.txt
> 
> diff --git a/Documentation/devicetree/bindings/i2c/qcom,i2c-qup.txt b/Documentation/devicetree/bindings/i2c/qcom,i2c-qup.txt
> new file mode 100644
> index 0000000..38d505f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/i2c/qcom,i2c-qup.txt
> @@ -0,0 +1,44 @@
> +Qualcomm Universal Peripheral (QUP) I2C controller
> +
> +Required properties:
> + - compatible: Should be
> +		"qcom,i2c-qup-v1.1.1" for 8660, 8960 and 8064.
> +		"qcom,i2c-qup-v2.1.1" for 8974 v1.
> +		"qcom,i2c-qup-v2.2.1" for 8974 v2 and later.
> + - reg: Should contain QUP register address and length.
> + - interrupts: Should contain I2C interrupt.
> +
> + - clocks: Should contain the core clock and the AHB clock.
> + - clock-names: Should be "core" for the core clock and "iface" for the
> +                AHB clock.

Could you please describe clocks in terms of clock-names? It makes the
relationship clearer, and avoids redundancy (which makes extending the
bindnig easier):

- clocks: A list of phandles + clock-specifiers, one for each entry in
  clock-names.
- clock-names: should contain:
  * "core" for the core clock
  * "iface" for the AHB clock

Cheers,
Mark.

> +
> + - #address-cells: Should be <1> Address cells for i2c device address
> + - #size-cells: Should be <0> as i2c addresses have no size component
> +
> +Optional properties:
> + - clock-frequency: Should specify the desired i2c bus clock frequency in Hz,
> +                    defaults to 100kHz if omitted.
> +
> +Child nodes should conform to i2c bus binding.
> +
> +Example:
> +
> + i2c@f9924000 {
> + 	compatible = "qcom,i2c-qup-v2.2.1";
> + 	reg = <0xf9924000 0x1000>;
> + 	interrupts = <0 96 0>;
> +
> + 	clocks = <&gcc GCC_BLSP1_QUP2_I2C_APPS_CLK>, <&gcc GCC_BLSP1_AHB_CLK>;
> + 	clock-names = "core", "iface";
> +
> + 	clock-frequency = <355000>;
> +
> + 	#address-cells = <1>;
> + 	#size-cells = <0>;
> +
> + 	dummy@60 {
> + 		compatible = "dummy";
> + 		reg = <0x60>;
> + 	};
> + };
> +
> -- 
> 1.7.9.5
> 
> 

[PATCH v3 2/2] i2c: New bus driver for the Qualcomm QUP I2C controller

[Bjorn Andersson]

This bus driver supports the QUP i2c hardware controller in the Qualcomm SOCs.
The Qualcomm Universal Peripheral Engine (QUP) is a general purpose data path
engine with input/output FIFOs and an embedded i2c mini-core. The driver
supports FIFO mode (for low bandwidth applications) and block mode (interrupt
generated for each block-size data transfer).

Cc: Andy Gross <agross@codeaurora.org>
Cc: Stephen Boyd <sboyd@codeaurora.org>
Signed-off-by: Ivan T. Ivanov <iivanov@mm-sol.com>
Signed-off-by: Bjorn Andersson <bjorn.andersson@sonymobile.com>
---
 drivers/i2c/busses/Kconfig   |   10 +
 drivers/i2c/busses/Makefile  |    1 +
 drivers/i2c/busses/i2c-qup.c |  772 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 783 insertions(+)
 create mode 100644 drivers/i2c/busses/i2c-qup.c

diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index f5ed031..403e196 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -648,6 +648,16 @@ config I2C_PXA_SLAVE
 	  is necessary for systems where the PXA may be a target on the
 	  I2C bus.
 
+config I2C_QUP
+	tristate "Qualcomm QUP based I2C controller"
+	depends on ARCH_MSM
+	help
+	  If you say yes to this option, support will be included for the
+	  built-in I2C interface on the Qualcomm SoCs.
+
+	  This driver can also be built as a module.  If so, the module
+	  will be called i2c-qup.
+
 config I2C_RIIC
 	tristate "Renesas RIIC adapter"
 	depends on ARCH_SHMOBILE || COMPILE_TEST
diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
index a08931f..bf2257b 100644
--- a/drivers/i2c/busses/Makefile
+++ b/drivers/i2c/busses/Makefile
@@ -63,6 +63,7 @@ obj-$(CONFIG_I2C_PNX)		+= i2c-pnx.o
 obj-$(CONFIG_I2C_PUV3)		+= i2c-puv3.o
 obj-$(CONFIG_I2C_PXA)		+= i2c-pxa.o
 obj-$(CONFIG_I2C_PXA_PCI)	+= i2c-pxa-pci.o
+obj-$(CONFIG_I2C_QUP)		+= i2c-qup.o
 obj-$(CONFIG_I2C_RIIC)		+= i2c-riic.o
 obj-$(CONFIG_I2C_S3C2410)	+= i2c-s3c2410.o
 obj-$(CONFIG_I2C_S6000)		+= i2c-s6000.o
diff --git a/drivers/i2c/busses/i2c-qup.c b/drivers/i2c/busses/i2c-qup.c
new file mode 100644
index 0000000..a2dfd57
--- /dev/null
+++ b/drivers/i2c/busses/i2c-qup.c
@@ -0,0 +1,772 @@
+/*
+ * Copyright (c) 2009-2013, The Linux Foundation. All rights reserved.
+ * Copyright (c) 2014, Sony Mobile Communications AB.
+ *
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 and
+ * only version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/err.h>
+#include <linux/i2c.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+
+/* QUP Registers */
+#define QUP_CONFIG		0x000
+#define QUP_STATE		0x004
+#define QUP_IO_MODE		0x008
+#define QUP_SW_RESET		0x00c
+#define QUP_OPERATIONAL		0x018
+#define QUP_ERROR_FLAGS		0x01c
+#define QUP_ERROR_FLAGS_EN	0x020
+#define QUP_HW_VERSION		0x030
+#define QUP_MX_OUTPUT_CNT	0x100
+#define QUP_OUT_FIFO_BASE	0x110
+#define QUP_MX_WRITE_CNT	0x150
+#define QUP_MX_INPUT_CNT	0x200
+#define QUP_MX_READ_CNT		0x208
+#define QUP_IN_FIFO_BASE	0x218
+#define QUP_I2C_CLK_CTL		0x400
+#define QUP_I2C_STATUS		0x404
+
+/* QUP States and reset values */
+#define QUP_RESET_STATE		0
+#define QUP_RUN_STATE		1
+#define QUP_PAUSE_STATE		3
+#define QUP_STATE_MASK		3
+
+#define QUP_STATE_VALID		BIT(2)
+#define QUP_I2C_MAST_GEN	BIT(4)
+
+#define QUP_OPERATIONAL_RESET	0x000ff0
+#define QUP_I2C_STATUS_RESET	0xfffffc
+
+/* QUP OPERATIONAL FLAGS */
+#define QUP_I2C_NACK_FLAG	BIT(3)
+#define QUP_OUT_NOT_EMPTY	BIT(4)
+#define QUP_IN_NOT_EMPTY	BIT(5)
+#define QUP_OUT_FULL		BIT(6)
+#define QUP_OUT_SVC_FLAG	BIT(8)
+#define QUP_IN_SVC_FLAG		BIT(9)
+#define QUP_MX_OUTPUT_DONE	BIT(10)
+#define QUP_MX_INPUT_DONE	BIT(11)
+
+/* I2C mini core related values */
+#define QUP_CLOCK_AUTO_GATE	BIT(13)
+#define I2C_MINI_CORE		(2 << 8)
+#define I2C_N_VAL		15
+/* Most significant word offset in FIFO port */
+#define QUP_MSW_SHIFT		(I2C_N_VAL + 1)
+
+/* Packing/Unpacking words in FIFOs, and IO modes */
+#define QUP_OUTPUT_BLK_MODE	(1 << 10)
+#define QUP_INPUT_BLK_MODE	(1 << 12)
+#define QUP_UNPACK_EN		BIT(14)
+#define QUP_PACK_EN		BIT(15)
+
+#define QUP_REPACK_EN		(QUP_UNPACK_EN | QUP_PACK_EN)
+
+#define QUP_OUTPUT_BLOCK_SIZE(x)(((x) >> 0) & 0x03)
+#define QUP_OUTPUT_FIFO_SIZE(x)	(((x) >> 2) & 0x07)
+#define QUP_INPUT_BLOCK_SIZE(x)	(((x) >> 5) & 0x03)
+#define QUP_INPUT_FIFO_SIZE(x)	(((x) >> 7) & 0x07)
+
+/* QUP tags */
+#define QUP_TAG_START		(1 << 8)
+#define QUP_TAG_DATA		(2 << 8)
+#define QUP_TAG_STOP		(3 << 8)
+#define QUP_TAG_REC		(4 << 8)
+
+/* Status, Error flags */
+#define I2C_STATUS_WR_BUFFER_FULL	BIT(0)
+#define I2C_STATUS_BUS_ACTIVE		BIT(8)
+#define I2C_STATUS_ERROR_MASK		0x38000fc
+#define QUP_STATUS_ERROR_FLAGS		0x7c
+
+#define QUP_READ_LIMIT			256
+
+struct qup_i2c_dev {
+	struct device		*dev;
+	void __iomem		*base;
+	int			irq;
+	struct clk		*clk;
+	struct clk		*pclk;
+	struct i2c_adapter	adap;
+
+	int			clk_ctl;
+	int			out_fifo_sz;
+	int			in_fifo_sz;
+	int			out_blk_sz;
+	int			in_blk_sz;
+
+	unsigned long		one_byte_t;
+
+	struct i2c_msg		*msg;
+	/* Current posion in user message buffer */
+	int			pos;
+	/* I2C protocol errors */
+	u32			bus_err;
+	/* QUP core errors */
+	u32			qup_err;
+
+	struct completion	xfer;
+};
+
+static irqreturn_t qup_i2c_interrupt(int irq, void *dev)
+{
+	struct qup_i2c_dev *qup = dev;
+	u32 bus_err;
+	u32 qup_err;
+	u32 opflags;
+
+	bus_err = readl(qup->base + QUP_I2C_STATUS);
+	qup_err = readl(qup->base + QUP_ERROR_FLAGS);
+	opflags = readl(qup->base + QUP_OPERATIONAL);
+
+	if (!qup->msg) {
+		/* Clear Error interrupt */
+		writel(QUP_RESET_STATE, qup->base + QUP_STATE);
+		return IRQ_HANDLED;
+	}
+
+	bus_err &= I2C_STATUS_ERROR_MASK;
+	qup_err &= QUP_STATUS_ERROR_FLAGS;
+
+	if (qup_err) {
+		/* Clear Error interrupt */
+		writel(qup_err, qup->base + QUP_ERROR_FLAGS);
+		goto done;
+	}
+
+	if (bus_err) {
+		/* Clear Error interrupt */
+		writel(QUP_RESET_STATE, qup->base + QUP_STATE);
+		goto done;
+	}
+
+	if (opflags & QUP_IN_SVC_FLAG)
+		writel(QUP_IN_SVC_FLAG, qup->base + QUP_OPERATIONAL);
+
+	if (opflags & QUP_OUT_SVC_FLAG)
+		writel(QUP_OUT_SVC_FLAG, qup->base + QUP_OPERATIONAL);
+
+done:
+	qup->qup_err = qup_err;
+	qup->bus_err = bus_err;
+	complete(&qup->xfer);
+	return IRQ_HANDLED;
+}
+
+static int qup_i2c_poll_state_mask(struct qup_i2c_dev *qup,
+				   u32 req_state, u32 req_mask)
+{
+	int retries = 1;
+	u32 state;
+
+	/*
+	 * State transition takes 3 AHB clocks cycles + 3 I2C master clock
+	 * cycles. So retry once after a 1uS delay.
+	 */
+	do {
+		state = readl(qup->base + QUP_STATE);
+
+		if (state & QUP_STATE_VALID &&
+		    (state & req_mask) == req_state)
+			return 0;
+
+		udelay(1);
+	} while (retries--);
+
+	return -ETIMEDOUT;
+}
+
+static int qup_i2c_poll_state(struct qup_i2c_dev *qup, u32 req_state)
+{
+	return qup_i2c_poll_state_mask(qup, req_state, QUP_STATE_MASK);
+}
+
+static int qup_i2c_poll_state_valid(struct qup_i2c_dev *qup)
+{
+	return qup_i2c_poll_state_mask(qup, 0, 0);
+}
+
+static int qup_i2c_poll_state_i2c_master(struct qup_i2c_dev *qup)
+{
+	return qup_i2c_poll_state_mask(qup, QUP_I2C_MAST_GEN, QUP_I2C_MAST_GEN);
+}
+
+static int qup_i2c_change_state(struct qup_i2c_dev *qup, u32 state)
+{
+	if (qup_i2c_poll_state_valid(qup) != 0)
+		return -EIO;
+
+	writel(state, qup->base + QUP_STATE);
+
+	if (qup_i2c_poll_state(qup, state) != 0)
+		return -EIO;
+	return 0;
+}
+
+static int qup_i2c_wait_writeready(struct qup_i2c_dev *qup)
+{
+	unsigned long timeout;
+	u32 opflags;
+	u32 status;
+
+	timeout = jiffies + HZ;
+
+	for (;;) {
+		opflags = readl(qup->base + QUP_OPERATIONAL);
+		status = readl(qup->base + QUP_I2C_STATUS);
+
+		if (!(opflags & QUP_OUT_NOT_EMPTY) &&
+		    !(status & I2C_STATUS_BUS_ACTIVE))
+			return 0;
+
+		if (time_after(jiffies, timeout))
+			return -ETIMEDOUT;
+
+		usleep_range(qup->one_byte_t, qup->one_byte_t * 2);
+	}
+}
+
+static void qup_i2c_set_write_mode(struct qup_i2c_dev *qup, struct i2c_msg *msg)
+{
+	/* Number of entries to shift out, including the start */
+	int total = msg->len + 1;
+
+	if (total < qup->out_fifo_sz) {
+		/* FIFO mode */
+		writel(QUP_REPACK_EN, qup->base + QUP_IO_MODE);
+		writel(total, qup->base + QUP_MX_WRITE_CNT);
+	} else {
+		/* BLOCK mode (transfer data on chunks) */
+		writel(QUP_OUTPUT_BLK_MODE | QUP_REPACK_EN,
+		       qup->base + QUP_IO_MODE);
+		writel(total, qup->base + QUP_MX_OUTPUT_CNT);
+	}
+}
+
+static void qup_i2c_issue_write(struct qup_i2c_dev *qup, struct i2c_msg *msg)
+{
+	u32 addr = msg->addr << 1;
+	u32 qup_tag;
+	u32 opflags;
+	int idx;
+	u32 val;
+
+	if (qup->pos == 0) {
+		val = QUP_TAG_START | addr;
+		idx = 1;
+	} else {
+		val = 0;
+		idx = 0;
+	}
+
+	while (qup->pos < msg->len) {
+		/* Check that there's space in the FIFO for our pair */
+		opflags = readl(qup->base + QUP_OPERATIONAL);
+		if (opflags & QUP_OUT_FULL)
+			break;
+
+		if (qup->pos == msg->len - 1)
+			qup_tag = QUP_TAG_STOP;
+		else
+			qup_tag = QUP_TAG_DATA;
+
+		if (idx & 1)
+			val |= (qup_tag | msg->buf[qup->pos]) << QUP_MSW_SHIFT;
+		else
+			val = qup_tag | msg->buf[qup->pos];
+
+		/* Write out the pair and the last odd value */
+		if (idx & 1 || qup->pos == msg->len - 1)
+			writel(val, qup->base + QUP_OUT_FIFO_BASE);
+
+		qup->pos++;
+		idx++;
+	}
+}
+
+static int qup_i2c_write_one(struct qup_i2c_dev *qup, struct i2c_msg *msg)
+{
+	unsigned long left;
+	int ret;
+
+	qup->msg = msg;
+	qup->pos = 0;
+
+	enable_irq(qup->irq);
+
+	qup_i2c_set_write_mode(qup, msg);
+
+	ret = qup_i2c_change_state(qup, QUP_RUN_STATE);
+	if (ret)
+		goto err;
+
+	writel(qup->clk_ctl, qup->base + QUP_I2C_CLK_CTL);
+
+	do {
+		ret = qup_i2c_change_state(qup, QUP_PAUSE_STATE);
+		if (ret)
+			goto err;
+
+		qup_i2c_issue_write(qup, msg);
+
+		ret = qup_i2c_change_state(qup, QUP_RUN_STATE);
+		if (ret)
+			goto err;
+
+		left = wait_for_completion_timeout(&qup->xfer, HZ);
+		if (!left) {
+			writel(1, qup->base + QUP_SW_RESET);
+			ret = -ETIMEDOUT;
+			goto err;
+		}
+
+		if (qup->bus_err || qup->qup_err) {
+			if (qup->bus_err & QUP_I2C_NACK_FLAG)
+				dev_err(qup->dev, "NACK from %x\n", msg->addr);
+			ret = -EIO;
+			goto err;
+		}
+	} while (qup->pos < msg->len);
+
+	/* Wait for the outstanding data in the fifo to drain */
+	ret = qup_i2c_wait_writeready(qup);
+
+err:
+	disable_irq(qup->irq);
+	qup->msg = NULL;
+
+	return ret;
+}
+
+static void qup_i2c_set_read_mode(struct qup_i2c_dev *qup, int len)
+{
+	if (len < qup->in_fifo_sz) {
+		/* FIFO mode */
+		writel(QUP_REPACK_EN, qup->base + QUP_IO_MODE);
+		writel(len, qup->base + QUP_MX_READ_CNT);
+	} else {
+		/* BLOCK mode (transfer data on chunks) */
+		writel(QUP_INPUT_BLK_MODE | QUP_REPACK_EN,
+		       qup->base + QUP_IO_MODE);
+		writel(len, qup->base + QUP_MX_INPUT_CNT);
+	}
+}
+
+static void qup_i2c_issue_read(struct qup_i2c_dev *qup, struct i2c_msg *msg)
+{
+	u32 addr, len, val;
+
+	addr = (msg->addr << 1) | 1;
+
+	/* 0 is used to specify a length 256 (QUP_READ_LIMIT) */
+	len = (msg->len == QUP_READ_LIMIT) ? 0 : msg->len;
+
+	val = ((QUP_TAG_REC | len) << QUP_MSW_SHIFT) | QUP_TAG_START | addr;
+	writel(val, qup->base + QUP_OUT_FIFO_BASE);
+}
+
+
+static void qup_i2c_read_fifo(struct qup_i2c_dev *qup, struct i2c_msg *msg)
+{
+	u32 opflags;
+	u32 val = 0;
+	int idx;
+
+	for (idx = 0; qup->pos < msg->len; idx++) {
+		if ((idx & 1) == 0) {
+			/* Check that FIFO have data */
+			opflags = readl(qup->base + QUP_OPERATIONAL);
+			if (!(opflags & QUP_IN_NOT_EMPTY))
+				break;
+
+			/* Reading 2 words at time */
+			val = readl(qup->base + QUP_IN_FIFO_BASE);
+
+			msg->buf[qup->pos++] = val & 0xFF;
+		} else {
+			msg->buf[qup->pos++] = val >> QUP_MSW_SHIFT;
+		}
+	}
+}
+
+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;
+
+	enable_irq(qup->irq);
+
+	qup_i2c_set_read_mode(qup, msg->len);
+
+	ret = qup_i2c_change_state(qup, QUP_RUN_STATE);
+	if (ret)
+		goto err;
+
+	writel(qup->clk_ctl, qup->base + QUP_I2C_CLK_CTL);
+
+	ret = qup_i2c_change_state(qup, QUP_PAUSE_STATE);
+	if (ret)
+		goto err;
+
+	qup_i2c_issue_read(qup, msg);
+
+	ret = qup_i2c_change_state(qup, QUP_RUN_STATE);
+	if (ret)
+		goto err;
+
+	do {
+		left = wait_for_completion_timeout(&qup->xfer, HZ);
+		if (!left) {
+			writel(1, qup->base + QUP_SW_RESET);
+			ret = -ETIMEDOUT;
+			goto err;
+		}
+
+		if (qup->bus_err || qup->qup_err) {
+			if (qup->bus_err & QUP_I2C_NACK_FLAG)
+				dev_err(qup->dev, "NACK from %x\n", msg->addr);
+			ret = -EIO;
+			goto err;
+		}
+
+		qup_i2c_read_fifo(qup, msg);
+	} while (qup->pos < msg->len);
+
+err:
+	disable_irq(qup->irq);
+	qup->msg = NULL;
+
+	return ret;
+}
+
+static int qup_i2c_xfer(struct i2c_adapter *adap,
+			struct i2c_msg msgs[],
+			int num)
+{
+	struct qup_i2c_dev *qup = i2c_get_adapdata(adap);
+	int ret, idx;
+
+	ret = pm_runtime_get_sync(qup->dev);
+	if (IS_ERR_VALUE(ret))
+		goto out;
+
+	writel(1, qup->base + QUP_SW_RESET);
+	ret = qup_i2c_poll_state(qup, QUP_RESET_STATE);
+	if (ret)
+		goto out;
+
+	/* Configure QUP as I2C mini core */
+	writel(I2C_MINI_CORE | I2C_N_VAL, qup->base + QUP_CONFIG);
+
+	for (idx = 0; idx < num; idx++) {
+		if (msgs[idx].len == 0) {
+			ret = -EINVAL;
+			goto out;
+		}
+
+		if (qup_i2c_poll_state_i2c_master(qup)) {
+			ret = -EIO;
+			goto out;
+		}
+
+		if (msgs[idx].flags & I2C_M_RD)
+			ret = qup_i2c_read_one(qup, &msgs[idx]);
+		else
+			ret = qup_i2c_write_one(qup, &msgs[idx]);
+
+		if (ret)
+			break;
+
+		ret = qup_i2c_change_state(qup, QUP_RESET_STATE);
+		if (ret)
+			break;
+	}
+
+	if (ret == 0)
+		ret = num;
+out:
+
+	pm_runtime_mark_last_busy(qup->dev);
+	pm_runtime_put_autosuspend(qup->dev);
+
+	return ret;
+}
+
+static u32 qup_i2c_func(struct i2c_adapter *adap)
+{
+	return I2C_FUNC_I2C | (I2C_FUNC_SMBUS_EMUL & ~I2C_FUNC_SMBUS_QUICK);
+}
+
+static const struct i2c_algorithm qup_i2c_algo = {
+	.master_xfer	= qup_i2c_xfer,
+	.functionality	= qup_i2c_func,
+};
+
+static void qup_i2c_enable_clocks(struct qup_i2c_dev *qup)
+{
+	clk_prepare_enable(qup->clk);
+	clk_prepare_enable(qup->pclk);
+}
+
+static void qup_i2c_disable_clocks(struct qup_i2c_dev *qup)
+{
+	u32 config;
+
+	qup_i2c_change_state(qup, QUP_RESET_STATE);
+	clk_disable_unprepare(qup->clk);
+	config = readl(qup->base + QUP_CONFIG);
+	config |= QUP_CLOCK_AUTO_GATE;
+	writel(config, qup->base + QUP_CONFIG);
+	clk_disable_unprepare(qup->pclk);
+}
+
+static int qup_i2c_probe(struct platform_device *pdev)
+{
+	static const int blk_sizes[] = {4, 16, 32};
+	struct device_node *node = pdev->dev.of_node;
+	struct qup_i2c_dev *qup;
+	unsigned long one_bit_t;
+	struct resource *res;
+	u32 val, io_mode, hw_ver, size;
+	int ret, fs_div, hs_div;
+	int src_clk_freq;
+	int clk_freq;
+
+	qup = devm_kzalloc(&pdev->dev, sizeof(*qup), GFP_KERNEL);
+	if (!qup)
+		return -ENOMEM;
+
+	qup->dev = &pdev->dev;
+	init_completion(&qup->xfer);
+	platform_set_drvdata(pdev, qup);
+
+	clk_freq = 100000;
+	if (!of_property_read_u32(node, "clock-frequency", &val))
+		clk_freq = val;
+
+	/* We support frequencies up to FAST Mode (400KHz) */
+	if (clk_freq <= 0 || clk_freq > 400000) {
+		dev_err(qup->dev, "clock frequency not supported %d\n",
+			clk_freq);
+		return -EIO;
+	}
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	qup->base = devm_ioremap_resource(qup->dev, res);
+	if (IS_ERR(qup->base))
+		return PTR_ERR(qup->base);
+
+	qup->irq = platform_get_irq(pdev, 0);
+	if (qup->irq < 0) {
+		dev_err(qup->dev, "No IRQ defined\n");
+		return qup->irq;
+	}
+
+	qup->clk = devm_clk_get(qup->dev, "core");
+	if (IS_ERR(qup->clk)) {
+		dev_err(qup->dev, "Could not get core clock\n");
+		return PTR_ERR(qup->clk);
+	}
+
+	qup->pclk = devm_clk_get(qup->dev, "iface");
+	if (IS_ERR(qup->pclk)) {
+		dev_err(qup->dev, "Could not get iface clock\n");
+		return PTR_ERR(qup->pclk);
+	}
+
+	qup_i2c_enable_clocks(qup);
+
+	/*
+	 * Bootloaders might leave a pending interrupt on certain QUP's,
+	 * so we reset the core before registering for interrupts.
+	 */
+	writel(1, qup->base + QUP_SW_RESET);
+	ret = qup_i2c_poll_state_valid(qup);
+	if (ret)
+		goto fail;
+
+	ret = devm_request_irq(qup->dev, qup->irq, qup_i2c_interrupt,
+			       IRQF_TRIGGER_HIGH, "i2c_qup", qup);
+	if (ret) {
+		dev_err(qup->dev, "Request %d IRQ failed\n", qup->irq);
+		goto fail;
+	}
+	disable_irq(qup->irq);
+
+	hw_ver = readl(qup->base + QUP_HW_VERSION);
+	dev_dbg(qup->dev, "Revision %x\n", hw_ver);
+
+	io_mode = readl(qup->base + QUP_IO_MODE);
+
+	/*
+	 * The block/fifo size w.r.t. 'actual data' is 1/2 due to 'tag'
+	 * associated with each byte written/received
+	 */
+	size = QUP_OUTPUT_BLOCK_SIZE(io_mode);
+	if (size > ARRAY_SIZE(blk_sizes))
+		return -EIO;
+	qup->out_blk_sz = blk_sizes[size] / 2;
+
+	size = QUP_INPUT_BLOCK_SIZE(io_mode);
+	if (size > ARRAY_SIZE(blk_sizes))
+		return -EIO;
+	qup->in_blk_sz = blk_sizes[size] / 2;
+
+	size = QUP_OUTPUT_FIFO_SIZE(io_mode);
+	qup->out_fifo_sz = qup->out_blk_sz * (2 << size);
+
+	size = QUP_INPUT_FIFO_SIZE(io_mode);
+	qup->in_fifo_sz = qup->in_blk_sz * (2 << size);
+
+	src_clk_freq = clk_get_rate(qup->clk);
+	fs_div = ((src_clk_freq / clk_freq) / 2) - 3;
+	hs_div = 3;
+	qup->clk_ctl = (hs_div << 8) | (fs_div & 0xff);
+
+	/*
+	 * Time it takes for a byte to be clocked out on the bus.
+	 * Each byte takes 9 clock cycles (8 bits + 1 ack).
+	 */
+	one_bit_t = (USEC_PER_SEC / clk_freq) + 1;
+	qup->one_byte_t = one_bit_t * 9;
+
+	dev_dbg(qup->dev, "IN:block:%d, fifo:%d, OUT:block:%d, fifo:%d\n",
+		qup->in_blk_sz, qup->in_fifo_sz,
+		qup->out_blk_sz, qup->out_fifo_sz);
+
+	i2c_set_adapdata(&qup->adap, qup);
+	qup->adap.algo = &qup_i2c_algo;
+	qup->adap.nr = pdev->id;
+	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));
+
+	ret = i2c_add_numbered_adapter(&qup->adap);
+	if (ret)
+		goto fail;
+
+	pm_runtime_set_autosuspend_delay(qup->dev, MSEC_PER_SEC);
+	pm_runtime_use_autosuspend(qup->dev);
+	pm_runtime_enable(qup->dev);
+	return 0;
+
+fail:
+	qup_i2c_disable_clocks(qup);
+	return ret;
+}
+
+static int qup_i2c_remove(struct platform_device *pdev)
+{
+	struct qup_i2c_dev *qup = platform_get_drvdata(pdev);
+
+	disable_irq(qup->irq);
+	qup_i2c_disable_clocks(qup);
+	i2c_del_adapter(&qup->adap);
+	pm_runtime_disable(qup->dev);
+	pm_runtime_set_suspended(qup->dev);
+	return 0;
+}
+
+#ifdef CONFIG_PM
+static int qup_i2c_pm_suspend_runtime(struct device *device)
+{
+	struct qup_i2c_dev *qup = dev_get_drvdata(device);
+
+	dev_dbg(device, "pm_runtime: suspending...\n");
+	qup_i2c_disable_clocks(qup);
+	return 0;
+}
+
+static int qup_i2c_pm_resume_runtime(struct device *device)
+{
+	struct qup_i2c_dev *qup = dev_get_drvdata(device);
+
+	dev_dbg(device, "pm_runtime: resuming...\n");
+	qup_i2c_enable_clocks(qup);
+	return 0;
+}
+#endif
+
+#ifdef CONFIG_PM_SLEEP
+static int qup_i2c_suspend(struct device *device)
+{
+	dev_dbg(device, "system suspend");
+	qup_i2c_pm_suspend_runtime(device);
+	return 0;
+}
+
+static int qup_i2c_resume(struct device *device)
+{
+	dev_dbg(device, "system resume");
+	qup_i2c_pm_resume_runtime(device);
+	pm_runtime_mark_last_busy(device);
+	pm_request_autosuspend(device);
+	return 0;
+}
+#endif
+
+static const struct dev_pm_ops qup_i2c_qup_pm_ops = {
+	SET_SYSTEM_SLEEP_PM_OPS(
+		qup_i2c_suspend,
+		qup_i2c_resume)
+	SET_RUNTIME_PM_OPS(
+		qup_i2c_pm_suspend_runtime,
+		qup_i2c_pm_resume_runtime,
+		NULL)
+};
+
+static const struct of_device_id qup_i2c_dt_match[] = {
+	{ .compatible = "qcom,i2c-qup-v1.1.1" },
+	{ .compatible = "qcom,i2c-qup-v2.1.1" },
+	{ .compatible = "qcom,i2c-qup-v2.2.1" },
+	{}
+};
+MODULE_DEVICE_TABLE(of, qup_i2c_dt_match);
+
+static struct platform_driver qup_i2c_driver = {
+	.probe  = qup_i2c_probe,
+	.remove = qup_i2c_remove,
+	.driver = {
+		.name = "i2c_qup",
+		.owner = THIS_MODULE,
+		.pm = &qup_i2c_qup_pm_ops,
+		.of_match_table = qup_i2c_dt_match,
+	},
+};
+
+module_platform_driver(qup_i2c_driver);
+
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform:i2c_qup");
-- 
1.7.9.5

Re: [PATCH v3 2/2] i2c: New bus driver for the Qualcomm QUP I2C controller

[Joe Perches]

On Thu, 2014-02-20 at 16:38 -0800, Bjorn Andersson wrote:
> This bus driver supports the QUP i2c hardware controller in the Qualcomm SOCs.
> The Qualcomm Universal Peripheral Engine (QUP) is a general purpose data path
> engine with input/output FIFOs and an embedded i2c mini-core. The driver
> supports FIFO mode (for low bandwidth applications) and block mode (interrupt
> generated for each block-size data transfer).

trivia:

> diff --git a/drivers/i2c/busses/i2c-qup.c b/drivers/i2c/busses/i2c-qup.c
[]
> +#ifdef CONFIG_PM_SLEEP
> +static int qup_i2c_suspend(struct device *device)
> +{
> +	dev_dbg(device, "system suspend");
> +	qup_i2c_pm_suspend_runtime(device);
> +	return 0;
> +}
> +
> +static int qup_i2c_resume(struct device *device)
> +{
> +	dev_dbg(device, "system resume");
> +	qup_i2c_pm_resume_runtime(device);
> +	pm_runtime_mark_last_busy(device);
> +	pm_request_autosuspend(device);
> +	return 0;
> +}
> +#endif

Missing terminating newlines for dev_dbg statements.

Re: [PATCH v3 2/2] i2c: New bus driver for the Qualcomm QUP I2C controller

[Bjorn Andersson]

On Thu 20 Feb 16:52 PST 2014, Joe Perches wrote:

> On Thu, 2014-02-20 at 16:38 -0800, Bjorn Andersson wrote:
> > This bus driver supports the QUP i2c hardware controller in the Qualcomm SOCs.
> > The Qualcomm Universal Peripheral Engine (QUP) is a general purpose data path
> > engine with input/output FIFOs and an embedded i2c mini-core. The driver
> > supports FIFO mode (for low bandwidth applications) and block mode (interrupt
> > generated for each block-size data transfer).
> 
> trivia:
> 
> > diff --git a/drivers/i2c/busses/i2c-qup.c b/drivers/i2c/busses/i2c-qup.c
> []
> > +#ifdef CONFIG_PM_SLEEP
> > +static int qup_i2c_suspend(struct device *device)
> > +{
> > +	dev_dbg(device, "system suspend");
> > +	qup_i2c_pm_suspend_runtime(device);
> > +	return 0;
> > +}
> > +
> > +static int qup_i2c_resume(struct device *device)
> > +{
> > +	dev_dbg(device, "system resume");
> > +	qup_i2c_pm_resume_runtime(device);
> > +	pm_runtime_mark_last_busy(device);
> > +	pm_request_autosuspend(device);
> > +	return 0;
> > +}
> > +#endif
> 
> Missing terminating newlines for dev_dbg statements.

Thanks Joe, missed those.

Regards,
Bjorn

Re: [PATCH v3 2/2] i2c: New bus driver for the Qualcomm QUP I2C controller

[Emilio López]

Hi Bjorn,

El 20/02/14 21:38, Bjorn Andersson escribió:
> This bus driver supports the QUP i2c hardware controller in the Qualcomm SOCs.
> The Qualcomm Universal Peripheral Engine (QUP) is a general purpose data path
> engine with input/output FIFOs and an embedded i2c mini-core. The driver
> supports FIFO mode (for low bandwidth applications) and block mode (interrupt
> generated for each block-size data transfer).
>
> Cc: Andy Gross <agross@codeaurora.org>
> Cc: Stephen Boyd <sboyd@codeaurora.org>
> Signed-off-by: Ivan T. Ivanov <iivanov@mm-sol.com>
> Signed-off-by: Bjorn Andersson <bjorn.andersson@sonymobile.com>
> ---

I'm not familiar with the subsystem nor the hardware, so I had a look 
around for general stuff. I have some minor comments to make on the 
probe code.

(snip)
> +static int qup_i2c_probe(struct platform_device *pdev)
> +{
> +	static const int blk_sizes[] = {4, 16, 32};
> +	struct device_node *node = pdev->dev.of_node;
> +	struct qup_i2c_dev *qup;
> +	unsigned long one_bit_t;
> +	struct resource *res;
> +	u32 val, io_mode, hw_ver, size;
> +	int ret, fs_div, hs_div;
> +	int src_clk_freq;
> +	int clk_freq;
> +
> +	qup = devm_kzalloc(&pdev->dev, sizeof(*qup), GFP_KERNEL);
> +	if (!qup)
> +		return -ENOMEM;
> +
> +	qup->dev = &pdev->dev;
> +	init_completion(&qup->xfer);
> +	platform_set_drvdata(pdev, qup);
> +
> +	clk_freq = 100000;
> +	if (!of_property_read_u32(node, "clock-frequency", &val))
> +		clk_freq = val;

val will be modified only if no error occurs, so you may rewrite this 
bit as

+	u32 clk_freq = 100000;
...
+	of_property_read_u32(node, "clock-frequency", &clk_freq)

> +
> +	/* We support frequencies up to FAST Mode (400KHz) */
> +	if (clk_freq <= 0 || clk_freq > 400000) {

And you can drop the < check when it's unsigned.

> +		dev_err(qup->dev, "clock frequency not supported %d\n",
> +			clk_freq);
> +		return -EIO;

EIO?

Cheers,

Emilio

Re: [PATCH v3 2/2] i2c: New bus driver for the Qualcomm QUP I2C controller

[Bjorn Andersson]

On Thu 20 Feb 18:53 PST 2014, Emilio L?pez wrote:

> Hi Bjorn,
> 
> El 20/02/14 21:38, Bjorn Andersson escribió:
[...]
> > +	clk_freq = 100000;
> > +	if (!of_property_read_u32(node, "clock-frequency", &val))
> > +		clk_freq = val;
> 
> val will be modified only if no error occurs, so you may rewrite this 
> bit as
> 
> +	u32 clk_freq = 100000;
> ...
> +	of_property_read_u32(node, "clock-frequency", &clk_freq)

Seems the two conventions used in the kernel is either your proposal or "if
fail the assign default". So I'll change this.

> 
> > +
> > +	/* We support frequencies up to FAST Mode (400KHz) */
> > +	if (clk_freq <= 0 || clk_freq > 400000) {
> 
> And you can drop the < check when it's unsigned.

Indeed!

> 
> > +		dev_err(qup->dev, "clock frequency not supported %d\n",
> > +			clk_freq);
> > +		return -EIO;
> 
> EIO?

EINVAL

> 
> Cheers,
> 
> Emilio

Thanks,
Bjorn

Re: [PATCH v3 2/2] i2c: New bus driver for the Qualcomm QUP I2C controller

[Maxime Ripard]

[-- Attachment #1.1: Type: text/plain, Size: 579 bytes --]

Hi Bjorn,

On Thu, Feb 20, 2014 at 04:38:10PM -0800, Bjorn Andersson wrote:
> +static int qup_i2c_probe(struct platform_device *pdev)
> +{

[ snip ]

> +
> +	qup_i2c_enable_clocks(qup);
> +

[ snip ]

> +
> +	pm_runtime_set_autosuspend_delay(qup->dev, MSEC_PER_SEC);
> +	pm_runtime_use_autosuspend(qup->dev);
> +	pm_runtime_enable(qup->dev);

Since the device is already woken up, you probably need to call
pm_runtime_set_active here.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v3 2/2] i2c: New bus driver for the Qualcomm QUP I2C controller

[Bjorn Andersson]

On Fri 21 Feb 00:16 PST 2014, Maxime Ripard wrote:

> Hi Bjorn,
> 
> On Thu, Feb 20, 2014 at 04:38:10PM -0800, Bjorn Andersson wrote:
> > +static int qup_i2c_probe(struct platform_device *pdev)
> > +{
> 
> [ snip ]
> 
> > +
> > +	qup_i2c_enable_clocks(qup);
> > +
> 
> [ snip ]
> 
> > +
> > +	pm_runtime_set_autosuspend_delay(qup->dev, MSEC_PER_SEC);
> > +	pm_runtime_use_autosuspend(qup->dev);
> > +	pm_runtime_enable(qup->dev);
> 
> Since the device is already woken up, you probably need to call
> pm_runtime_set_active here.

Thanks.

> 
> Maxime

Regards,
Bjorn

Re: [PATCH v3 2/2] i2c: New bus driver for the Qualcomm QUP I2C controller

[Mark Rutland]

On Fri, Feb 21, 2014 at 12:38:10AM +0000, Bjorn Andersson wrote:
> This bus driver supports the QUP i2c hardware controller in the Qualcomm SOCs.
> The Qualcomm Universal Peripheral Engine (QUP) is a general purpose data path
> engine with input/output FIFOs and an embedded i2c mini-core. The driver
> supports FIFO mode (for low bandwidth applications) and block mode (interrupt
> generated for each block-size data transfer).
>
> Cc: Andy Gross <agross@codeaurora.org>
> Cc: Stephen Boyd <sboyd@codeaurora.org>
> Signed-off-by: Ivan T. Ivanov <iivanov@mm-sol.com>
> Signed-off-by: Bjorn Andersson <bjorn.andersson@sonymobile.com>
> ---
>  drivers/i2c/busses/Kconfig   |   10 +
>  drivers/i2c/busses/Makefile  |    1 +
>  drivers/i2c/busses/i2c-qup.c |  772 ++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 783 insertions(+)
>  create mode 100644 drivers/i2c/busses/i2c-qup.c
>
> diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
> index f5ed031..403e196 100644
> --- a/drivers/i2c/busses/Kconfig
> +++ b/drivers/i2c/busses/Kconfig
> @@ -648,6 +648,16 @@ config I2C_PXA_SLAVE
>           is necessary for systems where the PXA may be a target on the
>           I2C bus.

[...]

> +static const struct of_device_id qup_i2c_dt_match[] = {
> +       { .compatible = "qcom,i2c-qup-v1.1.1" },
> +       { .compatible = "qcom,i2c-qup-v2.1.1" },
> +       { .compatible = "qcom,i2c-qup-v2.2.1" },

The all seem to be handled the same.

Are they all compatible with the "qcom,i2c-qup-v1.1.1" programming
model, such that it could be used as a fallback in the compatible list
(and the driver would only need to look for it for now)?

Cheers,
Mark.

Re: [PATCH v3 2/2] i2c: New bus driver for the Qualcomm QUP I2C controller

[Bjorn Andersson]

On Fri, Feb 21, 2014 at 3:06 AM, Mark Rutland <mark.rutland@arm.com> wrote:
> On Fri, Feb 21, 2014 at 12:38:10AM +0000, Bjorn Andersson wrote:
[...]
>
>> +static const struct of_device_id qup_i2c_dt_match[] = {
>> +       { .compatible = "qcom,i2c-qup-v1.1.1" },
>> +       { .compatible = "qcom,i2c-qup-v2.1.1" },
>> +       { .compatible = "qcom,i2c-qup-v2.2.1" },
>
> The all seem to be handled the same.
>
> Are they all compatible with the "qcom,i2c-qup-v1.1.1" programming
> model, such that it could be used as a fallback in the compatible list
> (and the driver would only need to look for it for now)?

The v2 model will get BAM (DMAEngine) support soon, v1 uses an
older DMA core. So there's a difference. I'm not aware what differences
there are between 2.1.1 and 2.2.1.

Question is if next change will be called v3, as we then could skip the
reset of the version.

We could probably skip 2.1.1 as that's supposed to be the first revision
of 8x74, with all the expected HW quirks...I.e. not sure if anyone should
use that.

Regards,
Bjorn

Re: [PATCH v3 2/2] i2c: New bus driver for the Qualcomm QUP I2C controller

[Andy Gross]

On Tue, Feb 25, 2014 at 08:07:13AM -0800, Bjorn Andersson wrote:

[snip]

> The v2 model will get BAM (DMAEngine) support soon, v1 uses an
> older DMA core. So there's a difference. I'm not aware what differences
> there are between 2.1.1 and 2.2.1.

Difference between 2.1.1 and 2.2.1:
 - high speed support, up to 3.4MHz
 - reconfiguration during run state
 - READ_AND_NACK, STOP tag support
 - some h/w bug fixes that don't involve software changes.

 
> Question is if next change will be called v3, as we then could skip the
> reset of the version.

Next version is 2.4.

-- 
sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

Re: [PATCH v3 2/2] i2c: New bus driver for the Qualcomm QUP I2C controller

[Kumar Gala]

On Feb 20, 2014, at 6:38 PM, Bjorn Andersson <bjorn.andersson-/MT0OVThwyLZJqsBc5GL+g@public.gmane.org> wrote:

> This bus driver supports the QUP i2c hardware controller in the Qualcomm SOCs.
> The Qualcomm Universal Peripheral Engine (QUP) is a general purpose data path
> engine with input/output FIFOs and an embedded i2c mini-core. The driver
> supports FIFO mode (for low bandwidth applications) and block mode (interrupt
> generated for each block-size data transfer).
> 
> Cc: Andy Gross <agross-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
> Cc: Stephen Boyd <sboyd-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
> Signed-off-by: Ivan T. Ivanov <iivanov-NEYub+7Iv8PQT0dZR+AlfA@public.gmane.org>
> Signed-off-by: Bjorn Andersson <bjorn.andersson-/MT0OVThwyLZJqsBc5GL+g@public.gmane.org>
> ---
> drivers/i2c/busses/Kconfig   |   10 +
> drivers/i2c/busses/Makefile  |    1 +
> drivers/i2c/busses/i2c-qup.c |  772 ++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 783 insertions(+)
> create mode 100644 drivers/i2c/busses/i2c-qup.c
> 
> diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
> index f5ed031..403e196 100644
> --- a/drivers/i2c/busses/Kconfig
> +++ b/drivers/i2c/busses/Kconfig
> @@ -648,6 +648,16 @@ config I2C_PXA_SLAVE
> 	  is necessary for systems where the PXA may be a target on the
> 	  I2C bus.
> 
> +config I2C_QUP
> +	tristate "Qualcomm QUP based I2C controller"
> +	depends on ARCH_MSM

ARCH_QCOM

> +	help
> +	  If you say yes to this option, support will be included for the
> +	  built-in I2C interface on the Qualcomm SoCs.
> +
> +	  This driver can also be built as a module.  If so, the module
> +	  will be called i2c-qup.
> +
> config I2C_RIIC
> 	tristate "Renesas RIIC adapter"
> 	depends on ARCH_SHMOBILE || COMPILE_TEST

- k

-- 
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

Re: [PATCH v3 2/2] i2c: New bus driver for the Qualcomm QUP I2C controller

[Ivan T. Ivanov]

Hi, 

On Fri, 2014-02-21 at 09:35 -0600, Kumar Gala wrote: 
> On Feb 20, 2014, at 6:38 PM, Bjorn Andersson <bjorn.andersson@sonymobile.com> wrote:
> 
> > This bus driver supports the QUP i2c hardware controller in the Qualcomm SOCs.
> > The Qualcomm Universal Peripheral Engine (QUP) is a general purpose data path
> > engine with input/output FIFOs and an embedded i2c mini-core. The driver
> > supports FIFO mode (for low bandwidth applications) and block mode (interrupt
> > generated for each block-size data transfer).
> > 
> > Cc: Andy Gross <agross@codeaurora.org>
> > Cc: Stephen Boyd <sboyd@codeaurora.org>
> > Signed-off-by: Ivan T. Ivanov <iivanov@mm-sol.com>
> > Signed-off-by: Bjorn Andersson <bjorn.andersson@sonymobile.com>
> > ---

<snip>

> > 
> > +config I2C_QUP
> > +	tristate "Qualcomm QUP based I2C controller"
> > +	depends on ARCH_MSM
> 
> ARCH_QCOM

There is no such symbol, still.

Regards,
Ivan 


> - k
> 

Re: [PATCH v3 2/2] i2c: New bus driver for the Qualcomm QUP I2C controller

[Josh Cartwright]

On Mon, Feb 24, 2014 at 12:33:10PM +0200, Ivan T. Ivanov wrote:
> On Fri, 2014-02-21 at 09:35 -0600, Kumar Gala wrote: 
> > On Feb 20, 2014, at 6:38 PM, Bjorn Andersson <bjorn.andersson@sonymobile.com> wrote:
> > 
> > > This bus driver supports the QUP i2c hardware controller in the Qualcomm SOCs.
> > > The Qualcomm Universal Peripheral Engine (QUP) is a general purpose data path
> > > engine with input/output FIFOs and an embedded i2c mini-core. The driver
> > > supports FIFO mode (for low bandwidth applications) and block mode (interrupt
> > > generated for each block-size data transfer).
> > > 
> > > Cc: Andy Gross <agross@codeaurora.org>
> > > Cc: Stephen Boyd <sboyd@codeaurora.org>
> > > Signed-off-by: Ivan T. Ivanov <iivanov@mm-sol.com>
> > > Signed-off-by: Bjorn Andersson <bjorn.andersson@sonymobile.com>
> > > ---
> > > +config I2C_QUP
> > > +	tristate "Qualcomm QUP based I2C controller"
> > > +	depends on ARCH_MSM
> > 
> > ARCH_QCOM
> 
> There is no such symbol, still.

For what it's worth, the rename/split is in linux-next, and will land in
3.15 (as part of arm-soc cleanups).

Is the (short-term) problem of the lack of the symbol a problem?  Worse
case, this driver lands before the ARCH_QCOM rename lands, and the
driver is just not selectable.  Only when both are in place can the
driver be selected.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

Re: [PATCH v3 2/2] i2c: New bus driver for the Qualcomm QUP I2C controller

[Bjorn Andersson]

On Mon 24 Feb 09:40 PST 2014, Josh Cartwright wrote:

> On Mon, Feb 24, 2014 at 12:33:10PM +0200, Ivan T. Ivanov wrote:
> > On Fri, 2014-02-21 at 09:35 -0600, Kumar Gala wrote: 
> > > On Feb 20, 2014, at 6:38 PM, Bjorn Andersson <bjorn.andersson@sonymobile.com> wrote:
> > > 
> > > > This bus driver supports the QUP i2c hardware controller in the Qualcomm SOCs.
> > > > The Qualcomm Universal Peripheral Engine (QUP) is a general purpose data path
> > > > engine with input/output FIFOs and an embedded i2c mini-core. The driver
> > > > supports FIFO mode (for low bandwidth applications) and block mode (interrupt
> > > > generated for each block-size data transfer).
> > > > 
> > > > Cc: Andy Gross <agross@codeaurora.org>
> > > > Cc: Stephen Boyd <sboyd@codeaurora.org>
> > > > Signed-off-by: Ivan T. Ivanov <iivanov@mm-sol.com>
> > > > Signed-off-by: Bjorn Andersson <bjorn.andersson@sonymobile.com>
> > > > ---
> > > > +config I2C_QUP
> > > > +	tristate "Qualcomm QUP based I2C controller"
> > > > +	depends on ARCH_MSM
> > > 
> > > ARCH_QCOM
> > 
> > There is no such symbol, still.
> 
> For what it's worth, the rename/split is in linux-next, and will land in
> 3.15 (as part of arm-soc cleanups).
> 
> Is the (short-term) problem of the lack of the symbol a problem?  Worse
> case, this driver lands before the ARCH_QCOM rename lands, and the
> driver is just not selectable.  Only when both are in place can the
> driver be selected.

As it's best case that this driver lands at the same time as that cleanup I do
agree. I'll "rebase" this ontop of Kumar's series for the next revision.

Regards,
Bjorn

Re: [PATCH v3 2/2] i2c: New bus driver for the Qualcomm QUP I2C controller

[Wolfram Sang]

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


> +	qup->adap.nr = pdev->id;
> +	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));
> +
> +	ret = i2c_add_numbered_adapter(&qup->adap);

I'd suggest to use just i2c_add_adapter and let the core check the
aliases for you.


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

[PATCH v3 0/2] Qualcomm Universal Peripheral (QUP) I2C controller

[Bjorn Andersson]

Continuing on Ivans i2c-qup series.

Changes from v2:
 - Removed unused variables and includes
 - Corrected read logic in irq handler
 - Made the polling loop in qup_i2c_poll_state() less arbitrary
 - Only building suspend/resume if CONFIG_PM_SLEEP

Changes from v1:
 - Cleaned up device tree binding example.
 - Refrased device tree bindings.
 - Following changes in the i2c framework.
 - Use the core clock to calculate divider for the bus clock, instead of
   explicitly setting it.
 - Remove explicit pinctrl settting.
 - Split/renamed qup_i2c_enable(bool) into enable/disable functions.
 - Return value was overwritten on error in write_one/read_one.
 - Initialize the i2c core every time, so that we actually can execute
   more than 1 transmission per xfer.

Ivan T. Ivanov (2):
  i2c: qup: Add device tree bindings information
  i2c: New bus driver for the QUP I2C controller

 .../devicetree/bindings/i2c/qcom,i2c-qup.txt       |  41 +
 drivers/i2c/busses/Kconfig                         |  10 +
 drivers/i2c/busses/Makefile                        |   1 +
 drivers/i2c/busses/i2c-qup.c                       | 894 +++++++++++++++++++++
 4 files changed, 946 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/i2c/qcom,i2c-qup.txt
 create mode 100644 drivers/i2c/busses/i2c-qup.c

-- 
1.8.2.2

[PATCH v3 1/2] i2c: qup: Add device tree bindings information

[Bjorn Andersson]

From: "Ivan T. Ivanov" <iivanov@mm-sol.com>

The Qualcomm Universal Peripherial (QUP) wraps I2C mini-core and
provide input and output FIFO's for it. I2C controller can operate
as master with supported bus speeds of 100Kbps and 400Kbps.

Signed-off-by: Ivan T. Ivanov <iivanov@mm-sol.com>
[bjorn: reformulated part of binding description and cleaned up example]
Signed-off-by: Bjorn Andersson <bjorn.andersson@sonymobile.com>
---
 .../devicetree/bindings/i2c/qcom,i2c-qup.txt       | 41 ++++++++++++++++++++++
 1 file changed, 41 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/i2c/qcom,i2c-qup.txt

diff --git a/Documentation/devicetree/bindings/i2c/qcom,i2c-qup.txt b/Documentation/devicetree/bindings/i2c/qcom,i2c-qup.txt
new file mode 100644
index 0000000..a99711b
--- /dev/null
+++ b/Documentation/devicetree/bindings/i2c/qcom,i2c-qup.txt
@@ -0,0 +1,41 @@
+Qualcomm Universal Peripheral (QUP) I2C controller
+
+Required properties:
+ - compatible: Should be "qcom,i2c-qup".
+ - reg: Should contain QUP register address and length.
+ - interrupts: Should contain I2C interrupt.
+
+ - clocks: Should contain the core clock and the AHB clock.
+ - clock-names: Should be "core" for the core clock and "iface" for the
+                AHB clock.
+
+ - #address-cells: Should be <1> Address cells for i2c device address
+ - #size-cells: Should be <0> as i2c addresses have no size component
+
+Optional properties:
+ - clock-frequency: Should specify the desired i2c bus clock frequency in Hz,
+                    default is 100kHz if omitted.
+
+Child nodes should conform to i2c bus binding.
+
+Example:
+
+ i2c2: i2c@f9924000 {
+ 	compatible = "qcom,i2c-qup";
+ 	reg = <0xf9924000 0x1000>;
+ 	interrupts = <0 96 0>;
+
+ 	clocks = <&gcc_blsp1_qup2_i2c_apps_clk>, <&gcc_blsp1_ahb_clk>;
+ 	clock-names = "core", "iface";
+
+ 	clock-frequency = <355000>;
+
+ 	#address-cells = <1>;
+ 	#size-cells = <0>;
+
+ 	dummy@60 {
+ 		compatible = "dummy";
+ 		reg = <0x60>;
+ 	};
+ };
+
-- 
1.8.2.2

Re: [PATCH v3 1/2] i2c: qup: Add device tree bindings information

[Rob Herring]

On Fri, Jan 17, 2014 at 5:03 PM, Bjorn Andersson
<bjorn.andersson@sonymobile.com> wrote:
> From: "Ivan T. Ivanov" <iivanov@mm-sol.com>
>
> The Qualcomm Universal Peripherial (QUP) wraps I2C mini-core and
> provide input and output FIFO's for it. I2C controller can operate
> as master with supported bus speeds of 100Kbps and 400Kbps.
>
> Signed-off-by: Ivan T. Ivanov <iivanov@mm-sol.com>
> [bjorn: reformulated part of binding description and cleaned up example]
> Signed-off-by: Bjorn Andersson <bjorn.andersson@sonymobile.com>
> ---

Patch history?

>  .../devicetree/bindings/i2c/qcom,i2c-qup.txt       | 41 ++++++++++++++++++++++
>  1 file changed, 41 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/i2c/qcom,i2c-qup.txt
>
> diff --git a/Documentation/devicetree/bindings/i2c/qcom,i2c-qup.txt b/Documentation/devicetree/bindings/i2c/qcom,i2c-qup.txt
> new file mode 100644
> index 0000000..a99711b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/i2c/qcom,i2c-qup.txt
> @@ -0,0 +1,41 @@
> +Qualcomm Universal Peripheral (QUP) I2C controller
> +
> +Required properties:
> + - compatible: Should be "qcom,i2c-qup".

Seems a bit generic. All versions of the IP are exactly the same?
"qcom,<chip>-i2c-qup" would be better.

> + - reg: Should contain QUP register address and length.
> + - interrupts: Should contain I2C interrupt.
> +
> + - clocks: Should contain the core clock and the AHB clock.
> + - clock-names: Should be "core" for the core clock and "iface" for the
> +                AHB clock.
> +
> + - #address-cells: Should be <1> Address cells for i2c device address
> + - #size-cells: Should be <0> as i2c addresses have no size component
> +
> +Optional properties:
> + - clock-frequency: Should specify the desired i2c bus clock frequency in Hz,
> +                    default is 100kHz if omitted.
> +
> +Child nodes should conform to i2c bus binding.
> +
> +Example:
> +
> + i2c2: i2c@f9924000 {
> +       compatible = "qcom,i2c-qup";
> +       reg = <0xf9924000 0x1000>;
> +       interrupts = <0 96 0>;
> +
> +       clocks = <&gcc_blsp1_qup2_i2c_apps_clk>, <&gcc_blsp1_ahb_clk>;
> +       clock-names = "core", "iface";
> +
> +       clock-frequency = <355000>;
> +
> +       #address-cells = <1>;
> +       #size-cells = <0>;
> +
> +       dummy@60 {
> +               compatible = "dummy";
> +               reg = <0x60>;
> +       };
> + };
> +
> --
> 1.8.2.2
>
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 1/2] i2c: qup: Add device tree bindings information

[Stephen Boyd]

On 01/20, Rob Herring wrote:
> On Fri, Jan 17, 2014 at 5:03 PM, Bjorn Andersson
> <bjorn.andersson@sonymobile.com> wrote:
> 
> >  .../devicetree/bindings/i2c/qcom,i2c-qup.txt       | 41 ++++++++++++++++++++++
> >  1 file changed, 41 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/i2c/qcom,i2c-qup.txt
> >
> > diff --git a/Documentation/devicetree/bindings/i2c/qcom,i2c-qup.txt b/Documentation/devicetree/bindings/i2c/qcom,i2c-qup.txt
> > new file mode 100644
> > index 0000000..a99711b
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/i2c/qcom,i2c-qup.txt
> > @@ -0,0 +1,41 @@
> > +Qualcomm Universal Peripheral (QUP) I2C controller
> > +
> > +Required properties:
> > + - compatible: Should be "qcom,i2c-qup".
> 
> Seems a bit generic. All versions of the IP are exactly the same?
> "qcom,<chip>-i2c-qup" would be better.
> 

There are different versions of the IP in different SoCs. The
versions for platforms supported upstream are:

	1.1.1 (MSM8660, MSM8960)
	2.2.1 (MSM8974v2)

so how about one of "qcom,i2c-qup-v1.1.1" or
"qcom,i2c-qup-v2.2.1" should be specified as more specific
compatible strings in addition to "qcom,i2c-qup"?

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

Re: [PATCH v3 1/2] i2c: qup: Add device tree bindings information

[Andy Gross]

On Mon, Jan 20, 2014 at 06:40:07PM -0800, Stephen Boyd wrote:
> On 01/20, Rob Herring wrote:
> > On Fri, Jan 17, 2014 at 5:03 PM, Bjorn Andersson
> > <bjorn.andersson@sonymobile.com> wrote:
> > 
> > >  .../devicetree/bindings/i2c/qcom,i2c-qup.txt       | 41 ++++++++++++++++++++++
> > >  1 file changed, 41 insertions(+)
> > >  create mode 100644 Documentation/devicetree/bindings/i2c/qcom,i2c-qup.txt
> > >
> > > diff --git a/Documentation/devicetree/bindings/i2c/qcom,i2c-qup.txt b/Documentation/devicetree/bindings/i2c/qcom,i2c-qup.txt
> > > new file mode 100644
> > > index 0000000..a99711b
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/i2c/qcom,i2c-qup.txt
> > > @@ -0,0 +1,41 @@
> > > +Qualcomm Universal Peripheral (QUP) I2C controller
> > > +
> > > +Required properties:
> > > + - compatible: Should be "qcom,i2c-qup".
> > 
> > Seems a bit generic. All versions of the IP are exactly the same?
> > "qcom,<chip>-i2c-qup" would be better.
> > 
> 
> There are different versions of the IP in different SoCs. The
> versions for platforms supported upstream are:
> 
> 	1.1.1 (MSM8660, MSM8960)
> 	2.2.1 (MSM8974v2)
> 
> so how about one of "qcom,i2c-qup-v1.1.1" or
> "qcom,i2c-qup-v2.2.1" should be specified as more specific
> compatible strings in addition to "qcom,i2c-qup"?
> 

This definitely needs two compatible tags.  The v1.1.1 supports the FIFO, block,
and data mover modes.  The v2.2.1 supports FIFO, block, and BAM DMA.

The BAM patches are currently in review and ideally BAM support can be added to
the I2C.  We'll need a way to differentiate so BAM can be used.

Otherwise, the register interfaces are interchangeable except for BAM specific
fields in the registers.


> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

-- 
sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

Re: [PATCH v3 1/2] i2c: qup: Add device tree bindings information

[Matthew Locke]

Rob,

On Jan 20, 2014, at 8:10 AM, Rob Herring wrote:

> On Fri, Jan 17, 2014 at 5:03 PM, Bjorn Andersson
> <bjorn.andersson@sonymobile.com> wrote:
>> From: "Ivan T. Ivanov" <iivanov@mm-sol.com>
>> 
>> The Qualcomm Universal Peripherial (QUP) wraps I2C mini-core and
>> provide input and output FIFO's for it. I2C controller can operate
>> as master with supported bus speeds of 100Kbps and 400Kbps.
>> 
>> Signed-off-by: Ivan T. Ivanov <iivanov@mm-sol.com>
>> [bjorn: reformulated part of binding description and cleaned up example]
>> Signed-off-by: Bjorn Andersson <bjorn.andersson@sonymobile.com>
>> ---
> 
> Patch history?
> 
>> .../devicetree/bindings/i2c/qcom,i2c-qup.txt       | 41 ++++++++++++++++++++++
>> 1 file changed, 41 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/i2c/qcom,i2c-qup.txt
>> 
>> diff --git a/Documentation/devicetree/bindings/i2c/qcom,i2c-qup.txt b/Documentation/devicetree/bindings/i2c/qcom,i2c-qup.txt
>> new file mode 100644
>> index 0000000..a99711b
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/i2c/qcom,i2c-qup.txt
>> @@ -0,0 +1,41 @@
>> +Qualcomm Universal Peripheral (QUP) I2C controller
>> +
>> +Required properties:
>> + - compatible: Should be "qcom,i2c-qup".
> 
> Seems a bit generic. All versions of the IP are exactly the same?
> "qcom,<chip>-i2c-qup" would be better.

The QUP IP block is the same across chips.  Its possible there could be different versions of the IP block but its pretty stable now. 

> 
>> + - reg: Should contain QUP register address and length.
>> + - interrupts: Should contain I2C interrupt.
>> +
>> + - clocks: Should contain the core clock and the AHB clock.
>> + - clock-names: Should be "core" for the core clock and "iface" for the
>> +                AHB clock.
>> +
>> + - #address-cells: Should be <1> Address cells for i2c device address
>> + - #size-cells: Should be <0> as i2c addresses have no size component
>> +
>> +Optional properties:
>> + - clock-frequency: Should specify the desired i2c bus clock frequency in Hz,
>> +                    default is 100kHz if omitted.
>> +
>> +Child nodes should conform to i2c bus binding.
>> +
>> +Example:
>> +
>> + i2c2: i2c@f9924000 {
>> +       compatible = "qcom,i2c-qup";
>> +       reg = <0xf9924000 0x1000>;
>> +       interrupts = <0 96 0>;
>> +
>> +       clocks = <&gcc_blsp1_qup2_i2c_apps_clk>, <&gcc_blsp1_ahb_clk>;
>> +       clock-names = "core", "iface";
>> +
>> +       clock-frequency = <355000>;
>> +
>> +       #address-cells = <1>;
>> +       #size-cells = <0>;
>> +
>> +       dummy@60 {
>> +               compatible = "dummy";
>> +               reg = <0x60>;
>> +       };
>> + };
>> +
>> --
>> 1.8.2.2
>> 
>> --
>> To unsubscribe from this list: send the line "unsubscribe devicetree" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

Re: [PATCH v3 1/2] i2c: qup: Add device tree bindings information

[Arnd Bergmann]

On Thursday 23 January 2014, Matthew Locke wrote:
> The QUP IP block is the same across chips.  Its possible there could be different versions of the IP block but its pretty stable now. 

Do you know the version numbers of the IP block? That would be the ideal case. We just
fall back to using the SoC name in most cases because that's all that is publically
known.

	Arnd

Re: [PATCH v3 1/2] i2c: qup: Add device tree bindings information

[Bjorn Andersson]

On Thu, Jan 23, 2014 at 11:11 AM, Matthew Locke <mlocke@codeaurora.org> wrote:
> Rob,
>
> On Jan 20, 2014, at 8:10 AM, Rob Herring wrote:
>
>> On Fri, Jan 17, 2014 at 5:03 PM, Bjorn Andersson
[...]
>>> +Required properties:
>>> + - compatible: Should be "qcom,i2c-qup".
>>
>> Seems a bit generic. All versions of the IP are exactly the same?
>> "qcom,<chip>-i2c-qup" would be better.
>
> The QUP IP block is the same across chips.  Its possible there could be different versions of the IP block but its pretty stable now.

I have not been able to find any differences in this block between
8960 and 8074 and I have successfully tested the driver on both
platforms.

So the reason for making it more specific would be to handle any
future crazyness. Not sure what to do about this though, I have never
heard about the version numbers Stephen gave us and I can't find them
in the documentation available to us, so I wouldn't know which of the
two to select...

Regards,
Bjorn