diff mbox

[v9,15/18] input: cyapa: add gen3 trackpad device read firmware image function support

Message ID 1415003590-30485-16-git-send-email-dudl@cypress.com (mailing list archive)
State New, archived
Headers show

Commit Message

Dudley Du Nov. 3, 2014, 8:33 a.m. UTC
Add read firmware image function supported for gen3 trackpad device,
it can be used through debugfs read_fw interface.
TEST=test on Chromebooks.

Signed-off-by: Dudley Du <dudl@cypress.com>
---
 drivers/input/mouse/cyapa_gen3.c | 67 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 67 insertions(+)

Comments

Dmitry Torokhov Nov. 10, 2014, 8:39 a.m. UTC | #1
On Mon, Nov 03, 2014 at 04:33:07PM +0800, Dudley Du wrote:
> Add read firmware image function supported for gen3 trackpad device,
> it can be used through debugfs read_fw interface.

Why do we need this? Can we do the same via usespace program accessing
the i2c device through /dev/i2c-N?

Thanks.
Dudley Du Nov. 10, 2014, 11:05 a.m. UTC | #2
Thanks, Dmitry

> -----Original Message-----
> From: Dmitry Torokhov [mailto:dmitry.torokhov@gmail.com]
> Sent: 2014?11?10? 16:40
> To: Dudley Du
> Cc: rydberg@euromail.se; Dudley Du; bleung@google.com;
> linux-input@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH v9 15/18] input: cyapa: add gen3 trackpad device read
> firmware image function support
>
> On Mon, Nov 03, 2014 at 04:33:07PM +0800, Dudley Du wrote:
> > Add read firmware image function supported for gen3 trackpad device,
> > it can be used through debugfs read_fw interface.
>
> Why do we need this? Can we do the same via usespace program accessing
> the i2c device through /dev/i2c-N?

This interface is used to test and debug only. It's defined and required by the chromium projects.

And in the driver side, becase the firmware image read process is done based on interrupt signal,
if do this in userspace through /dev/i2c-N, there will be two issues:
1) for gen5, after each command, an interrupt will be asserted, so if throug /dev/i2c-N,
userspace program cannot get the interrupt signal.
2) and when the interrupt signal assert, driver won't know it’s a command signal for image read,
so it will try to process it as data report.
To avoid this, additional interface must be added to mark the image read status and
block interrupt signal to be process as data report.

>
> Thanks.
>
> --
> Dmitry

This message and any attachments may contain Cypress (or its subsidiaries) confidential information. If it has been received in error, please advise the sender and immediately delete this message.
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Dmitry Torokhov Dec. 4, 2014, 5:46 p.m. UTC | #3
On Mon, Nov 10, 2014 at 11:05:39AM +0000, Dudley Du wrote:
> Thanks, Dmitry
> 
> > -----Original Message-----
> > From: Dmitry Torokhov [mailto:dmitry.torokhov@gmail.com]
> > Sent: 2014?11?10? 16:40
> > To: Dudley Du
> > Cc: rydberg@euromail.se; Dudley Du; bleung@google.com;
> > linux-input@vger.kernel.org; linux-kernel@vger.kernel.org
> > Subject: Re: [PATCH v9 15/18] input: cyapa: add gen3 trackpad device read
> > firmware image function support
> >
> > On Mon, Nov 03, 2014 at 04:33:07PM +0800, Dudley Du wrote:
> > > Add read firmware image function supported for gen3 trackpad device,
> > > it can be used through debugfs read_fw interface.
> >
> > Why do we need this? Can we do the same via usespace program accessing
> > the i2c device through /dev/i2c-N?
> 
> This interface is used to test and debug only. It's defined and required by the chromium projects.
> 
> And in the driver side, becase the firmware image read process is done based on interrupt signal,
> if do this in userspace through /dev/i2c-N, there will be two issues:
> 1) for gen5, after each command, an interrupt will be asserted, so if throug /dev/i2c-N,
> userspace program cannot get the interrupt signal.
> 2) and when the interrupt signal assert, driver won't know it’s a command signal for image read,
> so it will try to process it as data report.
> To avoid this, additional interface must be added to mark the image read status and
> block interrupt signal to be process as data report.

