2024-04-11 21:18:04

by Ricardo Ribalda

[permalink] [raw]
Subject: [PATCH 0/7] media: Fix more smatch warnings

This has only been compile tested.

Signed-off-by: Ricardo Ribalda <[email protected]>
---
Ricardo Ribalda (7):
media: solo6x10: Use pcim functions
media: solo6x10: Use devm functions
media: saa7134: Use devm_request_irq
media: c8sectpfe: Refactor load_c8sectpfe_fw
media: tunner: xc5000: Refactor firmware load
media: dvb-frontends: drx39xyj: Refactor firmware upload
media: dvb-usb: dib0700_devices: Add missing release_firmware()

drivers/media/dvb-frontends/drx39xyj/drx_driver.h | 2 -
drivers/media/dvb-frontends/drx39xyj/drxj.c | 49 ++++++++++------------
drivers/media/pci/saa7134/saa7134-alsa.c | 9 +---
drivers/media/pci/solo6x10/solo6x10-core.c | 16 ++-----
.../platform/st/sti/c8sectpfe/c8sectpfe-core.c | 2 +-
drivers/media/tuners/xc5000.c | 39 ++++++++---------
drivers/media/usb/dvb-usb/dib0700_devices.c | 18 ++++++--
7 files changed, 62 insertions(+), 73 deletions(-)
---
base-commit: b14257abe7057def6127f6fb2f14f9adc8acabdb
change-id: 20240411-fix-smatch-3b1d0259f553

Best regards,
--
Ricardo Ribalda <[email protected]>



2024-04-11 21:18:21

by Ricardo Ribalda

[permalink] [raw]
Subject: [PATCH 2/7] media: solo6x10: Use devm functions

Let devm handle the life cycle of the irq request.

Makes smatch happier:

drivers/media/pci/solo6x10/solo6x10-core.c:631 solo_pci_probe() warn: 'pdev' from pci_request_regions() not released on lines: 631.

Signed-off-by: Ricardo Ribalda <[email protected]>
---
drivers/media/pci/solo6x10/solo6x10-core.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/media/pci/solo6x10/solo6x10-core.c b/drivers/media/pci/solo6x10/solo6x10-core.c
index abf30b7609e17..1a9e2bccc4136 100644
--- a/drivers/media/pci/solo6x10/solo6x10-core.c
+++ b/drivers/media/pci/solo6x10/solo6x10-core.c
@@ -144,7 +144,6 @@ static void free_solo_dev(struct solo_dev *solo_dev)

/* Now cleanup the PCI device */
solo_irq_off(solo_dev, ~0);
- free_irq(pdev->irq, solo_dev);
}

pci_disable_device(pdev);
@@ -544,8 +543,8 @@ static int solo_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
/* PLL locking time of 1ms */
mdelay(1);

- ret = request_irq(pdev->irq, solo_isr, IRQF_SHARED, SOLO6X10_NAME,
- solo_dev);
+ ret = devm_request_irq(&pdev->dev, pdev->irq, solo_isr, IRQF_SHARED,
+ SOLO6X10_NAME, solo_dev);
if (ret)
goto fail_probe;


--
2.44.0.683.g7961c838ac-goog


2024-04-11 21:18:22

by Ricardo Ribalda

[permalink] [raw]
Subject: [PATCH 1/7] media: solo6x10: Use pcim functions

Instead of handling manually the release of the memory regions let devm
do that for us.

Makes smatch happy:
drivers/media/pci/solo6x10/solo6x10-core.c:631 solo_pci_probe() warn: 'pdev' from pci_request_regions() not released on lines: 631.

Signed-off-by: Ricardo Ribalda <[email protected]>
---
drivers/media/pci/solo6x10/solo6x10-core.c | 11 ++---------
1 file changed, 2 insertions(+), 9 deletions(-)

diff --git a/drivers/media/pci/solo6x10/solo6x10-core.c b/drivers/media/pci/solo6x10/solo6x10-core.c
index 6d87fbb0ee04a..abf30b7609e17 100644
--- a/drivers/media/pci/solo6x10/solo6x10-core.c
+++ b/drivers/media/pci/solo6x10/solo6x10-core.c
@@ -145,10 +145,8 @@ static void free_solo_dev(struct solo_dev *solo_dev)
/* Now cleanup the PCI device */
solo_irq_off(solo_dev, ~0);
free_irq(pdev->irq, solo_dev);
- pci_iounmap(pdev, solo_dev->reg_base);
}

