Re: [PATCH v2] of: Add videomode helper

[Guennadi Liakhovetski <g.liakhovetski@gmx.de>]

On Wed, 11 Jul 2012, Sascha Hauer wrote:

> Hi Guennadi,
> 
> On Wed, Jul 11, 2012 at 10:34:54AM +0200, Guennadi Liakhovetski wrote:
> > Hi Sascha
> > 
> > > +
> > > +Optional properties:
> > > + - width-mm, height-mm: Display dimensions in mm
> > > + - hsync-active-high (bool): Hsync pulse is active high
> > > + - vsync-active-high (bool): Vsync pulse is active high
> > 
> > How about
> > 
> > + - hsync-active: Hsync pulse polarity: 1 for high, 0 for low
> 
> I am unsure if it's good to mix this with the other bool flags like:
> 
> > 
> > > + - interlaced (bool): This is an interlaced mode
> > > + - doublescan (bool): This is a doublescan mode
> 
> Which behave differently. 'interlaced' will be evaluated as true if
> the property is present, no matter which value it has. This might
> lead to confusion.

I don't feel strongly either way either. I don't think it'd be confusing - 
you have integer properties there too, and the logic in this case is not 
"active high - yes or now," but rather "active level - logical 1 (high) or 
0 (low)." But as I said - that was just an idea, unless someone has strong 
arguments either way - you're the original author, it's your call:-)

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/