[PATCH 1/2] Input: synaptics - print firmware ID and board number at init

[PATCH 1/2] Input: synaptics - print firmware ID and board number at init

[Daniel Kurtz]

Read the Firmware ID and Board Number from a synaptics device at init
and display them in the system log.

Signed-off-by: Daniel Kurtz <djkurtz@chromium.org>
---
 drivers/input/mouse/synaptics.c |   38 ++++++++++++++++++++++++++++++++++++--
 drivers/input/mouse/synaptics.h |    3 +++
 2 files changed, 39 insertions(+), 2 deletions(-)

diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
index c778f27..81685c1 100644
--- a/drivers/input/mouse/synaptics.c
+++ b/drivers/input/mouse/synaptics.c
@@ -139,6 +139,35 @@ static int synaptics_model_id(struct psmouse *psmouse)
 }
 
 /*
+ * Read the board id from the touchpad
+ * The board id is encoded in the "QUERY MODES" response
+ */
+static int synaptics_board_id(struct psmouse *psmouse)
+{
+	struct synaptics_data *priv = psmouse->private;
+	unsigned char bid[3];
+
+	if (synaptics_send_cmd(psmouse, SYN_QUE_MODES, bid))
+		return -1;
+	priv->board_id = (((unsigned)bid[0] & 0xfc) << 6) | (unsigned)bid[1];
+	return 0;
+}
+
+/*
+ * Read the firmware id from the touchpad
+ */
+static int synaptics_firmware_id(struct psmouse *psmouse)
+{
+	struct synaptics_data *priv = psmouse->private;
+	unsigned char fwid[3];
+
+	if (synaptics_send_cmd(psmouse, SYN_QUE_FIRMWARE_ID, fwid))
+		return -1;
+	priv->firmware_id = (fwid[0] << 16) | (fwid[1] << 8) | fwid[2];
+	return 0;
+}
+
+/*
  * Read the capability-bits from the touchpad
  * see also the SYN_CAP_* macros
  */
@@ -261,6 +290,10 @@ static int synaptics_query_hardware(struct psmouse *psmouse)
 		return -1;
 	if (synaptics_model_id(psmouse))
 		return -1;
+	if (synaptics_firmware_id(psmouse))
+		return -1;
+	if (synaptics_board_id(psmouse))
+		return -1;
 	if (synaptics_capability(psmouse))
 		return -1;
 	if (synaptics_resolution(psmouse))
@@ -1434,11 +1467,12 @@ static int __synaptics_init(struct psmouse *psmouse, bool absolute_mode)
 	priv->pkt_type = SYN_MODEL_NEWABS(priv->model_id) ? SYN_NEWABS : SYN_OLDABS;
 
 	psmouse_info(psmouse,
-		     "Touchpad model: %ld, fw: %ld.%ld, id: %#lx, caps: %#lx/%#lx/%#lx\n",
+		     "Touchpad model: %ld, fw: %ld.%ld, id: %#lx, caps: %#lx/%#lx/%#lx, board id: %lu, fw id: %lu\n",
 		     SYN_ID_MODEL(priv->identity),
 		     SYN_ID_MAJOR(priv->identity), SYN_ID_MINOR(priv->identity),
 		     priv->model_id,
-		     priv->capabilities, priv->ext_cap, priv->ext_cap_0c);
+		     priv->capabilities, priv->ext_cap, priv->ext_cap_0c,
+		     priv->board_id, priv->firmware_id);
 
 	set_input_params(psmouse->dev, priv);
 
diff --git a/drivers/input/mouse/synaptics.h b/drivers/input/mouse/synaptics.h
index fd26ccc..e594af0 100644
--- a/drivers/input/mouse/synaptics.h
+++ b/drivers/input/mouse/synaptics.h
@@ -18,6 +18,7 @@
 #define SYN_QUE_SERIAL_NUMBER_SUFFIX	0x07
 #define SYN_QUE_RESOLUTION		0x08
 #define SYN_QUE_EXT_CAPAB		0x09
+#define SYN_QUE_FIRMWARE_ID		0x0a
 #define SYN_QUE_EXT_CAPAB_0C		0x0c
 #define SYN_QUE_EXT_MAX_COORDS		0x0d
 #define SYN_QUE_EXT_MIN_COORDS		0x0f