- pci_release_regions(pdev);
pci_disable_device(pdev);
v4l2_device_unregister(&solo_dev->v4l2_dev);
pci_set_drvdata(pdev, NULL);
@@ -480,15 +478,10 @@ static int solo_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
pci_write_config_byte(pdev, 0x40, 0x00);
pci_write_config_byte(pdev, 0x41, 0x00);

- ret = pci_request_regions(pdev, SOLO6X10_NAME);
+ ret = pcim_iomap_regions(pdev, BIT(0), SOLO6X10_NAME);
if (ret)
goto fail_probe;
-
- solo_dev->reg_base = pci_ioremap_bar(pdev, 0);
- if (solo_dev->reg_base == NULL) {
- ret = -ENOMEM;
- goto fail_probe;
- }
+ solo_dev->reg_base = pcim_iomap_table(pdev)[0];

chip_id = solo_reg_read(solo_dev, SOLO_CHIP_OPTION) &
SOLO_CHIP_ID_MASK;

--
2.44.0.683.g7961c838ac-goog


2024-04-11 21:19:00

by Ricardo Ribalda

[permalink] [raw]
Subject: [PATCH 4/7] media: c8sectpfe: Refactor load_c8sectpfe_fw

release_firmware() in the same function that it was requested. It is
more clear and makes smatch happy.

drivers/media/platform/st/sti/c8sectpfe/c8sectpfe-core.c:1146 load_c8sectpfe_fw() warn: 'fw' from request_firmware() not released on lines: 1125,1132.

Signed-off-by: Ricardo Ribalda <[email protected]>
---
drivers/media/platform/st/sti/c8sectpfe/c8sectpfe-core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/platform/st/sti/c8sectpfe/c8sectpfe-core.c b/drivers/media/platform/st/sti/c8sectpfe/c8sectpfe-core.c
index e4cf27b5a0727..ce0fd6ace8032 100644
--- a/drivers/media/platform/st/sti/c8sectpfe/c8sectpfe-core.c
+++ b/drivers/media/platform/st/sti/c8sectpfe/c8sectpfe-core.c
@@ -1097,7 +1097,6 @@ static int load_slim_core_fw(const struct firmware *fw, struct c8sectpfei *fei)
}
}

- release_firmware(fw);
return err;
}

@@ -1121,6 +1120,7 @@ static int load_c8sectpfe_fw(struct c8sectpfei *fei)
}

