Tomi Valkeinen <tomi.valkeinen@xxxxxx> writes:
> On Wed, 2011-05-25 at 13:30 -0700, Kevin Hilman wrote:
>> Tomi Valkeinen <tomi.valkeinen@xxxxxx> writes:
>>
>> > On Wed, 2011-05-25 at 11:34 -0700, Kevin Hilman wrote:
>> >> Tomi Valkeinen <tomi.valkeinen@xxxxxx> 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.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
