[2/3] media: si2157: Add optional firmware download

Commit Message

Mauro Carvalho Chehab Dec. 8, 2021, 10:13 a.m. UTC

From: Robert Schlabbach <[e-mail]>

The Si2157 (A30) is functional with the ROM firmware 3.0.5, but can also
be patched at runtime, e.g. to firmware 3.1.3. However, although a
firmware filename for its firmware patch exists, that has only been used
for the Si2177 (A30) so far (which indeed takes the binary identical
firmware patch).

Add support for downloading firmware patches into the Si2157 (A30), but
make it optional, so that initialization can succeed with and without a
firmware patch being available. Keep the use of request_firmware() for
this purpose rather than firmware_request_nowarn(), so that the warning
in the log makes users aware that it is possible to provide a firmware
for this tuner.

The firmware patch is probably also optional for other (if not all)
tuners supported by the driver, but since I do not have the others
available to test, they are kept mandatory for now to avoid regressions.

Signed-off-by: Robert Schlabbach <[e-mail]>
Signed-off-by: Mauro Carvalho Chehab <[e-mail]>
---

To avoid mailbombing on a large number of people, only mailing lists were C/C on the cover.
See [PATCH 0/3] at: https://lore.kernel.org/all/[e-mail]/

 drivers/media/tuners/si2157.c | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

Comments

Robert Schlabbach Dec. 8, 2021, 4:45 p.m. UTC | #1

> 	ret = si2157_load_firmware(fe, fw_name);
>  	if (ret) {
> +		if (!fw_required)
> +			goto skip_fw_download;
> +

In conjunction with my proposal for PATCH 1/3, this can be simplified to:

ret = si2157_load_firmware(fe, fw_name);
if (ret && fw_required)
        goto err;

Best Regards,
-Robert Schlabbach

Mauro Carvalho Chehab Dec. 8, 2021, 5:09 p.m. UTC | #2

Em Wed, 8 Dec 2021 17:45:43 +0100
Robert Schlabbach <[e-mail]> escreveu:

> > 	ret = si2157_load_firmware(fe, fw_name);
> >  	if (ret) {
> > +		if (!fw_required)
> > +			goto skip_fw_download;
> > +  
> 
> In conjunction with my proposal for PATCH 1/3, this can be simplified to:
> 
> ret = si2157_load_firmware(fe, fw_name);
> if (ret && fw_required)
>         goto err;

See the patch 3:

	ret = si2157_load_firmware(fe, fw_name);
+	if (fw_required && ret == -ENOENT)
+		warn_firmware_not_loaded = true;
+	else if (ret < 0) {
+		dev_err(&client->dev, "error %d when loading firmware file '%s'\n",
+			ret, fw_name);

Basically, it will do (about) the same thing you proposed, with one
important difference: It should only ignore errors loading the firmware
when the error is due to a non-existing firmware file. If the firmware
file is corrupted or can't be load for other reasons (ENOMEM, corrupted
file, etc), it will print the error code and bail out.

Thanks,
Mauro

Patch

diff --git a/drivers/media/tuners/si2157.c b/drivers/media/tuners/si2157.c
index 481a5db7fb69..ed28672c060d 100644
--- a/drivers/media/tuners/si2157.c
+++ b/drivers/media/tuners/si2157.c
@@ -130,6 +130,7 @@  static int si2157_init(struct dvb_frontend *fe)
 	struct i2c_client *client = fe->tuner_priv;
 	struct si2157_dev *dev = i2c_get_clientdata(client);
 	unsigned int chip_id, xtal_trim;
+	unsigned int fw_required;
 	struct si2157_cmd cmd;
 	const char *fw_name;
 	int ret;
@@ -198,6 +199,10 @@  static int si2157_init(struct dvb_frontend *fe)
 	#define SI2146_A10 ('A' << 24 | 46 << 16 | '1' << 8 | '0' << 0)
 	#define SI2141_A10 ('A' << 24 | 41 << 16 | '1' << 8 | '0' << 0)
 
+	/* assume firmware is required, unless verified not to be */
+	/* only the SI2157_A30 has been verified not to yet */
+	fw_required = true;
+
 	switch (chip_id) {
 	case SI2158_A20:
 	case SI2148_A20:
@@ -206,10 +211,13 @@  static int si2157_init(struct dvb_frontend *fe)
 	case SI2141_A10:
 		fw_name = SI2141_A10_FIRMWARE;
 		break;
-	case SI2177_A30:
-		fw_name = SI2157_A30_FIRMWARE;
-		break;
 	case SI2157_A30:
+		fw_name = SI2157_A30_FIRMWARE;
+		fw_required = false;
+		break;
+	case SI2177_A30:
+		fw_name = SI2157_A30_FIRMWARE;
+		break;
 	case SI2147_A30:
 	case SI2146_A10:
 		fw_name = NULL;
@@ -230,6 +238,9 @@  static int si2157_init(struct dvb_frontend *fe)
 
 	ret = si2157_load_firmware(fe, fw_name);
 	if (ret) {
+		if (!fw_required)
+			goto skip_fw_download;
+
 		dev_err(&client->dev, "firmware file '%s' not found\n",
 			fw_name);
 		goto err;