Re: context_loss_count error value

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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


[Index of Archives]     [Linux Arm (vger)]     [ARM Kernel]     [ARM MSM]     [Linux Tegra]     [Linux WPAN Networking]     [Linux Wireless Networking]     [Maemo Users]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux