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

[PATCH 1/2][v3] 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 2/2][v4] 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:
-v4:
 - correct error code
- 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..576320f 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 = -EINVAL;
+			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 1/2][v3] i2c: qup: add ACPI support

[Andy Gross]

On Thu, May 26, 2016 at 01:37:56PM -0600, Austin Christ wrote:

<snip>

> @@ -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);

Why do we need a warning if the clock-frequency is not specified?  It is
optional in the DT documentation, is it not in the ACPI?

> +	if (ret) {
> +		dev_warn(qup->dev, "using default clock-frequency %d",
> +			DEFAULT_CLK_FREQ);
> +		clk_freq = DEFAULT_CLK_FREQ;

You could just assign the DEFAULT_CLK_FREQ in the variable declaration instead
of 0.  read_property does not modify the variable unless it finds it or does the
ACPI version modify if not found?

> +	}
>  
>  	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;
>  	}
>  

<snip>

Regards,

Andy

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

[Christ, Austin]

On 6/5/2016 12:57 PM, Andy Gross wrote:
> On Thu, May 26, 2016 at 01:37:56PM -0600, Austin Christ wrote:
>
> <snip>
>
>> @@ -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);
> Why do we need a warning if the clock-frequency is not specified?  It is
> optional in the DT documentation, is it not in the ACPI?
I agree a warning is too strong here. The information may still be 
useful about the driver configuration, so I will change it to a 
dev_notify() in v4.
>
>> +	if (ret) {
>> +		dev_warn(qup->dev, "using default clock-frequency %d",
>> +			DEFAULT_CLK_FREQ);
>> +		clk_freq = DEFAULT_CLK_FREQ;
> You could just assign the DEFAULT_CLK_FREQ in the variable declaration instead
> of 0.  read_property does not modify the variable unless it finds it or does the
> ACPI version modify if not found?
You are correct about this function only modifying when it finds a 
value. This will be corrected in v4.
>
>> +	}
>>   
>>   	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;
>>   	}
>>   
> <snip>
>
> Regards,
>
> Andy
> --
> 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