[PATCH] [v1.1 3/3] modify raydium firmware update rule

[PATCH] [v1.1 3/3] modify raydium firmware update rule

[jeffrey.lin]

modify raydium touch firmware update rule.

Signed-off-by: jeffrey.lin <jeffrey.lin@rad-ic.com>
---
 drivers/input/touchscreen/raydium_i2c_ts.c | 112 ++++++++++++++++++++++++-----
 1 file changed, 93 insertions(+), 19 deletions(-)

diff --git a/drivers/input/touchscreen/raydium_i2c_ts.c b/drivers/input/touchscreen/raydium_i2c_ts.c
index f3076d9..004c5a6 100644
--- a/drivers/input/touchscreen/raydium_i2c_ts.c
+++ b/drivers/input/touchscreen/raydium_i2c_ts.c
@@ -34,6 +34,9 @@
 #include <linux/slab.h>
 #include <asm/unaligned.h>
 
+/* Firmware */
+#define RAYDIUM_FW_NAME		"raydium.fw"
+
 /* Slave I2C mode */
 #define RM_BOOT_BLDR		0x02
 #define RM_BOOT_MAIN		0x03
@@ -78,7 +81,7 @@
 #define RM_MAX_FW_RETRIES	30
 #define RM_MAX_FW_SIZE		0xD000
 
