[Xenomai-core] Analogy: idx_write_subd field in a4l_desc_t

[Xenomai-core] Analogy: idx_write_subd field in a4l_desc_t

[Daniele Nicolodi]

Hello,

I'm using xenomai-head on a 2.6.38.8 kernel on x86 with a NI-6251 DAQ
board. In this configuration the idx_write_subd field of the a4l_desc_t
structure filled by a4l_open() is not set to the proper value but is set
to NULL.

In previous xanomai/analogy releases this was working properly. Has some
initialization code been removed in the latest analogy drivers refactoring?

Thank you. Cheers,
-- 
Daniele

Re: [Xenomai-core] Analogy: idx_write_subd field in a4l_desc_t

[Alexis Berlemont]

Hi,

On Tue, Sep 27, 2011 at 4:06 PM, Daniele Nicolodi <daniele@domain.hid> wrote:
> Hello,
>
> I'm using xenomai-head on a 2.6.38.8 kernel on x86 with a NI-6251 DAQ
> board. In this configuration the idx_write_subd field of the a4l_desc_t
> structure filled by a4l_open() is not set to the proper value but is set
> to NULL.
>
> In previous xanomai/analogy releases this was working properly. Has some
> initialization code been removed in the latest analogy drivers refactoring?

Yes. Formerly, on both sides (kernel and user), we used some
description fields (idx_read_subd and idx_write_subd) so as to quickly
identify default asynchronous input and output subdevices and to link
them with buffers (into which, input / output data are copied).

Most of the time, there were only one asynchronous read / write
subdevice per card; so, the original Comedi way was ok.

Unfortunately, some boards now display many asynchronous subdevices of
the same type. To tackle this issue, I could have implemented the
workaround proposed by Comedi (give the possibility to attach the
driver a second time and select the proper subdevice). I tried
something else: when the driver is opened an asynchronous buffer is
instanciated; at init time, this buffer does not belong to any
subdevice but when an asynchronous command is sent, the buffer is
temporarily reserved by the targeted subdevice.

Thus instead of re-attaching drivers with devices, the developer gets
direct access on any subdevice; if he needs to perform two
simultaneous asyncrhonous acquisitions on the same card, he just has
to open the analogy device twice (a4l_open).

For API / ABI compatibility reasons, I waited a major release before
removing the fields idx_{read, write}_subd. I should have thought
twice before removing their initializations. I will fix that soon,
sorry.

I implemented this change more than one year ago (here is on of the
many related commits 58ebd5b7efe0cc0ac1e82991d12125ce34dfeee3). That
was already present in 2.5. Which version were you formerly using?

Best regards,

Alexis.

> Thank you. Cheers,
> --
> Daniele
>
> _______________________________________________
> Xenomai-core mailing list
> Xenomai-core@domain.hid
> https://mail.gna.org/listinfo/xenomai-core
>

Re: [Xenomai-core] Analogy: idx_write_subd field in a4l_desc_t

[Gilles Chanteperdrix]

On 10/01/2011 12:03 AM, Alexis Berlemont wrote:
> Hi,
> 
> On Tue, Sep 27, 2011 at 4:06 PM, Daniele Nicolodi <daniele@domain.hid> wrote:
>> Hello,
>>
>> I'm using xenomai-head on a 2.6.38.8 kernel on x86 with a NI-6251 DAQ
>> board. In this configuration the idx_write_subd field of the a4l_desc_t
>> structure filled by a4l_open() is not set to the proper value but is set
>> to NULL.
>>
>> In previous xanomai/analogy releases this was working properly. Has some
>> initialization code been removed in the latest analogy drivers refactoring?
> 
> Yes. Formerly, on both sides (kernel and user), we used some
> description fields (idx_read_subd and idx_write_subd) so as to quickly
> identify default asynchronous input and output subdevices and to link
> them with buffers (into which, input / output data are copied).
> 
> Most of the time, there were only one asynchronous read / write
> subdevice per card; so, the original Comedi way was ok.
> 
> Unfortunately, some boards now display many asynchronous subdevices of
> the same type. To tackle this issue, I could have implemented the
> workaround proposed by Comedi (give the possibility to attach the
> driver a second time and select the proper subdevice). I tried
> something else: when the driver is opened an asynchronous buffer is
> instanciated; at init time, this buffer does not belong to any
> subdevice but when an asynchronous command is sent, the buffer is
> temporarily reserved by the targeted subdevice.
> 
> Thus instead of re-attaching drivers with devices, the developer gets
> direct access on any subdevice; if he needs to perform two
> simultaneous asyncrhonous acquisitions on the same card, he just has
> to open the analogy device twice (a4l_open).
> 
> For API / ABI compatibility reasons, I waited a major release before
> removing the fields idx_{read, write}_subd. I should have thought
> twice before removing their initializations. I will fix that soon,
> sorry.

2.6 is a new major release, and not out yet, so, if you want to remove
something, it is still time.

