[Skiboot] [PATCH 2/5] hw/lpc-mbox: Use message registers for interrupts

Cyril Bur cyril.bur at au1.ibm.com
Mon Apr 24 19:14:08 AEST 2017


Currently the BMC raises the interrupt using the BMC control register.
It does so on all accesses to the 16 'data' registers meaning that when
the BMC only wants to set the ATTN (on which we have interrupts enabled)
bit we will also get a control register based interrupt.

The solution here is to mask that interrupt permanently and enable
interrupts on the protocol defined 'response' data byte.

Signed-off-by: Cyril Bur <cyril.bur at au1.ibm.com>
---
 hw/lpc-mbox.c      | 63 ++++++++++++++++++++++++++++++++++++++++++------------
 include/lpc-mbox.h |  4 ++--
 2 files changed, 51 insertions(+), 16 deletions(-)

diff --git a/hw/lpc-mbox.c b/hw/lpc-mbox.c
index 21e6eee0..7e509f54 100644
--- a/hw/lpc-mbox.c
+++ b/hw/lpc-mbox.c
@@ -35,12 +35,14 @@
 
 #define MBOX_FLAG_REG 0x0f
 #define MBOX_STATUS_0 0x10
-#define   MBOX_STATUS_ATTN (1 << 7)
 #define MBOX_STATUS_1 0x11
+#define   MBOX_STATUS_1_ATTN (1 << 7)
+#define   MBOX_STATUS_1_RESP (1 << 5)
 #define MBOX_BMC_CTRL 0x12
 #define   MBOX_CTRL_INT_STATUS (1 << 7)
 #define   MBOX_CTRL_INT_MASK (1 << 1)
-#define   MBOX_CTRL_INT_SEND (1 << 0)
+#define   MBOX_CTRL_INT_PING (1 << 0)
+#define   MBOX_CTRL_INT_SEND (MBOX_CTRL_INT_PING | MBOX_CTRL_INT_MASK)
 #define MBOX_HOST_CTRL 0x13
 #define MBOX_BMC_INT_EN_0 0x14
 #define MBOX_BMC_INT_EN_1 0x15
@@ -85,7 +87,7 @@ static void bmc_mbox_recv_message(struct bmc_mbox_msg *msg)
 	uint8_t *msg_data = (uint8_t *)msg;
 	int i;
 
-	for (i = 0; i < BMC_MBOX_DATA_REGS; i++)
+	for (i = 0; i < BMC_MBOX_READ_REGS; i++)
 		msg_data[i] = bmc_mbox_inb(i);
 }
 
@@ -98,9 +100,18 @@ static void bmc_mbox_send_message(struct bmc_mbox_msg *msg)
 	if (!lpc_ok())
 		/* We're going to have to handle this better */
 		prlog(PR_ERR, "LPC isn't ok\n");
-	for (i = 0; i < BMC_MBOX_DATA_REGS; i++)
+
+	for (i = 0; i < BMC_MBOX_WRITE_REGS; i++)
 		bmc_mbox_outb(msg_data[i], i);
 
+	/*
+	 * Don't touch the response byte, we're listening on it to know
+	 * when we get a response
+	 * Don't touch our host status reg, it isn't a part of messages
+	 */
+
+	/* Absolutely don't touch the BMC status reg, we aren't allowed */
+
 	/* Ping */
 	prlog(PR_DEBUG, "Sending BMC interrupt\n");
 	bmc_mbox_outb(MBOX_CTRL_INT_SEND, MBOX_HOST_CTRL);
