Re: em28xx + ov2640 and v4l2-clk

["Frank Schäfer" <fschaefer.oss@googlemail.com>]

[snip]

Am 12.10.2013 05:45, schrieb Mauro Carvalho Chehab:
>
> Changing the input will likely power on the device. The design of the
> old suspend callback were to call it when the device is not being used.
> Any try to use the device makes it to wake up, as it makes no sense to
> use a device in standby state.
>
> Also, changing the power states is a requirement, when switching the
> mode between analog, digital TV (or capture without tuner - although I
> think em28xx will turn the analog tuner on in this case, even not being
> required).
>
> The patches that just rename the previous standby callback to s_power 
> callback did a crap job, as it didn't consider the nuances of the API
> used on that time nor they didn't change the drivers to move the GPIO
> bits into s_power().
>
> Looking with today's view, it would likely be better if those patches
> were just adding a power callback without touching the standby callback.

The main problem is, that since commit 622b828ab7 ("v4l2_subdev: rename
tuner s_standby operation to core s_power") all subdevices are powered
down, not only the tuners.
But other subdevices may not wake up automatically when needed, so this
commit should also have introduced (s_power, 1) calls.
At least for em28xx it seems that this din't cause any regressions up to
now, because none of the subdevs used by this driver did anything
essential in its s_power callbacks (most of them don't support it at all).
But it's of course a ticking bomb.
The introduction of v4l2-clk enabling/disabling in the sensor subdevs'
(soc_cameras') s_power callbacks now let this bomb in em28xx explode.
This time, it only caused scary warnings/traces, but it could be
non-working (never waking up) devices next time.

> I suspect that the solution would be to fork s_power into two different
> callbacks: one asymetric to just put the device into suspend mode (as
> before), and another symmetric one, where the device needs to be explicitly
> enabled before its usage and disabled at suspend or driver exit.
>

Or, if we want to keep the API as is, we have to introduce (s_power, 1)
calls in the affected drivers to avoid regressions due to future subdev
changes.

Regards,
Frank