2017-12-21 13:51:46

by jeffrey.lin

[permalink] [raw]
Subject: [PATCH] driver: input :touchscreen :Modify Raydium Firmware update input file

Modify update firmware to accept alternative file name

Signed-off-by: jeffrey.lin <[email protected]>
---
drivers/input/touchscreen/raydium_i2c_ts.c | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/input/touchscreen/raydium_i2c_ts.c b/drivers/input/touchscreen/raydium_i2c_ts.c
index a99fb5cac5a0..439d43c3519c 100644
--- a/drivers/input/touchscreen/raydium_i2c_ts.c
+++ b/drivers/input/touchscreen/raydium_i2c_ts.c
@@ -130,6 +130,7 @@ struct raydium_data {
struct gpio_desc *reset_gpio;

struct raydium_info info;
+ char fw_file[64];

struct mutex sysfs_mutex;

@@ -752,12 +753,16 @@ 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);
+ /* Firmware name */
+ snprintf(ts->fw_file, sizeof(ts->fw_file),
+ "raydium_%x.fw", ts->info.hw_ver);
+ dev_dbg(&client->dev, "firmware name: %s\n", ts->fw_file);
+
+ 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;
}

--
2.12.2


2018-01-05 05:53:44

by Dmitry Torokhov

[permalink] [raw]
Subject: Re: [PATCH] driver: input :touchscreen :Modify Raydium Firmware update input file

Hi Jeffrey,

On Thu, Dec 21, 2017 at 09:51:22PM +0800, jeffrey.lin wrote:
> Modify update firmware to accept alternative file name
>
> Signed-off-by: jeffrey.lin <[email protected]>
> ---
> drivers/input/touchscreen/raydium_i2c_ts.c | 11 ++++++++---
> 1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/input/touchscreen/raydium_i2c_ts.c b/drivers/input/touchscreen/raydium_i2c_ts.c
> index a99fb5cac5a0..439d43c3519c 100644
> --- a/drivers/input/touchscreen/raydium_i2c_ts.c
> +++ b/drivers/input/touchscreen/raydium_i2c_ts.c
> @@ -130,6 +130,7 @@ struct raydium_data {
> struct gpio_desc *reset_gpio;
>
> struct raydium_info info;
> + char fw_file[64];

You do not really need to keep the firmware name in driver data, just
use a temporary in raydium_i2c_fw_update().

>
> struct mutex sysfs_mutex;
>
> @@ -752,12 +753,16 @@ 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);
> + /* Firmware name */
> + snprintf(ts->fw_file, sizeof(ts->fw_file),
> + "raydium_%x.fw", ts->info.hw_ver);

hw_ver is LE32, you need to convert it to CPU endianness before using.
Also it would be better if we used the same encoding for the hardware
version as the one that we use when we output it in sysfs. It makes
userspace life a bit easier I think.

How about the version of the patch below?

Thanks.

--
Dmitry


Input: raydium_i2c_ts - include hardware version in firmware name

From: Jeffrey Lin <[email protected]>

Add hardware version to the firmware file name to handle scenarios where
single system image supports variety of devices.

Signed-off-by: Jeffrey Lin <[email protected]>
Signed-off-by: Dmitry Torokhov <[email protected]>
---
drivers/input/touchscreen/raydium_i2c_ts.c | 14 ++++++++++++--
1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/input/touchscreen/raydium_i2c_ts.c b/drivers/input/touchscreen/raydium_i2c_ts.c
index 100538d64fff..d1c09e6a2cb6 100644
--- a/drivers/input/touchscreen/raydium_i2c_ts.c
+++ b/drivers/input/touchscreen/raydium_i2c_ts.c
@@ -752,13 +752,20 @@ 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";
+ char *fw_file;
int error;

+ fw_file = kasprintf(GFP_KERNEL, "raydium_%#04x.fw",
+ le32_to_cpu(ts->info.hw_ver));
+ if (!fw_file)
+ return -ENOMEM;
+
+ dev_dbg(&client->dev, "firmware name: %s\n", fw_file);
+
error = request_firmware(&fw, fw_file, &client->dev);
if (error) {
dev_err(&client->dev, "Unable to open firmware %s\n", fw_file);
- return error;
+ goto out_free_fw_file;
}

disable_irq(client->irq);
@@ -787,6 +794,9 @@ static int raydium_i2c_fw_update(struct raydium_data *ts)

release_firmware(fw);

+out_free_fw_file:
+ kfree(fw_file);
+
return error;
}


2018-01-05 12:38:22

by Jeffrey Lin (林義章)

[permalink] [raw]
Subject: RE: [PATCH] driver: input :touchscreen :Modify Raydium Firmware update input file

Dmitry:
I've verified pass the patch you had modified. Many thanks.

Best Regards
----------------------------------------------------------------------
Jeffrey Lin, ?L?q??
Raydium Semiconductor Corporation, ?繩????
Tel:(03)666-1818 Ext.4163
Fax:(03)666-1919

-----Original Message-----
From: Dmitry Torokhov [mailto:[email protected]]
Sent: Friday, January 05, 2018 1:54 PM
To: jeffrey.lin
Cc: [email protected]; [email protected]; [email protected]; [email protected]; Jeffrey Lin (?L?q??); KP Li (??????); Albert Shieh (?ªY޳); Calvin Tseng (?????v); [email protected]; [email protected]
Subject: Re: [PATCH] driver: input :touchscreen :Modify Raydium Firmware update input file

Hi Jeffrey,

On Thu, Dec 21, 2017 at 09:51:22PM +0800, jeffrey.lin wrote:
> Modify update firmware to accept alternative file name
>
> Signed-off-by: jeffrey.lin <[email protected]>
> ---
> drivers/input/touchscreen/raydium_i2c_ts.c | 11 ++++++++---
> 1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/input/touchscreen/raydium_i2c_ts.c b/drivers/input/touchscreen/raydium_i2c_ts.c
> index a99fb5cac5a0..439d43c3519c 100644
> --- a/drivers/input/touchscreen/raydium_i2c_ts.c
> +++ b/drivers/input/touchscreen/raydium_i2c_ts.c
> @@ -130,6 +130,7 @@ struct raydium_data {
> struct gpio_desc *reset_gpio;
>
> struct raydium_info info;
> + char fw_file[64];

You do not really need to keep the firmware name in driver data, just
use a temporary in raydium_i2c_fw_update().

>
> struct mutex sysfs_mutex;
>
> @@ -752,12 +753,16 @@ 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);
> + /* Firmware name */
> + snprintf(ts->fw_file, sizeof(ts->fw_file),
> + "raydium_%x.fw", ts->info.hw_ver);

hw_ver is LE32, you need to convert it to CPU endianness before using.
Also it would be better if we used the same encoding for the hardware
version as the one that we use when we output it in sysfs. It makes
userspace life a bit easier I think.

How about the version of the patch below?

Thanks.

--
Dmitry


Input: raydium_i2c_ts - include hardware version in firmware name

From: Jeffrey Lin <[email protected]>

Add hardware version to the firmware file name to handle scenarios where
single system image supports variety of devices.

Signed-off-by: Jeffrey Lin <[email protected]>
Signed-off-by: Dmitry Torokhov <[email protected]>
---
drivers/input/touchscreen/raydium_i2c_ts.c | 14 ++++++++++++--
1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/input/touchscreen/raydium_i2c_ts.c b/drivers/input/touchscreen/raydium_i2c_ts.c
index 100538d64fff..d1c09e6a2cb6 100644
--- a/drivers/input/touchscreen/raydium_i2c_ts.c
+++ b/drivers/input/touchscreen/raydium_i2c_ts.c
@@ -752,13 +752,20 @@ 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";
+ char *fw_file;
int error;

+ fw_file = kasprintf(GFP_KERNEL, "raydium_%#04x.fw",
+ le32_to_cpu(ts->info.hw_ver));
+ if (!fw_file)
+ return -ENOMEM;
+
+ dev_dbg(&client->dev, "firmware name: %s\n", fw_file);
+
error = request_firmware(&fw, fw_file, &client->dev);
if (error) {
dev_err(&client->dev, "Unable to open firmware %s\n", fw_file);
- return error;
+ goto out_free_fw_file;
}

disable_irq(client->irq);
@@ -787,6 +794,9 @@ static int raydium_i2c_fw_update(struct raydium_data *ts)

release_firmware(fw);

+out_free_fw_file:
+ kfree(fw_file);
+
return error;
}

CONFIDENTIALITY AND PROPRIETARY REMINDER:
This message and any attachment may contain confidential information. All rights including intellectual property rights arising out of this correspondence shall belong to Raydium Semiconductor Corp. Any unauthorized disclosure, forwarding, using, modifying, spreading, publishing or copying this email and the content is illegal and strictly prohibited.
If you are not the intended recipient, please notify the sender by replying to this message and delete all copies of it from your system. Thank you for cooperation.