From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from EUR05-DB8-obe.outbound.protection.outlook.com (EUR05-DB8-obe.outbound.protection.outlook.com [40.107.20.81]) by mx.groups.io with SMTP id smtpd.web11.4626.1586417976554285161 for ; Thu, 09 Apr 2020 00:39:37 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@nxp.com header.s=selector2 header.b=kV6hkOAO; spf=pass (domain: nxp.com, ip: 40.107.20.81, mailfrom: v.sethi@nxp.com) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=mFD/P5IobeD2s/3eW/6KrP8abVLpHfsVc9PKihiDjeiu72eAlgSXSFUVhyMOUGepDYWjO4JkO71PLy0uX3jhJWXuRk2mz0PF6tX6kl1y7A0lUAfWgAHKbDQUYNIrE8DdPNmPj5abmJndrVezJ4qEwNtGkFYmbxoeG2S6cFmuWjf4Y2kYOzz7AoP/bB7l3d8M5ofSfU8aF5vK8MN4IvVnFcqOi987SBdub8kWedydlG43xsncvlXa2ja6g5yOprce+znxjt1uGI0d/EfP3QrL6gqH9NHciRjn7AOEwOkZ2s2up0Cim1ZREDGANT0lUriXEAB5Wa5qPVgkpVCWwPxRxw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=OcW/9u09Ydje0cqIIxTNwUSxGxXgwO6BxK+b7j+syF4=; b=SFKbxVlawco6I/WZZ5EXAAyIJJrB/U+jpYeG4K4pqZgIkFt9Ppd0tD11Gh6Iz6Jor43evkeQQDyHj+oBwyZMUYFkEaBRp07hYXalbrbBk7v3m23exa92HhHtV9cZ96qDXoNhNBUUCwSN6zovAf3fq+P4AmLMr7UgV2JuIkHN+x9VW5v9hdjObz23gAbIdUW2SgNmJNYyo4KX8TCYdH6kn2e2+cuY99xF43wOYagC/VmsVH6yPVDpsj2QN+ZBbnSazXu4AkvjdpWjvmsdvXU8rs0DN7cxS1pYyHjJhptKibZBmt3Q3LaNOKBuazseQYTjg6JMwV5eAczCs1yaC8DrQA== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=nxp.com; dmarc=pass action=none header.from=nxp.com; dkim=pass header.d=nxp.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=nxp.com; s=selector2; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=OcW/9u09Ydje0cqIIxTNwUSxGxXgwO6BxK+b7j+syF4=; b=kV6hkOAOewavuwvw/aRyQbvmDgl8CQjBfIAqLservW/FJtD5vTP6q+zDweZHTFJNdLzk6bdfi4g437P00M+hZ5LIs4/KKnUhG1jhuO/qAIBwje/ForV6IlLb+cWIRCElQLBqRxz69E1jObRNHr9rNGF1UbE1xv/Si4gF5X63Gfo= Received: from VI1PR04MB4592.eurprd04.prod.outlook.com (2603:10a6:803:75::31) by VI1PR04MB6016.eurprd04.prod.outlook.com (2603:10a6:803:d3::13) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.2900.15; Thu, 9 Apr 2020 07:39:34 +0000 Received: from VI1PR04MB4592.eurprd04.prod.outlook.com ([fe80::4ce4:6749:2a21:f14c]) by VI1PR04MB4592.eurprd04.prod.outlook.com ([fe80::4ce4:6749:2a21:f14c%7]) with mapi id 15.20.2878.022; Thu, 9 Apr 2020 07:39:34 +0000 From: Varun Sethi To: Leif Lindholm , "Pankaj Bansal (OSS)" CC: Meenakshi Aggarwal , Michael D Kinney , "devel@edk2.groups.io" , Samer El-Haj-Mahmoud , Jon Nettleton Subject: Re: [EXT] Re: [PATCH v2 01/28] Silicon/NXP: Add I2c lib Thread-Topic: [EXT] Re: [PATCH v2 01/28] Silicon/NXP: Add I2c lib Thread-Index: AQHWC9qofxTbXDAoe0WBo+1ggbvySahr8MGAgAR4GyA= Date: Thu, 9 Apr 2020 07:39:34 +0000 Message-ID: References: <20200320143543.18615-1-pankaj.bansal@oss.nxp.com> <20200320143543.18615-2-pankaj.bansal@oss.nxp.com> <20200331115114.GD7468@vanye> <20200406111217.GB14075@vanye> In-Reply-To: <20200406111217.GB14075@vanye> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: spf=none (sender IP is ) smtp.mailfrom=V.Sethi@nxp.com; x-originating-ip: [122.160.41.82] x-ms-publictraffictype: Email x-ms-office365-filtering-ht: Tenant x-ms-office365-filtering-correlation-id: 9359ac0d-08a1-429f-bcbe-08d7dc5925f4 x-ms-traffictypediagnostic: VI1PR04MB6016:|VI1PR04MB6016: x-ms-exchange-transport-forked: True x-microsoft-antispam-prvs: x-ms-oob-tlc-oobclassifiers: OLM:843; x-forefront-prvs: 0368E78B5B x-forefront-antispam-report: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:VI1PR04MB4592.eurprd04.prod.outlook.com;PTR:;CAT:NONE;SFTY:;SFS:(10009020)(4636009)(39860400002)(346002)(396003)(366004)(136003)(376002)(66446008)(5660300002)(64756008)(76116006)(8936002)(316002)(52536014)(66946007)(53546011)(81156014)(71200400001)(66556008)(66476007)(6506007)(7696005)(54906003)(110136005)(4326008)(478600001)(2906002)(33656002)(81166007)(186003)(8676002)(86362001)(26005)(55016002)(9686003);DIR:OUT;SFP:1101; received-spf: None (protection.outlook.com: nxp.com does not designate permitted sender hosts) x-ms-exchange-senderadcheck: 1 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: +y94srndaf7ueeUKKX/uqmVcXAk2ooqTrQdlXtRUtI6SHmxcZy8hxGgKt+QPnaI912Kn/d6DFBTt4gg8/1ReQn23CWXaSw37vyKwzUbRZ25rAAEezGZZ+2qGKOyIKoW4HnfMSfJ8Ci+vDidJHDm2ziw67ibSB5mfAZIE5ezq4/R8u0XNBpvWmwMBzDopcW0uQ5PzVrynDV6Ql8uSVVUc8VITqCAqzodqRCQneBSphAGvRdPy9oDLccE46swQqNSdxMGJ2PzaP/kAtK8zyYTlAz0coGUOei//5Ow2/LIpd5SOlUpiExD9pPLS/x0imY7B68YZ6/Qo56zELI4G7YIfJORgx8x7EJOZbmk9iGOkGASRJtKqfQavD/4opvB6pk3OJ736gaXLyqyS8Jqrc3s4zz2CkZKdRaiR0p5QtJhZxQGGTahiEAUKsuVpgAmTfT09 x-ms-exchange-antispam-messagedata: oZkMaY1npxtW8+5EHeiDuCPnv6iu0bSo3+iqmhYLAFi3RJXcXNCrLn/fBcUqCkLb8g9FL046h2s2gDXiT1zon9t7joacZBDg5h5ouN/boPjKpZPI2vj+cdwVJn2xNqXEhpQPIB69u7VS/ryodiFl3Q== MIME-Version: 1.0 X-OriginatorOrg: nxp.com X-MS-Exchange-CrossTenant-Network-Message-Id: 9359ac0d-08a1-429f-bcbe-08d7dc5925f4 X-MS-Exchange-CrossTenant-originalarrivaltime: 09 Apr 2020 07:39:34.2081 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 686ea1d3-bc2b-4c6f-a92c-d99c5c301635 X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-CrossTenant-userprincipalname: Jnf9dLH8GXZGp1DQHxjkpSA/sLQegSBCAYSD0EXxFzEZ6J6FtK9dTwXJjB2LS6W1 X-MS-Exchange-Transport-CrossTenantHeadersStamped: VI1PR04MB6016 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Hi Leif, Please find my response inline. Regards Varun -----Original Message----- From: Leif Lindholm =20 Sent: Monday, April 6, 2020 4:42 PM To: Pankaj Bansal (OSS) Cc: Meenakshi Aggarwal ; Michael D Kinney ; devel@edk2.groups.io; Varun Sethi ; Samer El-Haj-Mahmoud ; Jon Nettleton Subject: [EXT] Re: [PATCH v2 01/28] Silicon/NXP: Add I2c lib Caution: EXT Email On Mon, Apr 06, 2020 at 06:14:38 +0000, Pankaj Bansal (OSS) wrote: > > > > -----Original Message----- > > From: Leif Lindholm > > Sent: Tuesday, March 31, 2020 5:21 PM > > To: Pankaj Bansal (OSS) > > Cc: Meenakshi Aggarwal ; Michael D=20 > > Kinney ; devel@edk2.groups.io; Varun=20 > > Sethi ; Samer El-Haj-Mahmoud > Mahmoud@arm.com>; Jon Nettleton > > Subject: Re: [PATCH v2 01/28] Silicon/NXP: Add I2c lib > > > > On Fri, Mar 20, 2020 at 20:05:16 +0530, Pankaj Bansal wrote: > > > From: Pankaj Bansal > > > > > > I2c lib is going to be used in PrePeiCore sec module to get the=20 > > > System clock information from devices connected to i2c (like fpga=20 > > > or clock generator) > > > > > > since we don't have support of DXE modules this early in boot=20 > > > stage, move the i2c controller functionality in library. > > > > This isn't moving the functionality to a library though - it is=20 > > moving the functionality to a library *and* adding new features.=20 > > These are two separate changes that should be two separate patches. > > > > The content in this patch is mostly fine as the end result (but some=20 > > comments below). > > > > I suggest this patch is reordered with 2/28 and all of the splitting=20 > > out part takes place in that patch. This patch can then be reduced=20 > > to ... the bits that are currently impossible to see are changed (at=20 > > least the glitch fixing). > > Actually the I2cLib is not some bug fix over I2cDxe. > The I2cLib has been completely re-written. > This is because, I2cDxe was written assuming that the I2c transaction=20 > would always be of (reg + data) type only (i.e. two operations) . > And also the Repeat start condition was not generated In the I2cDxe drive= r. > > This caused the I2c Peripheral drivers which were written keeping the=20 > controller driver In mind to issue two operations. Which caused bug in Pc= f2129 RTC driver, that I am fixing in patch 3. > > Now I have removed these assumptions as well as added Repeat start=20 > between successive operations, which Comply with PI I2c spec. > > So, it would be difficult for me to merge the 1 and 2. >OK, that's fine[1] then, but that means that this commit message is mislea= ding. Please rewrite it to reflect the actual situation. >(And address the remaining comments on this.) >[1] It's not "fine" - this means we're ripping out and replacing even > more code that was just merged. This pattern needs to stop. I agree Leif, we will take care of this going forward. Some of the modifica= tions (as per the patchset) were done by Pankaj after the initial patches g= ot merged. We thought that it would be a good idea to post these patches in= oneshot. We will re-visit our upstream patch submission strategy and make = sure that there are no conflicts going forward. We are already aligning ups= tream patches for other platforms with Pankaj's patchset. =20 Really appreciate your help.