-- 
                                                                Gilles.

Re: [Xenomai-core] Analogy: idx_write_subd field in a4l_desc_t

[Daniele Nicolodi]

On 01/10/11 20:07, Gilles Chanteperdrix wrote:
>> For API / ABI compatibility reasons, I waited a major release before
>> removing the fields idx_{read, write}_subd. I should have thought
>> twice before removing their initializations. I will fix that soon,
>> sorry.
> 
> 2.6 is a new major release, and not out yet, so, if you want to remove
> something, it is still time.

If we are open to some partially backward incompatible changes I propose
to also change the meaning of the a4l_desc_t board_name field, to really
be the board name, instead of the driver name. I find this information
more useful when enumerating devices in my setup.

I would also like to propose a patch to demote some messages, logged at
each interrupt, in the ni_pcimio driver from the info to the debug
level. I run continuous acquisitions and those kernel messages pollute
my syslog with not much added value and fill my disc with redundant
information.

Cheers,
-- 
Daniele

Re: [Xenomai-core] Analogy: idx_write_subd field in a4l_desc_t

[Gilles Chanteperdrix]

On 10/04/2011 09:58 AM, Daniele Nicolodi wrote:
> On 01/10/11 20:07, Gilles Chanteperdrix wrote:
>>> For API / ABI compatibility reasons, I waited a major release before
>>> removing the fields idx_{read, write}_subd. I should have thought
>>> twice before removing their initializations. I will fix that soon,
>>> sorry.
>>
>> 2.6 is a new major release, and not out yet, so, if you want to remove
>> something, it is still time.
> 
> If we are open to some partially backward incompatible changes I propose
> to also change the meaning of the a4l_desc_t board_name field, to really
> be the board name, instead of the driver name. I find this information
> more useful when enumerating devices in my setup.
> 
> I would also like to propose a patch to demote some messages, logged at
> each interrupt, in the ni_pcimio driver from the info to the debug
> level. I run continuous acquisitions and those kernel messages pollute
> my syslog with not much added value and fill my disc with redundant
> information.

If you want things merged, send the patches, they will reach
xenomai-head repository through Alex.

-- 
                                                                Gilles.

Re: [Xenomai-core] Analogy: idx_write_subd field in a4l_desc_t

[Daniele Nicolodi]

