Re: [RFD] use-counting V4L2 clocks

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

Hi Guennadi and Mauro,

On Tuesday 08 October 2013 23:57:55 Guennadi Liakhovetski wrote:
> Hi Mauro,
> 
> Thanks for your long detailed mail. For the sake of brevity however I'll
> drop most of it in this my reply, everybody interested should be able to
> read the original.
> 
> On Wed, 9 Oct 2013, Mauro Carvalho Chehab wrote:
> 
> [snip]
> 
> > In other words, what you're actually proposing is to change the default
> > used by most drivers since 1997 from a POWER ON/CLOCK ON default, into a
> > POWER OFF/ CLOCK OFF default.
> 
> To remind, we are now trying to fix a problem, present in the current
> kernel. In one specific driver. And the proposed fix only affects one
> specific (family of) driver(s) - the em28xx USB driver. The two patches
> are quite simple:
> 
> (1) the first patch adds a clock to the em28xx driver, which only
> affects ov2640, because only it uses that clock
> 
> (2) the second patch adds a call to subdev's .s_power(1) method. And I
> cannot see how this change can be a problem either. Firstly I haven't
> found many subdevices, used by em28xx, that implement .s_power().
> Secondly, I don't think any of them does any kind of depth-counting in
> that method, apart from the one, that we're trying to fix - ov2640.
> 
> > Well, for me, it sounds that someone will need to re-test all supported
> > devices, to be sure that such change won't cause regressions.
> > 
> > If you are willing to do such tests (and to get all those hardware to be
> > sure that nothing will break) or to find someone to do it for you, I'm ok
> > with such change.
> 
> I'm willing to try to identify all subdevices, used by em28xx, look at
> their .s_power() methods and report my analysis, whether calling
> .s_power(1) for those respective drivers could cause problems. Would this
> suffice?

>From a high level point of view, I believe that's the way to go. V4L2 clock 
enable/disable calls must be balanced, as we will later switch to the non-V4L2 
clock API that requires calls to be balanced.

This pushes the problem back to the .s_power() implementation that call the 
clock enable/disable functions. As a temporary measure, we could add a use 
count to the .s_power() handlers of drivers used by both power-unbalanced and 
power-balanced bridges that call the clock API or the regulator API in their 
.s_power() implementation (that's just ov2640 if I'm not mistaken). This would 
ensure that clock calls are always balanced, even if the .s_power() calls are 
not.

Now I'd like to avoid that as possible: In the long term I believe we should 
switch all .s_power() calls to  balanced mode, a detailed analysis of the 
subdevices used by em28xx would thus have my preference. However, if it helps 
solving the issue right now, buying us time to fix the problem correctly, I 
could live with it.

> > Otherwise, we should stick with the present behavior, as otherwise we will
> > cause regressions.

-- 
Regards,

Laurent Pinchart