err = load_slim_core_fw(fw, fei);
+ release_firmware(fw);
if (err) {
dev_err(fei->dev, "load_slim_core_fw failed err=(%d)\n", err);
return err;

--
2.44.0.683.g7961c838ac-goog


2024-04-11 21:19:13

by Ricardo Ribalda

[permalink] [raw]
Subject: [PATCH 5/7] media: tunner: xc5000: Refactor firmware load

Make sure the firmware is released when we leave
xc_load_fw_and_init_tuner()

This change makes smatch happy:
drivers/media/tuners/xc5000.c:1213 xc_load_fw_and_init_tuner() warn: 'fw' from request_firmware() not released on lines: 1213.

Cc: Shuah Khan <[email protected]>
Signed-off-by: Ricardo Ribalda <[email protected]>
---
drivers/media/tuners/xc5000.c | 39 +++++++++++++++++----------------------
1 file changed, 17 insertions(+), 22 deletions(-)

diff --git a/drivers/media/tuners/xc5000.c b/drivers/media/tuners/xc5000.c
index 2182e5b7b6064..30aa4ee958bde 100644
--- a/drivers/media/tuners/xc5000.c
+++ b/drivers/media/tuners/xc5000.c
@@ -58,7 +58,7 @@ struct xc5000_priv {
struct dvb_frontend *fe;
struct delayed_work timer_sleep;

- const struct firmware *firmware;
+ bool inited;
};

/* Misc Defines */
@@ -1110,23 +1110,19 @@ static int xc_load_fw_and_init_tuner(struct dvb_frontend *fe, int force)
if (!force && xc5000_is_firmware_loaded(fe) == 0)
return 0;

- if (!priv->firmware) {
- ret = request_firmware(&fw, desired_fw->name,
- priv->i2c_props.adap->dev.parent);
- if (ret) {
- pr_err("xc5000: Upload failed. rc %d\n", ret);
- return ret;
- }
- dprintk(1, "firmware read %zu bytes.\n", fw->size);
+ ret = request_firmware(&fw, desired_fw->name,
+ priv->i2c_props.adap->dev.parent);
+ if (ret) {
+ pr_err("xc5000: Upload failed. rc %d\n", ret);
+ return ret;
+ }
+ dprintk(1, "firmware read %zu bytes.\n", fw->size);

- if (fw->size != desired_fw->size) {
- pr_err("xc5000: Firmware file with incorrect size\n");
- release_firmware(fw);
- return -EINVAL;
- }
- priv->firmware = fw;
- } else
- fw = priv->firmware;
+ if (fw->size != desired_fw->size) {
+ pr_err("xc5000: Firmware file with incorrect size\n");
+ release_firmware(fw);
+ return -EINVAL;
+ }

/* Try up to 5 times to load firmware */
for (i = 0; i < 5; i++) {
@@ -1204,6 +1200,7 @@ static int xc_load_fw_and_init_tuner(struct dvb_frontend *fe, int force)
}

err:
+ release_firmware(fw);
if (!ret)
printk(KERN_INFO "xc5000: Firmware %s loaded and running.\n",
desired_fw->name);
@@ -1274,7 +1271,7 @@ static int xc5000_resume(struct dvb_frontend *fe)

/* suspended before firmware is loaded.
Avoid firmware load in resume path. */
- if (!priv->firmware)
+ if (!priv->inited)
return 0;

return xc5000_set_params(fe);
@@ -1293,6 +1290,8 @@ static int xc5000_init(struct dvb_frontend *fe)
if (debug)
xc_debug_dump(priv);

+ priv->inited = true;
+
return 0;
}

@@ -1306,10 +1305,6 @@ static void xc5000_release(struct dvb_frontend *fe)

if (priv) {
cancel_delayed_work(&priv->timer_sleep);
- if (priv->firmware) {
- release_firmware(priv->firmware);
- priv->firmware = NULL;
- }
hybrid_tuner_release_state(priv);
}


--
2.44.0.683.g7961c838ac-goog


2024-04-11 21:19:47

by Ricardo Ribalda

[permalink] [raw]
Subject: [PATCH 7/7] media: dvb-usb: dib0700_devices: Add missing release_firmware()

Add missing release_firmware on the error paths.

drivers/media/usb/dvb-usb/dib0700_devices.c:2415 stk9090m_frontend_attach() warn: 'state->frontend_firmware' from request_firmware() not released on lines: 2415.
drivers/media/usb/dvb-usb/dib0700_devices.c:2497 nim9090md_frontend_attach() warn: 'state->frontend_firmware' from request_firmware() not released on lines: 2489,2497.

Signed-off-by: Ricardo Ribalda <[email protected]>
---
drivers/media/usb/dvb-usb/dib0700_devices.c | 18 +++++++++++++++---
1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/drivers/media/usb/dvb-usb/dib0700_devices.c b/drivers/media/usb/dvb-usb/dib0700_devices.c
index 3af594134a6de..6ddc205133939 100644
--- a/drivers/media/usb/dvb-usb/dib0700_devices.c
+++ b/drivers/media/usb/dvb-usb/dib0700_devices.c
@@ -2412,7 +2412,12 @@ static int stk9090m_frontend_attach(struct dvb_usb_adapter *adap)

adap->fe_adap[0].fe = dvb_attach(dib9000_attach, &adap->dev->i2c_adap, 0x80, &stk9090m_config);

- return adap->fe_adap[0].fe == NULL ? -ENODEV : 0;
+ if (!adap->fe_adap[0].fe) {
+ release_firmware(state->frontend_firmware);
+ return -ENODEV;
+ }
+
+ return 0;
}

static int dib9090_tuner_attach(struct dvb_usb_adapter *adap)
@@ -2485,8 +2490,10 @@ static int nim9090md_frontend_attach(struct dvb_usb_adapter *adap)
dib9000_i2c_enumeration(&adap->dev->i2c_adap, 1, 0x20, 0x80);
adap->fe_adap[0].fe = dvb_attach(dib9000_attach, &adap->dev->i2c_adap, 0x80, &nim9090md_config[0]);

