diff mbox series

hw/port80: Squash No SYNC error

Message ID 20191018043000.21033-1-oohall@gmail.com
State Accepted
Headers show
Series hw/port80: Squash No SYNC error | expand

Checks

Context Check Description
snowpatch_ozlabs/apply_patch success Successfully applied on branch master (d75e82dbfbb9443efeb3f9a5921ac23605aab469)
snowpatch_ozlabs/snowpatch_job_snowpatch-skiboot fail Test snowpatch/job/snowpatch-skiboot on branch master
snowpatch_ozlabs/snowpatch_job_snowpatch-skiboot-dco success Signed-off-by present

Commit Message

Oliver O'Halloran Oct. 18, 2019, 4:30 a.m. UTC
On Aspeed BMCs can be configured to route LPC IO address 0x80 to a GPIO
port. Some systems use this to implement a boot progress indicator, but
not all of them.

There's no easy way to tell if this has been setup or not and if it
hasn't we get an LPC SYNC no-response error from out LPC master. When we
reach Linux and enable interrupts this results in this spurious error
being printed:

    LPC[000]: Got SYNC no-response error. Error address reg: 0xd0010082

lpc_probe_write() is intended to catch situations where the peripherial
being written to might not be configured, so use that instead of
lpc_outb() to squash the error.

Cc: Ranga <stewart@flamingspork.com>
Cc: Andrew Jeffery <andrew@aj.id.au>
Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
---
Stewart, do you know what systems actually implment this feature?
Seem like you added op_display_lpc() to add the BMC systems, but
I'm guessing only a few of them actually support it.
---
 hw/lpc-port80h.c      | 7 ++++---
 hw/test/run-port80h.c | 5 ++++-
 2 files changed, 8 insertions(+), 4 deletions(-)

Comments

Andrew Jeffery Oct. 18, 2019, 4:44 a.m. UTC | #1
On Fri, 18 Oct 2019, at 15:00, Oliver O'Halloran wrote:
> On Aspeed BMCs can be configured to route LPC IO address 0x80 to a GPIO
> port. Some systems use this to implement a boot progress indicator, but
> not all of them.
> 
> There's no easy way to tell if this has been setup or not and if it
> hasn't we get an LPC SYNC no-response error from out LPC master. When we
> reach Linux and enable interrupts this results in this spurious error
> being printed:
> 
>     LPC[000]: Got SYNC no-response error. Error address reg: 0xd0010082
> 
> lpc_probe_write() is intended to catch situations where the peripherial
> being written to might not be configured, so use that instead of
> lpc_outb() to squash the error.
> 
> Cc: Ranga <stewart@flamingspork.com>
> Cc: Andrew Jeffery <andrew@aj.id.au>
> Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
> ---
> Stewart, do you know what systems actually implment this feature?
> Seem like you added op_display_lpc() to add the BMC systems, but
> I'm guessing only a few of them actually support it.

At least the Raptor systems as far as I'm aware, they needed some way
to observe boot progress.

Anyway, patch seems reasonable to me.

Acked-by: Andrew Jeffery <andrew@aj.id.au>
Stewart Smith Oct. 18, 2019, 6:36 a.m. UTC | #2
> On 17 Oct 2019, at 21:30, Oliver O'Halloran <oohall@gmail.com> wrote:
> 
> On Aspeed BMCs can be configured to route LPC IO address 0x80 to a GPIO
> port. Some systems use this to implement a boot progress indicator, but
> not all of them.
> 
> There's no easy way to tell if this has been setup or not and if it
> hasn't we get an LPC SYNC no-response error from out LPC master. When we
> reach Linux and enable interrupts this results in this spurious error
> being printed:
> 
>    LPC[000]: Got SYNC no-response error. Error address reg: 0xd0010082
> 
> lpc_probe_write() is intended to catch situations where the peripherial
> being written to might not be configured, so use that instead of
> lpc_outb() to squash the error.
> 
> Cc: Ranga <stewart@flamingspork.com>
> Cc: Andrew Jeffery <andrew@aj.id.au>
> Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
> ---
> Stewart, do you know what systems actually implment this feature?
> Seem like you added op_display_lpc() to add the BMC systems, but
> I'm guessing only a few of them actually support it.

All the Raptor systems do, and the code has (IIRC) headed upstream to OpenBMC.

Maybe should just silence the error on other systems? That was the original thought, nothing there listening = silence any error
diff mbox series

Patch

diff --git a/hw/lpc-port80h.c b/hw/lpc-port80h.c
index eb1378c8ad2b..b373af7bcc8f 100644
--- a/hw/lpc-port80h.c
+++ b/hw/lpc-port80h.c
@@ -164,9 +164,10 @@  void op_display_lpc(enum op_severity s, enum op_module m, uint16_t c)
 		return;
 
 	port80_val = op_display_to_port80(port80_val, s, m, c);
-	lpc_outb(port80_val, 0x80);
 	port8x_val = op_display_to_port8x(port8x_val, s, m, c);
-	lpc_outb(port8x_val >> 8, 0x81);
-	lpc_outb(port8x_val & 0xFF, 0x82);
+
+	lpc_probe_write(OPAL_LPC_IO, 0x80, port80_val,        1);
+	lpc_probe_write(OPAL_LPC_IO, 0x81, port8x_val >> 8,   1);
+	lpc_probe_write(OPAL_LPC_IO, 0x82, port8x_val & 0xff, 1);
 }
 
diff --git a/hw/test/run-port80h.c b/hw/test/run-port80h.c
index 85e8d8721244..4964f4836ba8 100644
--- a/hw/test/run-port80h.c
+++ b/hw/test/run-port80h.c
@@ -15,15 +15,18 @@ 
 uint8_t port80;
 uint16_t port8x;
 
-static inline void lpc_outb(uint8_t data, uint32_t addr)
+static int64_t lpc_probe_write(int addr_type __unused, uint32_t addr,
+                        uint32_t data, uint32_t sz)
 {
 	assert((addr - 0x80) <= 2);
+	assert(sz == 1);
 	if (addr == 0x80)
 		port80 = data;
 	if (addr == 0x81)
 		port8x = data << 8 | (port8x & 0xff);
 	if (addr == 0x82)
 		port8x = (port8x & 0xff00) | data;
+	return 0;
 }
 
 #include "op-panel.h"