From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=2a00:1450:400c:c09::244; helo=mail-wm0-x244.google.com; envelope-from=leif.lindholm@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-wm0-x244.google.com (mail-wm0-x244.google.com [IPv6:2a00:1450:400c:c09::244]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 8856F2034A884 for ; Fri, 27 Oct 2017 07:25:11 -0700 (PDT) Received: by mail-wm0-x244.google.com with SMTP id z3so4166763wme.5 for ; Fri, 27 Oct 2017 07:28:58 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=RtkCCFsP3fYWZmkmn2USkkZFb+enBatqB7xspaHkDL8=; b=iYMyfM0idfvlv0W8b3bx4VRHA0tJKi8DMHRtLInL99WFD27GDOYoYz/+6mCaDvAzAN crfsCqONG5XNbttmGFlJxbak+DEw6Q5cb1k21Mm78S5s9YkKyoYMlypNXFMLUblgxH7T SZIUJnSP8mtds+ZA0ZzzrIAQdkI08bk+bBWd0= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=RtkCCFsP3fYWZmkmn2USkkZFb+enBatqB7xspaHkDL8=; b=SIkSk+MuB79rf2vJOx6Syrn+GpXMwBK1y374au/DiJAclmIpdkIFNu74nzq0BRzCkI EsjowJT2+iexHPfB3WLH/q+EYDCOhfVhPtX3Pdo2mPJewlVDjRSz/3Teu2Z7K5LFIPC4 hly0gbOrm7U0R83+S6S/cmpGe29PfnrSJlY6BCbU6Hzbr4EnvrOtXo/nGtT1hAhYPShW ohzksdWzmhm4IvmmYSumrE8NbT4+VnC3jZFy3XCgfh1gCFbwHDGv/AXpYTo5xiZww9fT 9IetXpS0yhSlBrk44yFJ4vDPYxNnYS6XTM8JBvhm1+NwOuR28QWnTf6c7nQjvqfzmPYC GBww== X-Gm-Message-State: AMCzsaXKRbYZRUW1L9FY78uBHFa64UNOkLRIh3IxoMd3jYS6fKDAy0gK kaEAERWZHGmldJgalGILtIKt6Q== X-Google-Smtp-Source: ABhQp+TLUwHYJbMN1HSldjAg+WLVvWCe7/EwksT4OUzjMwq+QUFZjYrzYd0xIGiuhND86+ldGwQWng== X-Received: by 10.28.26.11 with SMTP id a11mr644365wma.90.1509114537260; Fri, 27 Oct 2017 07:28:57 -0700 (PDT) Received: from bivouac.eciton.net (bivouac.eciton.net. [2a00:1098:0:86:1000:23:0:2]) by smtp.gmail.com with ESMTPSA id j27sm6459812wrd.42.2017.10.27.07.28.55 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Fri, 27 Oct 2017 07:28:56 -0700 (PDT) Date: Fri, 27 Oct 2017 15:28:54 +0100 From: Leif Lindholm To: Marcin Wojtas Cc: edk2-devel@lists.01.org, ard.biesheuvel@linaro.org, nadavh@marvell.com, neta@marvell.com, kostap@marvell.com, jinghua@marvell.com, jsd@semihalf.com, David Greeson Message-ID: <20171027142854.wpcx6tcpel3nxteq@bivouac.eciton.net> References: <1509066832-5285-1-git-send-email-mw@semihalf.com> <1509066832-5285-2-git-send-email-mw@semihalf.com> MIME-Version: 1.0 In-Reply-To: <1509066832-5285-2-git-send-email-mw@semihalf.com> User-Agent: NeoMutt/20170113 (1.7.2) Subject: Re: [platforms: PATCH v2 01/10] Marvell/Drivers: MvI2cDxe: Abort transaction immediately upon fail X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 27 Oct 2017 14:25:11 -0000 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Fri, Oct 27, 2017 at 03:13:43AM +0200, Marcin Wojtas wrote: > From: David Greeson > > Although the I2C transaction routines were prepared to > return their status, they were never used. This could > cause bus lock-up e.g. in case of failing to send a > slave address, the data transfer was attempted to be > continued anyway. > > This patch fixes faulty behavior by checking transaction > status and stopping it immediately, once the fail > is detected. On the occasion fix style around modified > functions calls. > > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: David Greeson > [Style adjustment and cleanup] > Signed-off-by: Marcin Wojtas Reviewed-by: Leif Lindholm > --- > Platform/Marvell/Drivers/I2c/MvI2cDxe/MvI2cDxe.c | 62 +++++++++++++------- > 1 file changed, 41 insertions(+), 21 deletions(-) > > diff --git a/Platform/Marvell/Drivers/I2c/MvI2cDxe/MvI2cDxe.c b/Platform/Marvell/Drivers/I2c/MvI2cDxe/MvI2cDxe.c > index d85ee0b..b4599d2 100755 > --- a/Platform/Marvell/Drivers/I2c/MvI2cDxe/MvI2cDxe.c > +++ b/Platform/Marvell/Drivers/I2c/MvI2cDxe/MvI2cDxe.c > @@ -565,6 +565,7 @@ MvI2cStartRequest ( > UINTN Transmitted; > I2C_MASTER_CONTEXT *I2cMasterContext = I2C_SC_FROM_MASTER(This); > EFI_I2C_OPERATION *Operation; > + EFI_STATUS Status = EFI_SUCCESS; > > ASSERT (RequestPacket != NULL); > ASSERT (I2cMasterContext != NULL); > @@ -574,33 +575,52 @@ MvI2cStartRequest ( > ReadMode = Operation->Flags & I2C_FLAG_READ; > > if (Count == 0) { > - MvI2cStart ( I2cMasterContext, > - (SlaveAddress << 1) | ReadMode, > - I2C_TRANSFER_TIMEOUT > - ); > + Status = MvI2cStart (I2cMasterContext, > + (SlaveAddress << 1) | ReadMode, > + I2C_TRANSFER_TIMEOUT); > } else if (!(Operation->Flags & I2C_FLAG_NORESTART)) { > - MvI2cRepeatedStart ( I2cMasterContext, > - (SlaveAddress << 1) | ReadMode, > - I2C_TRANSFER_TIMEOUT > - ); > + Status = MvI2cRepeatedStart (I2cMasterContext, > + (SlaveAddress << 1) | ReadMode, > + I2C_TRANSFER_TIMEOUT); > } > > + /* I2C transaction was aborted, so stop further transactions */ > + if (EFI_ERROR (Status)) { > + MvI2cStop (I2cMasterContext); > + break; > + } > + > + /* > + * If sending the slave address was successful, > + * proceed to read or write section. > + */ > if (ReadMode) { > - MvI2cRead ( I2cMasterContext, > - Operation->Buffer, > - Operation->LengthInBytes, > - &Transmitted, > - Count == 1, > - I2C_TRANSFER_TIMEOUT > - ); > + Status = MvI2cRead (I2cMasterContext, > + Operation->Buffer, > + Operation->LengthInBytes, > + &Transmitted, > + Count == 1, > + I2C_TRANSFER_TIMEOUT); > + Operation->LengthInBytes = Transmitted; > } else { > - MvI2cWrite ( I2cMasterContext, > - Operation->Buffer, > - Operation->LengthInBytes, > - &Transmitted, > - I2C_TRANSFER_TIMEOUT > - ); > + Status = MvI2cWrite (I2cMasterContext, > + Operation->Buffer, > + Operation->LengthInBytes, > + &Transmitted, > + I2C_TRANSFER_TIMEOUT); > + Operation->LengthInBytes = Transmitted; > } > + > + /* > + * The I2C read or write transaction failed. > + * Stop the I2C transaction. > + */ > + if (EFI_ERROR (Status)) { > + MvI2cStop (I2cMasterContext); > + break; > + } > + > + /* Check if there is any more data to be sent */ > if (Count == RequestPacket->OperationCount - 1) { > MvI2cStop ( I2cMasterContext ); > } > -- > 2.7.4 >