@@ -148,6 +149,8 @@ struct synaptics_hw_state {
 struct synaptics_data {
 	/* Data read from the touchpad */
 	unsigned long int model_id;		/* Model-ID */
+	unsigned long int firmware_id;		/* Firmware-ID */
+	unsigned long int board_id;		/* Board-ID */
 	unsigned long int capabilities;		/* Capabilities */
 	unsigned long int ext_cap;		/* Extended Capabilities */
 	unsigned long int ext_cap_0c;		/* Ext Caps from 0x0c query */
-- 
1.7.7.3

[PATCH 2/2] Input: synaptics - add sysfs access to firmware ID & board ID

[Daniel Kurtz]

Allow userspace to query firmware ID and board ID, which are read and
cached during device initialization.

Signed-off-by: Daniel Kurtz <djkurtz@chromium.org>
---
 drivers/input/mouse/synaptics.c |   41 +++++++++++++++++++++++++++++++++++++++
 1 files changed, 41 insertions(+), 0 deletions(-)

diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
index 81685c1..985d550 100644
--- a/drivers/input/mouse/synaptics.c
+++ b/drivers/input/mouse/synaptics.c
@@ -1258,6 +1258,17 @@ static void set_input_params(struct input_dev *dev, struct synaptics_data *priv)
 	}
 }
 
+/*****************************************************************************
+ *	sysfs interface
+ ****************************************************************************/
+/* Sysfs entry for board ID */
+static ssize_t synaptics_show_board_id(struct psmouse *psmouse, void *data,
+				       char *buf)
+{
+	struct synaptics_data *priv = psmouse->private;
+	return scnprintf(buf, PAGE_SIZE, "%lu\n", priv->board_id);
+}
+
 static ssize_t synaptics_show_disable_gesture(struct psmouse *psmouse,
 					      void *data, char *buf)
 {
@@ -1296,9 +1307,30 @@ static ssize_t synaptics_set_disable_gesture(struct psmouse *psmouse,
 	return len;
 }
 
+/* Sysfs entry for firmware ID */
+static ssize_t synaptics_show_firmware_id(struct psmouse *psmouse, void *data,
+					  char *buf)
+{
+	struct synaptics_data *priv = psmouse->private;
+	return scnprintf(buf, PAGE_SIZE, "%lu\n", priv->firmware_id);
+}
+
+PSMOUSE_DEFINE_RO_ATTR(board_id, S_IRUGO, NULL, synaptics_show_board_id);
 PSMOUSE_DEFINE_ATTR(disable_gesture, S_IWUSR | S_IRUGO, NULL,
 		    synaptics_show_disable_gesture,
 		    synaptics_set_disable_gesture);
+PSMOUSE_DEFINE_RO_ATTR(firmware_id, S_IRUGO, NULL, synaptics_show_firmware_id);
+
+static struct attribute *synaptics_attrs[] = {
+	&psmouse_attr_board_id.dattr.attr,
+	&psmouse_attr_firmware_id.dattr.attr,
+	NULL
+};
+
+static struct attribute_group synaptics_attr_group = {
+	.attrs = synaptics_attrs,
+};
+
 
 static void synaptics_disconnect(struct psmouse *psmouse)
 {
@@ -1308,6 +1340,9 @@ static void synaptics_disconnect(struct psmouse *psmouse)
 		device_remove_file(&psmouse->ps2dev.serio->dev,
 				   &psmouse_attr_disable_gesture.dattr);
 
+	sysfs_remove_group(&psmouse->ps2dev.serio->dev.kobj,
+			   &synaptics_attr_group);
+
 	synaptics_reset(psmouse);
 	kfree(priv);
 	psmouse->private = NULL;
@@ -1528,6 +1563,12 @@ static int __synaptics_init(struct psmouse *psmouse, bool absolute_mode)
 		}
 	}
 
+	err = sysfs_create_group(&psmouse->ps2dev.serio->dev.kobj,
+				 &synaptics_attr_group);
+	if (err)
+		psmouse_warn(psmouse,
+			    "Failed to create sysfs attributes (%d)", err);
+
 	return 0;
 
  init_fail:
-- 
1.7.7.3

Re: [PATCH 2/2] Input: synaptics - add sysfs access to firmware ID & board ID

[Henrik Rydberg]

On Thu, Jun 28, 2012 at 12:51:57PM +0800, Daniel Kurtz wrote:
> Allow userspace to query firmware ID and board ID, which are read and
> cached during device initialization.
> 
> Signed-off-by: Daniel Kurtz <djkurtz@chromium.org>
> ---
>  drivers/input/mouse/synaptics.c |   41 +++++++++++++++++++++++++++++++++++++++
>  1 files changed, 41 insertions(+), 0 deletions(-)

Since there is no common way to extract the firmware id for input
devices, any userland code that wants to use these sysfs nodes needs
to know about the details of this specific driver. I am not sure the
usefulness of it all outweighs the complication it incurs. It would be
good to hear about the rationale for inclusion here.

Thanks,
Henrik

Re: [PATCH 1/2] Input: synaptics - print firmware ID and board number at init

[Henrik Rydberg]

Hi Daniel,

> Read the Firmware ID and Board Number from a synaptics device at init
> and display them in the system log.
> 
> Signed-off-by: Daniel Kurtz <djkurtz@chromium.org>
> ---
>  drivers/input/mouse/synaptics.c |   38 ++++++++++++++++++++++++++++++++++++--
>  drivers/input/mouse/synaptics.h |    3 +++
>  2 files changed, 39 insertions(+), 2 deletions(-)

Is there a specific usecase for this, except the nice-to-have?

> diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
> index c778f27..81685c1 100644
> --- a/drivers/input/mouse/synaptics.c
> +++ b/drivers/input/mouse/synaptics.c
> @@ -139,6 +139,35 @@ static int synaptics_model_id(struct psmouse *psmouse)
>  }
>  
>  /*
> + * Read the board id from the touchpad
> + * The board id is encoded in the "QUERY MODES" response
> + */
> +static int synaptics_board_id(struct psmouse *psmouse)
> +{
> +	struct synaptics_data *priv = psmouse->private;
> +	unsigned char bid[3];
> +
> +	if (synaptics_send_cmd(psmouse, SYN_QUE_MODES, bid))
> +		return -1;
> +	priv->board_id = (((unsigned)bid[0] & 0xfc) << 6) | (unsigned)bid[1];

The type cast is not necessary.

> +	return 0;
> +}
> +
> +/*
> + * Read the firmware id from the touchpad
> + */
> +static int synaptics_firmware_id(struct psmouse *psmouse)
> +{
> +	struct synaptics_data *priv = psmouse->private;
> +	unsigned char fwid[3];
> +
> +	if (synaptics_send_cmd(psmouse, SYN_QUE_FIRMWARE_ID, fwid))
> +		return -1;
> +	priv->firmware_id = (fwid[0] << 16) | (fwid[1] << 8) | fwid[2];
> +	return 0;
> +}
> +
> +/*
>   * Read the capability-bits from the touchpad
>   * see also the SYN_CAP_* macros
>   */
> @@ -261,6 +290,10 @@ static int synaptics_query_hardware(struct psmouse *psmouse)
>  		return -1;
>  	if (synaptics_model_id(psmouse))
>  		return -1;
> +	if (synaptics_firmware_id(psmouse))
> +		return -1;
> +	if (synaptics_board_id(psmouse))
> +		return -1;
>  	if (synaptics_capability(psmouse))
>  		return -1;
>  	if (synaptics_resolution(psmouse))
> @@ -1434,11 +1467,12 @@ static int __synaptics_init(struct psmouse *psmouse, bool absolute_mode)
>  	priv->pkt_type = SYN_MODEL_NEWABS(priv->model_id) ? SYN_NEWABS : SYN_OLDABS;
>  
>  	psmouse_info(psmouse,
> -		     "Touchpad model: %ld, fw: %ld.%ld, id: %#lx, caps: %#lx/%#lx/%#lx\n",
> +		     "Touchpad model: %ld, fw: %ld.%ld, id: %#lx, caps: %#lx/%#lx/%#lx, board id: %lu, fw id: %lu\n",
>  		     SYN_ID_MODEL(priv->identity),
>  		     SYN_ID_MAJOR(priv->identity), SYN_ID_MINOR(priv->identity),
>  		     priv->model_id,
> -		     priv->capabilities, priv->ext_cap, priv->ext_cap_0c);
> +		     priv->capabilities, priv->ext_cap, priv->ext_cap_0c,
> +		     priv->board_id, priv->firmware_id);
>  
>  	set_input_params(psmouse->dev, priv);
>  
> diff --git a/drivers/input/mouse/synaptics.h b/drivers/input/mouse/synaptics.h
> index fd26ccc..e594af0 100644
> --- a/drivers/input/mouse/synaptics.h
> +++ b/drivers/input/mouse/synaptics.h
> @@ -18,6 +18,7 @@
>  #define SYN_QUE_SERIAL_NUMBER_SUFFIX	0x07
>  #define SYN_QUE_RESOLUTION		0x08
>  #define SYN_QUE_EXT_CAPAB		0x09
> +#define SYN_QUE_FIRMWARE_ID		0x0a
>  #define SYN_QUE_EXT_CAPAB_0C		0x0c
>  #define SYN_QUE_EXT_MAX_COORDS		0x0d
>  #define SYN_QUE_EXT_MIN_COORDS		0x0f
> @@ -148,6 +149,8 @@ struct synaptics_hw_state {
>  struct synaptics_data {
>  	/* Data read from the touchpad */
>  	unsigned long int model_id;		/* Model-ID */
> +	unsigned long int firmware_id;		/* Firmware-ID */
> +	unsigned long int board_id;		/* Board-ID */
>  	unsigned long int capabilities;		/* Capabilities */
>  	unsigned long int ext_cap;		/* Extended Capabilities */
>  	unsigned long int ext_cap_0c;		/* Ext Caps from 0x0c query */
> -- 
> 1.7.7.3
> 

Thanks,
Henrik

Re: [PATCH 1/2] Input: synaptics - print firmware ID and board number at init

[Daniel Kurtz]

On Mon, Jul 2, 2012 at 3:27 PM, Henrik Rydberg <rydberg@euromail.se> wrote:
> Hi Daniel,
>
>> Read the Firmware ID and Board Number from a synaptics device at init
>> and display them in the system log.
>>
>> Signed-off-by: Daniel Kurtz <djkurtz@chromium.org>
>> ---
>>  drivers/input/mouse/synaptics.c |   38 ++++++++++++++++++++++++++++++++++++--
>>  drivers/input/mouse/synaptics.h |    3 +++
>>  2 files changed, 39 insertions(+), 2 deletions(-)
>
> Is there a specific usecase for this, except the nice-to-have?

Use cases (these are all very real use cases, that we use today, they
are not hypothetical):
  * To include firmware and hardware versions in user feedback reports.
  * Used by factory tests to ensure systems are being built and
deployed correctly.
  * Lastly, these are also precursor patches for a larger set that
adds firmware update capability.  A userspace firmware update script
can use these APIs to determine which file to load and/or whether such
a load is necessary.

If there is a standard non-driver-specific way of achieving these use
cases, I'd be happy to modify the implementation to adopt that
standard.  From what I've seen, however, it looks like most drivers
have their own ad hoc way of exposing device/vendor specific
properties.

-Dan

>
>> diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
>> index c778f27..81685c1 100644
>> --- a/drivers/input/mouse/synaptics.c
>> +++ b/drivers/input/mouse/synaptics.c
>> @@ -139,6 +139,35 @@ static int synaptics_model_id(struct psmouse *psmouse)
>>  }
>>
>>  /*
>> + * Read the board id from the touchpad
>> + * The board id is encoded in the "QUERY MODES" response
>> + */
>> +static int synaptics_board_id(struct psmouse *psmouse)
>> +{
>> +     struct synaptics_data *priv = psmouse->private;
>> +     unsigned char bid[3];
>> +
>> +     if (synaptics_send_cmd(psmouse, SYN_QUE_MODES, bid))
>> +             return -1;
>> +     priv->board_id = (((unsigned)bid[0] & 0xfc) << 6) | (unsigned)bid[1];
>
> The type cast is not necessary.
>
>> +     return 0;
>> +}
>> +
>> +/*
>> + * Read the firmware id from the touchpad
>> + */
>> +static int synaptics_firmware_id(struct psmouse *psmouse)
>> +{
>> +     struct synaptics_data *priv = psmouse->private;
>> +     unsigned char fwid[3];
>> +
>> +     if (synaptics_send_cmd(psmouse, SYN_QUE_FIRMWARE_ID, fwid))
>> +             return -1;
>> +     priv->firmware_id = (fwid[0] << 16) | (fwid[1] << 8) | fwid[2];
>> +     return 0;
>> +}
>> +
>> +/*
>>   * Read the capability-bits from the touchpad
>>   * see also the SYN_CAP_* macros
>>   */
>> @@ -261,6 +290,10 @@ static int synaptics_query_hardware(struct psmouse *psmouse)
>>               return -1;
>>       if (synaptics_model_id(psmouse))
>>               return -1;
>> +     if (synaptics_firmware_id(psmouse))
>> +             return -1;
>> +     if (synaptics_board_id(psmouse))
>> +             return -1;
>>       if (synaptics_capability(psmouse))
>>               return -1;
>>       if (synaptics_resolution(psmouse))
>> @@ -1434,11 +1467,12 @@ static int __synaptics_init(struct psmouse *psmouse, bool absolute_mode)
>>       priv->pkt_type = SYN_MODEL_NEWABS(priv->model_id) ? SYN_NEWABS : SYN_OLDABS;
>>
>>       psmouse_info(psmouse,
>> -                  "Touchpad model: %ld, fw: %ld.%ld, id: %#lx, caps: %#lx/%#lx/%#lx\n",
>> +                  "Touchpad model: %ld, fw: %ld.%ld, id: %#lx, caps: %#lx/%#lx/%#lx, board id: %lu, fw id: %lu\n",
>>                    SYN_ID_MODEL(priv->identity),
>>                    SYN_ID_MAJOR(priv->identity), SYN_ID_MINOR(priv->identity),
>>                    priv->model_id,
>> -                  priv->capabilities, priv->ext_cap, priv->ext_cap_0c);
>> +                  priv->capabilities, priv->ext_cap, priv->ext_cap_0c,
>> +                  priv->board_id, priv->firmware_id);
>>
>>       set_input_params(psmouse->dev, priv);
>>
>> diff --git a/drivers/input/mouse/synaptics.h b/drivers/input/mouse/synaptics.h
>> index fd26ccc..e594af0 100644
>> --- a/drivers/input/mouse/synaptics.h
>> +++ b/drivers/input/mouse/synaptics.h
>> @@ -18,6 +18,7 @@
>>  #define SYN_QUE_SERIAL_NUMBER_SUFFIX 0x07
>>  #define SYN_QUE_RESOLUTION           0x08
>>  #define SYN_QUE_EXT_CAPAB            0x09
>> +#define SYN_QUE_FIRMWARE_ID          0x0a
>>  #define SYN_QUE_EXT_CAPAB_0C         0x0c
>>  #define SYN_QUE_EXT_MAX_COORDS               0x0d
>>  #define SYN_QUE_EXT_MIN_COORDS               0x0f
>> @@ -148,6 +149,8 @@ struct synaptics_hw_state {
>>  struct synaptics_data {
>>       /* Data read from the touchpad */
>>       unsigned long int model_id;             /* Model-ID */
>> +     unsigned long int firmware_id;          /* Firmware-ID */
>> +     unsigned long int board_id;             /* Board-ID */
>>       unsigned long int capabilities;         /* Capabilities */
>>       unsigned long int ext_cap;              /* Extended Capabilities */
>>       unsigned long int ext_cap_0c;           /* Ext Caps from 0x0c query */
>> --
>> 1.7.7.3
>>
>
> Thanks,
> Henrik

Re: [PATCH 1/2] Input: synaptics - print firmware ID and board number at init

[Henrik Rydberg]

> > Is there a specific usecase for this, except the nice-to-have?
> 
> Use cases (these are all very real use cases, that we use today, they
> are not hypothetical):
>   * To include firmware and hardware versions in user feedback reports.
>   * Used by factory tests to ensure systems are being built and
> deployed correctly.
>   * Lastly, these are also precursor patches for a larger set that
> adds firmware update capability.  A userspace firmware update script
> can use these APIs to determine which file to load and/or whether such
> a load is necessary.

Thanks. Future firmware update patches is the prime reason for the
sysfs interface, IOW.

> If there is a standard non-driver-specific way of achieving these use
> cases, I'd be happy to modify the implementation to adopt that
> standard.  From what I've seen, however, it looks like most drivers
> have their own ad hoc way of exposing device/vendor specific
> properties.

Precisely. If there is no standard, it is quite alright to try to come
up with one. Otherwise we might as well leave all firmware-related
code in userspace.

Thanks,
Henrik