- if (adap->fe_adap[0].fe == NULL)
+ if (!adap->fe_adap[0].fe) {
+ release_firmware(state->frontend_firmware);
return -ENODEV;
+ }

i2c = dib9000_get_i2c_master(adap->fe_adap[0].fe, DIBX000_I2C_INTERFACE_GPIO_3_4, 0);
dib9000_i2c_enumeration(i2c, 1, 0x12, 0x82);
@@ -2494,7 +2501,12 @@ static int nim9090md_frontend_attach(struct dvb_usb_adapter *adap)
fe_slave = dvb_attach(dib9000_attach, i2c, 0x82, &nim9090md_config[1]);
dib9000_set_slave_frontend(adap->fe_adap[0].fe, fe_slave);

- return fe_slave == NULL ? -ENODEV : 0;
+ if (!fe_slave) {
+ release_firmware(state->frontend_firmware);
+ return -ENODEV;
+ }
+
+ return 0;
}

static int nim9090md_tuner_attach(struct dvb_usb_adapter *adap)

--
2.44.0.683.g7961c838ac-goog


2024-04-11 21:24:23

by Ricardo Ribalda

[permalink] [raw]
Subject: [PATCH 3/7] media: saa7134: Use devm_request_irq

The handled version of request_irq let us remove the free_irq and makes
smatch happier:

drivers/media/pci/saa7134/saa7134-alsa.c:1186 alsa_card_saa7134_create() warn: 'dev->pci->irq' from request_irq() not released on lines: 1186

Signed-off-by: Ricardo Ribalda <[email protected]>
---
drivers/media/pci/saa7134/saa7134-alsa.c | 9 ++-------
1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/drivers/media/pci/saa7134/saa7134-alsa.c b/drivers/media/pci/saa7134/saa7134-alsa.c
index d3cde05a6ebab..dd2236c5c4a17 100644
--- a/drivers/media/pci/saa7134/saa7134-alsa.c
+++ b/drivers/media/pci/saa7134/saa7134-alsa.c
@@ -1096,9 +1096,6 @@ static void snd_saa7134_free(struct snd_card * card)
if (chip->dev->dmasound.priv_data == NULL)
return;

- if (chip->irq >= 0)
- free_irq(chip->irq, &chip->dev->dmasound);
-
chip->dev->dmasound.priv_data = NULL;

}
@@ -1147,10 +1144,8 @@ static int alsa_card_saa7134_create(struct saa7134_dev *dev, int devnum)
chip->iobase = pci_resource_start(dev->pci, 0);


- err = request_irq(dev->pci->irq, saa7134_alsa_irq,
- IRQF_SHARED, dev->name,
- (void*) &dev->dmasound);
-
+ err = devm_request_irq(&dev->pci->dev, dev->pci->irq, saa7134_alsa_irq,
+ IRQF_SHARED, dev->name, &dev->dmasound);
if (err < 0) {
pr_err("%s: can't get IRQ %d for ALSA\n",
dev->name, dev->pci->irq);

--
2.44.0.683.g7961c838ac-goog


2024-04-11 21:25:49

by Ricardo Ribalda

[permalink] [raw]
Subject: [PATCH 6/7] media: dvb-frontends: drx39xyj: Refactor firmware upload

Do not cache the file, instead load it on demand.

This makes smatch a happy parser:
drivers/media/dvb-frontends/drx39xyj/drxj.c:11908 drx_ctrl_u_code() warn: 'fw' from request_firmware() not released on lines: 11877,11886,11896.

Signed-off-by: Ricardo Ribalda <[email protected]>
---
drivers/media/dvb-frontends/drx39xyj/drx_driver.h | 2 -
drivers/media/dvb-frontends/drx39xyj/drxj.c | 49 +++++++++++------------
2 files changed, 23 insertions(+), 28 deletions(-)

diff --git a/drivers/media/dvb-frontends/drx39xyj/drx_driver.h b/drivers/media/dvb-frontends/drx39xyj/drx_driver.h
index 15f7e58c5a308..2c2fd4bf79ccf 100644
--- a/drivers/media/dvb-frontends/drx39xyj/drx_driver.h
+++ b/drivers/media/dvb-frontends/drx39xyj/drx_driver.h
@@ -33,7 +33,6 @@

#include <linux/kernel.h>
#include <linux/errno.h>
-#include <linux/firmware.h>
#include <linux/i2c.h>

/*
@@ -1910,7 +1909,6 @@ struct drx_demod_instance {
/* generic demodulator data */

struct i2c_adapter *i2c;
- const struct firmware *firmware;
};

