[edk2-devel] [PATCH v2 01/28] Silicon/NXP: Add I2c lib
Pankaj Bansal
pankaj.bansal at nxp.com
Mon Apr 6 06:14:38 UTC 2020
> -----Original Message-----
> From: Leif Lindholm <leif at nuviainc.com>
> Sent: Tuesday, March 31, 2020 5:21 PM
> To: Pankaj Bansal (OSS) <pankaj.bansal at oss.nxp.com>
> Cc: Meenakshi Aggarwal <meenakshi.aggarwal at nxp.com>; Michael D Kinney
> <michael.d.kinney at intel.com>; devel at edk2.groups.io; Varun Sethi
> <V.Sethi at nxp.com>; Samer El-Haj-Mahmoud <Samer.El-Haj-
> Mahmoud at arm.com>; Jon Nettleton <jon at solid-run.com>
> 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 <pankaj.bansal at nxp.com>
> >
> > I2c lib is going to be used in PrePeiCore sec module to get the
> > System clock information from devices connected to i2c (like fpga
> > or clock generator)
> >
> > since we don't have support of DXE modules this early in boot stage,
> > move the i2c controller functionality in library.
>
> This isn't moving the functionality to a library though - it is moving
> the functionality to a library *and* adding new features. 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
> comments below).
>
> I suggest this patch is reordered with 2/28 and all of the splitting
> out part takes place in that patch. This patch can then be reduced to
> ... the bits that are currently impossible to see are changed (at
> 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 would
always be of (reg + data) type only (i.e. two operations) .
And also the Repeat start condition was not generated In the I2cDxe driver.
This caused the I2c Peripheral drivers which were written keeping the controller driver
In mind to issue two operations. Which caused bug in Pcf2129 RTC driver, that I am fixing in patch 3.
Now I have removed these assumptions as well as added Repeat start between successive operations, which
Comply with PI I2c spec.
So, it would be difficult for me to merge the 1 and 2.
>
> > Signed-off-by: Pankaj Bansal <pankaj.bansal at nxp.com>
> > ---
> > Platform/NXP/NxpQoriqLs.dsc.inc | 4 +-
> > Silicon/NXP/Include/Library/I2cLib.h | 120 ++++
> > Silicon/NXP/Library/I2cLib/I2cLib.c | 576 ++++++++++++++++++++
> > Silicon/NXP/Library/I2cLib/I2cLib.inf | 31 ++
> > Silicon/NXP/Library/I2cLib/I2cLibInternal.h | 105 ++++
> > Silicon/NXP/NxpQoriqLs.dec | 10 +-
> > 6 files changed, 844 insertions(+), 2 deletions(-)
> > create mode 100644 Silicon/NXP/Include/Library/I2cLib.h
> > create mode 100644 Silicon/NXP/Library/I2cLib/I2cLib.c
> > create mode 100644 Silicon/NXP/Library/I2cLib/I2cLib.inf
> > create mode 100644 Silicon/NXP/Library/I2cLib/I2cLibInternal.h
> >
> > diff --git a/Platform/NXP/NxpQoriqLs.dsc.inc
> b/Platform/NXP/NxpQoriqLs.dsc.inc
> > index fa5f30dd3909..b28e0615f7ca 100644
> > --- a/Platform/NXP/NxpQoriqLs.dsc.inc
> > +++ b/Platform/NXP/NxpQoriqLs.dsc.inc
> > @@ -1,6 +1,6 @@
> > # @file
> > #
> > -# Copyright 2017-2019 NXP.
> > +# Copyright 2017-2020 NXP.
> > #
> > # SPDX-License-Identifier: BSD-2-Clause-Patent
> > #
> > @@ -94,6 +94,8 @@ [LibraryClasses.common]
> >
> NonDiscoverableDeviceRegistrationLib|MdeModulePkg/Library/NonDiscoverabl
> eDeviceRegistrationLib/NonDiscoverableDeviceRegistrationLib.inf
> >
> ReportStatusCodeLib|MdeModulePkg/Library/DxeReportStatusCodeLib/DxeRep
> ortStatusCodeLib.inf
> >
> > + I2cLib|Silicon/NXP/Library/I2cLib/I2cLib.inf
> > +
>
> I think the changes to this file belong in 2/28.
This I can add to 2/28
>
> > [LibraryClasses.common.SEC]
> > PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
> >
> UefiDecompressLib|MdePkg/Library/BaseUefiDecompressLib/BaseUefiDecomp
> ressLib.inf
> > diff --git a/Silicon/NXP/Include/Library/I2cLib.h
> b/Silicon/NXP/Include/Library/I2cLib.h
> > new file mode 100644
> > index 000000000000..e39237abd3ee
> > --- /dev/null
> > +++ b/Silicon/NXP/Include/Library/I2cLib.h
> > @@ -0,0 +1,120 @@
> > +/** @file
> > + I2c Lib to control I2c controller.
> > +
> > + Copyright 2020 NXP
> > + SPDX-License-Identifier: BSD-2-Clause-Patent
> > +
> > +**/
> > +
> > +#ifndef I2C_LIB_H__
> > +#define I2C_LIB_H__
> > +
> > +#include <Uefi.h>
> > +#include <Pi/PiI2c.h>
> > +
> > +/**
> > + software reset of the entire I2C module.
> > + The module is reset and disabled.
> > + Status register fields (IBSR) are cleared.
> > +
> > + @param[in] Base Base Address of I2c controller's registers
> > +
> > + @return EFI_SUCCESS successfuly reset the I2c module
> > +**/
> > +EFI_STATUS
> > +I2cReset (
> > + IN UINTN Base
> > + );
> > +
> > +/**
> > + 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 generator.
> > + 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 the
> > + correct i2c bus clock.
> > +
> > + @param[in] Base Base Address of I2c controller's registers
> > +
> > + @return EFI_SUCCESS successfuly setup the i2c bus for reading sysclk
> > +**/
> > +EFI_STATUS
> > +I2cEarlyInitialize (
> > + IN UINTN Base
> > + );
> > +
> > +/**
> > + Configure I2c bus to operate at a given speed
> > +
> > + @param[in] Base Base Address of I2c controller's registers
> > + @param[in] I2cBusClock Input clock to I2c controller
> > + @param[in] Speed speed to be configured for I2c bus
> > +
> > + @return EFI_SUCCESS successfuly setup the i2c bus
> > +**/
> > +EFI_STATUS
> > +I2cInitialize (
> > + IN UINTN Base,
> > + IN UINT64 I2cBusClock,
> > + IN UINT64 Speed
> > + );
> > +
> > +/**
> > + Transfer data to/from I2c slave device
> > +
> > + @param[in] Base Base Address of I2c controller's registers
> > + @param[in] SlaveAddress Slave Address from which data is to be read
> > + @param[in] RequestPacket Pointer to an EFI_I2C_REQUEST_PACKET
> structure
> > + describing the I2C transaction
> > +
> > + @return EFI_SUCCESS successfuly transfer the data
> > + @return EFI_DEVICE_ERROR There was an error while transferring data
> through
> > + I2c bus
> > + @return EFI_NO_RESPONSE There was no Ack from i2c device
> > + @return EFI_TIMEOUT I2c Bus is busy
> > + @return EFI_NOT_READY I2c Bus Arbitration lost
> > +**/
> > +EFI_STATUS
> > +I2cBusXfer (
> > + IN UINTN Base,
> > + IN UINT32 SlaveAddress,
> > + IN EFI_I2C_REQUEST_PACKET *RequestPacket
> > + );
> > +
> > +/**
> > + Read a register from I2c slave device. This API is wrapper around I2cBusXfer
> > +
> > + @param[in] Base Base Address of I2c controller's registers
> > + @param[in] SlaveAddress Slave Address from which register value is
> > + to be read
> > + @param[in] RegAddress Register Address in Slave's memory map
> > + @param[in] RegAddressWidthInBytes Number of bytes in RegAddress to
> send to
> > + I2c Slave for simple reads without any
> > + register, make this value = 0
> > + (RegAddress is don't care in that case)
> > + @param[out] RegValue Value to be read from I2c slave's regiser
> > + @param[in] RegValueNumBytes Number of bytes to read from I2c slave
> > + register
> > +
> > + @return EFI_SUCCESS successfuly read the registers
> > + @return EFI_DEVICE_ERROR There was an error while transferring data
> through
> > + I2c bus
> > + @return EFI_NO_RESPONSE There was no Ack from i2c device
> > + @return EFI_TIMEOUT I2c Bus is busy
> > + @return EFI_NOT_READY I2c Bus Arbitration lost
> > +**/
> > +EFI_STATUS
> > +I2cBusReadReg (
> > + IN UINTN Base,
> > + IN UINT32 SlaveAddress,
> > + IN UINT64 RegAddress,
> > + IN UINT8 RegAddressWidthInBytes,
> > + OUT UINT8 *RegValue,
> > + IN UINT32 RegValueNumBytes
> > + );
> > +
> > +#endif // I2C_LIB_H__
> > diff --git a/Silicon/NXP/Library/I2cLib/I2cLib.c
> b/Silicon/NXP/Library/I2cLib/I2cLib.c
> > new file mode 100644
> > index 000000000000..08bf14c490be
> > --- /dev/null
> > +++ b/Silicon/NXP/Library/I2cLib/I2cLib.c
> > @@ -0,0 +1,576 @@
> > +/** @file
> > + I2c Lib to control I2c controller.
> > +
> > + Copyright 2017, 2020 NXP
> > +
> > + SPDX-License-Identifier: BSD-2-Clause-Patent
> > +
> > +**/
> > +#include <Uefi.h>
> > +#include <Library/BaseMemoryLib.h>
> > +#include <Library/I2cLib.h>
> > +#include <Library/IoLib.h>
> > +#include <Library/TimerLib.h>
> > +
> > +#include "I2cLibInternal.h"
> > +
> > +/**
> > + I2C divisor and Ibfd register values when glitch filter is enabled
> > +
> > + In case of duplicate SCL Divisor value, the Ibfd value with high MUL value
> > + has been selected. A higher MUL value results in a lower sampling rate of
> > + the I2C signals. This gives the I2C module greater immunity against glitches
> > + in the I2C signals.
>
> So this comment (and the subsequent one) have been cleaned up and no
> longer refers to an SoC not yet upstream. But this has also removed
> any reference to where these values come from.
>
> Has NXP (including acquisitions) only ever produced one I2C
> controller?
> If there is no standalone name for the controller, can something be
> said about the family of devices that contains it?
I don't know if this is the only I2c IP that we have ever used.
I asked around and I could not get any definite answer to this question.
However, I will add the name of NXP QoriQ Layerscape family of SOCs in comment.
>
> > +**/
> > +STATIC CONST I2C_CLOCK_DIVISOR_PAIR mI2cClockDivisorGlitchEnabled[] =
> {
> > + { 34, 0x0 }, { 36, 0x1 }, { 38, 0x2 }, { 40, 0x3 },
> > + { 42, 0x4 }, { 44, 0x8 }, { 48, 0x9 }, { 52, 0xA },
> > + { 54, 0x7 }, { 56, 0xB }, { 60, 0xC }, { 64, 0x10 },
> > + { 68, 0x40 }, { 72, 0x41 }, { 76, 0x42 }, { 80, 0x43 },
> > + { 84, 0x44 }, { 88, 0x48 }, { 96, 0x49 }, { 104, 0x4A },
> > + { 108, 0x47 }, { 112, 0x4B }, { 120, 0x4C }, { 128, 0x50 },
> > + { 136, 0x80 }, { 144, 0x81 }, { 152, 0x82 }, { 160, 0x83 },
> > + { 168, 0x84 }, { 176, 0x88 }, { 192, 0x89 }, { 208, 0x8A },
> > + { 216, 0x87 }, { 224, 0x8B }, { 240, 0x8C }, { 256, 0x90 },
> > + { 288, 0x91 }, { 320, 0x92 }, { 336, 0x8F }, { 352, 0x93 },
> > + { 384, 0x98 }, { 416, 0x95 }, { 448, 0x99 }, { 480, 0x96 },
> > + { 512, 0x9A }, { 576, 0x9B }, { 640, 0xA0 }, { 704, 0x9D },
> > + { 768, 0xA1 }, { 832, 0x9E }, { 896, 0xA2 }, { 960, 0x67 },
> > + { 1024, 0xA3 }, { 1152, 0xA4 }, { 1280, 0xA8 }, { 1536, 0xA9 },
> > + { 1792, 0xAA }, { 1920, 0xA7 }, { 2048, 0xAB }, { 2304, 0xAC },
> > + { 2560, 0xB0 }, { 3072, 0xB1 }, { 3584, 0xB2 }, { 3840, 0xAF },
> > + { 4096, 0xB3 }, { 4608, 0xB4 }, { 5120, 0xB8 }, { 6144, 0xB9 },
> > + { 7168, 0xBA }, { 7680, 0xB7 }, { 8192, 0xBB }, { 9216, 0xBC },
> > + { 10240, 0xBD }, { 12288, 0xBE }, { 15360, 0xBF }
> > +};
> > +
> > +/**
> > + I2C divisor and Ibfd register values when glitch filter is disabled
> > +
> > + In case of duplicate SCL Divisor value, the Ibfd value with high MUL value
> > + has been selected. A higher MUL value results in a lower sampling rate of
> > + the I2C signals. This gives the I2C module greater immunity against glitches
> > + in the I2C signals.
> > +**/
> > +STATIC CONST I2C_CLOCK_DIVISOR_PAIR mI2cClockDivisorGlitchDisabled[] =
> {
> > + { 20, 0x0 },{ 22, 0x1 },{ 24, 0x2 },{ 26, 0x3 },
> > + { 28, 0x8 },{ 30, 0x5 },{ 32, 0x9 },{ 34, 0x6 },
> > + { 36, 0x0A },{ 40, 0x40 },{ 44, 0x41 },{ 48, 0x42 },
> > + { 52, 0x43 },{ 56, 0x48 },{ 60, 0x45 },{ 64, 0x49 },
> > + { 68, 0x46 },{ 72, 0x4A },{ 80, 0x80 },{ 88, 0x81 },
> > + { 96, 0x82 },{ 104, 0x83 },{ 112, 0x88 },{ 120, 0x85 },
> > + { 128, 0x89 },{ 136, 0x86 },{ 144, 0x8A },{ 160, 0x8B },
> > + { 176, 0x8C },{ 192, 0x90 },{ 208, 0x56 },{ 224, 0x91 },
> > + { 240, 0x1F },{ 256, 0x92 },{ 272, 0x8F },{ 288, 0x93 },
> > + { 320, 0x98 },{ 352, 0x95 },{ 384, 0x99 },{ 416, 0x96 },
> > + { 448, 0x9A },{ 480, 0x5F },{ 512, 0x9B },{ 576, 0x9C },
> > + { 640, 0xA0 },{ 768, 0xA1 },{ 896, 0xA2 },{ 960, 0x9F },
> > + { 1024, 0xA3 },{ 1152, 0xA4 },{ 1280, 0xA8 },{ 1536, 0xA9 },
> > + { 1792, 0xAA },{ 1920, 0xA7 },{ 2048, 0xAB },{ 2304, 0xAC },
> > + { 2560, 0xAD },{ 3072, 0xB1 },{ 3584, 0xB2 },{ 3840, 0xAF },
> > + { 4096, 0xB3 },{ 4608, 0xB4 },{ 5120, 0xB8 },{ 6144, 0xB9 },
> > + { 7168, 0xBA },{ 7680, 0xB7 },{ 8192, 0xBB },{ 9216, 0xBC },
> > + { 10240, 0xBD },{ 12288, 0xBE },{ 15360, 0xBF }
> > +};
> > +
> > +/**
> > + ERR009203 : I2C may not work reliably with the default setting
> > +
> > + Description : The clocking circuitry of I2C module may not work reliably due
> > + to the slow rise time of SCL signal.
> > + Workaround : Enable the receiver digital filter by setting IBDBG[GLFLT_EN]
> > + to 1.
> > +**/
> > +STATIC
> > +VOID
> > +I2cErratumA009203 (
>
> I'm still missing an explanation of why The function contains A009203
> but the comment header describes ERR009203.
> I suggested an addition to the file header comment, but can't see
> anything there.
The Errata in NXP design database are numbered as ERRXXXXX, while in software database
the errata are numbered as AXXXXX.
I asked around and I could only get that this is the convention that we have followed everywhere.
I will change the ERRXXXXX to AXXXXX.
The errata document that mentions this errata is not published in public domain.
However, I can give the link to other software in which this same errata has been added like:
https://elixir.bootlin.com/u-boot/latest/source/arch/arm/cpu/armv8/fsl-layerscape/soc.c#L283
>
> > + IN UINTN Base
> > + )
> > +{
> > + I2C_REGS *Regs;
> > +
> > + Regs = (I2C_REGS *)Base;
> > +
> > + MmioOr8 ((UINTN)&Regs->Ibdbg, I2C_IBDBG_GLFLT_EN);
> > +}
> > +
> > +/**
> > + software reset of the entire I2C module.
> > + The module is reset and disabled.
> > + Status register fields (IBSR) are cleared.
> > +
> > + @param[in] Base Base Address of I2c controller's registers
> > +
> > + @return EFI_SUCCESS successfuly reset the I2c module
> > +**/
> > +EFI_STATUS
> > +I2cReset (
> > + IN UINTN Base
> > + )
> > +{
> > + I2C_REGS *Regs;
> > +
> > + Regs = (I2C_REGS *)Base;
> > +
> > + MmioOr8 ((UINTN)&Regs->Ibcr, I2C_IBCR_MDIS);
> > + MmioOr8 ((UINTN)&Regs->Ibsr, (I2C_IBSR_IBAL | I2C_IBSR_IBIF));
> > + MmioAnd8 ((UINTN)&Regs->Ibcr, ~(I2C_IBCR_IBIE | I2C_IBCR_DMAEN));
> > + MmioAnd8 ((UINTN)&Regs->Ibic, (UINT8)(~I2C_IBIC_BIIE));
> > +
> > + return EFI_SUCCESS;
> > +}
> > +
> > +/**
> > + 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 generator.
> > + 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 the
> > + correct i2c bus clock.
> > +
> > + @param[in] Base Base Address of I2c controller's registers
> > +
> > + @return EFI_SUCCESS successfuly setup the i2c bus for reading sysclk
> > +**/
> > +EFI_STATUS
> > +I2cEarlyInitialize (
> > + IN UINTN Base
> > + )
> > +{
> > + I2C_REGS *Regs;
> > + UINT8 Ibfd;
>
> "Ibfd" is not more CamelCase compliant than "Ibc" is, and no more
> descriptive. Please, if this is a term used in official NXP
> documentation, add it to a glossary in the file header comment.
> If not, please name it using camel case as something I can understand
> what it is intended to be used for without already knowing.
Ok, I will add the abbreviations used in file header
>
> ... skims ahead ...
>
> OK, so now it's "I2c Bus Frequency Dividor" (think that should be
> Divider). Sure, that can be Ibfd *if* introduced in the file header
> glossary, in *every* file that contains this abbreviation.
>
Ok, I will add the abbreviations used in file header
> > +
> > + Regs = (I2C_REGS *)Base;
> > + if (FeaturePcdGet (PcdI2cErratumA009203)) {
> > + I2cErratumA009203 (Base);
> > + }
> > +
> > + if (MmioRead8 ((UINTN)&Regs->Ibdbg) & I2C_IBDBG_GLFLT_EN) {
> > + Ibfd = ARRAY_LAST_ELEM (mI2cClockDivisorGlitchEnabled).Ibfd;
> > + } else {
> > + Ibfd = ARRAY_LAST_ELEM (mI2cClockDivisorGlitchDisabled).Ibfd;
> > + }
> > +
> > + MmioWrite8 ((UINTN)&Regs->Ibfd, Ibfd);
> > +
> > + I2cReset (Base);
> > +
> > + return EFI_SUCCESS;
> > +}
> > +
> > +/**
> > + Configure I2c bus to operate at a given speed
> > +
> > + @param[in] Base Base Address of I2c controller's registers
> > + @param[in] I2cBusClock Input clock to I2c controller
> > + @param[in] Speed speed to be configured for I2c bus
> > +
> > + @return EFI_SUCCESS successfuly setup the i2c bus
> > +**/
> > +EFI_STATUS
> > +I2cInitialize (
> > + IN UINTN Base,
> > + IN UINT64 I2cBusClock,
> > + IN UINT64 Speed
> > + )
> > +{
> > + I2C_REGS *Regs;
> > + UINT16 ClockDivisor;
> > + UINT8 Ibfd; // I2c Bus Frequency Dividor Register
>
> Introduction of non-standard abbreviations happen in the file header comment
> block:
> https://edk2-docs.gitbooks.io/edk-ii-c-coding-standards-
> specification/content/v/release/2.20/5_source_files/52_spacing.html#5231-
> every-new-file-shall-begin-with-a-file-header-comment-block
Ok, I will add the abbreviations used in file header
>
> > + CONST I2C_CLOCK_DIVISOR_PAIR *ClockDivisorPair;
> > + UINT32 ClockDivisorPairSize;
> > + UINT32 Index;
> > +
> > + Regs = (I2C_REGS *)Base;
> > + if (FeaturePcdGet (PcdI2cErratumA009203)) {
> > + I2cErratumA009203 (Base);
> > + }
>
> Is this erratum workaround intentionally invoked twice?
> If so, the code could do with comments at all call sites explaining why.
> I.e. "apply workaround before enabling controller", "re-apply
> workaround after updating X", ...
Ok I will add the comments at call sites.
>
> > +
> > + Ibfd = 0;
> > + ClockDivisor = (I2cBusClock + Speed - 1) / Speed;
> > +
> > + if (MmioRead8 ((UINTN)&Regs->Ibdbg) & I2C_IBDBG_GLFLT_EN) {
> > + ClockDivisorPair = mI2cClockDivisorGlitchEnabled;
> > + ClockDivisorPairSize = ARRAY_SIZE (mI2cClockDivisorGlitchEnabled);
> > + } else {
> > + ClockDivisorPair = mI2cClockDivisorGlitchDisabled;
> > + ClockDivisorPairSize = ARRAY_SIZE (mI2cClockDivisorGlitchDisabled);
> > + }
> > +
> > + if (ClockDivisor > ClockDivisorPair[ClockDivisorPairSize - 1].Divisor) {
> > + Ibfd = ClockDivisorPair[ClockDivisorPairSize - 1].Ibfd;
> > + } else {
> > + for (Index = 0; Index < ClockDivisorPairSize; Index++) {
> > + if (ClockDivisorPair[Index].Divisor >= ClockDivisor) {
> > + Ibfd = ClockDivisorPair[Index].Ibfd;
> > + break;
> > + }
> > + }
> > + }
> > +
> > + MmioWrite8 ((UINTN)&Regs->Ibfd, Ibfd);
> > +
> > + I2cReset (Base);
> > +
> > + return EFI_SUCCESS;
> > +}
> > +
> > +STATIC
> > +EFI_STATUS
> > +I2cBusTestBusBusy (
> > + IN I2C_REGS *Regs,
> > + IN BOOLEAN TestBusy
> > + )
> > +{
> > + UINT32 Index;
> > + UINT8 Reg;
> > +
> > + for (Index = 0; Index < I2C_NUM_RETRIES; Index++) {
> > + Reg = MmioRead8 ((UINTN)&Regs->Ibsr);
> > +
> > + if (Reg & I2C_IBSR_IBAL) {
> > + MmioWrite8 ((UINTN)&Regs->Ibsr, Reg);
> > + return EFI_NOT_READY;
> > + }
> > +
> > + if (TestBusy && (Reg & I2C_IBSR_IBB)) {
> > + break;
> > + }
> > +
> > + if (!TestBusy && !(Reg & I2C_IBSR_IBB)) {
> > + break;
> > + }
> > +
> > + MicroSecondDelay (1);
> > + }
> > +
> > + if (Index == I2C_NUM_RETRIES) {
> > + return EFI_TIMEOUT;
> > + }
> > +
> > + return EFI_SUCCESS;
> > +}
> > +
> > +STATIC
> > +EFI_STATUS
> > +I2cTransferComplete (
> > + IN I2C_REGS *Regs,
> > + IN BOOLEAN TestRxAck
> > +)
> > +{
> > + UINT32 Index;
> > + UINT8 Reg;
> > +
> > + for (Index = 0; Index < I2C_NUM_RETRIES; Index++) {
> > + Reg = MmioRead8 ((UINTN)&Regs->Ibsr);
> > +
> > + if (Reg & I2C_IBSR_IBIF) {
> > + // Write 1 to clear the IBIF field
> > + MmioWrite8 ((UINTN)&Regs->Ibsr, Reg);
> > + break;
> > + }
> > +
> > + MicroSecondDelay (1);
> > + }
> > +
> > + if (Index == I2C_NUM_RETRIES) {
> > + return EFI_TIMEOUT;
> > + }
> > +
> > + if (TestRxAck && (Reg & I2C_IBSR_RXAK)) {
> > + return EFI_NO_RESPONSE;
> > + }
> > +
> > + if (Reg & I2C_IBSR_TCF) {
> > + return EFI_SUCCESS;
> > + }
> > +
> > + return EFI_DEVICE_ERROR;
> > +}
> > +
> > +STATIC
> > +EFI_STATUS
> > +I2cRead (
> > + IN I2C_REGS *Regs,
> > + IN UINT32 SlaveAddress,
> > + IN EFI_I2C_OPERATION *Operation,
> > + IN BOOLEAN IsLastOperation
> > +)
> > +{
> > + EFI_STATUS Status;
> > + UINTN Index;
> > +
> > + // Write Slave Address
> > + MmioWrite8 ((UINTN)&Regs->Ibdr, (SlaveAddress << BIT0) | BIT0);
> > + Status = I2cTransferComplete (Regs, I2C_BUS_TEST_RX_ACK);
> > + if (EFI_ERROR (Status)) {
> > + return Status;
> > + }
> > + // select Receive mode.
> > + MmioAnd8 ((UINTN)&Regs->Ibcr, ~I2C_IBCR_TXRX);
> > + if (Operation->LengthInBytes > 1) {
> > + // Set No ACK = 0
> > + MmioAnd8 ((UINTN)&Regs->Ibcr, ~I2C_IBCR_NOACK);
> > + }
> > +
> > + // Perform a dummy read to initiate the receive operation.
> > + MmioRead8 ((UINTN)&Regs->Ibdr);
> > +
> > + for (Index = 0; Index < Operation->LengthInBytes; Index++) {
> > + Status = I2cTransferComplete (Regs, I2C_BUS_NO_TEST_RX_ACK);
> > + if (EFI_ERROR (Status)) {
> > + return Status;
> > + }
> > + if (Index == (Operation->LengthInBytes - 2)) {
> > + // Set No ACK = 1
> > + MmioOr8 ((UINTN)&Regs->Ibcr, I2C_IBCR_NOACK);
> > + } else if (Index == (Operation->LengthInBytes - 1)) {
> > + if (!IsLastOperation) {
> > + // select Transmit mode (for repeat start)
> > + MmioOr8 ((UINTN)&Regs->Ibcr, I2C_IBCR_TXRX);
> > + } else {
> > + // Generate Stop Signal
> > + MmioAnd8 ((UINTN)&Regs->Ibcr, ~(I2C_IBCR_MSSL | I2C_IBCR_TXRX));
> > + Status = I2cBusTestBusBusy (Regs, I2C_BUS_TEST_IDLE);
> > + if (EFI_ERROR (Status)) {
> > + return Status;
> > + }
> > + }
> > + }
> > + Operation->Buffer[Index] = MmioRead8 ((UINTN)&Regs->Ibdr);
> > + }
> > +
> > + return EFI_SUCCESS;
> > +}
> > +
> > +STATIC
> > +EFI_STATUS
> > +I2cWrite (
> > + IN I2C_REGS *Regs,
> > + IN UINT32 SlaveAddress,
> > + IN EFI_I2C_OPERATION *Operation
> > +)
> > +{
> > + EFI_STATUS Status;
> > + UINTN Index;
> > +
> > + // Write Slave Address
> > + MmioWrite8 ((UINTN)&Regs->Ibdr, (SlaveAddress << BIT0) &
> (UINT8)(~BIT0));
> > + Status = I2cTransferComplete (Regs, I2C_BUS_TEST_RX_ACK);
> > + if (EFI_ERROR (Status)) {
> > + return Status;
> > + }
> > +
> > + // Write Data
> > + for (Index = 0; Index < Operation->LengthInBytes; Index++) {
> > + MmioWrite8 ((UINTN)&Regs->Ibdr, Operation->Buffer[Index]);
> > + Status = I2cTransferComplete (Regs, I2C_BUS_TEST_RX_ACK);
> > + if (EFI_ERROR (Status)) {
> > + return Status;
> > + }
> > + }
> > + return EFI_SUCCESS;
> > +}
> > +
> > +STATIC
> > +EFI_STATUS
> > +I2cStop (
> > + IN I2C_REGS *Regs
> > + )
> > +{
> > + EFI_STATUS Status;
> > + UINT8 Reg;
> > +
> > + Status = EFI_SUCCESS;
> > + Reg = MmioRead8 ((UINTN)&Regs->Ibsr);
> > + if (Reg & I2C_IBSR_IBB) {
> > + // Generate Stop Signal
> > + MmioAnd8 ((UINTN)&Regs->Ibcr, ~(I2C_IBCR_MSSL | I2C_IBCR_TXRX));
> > + Status = I2cBusTestBusBusy (Regs, I2C_BUS_TEST_IDLE);
> > + if (EFI_ERROR (Status)) {
> > + return Status;
> > + }
> > + }
> > +
> > + // Disable I2c Controller
> > + MmioOr8 ((UINTN)&Regs->Ibcr, I2C_IBCR_MDIS);
> > +
> > + return Status;
> > +}
> > +
> > +STATIC
> > +EFI_STATUS
> > +I2cStart (
> > + IN I2C_REGS *Regs
> > + )
> > +{
> > + EFI_STATUS Status;
> > +
> > + MmioOr8 ((UINTN)&Regs->Ibsr, (I2C_IBSR_IBAL | I2C_IBSR_IBIF));
> > + MmioAnd8 ((UINTN)&Regs->Ibcr, (UINT8)(~I2C_IBCR_MDIS));
> > +
> > + //Wait controller to be stable
> > + MicroSecondDelay (1);
> > +
> > + // Generate Start Signal
> > + MmioOr8 ((UINTN)&Regs->Ibcr, I2C_IBCR_MSSL);
> > + Status = I2cBusTestBusBusy (Regs, I2C_BUS_TEST_BUSY);
> > + if (EFI_ERROR (Status)) {
> > + return Status;
> > + }
> > +
> > + // Select Transmit Mode. set No ACK = 1
> > + MmioOr8 ((UINTN)&Regs->Ibcr, (I2C_IBCR_TXRX | I2C_IBCR_NOACK));
> > +
> > + return Status;
> > +}
> > +
> > +/**
> > + Transfer data to/from I2c slave device
> > +
> > + @param[in] Base Base Address of I2c controller's registers
> > + @param[in] SlaveAddress Slave Address from which data is to be read
> > + @param[in] RequestPacket Pointer to an EFI_I2C_REQUEST_PACKET
> structure
> > + describing the I2C transaction
> > +
> > + @return EFI_SUCCESS successfuly transfer the data
> > + @return EFI_DEVICE_ERROR There was an error while transferring data
> through
> > + I2c bus
> > + @return EFI_NO_RESPONSE There was no Ack from i2c device
> > + @return EFI_TIMEOUT I2c Bus is busy
> > + @return EFI_NOT_READY I2c Bus Arbitration lost
> > +**/
> > +EFI_STATUS
> > +I2cBusXfer (
> > + IN UINTN Base,
> > + IN UINT32 SlaveAddress,
> > + IN EFI_I2C_REQUEST_PACKET *RequestPacket
> > + )
> > +{
> > + UINTN Index;
> > + I2C_REGS *Regs;
> > + EFI_I2C_OPERATION *Operation;
> > + EFI_STATUS Status;
> > + BOOLEAN IsLastOperation;
> > +
> > + Regs = (I2C_REGS *)Base;
> > + IsLastOperation = FALSE;
> > +
> > + Status = I2cBusTestBusBusy (Regs, I2C_BUS_TEST_IDLE);
> > + if (EFI_ERROR (Status)) {
> > + goto ErrorExit;
> > + }
> > +
> > + Status = I2cStart (Regs);
> > + if (EFI_ERROR (Status)) {
> > + goto ErrorExit;
> > + }
> > +
> > + for (Index = 0, Operation = RequestPacket->Operation;
> > + Index < RequestPacket->OperationCount;
> > + Index++, Operation++) {
> > + if (Index == (RequestPacket->OperationCount - 1)) {
> > + IsLastOperation = TRUE;
> > + }
> > + // Send repeat start after first transmit/recieve
> > + if (Index) {
> > + MmioOr8 ((UINTN)&Regs->Ibcr, I2C_IBCR_RSTA);
> > + Status = I2cBusTestBusBusy (Regs, I2C_BUS_TEST_BUSY);
> > + if (EFI_ERROR (Status)) {
> > + goto ErrorExit;
> > + }
> > + }
> > + // Read/write data
> > + if (Operation->Flags & I2C_FLAG_READ) {
> > + Status = I2cRead (Regs, SlaveAddress, Operation, IsLastOperation);
> > + } else {
> > + Status = I2cWrite (Regs, SlaveAddress, Operation);
> > + }
> > + if (EFI_ERROR (Status)) {
> > + goto ErrorExit;
> > + }
> > + }
> > +
> > +ErrorExit:
> > +
> > + I2cStop (Regs);
> > +
> > + return Status;
> > +}
> > +
> > +/**
> > + Read a register from I2c slave device. This API is wrapper around I2cBusXfer
> > +
> > + @param[in] Base Base Address of I2c controller's registers
> > + @param[in] SlaveAddress Slave Address from which register value is
> > + to be read
> > + @param[in] RegAddress Register Address in Slave's memory map
> > + @param[in] RegAddressWidthInBytes Number of bytes in RegAddress to
> send to
> > + I2c Slave for simple reads without any
> > + register, make this value = 0
> > + (RegAddress is don't care in that case)
> > + @param[out] RegValue Value to be read from I2c slave's regiser
> > + @param[in] RegValueNumBytes Number of bytes to read from I2c slave
> > + register
> > +
> > + @return EFI_SUCCESS successfuly read the registers
> > + @return EFI_DEVICE_ERROR There was an error while transferring data
> through
> > + I2c bus
> > + @return EFI_NO_RESPONSE There was no Ack from i2c device
> > + @return EFI_TIMEOUT I2c Bus is busy
> > + @return EFI_NOT_READY I2c Bus Arbitration lost
> > +**/
> > +EFI_STATUS
> > +I2cBusReadReg (
> > + IN UINTN Base,
> > + IN UINT32 SlaveAddress,
> > + IN UINT64 RegAddress,
> > + IN UINT8 RegAddressWidthInBytes,
> > + OUT UINT8 *RegValue,
> > + IN UINT32 RegValueNumBytes
> > + )
> > +{
> > + EFI_I2C_OPERATION *Operations;
> > + I2C_REG_REQUEST RequestPacket;
> > + UINTN OperationCount;
> > + UINT8 Address[sizeof (RegAddress)];
> > + UINT8 *PtrAddress;
>
> This is too close to hungarian notation - AddressPtr works better.
Ok. I will change it.
>
> /
> Leif
>
> > + EFI_STATUS Status;
> > +
> > + ZeroMem (&RequestPacket, sizeof (RequestPacket));
> > + OperationCount = 0;
> > + Operations = RequestPacket.Operation;
> > + PtrAddress = Address;
> > +
> > + if (RegAddressWidthInBytes > ARRAY_SIZE (Address)) {
> > + return EFI_INVALID_PARAMETER;
> > + }
> > +
> > + if (RegAddressWidthInBytes != 0) {
> > + Operations[OperationCount].LengthInBytes = RegAddressWidthInBytes;
> > + Operations[OperationCount].Buffer = PtrAddress;
> > + while (RegAddressWidthInBytes--) {
> > + *PtrAddress++ = RegAddress >> (8 * RegAddressWidthInBytes);
> > + }
> > + OperationCount++;
> > + }
> > +
> > + Operations[OperationCount].LengthInBytes = RegValueNumBytes;
> > + Operations[OperationCount].Buffer = RegValue;
> > + Operations[OperationCount].Flags = I2C_FLAG_READ;
> > + OperationCount++;
> > +
> > + RequestPacket.OperationCount = OperationCount;
> > +
> > + Status = I2cBusXfer (
> > + Base, SlaveAddress,
> > + (EFI_I2C_REQUEST_PACKET *)&RequestPacket
> > + );
> > +
> > + return Status;
> > +}
> > +
> > diff --git a/Silicon/NXP/Library/I2cLib/I2cLib.inf
> b/Silicon/NXP/Library/I2cLib/I2cLib.inf
> > new file mode 100644
> > index 000000000000..b9bd79ac1ef1
> > --- /dev/null
> > +++ b/Silicon/NXP/Library/I2cLib/I2cLib.inf
> > @@ -0,0 +1,31 @@
> > +# @file
> > +#
> > +# Component description file for I2cLib module
> > +# Copyright 2017, 2020 NXP
> > +#
> > +# SPDX-License-Identifier: BSD-2-Clause-Patent
> > +#
> > +#
> > +
> > +[Defines]
> > + INF_VERSION = 0x0001001A
> > + BASE_NAME = I2cLib
> > + FILE_GUID = 8ecefc8f-a2c4-4091-b81f-20f7aeb0567f
> > + MODULE_TYPE = BASE
> > + VERSION_STRING = 1.0
> > + LIBRARY_CLASS = I2cLib
> > +
> > +[Sources.common]
> > + I2cLib.c
> > +
> > +[LibraryClasses]
> > + IoLib
> > + TimerLib
> > +
> > +[Packages]
> > + MdePkg/MdePkg.dec
> > + Silicon/NXP/NxpQoriqLs.dec
> > +
> > +[FeaturePcd]
> > + gNxpQoriqLsTokenSpaceGuid.PcdI2cErratumA009203
> > +
> > diff --git a/Silicon/NXP/Library/I2cLib/I2cLibInternal.h
> b/Silicon/NXP/Library/I2cLib/I2cLibInternal.h
> > new file mode 100644
> > index 000000000000..2ca4a3639d2c
> > --- /dev/null
> > +++ b/Silicon/NXP/Library/I2cLib/I2cLibInternal.h
> > @@ -0,0 +1,105 @@
> > +/** @file
> > + I2c Lib to control I2c controller.
> > +
> > + Copyright 2020 NXP
> > + SPDX-License-Identifier: BSD-2-Clause-Patent
> > +
> > +**/
> > +
> > +#ifndef I2C_LIB_INTERNAL_H__
> > +#define I2C_LIB_INTERNAL_H__
> > +
> > +#include <Pi/PiI2c.h>
> > +#include <Uefi.h>
> > +
> > +/** Module Disable
> > + 0b - The module is enabled. You must clear this field before any other IBCR
> > + fields have any effect.
> > + 1b - The module is reset and disabled. This is the power-on reset situation.
> > + When high, the interface is held in reset, but registers can still be
> > + accessed. Status register fields (IBSR) are not valid when the module
> > + is disabled.
> > +**/
> > +#define I2C_IBCR_MDIS BIT7
> > +// I2c Bus Interrupt Enable
> > +#define I2C_IBCR_IBIE BIT6
> > +/** Master / Slave Mode 0b - Slave mode 1b - Master mode
> > + When you change this field from 0 to 1, the module generates a START
> signal
> > + on the bus and selects the master mode. When you change this field from 1
> to
> > + 0, the module generates a STOP signal and changes the operation mode
> from
> > + master to slave. You should generate a STOP signal only if IBSR[IBIF]=1.
> > + The module clears this field without generating a STOP signal when the
> > + master loses arbitration.
> > +*/
> > +#define I2C_IBCR_MSSL BIT5
> > +// 0b - Receive 1b - Transmit
> > +#define I2C_IBCR_TXRX BIT4
> > +/** Data acknowledge disable
> > + Values written to this field are only used when the I2C module is a receiver,
> > + not a transmitter.
> > + 0b - The module sends an acknowledge signal to the bus at the 9th clock bit
> > + after receiving one byte of data.
> > + 1b - The module does not send an acknowledge-signal response (that is,
> > + acknowledge bit = 1).
> > +**/
> > +#define I2C_IBCR_NOACK BIT3
> > +/**Repeat START
> > + If the I2C module is the current bus master, and you program RSTA=1, the
> I2C
> > + module generates a repeated START condition. This field always reads as a 0.
> > + If you attempt a repeated START at the wrong time, if the bus is owned by
> > + another master the result is loss of arbitration.
> > +**/
> > +#define I2C_IBCR_RSTA BIT2
> > +// DMA enable
> > +#define I2C_IBCR_DMAEN BIT1
> > +
> > +// Transfer Complete
> > +#define I2C_IBSR_TCF BIT7
> > +// I2C bus Busy. 0b - Bus is idle, 1b - Bus is busy
> > +#define I2C_IBSR_IBB BIT5
> > +// Arbitration Lost. software must clear this field by writing a one to it.
> > +#define I2C_IBSR_IBAL BIT4
> > +// I2C bus interrupt flag
> > +#define I2C_IBSR_IBIF BIT1
> > +// Received acknowledge 0b - Acknowledge received 1b - No acknowledge
> received
> > +#define I2C_IBSR_RXAK BIT0
> > +
> > +//Bus idle interrupt enable
> > +#define I2C_IBIC_BIIE BIT7
> > +
> > +// Glitch filter enable
> > +#define I2C_IBDBG_GLFLT_EN BIT3
> > +
> > +#define I2C_BUS_TEST_BUSY TRUE
> > +#define I2C_BUS_TEST_IDLE !I2C_BUS_TEST_BUSY
> > +#define I2C_BUS_TEST_RX_ACK TRUE
> > +#define I2C_BUS_NO_TEST_RX_ACK !I2C_BUS_TEST_RX_ACK
> > +
> > +#define ARRAY_LAST_ELEM(x) (x)[ARRAY_SIZE (x) - 1]
> > +#define I2C_NUM_RETRIES 500
> > +
> > +typedef struct _I2C_REGS {
> > + UINT8 Ibad; // I2c Bus Address Register
> > + UINT8 Ibfd; // I2c Bus Frequency Dividor Register
> > + UINT8 Ibcr; // I2c Bus Control Register
> > + UINT8 Ibsr; // I2c Bus Status Register
> > + UINT8 Ibdr; // I2C Bus Data I/O Register
> > + UINT8 Ibic; // I2C Bus Interrupt Config Register
> > + UINT8 Ibdbg; // I2C Bus Debug Register
> > +} I2C_REGS;
> > +
> > +/*
> > + * sorted list of clock divisor, Ibfd register value pairs
> > + */
> > +typedef struct _I2C_CLOCK_DIVISOR_PAIR {
> > + UINT16 Divisor;
> > + UINT16 Ibfd; // I2c Bus Frequency Dividor Register value
> > +} I2C_CLOCK_DIVISOR_PAIR;
> > +
> > +typedef struct {
> > + UINTN OperationCount;
> > + EFI_I2C_OPERATION Operation[2];
> > +} I2C_REG_REQUEST;
> > +
> > +#endif // I2C_LIB_INTERNAL_H__
> > +
> > diff --git a/Silicon/NXP/NxpQoriqLs.dec b/Silicon/NXP/NxpQoriqLs.dec
> > index 764b9bb0e2d3..4a1cfb3e278e 100644
> > --- a/Silicon/NXP/NxpQoriqLs.dec
> > +++ b/Silicon/NXP/NxpQoriqLs.dec
> > @@ -1,6 +1,6 @@
> > # @file.
> > #
> > -# Copyright 2017-2019 NXP
> > +# Copyright 2017-2020 NXP
> > #
> > # SPDX-License-Identifier: BSD-2-Clause-Patent
> > #
> > @@ -13,6 +13,10 @@ [Defines]
> > [Includes]
> > Include
> >
> > +[LibraryClasses]
> > + ## @libraryclass Provides services to read/write to I2c devices
> > + I2cLib|Include/Library/I2cLib.h
> > +
> > [Guids.common]
> > gNxpQoriqLsTokenSpaceGuid = {0x98657342, 0x4aee, 0x4fc6, {0xbc, 0xb5,
> 0xff, 0x45, 0xb7, 0xa8, 0x71, 0xf2}}
> > gNxpNonDiscoverableI2cMasterGuid = { 0x5f2c099c, 0x54a3, 0x4dd4, {0x9e,
> 0xc5, 0xe9, 0x12, 0x8c, 0x36, 0x81, 0x6a}}
> > @@ -101,3 +105,7 @@ [PcdsFixedAtBuild.common]
> >
> gNxpQoriqLsTokenSpaceGuid.PcdPciLutBigEndian|FALSE|BOOLEAN|0x0000031
> 2
> >
> gNxpQoriqLsTokenSpaceGuid.PcdWatchdogBigEndian|FALSE|BOOLEAN|0x0000
> 0313
> >
> gNxpQoriqLsTokenSpaceGuid.PcdIfcBigEndian|FALSE|BOOLEAN|0x00000314
> > +
> > +[PcdsFeatureFlag]
> > +
> gNxpQoriqLsTokenSpaceGuid.PcdI2cErratumA009203|FALSE|BOOLEAN|0x0000
> 0315
> > +
> > --
> > 2.17.1
> >
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#56969): https://edk2.groups.io/g/devel/message/56969
Mute This Topic: https://groups.io/mt/72077423/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-
More information about the edk2-devel-archive
mailing list