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

Reply via email to