On Mon, Apr 23, 2018 at 08:21:22AM +0000, Meenakshi Aggarwal wrote: > > > +/** > > > + Function to read data using i2c bus > > > + > > > + @param I2cBus I2c Controller number > > > + @param Chip Address of slave device from where data to be > > > read > > > + @param Offset Offset of slave memory > > > + @param Alen Address length of slave > > > + @param Buffer A pointer to the destination buffer for the > > > data > > > + @param Len Length of data to be read > > > + > > > + @retval EFI_NOT_READY Arbitration lost > > > + @retval EFI_TIMEOUT Failed to initialize data transfer in > > > predefined > > time > > > + @retval EFI_NOT_FOUND ACK was not recieved > > > + @retval EFI_SUCCESS Read was successful > > > + > > > +**/ > > > +STATIC > > > +EFI_STATUS > > > +I2cDataRead ( > > > + IN UINT32 I2cBus, > > > + IN UINT8 Chip, > > > + IN UINT32 Offset, > > > + IN UINT32 Alen, > > > + IN UINT8 *Buffer, > > > + IN UINT32 Len > > > + ) > > > +{ > > > + EFI_STATUS Status; > > > + UINT32 Temp; > > > + INT32 I; > > > + I2C_REGS *I2cRegs; > > > + > > > + I2cRegs = (I2C_REGS *)(FixedPcdGet64 (PcdI2c0BaseAddr + > > > + (I2cBus * FixedPcdGet32 (PcdI2cSize)))); > > > > Please get rid of this hardcoded base address and use NonDiscoverable > > Have a look at Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/ and > > Silicon/Socionext/SynQuacer/Drivers/SynQuacerI2cDxe/ > > for example. > > > I have checked SynQuacer code and i dont see its adding much advantage.
What it gives is the ability to cover more than one controller by this driver, regardless of whether you need it for this particular platform port or not. > There also base address is hard coded SYNQUACER_I2C1_BASE in > PlatformDxe driver. Yes, the PlatformDxe driver statically instantiates dynamic drivers for non-discoverable buses. But there is no longer any hard-coded addresses in the device driver itself. / Leif _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel