Re: [PATCH 1/2] media: si2168: request caching of firmware to make it available on resume

[Mauro Carvalho Chehab <mchehab@kernel.org>]

Em Thu, 1 Apr 2021 16:42:26 +0200
Lukas Middendorf <kernel@tuxforce.de> escreveu:

> Hi,
> 
> I see this (or a similar fix) has not yet been included in 5.12-rc5.
> Any further problems or comments regarding this patch? It still applies 
> cleanly to current git master and the problem is still relevant.

Well, I fail to see why si2168 is so special that it would require it...

on a quick check, it sounds that there's just a single driver using this
kAPI:

	drivers/net/wireless/mediatek/mt7601u/mcu.c:            return firmware_request_cache(dev->dev, MT7601U_FIRMWARE);

while there are several drivers on media that require firmware.

Btw, IMHO, the better would be to reload the firmware at resume
time, instead of caching it, just like other media drivers.



> 
> Best regards
> Lukas
> 
> On 13/08/2020 23:45, Lukas Middendorf wrote:
> > even though request_firmware() is supposed to be safe to call during
> > resume, it might fail (or even hang the system) when the firmware
> > has not been loaded previously. Use firmware_request_cache() to
> > have it cached so it is available reliably on resume.
> > 
> > Signed-off-by: Lukas Middendorf <kernel@tuxforce.de>
> > ---
> >   drivers/media/dvb-frontends/si2168.c | 11 +++++++++++
> >   1 file changed, 11 insertions(+)
> > 
> > diff --git a/drivers/media/dvb-frontends/si2168.c b/drivers/media/dvb-frontends/si2168.c
> > index 14b93a7d3358..ea4b2d91697e 100644
> > --- a/drivers/media/dvb-frontends/si2168.c
> > +++ b/drivers/media/dvb-frontends/si2168.c
> > @@ -757,6 +757,17 @@ static int si2168_probe(struct i2c_client *client,
> >   		 dev->version >> 24 & 0xff, dev->version >> 16 & 0xff,
> >   		 dev->version >> 8 & 0xff, dev->version >> 0 & 0xff);
> >   
> > +	/* request caching of the firmware so it is available on resume after suspend.
> > +	 * The actual caching of the firmware file only occurs during suspend
> > +	 * The return value does not show whether the firmware file exists
> > +	 */
> > +	ret = firmware_request_cache(&client->dev, dev->firmware_name);
> > +	if (ret) {
> > +		dev_err(&client->dev,
> > +				"firmware caching for '%s' failed\n",
> > +				dev->firmware_name);
> > +	}
> > +
> >   	return 0;
> >   err_kfree:
> >   	kfree(dev);
> >   



Thanks,
Mauro