diff mbox

[2/9] RapidIO, powerpc/85xx: modify RIO port-write interrupt handler

Message ID 1281712686-31308-3-git-send-email-alexandre.bounine@idt.com (mailing list archive)
State Superseded, archived
Delegated to: Kumar Gala
Headers show

Commit Message

Bounine, Alexandre Aug. 13, 2010, 3:17 p.m. UTC 
- Rearranged RIO port-write interrupt handling to perform message buffering
as soon as possible.
- Modified to disable port-write controller when clearing Transaction Error (TE)
bit.

Signed-off-by: Alexandre Bounine <alexandre.bounine@idt.com>
Reviewed-by: Thomas Moll <thomas.moll@sysgo.com>
Cc: Matt Porter <mporter@kernel.crashing.org>
Cc: Li Yang <leoli@freescale.com>
Cc: Kumar Gala <galak@kernel.crashing.org>
---
 arch/powerpc/sysdev/fsl_rio.c |   67 ++++++++++++++++++++++------------------
 1 files changed, 37 insertions(+), 30 deletions(-)

Comments

Micha Nelissen Aug. 16, 2010, 12:12 p.m. UTC | #1 
Alexandre Bounine wrote:
> - Rearranged RIO port-write interrupt handling to perform message buffering
> as soon as possible.

I don't understand this comment: you still schedule work to read the 
port-write queue; so how is this message buffering performed as soon as 
possible?

