Re: [PATCH v2 5/6] x86/microcode/intel: Support mailbox transfer
From: Dave Hansen
Date: Mon Mar 31 2025 - 15:16:20 EST
On 3/26/25 20:32, Chao Gao wrote:
> static enum ucode_state wait_for_transaction(void)
> {
> u32 timeout, status;
>
> /* Allow time for hardware to complete the operation: */
> for (timeout = 0; timeout < MBOX_XACTION_TIMEOUT_MS; timeout++) {
> msleep(1);
>
> status = readl(staging.mmio_base + MBOX_STATUS_OFFSET);
>
> if (status & MASK_MBOX_STATUS_READY)
> return UCODE_OK;
>
> /* Check for explicit error response */
> if (status & MASK_MBOX_STATUS_ERROR)
> return UCODE_ERROR;
> }
>
> /*
> * Hardware is neither responded to the action nor
> * signaled any error. Treat the case as timeout.
> */
> return UCODE_TIMEOUT;
> }
The flow at minimal indent in the function should be the _main_ control
flow: the common case.
The common case here is the "return UCODE_OK". It should not be buried
in a for() loop. I kinda like what Chang had before.