Re: [PATCH 1/3] mt9v022: add v4l2 controls for blanking and other register settings

[Detlev Zundel <dzu@denx.de>]

Hi Guennadi,

> Hi Detlev
>
> On Fri, 24 Aug 2012, Detlev Zundel wrote:
>
>> Hello Guennadi,
>> 
>> > Hi Anatolij
>> >
>> > On Fri, 24 Aug 2012, Anatolij Gustschin wrote:
>> >
>> >> Add controls for horizontal and vertical blanking, analog control
>> >> and control for undocumented register 32.
>> >
>> > Sorry, I don't think this is a good idea to export an undocumented 
>> > register as a control.
>> 
>> Why exactly is that?  Even though it is not documented, we need to
>> fiddle with it to make our application work at all.  So we tend to
>> believe that other users of the chip will want to use it also.
>
> Below I asked to provide details about how you have to change this 
> register value: toggle dynamically at run-time or just set once at 
> initialisation? Even if toggle: are this certain moments, related to 
> standard camera activities (e.g., starting and stopping streaming, 
> changing geometry etc.) or you have to set this absolutely asynchronously 
> at moments of time, that only your application knows about?

Anatolij can answer those detail questions, all I know is that without
fiddling with the register we do not receive valid pictures at all.

>> Furthermore I don't see that we fundamentally reject patches for other
>> parts in the Linux kernel where people found out things not in the
>> official datasheets.
>
> The problem is not, that this register is undocumented, the problem rather 
> is, that IMHO exporting an API to user-space, setting an undocumented 
> register to arbitrary values is, hm, at least pretty dubious.

As I wrote above, without fiddling with the register, we do _not_
receive correct pictures at all.  So this is not dubious but shown by
experiment to be needed (at least in our setup).

Best wishes
  Detlev

-- 
A language that doesn't affect the way you think about programming, is
not worth knowing.             -- Alan Perlis, Epigrams on Programming