[PATCH v3 1/2] i2c: qup: add ACPI support

[PATCH v3 1/2] i2c: qup: add ACPI support

[Austin Christ]

From: Naveen Kaje <nkaje@codeaurora.org>

Add support to get the device parameters from ACPI. Assume
that the clocks are managed by firmware.

Signed-off-by: Naveen Kaje <nkaje@codeaurora.org>
Signed-off-by: Austin Christ <austinwc@codeaurora.org>
---
 drivers/i2c/busses/i2c-qup.c | 60 ++++++++++++++++++++++++++++++++------------
 1 file changed, 44 insertions(+), 16 deletions(-)

Changes:
- v3:
 - clean up unused variable
- v2:
 - clean up redundant checks and variables

diff --git a/drivers/i2c/busses/i2c-qup.c b/drivers/i2c/busses/i2c-qup.c
index dddd4da..ea6ca5f 100644
--- a/drivers/i2c/busses/i2c-qup.c
+++ b/drivers/i2c/busses/i2c-qup.c
@@ -29,6 +29,7 @@
 #include <linux/platform_device.h>
 #include <linux/pm_runtime.h>
 #include <linux/scatterlist.h>
+#include <linux/acpi.h>
 
 /* QUP Registers */
 #define QUP_CONFIG		0x000
@@ -132,6 +133,10 @@
 /* Max timeout in ms for 32k bytes */
 #define TOUT_MAX			300
 
+/* Default values. Use these if FW query fails */
+#define DEFAULT_CLK_FREQ 100000
+#define DEFAULT_SRC_CLK 20000000
+
 struct qup_i2c_block {
 	int	count;
 	int	pos;
@@ -1354,14 +1359,13 @@ static void qup_i2c_disable_clocks(struct qup_i2c_dev *qup)
 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 io_mode, hw_ver, size;
 	int ret, fs_div, hs_div;
-	int src_clk_freq;
-	u32 clk_freq = 100000;
+	u32 src_clk_freq = 0;
+	u32 clk_freq = 0;
 	int blocks;
 
 	qup = devm_kzalloc(&pdev->dev, sizeof(*qup), GFP_KERNEL);
@@ -1372,7 +1376,12 @@ static int qup_i2c_probe(struct platform_device *pdev)
 	init_completion(&qup->xfer);
 	platform_set_drvdata(pdev, qup);
 
-	of_property_read_u32(node, "clock-frequency", &clk_freq);
+	ret = device_property_read_u32(qup->dev, "clock-frequency", &clk_freq);
+	if (ret) {
+		dev_warn(qup->dev, "using default clock-frequency %d",
+			DEFAULT_CLK_FREQ);
+		clk_freq = DEFAULT_CLK_FREQ;
+	}
 
 	if (of_device_is_compatible(pdev->dev.of_node, "qcom,i2c-qup-v1.1.1")) {
 		qup->adap.algo = &qup_i2c_algo;
@@ -1454,20 +1463,31 @@ nodma:
 		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);
-	}
+	if (ACPI_HANDLE(qup->dev)) {
+		ret = device_property_read_u32(qup->dev,
+				"src-clock-hz", &src_clk_freq);
+		if (ret) {
+			dev_warn(qup->dev, "using default src-clock-hz %d",
+				DEFAULT_SRC_CLK);
+			src_clk_freq = DEFAULT_SRC_CLK;
+		}
+		ACPI_COMPANION_SET(&qup->adap.dev, ACPI_COMPANION(qup->dev));
+	} else {
+		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->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);
+		src_clk_freq = clk_get_rate(qup->clk);
 	}
 
