From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from EUR03-DB5-obe.outbound.protection.outlook.com (EUR03-DB5-obe.outbound.protection.outlook.com [40.107.4.82]) by mx.groups.io with SMTP id smtpd.web10.1024.1587711198081386733 for ; Thu, 23 Apr 2020 23:53:18 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@nxp1.onmicrosoft.com header.s=selector2-nxp1-onmicrosoft-com header.b=eb/cZmWi; spf=pass (domain: oss.nxp.com, ip: 40.107.4.82, mailfrom: pankaj.bansal@oss.nxp.com) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=H1Ngt0jyAe3YzlciiIoxEH+QPEMEJixJpSDrR0cnvUzUzW/3q4lqoUkUZYOOvF96LC5cxj4I+70nA1s9l6K7E9lCgzYRUC9KPL+hMCr7Tv2bH6HfXMCbSP+W8kbRc3AZp2cYZ72l+EyhJfMzahUx1nj8BYTK7UosI/BntD8sFubToOwo/c+gbCXUPjJCqqShtdo19AbAKMHG+layt7vMIgb8gqMn2m0b4zS/4k7YJlu2NcW8Jx2k0skUFO/AheEw7mmEx2ylTl+n2spouw3zscBuPt3foS1Zh1tsgpy7dZ47Zi7TksgE1sGCGqfyGMJuLJ8HlAN8vzZPj8980W/swA== 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=6a/Wg+nx7PdPzro25+nybgKXtVuoZak7Y/3sB0cXEW8=; b=cDL10P+hdiD8JCxbcA787bszaQqW7nQcNj7gt41EuB+38Qy++z6DTzbYfKmAC3gMIiA0YnfPG4OR+bCuDzYWw8t8H1gYNB53LbxN7pfHFj8sOTP+i9+sCzZcUd4su7xtja5rn4DJTcXZzIl8mpf4/V7x6vbr54M1sAlgTicZ9qGLeNf7qA1i09cNVZLKga5IjjlVpSwRZRAUp+2z6kHnph3tfpuKOcHHU1siEKVcNXA96Mqci7i1zFc6VdO4EwGbbhgJ6zWKiMvKpbuEykkIN47Y3eugneSlGfrPnuhhmQ8C1LcrIgE9a9PM0EHlXkVbWQ9VFH9eLxBTW028EvsGdw== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=oss.nxp.com; dmarc=pass action=none header.from=oss.nxp.com; dkim=pass header.d=oss.nxp.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=NXP1.onmicrosoft.com; s=selector2-NXP1-onmicrosoft-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=6a/Wg+nx7PdPzro25+nybgKXtVuoZak7Y/3sB0cXEW8=; b=eb/cZmWiWdrI5WVGU+sqFzPJIHpK6+iAISbtFRbMsbcPcEFwHCPpYrZy36a/biWqJsOe7X0cw3dwxoEY0wLDU1iswr7Cy578C+s3rEne5hK9k0yvtYQX/HjaNxmCK9umyy1FKi2YcH4eLdrpW7O84LENqjC9QbZ6T8Rfk7fqjOk= Received: from AM0PR04MB5922.eurprd04.prod.outlook.com (2603:10a6:208:125::22) by AM0PR04MB4770.eurprd04.prod.outlook.com (2603:10a6:208:cd::26) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.2937.13; Fri, 24 Apr 2020 06:53:16 +0000 Received: from AM0PR04MB5922.eurprd04.prod.outlook.com ([fe80::602b:aaa7:b3fe:37e6]) by AM0PR04MB5922.eurprd04.prod.outlook.com ([fe80::602b:aaa7:b3fe:37e6%3]) with mapi id 15.20.2921.030; Fri, 24 Apr 2020 06:53:16 +0000 From: "Pankaj Bansal" To: Leif Lindholm , "Pankaj Bansal (OSS)" CC: Meenakshi Aggarwal , Michael D Kinney , "devel@edk2.groups.io" , Varun Sethi , Samer El-Haj-Mahmoud , Jon Nettleton , Ard Biesheuvel Subject: Re: [PATCH edk2-platforms v3 01/24] Silicon/NXP: Add I2c lib Thread-Topic: [PATCH edk2-platforms v3 01/24] Silicon/NXP: Add I2c lib Thread-Index: AQHWGgUHN7uc5i6wTEGxTQukZsRIag== Date: Fri, 24 Apr 2020 06:53:15 +0000 Message-ID: References: <20200415121342.9246-1-pankaj.bansal@oss.nxp.com> <20200415121342.9246-2-pankaj.bansal@oss.nxp.com> <20200422163829.GO14075@vanye> In-Reply-To: <20200422163829.GO14075@vanye> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: spf=none (sender IP is ) smtp.mailfrom=pankaj.bansal@oss.nxp.com; x-ms-exchange-messagesentrepresentingtype: 1 x-originating-ip: [49.36.135.81] x-ms-publictraffictype: Email x-ms-office365-filtering-ht: Tenant x-ms-office365-filtering-correlation-id: 7aeae9a4-6a77-4975-ad4b-08d7e81c2a2f x-ms-traffictypediagnostic: AM0PR04MB4770:|AM0PR04MB4770: x-ms-exchange-sharedmailbox-routingagent-processed: True x-ms-exchange-transport-forked: True x-microsoft-antispam-prvs: x-ms-oob-tlc-oobclassifiers: OLM:9508; x-forefront-prvs: 03838E948C x-forefront-antispam-report: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:AM0PR04MB5922.eurprd04.prod.outlook.com;PTR:;CAT:NONE;SFTY:;SFS:(4636009)(396003)(366004)(346002)(136003)(39860400002)(376002)(81156014)(33656002)(8676002)(4326008)(8936002)(55016002)(86362001)(19627235002)(186003)(54906003)(52536014)(110136005)(6506007)(9686003)(71200400001)(316002)(478600001)(5660300002)(66476007)(66946007)(2906002)(66446008)(64756008)(66556008)(76116006)(7696005)(26005);DIR:OUT;SFP:1101; received-spf: None (protection.outlook.com: oss.nxp.com does not designate permitted sender hosts) x-ms-exchange-senderadcheck: 1 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: /F6RjqRWyGCNhIQtPOKAdkS//Fina99Dzy7IzZQKolbSv5gAeNw/JonPnrhTC0/yfMHL1KL1SbFXeJUOm5+nRoIOizaLkhMwU2P1B6gcyy471Wy4BYvWEDKNR8EGjH4JGJGT6+hI15GLz6JKpqUrfa7ygIjkuMuzJ4kNFfSMi23E4p1ZIJ+qwuFqO1hhdWoz5XsLGkXSpyOuksTLS5NbiV+I+PArANhboFOlNCeewm5x3SsGU2Vfwhf5SAXZUcZ1XrPv0WgOLU000XzSJuObe+DjkfNjlstr5V5/fQ6AlEwkGfRuGVN5SYjI97Qn37ISl8UNqTnwz1fJ5f1LlxrOpilukzlstClM60r5UH5gPKCIOqpdqiSgec6eATea0S2weLzvMK73rO4LRbShP0CiTJzjYrnr7qfHRR70sBIER6Q5bJMnSmeoxy/eY8ONDakm x-ms-exchange-antispam-messagedata: Sktj9m54HVwDaRTKEkqR8e/Mt0LbEiKdANQcOKS35BVI9wt+RRBTrLB0K54SN/mROUktR9AKWEZ6OYjpfo1VdFaKHlz+K7YaXdEiBAlWb7MBS0acsZ7f+34C8FQeRN9uYHWNuDUhkLGvNCndjAvtDw== MIME-Version: 1.0 X-OriginatorOrg: oss.nxp.com X-MS-Exchange-CrossTenant-Network-Message-Id: 7aeae9a4-6a77-4975-ad4b-08d7e81c2a2f X-MS-Exchange-CrossTenant-originalarrivaltime: 24 Apr 2020 06:53:15.8981 (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: iNZQgYJ2rEiIh2GGSfWVV9GAInIg+7UVAZtPipbYFQyeNogtpnw2BZl3QZqo+TbSXgOYxI7QMW1k339jn6xWfA== X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM0PR04MB4770 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable > > +/** > > + Early init I2C for reading the sysclk from I2c slave device. > > + I2c bus clock is determined from the clock input to I2c controller. > > + The clock input to I2c controller is derived from the sysclk. > > + sysclk is determined by clock generator, which is controller by i2c. > > + > > + So, it's a chicken-egg problem to read the sysclk from clock generat= or. > > + To break this cycle (i.e. to read the sysclk), we setup the i2c bus = clock to > > + lowest value, in the hope that it won't be out of clock generator's = supported > > + i2c clock frequency. Once we have the correct sysclk, we can setup t= he > > + correct i2c bus clock. > > + > > + @param[in] Base Base Address of I2c controller's registers > > + > > + @return EFI_SUCCESS successfuly setup the i2c bus for reading sysc= lk > > +**/ > > +EFI_STATUS > > +I2cEarlyInitialize ( > > + IN UINTN Base > > + ) > > +{ > > + I2C_REGS *Regs; > > + UINT8 Ibfd; > > + > > + Regs =3D (I2C_REGS *)Base; > > + if (FeaturePcdGet (PcdI2cErratumA009203)) { > > + // Apply Erratum A-009203 before writing Ibfd register >=20 > It is an improvement, but there is still nothing in here that makes it > obvious why this is being done twice. The referenced u-boot patch does > it only once. It's not necessary to call I2cEarlyInitialize for all I2c controllers. This function has been written so that i2c bus can be initialized to read t= he system clock from a clock generator or an FPFA. To make a call to I2cInitialize, we need to know the I2cBusClock, which is = a derivative of System clock. I2cBusClock is calculated after PLL multiplication to system = clock. Therefore, we won't be able to call I2cInitialize without knowing System cl= ock. The idea is that for i2c controller to which clock generator or an FPFA is = connected, we first Call I2cEarlyInitialize. Then we read system clock and then we can call I2c= Initialize for that Controller as well as any other i2c controller(s) in the system. So, for all i2c controllers (except one) I2cInitialize would be called and = not I2cEarlyInitialize >=20 > Hmm, furthermore, I don't see this function called at all? Why is it > included? If you delete it (and its declaration in .h), I'm OK with > the result. >=20 > > + I2cErratumA009203 (Base); > > + } > > + > > + if (MmioRead8 ((UINTN)&Regs->Ibdbg) & I2C_IBDBG_GLFLT_EN) { > > + Ibfd =3D ARRAY_LAST_ELEM (mI2cClockDivisorGlitchEnabled).Ibfd; > > + } else { > > + Ibfd =3D ARRAY_LAST_ELEM (mI2cClockDivisorGlitchDisabled).Ibfd; > > + } > > + > > + MmioWrite8 ((UINTN)&Regs->Ibfd, Ibfd); > > + > > + I2cReset (Base); > > + > > + return EFI_SUCCESS; > > +} > >