> > +/**
> > +  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;
> > +
> > +  Regs = (I2C_REGS *)Base;
> > +  if (FeaturePcdGet (PcdI2cErratumA009203)) {
> > +    // Apply Erratum A-009203 before writing Ibfd register
> 
> It is an improvement, but there is still nothing in here that makes it
> obvious why this is being done twice. The referenced u-boot patch does
> it only once.

It's not necessary to call I2cEarlyInitialize for all I2c controllers.
This function has been written so that i2c bus can be initialized to read the 
system clock
from a clock generator or an FPFA.

To make a call to I2cInitialize, we need to know the I2cBusClock, which is a 
derivative of
System clock. I2cBusClock is calculated after PLL multiplication to system 
clock.
Therefore, we won't be able to call I2cInitialize without knowing System clock.

The idea is that for i2c controller to which clock generator or an FPFA is 
connected, we first
Call I2cEarlyInitialize. Then we read system clock and then we can call 
I2cInitialize for that
Controller as well as any other i2c controller(s) in the system.

So, for all i2c controllers (except one) I2cInitialize would be called and not 
I2cEarlyInitialize

> 
> Hmm, furthermore, I don't see this function called at all? Why is it
> included? If you delete it (and its declaration in .h), I'm OK with
> the result.
> 
> > +    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;
> > +}
> >

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#58023): https://edk2.groups.io/g/devel/message/58023
Mute This Topic: https://groups.io/mt/73008796/21656
Group Owner: [email protected]
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [[email protected]]
-=-=-=-=-=-=-=-=-=-=-=-

Reply via email to