On 07/14/2014 05:07 PM, Jorge Ramirez Ortiz wrote:


On 07/14/2014 03:09 PM, Wojciech Domski wrote:
 W dniu 2014-07-14 16:18, Jorge Ramirez Ortiz pisze:
 ---------------------------------------------------------------------- 
Message: 1
 Date: Mon, 07 Jul 2014 12:38:42 +0200 From: Wojciech Domski < To: Gilles
 Chanteperdrix <gilles.chanteperd...@xenomai.org Cc: xenomai@xenomai.org 
Subject:
 Re: [Xenomai] Sensoray 626 analogy driver Message-ID:
 <53ba78b2.2010...@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.chanteperd...@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



 Thank you for committing the driver.
 I have access to the Sensoray 626 board and I will test the remarks you have
 proposed. It will take me sometime, however. First of all I would like to test 
my
 driver with two Sensoray boards. For now, I see that it won't work with more 
than
 one card. It requires some changes to the attach and probe function. After I
 implement this functionality I will take a look into the interrupt handling 
service.

 Seizing the opportunity, I would like to ask you a question about handling PCI
 device. When I'm calling pci_register_driver() and I have two exactly the same 
PCI
 devices will this function call appropriate probe() function two times? If yes,
 then is the probe() function an appropriate place to make list of available 
devices?

 Wojciech Domski

 Domski.pl

 Wojciech.Domski.pl


 invoking pci_register_driver in dev_s626_attach causes the probe interface in 
the
 pci driver to be executed for all the devices of the kind described in the pci 
id
 table; this is probably the reason why you can only register one.


of course, in case it was not clear, you can only register the pci driver once

 It might be easier to follow the approach in 
analogy/national_instruments/mite.c

 mite_init registers the driver which ends up calling mite_probe which will 
create a
 mite_struct and add it to the list of mite_devices.
 Incidentally if you were wondering, this is the reason why if the
 pci_register_driver call succeeded the driver can display all the devices in 
the
 system.

 the same would need to be done for the s626

 on a different note, we are adding a new interface to rtdm to support the busy 
loops.
 rtdm_busywait_safe(__condition, __busy_delay, __busy_wait)

 this should allow you to do things like this

 -       while (!MC_TEST(P_MC2, MC2_UPLD_DEBI))
 -               rtdm_task_sleep(2000);
 +       ret = rtdm_busywait_safe(!MC_TEST(P_MC2, MC2_UPLD_DEBI), BUSY_DELAY,
 BUSY_SLEEP);
 +       if (ret)
 +               __a4l_err( "error waiting");

>
>
>
>
>


--
jro


_______________________________________________
Xenomai mailing list
Xenomai@xenomai.org
http://www.xenomai.org/mailman/listinfo/xenomai

Reply via email to