[PATCH v2 1/1] Input: atmel_mxt_ts - allow specification of firmware file name

[PATCH v2 1/1] Input: atmel_mxt_ts - allow specification of firmware file name

[Jiada Wang]

From: Nick Dyer <nick.dyer@itdev.co.uk>

On platforms which have multiple device instances using this driver, the
firmware may be different on each device. This patch makes the user give
the name of the firmware file when flashing.

If user specified firmware file can't be found, then driver will try to
flash default firmware "maxtouch.fw".

Signed-off-by: Nick Dyer <nick.dyer@itdev.co.uk>
Acked-by: Benson Leung <bleung@chromium.org>
Acked-by: Yufeng Shen <miletus@chromium.org>
(cherry picked from ndyer/linux/for-upstream commit 76ebb7cee971cb42dfb0a3a9224403b8b09abcf1)
[gdavis: Forward port and fix conflicts.]
Signed-off-by: George G. Davis <george_davis@mentor.com>
[jiada: change to flash default firmware file, when user specified firmware can't be found]
Signed-off-by: Jiada Wang <jiada_wang@mentor.com>
---
 drivers/input/touchscreen/atmel_mxt_ts.c | 33 +++++++++++++++++++++++-
 1 file changed, 32 insertions(+), 1 deletion(-)

diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
index a2189739e30f..a20bc1bf8d52 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -34,6 +34,7 @@
 #define MXT_FW_NAME		"maxtouch.fw"
 #define MXT_CFG_NAME		"maxtouch.cfg"
 #define MXT_CFG_MAGIC		"OBP_RAW V1"
+#define MAX_FILENAME_SIZE	64
 
 /* Registers */
 #define MXT_OBJECT_START	0x07
@@ -308,6 +309,7 @@ struct mxt_data {
 	struct t7_config t7_cfg;
 	struct mxt_dbg dbg;
 	struct gpio_desc *reset_gpio;
+	char fw_name[MAX_FILENAME_SIZE];
 
 	/* Cached parameters from object table */
 	u16 T5_address;
@@ -2887,6 +2889,26 @@ static int mxt_load_fw(struct device *dev, const char *fn)
 	return ret;
 }
 
+static int mxt_update_file_name(struct device *dev, char *file_name,
+				const char *buf, size_t count)
+{
+	size_t len = count;
+
+	/* Echo into the sysfs entry may append newline at the end of buf */
+	if (buf[count - 1] == '\n')
+		len = count - 1;
+
+	/* Simple sanity check */
+	if (len > MAX_FILENAME_SIZE - 1) {
+		dev_warn(dev, "File name too long\n");
+		return -EINVAL;
+	}
+
+	strscpy(file_name, buf, len + 1);
+
+	return 0;
+}
+
 static ssize_t mxt_update_fw_store(struct device *dev,
 					struct device_attribute *attr,
 					const char *buf, size_t count)
@@ -2894,7 +2916,16 @@ static ssize_t mxt_update_fw_store(struct device *dev,
 	struct mxt_data *data = dev_get_drvdata(dev);
 	int error;
 
-	error = mxt_load_fw(dev, MXT_FW_NAME);
+	error = mxt_update_file_name(dev, data->fw_name, buf, count);
+	if (error)
+		return error;
+
+	error = mxt_load_fw(dev, data->fw_name);
+	if (error) {
+		dev_warn(dev, "try %s instead\n", MXT_FW_NAME);
+		error = mxt_load_fw(dev, MXT_FW_NAME);
+	}
+
 	if (error) {
 		dev_err(dev, "The firmware update failed(%d)\n", error);
 		count = error;
-- 
2.17.1

Re: [PATCH v2 1/1] Input: atmel_mxt_ts - allow specification of firmware file name

[Dmitry Osipenko]

21.08.2020 10:54, Jiada Wang пишет:
> From: Nick Dyer <nick.dyer@itdev.co.uk>
> 
> On platforms which have multiple device instances using this driver, the
> firmware may be different on each device. This patch makes the user give
> the name of the firmware file when flashing.
> 
> If user specified firmware file can't be found, then driver will try to
> flash default firmware "maxtouch.fw".
> 
> Signed-off-by: Nick Dyer <nick.dyer@itdev.co.uk>
> Acked-by: Benson Leung <bleung@chromium.org>
> Acked-by: Yufeng Shen <miletus@chromium.org>
> (cherry picked from ndyer/linux/for-upstream commit 76ebb7cee971cb42dfb0a3a9224403b8b09abcf1)
> [gdavis: Forward port and fix conflicts.]
> Signed-off-by: George G. Davis <george_davis@mentor.com>
> [jiada: change to flash default firmware file, when user specified firmware can't be found]
> Signed-off-by: Jiada Wang <jiada_wang@mentor.com>
> ---
>  drivers/input/touchscreen/atmel_mxt_ts.c | 33 +++++++++++++++++++++++-
>  1 file changed, 32 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
> index a2189739e30f..a20bc1bf8d52 100644
> --- a/drivers/input/touchscreen/atmel_mxt_ts.c
> +++ b/drivers/input/touchscreen/atmel_mxt_ts.c
> @@ -34,6 +34,7 @@
>  #define MXT_FW_NAME		"maxtouch.fw"
>  #define MXT_CFG_NAME		"maxtouch.cfg"
>  #define MXT_CFG_MAGIC		"OBP_RAW V1"
> +#define MAX_FILENAME_SIZE	64
>  
>  /* Registers */
>  #define MXT_OBJECT_START	0x07
> @@ -308,6 +309,7 @@ struct mxt_data {
>  	struct t7_config t7_cfg;
>  	struct mxt_dbg dbg;
>  	struct gpio_desc *reset_gpio;
> +	char fw_name[MAX_FILENAME_SIZE];
>  
>  	/* Cached parameters from object table */
>  	u16 T5_address;
> @@ -2887,6 +2889,26 @@ static int mxt_load_fw(struct device *dev, const char *fn)
>  	return ret;
>  }
>  
> +static int mxt_update_file_name(struct device *dev, char *file_name,
> +				const char *buf, size_t count)
> +{
> +	size_t len = count;
> +
> +	/* Echo into the sysfs entry may append newline at the end of buf */
> +	if (buf[count - 1] == '\n')
> +		len = count - 1;
> +
> +	/* Simple sanity check */
> +	if (len > MAX_FILENAME_SIZE - 1) {
> +		dev_warn(dev, "File name too long\n");
> +		return -EINVAL;
> +	}
> +
> +	strscpy(file_name, buf, len + 1);
> +
> +	return 0;
> +}
> +
>  static ssize_t mxt_update_fw_store(struct device *dev,
>  					struct device_attribute *attr,
>  					const char *buf, size_t count)
> @@ -2894,7 +2916,16 @@ static ssize_t mxt_update_fw_store(struct device *dev,
>  	struct mxt_data *data = dev_get_drvdata(dev);
>  	int error;
>  
> -	error = mxt_load_fw(dev, MXT_FW_NAME);
> +	error = mxt_update_file_name(dev, data->fw_name, buf, count);
> +	if (error)
> +		return error;
> +
> +	error = mxt_load_fw(dev, data->fw_name);
> +	if (error) {
> +		dev_warn(dev, "try %s instead\n", MXT_FW_NAME);

Hello, Jiada!

Will this message be printed even if fw_name is an empty string or
whatever string that is already used by existing userspace for writing
to update_fw?

Maybe it actually should be better to add a new device attribute that
will take the fw_name? Then the old attribute will continue to work like
it worked before.

> +		error = mxt_load_fw(dev, MXT_FW_NAME);
> +	}
> +
>  	if (error) {
>  		dev_err(dev, "The firmware update failed(%d)\n", error);
>  		count = error;
>