Hi Lewis, Did you have a chance to check if this fixes your race condition? Does the SPI work reliably now?
Miklos On Thu, Dec 6, 2012 at 12:59 PM, Miklos Maroti <mmar...@math.u-szeged.hu> wrote: > Excellent. Thanks! Miklos > > On Thu, Dec 6, 2012 at 12:38 PM, Oldrine Lewis <ole...@sutron.com> wrote: >> Hi Miklos, >> Thanks for the updates. I just downloaded the new files. Will test and keep >> you posted. >> Thanks, >> Lewis >> >> >> -----Original Message----- >> From: mmar...@gmail.com [mailto:mmar...@gmail.com] On Behalf Of Miklos Maroti >> Sent: Wednesday, December 05, 2012 1:35 PM >> To: Eric Decker >> Cc: Oldrine Lewis; tinyos-help@millennium.berkeley.edu; Philip Levis; Janos >> Sallai >> Subject: Re: [Tinyos-help] IRIS SPI lockup >> >> Dear Lewis, >> >> Thanks for finding this bug. Just to clarify, this happens ONLY if the >> Resource.immediateRequest is called from a task and then from an >> interrupt AND they use the SAME client id. Otherwise, the second call >> will return with fail, no? We have discussed this with Janos Sallai, >> and he says that the best option would be to initialize the SPI with >> the ResourceConfigure interface as Eric has also suggested. >> >> I have put together a patch that fixes this (includes some typo fixes >> in tep108): >> >> https://github.com/mmaroti/tinyos/commit/a45de584bd79c0be2853d011c129dc4cb4b6976c >> >> Lewis, can you please try this out if this works for you? (It actually >> reduces the code too) >> >> Best, >> Miklos >> >> On Sat, Dec 1, 2012 at 2:28 PM, Eric Decker <cire...@gmail.com> wrote: >>> >>> >>> On Fri, Nov 30, 2012 at 11:13 PM, Oldrine Lewis <ole...@sutron.com> wrote: >>>> >>>> Atm128SpiP.nc seems to make separate calls ( sequentially) to the >>>> ResourceArbiter and calls for initializing the SPI. >>>> >>>> In case of Resource.request (), startSpi() is called before the call to >>>> ResourceArbiter.request(), This ensures that the SPI will always be >>>> initialized before Resource.granted() is signaled. >>> >>> >>> The problem with Atm128P is it doesn't use the configurator call out from >>> the Arbiter. The whole point behind the configurator call out is to avoid >>> this timing window when h/w is configured. >>> >>> The reason that the Atm128P use of request/granted "works" is because it >>> cheats. It looks to see if someother entity is using the device and if not >>> it starts the SPI. Then it calls ResourceArbiter.request. Which means >>> that it is touching the resource without really owning it. Not exactly >>> proper. >>> >>> Request/Granted should also use the configuration interface. That is why >>> it is there. It lets the Arbiter configure the hardware properly according >>> to what the driver wants prior to switching the resource state to other than >>> RES_CONTROLLED which let's interrupts in. >>> >>> >>> >>>> Incase of Resource.immediateRequest(), the resource is first locked and >>>> only if the lock succeeds, is the SPI initialized. This sometime leads to >>>> an >>>> uninitialized SPI being used >>> >>> >>> Thinking about this as locking the resource is kind of sideways. It isn't >>> like we are asking for a semaphore protecting the device and then if we get >>> proceeding. Rather immediateRequest is responsible for checking to see if >>> the device (Resource) and be assigned and if so it will call the >>> configurator that should be wired in. If the device isn't available the >>> configurator isn't called and immediateResource return FAIL. >>> >>> >>> Agreed. That is because Atm128SpiP does the startSPI after the return from >>> the ArbriterResource.immediateRequest. That's broken and is what is >>> causing your problem. >>> >>> ArbiterResource.immediateRequest (in tos/system/ArbiterP.nc) does a callout >>> to ResourceConfigure.configure. This is done prior to setting the state to >>> other than RES_CONTROLLED. It is the switch to other than RES_CONTROLLED >>> that lets interrupts get to the device driver. >>> >>> Atm128SpiP when making use of immediateRequest, should also have wired in a >>> ResourceConfigure wiring which should contain the spiStart. >>> >>> In all cases, either after a granted or on return from immediateRequest, >>> interrupt should be handled correctly. It is assumed that the driver is >>> prepared. In the case of Atm128SpiP that isn't the case because it hasn't >>> wired in the correct configurator. Calling startSpi after the call to >>> immediateRequest introduces a timing window and as you found out you are >>> catching it. >>> >>> >>> My C$0.02 >>> >>> eric >>> >>> >>>> >>>> >>>> From: Eric Decker [mailto:cire...@gmail.com] >>>> Sent: Friday, November 30, 2012 11:38 PM >>>> To: Oldrine Lewis >>>> Cc: tinyos-help@millennium.berkeley.edu >>>> Subject: Re: [Tinyos-help] IRIS SPI lockup >>>> >>>> >>>> >>>> >>>> >>>> I thought the ResourceArbiter.immediateRequest symantic was that the >>>> resource in question should be fully initilized following return from the >>>> call. This should follow the same symantic as a ResourceArbiter.Request >>>> and Granted. Prior to interrupts being allowed and the Granted being >>>> signalled the resource is set up. To be consistent, >>>> ResourceArbiter.immediateRequest should follow the same symantic. >>>> >>>> >>>> >>>> >>>> >>>> If that premise is true. I would assert that the Atm128SpiP >>>> implementation has a bug and should get fixed. >>>> >>>> >>>> >>>> That would mean that the following as submitted is a work around and not >>>> an actual fix. >>>> >>>> >>>> >>>> >>>> >>>> thoughts? >>>> >>>> On Fri, Nov 30, 2012 at 12:58 PM, Oldrine Lewis <ole...@sutron.com> wrote: >>>> >>>> Recently, I have been experiencing frequent lockups with the IRIS nodes. I >>>> discovered that the lockup occurred in Atm128SpiP.nc, looping infinitely on >>>> while ( !( SPSR & 0x80 ) ); >>>> >>>> >>>> >>>> This bug, manifests if a task calls Resource.immediateRequest(), and an >>>> interrupt occurs after the task calls ResourceArbiter.immediateRequest[ id >>>> ]() , but before calling startSpi(). At this moment, the task already has >>>> the handle for the resource and hence the ISR can also access the resource. >>>> >>>> If the interrupt tries to use the SPI, since it has locked the resource it >>>> will go ahead and use the SPI, causing a lockup >>>> >>>> I recommend encapsulating the call to immediateRequest and start SPI in a >>>> critical section. I have commented my observations below >>>> >>>> >>>> >>>> async command error_t Resource.immediateRequest[ uint8_t id ]() >>>> >>>> { >>>> >>>> atomic >>>> >>>> { >>>> >>>> error_t result = call ResourceArbiter.immediateRequest[ id >>>> ](); >>>> >>>> //what if i get an interrupt here and the interrupt >>>> wants to use the SPI >>>> >>>> //the resource is already allocated so it will return >>>> TRUE. But the spi might be turned off >>>> >>>> //this will cause the SPI driver to lockup waiting at >>>> while( !( SPSR & 0x80 ) ) ; >>>> >>>> if ( result == SUCCESS ) >>>> >>>> { >>>> >>>> startSpi(); >>>> >>>> } >>>> >>>> return result; >>>> >>>> } >>>> >>>> } >>>> >>>> >>>> >>>> Please advice >>>> >>>> >>>> >>>> Thanks, >>>> >>>> Lewis >>>> >>>> >>>> >>>> >>>> _______________________________________________ >>>> Tinyos-help mailing list >>>> Tinyos-help@millennium.berkeley.edu >>>> https://www.millennium.berkeley.edu/cgi-bin/mailman/listinfo/tinyos-help >>>> >>>> >>>> >>>> >>>> >>>> -- >>>> Eric B. Decker >>>> Senior (over 50 :-) Researcher >>>> >>> >>> >>> >>> -- >>> Eric B. Decker >>> Senior (over 50 :-) Researcher >>> >>> _______________________________________________ Tinyos-help mailing list Tinyos-help@millennium.berkeley.edu https://www.millennium.berkeley.edu/cgi-bin/mailman/listinfo/tinyos-help