I had this idea for quite some time on my todo list but a soon to be
implemented refactoring in the i2c-rcar driver now finally made me do it. Add a
'can't do 0 length messages' quirk to the quirk infrastructure for and remove
the manual handling from the drivers. This makes the quirk much more visible.
(Quite some prominent vendors in that list) We also have a centralized place to
handle updates to the quirk detection if that is ever needed.
I have tested this with the i2c-rcar and i2c-sh_mobile driver on a Renesas
SalvatorXS board equipped with M3-N (r8a77965).
A git branch can be found here:
git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git i2c/quirk-no-zero-len
Looking forward to comments, reviews, tests...
Thanks,
Wolfram
Wolfram Sang (12):
i2c: quirks: add zero length checks
i2c: designware-master: use core to detect 'no zero length' quirk
i2c: mxs: use core to detect 'no zero length' quirk
i2c: omap: use core to detect 'no zero length' quirk
i2c: pmcmsp: use core to detect 'no zero length' quirk
i2c: qup: use core to detect 'no zero length' quirk
i2c: stu300: use core to detect 'no zero length' quirk
i2c: tegra: use core to detect 'no zero length' quirk
i2c: zx2967: use core to detect 'no zero length' quirk
i2c: rcar: use core to detect 'no zero length' quirk
i2c: xlr: use core to detect 'no zero length' quirk
i2c: sh_mobile: use core to detect 'no zero length read' quirk
drivers/i2c/busses/i2c-designware-master.c | 12 +++++-------
drivers/i2c/busses/i2c-mxs.c | 8 +++++---
drivers/i2c/busses/i2c-omap.c | 8 +++++---
drivers/i2c/busses/i2c-pmcmsp.c | 17 +----------------
drivers/i2c/busses/i2c-qup.c | 14 ++++++--------
drivers/i2c/busses/i2c-rcar.c | 13 ++++++-------
drivers/i2c/busses/i2c-sh_mobile.c | 10 +++++-----
drivers/i2c/busses/i2c-stu300.c | 12 ++++++------
drivers/i2c/busses/i2c-tegra.c | 4 +---
drivers/i2c/busses/i2c-xlr.c | 11 +++++------
drivers/i2c/busses/i2c-zx2967.c | 8 +++++---
drivers/i2c/i2c-core-base.c | 6 ++++++
include/linux/i2c.h | 4 ++++
13 files changed, 60 insertions(+), 67 deletions(-)
--
2.11.0
And don't reimplement in the driver.
Signed-off-by: Wolfram Sang <[email protected]>
---
Only build tested.
drivers/i2c/busses/i2c-stu300.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/i2c/busses/i2c-stu300.c b/drivers/i2c/busses/i2c-stu300.c
index fce52bdab2b7..5503fa171df0 100644
--- a/drivers/i2c/busses/i2c-stu300.c
+++ b/drivers/i2c/busses/i2c-stu300.c
@@ -673,12 +673,6 @@ static int stu300_xfer_msg(struct i2c_adapter *adap,
msg->addr, msg->len, msg->flags, stop);
}
- /* Zero-length messages are not supported by this hardware */
- if (msg->len == 0) {
- ret = -EINVAL;
- goto exit_disable;
- }
-
/*
* For some reason, sending the address sometimes fails when running
* on the 13 MHz clock. No interrupt arrives. This is a work around,
@@ -863,6 +857,10 @@ static const struct i2c_algorithm stu300_algo = {
.functionality = stu300_func,
};
+static const struct i2c_adapter_quirks stu300_quirks = {
+ .flags = I2C_AQ_NO_ZERO_LEN,
+};
+
static int stu300_probe(struct platform_device *pdev)
{
struct stu300_dev *dev;
@@ -920,6 +918,8 @@ static int stu300_probe(struct platform_device *pdev)
adap->algo = &stu300_algo;
adap->dev.parent = &pdev->dev;
adap->dev.of_node = pdev->dev.of_node;
+ adap->quirks = &stu300_quirks;
+
i2c_set_adapdata(adap, dev);
/* i2c device drivers may be active on return from add_adapter() */
--
2.11.0
And don't reimplement in the driver.
Signed-off-by: Wolfram Sang <[email protected]>
---
drivers/i2c/busses/i2c-sh_mobile.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/i2c/busses/i2c-sh_mobile.c b/drivers/i2c/busses/i2c-sh_mobile.c
index 5fda4188a9e5..9c7f6f8ceb22 100644
--- a/drivers/i2c/busses/i2c-sh_mobile.c
+++ b/drivers/i2c/busses/i2c-sh_mobile.c
@@ -613,11 +613,6 @@ static void sh_mobile_i2c_xfer_dma(struct sh_mobile_i2c_data *pd)
static int start_ch(struct sh_mobile_i2c_data *pd, struct i2c_msg *usr_msg,
bool do_init)
{
- if (usr_msg->len == 0 && (usr_msg->flags & I2C_M_RD)) {
- dev_err(pd->dev, "Unsupported zero length i2c read\n");
- return -EOPNOTSUPP;
- }
-
if (do_init) {
/* Initialize channel registers */
iic_wr(pd, ICCR, ICCR_SCP);
@@ -758,6 +753,10 @@ static const struct i2c_algorithm sh_mobile_i2c_algorithm = {
.master_xfer = sh_mobile_i2c_xfer,
};
+static const struct i2c_adapter_quirks sh_mobile_i2c_quirks = {
+ .flags = I2C_AQ_NO_ZERO_LEN_READ,
+};
+
/*
* r8a7740 chip has lasting errata on I2C I/O pad reset.
* this is work-around for it.
@@ -925,6 +924,7 @@ static int sh_mobile_i2c_probe(struct platform_device *dev)
adap->owner = THIS_MODULE;
adap->algo = &sh_mobile_i2c_algorithm;
+ adap->quirks = &sh_mobile_i2c_quirks;
adap->dev.parent = &dev->dev;
adap->retries = 5;
adap->nr = dev->id;
--
2.11.0
And don't reimplement in the driver.
Signed-off-by: Wolfram Sang <[email protected]>
---
Only build tested.
drivers/i2c/busses/i2c-tegra.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c
index 60c8561fbe65..437294ea2f0a 100644
--- a/drivers/i2c/busses/i2c-tegra.c
+++ b/drivers/i2c/busses/i2c-tegra.c
@@ -684,9 +684,6 @@ static int tegra_i2c_xfer_msg(struct tegra_i2c_dev *i2c_dev,
tegra_i2c_flush_fifos(i2c_dev);
- if (msg->len == 0)
- return -EINVAL;
-
i2c_dev->msg_buf = msg->buf;
i2c_dev->msg_buf_remaining = msg->len;
i2c_dev->msg_err = I2C_ERR_NONE;
@@ -831,6 +828,7 @@ static const struct i2c_algorithm tegra_i2c_algo = {
/* payload size is only 12 bit */
static const struct i2c_adapter_quirks tegra_i2c_quirks = {
+ .flags = I2C_AQ_NO_ZERO_LEN,
.max_read_len = 4096,
.max_write_len = 4096,
};
--
2.11.0
And don't reimplement in the driver.
Signed-off-by: Wolfram Sang <[email protected]>
---
drivers/i2c/busses/i2c-rcar.c | 13 ++++++-------
1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/drivers/i2c/busses/i2c-rcar.c b/drivers/i2c/busses/i2c-rcar.c
index 2688520110d1..791a4aa34fdd 100644
--- a/drivers/i2c/busses/i2c-rcar.c
+++ b/drivers/i2c/busses/i2c-rcar.c
@@ -796,14 +796,8 @@ static int rcar_i2c_master_xfer(struct i2c_adapter *adap,
if (ret < 0)
goto out;
- for (i = 0; i < num; i++) {
- /* This HW can't send STOP after address phase */
- if (msgs[i].len == 0) {
- ret = -EOPNOTSUPP;
- goto out;
- }
+ for (i = 0; i < num; i++)
rcar_i2c_request_dma(priv, msgs + i);
- }
/* init first message */
priv->msg = msgs;
@@ -890,6 +884,10 @@ static const struct i2c_algorithm rcar_i2c_algo = {
.unreg_slave = rcar_unreg_slave,
};
+static const struct i2c_adapter_quirks rcar_i2c_quirks = {
+ .flags = I2C_AQ_NO_ZERO_LEN,
+};
+
static const struct of_device_id rcar_i2c_dt_ids[] = {
{ .compatible = "renesas,i2c-r8a7778", .data = (void *)I2C_RCAR_GEN1 },
{ .compatible = "renesas,i2c-r8a7779", .data = (void *)I2C_RCAR_GEN1 },
@@ -943,6 +941,7 @@ static int rcar_i2c_probe(struct platform_device *pdev)
adap->dev.parent = dev;
adap->dev.of_node = dev->of_node;
adap->bus_recovery_info = &rcar_i2c_bri;
+ adap->quirks = &rcar_i2c_quirks;
i2c_set_adapdata(adap, priv);
strlcpy(adap->name, pdev->name, sizeof(adap->name));
--
2.11.0
And don't reimplement in the driver.
Signed-off-by: Wolfram Sang <[email protected]>
---
Only build tested.
drivers/i2c/busses/i2c-xlr.c | 11 +++++------
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/drivers/i2c/busses/i2c-xlr.c b/drivers/i2c/busses/i2c-xlr.c
index 484bfa15d58e..34cd4b308540 100644
--- a/drivers/i2c/busses/i2c-xlr.c
+++ b/drivers/i2c/busses/i2c-xlr.c
@@ -173,9 +173,6 @@ static int xlr_i2c_tx(struct xlr_i2c_private *priv, u16 len,
u8 offset;
u32 xfer;
- if (!len)
- return -EOPNOTSUPP;
-
offset = buf[0];
xlr_i2c_wreg(priv->iobase, XLR_I2C_ADDR, offset);
xlr_i2c_wreg(priv->iobase, XLR_I2C_DEVADDR, addr);
@@ -241,9 +238,6 @@ static int xlr_i2c_rx(struct xlr_i2c_private *priv, u16 len, u8 *buf, u16 addr)
unsigned long timeout, stoptime, checktime;
int nbytes, timedout;
- if (!len)
- return -EOPNOTSUPP;
-
xlr_i2c_wreg(priv->iobase, XLR_I2C_CFG,
XLR_I2C_CFG_NOADDR | priv->cfg->cfg_extra);
xlr_i2c_wreg(priv->iobase, XLR_I2C_BYTECNT, len - 1);
@@ -340,6 +334,10 @@ static const struct i2c_algorithm xlr_i2c_algo = {
.functionality = xlr_func,
};
+static const struct i2c_adapter_quirks xlr_i2c_quirks = {
+ .flags = I2C_AQ_NO_ZERO_LEN,
+};
+
static const struct xlr_i2c_config xlr_i2c_config_default = {
.status_busy = XLR_I2C_BUS_BUSY,
.cfg_extra = 0,
@@ -427,6 +425,7 @@ static int xlr_i2c_probe(struct platform_device *pdev)
priv->adap.owner = THIS_MODULE;
priv->adap.algo_data = priv;
priv->adap.algo = &xlr_i2c_algo;
+ priv->adap.quirks = &xlr_i2c_quirks;
priv->adap.nr = pdev->id;
priv->adap.class = I2C_CLASS_HWMON;
snprintf(priv->adap.name, sizeof(priv->adap.name), "xlr-i2c");
--
2.11.0
And don't reimplement in the driver.
Signed-off-by: Wolfram Sang <[email protected]>
---
Only build tested.
drivers/i2c/busses/i2c-zx2967.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/i2c/busses/i2c-zx2967.c b/drivers/i2c/busses/i2c-zx2967.c
index 48281c1b30c6..b8f9e020d80e 100644
--- a/drivers/i2c/busses/i2c-zx2967.c
+++ b/drivers/i2c/busses/i2c-zx2967.c
@@ -281,9 +281,6 @@ static int zx2967_i2c_xfer_msg(struct zx2967_i2c *i2c,
int ret;
int i;
- if (msg->len == 0)
- return -EINVAL;
-
zx2967_i2c_flush_fifos(i2c);
i2c->cur_trans = msg->buf;
@@ -498,6 +495,10 @@ static const struct i2c_algorithm zx2967_i2c_algo = {
.functionality = zx2967_i2c_func,
};
+static const struct i2c_adapter_quirks zx2967_i2c_quirks = {
+ .flags = I2C_AQ_NO_ZERO_LEN,
+};
+
static const struct of_device_id zx2967_i2c_of_match[] = {
{ .compatible = "zte,zx296718-i2c", },
{ },
@@ -568,6 +569,7 @@ static int zx2967_i2c_probe(struct platform_device *pdev)
strlcpy(i2c->adap.name, "zx2967 i2c adapter",
sizeof(i2c->adap.name));
i2c->adap.algo = &zx2967_i2c_algo;
+ i2c->adap.quirks = &zx2967_i2c_quirks;
i2c->adap.nr = pdev->id;
i2c->adap.dev.parent = &pdev->dev;
i2c->adap.dev.of_node = pdev->dev.of_node;
--
2.11.0
And don't reimplement in the driver.
Signed-off-by: Wolfram Sang <[email protected]>
---
Only build tested.
drivers/i2c/busses/i2c-qup.c | 14 ++++++--------
1 file changed, 6 insertions(+), 8 deletions(-)
diff --git a/drivers/i2c/busses/i2c-qup.c b/drivers/i2c/busses/i2c-qup.c
index c86c3ae1318f..e09cd0775ae9 100644
--- a/drivers/i2c/busses/i2c-qup.c
+++ b/drivers/i2c/busses/i2c-qup.c
@@ -1088,11 +1088,6 @@ static int qup_i2c_xfer(struct i2c_adapter *adap,
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;
@@ -1520,9 +1515,6 @@ qup_i2c_determine_mode_v2(struct qup_i2c_dev *qup,
/* All i2c_msgs should be transferred using either dma or cpu */
for (idx = 0; idx < num; idx++) {
- if (msgs[idx].len == 0)
- return -EINVAL;
-
if (msgs[idx].flags & I2C_M_RD)
max_rx_len = max_t(unsigned int, max_rx_len,
msgs[idx].len);
@@ -1636,9 +1628,14 @@ static const struct i2c_algorithm qup_i2c_algo_v2 = {
* which limits the possible read to 256 (QUP_READ_LIMIT) bytes.
*/
static const struct i2c_adapter_quirks qup_i2c_quirks = {
+ .flags = I2C_AQ_NO_ZERO_LEN,
.max_read_len = QUP_READ_LIMIT,
};
+static const struct i2c_adapter_quirks qup_i2c_quirks_v2 = {
+ .flags = I2C_AQ_NO_ZERO_LEN,
+};
+
static void qup_i2c_enable_clocks(struct qup_i2c_dev *qup)
{
clk_prepare_enable(qup->clk);
@@ -1701,6 +1698,7 @@ static int qup_i2c_probe(struct platform_device *pdev)
is_qup_v1 = true;
} else {
qup->adap.algo = &qup_i2c_algo_v2;
+ qup->adap.quirks = &qup_i2c_quirks_v2;
is_qup_v1 = false;
if (acpi_match_device(qup_i2c_acpi_match, qup->dev))
goto nodma;
--
2.11.0
And don't reimplement in the driver.
Signed-off-by: Wolfram Sang <[email protected]>
---
Only build tested.
drivers/i2c/busses/i2c-mxs.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/i2c/busses/i2c-mxs.c b/drivers/i2c/busses/i2c-mxs.c
index 642c58946d8d..7d79317a1046 100644
--- a/drivers/i2c/busses/i2c-mxs.c
+++ b/drivers/i2c/busses/i2c-mxs.c
@@ -567,9 +567,6 @@ static int mxs_i2c_xfer_msg(struct i2c_adapter *adap, struct i2c_msg *msg,
dev_dbg(i2c->dev, "addr: 0x%04x, len: %d, flags: 0x%x, stop: %d\n",
msg->addr, msg->len, msg->flags, stop);
- if (msg->len == 0)
- return -EINVAL;
-
/*
* The MX28 I2C IP block can only do PIO READ for transfer of to up
* 4 bytes of length. The write transfer is not limited as it can use
@@ -683,6 +680,10 @@ static const struct i2c_algorithm mxs_i2c_algo = {
.functionality = mxs_i2c_func,
};
+static const struct i2c_adapter_quirks mxs_i2c_quirks = {
+ .flags = I2C_AQ_NO_ZERO_LEN,
+};
+
static void mxs_i2c_derive_timing(struct mxs_i2c_dev *i2c, uint32_t speed)
{
/* The I2C block clock runs at 24MHz */
@@ -854,6 +855,7 @@ static int mxs_i2c_probe(struct platform_device *pdev)
strlcpy(adap->name, "MXS I2C adapter", sizeof(adap->name));
adap->owner = THIS_MODULE;
adap->algo = &mxs_i2c_algo;
+ adap->quirks = &mxs_i2c_quirks;
adap->dev.parent = dev;
adap->nr = pdev->id;
adap->dev.of_node = pdev->dev.of_node;
--
2.11.0
And don't reimplement in the driver.
Signed-off-by: Wolfram Sang <[email protected]>
---
Only build tested.
drivers/i2c/busses/i2c-designware-master.c | 12 +++++-------
1 file changed, 5 insertions(+), 7 deletions(-)
diff --git a/drivers/i2c/busses/i2c-designware-master.c b/drivers/i2c/busses/i2c-designware-master.c
index fc7c255c80af..a1717bff06a8 100644
--- a/drivers/i2c/busses/i2c-designware-master.c
+++ b/drivers/i2c/busses/i2c-designware-master.c
@@ -274,13 +274,6 @@ i2c_dw_xfer_msg(struct dw_i2c_dev *dev)
break;
}
- if (msgs[dev->msg_write_idx].len == 0) {
- dev_err(dev->dev,
- "%s: invalid message length\n", __func__);
- dev->msg_err = -EINVAL;
- break;
- }
-
if (!(dev->status & STATUS_WRITE_IN_PROGRESS)) {
/* new i2c_msg */
buf = msgs[dev->msg_write_idx].buf;
@@ -523,6 +516,10 @@ static const struct i2c_algorithm i2c_dw_algo = {
.functionality = i2c_dw_func,
};
+static const struct i2c_adapter_quirks i2c_dw_quirks = {
+ .flags = I2C_AQ_NO_ZERO_LEN,
+};
+
static u32 i2c_dw_read_clear_intrbits(struct dw_i2c_dev *dev)
{
u32 stat;
@@ -718,6 +715,7 @@ int i2c_dw_probe(struct dw_i2c_dev *dev)
"Synopsys DesignWare I2C adapter");
adap->retries = 3;
adap->algo = &i2c_dw_algo;
+ adap->quirks = &i2c_dw_quirks;
adap->dev.parent = dev->dev;
i2c_set_adapdata(adap, dev);
--
2.11.0
And don't reimplement in the driver.
Signed-off-by: Wolfram Sang <[email protected]>
---
Only build tested.
drivers/i2c/busses/i2c-pmcmsp.c | 17 +----------------
1 file changed, 1 insertion(+), 16 deletions(-)
diff --git a/drivers/i2c/busses/i2c-pmcmsp.c b/drivers/i2c/busses/i2c-pmcmsp.c
index dae8ac618a52..0829cb696d9d 100644
--- a/drivers/i2c/busses/i2c-pmcmsp.c
+++ b/drivers/i2c/busses/i2c-pmcmsp.c
@@ -444,16 +444,6 @@ static enum pmcmsptwi_xfer_result pmcmsptwi_xfer_cmd(
{
enum pmcmsptwi_xfer_result retval;
- if ((cmd->type == MSP_TWI_CMD_WRITE && cmd->write_len == 0) ||
- (cmd->type == MSP_TWI_CMD_READ && cmd->read_len == 0) ||
- (cmd->type == MSP_TWI_CMD_WRITE_READ &&
- (cmd->read_len == 0 || cmd->write_len == 0))) {
- dev_err(&pmcmsptwi_adapter.dev,
- "%s: Cannot transfer less than 1 byte\n",
- __func__);
- return -EINVAL;
- }
-
mutex_lock(&data->lock);
dev_dbg(&pmcmsptwi_adapter.dev,
"Setting address to 0x%04x\n", cmd->addr);
@@ -532,11 +522,6 @@ static int pmcmsptwi_master_xfer(struct i2c_adapter *adap,
cmd.write_data = msg->buf;
}
- if (msg->len == 0) {
- dev_err(&adap->dev, "Zero-byte messages unsupported\n");
- return -EINVAL;
- }
-
cmd.addr = msg->addr;
if (msg->flags & I2C_M_TEN) {
@@ -578,7 +563,7 @@ static u32 pmcmsptwi_i2c_func(struct i2c_adapter *adapter)
}
static const struct i2c_adapter_quirks pmcmsptwi_i2c_quirks = {
- .flags = I2C_AQ_COMB_WRITE_THEN_READ,
+ .flags = I2C_AQ_COMB_WRITE_THEN_READ | I2C_AQ_NO_ZERO_LEN,
.max_write_len = MSP_MAX_BYTES_PER_RW,
.max_read_len = MSP_MAX_BYTES_PER_RW,
.max_comb_1st_msg_len = MSP_MAX_BYTES_PER_RW,
--
2.11.0
Some adapters do not support a message length of 0. Add this as a quirk
so drivers don't have to open code it.
Signed-off-by: Wolfram Sang <[email protected]>
---
Only build tested.
drivers/i2c/i2c-core-base.c | 6 ++++++
include/linux/i2c.h | 4 ++++
2 files changed, 10 insertions(+)
diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
index 02d6f27b19e4..a26b3e9cc441 100644
--- a/drivers/i2c/i2c-core-base.c
+++ b/drivers/i2c/i2c-core-base.c
@@ -1839,9 +1839,15 @@ static int i2c_check_for_quirks(struct i2c_adapter *adap, struct i2c_msg *msgs,
if (msgs[i].flags & I2C_M_RD) {
if (do_len_check && i2c_quirk_exceeded(len, q->max_read_len))
return i2c_quirk_error(adap, &msgs[i], "msg too long");
+
+ if (q->flags & I2C_AQ_NO_ZERO_LEN_READ && len == 0)
+ return i2c_quirk_error(adap, &msgs[i], "no zero length");
} else {
if (do_len_check && i2c_quirk_exceeded(len, q->max_write_len))
return i2c_quirk_error(adap, &msgs[i], "msg too long");
+
+ if (q->flags & I2C_AQ_NO_ZERO_LEN_WRITE && len == 0)
+ return i2c_quirk_error(adap, &msgs[i], "no zero length");
}
}
diff --git a/include/linux/i2c.h b/include/linux/i2c.h
index bc8d42f8544f..2a98d0886d2e 100644
--- a/include/linux/i2c.h
+++ b/include/linux/i2c.h
@@ -661,6 +661,10 @@ struct i2c_adapter_quirks {
I2C_AQ_COMB_READ_SECOND | I2C_AQ_COMB_SAME_ADDR)
/* clock stretching is not supported */
#define I2C_AQ_NO_CLK_STRETCH BIT(4)
+/* message cannot have length of 0 */
+#define I2C_AQ_NO_ZERO_LEN_READ BIT(5)
+#define I2C_AQ_NO_ZERO_LEN_WRITE BIT(6)
+#define I2C_AQ_NO_ZERO_LEN (I2C_AQ_NO_ZERO_LEN_READ | I2C_AQ_NO_ZERO_LEN_WRITE)
/*
* i2c_adapter is the structure used to identify a physical i2c bus along
--
2.11.0
And don't reimplement in the driver.
Signed-off-by: Wolfram Sang <[email protected]>
---
Only build tested.
drivers/i2c/busses/i2c-omap.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
index 65d06a819307..b1086bfb0465 100644
--- a/drivers/i2c/busses/i2c-omap.c
+++ b/drivers/i2c/busses/i2c-omap.c
@@ -661,9 +661,6 @@ static int omap_i2c_xfer_msg(struct i2c_adapter *adap,
dev_dbg(omap->dev, "addr: 0x%04x, len: %d, flags: 0x%x, stop: %d\n",
msg->addr, msg->len, msg->flags, stop);
- if (msg->len == 0)
- return -EINVAL;
-
omap->receiver = !!(msg->flags & I2C_M_RD);
omap_i2c_resize_fifo(omap, msg->len, omap->receiver);
@@ -1179,6 +1176,10 @@ static const struct i2c_algorithm omap_i2c_algo = {
.functionality = omap_i2c_func,
};
+static const struct i2c_adapter_quirks omap_i2c_quirks = {
+ .flags = I2C_AQ_NO_ZERO_LEN,
+};
+
#ifdef CONFIG_OF
static struct omap_i2c_bus_platform_data omap2420_pdata = {
.rev = OMAP_I2C_IP_VERSION_1,
@@ -1453,6 +1454,7 @@ omap_i2c_probe(struct platform_device *pdev)
adap->class = I2C_CLASS_DEPRECATED;
strlcpy(adap->name, "OMAP I2C adapter", sizeof(adap->name));
adap->algo = &omap_i2c_algo;
+ adap->quirks = &omap_i2c_quirks;
adap->dev.parent = &pdev->dev;
adap->dev.of_node = pdev->dev.of_node;
adap->bus_recovery_info = &omap_i2c_bus_recovery_info;
--
2.11.0
Hi Wolfram,
Thanks for your patch.
On 2018-07-23 22:26:05 +0200, Wolfram Sang wrote:
> Some adapters do not support a message length of 0. Add this as a quirk
> so drivers don't have to open code it.
>
> Signed-off-by: Wolfram Sang <[email protected]>
> ---
>
> Only build tested.
Was this not tested when you also tested i2c-rcar and i2c-sh_mobile
drivers? In any case I think this change make much sens.
Reviewed-by: Niklas S?derlund <[email protected]>
>
> drivers/i2c/i2c-core-base.c | 6 ++++++
> include/linux/i2c.h | 4 ++++
> 2 files changed, 10 insertions(+)
>
> diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
> index 02d6f27b19e4..a26b3e9cc441 100644
> --- a/drivers/i2c/i2c-core-base.c
> +++ b/drivers/i2c/i2c-core-base.c
> @@ -1839,9 +1839,15 @@ static int i2c_check_for_quirks(struct i2c_adapter *adap, struct i2c_msg *msgs,
> if (msgs[i].flags & I2C_M_RD) {
> if (do_len_check && i2c_quirk_exceeded(len, q->max_read_len))
> return i2c_quirk_error(adap, &msgs[i], "msg too long");
> +
> + if (q->flags & I2C_AQ_NO_ZERO_LEN_READ && len == 0)
> + return i2c_quirk_error(adap, &msgs[i], "no zero length");
> } else {
> if (do_len_check && i2c_quirk_exceeded(len, q->max_write_len))
> return i2c_quirk_error(adap, &msgs[i], "msg too long");
> +
> + if (q->flags & I2C_AQ_NO_ZERO_LEN_WRITE && len == 0)
> + return i2c_quirk_error(adap, &msgs[i], "no zero length");
> }
> }
>
> diff --git a/include/linux/i2c.h b/include/linux/i2c.h
> index bc8d42f8544f..2a98d0886d2e 100644
> --- a/include/linux/i2c.h
> +++ b/include/linux/i2c.h
> @@ -661,6 +661,10 @@ struct i2c_adapter_quirks {
> I2C_AQ_COMB_READ_SECOND | I2C_AQ_COMB_SAME_ADDR)
> /* clock stretching is not supported */
> #define I2C_AQ_NO_CLK_STRETCH BIT(4)
> +/* message cannot have length of 0 */
> +#define I2C_AQ_NO_ZERO_LEN_READ BIT(5)
> +#define I2C_AQ_NO_ZERO_LEN_WRITE BIT(6)
> +#define I2C_AQ_NO_ZERO_LEN (I2C_AQ_NO_ZERO_LEN_READ | I2C_AQ_NO_ZERO_LEN_WRITE)
>
> /*
> * i2c_adapter is the structure used to identify a physical i2c bus along
> --
> 2.11.0
>
--
Regards,
Niklas S?derlund
Hi Wolfram,
Thanks for your work.
On 2018-07-23 22:26:14 +0200, Wolfram Sang wrote:
> And don't reimplement in the driver.
>
> Signed-off-by: Wolfram Sang <[email protected]>
Reviewed-by: Niklas S?derlund <[email protected]>
> ---
>
> drivers/i2c/busses/i2c-rcar.c | 13 ++++++-------
> 1 file changed, 6 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-rcar.c b/drivers/i2c/busses/i2c-rcar.c
> index 2688520110d1..791a4aa34fdd 100644
> --- a/drivers/i2c/busses/i2c-rcar.c
> +++ b/drivers/i2c/busses/i2c-rcar.c
> @@ -796,14 +796,8 @@ static int rcar_i2c_master_xfer(struct i2c_adapter *adap,
> if (ret < 0)
> goto out;
>
> - for (i = 0; i < num; i++) {
> - /* This HW can't send STOP after address phase */
> - if (msgs[i].len == 0) {
> - ret = -EOPNOTSUPP;
> - goto out;
> - }
> + for (i = 0; i < num; i++)
> rcar_i2c_request_dma(priv, msgs + i);
> - }
>
> /* init first message */
> priv->msg = msgs;
> @@ -890,6 +884,10 @@ static const struct i2c_algorithm rcar_i2c_algo = {
> .unreg_slave = rcar_unreg_slave,
> };
>
> +static const struct i2c_adapter_quirks rcar_i2c_quirks = {
> + .flags = I2C_AQ_NO_ZERO_LEN,
> +};
> +
> static const struct of_device_id rcar_i2c_dt_ids[] = {
> { .compatible = "renesas,i2c-r8a7778", .data = (void *)I2C_RCAR_GEN1 },
> { .compatible = "renesas,i2c-r8a7779", .data = (void *)I2C_RCAR_GEN1 },
> @@ -943,6 +941,7 @@ static int rcar_i2c_probe(struct platform_device *pdev)
> adap->dev.parent = dev;
> adap->dev.of_node = dev->of_node;
> adap->bus_recovery_info = &rcar_i2c_bri;
> + adap->quirks = &rcar_i2c_quirks;
> i2c_set_adapdata(adap, priv);
> strlcpy(adap->name, pdev->name, sizeof(adap->name));
>
> --
> 2.11.0
>
--
Regards,
Niklas S?derlund
On Mon, Jul 23, 2018 at 11:26 PM, Wolfram Sang
<[email protected]> wrote:
> I had this idea for quite some time on my todo list but a soon to be
> implemented refactoring in the i2c-rcar driver now finally made me do it. Add a
> 'can't do 0 length messages' quirk to the quirk infrastructure for and remove
> the manual handling from the drivers. This makes the quirk much more visible.
> (Quite some prominent vendors in that list) We also have a centralized place to
> handle updates to the quirk detection if that is ever needed.
>
> I have tested this with the i2c-rcar and i2c-sh_mobile driver on a Renesas
> SalvatorXS board equipped with M3-N (r8a77965).
>
> A git branch can be found here:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git i2c/quirk-no-zero-len
>
> Looking forward to comments, reviews, tests...
Thanks!
Reviewed-by: Andy Shevchenko <[email protected]>
for patches 1 and 2.
>
> Thanks,
>
> Wolfram
>
> Wolfram Sang (12):
> i2c: quirks: add zero length checks
> i2c: designware-master: use core to detect 'no zero length' quirk
> i2c: mxs: use core to detect 'no zero length' quirk
> i2c: omap: use core to detect 'no zero length' quirk
> i2c: pmcmsp: use core to detect 'no zero length' quirk
> i2c: qup: use core to detect 'no zero length' quirk
> i2c: stu300: use core to detect 'no zero length' quirk
> i2c: tegra: use core to detect 'no zero length' quirk
> i2c: zx2967: use core to detect 'no zero length' quirk
> i2c: rcar: use core to detect 'no zero length' quirk
> i2c: xlr: use core to detect 'no zero length' quirk
> i2c: sh_mobile: use core to detect 'no zero length read' quirk
>
> drivers/i2c/busses/i2c-designware-master.c | 12 +++++-------
> drivers/i2c/busses/i2c-mxs.c | 8 +++++---
> drivers/i2c/busses/i2c-omap.c | 8 +++++---
> drivers/i2c/busses/i2c-pmcmsp.c | 17 +----------------
> drivers/i2c/busses/i2c-qup.c | 14 ++++++--------
> drivers/i2c/busses/i2c-rcar.c | 13 ++++++-------
> drivers/i2c/busses/i2c-sh_mobile.c | 10 +++++-----
> drivers/i2c/busses/i2c-stu300.c | 12 ++++++------
> drivers/i2c/busses/i2c-tegra.c | 4 +---
> drivers/i2c/busses/i2c-xlr.c | 11 +++++------
> drivers/i2c/busses/i2c-zx2967.c | 8 +++++---
> drivers/i2c/i2c-core-base.c | 6 ++++++
> include/linux/i2c.h | 4 ++++
> 13 files changed, 60 insertions(+), 67 deletions(-)
>
> --
> 2.11.0
>
--
With Best Regards,
Andy Shevchenko
Hi Wolfram,
Thanks for your work.
On 2018-07-23 22:26:16 +0200, Wolfram Sang wrote:
> And don't reimplement in the driver.
>
> Signed-off-by: Wolfram Sang <[email protected]>
Reviewed-by: Niklas S?derlund <[email protected]>
> ---
>
> drivers/i2c/busses/i2c-sh_mobile.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-sh_mobile.c b/drivers/i2c/busses/i2c-sh_mobile.c
> index 5fda4188a9e5..9c7f6f8ceb22 100644
> --- a/drivers/i2c/busses/i2c-sh_mobile.c
> +++ b/drivers/i2c/busses/i2c-sh_mobile.c
> @@ -613,11 +613,6 @@ static void sh_mobile_i2c_xfer_dma(struct sh_mobile_i2c_data *pd)
> static int start_ch(struct sh_mobile_i2c_data *pd, struct i2c_msg *usr_msg,
> bool do_init)
> {
> - if (usr_msg->len == 0 && (usr_msg->flags & I2C_M_RD)) {
> - dev_err(pd->dev, "Unsupported zero length i2c read\n");
> - return -EOPNOTSUPP;
> - }
> -
> if (do_init) {
> /* Initialize channel registers */
> iic_wr(pd, ICCR, ICCR_SCP);
> @@ -758,6 +753,10 @@ static const struct i2c_algorithm sh_mobile_i2c_algorithm = {
> .master_xfer = sh_mobile_i2c_xfer,
> };
>
> +static const struct i2c_adapter_quirks sh_mobile_i2c_quirks = {
> + .flags = I2C_AQ_NO_ZERO_LEN_READ,
> +};
> +
> /*
> * r8a7740 chip has lasting errata on I2C I/O pad reset.
> * this is work-around for it.
> @@ -925,6 +924,7 @@ static int sh_mobile_i2c_probe(struct platform_device *dev)
>
> adap->owner = THIS_MODULE;
> adap->algo = &sh_mobile_i2c_algorithm;
> + adap->quirks = &sh_mobile_i2c_quirks;
> adap->dev.parent = &dev->dev;
> adap->retries = 5;
> adap->nr = dev->id;
> --
> 2.11.0
>
--
Regards,
Niklas S?derlund
> > Only build tested.
>
> Was this not tested when you also tested i2c-rcar and i2c-sh_mobile
> drivers? In any case I think this change make much sens.
>
> Reviewed-by: Niklas Söderlund <[email protected]>
You are right, of course. This text was added by a script and I forgot to
remove it :( This code was tested!
Thanks for the review!
On Mon, Jul 23, 2018 at 10:26 PM Wolfram Sang
<[email protected]> wrote:
> And don't reimplement in the driver.
>
> Signed-off-by: Wolfram Sang <[email protected]>
Reviewed-by: Linus Walleij <[email protected]>
Yours,
Linus Walleij
On 07/23/2018 11:26 PM, Wolfram Sang wrote:
> And don't reimplement in the driver.
>
> Signed-off-by: Wolfram Sang <[email protected]>
> ---
>
> Only build tested.
>
> drivers/i2c/busses/i2c-designware-master.c | 12 +++++-------
> 1 file changed, 5 insertions(+), 7 deletions(-)
>
I tested this with patch 1 and I like it since with these patches the
test is done before firing up the HW, waiting for an interrupt and then
see the length is invalid for us.
i2ctransfer -f 8 r0@0x10
Before (with i2c_designware debugging on):
[ 15.607283] i2c_designware i2c_designware.1: i2c_dw_xfer: msgs: 1
[ 15.711988] i2c_designware i2c_designware.1: enabled=0x1 stat=0x10
[ 15.712002] i2c_designware i2c_designware.1: i2c_dw_xfer_msg: invalid
message length
Error: Sending messages failed: Invalid argument
After:
[ 25.838661] i2c i2c-8: adapter quirk: no zero length (addr 0x0010,
size 0, read)
Error: Sending messages failed: Operation not supported
Tested-by: Jarkko Nikula <[email protected]>
Acked-by: Jarkko Nikula <[email protected]>
On Mon, Jul 23, 2018 at 10:26:04PM +0200, Wolfram Sang wrote:
> I had this idea for quite some time on my todo list but a soon to be
> implemented refactoring in the i2c-rcar driver now finally made me do it. Add a
> 'can't do 0 length messages' quirk to the quirk infrastructure for and remove
> the manual handling from the drivers. This makes the quirk much more visible.
> (Quite some prominent vendors in that list) We also have a centralized place to
> handle updates to the quirk detection if that is ever needed.
>
> I have tested this with the i2c-rcar and i2c-sh_mobile driver on a Renesas
> SalvatorXS board equipped with M3-N (r8a77965).
>
> A git branch can be found here:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git i2c/quirk-no-zero-len
>
> Looking forward to comments, reviews, tests...
I applied all the patches which either got acks from the maintainers or
have no maintainers. I'll reply individually to which I applied. I will
wait some more for other acks before I'll resend next cycle.
> Reviewed-by: Andy Shevchenko <[email protected]>
>
> for patches 1 and 2.
Thanks, Andy. If you could reply with that to the individual patches,
then patchwork picks up the tags for me making my life a little easier.
On Mon, Jul 23, 2018 at 10:26:06PM +0200, Wolfram Sang wrote:
> And don't reimplement in the driver.
>
> Signed-off-by: Wolfram Sang <[email protected]>
Applied to for-next, thanks!
On Mon, Jul 23, 2018 at 10:26:07PM +0200, Wolfram Sang wrote:
> And don't reimplement in the driver.
>
> Signed-off-by: Wolfram Sang <[email protected]>
Applied to for-next, thanks!
On Mon, Jul 23, 2018 at 10:26:05PM +0200, Wolfram Sang wrote:
> Some adapters do not support a message length of 0. Add this as a quirk
> so drivers don't have to open code it.
>
> Signed-off-by: Wolfram Sang <[email protected]>
Applied to for-next, thanks!
On Mon, Jul 23, 2018 at 10:26:11PM +0200, Wolfram Sang wrote:
> And don't reimplement in the driver.
>
> Signed-off-by: Wolfram Sang <[email protected]>
Applied to for-next, thanks!
On Mon, Jul 23, 2018 at 10:26:09PM +0200, Wolfram Sang wrote:
> And don't reimplement in the driver.
>
> Signed-off-by: Wolfram Sang <[email protected]>
Applied to for-next, thanks!
On Mon, Jul 23, 2018 at 10:26:15PM +0200, Wolfram Sang wrote:
> And don't reimplement in the driver.
>
> Signed-off-by: Wolfram Sang <[email protected]>
Applied to for-next, thanks!
On Mon, Jul 23, 2018 at 10:26:16PM +0200, Wolfram Sang wrote:
> And don't reimplement in the driver.
>
> Signed-off-by: Wolfram Sang <[email protected]>
Applied to for-next, thanks!
On Mon, Jul 23, 2018 at 10:26:14PM +0200, Wolfram Sang wrote:
> And don't reimplement in the driver.
>
> Signed-off-by: Wolfram Sang <[email protected]>
Applied to for-next, thanks!
On Mon, Jul 23, 2018 at 10:26:08PM +0200, Wolfram Sang wrote:
> And don't reimplement in the driver.
>
> Signed-off-by: Wolfram Sang <[email protected]>
Ping.
On Mon, Jul 23, 2018 at 10:26:10PM +0200, Wolfram Sang wrote:
> And don't reimplement in the driver.
>
> Signed-off-by: Wolfram Sang <[email protected]>
Ping.
On Mon, Jul 23, 2018 at 10:26:12PM +0200, Wolfram Sang wrote:
> And don't reimplement in the driver.
>
> Signed-off-by: Wolfram Sang <[email protected]>
Ping.
On Mon, Jul 23, 2018 at 10:26:13PM +0200, Wolfram Sang wrote:
> And don't reimplement in the driver.
>
> Signed-off-by: Wolfram Sang <[email protected]>
Ping.
On Mon, Jul 23, 2018 at 10:26:13PM +0200, Wolfram Sang wrote:
> And don't reimplement in the driver.
>
> Signed-off-by: Wolfram Sang <[email protected]>
Acked-by: Shawn Guo <[email protected]>
On 23/07/18 21:26, Wolfram Sang wrote:
> And don't reimplement in the driver.
>
> Signed-off-by: Wolfram Sang <[email protected]>
> ---
>
> Only build tested.
>
> drivers/i2c/busses/i2c-tegra.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c
> index 60c8561fbe65..437294ea2f0a 100644
> --- a/drivers/i2c/busses/i2c-tegra.c
> +++ b/drivers/i2c/busses/i2c-tegra.c
> @@ -684,9 +684,6 @@ static int tegra_i2c_xfer_msg(struct tegra_i2c_dev *i2c_dev,
>
> tegra_i2c_flush_fifos(i2c_dev);
>
> - if (msg->len == 0)
> - return -EINVAL;
> -
> i2c_dev->msg_buf = msg->buf;
> i2c_dev->msg_buf_remaining = msg->len;
> i2c_dev->msg_err = I2C_ERR_NONE;
> @@ -831,6 +828,7 @@ static const struct i2c_algorithm tegra_i2c_algo = {
>
> /* payload size is only 12 bit */
> static const struct i2c_adapter_quirks tegra_i2c_quirks = {
> + .flags = I2C_AQ_NO_ZERO_LEN,
> .max_read_len = 4096,
> .max_write_len = 4096,
> };
Sorry for missing this, but looks fine to me, so ...
Acked-by: Jon Hunter <[email protected]>
Cheers
Jon
--
nvpublic
On 10/05/2018 07:24 AM, Wolfram Sang wrote:
> On Mon, Jul 23, 2018 at 10:26:08PM +0200, Wolfram Sang wrote:
>> And don't reimplement in the driver.
>>
>> Signed-off-by: Wolfram Sang <[email protected]>
>
> Ping.
>
Reviewed-by: Grygorii Strashko <[email protected]>
--
regards,
-grygorii
On Mon, Jul 23, 2018 at 10:26:10PM +0200, Wolfram Sang wrote:
> And don't reimplement in the driver.
>
> Signed-off-by: Wolfram Sang <[email protected]>
> ---
Reviewed-by: Andy Gross <[email protected]>
* Grygorii Strashko <[email protected]> [181005 14:16]:
>
>
> On 10/05/2018 07:24 AM, Wolfram Sang wrote:
> > On Mon, Jul 23, 2018 at 10:26:08PM +0200, Wolfram Sang wrote:
> > > And don't reimplement in the driver.
> > >
> > > Signed-off-by: Wolfram Sang <[email protected]>
> >
> > Ping.
> >
>
> Reviewed-by: Grygorii Strashko <[email protected]>
Acked-by: Tony Lindgren <[email protected]>
On Mon, Jul 23, 2018 at 10:26:08PM +0200, Wolfram Sang wrote:
> And don't reimplement in the driver.
>
> Signed-off-by: Wolfram Sang <[email protected]>
Applied to for-next, thanks!
On Mon, Jul 23, 2018 at 10:26:10PM +0200, Wolfram Sang wrote:
> And don't reimplement in the driver.
>
> Signed-off-by: Wolfram Sang <[email protected]>
Applied to for-next, thanks!
On Mon, Jul 23, 2018 at 10:26:13PM +0200, Wolfram Sang wrote:
> And don't reimplement in the driver.
>
> Signed-off-by: Wolfram Sang <[email protected]>
Applied to for-next, thanks!
On Mon, Jul 23, 2018 at 10:26:12PM +0200, Wolfram Sang wrote:
> And don't reimplement in the driver.
>
> Signed-off-by: Wolfram Sang <[email protected]>
Applied to for-next, thanks!