@@ -136,10 +147,14 @@ static void mbox_poll(struct timer *t __unused, void *data __unused,
 {
 	struct bmc_mbox_msg *msg;
 
-	/* This is a 'registered' the message you just sent me */
-	if (bmc_mbox_inb(MBOX_HOST_CTRL) & MBOX_CTRL_INT_STATUS) {
+	/*
+	 * This status bit being high means that someone touched the
+	 * response byte (byte 13).
+	 * There is probably a response for the previously sent commant
+	 */
+	if (bmc_mbox_inb(MBOX_STATUS_1) & MBOX_STATUS_1_RESP) {
 		/* W1C on that reg */
-		bmc_mbox_outb(MBOX_CTRL_INT_STATUS, MBOX_HOST_CTRL);
+		bmc_mbox_outb(MBOX_STATUS_1_RESP, MBOX_STATUS_1);
 
 		prlog(PR_INSANE, "Got a regular interrupt\n");
 		/*
@@ -148,7 +163,7 @@ static void mbox_poll(struct timer *t __unused, void *data __unused,
 		msg = mbox.in_flight;
 		if (msg == NULL) {
 			prlog(PR_CRIT, "Couldn't find the message!!\n");
-			return;
+			goto out_response;
 		}
 		bmc_mbox_recv_message(msg);
 		if (mbox.callback)
@@ -162,19 +177,29 @@ static void mbox_poll(struct timer *t __unused, void *data __unused,
 		unlock(&mbox.lock);
 	}
 
-	/* This is to indicate that the BMC has information to tell us */
-	if (bmc_mbox_inb(MBOX_STATUS_1) & MBOX_STATUS_ATTN) {
+out_response:
+
+	/*
+	 * The BMC has touched byte 15 to get our attention as it has
+	 * something to tell us.
+	 */
+	if (bmc_mbox_inb(MBOX_STATUS_1) & MBOX_STATUS_1_ATTN) {
 		uint8_t action;
 
 		/* W1C on that reg */
-		bmc_mbox_outb(MBOX_STATUS_ATTN, MBOX_STATUS_1);
+		bmc_mbox_outb(MBOX_STATUS_1_ATTN, MBOX_STATUS_1);
 
 		action = bmc_mbox_inb(MBOX_FLAG_REG);
-		prlog(PR_INSANE, "Got a status register interrupt with action 0x%02x\n",
+		prlog(PR_TRACE, "Got a status register interrupt with action 0x%02x\n",
 				action);
 
 		if (action & BMC_RESET) {
-			/* TODO Freak */
+			/*
+			 * It's unlikely that something needs to be done at the
+			 * driver level. Let libflash deal with it.
+			 * Print something just in case, it is quite a signficant
+			 * event.
+			 */
 			prlog(PR_WARNING, "BMC reset detected\n");
 			action &= ~BMC_RESET;
 		}
@@ -202,7 +227,7 @@ static bool mbox_init_hw(void)
 {
 	/* Disable all status interrupts except attentions */
 	bmc_mbox_outb(0x00, MBOX_HOST_INT_EN_0);
-	bmc_mbox_outb(MBOX_STATUS_ATTN, MBOX_HOST_INT_EN_1);
+	bmc_mbox_outb(MBOX_STATUS_1_ATTN, MBOX_HOST_INT_EN_1);
 
 	/* Cleanup host interrupt and status */
 	bmc_mbox_outb(MBOX_CTRL_INT_STATUS, MBOX_HOST_CTRL);
@@ -274,6 +299,13 @@ void mbox_init(void)
 		return;
 	}
 
+	/* Disable the standard interrupt we don't care */
+	bmc_mbox_outb(MBOX_CTRL_INT_MASK, MBOX_HOST_CTRL);
+
+	/* Clear the status reg bits that we intend to use for interrupts */
+	/* W1C */
+	bmc_mbox_outb(MBOX_STATUS_1_RESP | MBOX_STATUS_1_ATTN, MBOX_STATUS_1);
+
 	mbox.queue_len = 0;
 	mbox.in_flight = NULL;
 	mbox.callback = NULL;
@@ -286,6 +318,9 @@ void mbox_init(void)
 	mbox_lpc_client.interrupts = LPC_IRQ(irq);
 	lpc_register_client(chip_id, &mbox_lpc_client, IRQ_ATTR_TARGET_OPAL);
 
+	/* Enable interrupts */
+	bmc_mbox_outb(MBOX_STATUS_1_ATTN | MBOX_STATUS_1_RESP, MBOX_HOST_INT_EN_1);
+
 	prlog(PR_DEBUG, "Enabled on chip %d, IO port 0x%x, IRQ %d\n",
 	      chip_id, mbox.base, irq);
 }
diff --git a/include/lpc-mbox.h b/include/lpc-mbox.h
index c67efee9..14e38cf8 100644
--- a/include/lpc-mbox.h
+++ b/include/lpc-mbox.h
@@ -20,9 +20,9 @@
 #include <opal.h>
 #include <ccan/endian/endian.h>
 
-/* Not 16 because the last two are interrupt based status regs */
-#define BMC_MBOX_DATA_REGS 14
 #define BMC_MBOX_ARGS_REGS 11
+#define BMC_MBOX_READ_REGS 16
+#define BMC_MBOX_WRITE_REGS 13
 
 #define MBOX_C_RESET_STATE 0x01
 #define MBOX_C_GET_MBOX_INFO 0x02
-- 
2.12.2



More information about the Skiboot mailing list