[Skiboot] [PATCH 08/12] ast-io: Fix Ready/Busy indication

Andrew Jeffery andrew at aj.id.au
Tue Oct 22 17:08:15 AEDT 2019


Hi Deb,

On Fri, 18 Oct 2019, at 08:53, Deb McLemore wrote:
> Properly report if the SuperIO is ready or busy.

You're conflating SuperIO with the iLPC2AHB backdoor here. The iLPC2AHB
is a SuperIO device, so they're related but different entities: SuperIO is a
byte-based indexed-IO interface to devices, the iLPC2AHB maps read and
write operations from the host onto the BMC's physical address space.

Also the bit being read doesn't determine whether SuperIO is ready or busy,
but whether the iLPC2AHB bridge has been "disabled" (i.e. writes have no
effect). SuperIO and its devices have no concept of being busy, SuperIO
relies on the firmware to protect against concurrent access. If SuperIO is
enabled we can always read the state of the "disable" bit via the iLPC2AHB
device as the disable bit only impacts the device's ability to write.

Without the ability to write via the iLPC2AHB bridge we have no means to
drive the flash controller in the event that HIOMAP support isn't present
(e.g. older P8 firmwares).

> 
> Signed-off-by: Deb McLemore <debmc at linux.ibm.com>
> ---
>  hw/ast-bmc/ast-io.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/ast-bmc/ast-io.c b/hw/ast-bmc/ast-io.c
> index 171942a..22fb59f 100644
> --- a/hw/ast-bmc/ast-io.c
> +++ b/hw/ast-bmc/ast-io.c
> @@ -361,7 +361,13 @@ bool ast_sio_init(void)
>  
>  bool ast_io_is_rw(void)
>  {
> -	return !(ast_ahb_readl(LPC_HICRB) & LPC_HICRB_ILPC_DISABLE);
> +	int rc;
> +	rc = ast_ahb_readl(LPC_HICRB);
> +	prlog(PR_NOTICE, "LPC: SuperIO Ready/Busy=%s rc=0x%x LPC_HICRB 0x%x\n",
> +		((rc & LPC_HICRB_ILPC_DISABLE) ? "Ready" : "Busy"),
> +		rc,
> +		(rc & LPC_HICRB_ILPC_DISABLE));
> +	return (rc & LPC_HICRB_ILPC_DISABLE);

As Oliver points out your change inverts the condition - this means the function
will indicate to the caller that the iLPC2AHB bridge is RW capable when in fact
it is configured as RO (and RO when it is RW). This isn't what we want :)

Andrew


More information about the Skiboot mailing list