From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: None (no SPF record) identity=mailfrom; client-ip=2607:f8b0:4001:c06::22f; helo=mail-io0-x22f.google.com; envelope-from=mw@semihalf.com; receiver=edk2-devel@lists.01.org Received: from mail-io0-x22f.google.com (mail-io0-x22f.google.com [IPv6:2607:f8b0:4001:c06::22f]) (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 2217A20352609 for ; Thu, 26 Oct 2017 06:52:01 -0700 (PDT) Received: by mail-io0-x22f.google.com with SMTP id 189so5516840iow.10 for ; Thu, 26 Oct 2017 06:55:47 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=semihalf-com.20150623.gappssmtp.com; s=20150623; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=17Otbqbzk1vvn+mt4M92hAAts6SVf9ZWQ259n4w7ETo=; b=C3s8eZREP4GFXNwoxQKJGIt89hEDHLHxcbVeQGIYZHen5N6RbSpuYT2wLTp298WMar rZXqzTG+JUegWYmYBzacgdfUu2DEqYIx0oPAhLQH739Uoal2CREOLLOJfTubZXiPct17 EXDFtyNKHoJVWWT+c+zvfebAEz+bcypXJ/3PVjpsE6dDpyJrWAOo1F5pCvlGhhlWk3gQ hWwBg1ouRY6VvMTqzxhllQaVTAoTEONVBC1dFYs71u94jImC9n6IplGovd6pyWv9+q8k KLfulaA3RUrMv3/BAfCOqPBpkJ7f7G+I0JO0LUIYMhS9GaLMWCSdU9xjweoBaY8+HawN Erjw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=17Otbqbzk1vvn+mt4M92hAAts6SVf9ZWQ259n4w7ETo=; b=qne++qfQ+z4jtkt/Ar2f+tPEr1mm30/vXPT8S5xGwFZfSouh8mJWNQp/Cr3EF6nHoG G3nn4PQklnk3KZuWwXh0DHseJwrIa00CyJt9BhRDY5f85RXntPxQHN5L0fhHzCK0iPXG PUNptLbQk6obr3hhBESZVYvkcZ4hR13m+upJnYQ/lMOHZ3k7yjp66FBGXp4QkB7Q7pfY SrjTQiQPPz15ilLBI6ZjsyBruINYT2m13f/YgERc4kVKGJjsHPm7hVXxXU0Mp8B3q0sj 7/p0l86dE+teyUu3MhYfE54pmBPLhBHU58SLGbdx3Eq3zx8EM9itnsmYdXOaFpiGjhvE tKsA== X-Gm-Message-State: AMCzsaVcp82HWBQqmrRxhWFXKMwtqzH2R4QPkrL8btAq1lmTFaE8AjJD dfkQAUd84E+z8Smzhih2qyCUAPCqeILeZ5blmcpM7w== X-Google-Smtp-Source: ABhQp+Sj0YcbP/iRIMOSkjg6NRJVRWGn0BJDqTD6o8/EAVodzHGXhBlgOjUFlmSF9tH/SVv4dNTaJqG2X/WJYLg1iCc= X-Received: by 10.107.68.10 with SMTP id r10mr30718257ioa.202.1509026146726; Thu, 26 Oct 2017 06:55:46 -0700 (PDT) MIME-Version: 1.0 Received: by 10.107.167.208 with HTTP; Thu, 26 Oct 2017 06:55:45 -0700 (PDT) In-Reply-To: <20171026135415.yui6fvz44je334b2@bivouac.eciton.net> References: <1508980777-29006-1-git-send-email-mw@semihalf.com> <1508980777-29006-2-git-send-email-mw@semihalf.com> <20171026125143.bki4fywgtf3ybvmm@bivouac.eciton.net> <20171026135415.yui6fvz44je334b2@bivouac.eciton.net> From: Marcin Wojtas Date: Thu, 26 Oct 2017 15:55:45 +0200 Message-ID: To: Leif Lindholm Cc: edk2-devel-01 , Ard Biesheuvel , nadavh@marvell.com, Neta Zur Hershkovits , Kostya Porotchkin , Hua Jing , semihalf-dabros-jan , David Greeson Subject: Re: [platforms: PATCH 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: Thu, 26 Oct 2017 13:52:01 -0000 Content-Type: text/plain; charset="UTF-8" 2017-10-26 15:54 GMT+02:00 Leif Lindholm : > On Thu, Oct 26, 2017 at 03:19:36PM +0200, Marcin Wojtas wrote: >> Hi Leif, >> >> 2017-10-26 14:51 GMT+02:00 Leif Lindholm : >> > On Thu, Oct 26, 2017 at 03:19:28AM +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. >> >> >> >> Contributed-under: TianoCore Contribution Agreement 1.1 >> >> Signed-off-by: David Greeson >> >> [Style adjustment and cleanup] >> >> Signed-off-by: Marcin Wojtas >> >> --- >> >> Platform/Marvell/Drivers/I2c/MvI2cDxe/MvI2cDxe.c | 34 +++++++++++++++++--- >> >> 1 file changed, 29 insertions(+), 5 deletions(-) >> >> >> >> diff --git a/Platform/Marvell/Drivers/I2c/MvI2cDxe/MvI2cDxe.c b/Platform/Marvell/Drivers/I2c/MvI2cDxe/MvI2cDxe.c >> >> index d85ee0b..7faf1f7 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,35 +575,58 @@ MvI2cStartRequest ( >> >> ReadMode = Operation->Flags & I2C_FLAG_READ; >> >> >> >> if (Count == 0) { >> >> - MvI2cStart ( I2cMasterContext, >> >> + Status = MvI2cStart (I2cMasterContext, >> >> (SlaveAddress << 1) | ReadMode, >> >> I2C_TRANSFER_TIMEOUT >> > >> > Much as I appreciate seeing this form of the code, since it simplifies >> > seeing the functional changes, this does cause those lines left >> > unchanges to no longer conform to coding style. >> > Can you please adjust throughout for a v2? >> > >> >> No problem. I of course saw style violations, but I gave up on them >> for "no mix of functional improvements and style cleanups" contraint >> :) I will correct the modified function calls. > > Clarification: this is and has always been _unrelated_ style cleanups. > Any statement that is actually being modified should be conformant > afterwards. > Ok, thanks for clarification. Marcin