Re: [PATCH 2/2] Input: synaptics-rmi4 - add sysfs interfaces for hardware IDs

On Fri, Jan 20, 2017 at 02:42:52PM +0100, Benjamin Tissoires wrote:

Subject: Re: [PATCH 2/2] Input: synaptics-rmi4 - add sysfs interfaces for
 hardware IDs

Hi Nick,

Well, first, you need a changelog explaining why you need custom sysfs
files.

It is for a firmware update system that works across a number of devices
supported by the synaptics-rmi4 driver. It needs to manage multiple
different versions of the firmware/hardware and which ones are
compatible with which. I will put it in the commit message: although
possibly the information would be useful even if you weren't trying to
update firmware.

Then, I really wonder if this is the best solution. You are trying to
solve a general problem in a specific way, while other drivers/devices
could benefit from reporting such information in a generic way.

I don't have a solution, but I know that we already had the case where
userspace wants to know which firmware is currently loaded on a
board[1].

A solution could be to add some ioctls to evdev to report those various
hardware IDs, but I am not so sure it will help in the RMI4 device while
in bootloader case.

If we are trying to recover a broken device (eg corrupt firmware), we
might not have the information to register an input device anyway.

I agree that a generic solution to get this information would be nice.
But we would have to map the complexities of the underlying
hardware/firmware anyway - for example, an "input device" may have
multiple components with separate firmwares having individual versions.

So it would end up being a hierarchy with attributes of various types:
isn't that just sysfs?

If Dmitry agrees to take this one, I have nothing against it.

Cheers,
Benjamin

[1] https://bugs.freedesktop.org/show_bug.cgi?id=97912

On Dec 30 2016 or thereabouts, Nick Dyer wrote:

quoted

Signed-off-by: Nick Dyer <nick@shmanahar.org>
---
 drivers/input/rmi4/rmi_driver.c |   2 +-
 drivers/input/rmi4/rmi_driver.h |   6 +-
 drivers/input/rmi4/rmi_f01.c    |  45 ++++++++++++++-
 drivers/input/rmi4/rmi_f34.c    | 123 ++++++++++++++++++++++++++++++++++++++++
 4 files changed, 172 insertions(+), 4 deletions(-)

diff --git a/drivers/input/rmi4/rmi_driver.c b/drivers/input/rmi4/rmi_driver.c
index cb6efe6..e0359d6 100644
--- a/drivers/input/rmi4/rmi_driver.c
+++ b/drivers/input/rmi4/rmi_driver.c

