[PATCH 2/3] Input: ili210x - switch to using cleanup functions in firmware code

From: Dmitry Torokhov
Date: Sun Jun 09 2024 - 19:48:22 EST


Start using __free() attributes to simplify the code and error handling.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@xxxxxxxxx>
---
drivers/input/touchscreen/ili210x.c | 123 ++++++++++++++--------------
1 file changed, 63 insertions(+), 60 deletions(-)

diff --git a/drivers/input/touchscreen/ili210x.c b/drivers/input/touchscreen/ili210x.c
index f3c3ad70244f..55f3852c8dae 100644
--- a/drivers/input/touchscreen/ili210x.c
+++ b/drivers/input/touchscreen/ili210x.c
@@ -582,14 +582,12 @@ static ssize_t ili210x_calibrate(struct device *dev,
}
static DEVICE_ATTR(calibrate, S_IWUSR, NULL, ili210x_calibrate);

-static int ili251x_firmware_to_buffer(const struct firmware *fw,
- u8 **buf, u16 *ac_end, u16 *df_end)
+static const u8 *ili251x_firmware_to_buffer(const struct firmware *fw,
+ u16 *ac_end, u16 *df_end)
{
const struct ihex_binrec *rec;
u32 fw_addr, fw_last_addr = 0;
u16 fw_len;
- u8 *fw_buf;
- int error;

/*
* The firmware ihex blob can never be bigger than 64 kiB, so make this
@@ -597,9 +595,9 @@ static int ili251x_firmware_to_buffer(const struct firmware *fw,
* once, copy them all into this buffer at the right locations, and then
* do all operations on this linear buffer.
*/
- fw_buf = kvmalloc(SZ_64K, GFP_KERNEL);
+ u8* fw_buf __free(kvfree) = kvmalloc(SZ_64K, GFP_KERNEL);
if (!fw_buf)
- return -ENOMEM;
+ return ERR_PTR(-ENOMEM);

rec = (const struct ihex_binrec *)fw->data;
while (rec) {
@@ -607,10 +605,8 @@ static int ili251x_firmware_to_buffer(const struct firmware *fw,
fw_len = be16_to_cpu(rec->len);

/* The last 32 Byte firmware block can be 0xffe0 */
- if (fw_addr + fw_len > SZ_64K || fw_addr > SZ_64K - 32) {
- error = -EFBIG;
- goto err_big;
- }
+ if (fw_addr + fw_len > SZ_64K || fw_addr > SZ_64K - 32)
+ return ERR_PTR(-EFBIG);

/* Find the last address before DF start address, that is AC end */
if (fw_addr == 0xf000)
@@ -623,12 +619,8 @@ static int ili251x_firmware_to_buffer(const struct firmware *fw,

/* DF end address is the last address in the firmware blob */
*df_end = fw_addr + fw_len;
- *buf = fw_buf;
- return 0;

-err_big:
- kvfree(fw_buf);
- return error;
+ return_ptr(fw_buf);
}

/* Switch mode between Application and BootLoader */
@@ -691,7 +683,7 @@ static int ili251x_firmware_busy(struct i2c_client *client)
return 0;
}

-static int ili251x_firmware_write_to_ic(struct device *dev, u8 *fwbuf,
+static int ili251x_firmware_write_to_ic(struct device *dev, const u8 *fwbuf,
u16 start, u16 end, u8 dataflash)
{
struct i2c_client *client = to_i2c_client(dev);
@@ -776,47 +768,17 @@ static void ili210x_hardware_reset(struct gpio_desc *reset_gpio)
msleep(300);
}

-static ssize_t ili210x_firmware_update_store(struct device *dev,
- struct device_attribute *attr,
- const char *buf, size_t count)
+static int ili210x_do_firmware_update(struct ili210x *priv,
+ const u8 *fwbuf, u16 ac_end, u16 df_end)
{
- struct i2c_client *client = to_i2c_client(dev);
- struct ili210x *priv = i2c_get_clientdata(client);
- const char *fwname = ILI251X_FW_FILENAME;
- const struct firmware *fw;
- u16 ac_end, df_end;
- u8 *fwbuf;
+ struct i2c_client *client = priv->client;
+ struct device *dev = &client->dev;
int error;
int i;

- error = request_ihex_firmware(&fw, fwname, dev);
- if (error) {
- dev_err(dev, "Failed to request firmware %s, error=%d\n",
- fwname, error);
- return error;
- }
-
- error = ili251x_firmware_to_buffer(fw, &fwbuf, &ac_end, &df_end);
- release_firmware(fw);
- if (error)
- return error;
-
- /*
- * Disable touchscreen IRQ, so that we would not get spurious touch
- * interrupt during firmware update, and so that the IRQ handler won't
- * trigger and interfere with the firmware update. There is no bit in
- * the touch controller to disable the IRQs during update, so we have
- * to do it this way here.
- */
- disable_irq(client->irq);
-
- dev_dbg(dev, "Firmware update started, firmware=%s\n", fwname);
-
- ili210x_hardware_reset(priv->reset_gpio);
-
error = ili251x_firmware_reset(client);
if (error)
- goto exit;
+ return error;

/* This may not succeed on first try, so re-try a few times. */
for (i = 0; i < 5; i++) {
@@ -826,7 +788,7 @@ static ssize_t ili210x_firmware_update_store(struct device *dev,
}

if (error)
- goto exit;
+ return error;

dev_dbg(dev, "IC is now in BootLoader mode\n");

@@ -835,7 +797,7 @@ static ssize_t ili210x_firmware_update_store(struct device *dev,
error = ili251x_firmware_write_to_ic(dev, fwbuf, 0xf000, df_end, 1);
if (error) {
dev_err(dev, "DF firmware update failed, error=%d\n", error);
- goto exit;
+ return error;
}

dev_dbg(dev, "DataFlash firmware written\n");
@@ -843,7 +805,7 @@ static ssize_t ili210x_firmware_update_store(struct device *dev,
error = ili251x_firmware_write_to_ic(dev, fwbuf, 0x2000, ac_end, 0);
if (error) {
dev_err(dev, "AC firmware update failed, error=%d\n", error);
- goto exit;
+ return error;
}

dev_dbg(dev, "Application firmware written\n");
@@ -856,22 +818,63 @@ static ssize_t ili210x_firmware_update_store(struct device *dev,
}

if (error)
- goto exit;
+ return error;

dev_dbg(dev, "IC is now in Application mode\n");

error = ili251x_firmware_update_cached_state(dev);
if (error)
- goto exit;
+ return error;

- error = count;
+ return 0;
+}
+
+static ssize_t ili210x_firmware_update_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct i2c_client *client = to_i2c_client(dev);
+ struct ili210x *priv = i2c_get_clientdata(client);
+ const char *fwname = ILI251X_FW_FILENAME;
+ u16 ac_end, df_end;
+ int error;
+
+ const struct firmware *fw __free(firmware) = NULL;
+ error = request_ihex_firmware(&fw, fwname, dev);
+ if (error) {
+ dev_err(dev, "Failed to request firmware %s, error=%d\n",
+ fwname, error);
+ return error;
+ }
+
+ const u8* fwbuf __free(kvfree) =
+ ili251x_firmware_to_buffer(fw, &ac_end, &df_end);
+ error = PTR_ERR_OR_ZERO(fwbuf);
+ if (error)
+ return error;
+
+ /*
+ * Disable touchscreen IRQ, so that we would not get spurious touch
+ * interrupt during firmware update, and so that the IRQ handler won't
+ * trigger and interfere with the firmware update. There is no bit in
+ * the touch controller to disable the IRQs during update, so we have
+ * to do it this way here.
+ */
+ disable_irq(client->irq);
+
+ dev_dbg(dev, "Firmware update started, firmware=%s\n", fwname);
+
+ ili210x_hardware_reset(priv->reset_gpio);
+
+ error = ili210x_do_firmware_update(priv, fwbuf, ac_end, df_end);

-exit:
ili210x_hardware_reset(priv->reset_gpio);
+
dev_dbg(dev, "Firmware update ended, error=%i\n", error);
+
enable_irq(client->irq);
- kvfree(fwbuf);
- return error;
+
+ return error ?: count;
}

static DEVICE_ATTR(firmware_update, 0200, NULL, ili210x_firmware_update_store);
--
2.45.2.505.gda0bf45e8d-goog