From: Michael Zaidman <michael.zaidman@gmail.com>
To: jikos@kernel.org
Cc: linux-kernel@vger.kernel.org, linux-input@vger.kernel.org,
linux-i2c@vger.kernel.org, germain.hebert@ca.abb.com,
Enrik.Berkhan@inka.de,
Michael Zaidman <michael.zaidman@gmail.com>
Subject: [PATCH v3 12/12] HID: ft260: missed NACK from busy device
Date: Sun, 30 Oct 2022 22:34:03 +0200 [thread overview]
Message-ID: <20221030203403.4637-13-michael.zaidman@gmail.com> (raw)
In-Reply-To: <20221030203403.4637-1-michael.zaidman@gmail.com>
When writing into a slow device like an EEPROM chip, the
controller may exit the busy state before the device releases
the bus. In this case, the ft260_xfer_status returns success
before the data transfer completion.
The patch fixes it by returning from the ft260_xfer_status()
with the "-EAGAIN" on both controller and bus busy status when
appropriate.
It does not apply to the i2c combined transactions when after
the write IO, the controller keeps the bus busy until the read
IO and then between reading IOs to ensure an atomic operation.
Co-developed-by: Germain Hebert <germain.hebert@ca.abb.com>
Signed-off-by: Germain Hebert <germain.hebert@ca.abb.com>
Signed-off-by: Michael Zaidman <michael.zaidman@gmail.com>
---
drivers/hid/hid-ft260.c | 31 ++++++++++++++++++++++++-------
1 file changed, 24 insertions(+), 7 deletions(-)
diff --git a/drivers/hid/hid-ft260.c b/drivers/hid/hid-ft260.c
index b3f715f6ea86..da8ea0d16059 100644
--- a/drivers/hid/hid-ft260.c
+++ b/drivers/hid/hid-ft260.c
@@ -303,7 +303,7 @@ static int ft260_i2c_reset(struct hid_device *hdev)
return ret;
}
-static int ft260_xfer_status(struct ft260_device *dev)
+static int ft260_xfer_status(struct ft260_device *dev, u8 bus_busy)
{
struct hid_device *hdev = dev->hdev;
struct ft260_get_i2c_status_report report;
@@ -320,7 +320,7 @@ static int ft260_xfer_status(struct ft260_device *dev)
ft260_dbg("bus_status %#02x, clock %u\n", report.bus_status,
dev->clock);
- if (report.bus_status & FT260_I2C_STATUS_CTRL_BUSY)
+ if (report.bus_status & (FT260_I2C_STATUS_CTRL_BUSY | bus_busy))
return -EAGAIN;
/*
@@ -355,8 +355,11 @@ static int ft260_hid_output_report(struct hid_device *hdev, u8 *data,
static int ft260_hid_output_report_check_status(struct ft260_device *dev,
u8 *data, int len)
{
+ u8 bus_busy;
int ret, usec, try = 100;
struct hid_device *hdev = dev->hdev;
+ struct ft260_i2c_write_request_report *rep =
+ (struct ft260_i2c_write_request_report *)data;
ret = ft260_hid_output_report(hdev, data, len);
if (ret < 0) {
@@ -374,8 +377,18 @@ static int ft260_hid_output_report_check_status(struct ft260_device *dev,
ft260_dbg("wait %d usec, len %d\n", usec, len);
}
+ /*
+ * Do not check the busy bit for combined transactions
+ * since the controller keeps the bus busy between writing
+ * and reading IOs to ensure an atomic operation.
+ */
+ if (rep->flag == FT260_FLAG_START)
+ bus_busy = 0;
+ else
+ bus_busy = FT260_I2C_STATUS_BUS_BUSY;
+
do {
- ret = ft260_xfer_status(dev);
+ ret = ft260_xfer_status(dev, bus_busy);
if (ret != -EAGAIN)
break;
} while (--try);
@@ -399,7 +412,7 @@ static int ft260_i2c_write(struct ft260_device *dev, u8 addr, u8 *data,
return -EINVAL;
if (time_is_before_jiffies(dev->need_wakeup_at)) {
- (void)ft260_xfer_status(dev);
+ (void)ft260_xfer_status(dev, 0);
ft260_dbg("device wakeup");
}
@@ -453,7 +466,7 @@ static int ft260_smbus_write(struct ft260_device *dev, u8 addr, u8 cmd,
return -EINVAL;
if (time_is_before_jiffies(dev->need_wakeup_at)) {
- (void)ft260_xfer_status(dev);
+ (void)ft260_xfer_status(dev, 0);
ft260_dbg("device wakeup");
}
@@ -484,6 +497,7 @@ static int ft260_i2c_read(struct ft260_device *dev, u8 addr, u8 *data,
int timeout, ret = 0;
struct ft260_i2c_read_request_report rep;
struct hid_device *hdev = dev->hdev;
+ u8 bus_busy = 0;
if ((flag & FT260_FLAG_START_REPEATED) == FT260_FLAG_START_REPEATED)
flag = FT260_FLAG_START_REPEATED;
@@ -527,7 +541,10 @@ static int ft260_i2c_read(struct ft260_device *dev, u8 addr, u8 *data,
dev->read_buf = NULL;
- ret = ft260_xfer_status(dev);
+ if (flag & FT260_FLAG_STOP)
+ bus_busy = FT260_I2C_STATUS_BUS_BUSY;
+
+ ret = ft260_xfer_status(dev, bus_busy);
if (ret < 0) {
ret = -EIO;
ft260_i2c_reset(hdev);
@@ -1003,7 +1020,7 @@ static int ft260_probe(struct hid_device *hdev, const struct hid_device_id *id)
mutex_init(&dev->lock);
init_completion(&dev->wait);
- ret = ft260_xfer_status(dev);
+ ret = ft260_xfer_status(dev, FT260_I2C_STATUS_BUS_BUSY);
if (ret)
ft260_i2c_reset(hdev);
--
2.34.1
prev parent reply other threads:[~2022-10-30 20:35 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-10-30 20:33 [PATCH v3 00/12] HID: ft260: fixes and performance improvements Michael Zaidman
2022-10-30 20:33 ` [PATCH v3 01/12] HID: ft260: ft260_xfer_status routine cleanup Michael Zaidman
2022-10-30 20:33 ` [PATCH v3 02/12] HID: ft260: improve i2c write performance Michael Zaidman
2022-10-30 20:33 ` [PATCH v3 03/12] HID: ft260: support i2c writes larger than HID report size Michael Zaidman
2022-10-30 20:33 ` [PATCH v3 04/12] HID: ft260: support i2c reads greater " Michael Zaidman
2022-10-30 20:33 ` [PATCH v3 05/12] HID: ft260: improve i2c large reads performance Michael Zaidman
2022-10-30 20:33 ` [PATCH v3 06/12] HID: ft260: do not populate /dev/hidraw device Michael Zaidman
2022-10-30 20:33 ` [PATCH v3 07/12] HID: ft260: skip unexpected HID input reports Michael Zaidman
2022-10-30 20:33 ` [PATCH v3 08/12] HID: ft260: remove SMBus Quick command support Michael Zaidman
2022-10-30 20:34 ` [PATCH v3 09/12] HID: ft260: missed NACK from big i2c read Michael Zaidman
2022-10-30 20:34 ` [PATCH v3 10/12] HID: ft260: wake up device from power saving mode Michael Zaidman
2022-10-31 17:12 ` Enrik Berkhan
2022-10-30 20:34 ` [PATCH v3 11/12] HID: ft260: fix a NULL pointer dereference in ft260_i2c_write Michael Zaidman
2022-10-30 20:34 ` Michael Zaidman [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20221030203403.4637-13-michael.zaidman@gmail.com \
--to=michael.zaidman@gmail.com \
--cc=Enrik.Berkhan@inka.de \
--cc=germain.hebert@ca.abb.com \
--cc=jikos@kernel.org \
--cc=linux-i2c@vger.kernel.org \
--cc=linux-input@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).