> - Modified to disable port-write controller when clearing Transaction Error (TE)
> bit.
>  	/* Schedule deferred processing if PW was received */
> -	if (ipwsr & RIO_IPWSR_QFI) {
> +	if ((ipwmr & RIO_IPWMR_QFIE) && (ipwsr & RIO_IPWSR_QFI)) {

Why check the QFIE bit also?

> +pw_done:
> +	if (epwisr & 0x80000000) {

Magic value.

Micha
Bounine, Alexandre Aug. 16, 2010, 3:27 p.m. UTC | #2 
Micha Nelissen wrote:
> 
> Alexandre Bounine wrote:
> > - Rearranged RIO port-write interrupt handling to perform message
buffering
> > as soon as possible.
> 
> I don't understand this comment: you still schedule work to read the
> port-write queue; so how is this message buffering performed as soon
as
> possible?


Compared to the original code, I rearranged order of checking interrupt
status bits to check the queue status first. The 85xx PW controller is
capable to receive and keep only one PW message. Therefore, I copy it
into the driver's FIFO and re-enable HW Rx queue (it is called queue but
can accept only one entry) ASAP. I have a test setup that is capable
generate multiple PW messages and see many messages discarded by PW
controller
because of this single-entry HW queue.
 
> 
> > - Modified to disable port-write controller when clearing
Transaction Error (TE)
> > bit.
> >  	/* Schedule deferred processing if PW was received */
> > -	if (ipwsr & RIO_IPWSR_QFI) {
> > +	if ((ipwmr & RIO_IPWMR_QFIE) && (ipwsr & RIO_IPWSR_QFI)) {
> 
> Why check the QFIE bit also?

Oops! Leftover from some testing. Will clean it up.

> 
> > +pw_done:
> > +	if (epwisr & 0x80000000) {
> 
> Magic value.


Agree. Will correct.
Micha Nelissen Aug. 17, 2010, 7:12 a.m. UTC | #3 
Bounine, Alexandre wrote:
> capable to receive and keep only one PW message. Therefore, I copy it
> into the driver's FIFO and re-enable HW Rx queue (it is called queue but
> can accept only one entry) ASAP. I have a test setup that is capable
> generate multiple PW messages and see many messages discarded by PW
> controller because of this single-entry HW queue.

Primarily due to the single entry queue, the order of checking is 
probably insignificant? :-) Anyway, I don't mind changing the order.

Micha
Bounine, Alexandre Aug. 17, 2010, 5:16 p.m. UTC | #4 
Micha Nelissen wrote:
> 
> Primarily due to the single entry queue, the order of checking is
> probably insignificant? :-)

Help sometimes only but gives a feeling that I did all that is possible
;) 

> Anyway, I don't mind changing the order.
> 
> Micha
diff mbox

Patch

diff --git a/arch/powerpc/sysdev/fsl_rio.c b/arch/powerpc/sysdev/fsl_rio.c
index cd71dc1..708d94e 100644
--- a/arch/powerpc/sysdev/fsl_rio.c
+++ b/arch/powerpc/sysdev/fsl_rio.c
@@ -1065,18 +1065,12 @@  fsl_rio_port_write_handler(int irq, void *dev_instance)
 	struct rio_priv *priv = port->priv;
 	u32 epwisr, tmp;
 
-	ipwmr = in_be32(&priv->msg_regs->pwmr);
-	ipwsr = in_be32(&priv->msg_regs->pwsr);
-
 	epwisr = in_be32(priv->regs_win + RIO_EPWISR);
-	if (epwisr & 0x80000000) {
-		tmp = in_be32(priv->regs_win + RIO_LTLEDCSR);
-		pr_info("RIO_LTLEDCSR = 0x%x\n", tmp);
-		out_be32(priv->regs_win + RIO_LTLEDCSR, 0);
-	}
-
 	if (!(epwisr & 0x00000001))
-		return IRQ_HANDLED;
+		goto pw_done;
+
+	ipwmr = in_be32(&priv->msg_regs->pwmr);
+	ipwsr = in_be32(&priv->msg_regs->pwsr);
 
 #ifdef DEBUG_PW
 	pr_debug("PW Int->IPWMR: 0x%08x IPWSR: 0x%08x (", ipwmr, ipwsr);
@@ -1092,22 +1086,8 @@  fsl_rio_port_write_handler(int irq, void *dev_instance)
 		pr_debug(" PWB");
 	pr_debug(" )\n");
 #endif
-	out_be32(&priv->msg_regs->pwsr,
-		 ipwsr & (RIO_IPWSR_TE | RIO_IPWSR_QFI | RIO_IPWSR_PWD));
-
-	if ((ipwmr & RIO_IPWMR_EIE) && (ipwsr & RIO_IPWSR_TE)) {
-		priv->port_write_msg.err_count++;
-		pr_info("RIO: Port-Write Transaction Err (%d)\n",
-			 priv->port_write_msg.err_count);
-	}
-	if (ipwsr & RIO_IPWSR_PWD) {
-		priv->port_write_msg.discard_count++;
-		pr_info("RIO: Port Discarded Port-Write Msg(s) (%d)\n",
-			 priv->port_write_msg.discard_count);
-	}
-
 	/* Schedule deferred processing if PW was received */
-	if (ipwsr & RIO_IPWSR_QFI) {
+	if ((ipwmr & RIO_IPWMR_QFIE) && (ipwsr & RIO_IPWSR_QFI)) {
 		/* Save PW message (if there is room in FIFO),
 		 * otherwise discard it.
 		 */
@@ -1117,16 +1097,43 @@  fsl_rio_port_write_handler(int irq, void *dev_instance)
 				 RIO_PW_MSG_SIZE);
 		} else {
 			priv->port_write_msg.discard_count++;
-			pr_info("RIO: ISR Discarded Port-Write Msg(s) (%d)\n",
+			pr_debug("RIO: ISR Discarded Port-Write Msg(s) (%d)\n",
 				 priv->port_write_msg.discard_count);
 		}
+		/* Clear interrupt and issue Clear Queue command. This allows
+		 * another port-write to be received.
+		 */
+		out_be32(&priv->msg_regs->pwsr,	RIO_IPWSR_QFI);
+		out_be32(&priv->msg_regs->pwmr, ipwmr | RIO_IPWMR_CQ);
+
 		schedule_work(&priv->pw_work);
 	}
 
-	/* Issue Clear Queue command. This allows another
-	 * port-write to be received.
-	 */
-	out_be32(&priv->msg_regs->pwmr, ipwmr | RIO_IPWMR_CQ);
+	if ((ipwmr & RIO_IPWMR_EIE) && (ipwsr & RIO_IPWSR_TE)) {
+		priv->port_write_msg.err_count++;
+		pr_debug("RIO: Port-Write Transaction Err (%d)\n",
+			 priv->port_write_msg.err_count);
+		/* Clear Transaction Error: port-write controller should be
+		 * disabled when clearing this error
+		 */
+		out_be32(&priv->msg_regs->pwmr, ipwmr & ~RIO_IPWMR_PWE);
+		out_be32(&priv->msg_regs->pwsr,	RIO_IPWSR_TE);
+		out_be32(&priv->msg_regs->pwmr, ipwmr);
+	}
+
+	if (ipwsr & RIO_IPWSR_PWD) {
+		priv->port_write_msg.discard_count++;
+		pr_debug("RIO: Port Discarded Port-Write Msg(s) (%d)\n",
+			 priv->port_write_msg.discard_count);
+		out_be32(&priv->msg_regs->pwsr, RIO_IPWSR_PWD);
+	}
+
+pw_done:
+	if (epwisr & 0x80000000) {
+		tmp = in_be32(priv->regs_win + RIO_LTLEDCSR);
+		pr_debug("RIO_LTLEDCSR = 0x%x\n", tmp);
+		out_be32(priv->regs_win + RIO_LTLEDCSR, 0);
+	}
 
 	return IRQ_HANDLED;
 }