I believe that while userspace acc4esses the device through /dev/i2c-N
you should unbind the cyapa driver from the device (via sysfs driver
core bind/unbind attribute) so that it doe snot get in the way. Once
diagnostics is done you can bind the driver back to device, restoring
normal operation.

That is how we are planning to handle other devices at factory as
well.

Thanks.
diff mbox

Patch

diff --git a/drivers/input/mouse/cyapa_gen3.c b/drivers/input/mouse/cyapa_gen3.c
index b477d67..f8aeb58 100644
--- a/drivers/input/mouse/cyapa_gen3.c
+++ b/drivers/input/mouse/cyapa_gen3.c
@@ -710,6 +710,36 @@  static int cyapa_gen3_write_fw_block(struct cyapa *cyapa,
 	return ret;
 }
 
+/*
+ * A firmware block read command reads 16 bytes of data from flash starting
+ * from a given address.  The 12-byte block read command has the format:
+ *   <0xff> <CMD> <Key> <Addr>
+ *
+ *  <0xff>  - every command starts with 0xff
+ *  <CMD>   - the read command value is 0x3c
+ *  <Key>   - read commands include an 8-byte key: { 00 01 02 03 04 05 06 07 }
+ *  <Addr>  - Memory address (16-bit, big-endian)
+ *
+ * The command is followed by an i2c block read to read the 16 bytes of data.
+ */
+static int cyapa_gen3_read_fw_bytes(struct cyapa *cyapa, u16 addr, u8 *data)
+{
+	int ret;
+	u8 cmd[] = { 0xff, 0x3c, 0x00, 0x01, 0x02, 0x03, 0x04,
+		0x05, 0x06, 0x07, addr >> 8, addr };
+
+	ret = cyapa_gen3_write_buffer(cyapa, cmd, sizeof(cmd));
+	if (ret)
+		return ret;
+
+	/* Read data buffer starting from offset 16 */
+	ret = cyapa_i2c_reg_read_block(cyapa, 16, CYAPA_FW_READ_SIZE, data);
+	if (ret != CYAPA_FW_READ_SIZE)
+		return (ret < 0) ? ret : -EIO;
+
+	return 0;
+}
+
 static int cyapa_gen3_write_blocks(struct cyapa *cyapa,
 		size_t start_block, size_t block_count,
 		const u8 *image_data)
@@ -756,6 +786,41 @@  static int cyapa_gen3_do_fw_update(struct cyapa *cyapa,
 	return 0;
 }
 
+/*
+ * Read the entire firmware image into ->fw_image.
+ * If the ->fw_image has already been allocated, then this function
+ * doesn't do anything and just returns 0.
+ * If an error occurs while reading the image, ->fw_image is freed, and
+ * the error is returned.
+ *
+ * The firmware is a fixed size (CYAPA_FW_SIZE), and is read out in
+ * fixed length (CYAPA_FW_READ_SIZE) chunks.
+ */
+static int cyapa_gen3_read_fw(struct cyapa *cyapa)
+{
+	int ret;
+	int addr;
+
+	ret = cyapa_gen3_bl_enter(cyapa);
+	if (ret)
+		return ret;
+
+	cyapa->fw_image = cyapa->fw_image ? cyapa->fw_image :
+		devm_kzalloc(&cyapa->client->dev, CYAPA_FW_SIZE, GFP_KERNEL);
+	if (!cyapa->fw_image)
+		return -ENOMEM;
+
+	for (addr = 0; addr < CYAPA_FW_SIZE; addr += CYAPA_FW_READ_SIZE) {
+		ret = cyapa_gen3_read_fw_bytes(cyapa, CYAPA_FW_HDR_START + addr,
+					  &cyapa->fw_image[addr]);
+		if (ret)
+			return ret;
+	}
+
+	cyapa->fw_image_size = CYAPA_FW_SIZE;
+	return 0;
+}
+
 static ssize_t cyapa_gen3_do_calibrate(struct device *dev,
 				     struct device_attribute *attr,
 				     const char *buf, size_t count)
@@ -1203,6 +1268,8 @@  const struct cyapa_dev_ops cyapa_gen3_ops = {
 	.show_baseline = cyapa_gen3_show_baseline,
 	.calibrate_store = cyapa_gen3_do_calibrate,
 
+	.read_fw = cyapa_gen3_read_fw,
+
 	.state_parse = cyapa_gen3_state_parse,
 	.operational_check = cyapa_gen3_do_operational_check,