Skip to content

media: staging: imx: mxc_md: defer if no sensor firmware loaded yet#47

Open
RossPorter506 wants to merge 1 commit into
nxp-imx:lf-6.18.yfrom
RossPorter506:lf-6.18.y
Open

media: staging: imx: mxc_md: defer if no sensor firmware loaded yet#47
RossPorter506 wants to merge 1 commit into
nxp-imx:lf-6.18.yfrom
RossPorter506:lf-6.18.y

Conversation

@RossPorter506

Copy link
Copy Markdown

Issue

imx8-media-dev (aka mx8-img-md), expects all camera sensor firmware to be loaded before it is. Due to licensing issues there are some camera sensor modules that may have to be loaded late, after EULA, such as the AP1302. In this case mx8-img-md tears down /dev/video0 but reports a successful load:

[   12.839757] dwc-mipi-csi2-host 4ae00000.csi: lanes: 2, name: mxc-mipi-csi2.0
[   13.393563] mxc-isi_v1 4ae40000.isi: mxc_isi.0 registered successfully
[   13.790254] mx8-img-md: Registered mxc_isi.0.capture as /dev/video0
[   13.790656] unregister ISI channel: mxc_isi.0 # Immediately unregisters since AP1302 not loaded yet
...
[   17.923132] ap1302_mipi 2-003c: AP1302 Chip ID is 0x265
[   17.924401] ap1302_mipi 2-003c: AP1302 is found
[   20.011877] ap1302_mipi 2-003c: Load firmware successfully.
# Nothing tells mx8-img-md/mxc_isi to try again after AP1302 ready, because it's already 'succeeded'

(As a side note, notice that /dev/video0 briefly appears before checking if the camera stack is ready. Hold onto this for now.)

In this case the camera stack hasn't been fully configured but the driver reports success. This has knock-on effects in other parts of the system, such as causing a failure to resume after suspend.

b8939fc attempts a band-aid fix by extending the tear-down behaviour in this failure mode so the module could be reloaded by the user later. This of course requires user input and still leaves the camera stack half configured in the meantime.

Fix

We can instead ask for the driver to be deferred if the sensor firmware isn't loaded yet by changing the return value to -EPROBE_DEFER. We also need to do a more thorough cleanup since the driver code will be called again later.

Deferring the driver means that when another relevant kernel module finishes loading, the driver will attempt to load again. Eventually either the camera firmware loads, which triggers mx8-img-md to be loaded and the camera stack comes up properly, or no camera firmware ever loads (e.g. because no camera is connected), in which case mx8-img-md will sit unloaded on the list of deferred kernel modules, waiting for any kernel module changes. Because we more thoroughly clean up after ourselves before deferring we avoid a partially configured camera stack, and by extension we avoid the resume-after-suspend hang.

Regression risks

This branch is only taken if sensor firmware isn't built-in to the kernel. NXP tends to build all sensor firmware into the kernel, rather than making them loadable, so the risk of breaking existing functionality in the NXP fork is low.

Because of the way the NXP driver stack is currently architected, /dev/video0 briefly comes up before it's actually ready (as evidenced in the above logs). While this hasn't caused any problems (that we know of) so far, I could imagine some race-condition style issue occurring where something starts listening to /dev/video0 in the brief period where the camera stack registers it before it's ready.
This patch doesn't affect this behaviour, but in this late-loading sensor case it can cause the driver to attempt to load a few times, which provides more chances for such a race condition to occur. I don't expect this to be a big problem, since almost all kernel modules are loaded before user login, so there aren't many programs around that could be tricked by /dev/video0 briefly appearing before it's ready.

Previously if no sensor firmware had loaded yet the module would give up (but report success!), producing a partially configured camera stack that can cause a hang when resuming from suspend.
Instead, defer loading of the module until sensor firmware has loaded.

Signed-off-by: Ross Porter <ross.porter@canonical.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant