On Tue, May 01, 2012 at 21:07:39, Hunter, Jon wrote:
[...]
> > int pwrdm_read_pwrst(struct powerdomain *pwrdm)
> > {
> > - int ret = -EINVAL;
> > + int pwrst, ret = -EINVAL;
> >
> > if (!pwrdm)
> > return -EINVAL;
> >
> > - if (arch_pwrdm && arch_pwrdm->pwrdm_read_pwrst)
> > + if (!pwrdm_cache_read(pwrdm, PWRDM_CACHE_PWRST, &pwrst))
> > + return pwrst;
> > +
> > + if (arch_pwrdm && arch_pwrdm->pwrdm_read_pwrst) {
> > ret = arch_pwrdm->pwrdm_read_pwrst(pwrdm);
> > + if (ret >= 0)
> > + pwrdm_cache_write(pwrdm, PWRDM_CACHE_PWRST, ret);
> > + }
>
> Do we really want to use the cache for the current state? This is
> updated by hardware. In the above it appears that once we have read it
> once we will just return this value until the cache is invalidated.
> Makes me a little nervous.
>
I echo the concern here. If a powerdomain transition occurs when h/w conditions
are met, the cached values will go out of sync and this may lead to unexpected
behavior wherever this API is being used.
As Jon pointed out in another patch, the registers which can be updated by h/w should
be handled differently.
Just a wild thought, can PRCM_MPU_IRQ ([4]TRANSITION_EN) be used as a trigger to update
the cache if the transition happened under s/w control?
Regards,
Vaibhav
--
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
