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@xxxxxxxxx> 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@xxxxxxxxxxxx>
> >> ---
> >> 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@xxxxxxxxxxxx>
> >
> > 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@xxxxxxxxxxxx>
> > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@xxxxxxxxx>
> > ---
> > 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
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
