From: Lukas Middendorf <kernel@tuxforce.de>
To: Mauro Carvalho Chehab <mchehab@kernel.org>
Cc: linux-media@vger.kernel.org, Antti Palosaari <crope@iki.fi>,
Luis Chamberlain <mcgrof@kernel.org>,
Lukas Middendorf <kernel@tuxforce.de>
Subject: Re: [PATCH 1/2] media: si2168: request caching of firmware to make it available on resume
Date: Sat, 10 Apr 2021 00:02:10 +0200 [thread overview]
Message-ID: <e6344feb-00ec-0955-67af-02f0331e8719@tuxforce.de> (raw)
In-Reply-To: <20210409132957.08d7c7bf@coco.lan>
On 09/04/2021 13:29, Mauro Carvalho Chehab wrote:
> Well, I fail to see why si2168 is so special that it would require it...
The special case here is that si2168 does (try to) load the firmware for
the first time during resume. Most other drivers that use firmware do it
for the first time at boot (or when connecting the device) and therefore
will automatically have their firmware cached for use on resume.
> 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.
Any other driver that might load the firmware for the first time during
resume also has to be fixed. On a quick glance it looks like the si2165
for example might have the same problem. I think that at least all dvb
frontends which load the firmware in init callback but not during probe
are problematic.
The possible patch with the usermode helper lock by Luis causes uncached
firmware loading on resume to fail very noisily instead of just stalling
the system. That would show up other non-conformant drivers. There
likely would be some more bug reports coming in from users which dislike
the backtraces coming up in dmesg. You will likely want to fix the
drivers before that happens.
The fact that this bug is only exposed now that btrfs is seeing more
wide spread adoption does not make it less of a bug.
> Btw, IMHO, the better would be to reload the firmware at resume
> time, instead of caching it, just like other media drivers.
Loading the firmware on resume without it being cached is exactly what
causes problems (see Luis' explanation). The caching is set up
implicitly if the normal request_firmware() is used before suspend. The
firmware does not stay in cache permanently. The firmware is just cached
by the firmware loader api during suspend and cleaned again at the end
of resume when proper file system access is possible again.
A really better solution would be to not load the firmware on resume in
case it has not been previously loaded to the device (or not load it at
all on resume since playback has to be restarted after suspend anyway).
But it seems like the same init callback of the si2168 driver is called
both at resume and when the device is being used and therefore does not
easily allow for this. Likely the dvb_frontend api would have to be
extended to have a separate callback for resume.
prev parent reply other threads:[~2021-04-09 22:02 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-08-13 21:45 [PATCH 1/2] media: si2168: request caching of firmware to make it available on resume Lukas Middendorf
2020-08-13 21:45 ` [PATCH 2/2] media: si2168: also cache Si2168 B40 fallback firmware Lukas Middendorf
2020-08-13 21:55 ` Luis Chamberlain
2021-04-01 14:46 ` Lukas Middendorf
2020-08-13 21:54 ` [PATCH 1/2] media: si2168: request caching of firmware to make it available on resume Luis Chamberlain
2021-04-01 14:42 ` Lukas Middendorf
2021-04-02 18:04 ` Luis Chamberlain
2021-04-09 11:29 ` Mauro Carvalho Chehab
2021-04-09 16:58 ` Luis Chamberlain
2021-04-09 22:02 ` Lukas Middendorf [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=e6344feb-00ec-0955-67af-02f0331e8719@tuxforce.de \
--to=kernel@tuxforce.de \
--cc=crope@iki.fi \
--cc=linux-media@vger.kernel.org \
--cc=mcgrof@kernel.org \
--cc=mchehab@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.