[PATCH 09/15] staging: comedi: mite: introduce mite_ack_linkc()

[Hartley Sweeten]

On Wednesday, April 20, 2016 2:44 AM, Ian Abbott wrote:
> On 19/04/16 17:41, Hartley Sweeten wrote:
>> 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 found a titled "MITE RLPM: MITE PCI/PXI Interface and DMA Controller".
It's looks to be pretty recent but it's not very complete. The header of the
document states: "MITE RLPM rough draft v0.1 4/20/2016 (C) National
Instruments"

>From that document.

The MERR bits indicate a "Memory Transfer Error":
  00 - No Error
  01 - Bus Error
  10 - Retry Limit Exceeded
  11 - Other Transfer Error (Parity for MXI)

In this case the MRERR is indicating a retry limit exceeded

Currently the mite driver sets all the "retry limits" to 64. This results in
a CR_RL() value of 7 which is the maximum value that can be used by
those bits. I should just remove the mite_retry_limit() helper and just
use CR_RL(7) instead.

Unfortunately, the document does not describe these bits. The only
Information about the bits is:

Bits 31-24:	0 0 0 0 0 0 0 0
Bits 23-16:	1 1 1 0 0 0 0 0
Bits 15-8:	0 0 0 0 1 PSIZE1 PSIZE0
Bits 7-0:	0 0 0 0 0 0 0 0

PSIZE[1:0] - Port Transfer Size
	01 - 8 Bit
	10 - 16 Bit
	11 - 32 Bit

Note shown in the document are these bits defined in the mite driver:

Bits 23-21 are the CR_RL bits
Bits 11-10 are the CR_ASEQ bits

So, it looks like the mite driver is setting the bits "correctly" but for some
reason the memory transfer is failing due to a retry limit exceeded.

But, it looks like a "transfer error" can be detected by just checking the
XFERR bit. This bit indicates that the transfer was terminated by a LERR,
MERR, or DERR. This test could be moved into mite_ack_linkc() as:

-unsigned int mite_ack_linkc(struct mite_channel *mite_chan)
+unsigned int mite_ack_linkc(struct mite_channel *mite_chan,
+			    struct comedi_subdevice *s)
{
	struct mite_struct *mite = mite_chan->mite;
	unsigned int status;

	status = mite_get_status(mite_chan);
	if (status & CHSR_LINKC)
		writel(CHOR_CLRLC,
		       mite->mite_io_addr + MITE_CHOR(mite_chan->channel));

+	if (status & CHSR_XFERR) {
+		dev_err(s->device->class_dev,
+			"mite transfer error %08x\n", status);
+		s->async->events |= COMEDI_CB_ERROR;
+	}
+
	return status;
}
EXPORT_SYMBOL_GPL(mite_ack_linkc);

Then the CHSR defines can all be moved into mite.c and not get exposed.

Sound reasonable?

Hartley