Re: context_loss_count error value

[Kevin Hilman <khilman@ti.com>]

Tomi Valkeinen <tomi.valkeinen@ti.com> writes:

> On Wed, 2011-05-25 at 13:30 -0700, Kevin Hilman wrote:
>> Tomi Valkeinen <tomi.valkeinen@ti.com> writes:
>> 
>> > On Wed, 2011-05-25 at 11:34 -0700, Kevin Hilman wrote:
>> >> Tomi Valkeinen <tomi.valkeinen@ti.com> writes:
>
> <snip>
>
>> >> 
>> >> > +		if (off_mode_enabled) {
>> >> > +			count = (count + 1) & 0x7fffffff;
>> >> > +			dummy_context_loss_counter = count;
>> >> > +		}
>> >> 
>> >> Again, I don't think this masking is needed.   count is already an
>> >> 'int', so when it gets bigger than INT_MAX, it will wrap.
>> >
>> > When count is INT_MAX and one is added to it, it'll wrap to INT_MIN,
>> > i.e. maximum negative value, which would be an error value. So by
>> > masking out the highest bit we'll get nonnegative count range from 0 to
>> > INT_MAX.
>> >
>> > Perhaps a comment would be justified here =).
>> 
>> Indeed, and using INT_MAX instead of the hard-coded constants would help
>> readability also.
>
> It may be just me, but as I see it, INT_MAX is a number like any other,
> and using it as a mask feels confusing to me.
>
> Would this be ok to you:
>
> /*
>  * Context loss count has to be a non-negative value. Clear the sign
>  * bit to get a value range from 0 to INT_MAX.
>  */
> count &= ~(1 << 31);
>

Yes.