Re: context_loss_count error value

[Tomi Valkeinen <tomi.valkeinen@ti.com>]

On Wed, 2011-05-25 at 11:34 -0700, Kevin Hilman wrote:
> Tomi Valkeinen <tomi.valkeinen@ti.com> writes:
> 
> [...]
> 
> >> 
> >> You're right, the code is just wrong here and would lead to strange
> >> return value checking in the callers to be correct.
> >> 
> >> I think the best fix for this problem is to use a signed return value
> >> which can wrap as expected, and then use return negative error codes
> >> (e.g. -ENODEV).
> >> 
> >> Care to send a patch?  or do you have any other suggestions for a fix?
> >
> > Here's a patch. 
> 
> Thanks!

<snip>

> > @@ -311,22 +311,26 @@ void omap_pm_disable_off_mode(void)
> >  
> >  #ifdef CONFIG_ARCH_OMAP2PLUS
> >  
> > -u32 omap_pm_get_dev_context_loss_count(struct device *dev)
> > +int omap_pm_get_dev_context_loss_count(struct device *dev)
> >  {
> >  	struct platform_device *pdev = to_platform_device(dev);
> > -	u32 count;
> > +	int count;
> >  
> >  	if (WARN_ON(!dev))
> > -		return 0;
> > +		return -ENODEV;
> >  
> >  	if (dev->parent == &omap_device_parent) {
> >  		count = omap_device_get_context_loss_count(pdev);
> >  	} else {
> >  		WARN_ONCE(off_mode_enabled, "omap_pm: using dummy context loss counter; device %s should be converted to omap_device",
> >  			  dev_name(dev));
> > -		if (off_mode_enabled)
> > -			dummy_context_loss_counter++;
> > +
> >  		count = dummy_context_loss_counter;
> > +
> > +		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 =).

 Tomi