Hi Guys, I have committed this SPI fix today (and did the same fix for the atmega128rfa1 chip).
Best, Miklos On Thu, Dec 13, 2012 at 9:12 AM, Miklos Maroti <mmar...@math.u-szeged.hu> wrote: > Ok, thanks! I really would like to know how it works in your situation > before I commit to the mainline. Miklos > > On Wed, Dec 12, 2012 at 11:36 PM, Oldrine Lewis <ole...@sutron.com> wrote: >> >> >> Hi Miklos, >> >> A few other changes I had made to my program structure, seem to almost >> eliminiate the possibility of hitting the SPI lock situation. I will have to >> rerun the older version, of my software to replicate the previous situation >> to test the fix. I have a release scheduled soon and will get to the test >> the fix asap. >> >> Regards, >> Lewis >> >> >> >> -----Original Message----- >> From: mmar...@gmail.com [mailto:mmar...@gmail.com] On Behalf Of Miklos Maroti >> Sent: Monday, December 10, 2012 2:55 PM >> To: Oldrine Lewis >> Cc: Eric Decker; tinyos-help@millennium.berkeley.edu; Philip Levis; Janos >> Sallai >> Subject: Re: [Tinyos-help] IRIS SPI lockup >> >> 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