-	qup_i2c_enable_clocks(qup);
-
 	/*
 	 * Bootloaders might leave a pending interrupt on certain QUP's,
 	 * so we reset the core before registering for interrupts.
@@ -1514,7 +1534,6 @@ nodma:
 	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);
@@ -1639,6 +1658,14 @@ static const struct of_device_id qup_i2c_dt_match[] = {
 };
 MODULE_DEVICE_TABLE(of, qup_i2c_dt_match);
 
+#if IS_ENABLED(CONFIG_ACPI)
+static const struct acpi_device_id qup_i2c_acpi_match[] = {
+	{ "QCOM8010"},
+	{ },
+};
+MODULE_DEVICE_TABLE(acpi, qup_i2c_acpi_ids);
+#endif
+
 static struct platform_driver qup_i2c_driver = {
 	.probe  = qup_i2c_probe,
 	.remove = qup_i2c_remove,
@@ -1646,6 +1673,7 @@ static struct platform_driver qup_i2c_driver = {
 		.name = "i2c_qup",
 		.pm = &qup_i2c_qup_pm_ops,
 		.of_match_table = qup_i2c_dt_match,
+		.acpi_match_table = ACPI_PTR(qup_i2c_acpi_match),
 	},
 };
 
-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project.

[PATCH v3 2/2] i2c: qup: support SMBus block read

[Austin Christ]

From: Naveen Kaje <nkaje@codeaurora.org>

I2C QUP driver relies on SMBus emulation support from the framework.
To handle SMBus block reads, the driver should check I2C_M_RECV_LEN
flag and should read the first byte received as the message length.

The driver configures the QUP hardware to read one byte. Once the
message length is known from this byte, the QUP hardware is configured
to read the rest.

Signed-off-by: Naveen Kaje <nkaje@codeaurora.org>
Signed-off-by: Austin Christ <austinwc@codeaurora.org>
---
 drivers/i2c/busses/i2c-qup.c | 68 ++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 65 insertions(+), 3 deletions(-)

Changes:
- v3:
 - clean up redundant checks
 - use constant instead of variable for smbus length field
- v2:
 - rework the smbus block read and break into separate function

diff --git a/drivers/i2c/busses/i2c-qup.c b/drivers/i2c/busses/i2c-qup.c
index ea6ca5f..9fbed83 100644
--- a/drivers/i2c/busses/i2c-qup.c
+++ b/drivers/i2c/busses/i2c-qup.c
@@ -517,6 +517,33 @@ static int qup_i2c_get_data_len(struct qup_i2c_dev *qup)
 	return data_len;
 }
 
+static bool qup_i2c_check_msg_len(struct i2c_msg *msg)
+{
+	return ((msg->flags & I2C_M_RD) && (msg->flags & I2C_M_RECV_LEN));
+}
+
+static int qup_i2c_set_tags_smb(u16 addr, u8 *tags, struct qup_i2c_dev *qup,
+			struct i2c_msg *msg)
+{
+	int len = 0;
+
+	if (msg->len > 1) {
+		tags[len++] = QUP_TAG_V2_DATARD_STOP;
+		tags[len++] = qup_i2c_get_data_len(qup) - 1;
+	} else {
+		tags[len++] = QUP_TAG_V2_START;
+		tags[len++] = addr & 0xff;
+
+		if (msg->flags & I2C_M_TEN)
+			tags[len++] = addr >> 8;
+
+		tags[len++] = QUP_TAG_V2_DATARD;
+		/* Read 1 byte indicating the length of the SMBus message */
+		tags[len++] = 1;
+	}
+	return len;
+}
+
 static int qup_i2c_set_tags(u8 *tags, struct qup_i2c_dev *qup,
 			    struct i2c_msg *msg,  int is_dma)
 {
@@ -526,6 +553,10 @@ static int qup_i2c_set_tags(u8 *tags, struct qup_i2c_dev *qup,
 
 	int last = (qup->blk.pos == (qup->blk.count - 1)) && (qup->is_last);
 
+	/* Handle tags for SMBus block read */
+	if (qup_i2c_check_msg_len(msg))
+		return qup_i2c_set_tags_smb(addr, tags, qup, msg);
+
 	if (qup->blk.pos == 0) {
 		tags[len++] = QUP_TAG_V2_START;
 		tags[len++] = addr & 0xff;
@@ -1065,9 +1096,17 @@ static int qup_i2c_read_fifo_v2(struct qup_i2c_dev *qup,
 				struct i2c_msg *msg)
 {
 	u32 val;
-	int idx, pos = 0, ret = 0, total;
+	int idx, pos = 0, ret = 0, total, msg_offset = 0;
 
+	/*
+	 * If the message length is already read in
+	 * the first byte of the buffer, account for
+	 * that by setting the offset
+	 */
+	if (qup_i2c_check_msg_len(msg) && (msg->len > 1))
+		msg_offset = 1;
 	total = qup_i2c_get_data_len(qup);
+	total -= msg_offset;
 
 	/* 2 extra bytes for read tags */
 	while (pos < (total + 2)) {
@@ -1087,8 +1126,8 @@ static int qup_i2c_read_fifo_v2(struct qup_i2c_dev *qup,
 
 			if (pos >= (total + 2))
 				goto out;
-
-			msg->buf[qup->pos++] = val & 0xff;
+			msg->buf[qup->pos + msg_offset] = val & 0xff;
+			qup->pos++;
 		}
 	}
 
@@ -1128,6 +1167,24 @@ static int qup_i2c_read_one_v2(struct qup_i2c_dev *qup, struct i2c_msg *msg)
 			goto err;
 
 		qup->blk.pos++;
+
+		/* Handle SMBus block read length */
+		if (qup_i2c_check_msg_len(msg) && (msg->len == 1)) {
+			if (msg->buf[0] > I2C_SMBUS_BLOCK_MAX) {
+				ret = -EPROTO;
+				goto err;
+			}
+			msg->len += msg->buf[0];
+			qup->pos = 0;
+			qup_i2c_set_read_mode_v2(qup, msg->len);
+			ret = qup_i2c_issue_xfer_v2(qup, msg);
+			if (ret)
+				goto err;
+			ret = qup_i2c_wait_for_complete(qup, msg);
+			if (ret)
+				goto err;
+			qup_i2c_set_blk_data(qup, msg);
+		}
 	} while (qup->blk.pos < qup->blk.count);
 
 err:
@@ -1210,6 +1267,11 @@ static int qup_i2c_xfer(struct i2c_adapter *adap,
 			goto out;
 		}
 
+		if (qup_i2c_check_msg_len(&msgs[idx])) {
+			ret = -EOPNOTSUPP;
+			goto out;
+		}
+
 		if (msgs[idx].flags & I2C_M_RD)
 			ret = qup_i2c_read_one(qup, &msgs[idx]);
 		else
-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project.

RE: [PATCH v3 1/2] i2c: qup: add ACPI support

[Sricharan]

Hi,

>From: Naveen Kaje <nkaje@codeaurora.org>
>
>Add support to get the device parameters from ACPI. Assume
>that the clocks are managed by firmware.
>
>Signed-off-by: Naveen Kaje <nkaje@codeaurora.org>
>Signed-off-by: Austin Christ <austinwc@codeaurora.org>
>---
> drivers/i2c/busses/i2c-qup.c | 60 ++++++++++++++++++++++++++++++++------------
> 1 file changed, 44 insertions(+), 16 deletions(-)
>
>Changes:
>- v3:
> - clean up unused variable
>- v2:
> - clean up redundant checks and variables
>
>diff --git a/drivers/i2c/busses/i2c-qup.c b/drivers/i2c/busses/i2c-qup.c
>index dddd4da..ea6ca5f 100644
>--- a/drivers/i2c/busses/i2c-qup.c
>+++ b/drivers/i2c/busses/i2c-qup.c
>@@ -29,6 +29,7 @@
> #include <linux/platform_device.h>
> #include <linux/pm_runtime.h>
> #include <linux/scatterlist.h>
>+#include <linux/acpi.h>
>
> /* QUP Registers */
> #define QUP_CONFIG		0x000
>@@ -132,6 +133,10 @@
> /* Max timeout in ms for 32k bytes */
> #define TOUT_MAX			300
>
>+/* Default values. Use these if FW query fails */
>+#define DEFAULT_CLK_FREQ 100000
>+#define DEFAULT_SRC_CLK 20000000
>+
> struct qup_i2c_block {
> 	int	count;
> 	int	pos;
>@@ -1354,14 +1359,13 @@ static void qup_i2c_disable_clocks(struct qup_i2c_dev *qup)
> 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 io_mode, hw_ver, size;
> 	int ret, fs_div, hs_div;
>-	int src_clk_freq;
>-	u32 clk_freq = 100000;
>+	u32 src_clk_freq = 0;
>+	u32 clk_freq = 0;
> 	int blocks;
>
> 	qup = devm_kzalloc(&pdev->dev, sizeof(*qup), GFP_KERNEL);
>@@ -1372,7 +1376,12 @@ static int qup_i2c_probe(struct platform_device *pdev)
> 	init_completion(&qup->xfer);
> 	platform_set_drvdata(pdev, qup);
>
>-	of_property_read_u32(node, "clock-frequency", &clk_freq);
>+	ret = device_property_read_u32(qup->dev, "clock-frequency", &clk_freq);
>+	if (ret) {
>+		dev_warn(qup->dev, "using default clock-frequency %d",
>+			DEFAULT_CLK_FREQ);
>+		clk_freq = DEFAULT_CLK_FREQ;
>+	}
>
> 	if (of_device_is_compatible(pdev->dev.of_node, "qcom,i2c-qup-v1.1.1")) {
> 		qup->adap.algo = &qup_i2c_algo;
>@@ -1454,20 +1463,31 @@ nodma:
> 		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);
>-	}
>+	if (ACPI_HANDLE(qup->dev)) {
>+		ret = device_property_read_u32(qup->dev,
>+				"src-clock-hz", &src_clk_freq);
>+		if (ret) {
>+			dev_warn(qup->dev, "using default src-clock-hz %d",
>+				DEFAULT_SRC_CLK);
>+			src_clk_freq = DEFAULT_SRC_CLK;
>+		}
>+		ACPI_COMPANION_SET(&qup->adap.dev, ACPI_COMPANION(qup->dev));
>+	} else {
>+		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->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);
>+		src_clk_freq = clk_get_rate(qup->clk);
> 	}
>
>-	qup_i2c_enable_clocks(qup);
>-
> 	/*
> 	 * Bootloaders might leave a pending interrupt on certain QUP's,
> 	 * so we reset the core before registering for interrupts.
>@@ -1514,7 +1534,6 @@ nodma:
> 	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);
>@@ -1639,6 +1658,14 @@ static const struct of_device_id qup_i2c_dt_match[] = {
> };
> MODULE_DEVICE_TABLE(of, qup_i2c_dt_match);
>
>+#if IS_ENABLED(CONFIG_ACPI)
>+static const struct acpi_device_id qup_i2c_acpi_match[] = {
>+	{ "QCOM8010"},
>+	{ },
>+};
>+MODULE_DEVICE_TABLE(acpi, qup_i2c_acpi_ids);
>+#endif
>+
> static struct platform_driver qup_i2c_driver = {
> 	.probe  = qup_i2c_probe,
> 	.remove = qup_i2c_remove,
>@@ -1646,6 +1673,7 @@ static struct platform_driver qup_i2c_driver = {
> 		.name = "i2c_qup",
> 		.pm = &qup_i2c_qup_pm_ops,
> 		.of_match_table = qup_i2c_dt_match,
>+		.acpi_match_table = ACPI_PTR(qup_i2c_acpi_match),
> 	},
> };

Reviewed-by: sricharan@codeaurora.org

Regards,
 Sricharan

RE: [PATCH v3 2/2] i2c: qup: support SMBus block read

[Sricharan]

Hi,

>From: Naveen Kaje <nkaje@codeaurora.org>
>
>I2C QUP driver relies on SMBus emulation support from the framework.
>To handle SMBus block reads, the driver should check I2C_M_RECV_LEN
>flag and should read the first byte received as the message length.
>
>The driver configures the QUP hardware to read one byte. Once the
>message length is known from this byte, the QUP hardware is configured
>to read the rest.
>
>Signed-off-by: Naveen Kaje <nkaje@codeaurora.org>
>Signed-off-by: Austin Christ <austinwc@codeaurora.org>
>---
> drivers/i2c/busses/i2c-qup.c | 68 ++++++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 65 insertions(+), 3 deletions(-)
>
>Changes:
>- v3:
> - clean up redundant checks
> - use constant instead of variable for smbus length field
>- v2:
> - rework the smbus block read and break into separate function
>
>diff --git a/drivers/i2c/busses/i2c-qup.c b/drivers/i2c/busses/i2c-qup.c
>index ea6ca5f..9fbed83 100644
>--- a/drivers/i2c/busses/i2c-qup.c
>+++ b/drivers/i2c/busses/i2c-qup.c
>@@ -517,6 +517,33 @@ static int qup_i2c_get_data_len(struct qup_i2c_dev *qup)
> 	return data_len;
> }
>
>+static bool qup_i2c_check_msg_len(struct i2c_msg *msg)
>+{
>+	return ((msg->flags & I2C_M_RD) && (msg->flags & I2C_M_RECV_LEN));
>+}
>+
>+static int qup_i2c_set_tags_smb(u16 addr, u8 *tags, struct qup_i2c_dev *qup,
>+			struct i2c_msg *msg)
>+{
>+	int len = 0;
>+
>+	if (msg->len > 1) {
>+		tags[len++] = QUP_TAG_V2_DATARD_STOP;
>+		tags[len++] = qup_i2c_get_data_len(qup) - 1;
>+	} else {
>+		tags[len++] = QUP_TAG_V2_START;
>+		tags[len++] = addr & 0xff;
>+
>+		if (msg->flags & I2C_M_TEN)
>+			tags[len++] = addr >> 8;
>+
>+		tags[len++] = QUP_TAG_V2_DATARD;
>+		/* Read 1 byte indicating the length of the SMBus message */
>+		tags[len++] = 1;
>+	}
>+	return len;
>+}
>+
> static int qup_i2c_set_tags(u8 *tags, struct qup_i2c_dev *qup,
> 			    struct i2c_msg *msg,  int is_dma)
> {
>@@ -526,6 +553,10 @@ static int qup_i2c_set_tags(u8 *tags, struct qup_i2c_dev *qup,
>
> 	int last = (qup->blk.pos == (qup->blk.count - 1)) && (qup->is_last);
>
>+	/* Handle tags for SMBus block read */
>+	if (qup_i2c_check_msg_len(msg))
>+		return qup_i2c_set_tags_smb(addr, tags, qup, msg);
>+
> 	if (qup->blk.pos == 0) {
> 		tags[len++] = QUP_TAG_V2_START;
> 		tags[len++] = addr & 0xff;
>@@ -1065,9 +1096,17 @@ static int qup_i2c_read_fifo_v2(struct qup_i2c_dev *qup,
> 				struct i2c_msg *msg)
> {
> 	u32 val;
>-	int idx, pos = 0, ret = 0, total;
>+	int idx, pos = 0, ret = 0, total, msg_offset = 0;
>
>+	/*
>+	 * If the message length is already read in
>+	 * the first byte of the buffer, account for
>+	 * that by setting the offset
>+	 */
>+	if (qup_i2c_check_msg_len(msg) && (msg->len > 1))
>+		msg_offset = 1;
> 	total = qup_i2c_get_data_len(qup);
>+	total -= msg_offset;
>
> 	/* 2 extra bytes for read tags */
> 	while (pos < (total + 2)) {
>@@ -1087,8 +1126,8 @@ static int qup_i2c_read_fifo_v2(struct qup_i2c_dev *qup,
>
> 			if (pos >= (total + 2))
> 				goto out;
>-
>-			msg->buf[qup->pos++] = val & 0xff;
>+			msg->buf[qup->pos + msg_offset] = val & 0xff;
>+			qup->pos++;
> 		}
> 	}
>
>@@ -1128,6 +1167,24 @@ static int qup_i2c_read_one_v2(struct qup_i2c_dev *qup, struct i2c_msg *msg)
> 			goto err;
>
> 		qup->blk.pos++;
>+
>+		/* Handle SMBus block read length */
>+		if (qup_i2c_check_msg_len(msg) && (msg->len == 1)) {
>+			if (msg->buf[0] > I2C_SMBUS_BLOCK_MAX) {
>+				ret = -EPROTO;
>+				goto err;
>+			}
>+			msg->len += msg->buf[0];
>+			qup->pos = 0;
>+			qup_i2c_set_read_mode_v2(qup, msg->len);
>+			ret = qup_i2c_issue_xfer_v2(qup, msg);
>+			if (ret)
>+				goto err;
>+			ret = qup_i2c_wait_for_complete(qup, msg);
>+			if (ret)
>+				goto err;
>+			qup_i2c_set_blk_data(qup, msg);
>+		}
> 	} while (qup->blk.pos < qup->blk.count);
>
> err:
>@@ -1210,6 +1267,11 @@ static int qup_i2c_xfer(struct i2c_adapter *adap,
> 			goto out;
> 		}
>
>+		if (qup_i2c_check_msg_len(&msgs[idx])) {
>+			ret = -EOPNOTSUPP;
>+			goto out;
>+		}
>+
> 		if (msgs[idx].flags & I2C_M_RD)
> 			ret = qup_i2c_read_one(qup, &msgs[idx]);
> 		else

Reviewed-by: sricharan@codeaurora.org

Regards,
 Sricharan

Re: [PATCH v3 2/2] i2c: qup: support SMBus block read

[rajeev kumar]

On Fri, May 20, 2016 at 3:14 AM, Austin Christ <austinwc@codeaurora.org> wrote:
> From: Naveen Kaje <nkaje@codeaurora.org>
>
> I2C QUP driver relies on SMBus emulation support from the framework.
> To handle SMBus block reads, the driver should check I2C_M_RECV_LEN
> flag and should read the first byte received as the message length.
>
> The driver configures the QUP hardware to read one byte. Once the
> message length is known from this byte, the QUP hardware is configured
> to read the rest.
>
> Signed-off-by: Naveen Kaje <nkaje@codeaurora.org>
> Signed-off-by: Austin Christ <austinwc@codeaurora.org>
> ---
>  drivers/i2c/busses/i2c-qup.c | 68 ++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 65 insertions(+), 3 deletions(-)
>
> Changes:
> - v3:
>  - clean up redundant checks
>  - use constant instead of variable for smbus length field
> - v2:
>  - rework the smbus block read and break into separate function
>
> diff --git a/drivers/i2c/busses/i2c-qup.c b/drivers/i2c/busses/i2c-qup.c
> index ea6ca5f..9fbed83 100644
> --- a/drivers/i2c/busses/i2c-qup.c
> +++ b/drivers/i2c/busses/i2c-qup.c
> @@ -517,6 +517,33 @@ static int qup_i2c_get_data_len(struct qup_i2c_dev *qup)
>         return data_len;
>  }
>
> +static bool qup_i2c_check_msg_len(struct i2c_msg *msg)
> +{
> +       return ((msg->flags & I2C_M_RD) && (msg->flags & I2C_M_RECV_LEN));
> +}
> +
> +static int qup_i2c_set_tags_smb(u16 addr, u8 *tags, struct qup_i2c_dev *qup,
> +                       struct i2c_msg *msg)
> +{
> +       int len = 0;
> +
> +       if (msg->len > 1) {
> +               tags[len++] = QUP_TAG_V2_DATARD_STOP;
> +               tags[len++] = qup_i2c_get_data_len(qup) - 1;
> +       } else {
> +               tags[len++] = QUP_TAG_V2_START;
> +               tags[len++] = addr & 0xff;
> +
> +               if (msg->flags & I2C_M_TEN)
> +                       tags[len++] = addr >> 8;
> +
> +               tags[len++] = QUP_TAG_V2_DATARD;
> +               /* Read 1 byte indicating the length of the SMBus message */
> +               tags[len++] = 1;
> +       }
> +       return len;
> +}
> +
>  static int qup_i2c_set_tags(u8 *tags, struct qup_i2c_dev *qup,
>                             struct i2c_msg *msg,  int is_dma)
>  {
> @@ -526,6 +553,10 @@ static int qup_i2c_set_tags(u8 *tags, struct qup_i2c_dev *qup,
>
>         int last = (qup->blk.pos == (qup->blk.count - 1)) && (qup->is_last);
>
> +       /* Handle tags for SMBus block read */
> +       if (qup_i2c_check_msg_len(msg))
> +               return qup_i2c_set_tags_smb(addr, tags, qup, msg);
> +
>         if (qup->blk.pos == 0) {
>                 tags[len++] = QUP_TAG_V2_START;
>                 tags[len++] = addr & 0xff;
> @@ -1065,9 +1096,17 @@ static int qup_i2c_read_fifo_v2(struct qup_i2c_dev *qup,
>                                 struct i2c_msg *msg)
>  {
>         u32 val;
> -       int idx, pos = 0, ret = 0, total;
> +       int idx, pos = 0, ret = 0, total, msg_offset = 0;
>
> +       /*
> +        * If the message length is already read in
> +        * the first byte of the buffer, account for
> +        * that by setting the offset
> +        */
> +       if (qup_i2c_check_msg_len(msg) && (msg->len > 1))
> +               msg_offset = 1;
>         total = qup_i2c_get_data_len(qup);
> +       total -= msg_offset;
>
>         /* 2 extra bytes for read tags */
>         while (pos < (total + 2)) {
> @@ -1087,8 +1126,8 @@ static int qup_i2c_read_fifo_v2(struct qup_i2c_dev *qup,
>
>                         if (pos >= (total + 2))
>                                 goto out;
> -
> -                       msg->buf[qup->pos++] = val & 0xff;
> +                       msg->buf[qup->pos + msg_offset] = val & 0xff;
> +                       qup->pos++;
>                 }
>         }
>
> @@ -1128,6 +1167,24 @@ static int qup_i2c_read_one_v2(struct qup_i2c_dev *qup, struct i2c_msg *msg)
>                         goto err;
>
>                 qup->blk.pos++;
> +
> +               /* Handle SMBus block read length */
> +               if (qup_i2c_check_msg_len(msg) && (msg->len == 1)) {
> +                       if (msg->buf[0] > I2C_SMBUS_BLOCK_MAX) {
> +                               ret = -EPROTO;
> +                               goto err;
> +                       }
> +                       msg->len += msg->buf[0];
> +                       qup->pos = 0;
> +                       qup_i2c_set_read_mode_v2(qup, msg->len);
> +                       ret = qup_i2c_issue_xfer_v2(qup, msg);
> +                       if (ret)
> +                               goto err;
> +                       ret = qup_i2c_wait_for_complete(qup, msg);
> +                       if (ret)
> +                               goto err;
> +                       qup_i2c_set_blk_data(qup, msg);
> +               }
>         } while (qup->blk.pos < qup->blk.count);
>
>  err:
> @@ -1210,6 +1267,11 @@ static int qup_i2c_xfer(struct i2c_adapter *adap,
>                         goto out;
>                 }
>
> +               if (qup_i2c_check_msg_len(&msgs[idx])) {
> +                       ret = -EOPNOTSUPP;

Could you please rethink on this return value ? It should not be "EOPNOTSUPP"

~Rajeev

> +                       goto out;
> +               }
> +
>                 if (msgs[idx].flags & I2C_M_RD)
>                         ret = qup_i2c_read_one(qup, &msgs[idx]);
>                 else
> --
> Qualcomm Innovation Center, Inc.
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project.
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-i2c" 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 2/2] i2c: qup: support SMBus block read

[Christ, Austin]

Hello,


On 5/25/2016 3:31 AM, rajeev kumar wrote:
> On Fri, May 20, 2016 at 3:14 AM, Austin Christ <austinwc@codeaurora.org> wrote:
>> From: Naveen Kaje <nkaje@codeaurora.org>
>>
>> I2C QUP driver relies on SMBus emulation support from the framework.
>> To handle SMBus block reads, the driver should check I2C_M_RECV_LEN
>> flag and should read the first byte received as the message length.
>>
>> The driver configures the QUP hardware to read one byte. Once the
>> message length is known from this byte, the QUP hardware is configured
>> to read the rest.
>>
>> Signed-off-by: Naveen Kaje <nkaje@codeaurora.org>
>> Signed-off-by: Austin Christ <austinwc@codeaurora.org>
>> ---
>>   drivers/i2c/busses/i2c-qup.c | 68 ++++++++++++++++++++++++++++++++++++++++++--
>>   1 file changed, 65 insertions(+), 3 deletions(-)
>>
>> Changes:
>> - v3:
>>   - clean up redundant checks
>>   - use constant instead of variable for smbus length field
>> - v2:
>>   - rework the smbus block read and break into separate function
>>
>> diff --git a/drivers/i2c/busses/i2c-qup.c b/drivers/i2c/busses/i2c-qup.c
>> index ea6ca5f..9fbed83 100644
>> --- a/drivers/i2c/busses/i2c-qup.c
>> +++ b/drivers/i2c/busses/i2c-qup.c
>> @@ -517,6 +517,33 @@ static int qup_i2c_get_data_len(struct qup_i2c_dev *qup)
>>          return data_len;
>>   }
>>
>> +static bool qup_i2c_check_msg_len(struct i2c_msg *msg)
>> +{
>> +       return ((msg->flags & I2C_M_RD) && (msg->flags & I2C_M_RECV_LEN));
>> +}
>> +
>> +static int qup_i2c_set_tags_smb(u16 addr, u8 *tags, struct qup_i2c_dev *qup,
>> +                       struct i2c_msg *msg)
>> +{
>> +       int len = 0;
>> +
>> +       if (msg->len > 1) {
>> +               tags[len++] = QUP_TAG_V2_DATARD_STOP;
>> +               tags[len++] = qup_i2c_get_data_len(qup) - 1;
>> +       } else {
>> +               tags[len++] = QUP_TAG_V2_START;
>> +               tags[len++] = addr & 0xff;
>> +
>> +               if (msg->flags & I2C_M_TEN)
>> +                       tags[len++] = addr >> 8;
>> +
>> +               tags[len++] = QUP_TAG_V2_DATARD;
>> +               /* Read 1 byte indicating the length of the SMBus message */
>> +               tags[len++] = 1;
>> +       }
>> +       return len;
>> +}
>> +
>>   static int qup_i2c_set_tags(u8 *tags, struct qup_i2c_dev *qup,
>>                              struct i2c_msg *msg,  int is_dma)
>>   {
>> @@ -526,6 +553,10 @@ static int qup_i2c_set_tags(u8 *tags, struct qup_i2c_dev *qup,
>>
>>          int last = (qup->blk.pos == (qup->blk.count - 1)) && (qup->is_last);
>>
>> +       /* Handle tags for SMBus block read */
>> +       if (qup_i2c_check_msg_len(msg))
>> +               return qup_i2c_set_tags_smb(addr, tags, qup, msg);
>> +
>>          if (qup->blk.pos == 0) {
>>                  tags[len++] = QUP_TAG_V2_START;
>>                  tags[len++] = addr & 0xff;
>> @@ -1065,9 +1096,17 @@ static int qup_i2c_read_fifo_v2(struct qup_i2c_dev *qup,
>>                                  struct i2c_msg *msg)
>>   {
>>          u32 val;
>> -       int idx, pos = 0, ret = 0, total;
>> +       int idx, pos = 0, ret = 0, total, msg_offset = 0;
>>
>> +       /*
>> +        * If the message length is already read in
>> +        * the first byte of the buffer, account for
>> +        * that by setting the offset
>> +        */
>> +       if (qup_i2c_check_msg_len(msg) && (msg->len > 1))
>> +               msg_offset = 1;
>>          total = qup_i2c_get_data_len(qup);
>> +       total -= msg_offset;
>>
>>          /* 2 extra bytes for read tags */
>>          while (pos < (total + 2)) {
>> @@ -1087,8 +1126,8 @@ static int qup_i2c_read_fifo_v2(struct qup_i2c_dev *qup,
>>
>>                          if (pos >= (total + 2))
>>                                  goto out;
>> -
>> -                       msg->buf[qup->pos++] = val & 0xff;
>> +                       msg->buf[qup->pos + msg_offset] = val & 0xff;
>> +                       qup->pos++;
>>                  }
>>          }
>>
>> @@ -1128,6 +1167,24 @@ static int qup_i2c_read_one_v2(struct qup_i2c_dev *qup, struct i2c_msg *msg)
>>                          goto err;
>>
>>                  qup->blk.pos++;
>> +
>> +               /* Handle SMBus block read length */
>> +               if (qup_i2c_check_msg_len(msg) && (msg->len == 1)) {
>> +                       if (msg->buf[0] > I2C_SMBUS_BLOCK_MAX) {
>> +                               ret = -EPROTO;
>> +                               goto err;
>> +                       }
>> +                       msg->len += msg->buf[0];
>> +                       qup->pos = 0;
>> +                       qup_i2c_set_read_mode_v2(qup, msg->len);
>> +                       ret = qup_i2c_issue_xfer_v2(qup, msg);
>> +                       if (ret)
>> +                               goto err;
>> +                       ret = qup_i2c_wait_for_complete(qup, msg);
>> +                       if (ret)
>> +                               goto err;
>> +                       qup_i2c_set_blk_data(qup, msg);
>> +               }
>>          } while (qup->blk.pos < qup->blk.count);
>>
>>   err:
>> @@ -1210,6 +1267,11 @@ static int qup_i2c_xfer(struct i2c_adapter *adap,
>>                          goto out;
>>                  }
>>
>> +               if (qup_i2c_check_msg_len(&msgs[idx])) {
>> +                       ret = -EOPNOTSUPP;
> Could you please rethink on this return value ? It should not be "EOPNOTSUPP"
>
> ~Rajeev

This will be replaced with "EINVAL" in v4 patches.
>
>> +                       goto out;
>> +               }
>> +
>>                  if (msgs[idx].flags & I2C_M_RD)
>>                          ret = qup_i2c_read_one(qup, &msgs[idx]);
>>                  else
>> --
>> Qualcomm Innovation Center, Inc.
>> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
>> a Linux Foundation Collaborative Project.
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> --
> To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Thanks,
Austin