2016-10-11 16:39:34

by Christ, Austin

[permalink] [raw]
Subject: [PATCH v6 0/2] Add ACPI support and SMBus block read

From: Austin Christ <[email protected]>

Add ACPI support to I2C QUP driver and get properties from ACPI table.

Add support to the I2C QUP driver to correctly handle SMBus block reads by
checking I2C_M_RECV_LEN flag and reading the first byte received as the
message length.

Documentation for the two properites used from ACPI has been submitted through
[email protected] to https://github.com/ahs3/dsd. The documentation can be viewed
at https://lists.acpica.org/pipermail/dsd/2016-September/000095.html.

[V6]
- correct block size for SMBus data read
[V5]
- remove warning and use correct ACPI function
[V4]
- correct error code
- remove warning for fall back to default clock frequency
[V3]
- clean up unused variables
- use constant instead of variable for smbus length field
[V2]
- rework the smbus block read and break into separate function
- clean up redundant checks and variables

Naveen Kaje (2):
i2c: qup: add ACPI support
i2c: qup: support SMBus block read

drivers/i2c/busses/i2c-qup.c | 122 ++++++++++++++++++++++++++++++++++++-------
1 file changed, 103 insertions(+), 19 deletions(-)

--
Qualcomm Datacenter Technologies as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.


2016-10-11 16:39:38

by Christ, Austin

[permalink] [raw]
Subject: [PATCH v6 2/2] i2c: qup: support SMBus block read

From: Naveen Kaje <[email protected]>

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 <[email protected]>
Signed-off-by: Austin Christ <[email protected]>
Reviewed-by: Sricharan R <[email protected]>
---
drivers/i2c/busses/i2c-qup.c | 64 +++++++++++++++++++++++++++++++++++++++++---
1 file changed, 61 insertions(+), 3 deletions(-)

diff --git a/drivers/i2c/busses/i2c-qup.c b/drivers/i2c/busses/i2c-qup.c
index bf0957e..d34120a 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,20 @@ 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_blk_data(qup, msg);
+ /* set tag length for block read */
+ qup->blk.tx_tag_len = 2;
+ qup_i2c_set_read_mode_v2(qup, msg->buf[0]);
+ }
} while (qup->blk.pos < qup->blk.count);

err:
@@ -1210,6 +1263,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 Datacenter Technologies as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.

2016-10-11 16:39:37

by Christ, Austin

[permalink] [raw]
Subject: [PATCH v6 1/2] i2c: qup: add ACPI support

From: Naveen Kaje <[email protected]>

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

Signed-off-by: Naveen Kaje <[email protected]>
Signed-off-by: Austin Christ <[email protected]>
Reviewed-by: Sricharan R <[email protected]>
---
drivers/i2c/busses/i2c-qup.c | 58 ++++++++++++++++++++++++++++++++------------
1 file changed, 42 insertions(+), 16 deletions(-)

diff --git a/drivers/i2c/busses/i2c-qup.c b/drivers/i2c/busses/i2c-qup.c
index 041050e..bf0957e 100644
--- a/drivers/i2c/busses/i2c-qup.c
+++ b/drivers/i2c/busses/i2c-qup.c
@@ -14,6 +14,7 @@
*
*/

+#include <linux/acpi.h>
#include <linux/atomic.h>
#include <linux/clk.h>
#include <linux/delay.h>
@@ -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;
@@ -1356,14 +1361,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 = DEFAULT_SRC_CLK;
+ u32 clk_freq = DEFAULT_CLK_FREQ;
int blocks;

qup = devm_kzalloc(&pdev->dev, sizeof(*qup), GFP_KERNEL);
@@ -1374,7 +1378,11 @@ 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_notice(qup->dev, "using default clock-frequency %d",
+ DEFAULT_CLK_FREQ);
+ }

if (of_device_is_compatible(pdev->dev.of_node, "qcom,i2c-qup-v1.1.1")) {
qup->adap.algo = &qup_i2c_algo;
@@ -1456,20 +1464,30 @@ 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 (has_acpi_companion(qup->dev)) {
+ ret = device_property_read_u32(qup->dev,
+ "src-clock-hz", &src_clk_freq);
+ if (ret) {
+ dev_notice(qup->dev, "using default src-clock-hz %d",
+ 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.
@@ -1516,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);
@@ -1641,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_match);
+#endif
+
static struct platform_driver qup_i2c_driver = {
.probe = qup_i2c_probe,
.remove = qup_i2c_remove,
@@ -1648,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 Datacenter Technologies as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.

2016-11-29 21:47:04

by Wolfram Sang

[permalink] [raw]
Subject: Re: [v6,2/2] i2c: qup: support SMBus block read

On Tue, Oct 11, 2016 at 10:27:57AM -0600, Austin Christ wrote:
> From: Naveen Kaje <[email protected]>
>
> 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 <[email protected]>
> Signed-off-by: Austin Christ <[email protected]>
> Reviewed-by: Sricharan R <[email protected]>

Applied to for-next, thanks!


Attachments:
(No filename) (681.00 B)
signature.asc (819.00 B)
Download all attachments

2016-11-29 21:47:14

by Wolfram Sang

[permalink] [raw]
Subject: Re: [v6,1/2] i2c: qup: add ACPI support

On Tue, Oct 11, 2016 at 10:27:56AM -0600, Austin Christ wrote:
> From: Naveen Kaje <[email protected]>
>
> Add support to get the device parameters from ACPI. Assume
> that the clocks are managed by firmware.
>
> Signed-off-by: Naveen Kaje <[email protected]>
> Signed-off-by: Austin Christ <[email protected]>
> Reviewed-by: Sricharan R <[email protected]>

Applied to for-next, thanks!


Attachments:
(No filename) (410.00 B)
signature.asc (819.00 B)
Download all attachments