Fix firmware file names assignment in si2157 tuner, allow for running
devices without firmware files needed.
It's regression in kernel 5.17.0, worked fine in 5.16 series.
device: 07ca:1871 AVerMedia Technologies, Inc. TD310 DVB-T/T2/C dongle
modprobe gives error: unknown chip version Si2147-A30 ROM 0x50
Device initialization is interrupted.
caused by:
1. table si2157_tuners has swapped fields rom_id and required vs struct
si2157_tuner_info.
2. both firmware file names can be null for devices with required ==
false - device uses build-in firmware in this case
Fix:
1. Rearrange fields in table si2157_tuners
2. Allow both firmware file names be NULL for devices defined with
required == false
Fixes: 1c35ba3bf972 ("media: si2157: use a different namespace for
firmware")
Link: https://bugzilla.kernel.org/show_bug.cgi?id=215726
Link:
https://lore.kernel.org/lkml/[email protected]/
Cc: [email protected] # 5.17.x
Signed-off-by: Piotr Chmura <[email protected]>
Tested-by: Robert Schlabbach <[email protected]>
---
--- a/drivers/media/tuners/si2157.c 2022-03-20 21:14:17.000000000 +0100
+++ b/drivers/media/tuners/si2157.c 2022-03-22 23:48:05.604408331 +0100
@@ -77,16 +77,16 @@ err_mutex_unlock:
}
static const struct si2157_tuner_info si2157_tuners[] = {
- { SI2141, false, 0x60, SI2141_60_FIRMWARE, SI2141_A10_FIRMWARE },
- { SI2141, false, 0x61, SI2141_61_FIRMWARE, SI2141_A10_FIRMWARE },
- { SI2146, false, 0x11, SI2146_11_FIRMWARE, NULL },
- { SI2147, false, 0x50, SI2147_50_FIRMWARE, NULL },
- { SI2148, true, 0x32, SI2148_32_FIRMWARE, SI2158_A20_FIRMWARE },
- { SI2148, true, 0x33, SI2148_33_FIRMWARE, SI2158_A20_FIRMWARE },
- { SI2157, false, 0x50, SI2157_50_FIRMWARE, SI2157_A30_FIRMWARE },
- { SI2158, false, 0x50, SI2158_50_FIRMWARE, SI2158_A20_FIRMWARE },
- { SI2158, false, 0x51, SI2158_51_FIRMWARE, SI2158_A20_FIRMWARE },
- { SI2177, false, 0x50, SI2177_50_FIRMWARE, SI2157_A30_FIRMWARE },
+ { SI2141, 0x60, false, SI2141_60_FIRMWARE, SI2141_A10_FIRMWARE },
+ { SI2141, 0x61, false, SI2141_61_FIRMWARE, SI2141_A10_FIRMWARE },
+ { SI2146, 0x11, false, SI2146_11_FIRMWARE, NULL },
+ { SI2147, 0x50, false, SI2147_50_FIRMWARE, NULL },
+ { SI2148, 0x32, true, SI2148_32_FIRMWARE, SI2158_A20_FIRMWARE },
+ { SI2148, 0x33, true, SI2148_33_FIRMWARE, SI2158_A20_FIRMWARE },
+ { SI2157, 0x50, false, SI2157_50_FIRMWARE, SI2157_A30_FIRMWARE },
+ { SI2158, 0x50, false, SI2158_50_FIRMWARE, SI2158_A20_FIRMWARE },
+ { SI2158, 0x51, false, SI2158_51_FIRMWARE, SI2158_A20_FIRMWARE },
+ { SI2177, 0x50, false, SI2177_50_FIRMWARE, SI2157_A30_FIRMWARE },
};
static int si2157_load_firmware(struct dvb_frontend *fe,
@@ -178,7 +178,7 @@ static int si2157_find_and_load_firmware
}
}
- if (!fw_name && !fw_alt_name) {
+ if (required && !fw_name && !fw_alt_name) {
dev_err(&client->dev,
"unknown chip version Si21%d-%c%c%c ROM 0x%02x\n",
part_id, cmd.args[1], cmd.args[3], cmd.args[4], rom_id);
Hi, this is your Linux kernel regression tracker. Top-posting for once,
to make this easily accessible to everyone.
Mauro, what's the hold-up here? Below patch is fixing a regression
caused by one of your patches. Why haven't you applied this or provided
feedback to Piotr what needs to be improved? Or am I missing something
and progress was made?
This really is overdue, the first version of this patch was already
posted two and a half weeks ago. Reminder, the newly added document on
handling regressions (
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/handling-regressions.rst#n132
) for this case states this as rule of thumb: "Aim to fix regressions
within one week ".
Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
P.S.: As the Linux kernel's regression tracker I'm getting a lot of
reports on my table. I can only look briefly into most of them and lack
knowledge about most of the areas they concern. I thus unfortunately
will sometimes get things wrong or miss something important. I hope
that's not the case here; if you think it is, don't hesitate to tell me
in a public reply, it's in everyone's interest to set the public record
straight.
#regzbot poke
On 31.03.22 17:55, Piotr Chmura wrote:
> Fix firmware file names assignment in si2157 tuner, allow for running
> devices without firmware files needed.
>
>
> It's regression in kernel 5.17.0, worked fine in 5.16 series.
>
> device: 07ca:1871 AVerMedia Technologies, Inc. TD310 DVB-T/T2/C dongle
> modprobe gives error: unknown chip version Si2147-A30 ROM 0x50
> Device initialization is interrupted.
>
> caused by:
> 1. table si2157_tuners has swapped fields rom_id and required vs struct
> si2157_tuner_info.
> 2. both firmware file names can be null for devices with required ==
> false - device uses build-in firmware in this case
>
> Fix:
> 1. Rearrange fields in table si2157_tuners
> 2. Allow both firmware file names be NULL for devices defined with
> required == false
>
>
> Fixes: 1c35ba3bf972 ("media: si2157: use a different namespace for
> firmware")
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=215726
> Link:
> https://lore.kernel.org/lkml/[email protected]/
>
> Cc: [email protected] # 5.17.x
> Signed-off-by: Piotr Chmura <[email protected]>
> Tested-by: Robert Schlabbach <[email protected]>
>
> ---
>
> --- a/drivers/media/tuners/si2157.c 2022-03-20 21:14:17.000000000 +0100
> +++ b/drivers/media/tuners/si2157.c 2022-03-22 23:48:05.604408331 +0100
> @@ -77,16 +77,16 @@ err_mutex_unlock:
> }
>
> static const struct si2157_tuner_info si2157_tuners[] = {
> - { SI2141, false, 0x60, SI2141_60_FIRMWARE, SI2141_A10_FIRMWARE },
> - { SI2141, false, 0x61, SI2141_61_FIRMWARE, SI2141_A10_FIRMWARE },
> - { SI2146, false, 0x11, SI2146_11_FIRMWARE, NULL },
> - { SI2147, false, 0x50, SI2147_50_FIRMWARE, NULL },
> - { SI2148, true, 0x32, SI2148_32_FIRMWARE, SI2158_A20_FIRMWARE },
> - { SI2148, true, 0x33, SI2148_33_FIRMWARE, SI2158_A20_FIRMWARE },
> - { SI2157, false, 0x50, SI2157_50_FIRMWARE, SI2157_A30_FIRMWARE },
> - { SI2158, false, 0x50, SI2158_50_FIRMWARE, SI2158_A20_FIRMWARE },
> - { SI2158, false, 0x51, SI2158_51_FIRMWARE, SI2158_A20_FIRMWARE },
> - { SI2177, false, 0x50, SI2177_50_FIRMWARE, SI2157_A30_FIRMWARE },
> + { SI2141, 0x60, false, SI2141_60_FIRMWARE, SI2141_A10_FIRMWARE },
> + { SI2141, 0x61, false, SI2141_61_FIRMWARE, SI2141_A10_FIRMWARE },
> + { SI2146, 0x11, false, SI2146_11_FIRMWARE, NULL },
> + { SI2147, 0x50, false, SI2147_50_FIRMWARE, NULL },
> + { SI2148, 0x32, true, SI2148_32_FIRMWARE, SI2158_A20_FIRMWARE },
> + { SI2148, 0x33, true, SI2148_33_FIRMWARE, SI2158_A20_FIRMWARE },
> + { SI2157, 0x50, false, SI2157_50_FIRMWARE, SI2157_A30_FIRMWARE },
> + { SI2158, 0x50, false, SI2158_50_FIRMWARE, SI2158_A20_FIRMWARE },
> + { SI2158, 0x51, false, SI2158_51_FIRMWARE, SI2158_A20_FIRMWARE },
> + { SI2177, 0x50, false, SI2177_50_FIRMWARE, SI2157_A30_FIRMWARE },
> };
>
> static int si2157_load_firmware(struct dvb_frontend *fe,
> @@ -178,7 +178,7 @@ static int si2157_find_and_load_firmware
> }
> }
>
> - if (!fw_name && !fw_alt_name) {
> + if (required && !fw_name && !fw_alt_name) {
> dev_err(&client->dev,
> "unknown chip version Si21%d-%c%c%c ROM 0x%02x\n",
> part_id, cmd.args[1], cmd.args[3], cmd.args[4], rom_id);
>
Em Sat, 9 Apr 2022 17:31:38 +0200
Mauro Carvalho Chehab <[email protected]> escreveu:
> Em Sat, 9 Apr 2022 14:53:52 +0200
> Thorsten Leemhuis <[email protected]> escreveu:
>
> > Hi, this is your Linux kernel regression tracker. Top-posting for once,
> > to make this easily accessible to everyone.
> >
> > Mauro, what's the hold-up here? Below patch is fixing a regression
> > caused by one of your patches. Why haven't you applied this or provided
> > feedback to Piotr what needs to be improved? Or am I missing something
> > and progress was made?
>
> We're currently without a sub-maintainer for dvb, so it takes a longer
> time for patches like that to be noticed.
>
> In the specific case of this patch, the problem is that it doesn't apply...
> The emailer mangled the patch, replacing spaces by U+0xa0 characters.
>
> Piotr,
>
> Please re-submit them with git send-email (or use some other client
> that won't damage whitespaces.
Nevermind. as the patch is short, I fixed it this time. Next time, please
check please use an emailer that is capable of properly encoding whitespaces
and tabs.
Regards,
Mauro
>
> Regards,
> Mauro
>
> >
> > This really is overdue, the first version of this patch was already
> > posted two and a half weeks ago. Reminder, the newly added document on
> > handling regressions (
> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/handling-regressions.rst#n132
> > ) for this case states this as rule of thumb: "Aim to fix regressions
> > within one week ".
> >
> > Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
> >
> > P.S.: As the Linux kernel's regression tracker I'm getting a lot of
> > reports on my table. I can only look briefly into most of them and lack
> > knowledge about most of the areas they concern. I thus unfortunately
> > will sometimes get things wrong or miss something important. I hope
> > that's not the case here; if you think it is, don't hesitate to tell me
> > in a public reply, it's in everyone's interest to set the public record
> > straight.
> >
> >
> > #regzbot poke
> >
> >
> > On 31.03.22 17:55, Piotr Chmura wrote:
> > > Fix firmware file names assignment in si2157 tuner, allow for running
> > > devices without firmware files needed.
> > >
> > >
> > > It's regression in kernel 5.17.0, worked fine in 5.16 series.
> > >
> > > device: 07ca:1871 AVerMedia Technologies, Inc. TD310 DVB-T/T2/C dongle
> > > modprobe gives error: unknown chip version Si2147-A30 ROM 0x50
> > > Device initialization is interrupted.
> > >
> > > caused by:
> > > 1. table si2157_tuners has swapped fields rom_id and required vs struct
> > > si2157_tuner_info.
> > > 2. both firmware file names can be null for devices with required ==
> > > false - device uses build-in firmware in this case
> > >
> > > Fix:
> > > 1. Rearrange fields in table si2157_tuners
> > > 2. Allow both firmware file names be NULL for devices defined with
> > > required == false
> > >
> > >
> > > Fixes: 1c35ba3bf972 ("media: si2157: use a different namespace for
> > > firmware")
> > > Link: https://bugzilla.kernel.org/show_bug.cgi?id=215726
> > > Link:
> > > https://lore.kernel.org/lkml/[email protected]/
> > >
> > > Cc: [email protected] # 5.17.x
> > > Signed-off-by: Piotr Chmura <[email protected]>
> > > Tested-by: Robert Schlabbach <[email protected]>
> > >
> > > ---
> > >
> > > --- a/drivers/media/tuners/si2157.c 2022-03-20 21:14:17.000000000 +0100
> > > +++ b/drivers/media/tuners/si2157.c 2022-03-22 23:48:05.604408331 +0100
> > > @@ -77,16 +77,16 @@ err_mutex_unlock:
> > > }
> > >
> > > static const struct si2157_tuner_info si2157_tuners[] = {
> > > - { SI2141, false, 0x60, SI2141_60_FIRMWARE, SI2141_A10_FIRMWARE },
> > > - { SI2141, false, 0x61, SI2141_61_FIRMWARE, SI2141_A10_FIRMWARE },
> > > - { SI2146, false, 0x11, SI2146_11_FIRMWARE, NULL },
> > > - { SI2147, false, 0x50, SI2147_50_FIRMWARE, NULL },
> > > - { SI2148, true, 0x32, SI2148_32_FIRMWARE, SI2158_A20_FIRMWARE },
> > > - { SI2148, true, 0x33, SI2148_33_FIRMWARE, SI2158_A20_FIRMWARE },
> > > - { SI2157, false, 0x50, SI2157_50_FIRMWARE, SI2157_A30_FIRMWARE },
> > > - { SI2158, false, 0x50, SI2158_50_FIRMWARE, SI2158_A20_FIRMWARE },
> > > - { SI2158, false, 0x51, SI2158_51_FIRMWARE, SI2158_A20_FIRMWARE },
> > > - { SI2177, false, 0x50, SI2177_50_FIRMWARE, SI2157_A30_FIRMWARE },
> > > + { SI2141, 0x60, false, SI2141_60_FIRMWARE, SI2141_A10_FIRMWARE },
> > > + { SI2141, 0x61, false, SI2141_61_FIRMWARE, SI2141_A10_FIRMWARE },
> > > + { SI2146, 0x11, false, SI2146_11_FIRMWARE, NULL },
> > > + { SI2147, 0x50, false, SI2147_50_FIRMWARE, NULL },
> > > + { SI2148, 0x32, true, SI2148_32_FIRMWARE, SI2158_A20_FIRMWARE },
> > > + { SI2148, 0x33, true, SI2148_33_FIRMWARE, SI2158_A20_FIRMWARE },
> > > + { SI2157, 0x50, false, SI2157_50_FIRMWARE, SI2157_A30_FIRMWARE },
> > > + { SI2158, 0x50, false, SI2158_50_FIRMWARE, SI2158_A20_FIRMWARE },
> > > + { SI2158, 0x51, false, SI2158_51_FIRMWARE, SI2158_A20_FIRMWARE },
> > > + { SI2177, 0x50, false, SI2177_50_FIRMWARE, SI2157_A30_FIRMWARE },
> > > };
> > >
> > > static int si2157_load_firmware(struct dvb_frontend *fe,
> > > @@ -178,7 +178,7 @@ static int si2157_find_and_load_firmware
> > > }
> > > }
> > >
> > > - if (!fw_name && !fw_alt_name) {
> > > + if (required && !fw_name && !fw_alt_name) {
> > > dev_err(&client->dev,
> > > "unknown chip version Si21%d-%c%c%c ROM 0x%02x\n",
> > > part_id, cmd.args[1], cmd.args[3], cmd.args[4], rom_id);
> > >
> >
>
>
>
> Thanks,
> Mauro
Thanks,
Mauro
Em Sat, 9 Apr 2022 14:53:52 +0200
Thorsten Leemhuis <[email protected]> escreveu:
> Hi, this is your Linux kernel regression tracker. Top-posting for once,
> to make this easily accessible to everyone.
>
> Mauro, what's the hold-up here? Below patch is fixing a regression
> caused by one of your patches. Why haven't you applied this or provided
> feedback to Piotr what needs to be improved? Or am I missing something
> and progress was made?
We're currently without a sub-maintainer for dvb, so it takes a longer
time for patches like that to be noticed.
In the specific case of this patch, the problem is that it doesn't apply...
The emailer mangled the patch, replacing spaces by U+0xa0 characters.
Piotr,
Please re-submit them with git send-email (or use some other client
that won't damage whitespaces.
Regards,
Mauro
>
> This really is overdue, the first version of this patch was already
> posted two and a half weeks ago. Reminder, the newly added document on
> handling regressions (
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/handling-regressions.rst#n132
> ) for this case states this as rule of thumb: "Aim to fix regressions
> within one week ".
>
> Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
>
> P.S.: As the Linux kernel's regression tracker I'm getting a lot of
> reports on my table. I can only look briefly into most of them and lack
> knowledge about most of the areas they concern. I thus unfortunately
> will sometimes get things wrong or miss something important. I hope
> that's not the case here; if you think it is, don't hesitate to tell me
> in a public reply, it's in everyone's interest to set the public record
> straight.
>
>
> #regzbot poke
>
>
> On 31.03.22 17:55, Piotr Chmura wrote:
> > Fix firmware file names assignment in si2157 tuner, allow for running
> > devices without firmware files needed.
> >
> >
> > It's regression in kernel 5.17.0, worked fine in 5.16 series.
> >
> > device: 07ca:1871 AVerMedia Technologies, Inc. TD310 DVB-T/T2/C dongle
> > modprobe gives error: unknown chip version Si2147-A30 ROM 0x50
> > Device initialization is interrupted.
> >
> > caused by:
> > 1. table si2157_tuners has swapped fields rom_id and required vs struct
> > si2157_tuner_info.
> > 2. both firmware file names can be null for devices with required ==
> > false - device uses build-in firmware in this case
> >
> > Fix:
> > 1. Rearrange fields in table si2157_tuners
> > 2. Allow both firmware file names be NULL for devices defined with
> > required == false
> >
> >
> > Fixes: 1c35ba3bf972 ("media: si2157: use a different namespace for
> > firmware")
> > Link: https://bugzilla.kernel.org/show_bug.cgi?id=215726
> > Link:
> > https://lore.kernel.org/lkml/[email protected]/
> >
> > Cc: [email protected] # 5.17.x
> > Signed-off-by: Piotr Chmura <[email protected]>
> > Tested-by: Robert Schlabbach <[email protected]>
> >
> > ---
> >
> > --- a/drivers/media/tuners/si2157.c 2022-03-20 21:14:17.000000000 +0100
> > +++ b/drivers/media/tuners/si2157.c 2022-03-22 23:48:05.604408331 +0100
> > @@ -77,16 +77,16 @@ err_mutex_unlock:
> > }
> >
> > static const struct si2157_tuner_info si2157_tuners[] = {
> > - { SI2141, false, 0x60, SI2141_60_FIRMWARE, SI2141_A10_FIRMWARE },
> > - { SI2141, false, 0x61, SI2141_61_FIRMWARE, SI2141_A10_FIRMWARE },
> > - { SI2146, false, 0x11, SI2146_11_FIRMWARE, NULL },
> > - { SI2147, false, 0x50, SI2147_50_FIRMWARE, NULL },
> > - { SI2148, true, 0x32, SI2148_32_FIRMWARE, SI2158_A20_FIRMWARE },
> > - { SI2148, true, 0x33, SI2148_33_FIRMWARE, SI2158_A20_FIRMWARE },
> > - { SI2157, false, 0x50, SI2157_50_FIRMWARE, SI2157_A30_FIRMWARE },
> > - { SI2158, false, 0x50, SI2158_50_FIRMWARE, SI2158_A20_FIRMWARE },
> > - { SI2158, false, 0x51, SI2158_51_FIRMWARE, SI2158_A20_FIRMWARE },
> > - { SI2177, false, 0x50, SI2177_50_FIRMWARE, SI2157_A30_FIRMWARE },
> > + { SI2141, 0x60, false, SI2141_60_FIRMWARE, SI2141_A10_FIRMWARE },
> > + { SI2141, 0x61, false, SI2141_61_FIRMWARE, SI2141_A10_FIRMWARE },
> > + { SI2146, 0x11, false, SI2146_11_FIRMWARE, NULL },
> > + { SI2147, 0x50, false, SI2147_50_FIRMWARE, NULL },
> > + { SI2148, 0x32, true, SI2148_32_FIRMWARE, SI2158_A20_FIRMWARE },
> > + { SI2148, 0x33, true, SI2148_33_FIRMWARE, SI2158_A20_FIRMWARE },
> > + { SI2157, 0x50, false, SI2157_50_FIRMWARE, SI2157_A30_FIRMWARE },
> > + { SI2158, 0x50, false, SI2158_50_FIRMWARE, SI2158_A20_FIRMWARE },
> > + { SI2158, 0x51, false, SI2158_51_FIRMWARE, SI2158_A20_FIRMWARE },
> > + { SI2177, 0x50, false, SI2177_50_FIRMWARE, SI2157_A30_FIRMWARE },
> > };
> >
> > static int si2157_load_firmware(struct dvb_frontend *fe,
> > @@ -178,7 +178,7 @@ static int si2157_find_and_load_firmware
> > }
> > }
> >
> > - if (!fw_name && !fw_alt_name) {
> > + if (required && !fw_name && !fw_alt_name) {
> > dev_err(&client->dev,
> > "unknown chip version Si21%d-%c%c%c ROM 0x%02x\n",
> > part_id, cmd.args[1], cmd.args[3], cmd.args[4], rom_id);
> >
>
Thanks,
Mauro