On Wed, 2011-05-25 at 11:34 -0700, Kevin Hilman wrote:
> Tomi Valkeinen <tomi.valkeinen@xxxxxx> 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
--
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