[-- Attachment #1: Type: text/plain, Size: 219 bytes --]

On 04/10/11 13:17, Gilles Chanteperdrix wrote:
> If you want things merged, send the patches, they will reach
> xenomai-head repository through Alex.

Of course. Here is my simple patch.

Thank you. Cheers,
-- 
Daniele

[-- Attachment #2: 0001-analogy-demote-some-messages-logged-in-mio_common-dr.patch --]
[-- Type: text/x-diff, Size: 3470 bytes --]

>From 6944fe7465e1eb614cf9db3eccceb93d77bf95f1 Mon Sep 17 00:00:00 2001
From: Daniele Nicolodi <nicolodi@domain.hid>
Date: Tue, 4 Oct 2011 14:49:27 +0200
Subject: [PATCH 1/3] analogy: demote some messages logged in mio_common
 driver code to debug level

---
 .../analogy/national_instruments/mio_common.c      |   24 ++++++++++----------
 1 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/ksrc/drivers/analogy/national_instruments/mio_common.c b/ksrc/drivers/analogy/national_instruments/mio_common.c
index a68cf2c..4303934 100644
--- a/ksrc/drivers/analogy/national_instruments/mio_common.c
+++ b/ksrc/drivers/analogy/national_instruments/mio_common.c
@@ -835,8 +835,8 @@ static void handle_a_interrupt(a4l_dev_t *dev,
 	if((subd->flags & A4L_SUBD_TYPES) == A4L_SUBD_UNUSED)
 		return;
 
-	a4l_info(dev, "ni_mio_common: interrupt: "
-		 "a_status=%04x ai_mite_status=%08x\n",status, ai_mite_status);
+	a4l_dbg(1, drv_dbg, dev, "ni_mio_common: interrupt: "
+		"a_status=%04x ai_mite_status=%08x\n",status, ai_mite_status);
 	ni_mio_print_status_a(status);
 
 #if (defined(CONFIG_XENO_DRIVERS_ANALOGY_NI_MITE) || \
@@ -847,9 +847,9 @@ static void handle_a_interrupt(a4l_dev_t *dev,
 	if (ai_mite_status & ~(CHSR_INT | CHSR_LINKC | CHSR_DONE | CHSR_MRDY |
 			       CHSR_DRDY | CHSR_DRQ1 | CHSR_DRQ0 | CHSR_ERROR |
 			       CHSR_SABORT | CHSR_XFERR | CHSR_LxERR_mask)) {
-		a4l_info(dev, "ni_mio_common: interrupt: "
-			 "unknown mite interrupt, ack! (ai_mite_status=%08x)\n",
-			 ai_mite_status);
+		a4l_dbg(1, drv_dbg, dev, "ni_mio_common: interrupt: "
+			"unknown mite interrupt, ack! (ai_mite_status=%08x)\n",
+			ai_mite_status);
 		a4l_buf_evt(subd, A4L_BUF_ERROR);
 	}
 #endif /* CONFIG_XENO_DRIVERS_ANALOGY_NI_MITE */
@@ -858,8 +858,8 @@ static void handle_a_interrupt(a4l_dev_t *dev,
 	if (status & (AI_Overrun_St | AI_Overflow_St | AI_SC_TC_Error_St |
 		      AI_SC_TC_St | AI_START1_St)) {
 		if (status == 0xffff) {
-			a4l_info(dev, "ni_mio_common: interrupt: "
-				 "a_status=0xffff.  Card removed?\n");
+			a4l_dbg(1, drv_dbg, dev, "ni_mio_common: interrupt: "
+				"a_status=0xffff.  Card removed?\n");
 			/* TODO: we probably aren't even running a command now,
 			   so it's a good idea to be careful.
 			   we should check the transfer status */
@@ -869,8 +869,8 @@ static void handle_a_interrupt(a4l_dev_t *dev,
 		}
 		if (status & (AI_Overrun_St | AI_Overflow_St |
 			      AI_SC_TC_Error_St)) {
-			a4l_info(dev, "ni_mio_common: interrupt: "
-				 "ai error a_status=%04x\n", status);
+			a4l_dbg(1, drv_dbg, dev, "ni_mio_common: interrupt: "
+				"ai error a_status=%04x\n", status);
 			ni_mio_print_status_a(status);
 
 			shutdown_ai_command(subd);
@@ -881,7 +881,7 @@ static void handle_a_interrupt(a4l_dev_t *dev,
 			return;
 		}
 		if (status & AI_SC_TC_St) {
-			a4l_info(dev, "ni_mio_common: SC_TC interrupt\n");
+			a4l_dbg(1, drv_dbg, dev, "ni_mio_common: SC_TC interrupt\n");
 			if (!devpriv->ai_continuous) {
 				shutdown_ai_command(subd);
 			}
@@ -914,8 +914,8 @@ static void handle_a_interrupt(a4l_dev_t *dev,
 
 	status = devpriv->stc_readw(dev, AI_Status_1_Register);
 	if (status & Interrupt_A_St)
-		a4l_info(dev, "ni_mio_common: interrupt: "
-			 " didn't clear interrupt? status=0x%x\n", status);
+		a4l_dbg(1, drv_dbg, dev, "ni_mio_common: interrupt: "
+			" didn't clear interrupt? status=0x%x\n", status);
 }
 
 static void ack_b_interrupt(a4l_dev_t *dev, unsigned short b_status)
-- 
1.7.6.3

Re: [Xenomai-core] Analogy: idx_write_subd field in a4l_desc_t

[Alexis Berlemont]

Hi,

On Tue, Oct 4, 2011 at 3:31 PM, Daniele Nicolodi <daniele@domain.hid> wrote:
> On 04/10/11 13:17, Gilles Chanteperdrix wrote:
>> If you want things merged, send the patches, they will reach
>> xenomai-head repository through Alex.
>
> Of course. Here is my simple patch.
>

Thanks.

> Thank you. Cheers,
> --
> Daniele
>

Alexis.

Re: [Xenomai-core] Analogy: idx_write_subd field in a4l_desc_t

[Daniele Nicolodi]

On 01/10/11 00:03, Alexis Berlemont wrote:
>> I'm using xenomai-head on a 2.6.38.8 kernel on x86 with a NI-6251 DAQ
>> board. In this configuration the idx_write_subd field of the a4l_desc_t
>> structure filled by a4l_open() is not set to the proper value but is set
>> to NULL.
>>
>> In previous xanomai/analogy releases this was working properly. Has some
>> initialization code been removed in the latest analogy drivers refactoring?
> 
> Yes. Formerly, on both sides (kernel and user), we used some
> description fields (idx_read_subd and idx_write_subd) so as to quickly
> identify default asynchronous input and output subdevices and to link
> them with buffers (into which, input / output data are copied).

...

> For API / ABI compatibility reasons, I waited a major release before
> removing the fields idx_{read, write}_subd. I should have thought
> twice before removing their initializations. I will fix that soon,
> sorry.

Hello Alexis,

the rational of the change is clear, however having the field in the
structure, but with the wrong value is confusing, especially so because
the documentation does not mention that this field is deprecated and its
value should not be trusted.

I would remove the field altogether, or put back the initialization and
mention the deprecation in the documentation.

> I implemented this change more than one year ago (here is on of the
> many related commits 58ebd5b7efe0cc0ac1e82991d12125ce34dfeee3). That
> was already present in 2.5. Which version were you formerly using?

I thought I was using Xenomai 2.5.6.

Cheers,
-- 
Daniele