/*-------------------------------------------------------------------------
diff --git a/drivers/media/dvb-frontends/drx39xyj/drxj.c b/drivers/media/dvb-frontends/drx39xyj/drxj.c
index 19d8de400a687..1ef53754bc037 100644
--- a/drivers/media/dvb-frontends/drx39xyj/drxj.c
+++ b/drivers/media/dvb-frontends/drx39xyj/drxj.c
@@ -56,6 +56,7 @@ INCLUDE FILES
#define pr_fmt(fmt) KBUILD_MODNAME ":%s: " fmt, __func__

#include <linux/module.h>
+#include <linux/firmware.h>
#include <linux/init.h>
#include <linux/string.h>
#include <linux/slab.h>
@@ -11750,6 +11751,7 @@ static int drx_ctrl_u_code(struct drx_demod_instance *demod,
u8 *mc_data = NULL;
unsigned size;
char *mc_file;
+ const struct firmware *fw;

/* Check arguments */
if (!mc_info || !mc_info->mc_file)
@@ -11757,28 +11759,22 @@ static int drx_ctrl_u_code(struct drx_demod_instance *demod,

mc_file = mc_info->mc_file;

- if (!demod->firmware) {
- const struct firmware *fw = NULL;
-
- rc = request_firmware(&fw, mc_file, demod->i2c->dev.parent);
- if (rc < 0) {
- pr_err("Couldn't read firmware %s\n", mc_file);
- return rc;
- }
- demod->firmware = fw;
-
- if (demod->firmware->size < 2 * sizeof(u16)) {
- rc = -EINVAL;
- pr_err("Firmware is too short!\n");
- goto release;
- }
+ rc = request_firmware(&fw, mc_file, demod->i2c->dev.parent);
+ if (rc < 0) {
+ pr_err("Couldn't read firmware %s\n", mc_file);
+ return rc;
+ }

- pr_info("Firmware %s, size %zu\n",
- mc_file, demod->firmware->size);
+ if (fw->size < 2 * sizeof(u16)) {
+ rc = -EINVAL;
+ pr_err("Firmware is too short!\n");
+ goto release;
}

- mc_data_init = demod->firmware->data;
- size = demod->firmware->size;
+ pr_info("Firmware %s, size %zu\n", mc_file, fw->size);
+
+ mc_data_init = fw->data;
+ size = fw->size;

mc_data = (void *)mc_data_init;
/* Check data */
@@ -11874,7 +11870,8 @@ static int drx_ctrl_u_code(struct drx_demod_instance *demod,
0x0000)) {
pr_err("error reading firmware at pos %zd\n",
mc_data - mc_data_init);
- return -EIO;
+ rc = -EIO;
+ goto release;
}

result = memcmp(curr_ptr, mc_data_buffer,
@@ -11883,7 +11880,8 @@ static int drx_ctrl_u_code(struct drx_demod_instance *demod,
if (result) {
pr_err("error verifying firmware at pos %zd\n",
mc_data - mc_data_init);
- return -EIO;
+ rc = -EIO;
+ goto release;
}

curr_addr += ((dr_xaddr_t)(bytes_to_comp / 2));
@@ -11893,17 +11891,17 @@ static int drx_ctrl_u_code(struct drx_demod_instance *demod,
break;
}
default:
- return -EINVAL;
+ rc = -EINVAL;
+ goto release;

}
mc_data += mc_block_nr_bytes;
}

- return 0;
+ rc = 0;

release:
- release_firmware(demod->firmware);
- demod->firmware = NULL;
+ release_firmware(fw);

return rc;
}
@@ -12271,7 +12269,6 @@ static void drx39xxj_release(struct dvb_frontend *fe)
kfree(demod->my_ext_attr);
kfree(demod->my_common_attr);
kfree(demod->my_i2c_dev_addr);
- release_firmware(demod->firmware);
kfree(demod);
kfree(state);
}

--
2.44.0.683.g7961c838ac-goog