From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wr1-f50.google.com (mail-wr1-f50.google.com [209.85.221.50]) by mx.groups.io with SMTP id smtpd.web10.26907.1637948871217477169 for ; Fri, 26 Nov 2021 09:47:51 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@nuviainc-com.20210112.gappssmtp.com header.s=20210112 header.b=MEPJngiq; spf=pass (domain: nuviainc.com, ip: 209.85.221.50, mailfrom: leif@nuviainc.com) Received: by mail-wr1-f50.google.com with SMTP id o13so20165942wrs.12 for ; Fri, 26 Nov 2021 09:47:50 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=nuviainc-com.20210112.gappssmtp.com; s=20210112; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=vzGtjR1EIF2MNGLDZk3f3v2p3WkivURlcArGy7eJdkg=; b=MEPJngiqp2kB4FSiQ1oJE3m5iKVFgNd4MNuFFrnMTQxoN4fXjvl9q3p3IiYLh35ZZA Qb1DLQSSFeS6ETn2tsJbpMDTT/3hzSQqqq+7gqbLbNrqXI1fxSg5aLeVnD4KWGkoT8vd GueL43WZ1vFDNzv5ZqO3X8LkhfE30zLk8diVUKTqRSZZEvoSUrcnth0BPyMz/pXJFBGH yc9Rs4cn5g+Y6bhgSbqxLbbE6p/6QzbULTjbJjtuySPwnr+Pco7+/geob6koTUj1O5jK Dtt6U5LUobn+m1ErMe1syY8mqML14Bh5v4V16D5w55jKalz49DjSheOb/YboQwuqTUrW rUaQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=vzGtjR1EIF2MNGLDZk3f3v2p3WkivURlcArGy7eJdkg=; b=xYxxG85SXdkhdl5Xy9nAzVHnuWG/TDqxaSW0bvS0Nfy3ENiGt1v5m+pCpDaa3L8wZB WJbdcvG6Mikj9ua8N9lkRbLrvw3WIqRg4rGqP1lJ39SqXkl9Ag7GpJkWTnqz5T6MrP0K yyR9/42j3YJP0+G8R/M5l1UMJkeZbbeQmIvBuTYfHJqae1Q5jZO1jzD+HunKbpWF1mS+ qQP5rebZrlIMHtWyGmEcvH3D7I9jBFdFOqdjCllasKTY08js4Z8aGvmyiR25rgeBVski 1ndQ4pss3eHNeP0EmaZW5Ib/O/Emia53hncWzNJDhfZav7AEo3I/qX8+jKjJTm+MFloe IMhw== X-Gm-Message-State: AOAM533ulflJMelf/CIzT/f6CADPLTUhai5v7ZY+n0OOxhKXFfiZ+HWD F8sUcSoi/cU6dSH0Hta553QR9w== X-Google-Smtp-Source: ABdhPJwUb34GvFL4NGO10+KQ5GypArW9a0YGEFRSQ4TLnoVKpuRmokUZpa0HkVaSMAZBP8bk0UBPVA== X-Received: by 2002:a5d:628f:: with SMTP id k15mr16204001wru.363.1637948869664; Fri, 26 Nov 2021 09:47:49 -0800 (PST) Return-Path: Received: from leviathan (cpc92314-cmbg19-2-0-cust559.5-4.cable.virginm.net. [82.11.186.48]) by smtp.gmail.com with ESMTPSA id l7sm7743802wry.86.2021.11.26.09.47.48 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 26 Nov 2021 09:47:49 -0800 (PST) Date: Fri, 26 Nov 2021 17:47:47 +0000 From: "Leif Lindholm" To: Masami Hiramatsu Cc: Ard Biesheuvel , devel@edk2.groups.io, Kazuhiko Sakamoto , Masahisa Kojima Subject: Re: [PATCH 1/5] [RESEND][edk2-platforms] Silicon/SynQuacerI2cDxe: Wait for bus busy Message-ID: References: <163610419943.391624.9289897029386201296.stgit@localhost> <163610420797.391624.17492204385268340229.stgit@localhost> MIME-Version: 1.0 In-Reply-To: <163610420797.391624.17492204385268340229.stgit@localhost> Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Fri, Nov 05, 2021 at 18:23:28 +0900, Masami Hiramatsu wrote: > If an EFI application frequently repeats SetTime and GetTime, > the I2C bus can be busy and failed to start. To fix this issue, > add waiting loop for the bus busy status. (Usually, it is > enough to read 3 times for checking, but for safety this > sets 10 for timeout.) > > This also clean up the code path a bit so that it is easy to > understand what should do on each combinations of BSR.BB and > BCR.MSS. > > Signed-off-by: Masami Hiramatsu > Reported-by: Kazuhiko Sakamoto > --- > .../Drivers/SynQuacerI2cDxe/SynQuacerI2cDxe.c | 38 ++++++++++++++------ > 1 file changed, 26 insertions(+), 12 deletions(-) > > diff --git a/Silicon/Socionext/SynQuacer/Drivers/SynQuacerI2cDxe/SynQuacerI2cDxe.c b/Silicon/Socionext/SynQuacer/Drivers/SynQuacerI2cDxe/SynQuacerI2cDxe.c > index 31f6e3072f..380eba8059 100644 > --- a/Silicon/Socionext/SynQuacer/Drivers/SynQuacerI2cDxe/SynQuacerI2cDxe.c > +++ b/Silicon/Socionext/SynQuacer/Drivers/SynQuacerI2cDxe/SynQuacerI2cDxe.c > @@ -16,6 +16,8 @@ > // > #define WAIT_FOR_INTERRUPT_TIMEOUT 50000 > > +#define WAIT_FOR_BUS_BUSY_TIMEOUT 10 > + I think it would be more clear English to say that we are waiting _for_ the bus to be *ready* - meaning that we are waiting _while_ the bus is *busy*. So I suggest WAIT_FOR_BUS_BUSY_TIMEOUT -> WAIT_FOR_BUS_READY_TIMEOUT > /** > Set the frequency for the I2C clock line. > > @@ -152,6 +154,7 @@ SynQuacerI2cMasterStart ( > IN EFI_I2C_OPERATION *Op > ) > { > + UINTN Timeout = WAIT_FOR_BUS_BUSY_TIMEOUT; This indentation does not match the subsequent lines. / Leif > UINT8 Bsr; > UINT8 Bcr; > > @@ -167,24 +170,35 @@ SynQuacerI2cMasterStart ( > Bsr = MmioRead8 (I2c->MmioBase + F_I2C_REG_BSR); > Bcr = MmioRead8 (I2c->MmioBase + F_I2C_REG_BCR); > > - if ((Bsr & F_I2C_BSR_BB) && !(Bcr & F_I2C_BCR_MSS)) { > - DEBUG ((DEBUG_INFO, "%a: bus is busy\n", __FUNCTION__)); > - return EFI_ALREADY_STARTED; > - } > + if (!(Bcr & F_I2C_BCR_MSS)) { > > - if (Bsr & F_I2C_BSR_BB) { // Bus is busy > - DEBUG ((DEBUG_INFO, "%a: Continuous Start\n", __FUNCTION__)); > - MmioWrite8 (I2c->MmioBase + F_I2C_REG_BCR, Bcr | F_I2C_BCR_SCC); > - } else { > - if (Bcr & F_I2C_BCR_MSS) { > - DEBUG ((DEBUG_WARN, > - "%a: is not in master mode\n", __FUNCTION__)); > - return EFI_DEVICE_ERROR; > + if (Bsr & F_I2C_BSR_BB) { // Bus is busy > + do { > + Bsr = MmioRead8 (I2c->MmioBase + F_I2C_REG_BSR); > + } while (Timeout-- && (Bsr & F_I2C_BSR_BB)); > + > + if (Bsr & F_I2C_BSR_BB) { > + DEBUG ((DEBUG_INFO, "%a: bus is busy\n", __FUNCTION__)); > + return EFI_ALREADY_STARTED; > + } > } > + > DEBUG ((DEBUG_INFO, "%a: Start Condition\n", __FUNCTION__)); > MmioWrite8 (I2c->MmioBase + F_I2C_REG_BCR, > Bcr | F_I2C_BCR_MSS | F_I2C_BCR_INTE | F_I2C_BCR_BEIE); > + > + } else { // F_I2C_BCR_MSS is set > + > + if (!(Bsr & F_I2C_BSR_BB)) { > + DEBUG ((DEBUG_WARN, > + "%a: is not in master mode\n", __FUNCTION__)); > + return EFI_DEVICE_ERROR; > + } > + > + DEBUG ((DEBUG_INFO, "%a: Continuous Start\n", __FUNCTION__)); > + MmioWrite8 (I2c->MmioBase + F_I2C_REG_BCR, Bcr | F_I2C_BCR_SCC); > } > + > return EFI_SUCCESS; > } > >