[Tinyos-devel] Msp430RefVoltArbiterImplP.nc switchOff loop

[Jan Hauer]

Hi Maxime,
yes, this is a bug. I think your solution is good, but in some edge
cases can lead to a race condition: specifically, if in
ClientResource.release() "owner = NO_OWNER" is executed, and then
another client's call to ClientResource.immediateRequest() happens
before switchOff() is executed. Then the "owner" variable is
overridden, switchOff() will do nothing and the generator remains on
although it should be off.

Instead I changed the "owner" variable to become a "syncOwner"
variable, which I only modify in sync (task) context and have the
async path "parallel" to it (it's hard to explain take a look at the
diff) - I've committed it to CVS and put my test app under
tinyos-2.x/apps/tests/msp430/Adc12.

Jan


On 4/5/07, Philip Levis <pal at cs.stanford.edu> wrote:
> On Apr 4, 2007, at 8:37 AM, Maxime Muller - s h o c k f i s h / wrote:
>
> > Hi all,
> >
> > On tos 2.x, file $TOSDIR/chips/msp430/adc12/
> > Msp430RefVoltArbiterImplP.nc
> > during event void AdcResource.granted[uint8_t client]()
> > If your sref is not REFERENCE_VREFplus_AVss ||
> > REFERENCE_VREFplus_VREFnegterm
> > then you will never call Refvolt_1/2_5V.start()
> >
> > Now while releasing the resource, imho, we should do the same check
> > else we end up with a post switchOff() loop
> > as call RefVolt_1_5V.stop() will fail.
> >
> > async command error_t ClientResource.release[uint8_t client]()
> >  {
> >      const msp430adc12_channel_config_t* settings  = call
> > Config.getConfiguration[client]();
> >      atomic {
> >      if (owner == client){                  owner = NO_OWNER;
> >          if (settings->sref == REFERENCE_VREFplus_AVss ||
> >          settings->sref == REFERENCE_VREFplus_VREFnegterm)
> >          post switchOff();
> >      }
> >      }
> >    return call AdcResource.release[client]();
> >  }
> > seems better or did i miss something?
>
> We discussed this in the core WG meeting today. Jan (who's
> responsible for the MSP430 ADC) is going to take a look. The short
> story is that you've definitely found a bug, but your solution has a
> race condition in it. Our hope is to fix this for the 2.0.1 release;
> Jan will hopefully be getting in touch with you about it.
>
> Phil
>