On 21 February 2018 at 10:24, Leif Lindholm <[email protected]> wrote: > On Wed, Feb 21, 2018 at 10:19:18AM +0000, Ard Biesheuvel wrote: >> Currently, SynQuacerI2cStartRequest() increases the TPL to TPL_HIGH_LEVEL >> while accessing the I2C controller hardware, but fails to restore the TPL >> to the original level if the call to SynQuacerI2cMasterStart() fails, and >> returns right away. Given the TPL_HIGH_LEVEL implies that interrupts are >> disabled, this results in a complete system hang. So instead, break out >> of the loop, so that the TPL restore will occur before leaving the function. >> >> Contributed-under: TianoCore Contribution Agreement 1.1 >> Signed-off-by: Ard Biesheuvel <[email protected]> >> --- >> Silicon/Socionext/SynQuacer/Drivers/SynQuacerI2cDxe/SynQuacerI2cDxe.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git >> a/Silicon/Socionext/SynQuacer/Drivers/SynQuacerI2cDxe/SynQuacerI2cDxe.c >> b/Silicon/Socionext/SynQuacer/Drivers/SynQuacerI2cDxe/SynQuacerI2cDxe.c >> index 46c512a20151..b2318a6f5a8c 100644 >> --- a/Silicon/Socionext/SynQuacer/Drivers/SynQuacerI2cDxe/SynQuacerI2cDxe.c >> +++ b/Silicon/Socionext/SynQuacer/Drivers/SynQuacerI2cDxe/SynQuacerI2cDxe.c >> @@ -324,7 +324,7 @@ SynQuacerI2cStartRequest ( >> >> Status = SynQuacerI2cMasterStart (I2c, SlaveAddress, Op); >> if (EFI_ERROR (Status)) { >> - return Status; >> + break; >> } >> >> Status = WaitForInterrupt (I2c); > > This change also causes > > // Stop the transfer > MmioWrite8 (I2c->MmioBase + F_I2C_REG_BCR, 0); > > to be executed in the faulting case. > > Which at the very least looks quirky. >
I don't think it is unreasonable in general to stop any ongoing transfer no matter how we leave this function. And in this particular case, one of the failure modes is F_I2C_BCR_MSS being set without the bus being busy as a result, and so clearing BCR is actually rather appropriate here. _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel

