[Xenomai] analogy: sensory 626 patch [Re: Xenomai Digest, Vol 27, Issue 12]

[Jorge Ramirez Ortiz <jro@xenomai.org>]

---------------------------------------------------------------------- Message: 1 
Date: Mon, 07 Jul 2014 12:38:42 +0200 From: Wojciech Domski <> To: Gilles 
Chanteperdrix <gilles.chanteperdrix@xenomai.org> Cc: xenomai@xenomai.org Subject: Re: 
[Xenomai] Sensoray 626 analogy driver Message-ID: <53BA78B2.2010802@gmail.com> 
Content-Type: text/plain; charset="utf-8"; Format="flowed" W dniu 11.04.2014 00:01, 
Gilles Chanteperdrix pisze:
>> On 04/10/2014 02:25 PM, Wojciech Domski wrote:
>>> 2014-03-25 13:33 GMT+01:00 Gilles Chanteperdrix <
>>> gilles.chanteperdrix@xenomai.org>:
>>>
>>>> On 03/25/2014 01:14 PM, Wojciech Domski wrote:
>>>>> W dniu 25.03.2014 12:58, Gilles Chanteperdrix pisze:
>>>>>> On 03/25/2014 12:54 PM, Wojciech Domski wrote:
>>>>>>> W dniu 25.03.2014 12:36, Gilles Chanteperdrix pisze:
>>>>>>>> On 03/25/2014 12:28 PM, Wojciech Domski wrote:
>>>>>>>>> W dniu 25.03.2014 12:16, Gilles Chanteperdrix pisze:
>>>>>>>>>> On 03/25/2014 11:26 AM, Wojciech Domski wrote:
>>>>>>>>>>> W dniu 24.03.2014 13:19, Gilles Chanteperdrix pisze:
>>>>>>>>>>>> The solution which would be acceptable is not to have busy waits,
>>>> except
>>>>>>>>>>>> for very short durations. But for instance transferring a byte on
>>>> I2C
>>>>>>>>>>>> takes around 20us at 400 kHz, a 20us masking section is
>>>> unacceptable.
>>>>>>>>>>>> rtdm_task_busy_sleep, as the name suggests is a busy wait loop,
>>>> so, no,
>>>>>>>>>>>> it is not acceptable either.
>>>>>>>>>>>>
>>>>>>>>>>>> Use a thread or a timer to reschedule while you wait for the end
>>>> of the
>>>>>>>>>>>> transfer, instead of busy waiting.
>>>>>>>>>>> Dear Gilles,
>>>>>>>>>>>
>>>>>>>>>>> As you mentioned before the driver has few places like this:
>>>>>>>>>>>
>>>>>>>>>>> while(!FLAG1);
>>>>>>>>>>> while(!FLAG2);
>>>>>>>>>>>
>>>>>>>>>>> Would a solution of creating a separate task for this purpose be
>>>> ok?
>>>>>>>>>>> task()
>>>>>>>>>>> {
>>>>>>>>>>>           while(!FLAG1);
>>>>>>>>>>>           while(!FLAG2);
>>>>>>>>>>> }
>>>>>>>>>> I was rather thinking about rtdm_task_sleep() instead.
>>>>>>>>>>
>>>>>>>>>>> In the place of those loops a piece of code responsible for
>>>> creating and
>>>>>>>>>>> joining task would be put instead:
>>>>>>>>>>>
>>>>>>>>>>> rtdm_task_init();
>>>>>>>>>>> rtdm_task_join_nrt();
>>>>>>>>>> This will not work for code currently running in interrupt
>>>> handlers. An
>>>>>>>>>> interrupt handler can not call rtdm_task_join_nrt(). You will rather
>>>>>>>>>> have to wake up the thread, not reenabling the interrupt at the end
>>>> of
>>>>>>>>>> the handler, and reenable the interrupt in the thread when the job
>>>> is
>>>>>>>>>> done. Or alternatively, fire a timer instead of waking up a thread.
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>> Dear Gilles,
>>>>>>>>>
>>>>>>>>> If what I have understood correctly, basically what you mean is
>>>> changing:
>>>>>>>>> while(!FLAG) ;
>>>>>>>>>
>>>>>>>>> into
>>>>>>>>>
>>>>>>>>> while(!FLAG)
>>>>>>>>>          rtdm_task_sleep();
>>>>>>>>>
>>>>>>>>> Xenomai's documentation that rtdm_task_sleep() can be always
>>>>>>>>> rescheduled. In my opinion
>>>>>>>>> this solution is sufficient and meets Xenomai requirements.
>>>>>>>> This solution will not work if the busy loop was in an irq handler.
>>>> You
>>>>>>>> can not call rtdm_task_sleep from the context of an irq handler.
>>>>>>>>
>>>>>>>>
>>>>>>> Ok, now I get your point.
>>>>>>>
>>>>>>> However, what do you mean exactly by rescheduling while waiting? Do you
>>>>>>> mean something like
>>>>>>> putting all irq services into another thread and only in irq handler
>>>>>>> notify the separate thread
>>>>>>> to do the job accordingly?
>>>>>> Well, I do not know exactly, I have not looked at the details of the
>>>>>> driver, you are supposed to do that. But you may be able to split the
>>>>>> interrupt code in two parts: a first part that will run in interrupt
>>>>>> mode, and a second part that will run as a thread. The irq handler would
>>>>>> wake the thread up at the end of the first part, and return without
>>>>>> reenabling the interrupt at interrupt controller level. The thread would
>>>>>> execute, sleep during the I2C transfers as needed, and reenable the
>>>>>> interrupt before going back to sleep.
>>>>>>
>>>>>>
>>>>> So would it be a good practise to create a new task for irq service with
>>>>>
>>>>> rtdm_task_init();
>>>>>
>>>>> inside attach function and join it with
>>>>>
>>>>> rtdm_task_join_nrt();
>>>>>
>>>>> inside detach function?
>>>> I do not know the details about analogy drivers, so, again, it is left
>>>> to you to read other analogy drivers and see where is the best place to
>>>> do what you want to do.
>>>>
>>>>
>>>> --
>>>>                                                                   Gilles.
>>>>
>>> Dear Gilles,
>>>
>>> I have altered the driver according to your advices.
>>> If I had omitted something please let me know.
>>>
>>> Please find a patch enclosed in the attachment.
>>>
>>> Best regards,
>>> Wojciech Domski
>>>
>>> Domski.pl
>>>
>>> Wojciech.Domski.pl
>>> -------------- next part --------------
>>> A non-text attachment was scrubbed...
>>> Name: s626_patch.patch
>>> Type: text/x-patch
>>> Size: 141659 bytes
>>> Desc: not available
>>> URL: <http://www.xenomai.org/pipermail/xenomai/attachments/20140410/e7191247/attachment.bin>
>> scripts/checkpatch.pl attchment.bin:
>>
>> total: 325 errors, 392 warnings, 3991 lines checked
>>
>> Please fix these errors.
>>
>>
> Dear Gilles,
>
> In the attachment you will find patch for xenomai including support for
> analogy Sensoray 626.
>
> May I ask you to include this patch to main xenomai tree?
>
> Best regards,
>
> Wojciech Domski
>
> Domski.pl
>
> Wojciech.Domski.pl
>


Hi Wojciech

I did some changes to the patch that you sent and pushed it to my Xenomai 3 tree 
(branch sensoray) as work in progress; however we have no means to test or validate 
any of the changes.
http://git.xenomai.org/xenomai-jro.git/log/?h=sensoray

is this something that you can do? ie, do you have access to the sensoray device?

In any case after reviewing the Comedi implementation in 3.12 staging I think it 
would make sense to base off that newer release instead.

jro



-- 
jro