[PATCH v8 1/7] media: V4L2: add temporary clock helpers

[Mike Turquette]

Quoting Laurent Pinchart (2013-04-11 15:35:38)
> Hi Mike,
> 
> On Thursday 11 April 2013 11:52:58 Mike Turquette wrote:
> > Quoting Barry Song (2013-04-11 01:59:28)
> > > 2013/4/11 Guennadi Liakhovetski <g.liakhovetski at gmx.de>:
> > > > On Thu, 11 Apr 2013, Barry Song wrote:
> > > >> 2013/4/11 Guennadi Liakhovetski <g.liakhovetski at gmx.de>:
> > > >> > On Thu, 11 Apr 2013, Barry Song wrote:
> > > >> >> Hi Guennadi,
> > > >> >> 
> > > >> >> > Typical video devices like camera sensors require an external
> > > >> >> > clock source. Many such devices cannot even access their hardware
> > > >> >> > registers without a running clock. These clock sources should be
> > > >> >> > controlled by their consumers. This should be performed, using the
> > > >> >> > generic clock framework. Unfortunately so far only very few
> > > >> >> > systems have been ported to that framework. This patch adds a set
> > > >> >> > of temporary helpers, mimicking the generic clock API, to V4L2.
> > > >> >> > Platforms, adopting the clock API, should switch to using it.
> > > >> >> > Eventually this temporary API should be removed.
> > > >> >> > 
> > > >> >> > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski at xxxxxx>
> > > >> >> > ---
> > > >> >> 
> > > >> >> for your patch 1/8 and 3/8, i think it makes a lot of senses to let
> > > >> >> the object manages its own clock by itself.
> > > >> >> is it possible for us to implement v4l2-clk.c directly as an
> > > >> >> instance of standard clk driver for those systems which don't have
> > > >> >> generic clock,  and remove the V4L2 clock APIs like v4l2_clk_get,
> > > >> >> v4l2_clk_enable from the first day? i mean v4l2-clk.c becomes a temp
> > > >> >> and fake clock controller driver. finally, after people have
> > > >> >> generically clk, remove it.
> > > >> > 
> > > >> > I don't think you can force-enable the CFF on systems, that don't
> > > >> > support it, e.g. PXA.
> > > >> 
> > > >> yes. we can. clock is only a framework, has it any limitation to
> > > >> implement a driver instance on any platform?
> > > > 
> > > > So, you enable CFF, it provides its own clk_* implementation like
> > > > clk_get_rate() etc. Now, PXA already has it defined in
> > > > arch/arm/mach-pxa/clock.c. Don't think this is going to fly.
> > > 
> > > agree.
> > 
> > I came into this thread late and don't have the actual patches in my inbox
> > for review.  That said, I don't understand why V4L2 cares about the clk
> > framework *implementation*?  The clk.h api is the same for platforms using
> > the common struct clk and those still using the legacy method of defining
> > their own struct clk.  If drivers are only consumers of the clk.h api then
> > the implementation underneath should not matter.
> 
> The issue on non-CCF systems is that devices usually can't register clocks 
> dynamically. (Most of) those systems provide system clocks only through their 
> clock API, without a way for the camera IP core to hook up the clock(s) it can 
> provide to the camera sensor. On the consumer side we don't care much about 
> the clock framework implementation, but on the provider side we need a 
> framework that allows registering non-system clocks at runtime.
> 

Yes, you do care about the clock framework implementation if you are a
clock provider.  I still haven't gone through the archives to find these
patches but I hope that any dependency on CONFIG_COMMON_CLK is
conditionalized to have the smallest impact possible.  Making v4l2 as a
whole depend on COMMON_CLK might be a bit overkill compared to just
making individual camera drivers depend on it.

Regards,
Mike

> -- 
> Regards,
> 
> Laurent Pinchart