On Mon, 23 Mar 2009, [email protected] wrote:

> Dear Guennadi
> 
> Thank you for checking
> 
> > Wouldn't it be easier to use
> > 
> >     unsigned char   edge_strength;
> >     unsigned char   edge_threshold;
> >     unsigned char   edge_low;
> >     unsigned char   edge_high;
> > 
> > and thus avoid all that shifting?
> 
> Yes. it is easier to use.
> But, driver should judge whether to use this setting or not.
> Because this setting is optional.
> 
> Because user setting might be 0,
> we can not judge it like this.
>   if (edge_xxx)
>        ov772x_mask_set( xxxx )
> 
> So, we can use un-used bit to judge whether to use it.
> and edge_strength and edge_threshold have un-used bit.
> But edge_low and edge_high doesn't have un-used bit.
> 
> Another way to judge it is to use pointer or to add another variable.
> But I don't like these style.
> What do you think about this ?

Is edge_strength == 0 a useful edge configuration? Cannot you use it as a 
test whether to set all edge parameters or not? If you cannot, well, just 
do the same as what you have done with 32-bits - use one bit in strength 
as "edge enable" - just exactly in the same way as in your patch. Like

        if (edge_strength & EDGE_ENABLE) {
                set_strength;
                set_threshold;
                set_low;
                set_high;
        }

Thanks
Guennadi
---
Guennadi Liakhovetski
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to