* [PATCH] Input: elants_i2c: Append hw_version to FW file
@ 2015-03-10 19:57 Charlie Mooney
2015-03-11 1:41 ` Dmitry Torokhov
0 siblings, 1 reply; 4+ messages in thread
From: Charlie Mooney @ 2015-03-10 19:57 UTC (permalink / raw)
To: linux-input; +Cc: dmitry.torokhov, scott.liu, fabf, Charlie Mooney
Currently the elants_i2c driver simply requests a static filename
/lib/firmware/elants_i2c.bin when it gets firmware updates. This is
a problem if you have two Elan touchscreens using the same driver.
If both touchscreens have different firmwares, you would need to move
the files around in your filesystem when you're updating them so that
they don't get updated with the other's FW. If you have a read-only
filesystem then this is impossible, even.
This patch changes the elants_i2c driver to automatically append the
four-hex-digit hw_version of the device onto the name of the FW file
it's requesting for update. Since different touchscreens should have
a different hw_version's this means the user needs to append the hw
version of the touchscreen he or she intends to update onto the end
of the firmware filename and then the driver will do the rest.
The firmware filenames it looks for now are of the form:
elants_i2c_${HW_VERSION}.bin
eg:
elants_i2c_2a44.bin
Signed-off-by: Charlie Mooney <charliemooney@chromium.org>
---
drivers/input/touchscreen/elants_i2c.c | 13 ++++++++++---
1 file changed, 10 insertions(+), 3 deletions(-)
diff --git a/drivers/input/touchscreen/elants_i2c.c b/drivers/input/touchscreen/elants_i2c.c
index 926c58e..d34ba57 100644
--- a/drivers/input/touchscreen/elants_i2c.c
+++ b/drivers/input/touchscreen/elants_i2c.c
@@ -98,7 +98,10 @@
#define MAX_FW_UPDATE_RETRIES 30
#define ELAN_FW_PAGESIZE 132
-#define ELAN_FW_FILENAME "elants_i2c.bin"
+#define ELAN_FW_BASE_FILENAME "elants_i2c"
+#define ELAN_FW_EXTENSION "bin"
+#define ELAN_FW_FILENAME_MAX_LEN (ARRAY_SIZE(ELAN_FW_BASE_FILENAME) + \
+ ARRAY_SIZE(ELAN_FW_EXTENSION) + 5)
/* calibration timeout definition */
#define ELAN_CALI_TIMEOUT_MSEC 10000
@@ -697,12 +700,16 @@ static int elants_i2c_fw_update(struct elants_data *ts)
{
struct i2c_client *client = ts->client;
const struct firmware *fw;
+ char fw_filename_buffer[ELAN_FW_FILENAME_MAX_LEN];
int error;
- error = request_firmware(&fw, ELAN_FW_FILENAME, &client->dev);
+ snprintf(fw_filename_buffer, ELAN_FW_FILENAME_MAX_LEN, "%s_%4x.%s",
+ ELAN_FW_BASE_FILENAME, ts->hw_version, ELAN_FW_EXTENSION);
+ dev_info(&client->dev, "requesting fw name = %s\n", fw_filename_buffer);
+ error = request_firmware(&p_fw_entry, fw_filename_buffer, &client->dev);
if (error) {
dev_err(&client->dev, "failed to request firmware %s: %d\n",
- ELAN_FW_FILENAME, error);
+ fw_filename_buffer, error);
return error;
}
--
2.1.2
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] Input: elants_i2c: Append hw_version to FW file 2015-03-10 19:57 [PATCH] Input: elants_i2c: Append hw_version to FW file Charlie Mooney @ 2015-03-11 1:41 ` Dmitry Torokhov 2015-03-11 17:09 ` Charles Mooney 0 siblings, 1 reply; 4+ messages in thread From: Dmitry Torokhov @ 2015-03-11 1:41 UTC (permalink / raw) To: Charlie Mooney; +Cc: linux-input, scott.liu, fabf Hi Charlie, On Tue, Mar 10, 2015 at 12:57:03PM -0700, Charlie Mooney wrote: > Currently the elants_i2c driver simply requests a static filename > /lib/firmware/elants_i2c.bin when it gets firmware updates. This is > a problem if you have two Elan touchscreens using the same driver. > If both touchscreens have different firmwares, you would need to move > the files around in your filesystem when you're updating them so that > they don't get updated with the other's FW. If you have a read-only > filesystem then this is impossible, even. > > This patch changes the elants_i2c driver to automatically append the > four-hex-digit hw_version of the device onto the name of the FW file > it's requesting for update. Since different touchscreens should have > a different hw_version's this means the user needs to append the hw > version of the touchscreen he or she intends to update onto the end > of the firmware filename and then the driver will do the rest. > > The firmware filenames it looks for now are of the form: > > elants_i2c_${HW_VERSION}.bin > > eg: > > elants_i2c_2a44.bin > > Signed-off-by: Charlie Mooney <charliemooney@chromium.org> > --- > drivers/input/touchscreen/elants_i2c.c | 13 ++++++++++--- > 1 file changed, 10 insertions(+), 3 deletions(-) > > diff --git a/drivers/input/touchscreen/elants_i2c.c b/drivers/input/touchscreen/elants_i2c.c > index 926c58e..d34ba57 100644 > --- a/drivers/input/touchscreen/elants_i2c.c > +++ b/drivers/input/touchscreen/elants_i2c.c > @@ -98,7 +98,10 @@ > #define MAX_FW_UPDATE_RETRIES 30 > > #define ELAN_FW_PAGESIZE 132 > -#define ELAN_FW_FILENAME "elants_i2c.bin" > +#define ELAN_FW_BASE_FILENAME "elants_i2c" > +#define ELAN_FW_EXTENSION "bin" > +#define ELAN_FW_FILENAME_MAX_LEN (ARRAY_SIZE(ELAN_FW_BASE_FILENAME) + \ > + ARRAY_SIZE(ELAN_FW_EXTENSION) + 5) Looked at this again and I don't really like how we need to maintain the MAX_LEN and make sure it is in sync with the format string. How about the patch below instead? Thanks. -- Dmitry Input: elants_i2c - append hw_version to FW file From: Charlie Mooney <charliemooney@chromium.org> Currently the elants_i2c driver simply requests a static filename /lib/firmware/elants_i2c.bin when it gets firmware updates. This is a problem if you have two Elan touchscreens using the same driver. If both touchscreens have different firmwares, you would need to move the files around in your filesystem when you're updating them so that they don't get updated with the other's FW. If you have a read-only filesystem then this is impossible, even. This patch changes the elants_i2c driver to automatically append the four-hex-digit hw_version of the device onto the name of the FW file it's requesting for update. Since different touchscreens should have a different hw_version's this means the user needs to append the hw version of the touchscreen he or she intends to update onto the end of the firmware filename and then the driver will do the rest. The firmware filenames it looks for now are of the form: elants_i2c_${HW_VERSION}.bin eg: elants_i2c_2a44.bin Signed-off-by: Charlie Mooney <charliemooney@chromium.org> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com> --- drivers/input/touchscreen/elants_i2c.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/drivers/input/touchscreen/elants_i2c.c b/drivers/input/touchscreen/elants_i2c.c index 926c58e..43b3c9c 100644 --- a/drivers/input/touchscreen/elants_i2c.c +++ b/drivers/input/touchscreen/elants_i2c.c @@ -98,7 +98,6 @@ #define MAX_FW_UPDATE_RETRIES 30 #define ELAN_FW_PAGESIZE 132 -#define ELAN_FW_FILENAME "elants_i2c.bin" /* calibration timeout definition */ #define ELAN_CALI_TIMEOUT_MSEC 10000 @@ -697,12 +696,19 @@ static int elants_i2c_fw_update(struct elants_data *ts) { struct i2c_client *client = ts->client; const struct firmware *fw; + char *fw_name; int error; - error = request_firmware(&fw, ELAN_FW_FILENAME, &client->dev); + fw_name = kasprintf(GFP_KERNEL, "elants_i2c_%4x.bin", ts->hw_version); + if (!fw_name) + return -ENOMEM; + + dev_info(&client->dev, "requesting fw name = %s\n", fw_name); + error = request_firmware(&fw, fw_name, &client->dev); + kfree(fw_name); if (error) { - dev_err(&client->dev, "failed to request firmware %s: %d\n", - ELAN_FW_FILENAME, error); + dev_err(&client->dev, "failed to request firmware: %d\n", + error); return error; } ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] Input: elants_i2c: Append hw_version to FW file 2015-03-11 1:41 ` Dmitry Torokhov @ 2015-03-11 17:09 ` Charles Mooney 2015-03-11 17:23 ` Dmitry Torokhov 0 siblings, 1 reply; 4+ messages in thread From: Charles Mooney @ 2015-03-11 17:09 UTC (permalink / raw) To: Dmitry Torokhov Cc: linux-input, 劉嘉駿 劉嘉駿, fabf On Tue, Mar 10, 2015 at 6:41 PM, Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote: > Hi Charlie, > > On Tue, Mar 10, 2015 at 12:57:03PM -0700, Charlie Mooney wrote: >> Currently the elants_i2c driver simply requests a static filename >> /lib/firmware/elants_i2c.bin when it gets firmware updates. This is >> a problem if you have two Elan touchscreens using the same driver. >> If both touchscreens have different firmwares, you would need to move >> the files around in your filesystem when you're updating them so that >> they don't get updated with the other's FW. If you have a read-only >> filesystem then this is impossible, even. >> >> This patch changes the elants_i2c driver to automatically append the >> four-hex-digit hw_version of the device onto the name of the FW file >> it's requesting for update. Since different touchscreens should have >> a different hw_version's this means the user needs to append the hw >> version of the touchscreen he or she intends to update onto the end >> of the firmware filename and then the driver will do the rest. >> >> The firmware filenames it looks for now are of the form: >> >> elants_i2c_${HW_VERSION}.bin >> >> eg: >> >> elants_i2c_2a44.bin >> >> Signed-off-by: Charlie Mooney <charliemooney@chromium.org> >> --- >> drivers/input/touchscreen/elants_i2c.c | 13 ++++++++++--- >> 1 file changed, 10 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/input/touchscreen/elants_i2c.c b/drivers/input/touchscreen/elants_i2c.c >> index 926c58e..d34ba57 100644 >> --- a/drivers/input/touchscreen/elants_i2c.c >> +++ b/drivers/input/touchscreen/elants_i2c.c >> @@ -98,7 +98,10 @@ >> #define MAX_FW_UPDATE_RETRIES 30 >> >> #define ELAN_FW_PAGESIZE 132 >> -#define ELAN_FW_FILENAME "elants_i2c.bin" >> +#define ELAN_FW_BASE_FILENAME "elants_i2c" >> +#define ELAN_FW_EXTENSION "bin" >> +#define ELAN_FW_FILENAME_MAX_LEN (ARRAY_SIZE(ELAN_FW_BASE_FILENAME) + \ >> + ARRAY_SIZE(ELAN_FW_EXTENSION) + 5) > > Looked at this again and I don't really like how we need to maintain the > MAX_LEN and make sure it is in sync with the format string. How about > the patch below instead? > > Thanks. > > -- > Dmitry > > > Input: elants_i2c - append hw_version to FW file > > From: Charlie Mooney <charliemooney@chromium.org> > > Currently the elants_i2c driver simply requests a static filename > /lib/firmware/elants_i2c.bin when it gets firmware updates. This is a > problem if you have two Elan touchscreens using the same driver. If both > touchscreens have different firmwares, you would need to move the files > around in your filesystem when you're updating them so that they don't get > updated with the other's FW. If you have a read-only filesystem then this > is impossible, even. > > This patch changes the elants_i2c driver to automatically append the > four-hex-digit hw_version of the device onto the name of the FW file it's > requesting for update. Since different touchscreens should have a > different hw_version's this means the user needs to append the hw version > of the touchscreen he or she intends to update onto the end of the firmware > filename and then the driver will do the rest. > > The firmware filenames it looks for now are of the form: > > elants_i2c_${HW_VERSION}.bin > > eg: > > elants_i2c_2a44.bin > > Signed-off-by: Charlie Mooney <charliemooney@chromium.org> > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com> > --- > drivers/input/touchscreen/elants_i2c.c | 14 ++++++++++---- > 1 file changed, 10 insertions(+), 4 deletions(-) > > diff --git a/drivers/input/touchscreen/elants_i2c.c b/drivers/input/touchscreen/elants_i2c.c > index 926c58e..43b3c9c 100644 > --- a/drivers/input/touchscreen/elants_i2c.c > +++ b/drivers/input/touchscreen/elants_i2c.c > @@ -98,7 +98,6 @@ > #define MAX_FW_UPDATE_RETRIES 30 > > #define ELAN_FW_PAGESIZE 132 > -#define ELAN_FW_FILENAME "elants_i2c.bin" > > /* calibration timeout definition */ > #define ELAN_CALI_TIMEOUT_MSEC 10000 > @@ -697,12 +696,19 @@ static int elants_i2c_fw_update(struct elants_data *ts) > { > struct i2c_client *client = ts->client; > const struct firmware *fw; > + char *fw_name; > int error; > > - error = request_firmware(&fw, ELAN_FW_FILENAME, &client->dev); > + fw_name = kasprintf(GFP_KERNEL, "elants_i2c_%4x.bin", ts->hw_version); > + if (!fw_name) > + return -ENOMEM; > + > + dev_info(&client->dev, "requesting fw name = %s\n", fw_name); > + error = request_firmware(&fw, fw_name, &client->dev); > + kfree(fw_name); > if (error) { > - dev_err(&client->dev, "failed to request firmware %s: %d\n", > - ELAN_FW_FILENAME, error); > + dev_err(&client->dev, "failed to request firmware: %d\n", > + error); > return error; > } > I agree, that does indeed look better. kasprintf() is a smart choice. ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Input: elants_i2c: Append hw_version to FW file 2015-03-11 17:09 ` Charles Mooney @ 2015-03-11 17:23 ` Dmitry Torokhov 0 siblings, 0 replies; 4+ messages in thread From: Dmitry Torokhov @ 2015-03-11 17:23 UTC (permalink / raw) To: Charles Mooney Cc: linux-input, 劉嘉駿 劉嘉駿, fabf On Wed, Mar 11, 2015 at 10:09:18AM -0700, Charles Mooney wrote: > On Tue, Mar 10, 2015 at 6:41 PM, Dmitry Torokhov > <dmitry.torokhov@gmail.com> wrote: > > Hi Charlie, > > > > On Tue, Mar 10, 2015 at 12:57:03PM -0700, Charlie Mooney wrote: > >> Currently the elants_i2c driver simply requests a static filename > >> /lib/firmware/elants_i2c.bin when it gets firmware updates. This is > >> a problem if you have two Elan touchscreens using the same driver. > >> If both touchscreens have different firmwares, you would need to move > >> the files around in your filesystem when you're updating them so that > >> they don't get updated with the other's FW. If you have a read-only > >> filesystem then this is impossible, even. > >> > >> This patch changes the elants_i2c driver to automatically append the > >> four-hex-digit hw_version of the device onto the name of the FW file > >> it's requesting for update. Since different touchscreens should have > >> a different hw_version's this means the user needs to append the hw > >> version of the touchscreen he or she intends to update onto the end > >> of the firmware filename and then the driver will do the rest. > >> > >> The firmware filenames it looks for now are of the form: > >> > >> elants_i2c_${HW_VERSION}.bin > >> > >> eg: > >> > >> elants_i2c_2a44.bin > >> > >> Signed-off-by: Charlie Mooney <charliemooney@chromium.org> > >> --- > >> drivers/input/touchscreen/elants_i2c.c | 13 ++++++++++--- > >> 1 file changed, 10 insertions(+), 3 deletions(-) > >> > >> diff --git a/drivers/input/touchscreen/elants_i2c.c b/drivers/input/touchscreen/elants_i2c.c > >> index 926c58e..d34ba57 100644 > >> --- a/drivers/input/touchscreen/elants_i2c.c > >> +++ b/drivers/input/touchscreen/elants_i2c.c > >> @@ -98,7 +98,10 @@ > >> #define MAX_FW_UPDATE_RETRIES 30 > >> > >> #define ELAN_FW_PAGESIZE 132 > >> -#define ELAN_FW_FILENAME "elants_i2c.bin" > >> +#define ELAN_FW_BASE_FILENAME "elants_i2c" > >> +#define ELAN_FW_EXTENSION "bin" > >> +#define ELAN_FW_FILENAME_MAX_LEN (ARRAY_SIZE(ELAN_FW_BASE_FILENAME) + \ > >> + ARRAY_SIZE(ELAN_FW_EXTENSION) + 5) > > > > Looked at this again and I don't really like how we need to maintain the > > MAX_LEN and make sure it is in sync with the format string. How about > > the patch below instead? > > > > Thanks. > > > > -- > > Dmitry > > > > > > Input: elants_i2c - append hw_version to FW file > > > > From: Charlie Mooney <charliemooney@chromium.org> > > > > Currently the elants_i2c driver simply requests a static filename > > /lib/firmware/elants_i2c.bin when it gets firmware updates. This is a > > problem if you have two Elan touchscreens using the same driver. If both > > touchscreens have different firmwares, you would need to move the files > > around in your filesystem when you're updating them so that they don't get > > updated with the other's FW. If you have a read-only filesystem then this > > is impossible, even. > > > > This patch changes the elants_i2c driver to automatically append the > > four-hex-digit hw_version of the device onto the name of the FW file it's > > requesting for update. Since different touchscreens should have a > > different hw_version's this means the user needs to append the hw version > > of the touchscreen he or she intends to update onto the end of the firmware > > filename and then the driver will do the rest. > > > > The firmware filenames it looks for now are of the form: > > > > elants_i2c_${HW_VERSION}.bin > > > > eg: > > > > elants_i2c_2a44.bin > > > > Signed-off-by: Charlie Mooney <charliemooney@chromium.org> > > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com> > > --- > > drivers/input/touchscreen/elants_i2c.c | 14 ++++++++++---- > > 1 file changed, 10 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/input/touchscreen/elants_i2c.c b/drivers/input/touchscreen/elants_i2c.c > > index 926c58e..43b3c9c 100644 > > --- a/drivers/input/touchscreen/elants_i2c.c > > +++ b/drivers/input/touchscreen/elants_i2c.c > > @@ -98,7 +98,6 @@ > > #define MAX_FW_UPDATE_RETRIES 30 > > > > #define ELAN_FW_PAGESIZE 132 > > -#define ELAN_FW_FILENAME "elants_i2c.bin" > > > > /* calibration timeout definition */ > > #define ELAN_CALI_TIMEOUT_MSEC 10000 > > @@ -697,12 +696,19 @@ static int elants_i2c_fw_update(struct elants_data *ts) > > { > > struct i2c_client *client = ts->client; > > const struct firmware *fw; > > + char *fw_name; > > int error; > > > > - error = request_firmware(&fw, ELAN_FW_FILENAME, &client->dev); > > + fw_name = kasprintf(GFP_KERNEL, "elants_i2c_%4x.bin", ts->hw_version); > > + if (!fw_name) > > + return -ENOMEM; > > + > > + dev_info(&client->dev, "requesting fw name = %s\n", fw_name); > > + error = request_firmware(&fw, fw_name, &client->dev); > > + kfree(fw_name); > > if (error) { > > - dev_err(&client->dev, "failed to request firmware %s: %d\n", > > - ELAN_FW_FILENAME, error); > > + dev_err(&client->dev, "failed to request firmware: %d\n", > > + error); > > return error; > > } > > > > I agree, that does indeed look better. kasprintf() is a smart choice. Thanks, applied the 2nd version then. -- Dmitry ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2015-03-11 17:23 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2015-03-10 19:57 [PATCH] Input: elants_i2c: Append hw_version to FW file Charlie Mooney 2015-03-11 1:41 ` Dmitry Torokhov 2015-03-11 17:09 ` Charles Mooney 2015-03-11 17:23 ` Dmitry Torokhov
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).