-=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#54006): https://edk2.groups.io/g/devel/message/54006 Mute This Topic: https://groups.io/mt/71046320/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
-=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#54007): https://edk2.groups.io/g/devel/message/54007 Mute This Topic: https://groups.io/mt/71046321/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
On Fri, Feb 07, 2020 at 18:13:10 +0530, Pankaj Bansal wrote:
> 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 clcok generator)
>
> since we don't have support of DXE modules this early in boot stage,
> move the i2c controller functionality in library.
>
> Signed-off-by: Pankaj Bansal <pankaj.bansal@nxp.com>
> ---
> Platform/NXP/NxpQoriqLs.dsc.inc | 4 +-
> Silicon/NXP/Include/Library/I2cLib.h | 99 ++++
> Silicon/NXP/Library/I2cLib/I2cLib.c | 532 ++++++++++++++++++++
> Silicon/NXP/Library/I2cLib/I2cLib.inf | 30 ++
> Silicon/NXP/Library/I2cLib/I2cLibInternal.h | 95 ++++
> Silicon/NXP/NxpQoriqLs.dec | 10 +-
> 6 files changed, 768 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 fa5f30dd39..b28e0615f7 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 @@
Can you ensure to either follow the manual git setup steps from
https://github.com/tianocore/tianocore.github.io/wiki/Laszlo%27s-unkempt-git-guide-for-edk2-contributors-and-maintainers
or execute edk2/BaseTools/Scripts/SetupGit.py in each of the tianocore
repositories?
That makes the diff show which section in this file is being modified,
and it also orders the files in decreasing order of abstraction.
> NonDiscoverableDeviceRegistrationLib|MdeModulePkg/Library/NonDiscoverableDeviceRegistrationLib/NonDiscoverableDeviceRegistrationLib.inf
> ReportStatusCodeLib|MdeModulePkg/Library/DxeReportStatusCodeLib/DxeReportStatusCodeLib.inf
>
> + I2cLib|Silicon/NXP/Library/I2cLib/I2cLib.inf
> +
> [LibraryClasses.common.SEC]
> PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
> UefiDecompressLib|MdePkg/Library/BaseUefiDecompressLib/BaseUefiDecompressLib.inf
> diff --git a/Silicon/NXP/Include/Library/I2cLib.h b/Silicon/NXP/Include/Library/I2cLib.h
> new file mode 100644
> index 0000000000..3594e3fc2f
> --- /dev/null
> +++ b/Silicon/NXP/Include/Library/I2cLib.h
> @@ -0,0 +1,99 @@
> +/** @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__
Please drop leading __ from include header guards.
> +
> +#include <Pi/PiI2c.h>
> +
> +/**
> + 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
> + @return DEVICE_ERROR There was an error setting up the I2c bus
> +**/
> +EFI_STATUS
> +I2cEarlyInitialize (
> + IN UINTN Base
> + );
> +
> +/**
> + Configure I2c bus to opearte 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
> +**/
> +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 setup the i2c bus for reading sysclk
> + @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 setup the i2c bus for reading sysclk
> + @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 UINT8 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 0000000000..f33c09d6d1
> --- /dev/null
> +++ b/Silicon/NXP/Library/I2cLib/I2cLib.c
> @@ -0,0 +1,532 @@
> +/** @file
> + I2c Lib to control I2c controller.
> +
> + Copyright 2020 NXP
> +
> + SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +#include <Uefi.h>
> +#include <Library/I2cLib.h>
> +#include <Library/TimerLib.h>
> +#include <Library/IoLib.h>
> +#include <Library/BaseMemoryLib.h>
Please sort includes alphabetically (at least within each
subdirectory).
> +
> +#include "I2cLibInternal.h"
> +
> +/*
> + * I2C divider and hold values when glitch filter is enabled
> + * taken from table 21-14, LX2160ARM_RevE 01/2020
OK, so it looks to me like this adds a layer of abstraction compared
to what already exists in Silicon/NXP/Drivers/I2cDxe/. Which is fine.
However, that driver was added for LS1043A, which will also become a
consumer of this code once the subsequent patch gets merged.
Which is probably fine - but this comment (and the next one) would
ideally identify the SoCs supported; or if there is some common name
for the i2c controller used in all supported SoCs, that would be
ideal.
> + *
> + * In case of duplicate SCL Divider value, the IBC 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 I2C_CLOCK_DIVIDER_PAIR I2cClockDividerGlitchEnabled[] = {
Module-scope global variables should have 'm' prefix.
> + { 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 divider and hold values when glitch filter is disabled
> + * taken from table 21-13, LX2160ARM_RevE 01/2020
> + *
> + * In case of duplicate SCL Divider value, the IBC 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 I2C_CLOCK_DIVIDER_PAIR I2cClockDividerGlitchDisabled[] = {
Module-scope global variables should have 'm' prefix.
> + { 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
What erratum database does this ID refer to?
I don't need access to it, but an explanation at the end of the file
header comment would be helpful.
> +
> + Description : The clocking circuitry of I2C module may not work reliably due to the slow
> + rise time of SCL signal.
First line too long (wrap after 'due'). Second line weird indentation.
> + Workaround : Enable the receiver digital filter by setting IBDBG[GLFLT_EN] to 1.
Ideally, wrap this line after 'setting'.
> +*/
> +STATIC
> +VOID
> + I2cErratumA009203 (
No indentation of function name.
Explanation of where that A prefix of the erratum ID came from would
also be appreciated.
> + IN UINTN Base
> + )
> +{
> + I2C_REGS *Regs;
> +
> + Regs = (I2C_REGS *)Base;
> +
> + MmioOr8 ( (UINTN)&Regs->Ibdbg, I2C_IBDBG_GLFLT_EN);
No space before (UINTN).
> +}
> +
> +/**
> + 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 Ibc;
This is a CamelCase project.
"Regs" is clear enough, but Ibc needs to be written out.
I'm going to guess it stands for i2c bus clock.
Since we're in a function where the name starts with I2c, we don't
need to be explicit about that. And we don't seem to be having any
other clocks to keep track of in this - so how about just calling it
"Clock"?
But then from the way it's used, should is be called MaxClock?
> +
> + Regs = (I2C_REGS *)Base;
> + if (FeaturePcdGet (PcdI2cErratumA009203)) {
> + I2cErratumA009203 (Base);
> + }
> +
> + if (MmioRead8 ( (UINTN)&Regs->Ibdbg) & I2C_IBDBG_GLFLT_EN) {
No space before (UINTN).
In general, could you do a global search and replace for "( ("?
I won't mention it again, but since it is used consistently, I expect
to see much more of it in this set :)
> + Ibc = I2cClockDividerGlitchEnabled[ARRAY_SIZE (I2cClockDividerGlitchEnabled) - 1].Ibc;
> + } else {
> + Ibc = I2cClockDividerGlitchDisabled[ARRAY_SIZE (I2cClockDividerGlitchDisabled) - 1].Ibc;
This is way too hard to read.
Can you add a preprocessor macro for
X[ARRAY_SIZE (X) - 1] ?
with a helpful name like ARRAY_LAST_ELEM or something?
> + }
> +
> + MmioWrite8 ( (UINTN)&Regs->Ibfd, Ibc);
> + // Reset Module
> + MmioOr8 ( (UINTN)&Regs->Ibcr, I2C_IBCR_MDIS);
> + MmioAnd8 ( (UINTN)&Regs->Ibcr, ~(I2C_IBCR_IBIE | I2C_IBCR_DMAEN));
> + MmioAnd8 ( (UINTN)&Regs->Ibic, (UINT8)(~I2C_IBIC_BIIE));
> +
> + return EFI_SUCCESS;
> +}
> +
> +/**
> + Configure I2c bus to opearte at a given speed
operate
> +
> + @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
Align comments.
> +**/
> +EFI_STATUS
> +I2cInitialize (
> + IN UINTN Base,
> + IN UINT64 I2cBusClock,
> + IN UINT64 Speed
> + )
> +{
> + I2C_REGS *Regs;
> + UINT16 ClockDivider;
> + UINT8 Ibc;
> + I2C_CLOCK_DIVIDER_PAIR *ClockDividerPair;
> + UINT32 ClockDividerPairSize;
> + UINT32 Index;
> +
> + Regs = (I2C_REGS *)Base;
> + if (FeaturePcdGet (PcdI2cErratumA009203)) {
> + I2cErratumA009203 (Base);
> + }
> +
> + Ibc = 0;
> + ClockDivider = (I2cBusClock + Speed - 1) / Speed;
> +
> + if (MmioRead8 ( (UINTN)&Regs->Ibdbg) & I2C_IBDBG_GLFLT_EN) {
> + ClockDividerPair = I2cClockDividerGlitchEnabled;
> + ClockDividerPairSize = ARRAY_SIZE (I2cClockDividerGlitchEnabled);
> + } else {
> + ClockDividerPair = I2cClockDividerGlitchDisabled;
> + ClockDividerPairSize = ARRAY_SIZE (I2cClockDividerGlitchDisabled);
> + }
> +
> + if (ClockDivider > ClockDividerPair[ClockDividerPairSize - 1].Divider) {
> + Ibc = ClockDividerPair[ClockDividerPairSize - 1].Ibc;
> + } else {
> + for (Index = 0; Index < ClockDividerPairSize; Index++) {
> + if (ClockDividerPair[Index].Divider >= ClockDivider) {
> + Ibc = ClockDividerPair[Index].Ibc;
> + break;
> + }
> + }
> + }
> +
> + MmioWrite8 ( (UINTN)&Regs->Ibfd, Ibc);
> + // Reset Module
> + MmioOr8 ( (UINTN)&Regs->Ibcr, I2C_IBCR_MDIS);
> + MmioAnd8 ( (UINTN)&Regs->Ibcr, ~(I2C_IBCR_IBIE | I2C_IBCR_DMAEN));
> + MmioAnd8 ( (UINTN)&Regs->Ibic, (UINT8)(~I2C_IBIC_BIIE));
> +
> + return EFI_SUCCESS;
> +}
> +
> +STATIC
> +EFI_STATUS
> +I2cBusTestBusBusy (
> + IN I2C_REGS *Regs,
> + IN BOOLEAN TestBusy
> + )
> +{
> + UINT8 Index;
> + UINT8 Reg;
> +
> + for (Index = 0; Index < 500; Index++) {
What does looping over something 500 times signify?
Is there a time period we're waiting for or is it just arbitrary?
If it's just arbitrary, put it in a #define and name it accordingly.
> + 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);
Do we need a delay or do we need a barrier? Or do we need both?
> + }
> +
> + if (Index == 500) {
> + return EFI_TIMEOUT;
> + } else {
> + return EFI_SUCCESS;
> + }
> +}
> +
> +STATIC
> +EFI_STATUS
> +I2cTransferComplete (
> + IN I2C_REGS *Regs,
> + IN BOOLEAN TestRxAck
> +)
> +{
> + UINT8 Index;
> + UINT8 Reg;
> +
> + for (Index = 0; Index < 500; Index++) {
Same thing.
> + 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);
Do we need a delay or do we need a barrier? Or do we need both?
> + }
> +
> + if (Index == 500) {
> + 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);
> + // 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;
> +
> + 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));
> +
> + // Generate Start Signal
> + MmioOr8 ( (UINTN)&Regs->Ibcr, I2C_IBCR_MSSL);
> + Status = I2cBusTestBusBusy (Regs, I2C_BUS_TEST_BUSY);
> + if (EFI_ERROR (Status)) {
> + return Status;
> + }
> + 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 setup the i2c bus for reading sysclk
> + @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 setup the i2c bus for reading sysclk
> + @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
Align comments. And try to keep line lengths no longer than 80 characters.
> +**/
> +EFI_STATUS
> +I2cBusReadReg (
> + IN UINTN Base,
> + IN UINT32 SlaveAddress,
> + IN UINT64 RegAddress,
> + IN UINT8 RegAddressWidthInBytes,
> + OUT UINT8 *RegValue,
> + IN UINT8 RegValueNumBytes
> + )
> +{
> + EFI_I2C_OPERATION *Operations;
> + I2C_REG_REQUEST RequestPacket;
> + UINTN OperationCount;
> + UINT8 Address[8];
Create a well named #define for that 8.
> + UINT8 *Ptr;
The name Ptr does not convey information.
Give it a name that describes what it points to.
> + EFI_STATUS Status;
> +
> + ZeroMem (&RequestPacket, sizeof (RequestPacket));
> + OperationCount = 0;
> + Operations = RequestPacket.Operation;
> + Ptr = Address;
> +
> + if (RegAddressWidthInBytes > ARRAY_SIZE (Address)) {
> + return EFI_INVALID_PARAMETER;
> + }
> +
> + if (RegAddressWidthInBytes != 0) {
> + Operations[OperationCount].LengthInBytes = RegAddressWidthInBytes;
> + Operations[OperationCount].Buffer = Ptr;
> + while (RegAddressWidthInBytes--) {
> + *Ptr++ = 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 0000000000..9c8aae100b
> --- /dev/null
> +++ b/Silicon/NXP/Library/I2cLib/I2cLib.inf
> @@ -0,0 +1,30 @@
> +#/** @file
> +#
> +# Copyright 2020 NXP
> +#
> +# SPDX-License-Identifier: BSD-2-Clause
> +#
> +#**/
> +
> +[Defines]
> + INF_VERSION = 1.27
> + BASE_NAME = I2cLib
> + FILE_GUID = f22393b1-98b6-4067-9ec2-6aa436321f03
> + MODULE_TYPE = BASE
> + VERSION_STRING = 1.0
> + LIBRARY_CLASS = I2cLib
> +
> +[Packages]
> + MdePkg/MdePkg.dec
> + Silicon/NXP/NxpQoriqLs.dec
> +
> +[LibraryClasses]
> + TimerLib
> + IoLib
Please sort library classes alphabetically.
> +
> +[Sources.common]
> + I2cLib.c
> +
> +[FeaturePcd]
> + gNxpQoriqLsTokenSpaceGuid.PcdI2cErratumA009203
> +
> diff --git a/Silicon/NXP/Library/I2cLib/I2cLibInternal.h b/Silicon/NXP/Library/I2cLib/I2cLibInternal.h
> new file mode 100644
> index 0000000000..14be9cb740
> --- /dev/null
> +++ b/Silicon/NXP/Library/I2cLib/I2cLibInternal.h
> @@ -0,0 +1,95 @@
> +/** @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__
Please drop leading __ in header guards.
> +
> +#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.
Please re-wrap lines at 80 characters throughout this file.
^
> + */
> +#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
> +
> +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 divider, register value pairs
> + */
> +typedef struct _I2C_CLOCK_DIVIDER_PAIR {
> + UINT16 Divider;
> + UINT16 Ibc;
CamelCase - please expand Ibc.
/
Leif
> +} I2C_CLOCK_DIVIDER_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 764b9bb0e2..4a1cfb3e27 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 @@
> [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 @@
> gNxpQoriqLsTokenSpaceGuid.PcdPciLutBigEndian|FALSE|BOOLEAN|0x00000312
> gNxpQoriqLsTokenSpaceGuid.PcdWatchdogBigEndian|FALSE|BOOLEAN|0x00000313
> gNxpQoriqLsTokenSpaceGuid.PcdIfcBigEndian|FALSE|BOOLEAN|0x00000314
> +
> +[PcdsFeatureFlag]
> + gNxpQoriqLsTokenSpaceGuid.PcdI2cErratumA009203|FALSE|BOOLEAN|0x00000315
> +
> --
> 2.17.1
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#54097): https://edk2.groups.io/g/devel/message/54097
Mute This Topic: https://groups.io/mt/71046321/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
On Sat, 8 Feb 2020 at 18:14, Leif Lindholm <leif@nuviainc.com> wrote:
>
> On Fri, Feb 07, 2020 at 18:13:10 +0530, Pankaj Bansal wrote:
> > 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 clcok generator)
> >
> > since we don't have support of DXE modules this early in boot stage,
> > move the i2c controller functionality in library.
> >
> > Signed-off-by: Pankaj Bansal <pankaj.bansal@nxp.com>
> > ---
> > Platform/NXP/NxpQoriqLs.dsc.inc | 4 +-
> > Silicon/NXP/Include/Library/I2cLib.h | 99 ++++
> > Silicon/NXP/Library/I2cLib/I2cLib.c | 532 ++++++++++++++++++++
> > Silicon/NXP/Library/I2cLib/I2cLib.inf | 30 ++
> > Silicon/NXP/Library/I2cLib/I2cLibInternal.h | 95 ++++
> > Silicon/NXP/NxpQoriqLs.dec | 10 +-
> > 6 files changed, 768 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 fa5f30dd39..b28e0615f7 100644
> > --- a/Platform/NXP/NxpQoriqLs.dsc.inc
> > +++ b/Platform/NXP/NxpQoriqLs.dsc.inc
...
> > + *
> > + * In case of duplicate SCL Divider value, the IBC 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 I2C_CLOCK_DIVIDER_PAIR I2cClockDividerGlitchEnabled[] = {
>
> Module-scope global variables should have 'm' prefix.
>
These should be CONST as well afaict
> > + { 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 divider and hold values when glitch filter is disabled
> > + * taken from table 21-13, LX2160ARM_RevE 01/2020
> > + *
> > + * In case of duplicate SCL Divider value, the IBC 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 I2C_CLOCK_DIVIDER_PAIR I2cClockDividerGlitchDisabled[] = {
>
> Module-scope global variables should have 'm' prefix.
>
> > + { 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
>
> What erratum database does this ID refer to?
> I don't need access to it, but an explanation at the end of the file
> header comment would be helpful.
>
> > +
> > + Description : The clocking circuitry of I2C module may not work reliably due to the slow
> > + rise time of SCL signal.
>
> First line too long (wrap after 'due'). Second line weird indentation.
>
> > + Workaround : Enable the receiver digital filter by setting IBDBG[GLFLT_EN] to 1.
>
> Ideally, wrap this line after 'setting'.
>
> > +*/
> > +STATIC
> > +VOID
> > + I2cErratumA009203 (
>
> No indentation of function name.
> Explanation of where that A prefix of the erratum ID came from would
> also be appreciated.
>
> > + IN UINTN Base
> > + )
> > +{
> > + I2C_REGS *Regs;
> > +
> > + Regs = (I2C_REGS *)Base;
> > +
> > + MmioOr8 ( (UINTN)&Regs->Ibdbg, I2C_IBDBG_GLFLT_EN);
>
> No space before (UINTN).
>
> > +}
> > +
> > +/**
> > + 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 Ibc;
>
> This is a CamelCase project.
> "Regs" is clear enough, but Ibc needs to be written out.
> I'm going to guess it stands for i2c bus clock.
>
> Since we're in a function where the name starts with I2c, we don't
> need to be explicit about that. And we don't seem to be having any
> other clocks to keep track of in this - so how about just calling it
> "Clock"?
>
> But then from the way it's used, should is be called MaxClock?
>
> > +
> > + Regs = (I2C_REGS *)Base;
> > + if (FeaturePcdGet (PcdI2cErratumA009203)) {
> > + I2cErratumA009203 (Base);
> > + }
> > +
> > + if (MmioRead8 ( (UINTN)&Regs->Ibdbg) & I2C_IBDBG_GLFLT_EN) {
>
> No space before (UINTN).
> In general, could you do a global search and replace for "( ("?
> I won't mention it again, but since it is used consistently, I expect
> to see much more of it in this set :)
>
> > + Ibc = I2cClockDividerGlitchEnabled[ARRAY_SIZE (I2cClockDividerGlitchEnabled) - 1].Ibc;
> > + } else {
> > + Ibc = I2cClockDividerGlitchDisabled[ARRAY_SIZE (I2cClockDividerGlitchDisabled) - 1].Ibc;
>
> This is way too hard to read.
> Can you add a preprocessor macro for
> X[ARRAY_SIZE (X) - 1] ?
> with a helpful name like ARRAY_LAST_ELEM or something?
>
> > + }
> > +
> > + MmioWrite8 ( (UINTN)&Regs->Ibfd, Ibc);
> > + // Reset Module
> > + MmioOr8 ( (UINTN)&Regs->Ibcr, I2C_IBCR_MDIS);
> > + MmioAnd8 ( (UINTN)&Regs->Ibcr, ~(I2C_IBCR_IBIE | I2C_IBCR_DMAEN));
> > + MmioAnd8 ( (UINTN)&Regs->Ibic, (UINT8)(~I2C_IBIC_BIIE));
> > +
> > + return EFI_SUCCESS;
> > +}
> > +
> > +/**
> > + Configure I2c bus to opearte at a given speed
>
> operate
>
> > +
> > + @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
>
> Align comments.
>
> > +**/
> > +EFI_STATUS
> > +I2cInitialize (
> > + IN UINTN Base,
> > + IN UINT64 I2cBusClock,
> > + IN UINT64 Speed
> > + )
> > +{
> > + I2C_REGS *Regs;
> > + UINT16 ClockDivider;
> > + UINT8 Ibc;
> > + I2C_CLOCK_DIVIDER_PAIR *ClockDividerPair;
> > + UINT32 ClockDividerPairSize;
> > + UINT32 Index;
> > +
> > + Regs = (I2C_REGS *)Base;
> > + if (FeaturePcdGet (PcdI2cErratumA009203)) {
> > + I2cErratumA009203 (Base);
> > + }
> > +
> > + Ibc = 0;
> > + ClockDivider = (I2cBusClock + Speed - 1) / Speed;
> > +
> > + if (MmioRead8 ( (UINTN)&Regs->Ibdbg) & I2C_IBDBG_GLFLT_EN) {
> > + ClockDividerPair = I2cClockDividerGlitchEnabled;
> > + ClockDividerPairSize = ARRAY_SIZE (I2cClockDividerGlitchEnabled);
> > + } else {
> > + ClockDividerPair = I2cClockDividerGlitchDisabled;
> > + ClockDividerPairSize = ARRAY_SIZE (I2cClockDividerGlitchDisabled);
> > + }
> > +
> > + if (ClockDivider > ClockDividerPair[ClockDividerPairSize - 1].Divider) {
> > + Ibc = ClockDividerPair[ClockDividerPairSize - 1].Ibc;
> > + } else {
> > + for (Index = 0; Index < ClockDividerPairSize; Index++) {
> > + if (ClockDividerPair[Index].Divider >= ClockDivider) {
> > + Ibc = ClockDividerPair[Index].Ibc;
> > + break;
> > + }
> > + }
> > + }
> > +
> > + MmioWrite8 ( (UINTN)&Regs->Ibfd, Ibc);
> > + // Reset Module
> > + MmioOr8 ( (UINTN)&Regs->Ibcr, I2C_IBCR_MDIS);
> > + MmioAnd8 ( (UINTN)&Regs->Ibcr, ~(I2C_IBCR_IBIE | I2C_IBCR_DMAEN));
> > + MmioAnd8 ( (UINTN)&Regs->Ibic, (UINT8)(~I2C_IBIC_BIIE));
> > +
> > + return EFI_SUCCESS;
> > +}
> > +
> > +STATIC
> > +EFI_STATUS
> > +I2cBusTestBusBusy (
> > + IN I2C_REGS *Regs,
> > + IN BOOLEAN TestBusy
> > + )
> > +{
> > + UINT8 Index;
> > + UINT8 Reg;
> > +
> > + for (Index = 0; Index < 500; Index++) {
>
> What does looping over something 500 times signify?
> Is there a time period we're waiting for or is it just arbitrary?
> If it's just arbitrary, put it in a #define and name it accordingly.
>
> > + 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);
>
> Do we need a delay or do we need a barrier? Or do we need both?
>
> > + }
> > +
> > + if (Index == 500) {
> > + return EFI_TIMEOUT;
> > + } else {
> > + return EFI_SUCCESS;
> > + }
> > +}
> > +
> > +STATIC
> > +EFI_STATUS
> > +I2cTransferComplete (
> > + IN I2C_REGS *Regs,
> > + IN BOOLEAN TestRxAck
> > +)
> > +{
> > + UINT8 Index;
> > + UINT8 Reg;
> > +
> > + for (Index = 0; Index < 500; Index++) {
>
> Same thing.
>
> > + 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);
>
> Do we need a delay or do we need a barrier? Or do we need both?
>
> > + }
> > +
> > + if (Index == 500) {
> > + 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);
> > + // 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;
> > +
> > + 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));
> > +
> > + // Generate Start Signal
> > + MmioOr8 ( (UINTN)&Regs->Ibcr, I2C_IBCR_MSSL);
> > + Status = I2cBusTestBusBusy (Regs, I2C_BUS_TEST_BUSY);
> > + if (EFI_ERROR (Status)) {
> > + return Status;
> > + }
> > + 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 setup the i2c bus for reading sysclk
> > + @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 setup the i2c bus for reading sysclk
> > + @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
>
> Align comments. And try to keep line lengths no longer than 80 characters.
>
> > +**/
> > +EFI_STATUS
> > +I2cBusReadReg (
> > + IN UINTN Base,
> > + IN UINT32 SlaveAddress,
> > + IN UINT64 RegAddress,
> > + IN UINT8 RegAddressWidthInBytes,
> > + OUT UINT8 *RegValue,
> > + IN UINT8 RegValueNumBytes
> > + )
> > +{
> > + EFI_I2C_OPERATION *Operations;
> > + I2C_REG_REQUEST RequestPacket;
> > + UINTN OperationCount;
> > + UINT8 Address[8];
>
> Create a well named #define for that 8.
>
> > + UINT8 *Ptr;
>
> The name Ptr does not convey information.
> Give it a name that describes what it points to.
>
> > + EFI_STATUS Status;
> > +
> > + ZeroMem (&RequestPacket, sizeof (RequestPacket));
> > + OperationCount = 0;
> > + Operations = RequestPacket.Operation;
> > + Ptr = Address;
> > +
> > + if (RegAddressWidthInBytes > ARRAY_SIZE (Address)) {
> > + return EFI_INVALID_PARAMETER;
> > + }
> > +
> > + if (RegAddressWidthInBytes != 0) {
> > + Operations[OperationCount].LengthInBytes = RegAddressWidthInBytes;
> > + Operations[OperationCount].Buffer = Ptr;
> > + while (RegAddressWidthInBytes--) {
> > + *Ptr++ = 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 0000000000..9c8aae100b
> > --- /dev/null
> > +++ b/Silicon/NXP/Library/I2cLib/I2cLib.inf
> > @@ -0,0 +1,30 @@
> > +#/** @file
> > +#
> > +# Copyright 2020 NXP
> > +#
> > +# SPDX-License-Identifier: BSD-2-Clause
> > +#
> > +#**/
> > +
> > +[Defines]
> > + INF_VERSION = 1.27
> > + BASE_NAME = I2cLib
> > + FILE_GUID = f22393b1-98b6-4067-9ec2-6aa436321f03
> > + MODULE_TYPE = BASE
> > + VERSION_STRING = 1.0
> > + LIBRARY_CLASS = I2cLib
> > +
> > +[Packages]
> > + MdePkg/MdePkg.dec
> > + Silicon/NXP/NxpQoriqLs.dec
> > +
> > +[LibraryClasses]
> > + TimerLib
> > + IoLib
>
> Please sort library classes alphabetically.
>
> > +
> > +[Sources.common]
> > + I2cLib.c
> > +
> > +[FeaturePcd]
> > + gNxpQoriqLsTokenSpaceGuid.PcdI2cErratumA009203
> > +
> > diff --git a/Silicon/NXP/Library/I2cLib/I2cLibInternal.h b/Silicon/NXP/Library/I2cLib/I2cLibInternal.h
> > new file mode 100644
> > index 0000000000..14be9cb740
> > --- /dev/null
> > +++ b/Silicon/NXP/Library/I2cLib/I2cLibInternal.h
> > @@ -0,0 +1,95 @@
> > +/** @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__
>
> Please drop leading __ in header guards.
>
> > +
> > +#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.
>
> Please re-wrap lines at 80 characters throughout this file.
> ^
>
> > + */
> > +#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
> > +
> > +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 divider, register value pairs
> > + */
> > +typedef struct _I2C_CLOCK_DIVIDER_PAIR {
> > + UINT16 Divider;
> > + UINT16 Ibc;
>
> CamelCase - please expand Ibc.
>
> /
> Leif
>
> > +} I2C_CLOCK_DIVIDER_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 764b9bb0e2..4a1cfb3e27 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 @@
> > [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 @@
> > gNxpQoriqLsTokenSpaceGuid.PcdPciLutBigEndian|FALSE|BOOLEAN|0x00000312
> > gNxpQoriqLsTokenSpaceGuid.PcdWatchdogBigEndian|FALSE|BOOLEAN|0x00000313
> > gNxpQoriqLsTokenSpaceGuid.PcdIfcBigEndian|FALSE|BOOLEAN|0x00000314
> > +
> > +[PcdsFeatureFlag]
> > + gNxpQoriqLsTokenSpaceGuid.PcdI2cErratumA009203|FALSE|BOOLEAN|0x00000315
> > +
> > --
> > 2.17.1
> >
>
>
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#54102): https://edk2.groups.io/g/devel/message/54102
Mute This Topic: https://groups.io/mt/71046321/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
© 2016 - 2024 Red Hat, Inc.