On 19/04/16 17:41, Hartley Sweeten wrote:
On Tuesday, April 19, 2016 3:23 AM, Ian Abbott wrote:
On 18/04/16 21:28, H Hartley Sweeten wrote:
Introduce a helper function to handle the ack of a LINKC interrupt.
Tidy up the drivers that use the new helper.
The mite_get_status() function is not only used by the mite driver.
Make it static and remove the export.
Signed-off-by: H Hartley Sweeten <hsweeten@xxxxxxxxxxxxxxxxxxx>
Cc: Ian Abbott <abbotti@xxxxxxxxx>
Cc: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
---
drivers/staging/comedi/drivers/mite.c | 17 ++++++++++++++--
drivers/staging/comedi/drivers/mite.h | 2 +-
drivers/staging/comedi/drivers/ni_mio_common.c | 28 +++++---------------------
drivers/staging/comedi/drivers/ni_pcidio.c | 10 ++-------
drivers/staging/comedi/drivers/ni_tiocmd.c | 11 ++--------
5 files changed, 25 insertions(+), 43 deletions(-)
[snip]
diff --git a/drivers/staging/comedi/drivers/ni_pcidio.c b/drivers/staging/comedi/drivers/ni_pcidio.c
index c044c8b..b67358d 100644
--- a/drivers/staging/comedi/drivers/ni_pcidio.c
+++ b/drivers/staging/comedi/drivers/ni_pcidio.c
@@ -381,12 +381,10 @@ static irqreturn_t nidio_interrupt(int irq, void *d)
struct nidio96_private *devpriv = dev->private;
struct comedi_subdevice *s = dev->read_subdev;
struct comedi_async *async = s->async;
- struct mite_struct *mite = devpriv->mite;
unsigned int auxdata;
int flags;
int status;
int work = 0;
- unsigned int m_status = 0;
/* interrupcions parasites */
if (!dev->attached) {
@@ -401,14 +399,10 @@ static irqreturn_t nidio_interrupt(int irq, void *d)
flags = readb(dev->mmio + Group_1_Flags);
spin_lock(&devpriv->mite_channel_lock);
- if (devpriv->di_mite_chan)
- m_status = mite_get_status(devpriv->di_mite_chan);
+ if (devpriv->di_mite_chan) {
+ unsigned int m_status = mite_ack_linkc(devpriv->di_mite_chan);
- if (m_status & CHSR_INT) {
Is the removal of that `m_status & CHSR_INT` test deliberate? It looks
a bit iffy.
Yes. This is the only driver that uses mite that checks the CHSR_INT bit.
Unfortunately I have not been able to find any "good" information on the
NI MITE ASIC. My guess is that this bit is a global interrupt status bit. It gets
set if any of the other interrupt sources are generating an interrupt.
The mite_get_status() function also does not check for CHSR_INT when
acking the CHSR_DONE interrupt so I assume that the extra check is not
needed.
Let me know if you prefer to leave in the check.
I guess the extra check is not needed.
Side-note regarding the CHSR bits. Some of the subdevices that use mite
also check for "unknown" interrupts.
ni_pcimio for the DIO "read_subdev"
ni_mio_common for the AI "read_subdev"
for the AO "write_subdev"
But others don't:
ni_mio_common for the DIO subdevice
ni_tiocmd for the ni_660x COUNTER subdevices
ni_tiocmd for the ni_mio_common COUNTER subdevices
How do you feel about removing the "unknown" interrupt checks?
If they are removed the CHSR_* defines in mite.h can be moved to the
mite driver and not be needlessly exposed.
I've seen occasional (but unresolved) threads on the COMEDI mailing
lists from people getting the "unknown mite interrupt" messages, so it's
probably best to leave them in for now.
For example, this one:
https://groups.google.com/d/topic/comedi_list/dDH4BxqLhSk/discussion
The two MITE status register values mentioned in posts in that thread
are 0x82808248 and 0x82888248 and the problem seems to be that the mite
status & CHSR_MxERR_mask is non-zero (specifically CHSR_MRERR) and that
is not currently handled. It might be safe to ignore, or it might not,
but we'd need to check the docs.
I think I had some sort of MITE register-level manual once, but it seems
a bit hard to track down now!
--
-=( Ian Abbott @ MEV Ltd. E-mail: <abbotti@xxxxxxxxx> )=-
-=( Web: http://www.mev.co.uk/ )=-
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