-#define RM_POWERON_DELAY_USEC	500
+#define RM_POWERON_DELAY_MSEC	20
 #define RM_RESET_DELAY_MSEC	50
 
 enum raydium_bl_cmd {
@@ -97,6 +100,7 @@ enum raydium_bl_ack {
 enum raydium_boot_mode {
 	RAYDIUM_TS_MAIN = 0,
 	RAYDIUM_TS_BLDR,
+	INVALID_REGION,
 };
 
 /* Response to RM_CMD_DATA_BANK request */
@@ -141,6 +145,8 @@ struct raydium_data {
 	enum raydium_boot_mode boot_mode;
 
 	bool wake_irq_enabled;
+
+	char *fw_file;
 };
 
 static int raydium_i2c_send(struct i2c_client *client,
@@ -318,16 +324,23 @@ static int raydium_i2c_check_fw_status(struct raydium_data *ts)
 	struct i2c_client *client = ts->client;
 	static const u8 bl_ack = 0x62;
 	static const u8 main_ack = 0x66;
-	u8 buf[4];
+	u8 buf[5];
 	int error;
 
 	error = raydium_i2c_read(client, RM_CMD_BOOT_READ, buf, sizeof(buf));
 	if (!error) {
-		if (buf[0] == bl_ack)
-			ts->boot_mode = RAYDIUM_TS_BLDR;
-		else if (buf[0] == main_ack)
+		if (buf[0] == main_ack)
 			ts->boot_mode = RAYDIUM_TS_MAIN;
-		return 0;
+		else if (buf[0] == bl_ack) {
+			ts->boot_mode = RAYDIUM_TS_BLDR;
+			ts->info.main_ver = buf[4] >> 4;
+			ts->info.sub_ver = buf[4] & 0x0F;
+		} else {
+			ts->boot_mode = INVALID_REGION;
+			error = -EINVAL;
+			dev_err(&client->dev,
+				"invalid fw status: %x\n", buf[0]);
+		}
 	}
 
 	return error;
@@ -358,13 +371,10 @@ static int raydium_i2c_initialize(struct raydium_data *ts)
 	if (error)
 		ts->boot_mode = RAYDIUM_TS_BLDR;
 
-	if (ts->boot_mode == RAYDIUM_TS_BLDR) {
+	if (ts->boot_mode == RAYDIUM_TS_BLDR)
 		ts->info.hw_ver = cpu_to_le32(0xffffffffUL);
-		ts->info.main_ver = 0xff;
-		ts->info.sub_ver = 0xff;
-	} else {
+	else
 		raydium_i2c_query_ts_info(ts);
-	}
 
 	return error;
 }
@@ -739,12 +749,12 @@ static int raydium_i2c_fw_update(struct raydium_data *ts)
 {
 	struct i2c_client *client = ts->client;
 	const struct firmware *fw = NULL;
-	const char *fw_file = "raydium.fw";
 	int error;
 
-	error = request_firmware(&fw, fw_file, &client->dev);
+	error = request_firmware(&fw, ts->fw_file, &client->dev);
 	if (error) {
-		dev_err(&client->dev, "Unable to open firmware %s\n", fw_file);
+		dev_err(&client->dev, "Unable to open firmware %s\n",
+			ts->fw_file);
 		return error;
 	}
 
@@ -900,18 +910,75 @@ static ssize_t raydium_i2c_calibrate_store(struct device *dev,
 	return error ?: count;
 }
 
+static int raydium_update_file_name(struct device *dev, char **file_name,
+				const char *buf, size_t count)
+{
+	char *new_file_name;
+
+	/* Simple sanity check */
+	if (count > 64) {
+		dev_warn(dev, "File name too long\n");
+		return -EINVAL;
+	}
+
+	new_file_name = devm_kmalloc(dev, count + 1, GFP_KERNEL);
+	if (!new_file_name)
+		return -ENOMEM;
+
+	memcpy(new_file_name, buf, count + 1);
+
+	/* Echo into the sysfs entry may append newline at the end of buf */
+	if (new_file_name[count - 1] == '\n')
+		count--;
+
+	new_file_name[count] = '\0';
+
+	if (*file_name)
+		devm_kfree(dev, *file_name);
+
+	*file_name = new_file_name;
+
+	return 0;
+}
+
+static ssize_t raydium_fw_file_show(struct device *dev,
+				struct device_attribute *attr, char *buf)
+{
+	struct i2c_client *client = to_i2c_client(dev);
+	struct raydium_data *ts = i2c_get_clientdata(client);
+
+	return scnprintf(buf, PAGE_SIZE, "%s\n", ts->fw_file);
+}
+
+static ssize_t raydium_fw_file_store(struct device *dev,
+				 struct device_attribute *attr,
+				 const char *buf, size_t count)
+{
+	struct i2c_client *client = to_i2c_client(dev);
+	struct raydium_data *ts = i2c_get_clientdata(client);
+	int ret;
+
+	ret = raydium_update_file_name(dev, &ts->fw_file, buf, count);
+	if (ret)
+		return ret;
+
+	return count;
+}
+
 static DEVICE_ATTR(fw_version, S_IRUGO, raydium_i2c_fw_ver_show, NULL);
 static DEVICE_ATTR(hw_version, S_IRUGO, raydium_i2c_hw_ver_show, NULL);
 static DEVICE_ATTR(boot_mode, S_IRUGO, raydium_i2c_boot_mode_show, NULL);
 static DEVICE_ATTR(update_fw, S_IWUSR, NULL, raydium_i2c_update_fw_store);
 static DEVICE_ATTR(calibrate, S_IWUSR, NULL, raydium_i2c_calibrate_store);
-
+static DEVICE_ATTR(fw_file, S_IRUGO | S_IWUSR, raydium_fw_file_show,
+		   raydium_fw_file_store);
 static struct attribute *raydium_i2c_attributes[] = {
 	&dev_attr_update_fw.attr,
 	&dev_attr_boot_mode.attr,
 	&dev_attr_fw_version.attr,
 	&dev_attr_hw_version.attr,
 	&dev_attr_calibrate.attr,
+	&dev_attr_fw_file.attr,
 	NULL
 };
 
@@ -933,7 +1000,7 @@ static int raydium_i2c_power_on(struct raydium_data *ts)
 	if (!ts->reset_gpio)
 		return 0;
 
-	gpiod_set_value_cansleep(ts->reset_gpio, 1);
+	gpiod_set_value_cansleep(ts->reset_gpio, 0);
 
 	error = regulator_enable(ts->avdd);
 	if (error) {
@@ -950,10 +1017,10 @@ static int raydium_i2c_power_on(struct raydium_data *ts)
 		goto release_reset_gpio;
 	}
 
-	udelay(RM_POWERON_DELAY_USEC);
+	msleep(RM_POWERON_DELAY_MSEC);
 
 release_reset_gpio:
-	gpiod_set_value_cansleep(ts->reset_gpio, 0);
+	gpiod_set_value_cansleep(ts->reset_gpio, 1);
 
 	if (error)
 		return error;
@@ -968,7 +1035,6 @@ static void raydium_i2c_power_off(void *_data)
 	struct raydium_data *ts = _data;
 
 	if (ts->reset_gpio) {
-		gpiod_set_value_cansleep(ts->reset_gpio, 1);
 		regulator_disable(ts->vccio);
 		regulator_disable(ts->avdd);
 	}
@@ -1043,6 +1109,14 @@ static int raydium_i2c_probe(struct i2c_client *client,
 		return -ENXIO;
 	}
 
+	error = raydium_update_file_name(&client->dev, &ts->fw_file,
+			RAYDIUM_FW_NAME, strlen(RAYDIUM_FW_NAME));
+	if (error) {
+		dev_err(&client->dev, "failed to set firmware file name: %d\n",
+			error);
+		return error;
+	}
+
 	error = raydium_i2c_initialize(ts);
 	if (error) {
 		dev_err(&client->dev, "failed to initialize: %d\n", error);
-- 
2.1.2

Re: [PATCH] [v1.1 3/3] modify raydium firmware update rule

[Dmitry Torokhov]

On Thu, Jul 07, 2016 at 09:39:38AM -0700, jeffrey.lin wrote:
> modify raydium touch firmware update rule.

Why? You need to explain why you are proposing a change (but as I
mentioned I see no reason for using custom file names for firmware. Have
userspace adjust name as needed by the driver.

Thanks.

> 
> Signed-off-by: jeffrey.lin <jeffrey.lin@rad-ic.com>
> ---
>  drivers/input/touchscreen/raydium_i2c_ts.c | 112 ++++++++++++++++++++++++-----
>  1 file changed, 93 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/input/touchscreen/raydium_i2c_ts.c b/drivers/input/touchscreen/raydium_i2c_ts.c
> index f3076d9..004c5a6 100644
> --- a/drivers/input/touchscreen/raydium_i2c_ts.c
> +++ b/drivers/input/touchscreen/raydium_i2c_ts.c
> @@ -34,6 +34,9 @@
>  #include <linux/slab.h>
>  #include <asm/unaligned.h>
>  
> +/* Firmware */
> +#define RAYDIUM_FW_NAME		"raydium.fw"
> +
>  /* Slave I2C mode */
>  #define RM_BOOT_BLDR		0x02
>  #define RM_BOOT_MAIN		0x03
> @@ -78,7 +81,7 @@
>  #define RM_MAX_FW_RETRIES	30
>  #define RM_MAX_FW_SIZE		0xD000
>  
> -#define RM_POWERON_DELAY_USEC	500
> +#define RM_POWERON_DELAY_MSEC	20
>  #define RM_RESET_DELAY_MSEC	50
>  
>  enum raydium_bl_cmd {
> @@ -97,6 +100,7 @@ enum raydium_bl_ack {
>  enum raydium_boot_mode {
>  	RAYDIUM_TS_MAIN = 0,
>  	RAYDIUM_TS_BLDR,
> +	INVALID_REGION,
>  };
>  
>  /* Response to RM_CMD_DATA_BANK request */
> @@ -141,6 +145,8 @@ struct raydium_data {
>  	enum raydium_boot_mode boot_mode;
>  
>  	bool wake_irq_enabled;
> +
> +	char *fw_file;
>  };
>  
>  static int raydium_i2c_send(struct i2c_client *client,
> @@ -318,16 +324,23 @@ static int raydium_i2c_check_fw_status(struct raydium_data *ts)
>  	struct i2c_client *client = ts->client;
>  	static const u8 bl_ack = 0x62;
>  	static const u8 main_ack = 0x66;
> -	u8 buf[4];
> +	u8 buf[5];
>  	int error;
>  
>  	error = raydium_i2c_read(client, RM_CMD_BOOT_READ, buf, sizeof(buf));
>  	if (!error) {
> -		if (buf[0] == bl_ack)
> -			ts->boot_mode = RAYDIUM_TS_BLDR;
> -		else if (buf[0] == main_ack)
> +		if (buf[0] == main_ack)
>  			ts->boot_mode = RAYDIUM_TS_MAIN;
> -		return 0;
> +		else if (buf[0] == bl_ack) {
> +			ts->boot_mode = RAYDIUM_TS_BLDR;
> +			ts->info.main_ver = buf[4] >> 4;
> +			ts->info.sub_ver = buf[4] & 0x0F;
> +		} else {
> +			ts->boot_mode = INVALID_REGION;
> +			error = -EINVAL;
> +			dev_err(&client->dev,
> +				"invalid fw status: %x\n", buf[0]);
> +		}
>  	}
>  
>  	return error;
> @@ -358,13 +371,10 @@ static int raydium_i2c_initialize(struct raydium_data *ts)
>  	if (error)
>  		ts->boot_mode = RAYDIUM_TS_BLDR;
>  
> -	if (ts->boot_mode == RAYDIUM_TS_BLDR) {
> +	if (ts->boot_mode == RAYDIUM_TS_BLDR)
>  		ts->info.hw_ver = cpu_to_le32(0xffffffffUL);
> -		ts->info.main_ver = 0xff;
> -		ts->info.sub_ver = 0xff;
> -	} else {
> +	else
>  		raydium_i2c_query_ts_info(ts);
> -	}
>  
>  	return error;
>  }
> @@ -739,12 +749,12 @@ static int raydium_i2c_fw_update(struct raydium_data *ts)
>  {
>  	struct i2c_client *client = ts->client;
>  	const struct firmware *fw = NULL;
> -	const char *fw_file = "raydium.fw";
>  	int error;
>  
> -	error = request_firmware(&fw, fw_file, &client->dev);
> +	error = request_firmware(&fw, ts->fw_file, &client->dev);
>  	if (error) {
> -		dev_err(&client->dev, "Unable to open firmware %s\n", fw_file);
> +		dev_err(&client->dev, "Unable to open firmware %s\n",
> +			ts->fw_file);
>  		return error;
>  	}
>  
> @@ -900,18 +910,75 @@ static ssize_t raydium_i2c_calibrate_store(struct device *dev,
>  	return error ?: count;
>  }
>  
> +static int raydium_update_file_name(struct device *dev, char **file_name,
> +				const char *buf, size_t count)
> +{
> +	char *new_file_name;
> +
> +	/* Simple sanity check */
> +	if (count > 64) {
> +		dev_warn(dev, "File name too long\n");
> +		return -EINVAL;
> +	}
> +
> +	new_file_name = devm_kmalloc(dev, count + 1, GFP_KERNEL);
> +	if (!new_file_name)
> +		return -ENOMEM;
> +
> +	memcpy(new_file_name, buf, count + 1);
> +
> +	/* Echo into the sysfs entry may append newline at the end of buf */
> +	if (new_file_name[count - 1] == '\n')
> +		count--;
> +
> +	new_file_name[count] = '\0';
> +
> +	if (*file_name)
> +		devm_kfree(dev, *file_name);
> +
> +	*file_name = new_file_name;
> +
> +	return 0;
> +}
> +
> +static ssize_t raydium_fw_file_show(struct device *dev,
> +				struct device_attribute *attr, char *buf)
> +{
> +	struct i2c_client *client = to_i2c_client(dev);
> +	struct raydium_data *ts = i2c_get_clientdata(client);
> +
> +	return scnprintf(buf, PAGE_SIZE, "%s\n", ts->fw_file);
> +}
> +
> +static ssize_t raydium_fw_file_store(struct device *dev,
> +				 struct device_attribute *attr,
> +				 const char *buf, size_t count)
> +{
> +	struct i2c_client *client = to_i2c_client(dev);
> +	struct raydium_data *ts = i2c_get_clientdata(client);
> +	int ret;
> +
> +	ret = raydium_update_file_name(dev, &ts->fw_file, buf, count);
> +	if (ret)
> +		return ret;
> +
> +	return count;
> +}
> +
>  static DEVICE_ATTR(fw_version, S_IRUGO, raydium_i2c_fw_ver_show, NULL);
>  static DEVICE_ATTR(hw_version, S_IRUGO, raydium_i2c_hw_ver_show, NULL);
>  static DEVICE_ATTR(boot_mode, S_IRUGO, raydium_i2c_boot_mode_show, NULL);
>  static DEVICE_ATTR(update_fw, S_IWUSR, NULL, raydium_i2c_update_fw_store);
>  static DEVICE_ATTR(calibrate, S_IWUSR, NULL, raydium_i2c_calibrate_store);
> -
> +static DEVICE_ATTR(fw_file, S_IRUGO | S_IWUSR, raydium_fw_file_show,
> +		   raydium_fw_file_store);
>  static struct attribute *raydium_i2c_attributes[] = {
>  	&dev_attr_update_fw.attr,
>  	&dev_attr_boot_mode.attr,
>  	&dev_attr_fw_version.attr,
>  	&dev_attr_hw_version.attr,
>  	&dev_attr_calibrate.attr,
> +	&dev_attr_fw_file.attr,
>  	NULL
>  };
>  
> @@ -933,7 +1000,7 @@ static int raydium_i2c_power_on(struct raydium_data *ts)
>  	if (!ts->reset_gpio)
>  		return 0;
>  
> -	gpiod_set_value_cansleep(ts->reset_gpio, 1);
> +	gpiod_set_value_cansleep(ts->reset_gpio, 0);
>  
>  	error = regulator_enable(ts->avdd);
>  	if (error) {
> @@ -950,10 +1017,10 @@ static int raydium_i2c_power_on(struct raydium_data *ts)
>  		goto release_reset_gpio;
>  	}
>  
> -	udelay(RM_POWERON_DELAY_USEC);
> +	msleep(RM_POWERON_DELAY_MSEC);
>  
>  release_reset_gpio:
> -	gpiod_set_value_cansleep(ts->reset_gpio, 0);
> +	gpiod_set_value_cansleep(ts->reset_gpio, 1);
>  
>  	if (error)
>  		return error;
> @@ -968,7 +1035,6 @@ static void raydium_i2c_power_off(void *_data)
>  	struct raydium_data *ts = _data;
>  
>  	if (ts->reset_gpio) {
> -		gpiod_set_value_cansleep(ts->reset_gpio, 1);
>  		regulator_disable(ts->vccio);
>  		regulator_disable(ts->avdd);
>  	}
> @@ -1043,6 +1109,14 @@ static int raydium_i2c_probe(struct i2c_client *client,
>  		return -ENXIO;
>  	}
>  
> +	error = raydium_update_file_name(&client->dev, &ts->fw_file,
> +			RAYDIUM_FW_NAME, strlen(RAYDIUM_FW_NAME));
> +	if (error) {
> +		dev_err(&client->dev, "failed to set firmware file name: %d\n",
> +			error);
> +		return error;
> +	}
> +
>  	error = raydium_i2c_initialize(ts);
>  	if (error) {
>  		dev_err(&client->dev, "failed to initialize: %d\n", error);
> -- 
> 2.1.2
> 

-- 
Dmitry

Re:[v1.1,3/3] modify raydium firmware update rule

[jeffrey.lin]

Hi dmitry:
>> modify raydium touch firmware update rule.

>Why? You need to explain why you are proposing a change (but as I
>mentioned I see no reason for using custom file names for firmware. Have
>userspace adjust name as needed by the driver.

>Thanks.

Just want to easy to do firmware update version control in the factory. If do this,
factory do not easy update wrong version.

Thanks.

Jeffrey

Re: [v1.1,3/3] modify raydium firmware update rule

[Dmitry Torokhov]

Hi Jeffrey,

On Fri, Jul 08, 2016 at 07:38:59AM -0700, jeffrey.lin wrote:
> Hi dmitry:
> >> modify raydium touch firmware update rule.
> 
> >Why? You need to explain why you are proposing a change (but as I
> >mentioned I see no reason for using custom file names for firmware. Have
> >userspace adjust name as needed by the driver.
> 
> >Thanks.
> 
> Just want to easy to do firmware update version control in the factory. If do this,
> factory do not easy update wrong version.

Just have your factory image rename firmware to canonical name before
initiating update. There is no need to encumber kernel code with this.

Thanks.

-- 
Dmitry

Re:[v1.1,3/3] modify raydium firmware update rule

[jeffrey.lin]

Hi dmitry:

>> >> modify raydium touch firmware update rule.
>> 
>> >Why? You need to explain why you are proposing a change (but as I
>> >mentioned I see no reason for using custom file names for firmware. Have
>> >userspace adjust name as needed by the driver.
>> 
>> >Thanks.
>> 
>> Just want to easy to do firmware update version control in the factory. If do this,
>> factory do not easy update wrong version.
>
>Just have your factory image rename firmware to canonical name before
>initiating update. There is no need to encumber kernel code with this.
Okay
Thanks.

Jeffrey.