@@ -365,7 +365,7 @@ static void rmi_driver_set_input_name(struct rmi_device *rmi_dev,
 				struct input_dev *input)
 {
 	struct rmi_driver_data *data = dev_get_drvdata(&rmi_dev->dev);
-	char *device_name = rmi_f01_get_product_ID(data->f01_container);
+	const char *device_name = rmi_f01_get_product_ID(data->f01_container);
 	char *name;
 
 	name = devm_kasprintf(&rmi_dev->dev, GFP_KERNEL,

diff --git a/drivers/input/rmi4/rmi_driver.h b/drivers/input/rmi4/rmi_driver.h
index 24f8f76..48b3131 100644
--- a/drivers/input/rmi4/rmi_driver.h
+++ b/drivers/input/rmi4/rmi_driver.h

@@ -104,7 +104,11 @@ int rmi_init_functions(struct rmi_driver_data *data);
 int rmi_initial_reset(struct rmi_device *rmi_dev, void *ctx,
 		      const struct pdt_entry *pdt);
 
-char *rmi_f01_get_product_ID(struct rmi_function *fn);
+const char *rmi_f01_get_product_ID(struct rmi_function *fn);
+u8 rmi_f01_get_manufacturer_ID(struct rmi_function *fn);
+const char *rmi_f01_get_date_of_manufacture(struct rmi_function *fn);
+u32 rmi_f01_get_firmware_ID(struct rmi_function *fn);
+u32 rmi_f01_get_package_ID(struct rmi_function *fn);
 
 #ifdef CONFIG_RMI4_F34
 int rmi_f34_create_sysfs(struct rmi_device *rmi_dev);

diff --git a/drivers/input/rmi4/rmi_f01.c b/drivers/input/rmi4/rmi_f01.c
index cae35c6..1b03f4d 100644
--- a/drivers/input/rmi4/rmi_f01.c
+++ b/drivers/input/rmi4/rmi_f01.c

@@ -13,6 +13,7 @@
 #include <linux/slab.h>
 #include <linux/uaccess.h>
 #include <linux/of.h>
+#include <asm/unaligned.h>
 #include "rmi_driver.h"
 
 #define RMI_PRODUCT_ID_LENGTH    10

@@ -55,6 +56,7 @@ struct f01_basic_properties {
 	u8 product_id[RMI_PRODUCT_ID_LENGTH + 1];
 	u16 productinfo;
 	u32 firmware_id;
+	u32 package_id;
 };
 
 /* F01 device status bits */

@@ -221,8 +223,19 @@ static int rmi_f01_read_properties(struct rmi_device *rmi_dev,
 			has_build_id_query = !!(queries[0] & BIT(1));
 		}
 
-		if (has_package_id_query)
+		if (has_package_id_query) {
+			ret = rmi_read_block(rmi_dev, prod_info_addr,
+					     queries, sizeof(__le64));
+			if (ret) {
+				dev_err(&rmi_dev->dev,
+					"Failed to read package info: %d\n",
+					ret);
+				return ret;
+			}
+
+			props->package_id = get_unaligned_le64(queries);
 			prod_info_addr++;
+		}
 
 		if (has_build_id_query) {
 			ret = rmi_read_block(rmi_dev, prod_info_addr, queries,

@@ -242,13 +255,41 @@ static int rmi_f01_read_properties(struct rmi_device *rmi_dev,
 	return 0;
 }
 
-char *rmi_f01_get_product_ID(struct rmi_function *fn)
+const char *rmi_f01_get_product_ID(struct rmi_function *fn)
 {
 	struct f01_data *f01 = dev_get_drvdata(&fn->dev);
 
 	return f01->properties.product_id;
 }
 
+u8 rmi_f01_get_manufacturer_ID(struct rmi_function *fn)
+{
+	struct f01_data *f01 = dev_get_drvdata(&fn->dev);
+
+	return f01->properties.manufacturer_id;
+}
+
+const char *rmi_f01_get_date_of_manufacture(struct rmi_function *fn)
+{
+	struct f01_data *f01 = dev_get_drvdata(&fn->dev);
+
+	return f01->properties.dom;
+}
+
+u32 rmi_f01_get_firmware_ID(struct rmi_function *fn)
+{
+	struct f01_data *f01 = dev_get_drvdata(&fn->dev);
+
+	return f01->properties.firmware_id;
+}
+
+u32 rmi_f01_get_package_ID(struct rmi_function *fn)
+{
+	struct f01_data *f01 = dev_get_drvdata(&fn->dev);
+
+	return f01->properties.package_id;
+}
+
 #ifdef CONFIG_OF
 static int rmi_f01_of_probe(struct device *dev,
 				struct rmi_device_platform_data *pdata)

diff --git a/drivers/input/rmi4/rmi_f34.c b/drivers/input/rmi4/rmi_f34.c
index d7709bc..b7b38c1 100644
--- a/drivers/input/rmi4/rmi_f34.c
+++ b/drivers/input/rmi4/rmi_f34.c

@@ -394,6 +394,122 @@ static int rmi_firmware_update(struct rmi_driver_data *data,
 	return ret;
 }
 
+static ssize_t rmi_driver_manufacturer_id_show(struct device *dev,
+					       struct device_attribute *dattr,
+					       char *buf)
+{
+	struct rmi_driver_data *data = dev_get_drvdata(dev);
+	struct rmi_function *fn = data->f01_container;
+
+	return scnprintf(buf, PAGE_SIZE, "%d\n",
+			 rmi_f01_get_manufacturer_ID(fn));
+}
+
+static DEVICE_ATTR(manufacturer_id, 0444,
+		   rmi_driver_manufacturer_id_show, NULL);
+
+static ssize_t rmi_driver_dom_show(struct device *dev,
+				   struct device_attribute *dattr, char *buf)
+{
+	struct rmi_driver_data *data = dev_get_drvdata(dev);
+	struct rmi_function *fn = data->f01_container;
+
+	return scnprintf(buf, PAGE_SIZE, "%s\n",
+			 rmi_f01_get_date_of_manufacture(fn));
+}
+
+static DEVICE_ATTR(date_of_manufacture, 0444, rmi_driver_dom_show, NULL);
+
+static ssize_t rmi_driver_product_id_show(struct device *dev,
+					  struct device_attribute *dattr,
+					  char *buf)
+{
+	struct rmi_driver_data *data = dev_get_drvdata(dev);
+	struct rmi_function *fn = data->f01_container;
+
+	return scnprintf(buf, PAGE_SIZE, "%s\n",
+			 rmi_f01_get_product_ID(fn));
+}
+
+static DEVICE_ATTR(product_id, 0444,
+		   rmi_driver_product_id_show, NULL);
+
+static ssize_t rmi_driver_firmware_id_show(struct device *dev,
+					   struct device_attribute *dattr,
+					   char *buf)
+{
+	struct rmi_driver_data *data = dev_get_drvdata(dev);
+	struct rmi_function *fn = data->f01_container;
+
+	return scnprintf(buf, PAGE_SIZE, "%d\n",
+			 rmi_f01_get_firmware_ID(fn));
+}
+
+static DEVICE_ATTR(firmware_id, 0444,
+		   rmi_driver_firmware_id_show, NULL);
+
+static ssize_t rmi_driver_bootloader_id_show(struct device *dev,
+					     struct device_attribute *dattr,
+					     char *buf)
+{
+	struct rmi_driver_data *data = dev_get_drvdata(dev);
+	struct rmi_function *fn = data->f34_container;
+	struct f34_data *f34;
+
+	if (fn) {
+		f34 = dev_get_drvdata(&fn->dev);
+
+		if (f34->bl_version == 5)
+			return scnprintf(buf, PAGE_SIZE, "%c%c\n",
+					 f34->bootloader_id[0],
+					 f34->bootloader_id[1]);
+		else
+			return scnprintf(buf, PAGE_SIZE, "V%d.%d\n",
+					 f34->bootloader_id[1],
+					 f34->bootloader_id[0]);
+	}
+
+	return 0;
+}
+
+static DEVICE_ATTR(bootloader_id, 0444,
+		   rmi_driver_bootloader_id_show, NULL);
+
+static ssize_t rmi_driver_configuration_id_show(struct device *dev,
+						struct device_attribute *dattr,
+						char *buf)
+{
+	struct rmi_driver_data *data = dev_get_drvdata(dev);
+	struct rmi_function *fn = data->f34_container;
+	struct f34_data *f34;
+
+	if (fn) {
+		f34 = dev_get_drvdata(&fn->dev);
+
+		return scnprintf(buf, PAGE_SIZE, "%s\n", f34->configuration_id);
+	}
+
+	return 0;
+}
+
+static DEVICE_ATTR(configuration_id, 0444,
+		   rmi_driver_configuration_id_show, NULL);
+
+static ssize_t rmi_driver_package_id_show(struct device *dev,
+					  struct device_attribute *dattr,
+					  char *buf)
+{
+	struct rmi_driver_data *data = dev_get_drvdata(dev);
+	struct rmi_function *fn = data->f01_container;
+
+	u32 package_id = rmi_f01_get_package_ID(fn);
+
+	return scnprintf(buf, PAGE_SIZE, "%04x.%04x\n",
+			 package_id & 0xffff, (package_id >> 16) & 0xffff);
+}
+
+static DEVICE_ATTR(package_id, 0444, rmi_driver_package_id_show, NULL);
+
 static int rmi_firmware_update(struct rmi_driver_data *data,
 			       const struct firmware *fw);
 

@@ -448,6 +564,13 @@ static DEVICE_ATTR(update_fw_status, 0444,
 		   rmi_driver_update_fw_status_show, NULL);
 
 static struct attribute *rmi_firmware_attrs[] = {
+	&dev_attr_bootloader_id.attr,
+	&dev_attr_configuration_id.attr,
+	&dev_attr_manufacturer_id.attr,
+	&dev_attr_date_of_manufacture.attr,
+	&dev_attr_product_id.attr,
+	&dev_attr_package_id.attr,
+	&dev_attr_firmware_id.attr,
 	&dev_attr_update_fw.attr,
 	&dev_attr_update_fw_status.attr,
 	NULL

-- 
2.7.4