From b9dac6a932c6d5b03a090d227f404e1c43522026 Mon Sep 17 00:00:00 2001 From: eh-steve <16373174+eh-steve@users.noreply.github.com> Date: Fri, 19 May 2023 17:44:44 +0100 Subject: [PATCH 1/5] fix: bug where I2C slave device index was not guaranteed to be globally unique This could (and did) happen in the following cases: * the device tree node had no phandle (so would default to 0, which could collide with other devices which also don't have a phandle) * or if the device tree phandle address happened to be the same as the Count of EEPROM devices * or if an atmel compatible EEPROM was defined directly under the i2c bus, this `Count` could (incorrectly) collide with the `Count` used for /eeprom-manager defined devices Instead of using a mix of phandles and Counts, this commit switches to using ((ControllerID << 16) | NumberOfI2cDevices), which should be globally unique across all buses Signed-off-by: eh-steve <16373174+eh-steve@users.noreply.github.com> --- Silicon/NVIDIA/Drivers/TegraI2c/TegraI2c.h | 1 - Silicon/NVIDIA/Drivers/TegraI2c/TegraI2cDxe.c | 34 ++++++------------- 2 files changed, 10 insertions(+), 25 deletions(-) diff --git a/Silicon/NVIDIA/Drivers/TegraI2c/TegraI2c.h b/Silicon/NVIDIA/Drivers/TegraI2c/TegraI2c.h index 9beb126b96..f6f0d8e06a 100644 --- a/Silicon/NVIDIA/Drivers/TegraI2c/TegraI2c.h +++ b/Silicon/NVIDIA/Drivers/TegraI2c/TegraI2c.h @@ -54,7 +54,6 @@ typedef struct { UINT32 ControllerId; UINTN BusClockHertz; - UINT32 BusId; VOID *DeviceTreeBase; INT32 DeviceTreeNodeOffset; diff --git a/Silicon/NVIDIA/Drivers/TegraI2c/TegraI2cDxe.c b/Silicon/NVIDIA/Drivers/TegraI2c/TegraI2cDxe.c index 4c4ff19693..6bda9d18fd 100644 --- a/Silicon/NVIDIA/Drivers/TegraI2c/TegraI2cDxe.c +++ b/Silicon/NVIDIA/Drivers/TegraI2c/TegraI2cDxe.c @@ -948,7 +948,6 @@ TegraI2cEnableI2cBusConfiguration ( @param[in] Private Driver's private data. @param[in] I2cAddress Address of the device. @param[in] DeviceGuid GUID to identify the device type. - @param[in] DeviceIndex Index of the device derived from device tree. @retval EFI_SUCCESS Device added. @retval other Some error occurs when device is being added. @@ -958,8 +957,7 @@ EFI_STATUS TegraI2cAddDevice ( IN NVIDIA_TEGRA_I2C_PRIVATE_DATA *Private, IN UINT32 I2cAddress, - IN EFI_GUID *DeviceGuid, - IN UINT32 DeviceIndex + IN EFI_GUID *DeviceGuid ) { if (Private->NumberOfI2cDevices >= MAX_I2C_DEVICES) { @@ -970,7 +968,7 @@ TegraI2cAddDevice ( Private->SlaveAddressArray[Private->NumberOfI2cDevices * MAX_SLAVES_PER_DEVICE] = I2cAddress; Private->I2cDevices[Private->NumberOfI2cDevices].DeviceGuid = DeviceGuid; - Private->I2cDevices[Private->NumberOfI2cDevices].DeviceIndex = DeviceIndex; + Private->I2cDevices[Private->NumberOfI2cDevices].DeviceIndex = Private->NumberOfI2cDevices | (Private->ControllerId << 16); // Makes sure this is unique, even if a phandle is missing due to not being referenced elsewhere in the DT; Private->I2cDevices[Private->NumberOfI2cDevices].HardwareRevision = 1; Private->I2cDevices[Private->NumberOfI2cDevices].I2cBusConfiguration = 0; Private->I2cDevices[Private->NumberOfI2cDevices].SlaveAddressCount = 1; @@ -1024,7 +1022,6 @@ TegraI2CDriverBindingStart ( EFI_DEVICE_PATH *OldDevicePath; EFI_DEVICE_PATH *NewDevicePath; EFI_DEVICE_PATH_PROTOCOL *DevicePathNode; - UINT32 Count; Status = gBS->HandleProtocol ( ControllerHandle, @@ -1202,7 +1199,6 @@ TegraI2CDriverBindingStart ( Private->NumberOfI2cDevices = 0; I2cNodeHandle = fdt_get_phandle (DeviceTreeNode->DeviceTreeBase, DeviceTreeNode->NodeOffset); - Count = 0; fdt_for_each_subnode (I2cNodeOffset, DeviceTreeNode->DeviceTreeBase, DeviceTreeNode->NodeOffset) { if (fdt_node_check_compatible ( DeviceTreeNode->DeviceTreeBase, @@ -1219,14 +1215,11 @@ TegraI2CDriverBindingStart ( Status = TegraI2cAddDevice ( Private, I2cAddress, - DeviceGuid, - Count + DeviceGuid ); if (EFI_ERROR (Status)) { goto ErrorExit; } - - Count++; DEBUG ((DEBUG_INFO, "%a: Eeprom Slave Address: 0x%lx on I2c Bus 0x%lx.\n", __FUNCTION__, I2cAddress, Private->ControllerId)); } } else if (fdt_node_check_compatible ( @@ -1244,8 +1237,7 @@ TegraI2CDriverBindingStart ( Status = TegraI2cAddDevice ( Private, I2cAddress, - DeviceGuid, - fdt_get_phandle (DeviceTreeNode->DeviceTreeBase, I2cNodeOffset) + DeviceGuid ); if (EFI_ERROR (Status)) { goto ErrorExit; @@ -1268,8 +1260,7 @@ TegraI2CDriverBindingStart ( Status = TegraI2cAddDevice ( Private, I2cAddress, - DeviceGuid, - fdt_get_phandle (DeviceTreeNode->DeviceTreeBase, I2cNodeOffset) + DeviceGuid ); if (EFI_ERROR (Status)) { goto ErrorExit; @@ -1292,8 +1283,7 @@ TegraI2CDriverBindingStart ( Status = TegraI2cAddDevice ( Private, I2cAddress, - DeviceGuid, - fdt_get_phandle (DeviceTreeNode->DeviceTreeBase, I2cNodeOffset) + DeviceGuid ); if (EFI_ERROR (Status)) { goto ErrorExit; @@ -1314,8 +1304,7 @@ TegraI2CDriverBindingStart ( Status = TegraI2cAddDevice ( Private, I2cAddress, - DeviceGuid, - fdt_get_phandle (DeviceTreeNode->DeviceTreeBase, I2cNodeOffset) + DeviceGuid ); if (EFI_ERROR (Status)) { goto ErrorExit; @@ -1336,8 +1325,7 @@ TegraI2CDriverBindingStart ( Status = TegraI2cAddDevice ( Private, I2cAddress, - DeviceGuid, - fdt_get_phandle (DeviceTreeNode->DeviceTreeBase, I2cNodeOffset) + DeviceGuid ); if (EFI_ERROR (Status)) { goto ErrorExit; @@ -1349,7 +1337,7 @@ TegraI2CDriverBindingStart ( } } - Count = 0; + EepromManagerNodeOffset = fdt_path_offset (DeviceTreeNode->DeviceTreeBase, "/eeprom-manager"); if (EepromManagerNodeOffset >= 0) { fdt_for_each_subnode (EepromManagerBusNodeOffset, DeviceTreeNode->DeviceTreeBase, EepromManagerNodeOffset) { @@ -1376,14 +1364,12 @@ TegraI2CDriverBindingStart ( Status = TegraI2cAddDevice ( Private, I2cAddress, - DeviceGuid, - Count + DeviceGuid ); if (EFI_ERROR (Status)) { goto ErrorExit; } - Count++; DEBUG ((DEBUG_INFO, "%a: Eeprom Slave Address: 0x%lx on I2c Bus 0x%lx.\n", __FUNCTION__, I2cAddress, I2cBusHandle)); } } From 62bda594601d7ec50a7b9e5ca85c7962d92abcfc Mon Sep 17 00:00:00 2001 From: eh-steve <16373174+eh-steve@users.noreply.github.com> Date: Fri, 19 May 2023 19:09:53 +0100 Subject: [PATCH 2/5] feat: I2C mux/switch devices via EFI_I2C_BUS_CONFIGURATION_MANAGEMENT_PROTOCOL Previously I2C devices behind a mux or switch were not visible to UEFI, and camera EEPROMs behind a mux would not get probed during boot. This commit adds support for all pca954x type I2C mux/switches, which the TCA9548A on the reference e3333 interposer board is compatible with. This allows the device tree to define an I2c mux using a `compatible = "nxp,pca9547"` prop, with child nodes inside, and for UEFI to be able to read camera EEPROMS behind these muxes. It does not yet add support for "i2c-mux-gpio" type muxes, but could potentially be added later. Signed-off-by: eh-steve <16373174+eh-steve@users.noreply.github.com> --- Silicon/NVIDIA/Drivers/TegraI2c/TegraI2c.h | 26 ++- Silicon/NVIDIA/Drivers/TegraI2c/TegraI2cDxe.c | 163 ++++++++++++++++-- .../NVIDIA/Drivers/TegraI2c/TegraI2cDxe.inf | 1 + Silicon/NVIDIA/NVIDIA.dec | 1 + 4 files changed, 170 insertions(+), 21 deletions(-) diff --git a/Silicon/NVIDIA/Drivers/TegraI2c/TegraI2c.h b/Silicon/NVIDIA/Drivers/TegraI2c/TegraI2c.h index f6f0d8e06a..fa900aa19c 100644 --- a/Silicon/NVIDIA/Drivers/TegraI2c/TegraI2c.h +++ b/Silicon/NVIDIA/Drivers/TegraI2c/TegraI2c.h @@ -22,6 +22,27 @@ #define MAX_I2C_DEVICES 16 #define MAX_SLAVES_PER_DEVICE 1 +enum MuxType { + MUX = 0, + SWITCH = 1, + GPIO_CONTROLLED = 2, // Not supported yet, but will leave it here as bait +}; + +typedef struct { + enum MuxType MuxType :8; + UINT8 MuxAddress :8; // Only 7 bit addressing supported for now + UINT8 EnableBitNumber :8; // either 0, 2 or 3 + UINT8 Channel :8; // 0-7 +} BUS_CONFIGURATION_FIELDS; + +// This approach of encoding all mux configuration into a UINT32 isn't strictly compliant with the UEFI spec, +// which suggests that configurations should be sequential indices rather than packed data +typedef union { + BUS_CONFIGURATION_FIELDS Fields; + UINT32 Value; +} NVIDIA_TEGRA_I2C_BUS_CONFIGURATION; + + typedef struct { // // Standard signature used to identify TegraI2c private data @@ -69,8 +90,9 @@ typedef struct { BOOLEAN SkipOnExitDisabled; } NVIDIA_TEGRA_I2C_PRIVATE_DATA; -#define TEGRA_I2C_PRIVATE_DATA_FROM_MASTER(a) CR(a, NVIDIA_TEGRA_I2C_PRIVATE_DATA, I2cMaster, TEGRA_I2C_SIGNATURE) -#define TEGRA_I2C_PRIVATE_DATA_FROM_ENUMERATE(a) CR(a, NVIDIA_TEGRA_I2C_PRIVATE_DATA, I2cEnumerate, TEGRA_I2C_SIGNATURE) +#define TEGRA_I2C_PRIVATE_DATA_FROM_MASTER(a) CR(a, NVIDIA_TEGRA_I2C_PRIVATE_DATA, I2cMaster, TEGRA_I2C_SIGNATURE) +#define TEGRA_I2C_PRIVATE_DATA_FROM_ENUMERATE(a) CR(a, NVIDIA_TEGRA_I2C_PRIVATE_DATA, I2cEnumerate, TEGRA_I2C_SIGNATURE) +#define TEGRA_I2C_PRIVATE_DATA_FROM_BUS_CONFIGURATION(a) CR(a, NVIDIA_TEGRA_I2C_PRIVATE_DATA, I2CConfiguration, TEGRA_I2C_SIGNATURE) /** * @addtogroup SPEED_MODES I2C Mode frequencies diff --git a/Silicon/NVIDIA/Drivers/TegraI2c/TegraI2cDxe.c b/Silicon/NVIDIA/Drivers/TegraI2c/TegraI2cDxe.c index 6bda9d18fd..20fbf5d389 100644 --- a/Silicon/NVIDIA/Drivers/TegraI2c/TegraI2cDxe.c +++ b/Silicon/NVIDIA/Drivers/TegraI2c/TegraI2cDxe.c @@ -859,10 +859,6 @@ TegraI2cGetBusFrequency ( Private = TEGRA_I2C_PRIVATE_DATA_FROM_ENUMERATE (This); - if (0 != I2cBusConfiguration) { - return EFI_NO_MAPPING; - } - *BusClockHertz = Private->BusClockHertz; return EFI_SUCCESS; } @@ -926,8 +922,32 @@ TegraI2cEnableI2cBusConfiguration ( IN EFI_STATUS *I2cStatus OPTIONAL ) { + NVIDIA_TEGRA_I2C_PRIVATE_DATA *Private = NULL; + EFI_I2C_REQUEST_PACKET RequestPacket; + if (I2cBusConfiguration != 0) { - return EFI_NO_MAPPING; + EFI_STATUS I2CRequestStatus; + + Private = TEGRA_I2C_PRIVATE_DATA_FROM_BUS_CONFIGURATION (This); + + // There must be an I2C mux or switch (currently we only support PCA954x compatible muxes) + NVIDIA_TEGRA_I2C_BUS_CONFIGURATION Config; + Config.Value = I2cBusConfiguration; + + UINT8 MuxChannel = 0; + if (Config.Fields.MuxType == MUX) { + MuxChannel = Config.Fields.Channel; + MuxChannel |= (1 << Config.Fields.EnableBitNumber); + } else if (Config.Fields.MuxType == SWITCH) { + MuxChannel = (1 << Config.Fields.Channel); // This forces a single channel at a time and makes switches behave like muxes, but that's ok... + } + RequestPacket.OperationCount = 1; + RequestPacket.Operation[0].Flags = 0; // Write + RequestPacket.Operation[0].LengthInBytes = sizeof(MuxChannel); + RequestPacket.Operation[0].Buffer = &MuxChannel; + + I2CRequestStatus = Private->I2cMaster.StartRequest(&Private->I2cMaster, Config.Fields.MuxAddress, &RequestPacket, NULL, NULL); + DEBUG((DEBUG_INFO, "%a: Got a non-zero I2cBusConfiguration %x, Mux: %x, Channel: %x, setting I2C Mux register status: %d\n", __FUNCTION__, I2cBusConfiguration, Config.Fields.MuxAddress, Config.Fields.Channel, I2CRequestStatus)); } if (NULL != Event) { @@ -945,19 +965,21 @@ TegraI2cEnableI2cBusConfiguration ( /** This routine is called to add an I2C device to the controller. - @param[in] Private Driver's private data. - @param[in] I2cAddress Address of the device. - @param[in] DeviceGuid GUID to identify the device type. + @param[in] Private Driver's private data. + @param[in] I2cAddress Address of the device. + @param[in] DeviceGuid GUID to identify the device type. + @param[in] I2cBusConfiguration The bus configuration (mux + switch settings) needed to access the device - @retval EFI_SUCCESS Device added. - @retval other Some error occurs when device is being added. + @retval EFI_SUCCESS Device added. + @retval other Some error occurs when device is being added. **/ EFI_STATUS TegraI2cAddDevice ( IN NVIDIA_TEGRA_I2C_PRIVATE_DATA *Private, IN UINT32 I2cAddress, - IN EFI_GUID *DeviceGuid + IN EFI_GUID *DeviceGuid, + IN UINT32 I2cBusConfiguration ) { if (Private->NumberOfI2cDevices >= MAX_I2C_DEVICES) { @@ -970,7 +992,7 @@ TegraI2cAddDevice ( Private->I2cDevices[Private->NumberOfI2cDevices].DeviceGuid = DeviceGuid; Private->I2cDevices[Private->NumberOfI2cDevices].DeviceIndex = Private->NumberOfI2cDevices | (Private->ControllerId << 16); // Makes sure this is unique, even if a phandle is missing due to not being referenced elsewhere in the DT; Private->I2cDevices[Private->NumberOfI2cDevices].HardwareRevision = 1; - Private->I2cDevices[Private->NumberOfI2cDevices].I2cBusConfiguration = 0; + Private->I2cDevices[Private->NumberOfI2cDevices].I2cBusConfiguration = I2cBusConfiguration; Private->I2cDevices[Private->NumberOfI2cDevices].SlaveAddressCount = 1; Private->I2cDevices[Private->NumberOfI2cDevices].SlaveAddressArray = &Private->SlaveAddressArray[Private->NumberOfI2cDevices * MAX_SLAVES_PER_DEVICE]; Private->NumberOfI2cDevices++; @@ -1215,7 +1237,8 @@ TegraI2CDriverBindingStart ( Status = TegraI2cAddDevice ( Private, I2cAddress, - DeviceGuid + DeviceGuid, + 0 ); if (EFI_ERROR (Status)) { goto ErrorExit; @@ -1237,7 +1260,8 @@ TegraI2CDriverBindingStart ( Status = TegraI2cAddDevice ( Private, I2cAddress, - DeviceGuid + DeviceGuid, + 0 ); if (EFI_ERROR (Status)) { goto ErrorExit; @@ -1260,7 +1284,8 @@ TegraI2CDriverBindingStart ( Status = TegraI2cAddDevice ( Private, I2cAddress, - DeviceGuid + DeviceGuid, + 0 ); if (EFI_ERROR (Status)) { goto ErrorExit; @@ -1268,6 +1293,102 @@ TegraI2CDriverBindingStart ( DEBUG ((DEBUG_INFO, "%a: PCA9535 Slave Address: 0x%lx on I2c Bus 0x%lx.\n", __FUNCTION__, I2cAddress, Private->ControllerId)); } + } else if (fdt_node_check_compatible(DeviceTreeNode->DeviceTreeBase, I2cNodeOffset, "nxp,pca9540") == 0 || + fdt_node_check_compatible(DeviceTreeNode->DeviceTreeBase, I2cNodeOffset, "nxp,pca9542") == 0 || + fdt_node_check_compatible(DeviceTreeNode->DeviceTreeBase, I2cNodeOffset, "nxp,pca9543") == 0 || + fdt_node_check_compatible(DeviceTreeNode->DeviceTreeBase, I2cNodeOffset, "nxp,pca9544") == 0 || + fdt_node_check_compatible(DeviceTreeNode->DeviceTreeBase, I2cNodeOffset, "nxp,pca9545") == 0 || + fdt_node_check_compatible(DeviceTreeNode->DeviceTreeBase, I2cNodeOffset, "nxp,pca9546") == 0 || + fdt_node_check_compatible(DeviceTreeNode->DeviceTreeBase, I2cNodeOffset, "nxp,pca9547") == 0 || + fdt_node_check_compatible(DeviceTreeNode->DeviceTreeBase, I2cNodeOffset, "nxp,pca9548") == 0) { + + enum MuxType MuxTypeVal; + UINT8 EnableBit = 0; + if (fdt_node_check_compatible(DeviceTreeNode->DeviceTreeBase, I2cNodeOffset, "nxp,pca9540") == 0 || + fdt_node_check_compatible(DeviceTreeNode->DeviceTreeBase, I2cNodeOffset, "nxp,pca9542") == 0 || + fdt_node_check_compatible(DeviceTreeNode->DeviceTreeBase, I2cNodeOffset, "nxp,pca9544") == 0) { + MuxTypeVal = MUX; + EnableBit = 2; + } else if (fdt_node_check_compatible(DeviceTreeNode->DeviceTreeBase, I2cNodeOffset, "nxp,pca9547") == 0) { + MuxTypeVal = MUX; + EnableBit = 3; + } else if (fdt_node_check_compatible(DeviceTreeNode->DeviceTreeBase, I2cNodeOffset, "nxp,pca9543") == 0 || + fdt_node_check_compatible(DeviceTreeNode->DeviceTreeBase, I2cNodeOffset, "nxp,pca9545") == 0 || + fdt_node_check_compatible(DeviceTreeNode->DeviceTreeBase, I2cNodeOffset, "nxp,pca9546") == 0 || + fdt_node_check_compatible(DeviceTreeNode->DeviceTreeBase, I2cNodeOffset, "nxp,pca9548") == 0) { + MuxTypeVal = SWITCH; + } + + Property = fdt_getprop(DeviceTreeNode->DeviceTreeBase, I2cNodeOffset, "reg", &PropertyLen); + if ((Property != NULL) && (PropertyLen == sizeof(UINT32))) { + gBS->CopyMem(&I2cAddress, (VOID *) Property, PropertyLen); + I2cAddress = SwapBytes32(I2cAddress); + DEBUG((DEBUG_INFO, "%a: PCA954x Found.\n", __FUNCTION__)); + DeviceGuid = &gNVIDIAI2cPca954x; + Status = TegraI2cAddDevice( + Private, + I2cAddress, + DeviceGuid, + 0 + ); + if (EFI_ERROR(Status)) { + goto ErrorExit; + } + + INT32 I2cMuxNodeOffset = I2cNodeOffset; + DEBUG((DEBUG_INFO, "%a: PCA954x Slave Address: 0x%lx on I2c Bus 0x%lx.\n", __FUNCTION__, I2cAddress, Private->ControllerId)); + INT32 MuxI2CControlAddress = I2cAddress; + + if ((Property != NULL) && (PropertyLen == sizeof(UINT32))) { + fdt_for_each_subnode(I2cMuxNodeOffset, DeviceTreeNode->DeviceTreeBase, I2cNodeOffset) { + UINT32 I2cMuxChannelReg; + + Property = fdt_getprop(DeviceTreeNode->DeviceTreeBase, I2cMuxNodeOffset, "reg", &PropertyLen); + if ((Property != NULL) && (PropertyLen == sizeof(UINT32))) { + gBS->CopyMem(&I2cMuxChannelReg, (VOID *) Property, PropertyLen); + I2cMuxChannelReg = SwapBytes32(I2cMuxChannelReg); + INT32 I2cMuxSubNodeOffset = I2cNodeOffset; + fdt_for_each_subnode(I2cMuxSubNodeOffset, DeviceTreeNode->DeviceTreeBase, I2cMuxNodeOffset) { + + if (fdt_node_check_compatible( + DeviceTreeNode->DeviceTreeBase, + I2cMuxSubNodeOffset, + "atmel,24c02") == 0) { + + UINT32 I2cAddressEeprom; + Property = fdt_getprop(DeviceTreeNode->DeviceTreeBase, I2cMuxSubNodeOffset, "reg", &PropertyLen); + if ((Property != NULL) && (PropertyLen == sizeof(UINT32))) { + gBS->CopyMem(&I2cAddressEeprom, (VOID *) Property, PropertyLen); + I2cAddressEeprom = SwapBytes32(I2cAddressEeprom); + DeviceGuid = &gNVIDIAEeprom; + // encodes all the information needed to set the mux channel into 32 bits. + // Might be more compliant to allocate an array in Private and index that instead though? + NVIDIA_TEGRA_I2C_BUS_CONFIGURATION BusConfiguration; + BusConfiguration.Value = 0; + BusConfiguration.Fields.MuxAddress = MuxI2CControlAddress; + BusConfiguration.Fields.Channel = I2cMuxChannelReg; + BusConfiguration.Fields.EnableBitNumber = EnableBit; + BusConfiguration.Fields.MuxType = MuxTypeVal; + + Status = TegraI2cAddDevice( + Private, + I2cAddressEeprom, + DeviceGuid, + BusConfiguration.Value + ); + + if (EFI_ERROR(Status)) { + goto ErrorExit; + } + + DEBUG((DEBUG_INFO, "%a: Got an atmel,24c02 at mux channel %x with reg address: %x\n", __FUNCTION__, I2cMuxChannelReg, I2cAddressEeprom)); + } + } + } + } + } + } + } } else if (fdt_node_check_compatible ( DeviceTreeNode->DeviceTreeBase, I2cNodeOffset, @@ -1283,7 +1404,8 @@ TegraI2CDriverBindingStart ( Status = TegraI2cAddDevice ( Private, I2cAddress, - DeviceGuid + DeviceGuid, + 0 ); if (EFI_ERROR (Status)) { goto ErrorExit; @@ -1304,7 +1426,8 @@ TegraI2CDriverBindingStart ( Status = TegraI2cAddDevice ( Private, I2cAddress, - DeviceGuid + DeviceGuid, + 0 ); if (EFI_ERROR (Status)) { goto ErrorExit; @@ -1325,7 +1448,8 @@ TegraI2CDriverBindingStart ( Status = TegraI2cAddDevice ( Private, I2cAddress, - DeviceGuid + DeviceGuid, + 0 ); if (EFI_ERROR (Status)) { goto ErrorExit; @@ -1364,7 +1488,8 @@ TegraI2CDriverBindingStart ( Status = TegraI2cAddDevice ( Private, I2cAddress, - DeviceGuid + DeviceGuid, + 0 ); if (EFI_ERROR (Status)) { goto ErrorExit; diff --git a/Silicon/NVIDIA/Drivers/TegraI2c/TegraI2cDxe.inf b/Silicon/NVIDIA/Drivers/TegraI2c/TegraI2cDxe.inf index ebfb9724b8..aaf0d4f968 100644 --- a/Silicon/NVIDIA/Drivers/TegraI2c/TegraI2cDxe.inf +++ b/Silicon/NVIDIA/Drivers/TegraI2c/TegraI2cDxe.inf @@ -52,6 +52,7 @@ gNVIDIAEeprom gNVIDIAI2cTca9539 gNVIDIAI2cPca9535 + gNVIDIAI2cPca954x gNVIDIANonDiscoverableI2cDeviceGuid gNVIDIAI2cNcp81599 gNVIDIAI2cNct3018y diff --git a/Silicon/NVIDIA/NVIDIA.dec b/Silicon/NVIDIA/NVIDIA.dec index 35727d20b1..b9087ebb44 100644 --- a/Silicon/NVIDIA/NVIDIA.dec +++ b/Silicon/NVIDIA/NVIDIA.dec @@ -61,6 +61,7 @@ gNVIDIAEeprom = { 0xe7da2b8d, 0x25bd, 0x4e6f, { 0xac, 0xfc, 0x3b, 0x62, 0x18, 0x70, 0x73, 0xbd } } gNVIDIAI2cTca9539 = { 0x640bcec1, 0x2c6a, 0x4be6, { 0x86, 0x59, 0x5a, 0x37, 0xaa, 0x0c, 0xc8, 0x30 } } gNVIDIAI2cPca9535 = { 0x216eac27, 0xdae3, 0x43b1, { 0x93, 0xd8, 0x8d, 0xc7, 0xe3, 0x94, 0x88, 0x91 } } + gNVIDIAI2cPca954x = { 0xd18d8e54, 0xa753, 0x4385, { 0xb9, 0x28, 0xbf, 0xb9, 0x66, 0xeb, 0x02, 0x7f } } gNVIDIAI2cNcp81599 = { 0x9a6029ec, 0xee77, 0x4f8a, { 0x8d, 0x21, 0xb7, 0x11, 0xc8, 0x5a, 0x57, 0xa1 } } gNVIDIAI2cBmcSSIF = { 0xb4fcca9e, 0x93ec, 0x4fb5, { 0x87, 0x81, 0x54, 0x07, 0x0c, 0x54, 0x39, 0x06 } } gNVIDIAI2cUnknown = { 0xc58c5085, 0xafed, 0x4844, { 0x81, 0x06, 0x2a, 0xc6, 0xf2, 0x5a, 0xf5, 0xd7 } } From 3f8dbf4e55255e75493ab130f3bc1f72a76f08d2 Mon Sep 17 00:00:00 2001 From: eh-steve <16373174+eh-steve@users.noreply.github.com> Date: Fri, 19 May 2023 19:16:58 +0100 Subject: [PATCH 3/5] feat: TegraI2cSlaveDeviceTreeProtocol To allow I2cIo drivers (and any other I2C driver) to lookup device tree nodes using DeviceGUID + DeviceIndex. This allows I2C device drivers to get more information about their hardware (e.g. full device tree paths of I2C devices, and read device tree properties) Signed-off-by: eh-steve <16373174+eh-steve@users.noreply.github.com> --- Silicon/NVIDIA/Drivers/TegraI2c/TegraI2c.h | 4 + Silicon/NVIDIA/Drivers/TegraI2c/TegraI2cDxe.c | 92 ++++++++++++++----- .../NVIDIA/Drivers/TegraI2c/TegraI2cDxe.inf | 1 + .../Protocol/TegraI2cSlaveDeviceTreeNode.h | 21 +++++ Silicon/NVIDIA/NVIDIA.dec | 1 + 5 files changed, 97 insertions(+), 22 deletions(-) create mode 100644 Silicon/NVIDIA/Include/Protocol/TegraI2cSlaveDeviceTreeNode.h diff --git a/Silicon/NVIDIA/Drivers/TegraI2c/TegraI2c.h b/Silicon/NVIDIA/Drivers/TegraI2c/TegraI2c.h index fa900aa19c..9bb764920f 100644 --- a/Silicon/NVIDIA/Drivers/TegraI2c/TegraI2c.h +++ b/Silicon/NVIDIA/Drivers/TegraI2c/TegraI2c.h @@ -57,6 +57,8 @@ typedef struct { EFI_I2C_ENUMERATE_PROTOCOL I2cEnumerate; EFI_I2C_BUS_CONFIGURATION_MANAGEMENT_PROTOCOL I2CConfiguration; + NVIDIA_TEGRA_I2C_SLAVE_DEVICE_TREE_NODE_PROTOCOL I2cSlaveDeviceTreeNode; + // // Indicates if the protocols are installed // @@ -83,6 +85,7 @@ typedef struct { // EFI_I2C_DEVICE I2cDevices[MAX_I2C_DEVICES]; UINT32 SlaveAddressArray[MAX_I2C_DEVICES*MAX_SLAVES_PER_DEVICE]; + UINT32 SlaveDeviceTreeNodeOffsets[MAX_I2C_DEVICES]; UINTN NumberOfI2cDevices; UINT32 PinControlId; @@ -93,6 +96,7 @@ typedef struct { #define TEGRA_I2C_PRIVATE_DATA_FROM_MASTER(a) CR(a, NVIDIA_TEGRA_I2C_PRIVATE_DATA, I2cMaster, TEGRA_I2C_SIGNATURE) #define TEGRA_I2C_PRIVATE_DATA_FROM_ENUMERATE(a) CR(a, NVIDIA_TEGRA_I2C_PRIVATE_DATA, I2cEnumerate, TEGRA_I2C_SIGNATURE) #define TEGRA_I2C_PRIVATE_DATA_FROM_BUS_CONFIGURATION(a) CR(a, NVIDIA_TEGRA_I2C_PRIVATE_DATA, I2CConfiguration, TEGRA_I2C_SIGNATURE) +#define TEGRA_I2C_PRIVATE_DATA_FROM_SLAVE_DEVICE_TREE_NODE(a) CR(a, NVIDIA_TEGRA_I2C_PRIVATE_DATA, I2cSlaveDeviceTreeNode, TEGRA_I2C_SIGNATURE) /** * @addtogroup SPEED_MODES I2C Mode frequencies diff --git a/Silicon/NVIDIA/Drivers/TegraI2c/TegraI2cDxe.c b/Silicon/NVIDIA/Drivers/TegraI2c/TegraI2cDxe.c index 20fbf5d389..cf67f63449 100644 --- a/Silicon/NVIDIA/Drivers/TegraI2c/TegraI2cDxe.c +++ b/Silicon/NVIDIA/Drivers/TegraI2c/TegraI2cDxe.c @@ -11,19 +11,20 @@ #include #include +#include #include #include -#include -#include #include -#include -#include -#include #include +#include #include -#include +#include +#include +#include #include #include +#include +#include #include #include "TegraI2c.h" @@ -962,16 +963,43 @@ TegraI2cEnableI2cBusConfiguration ( return EFI_SUCCESS; } +EFI_STATUS +TegraI2cLookupSlaveDeviceTreeNode( + IN CONST NVIDIA_TEGRA_I2C_SLAVE_DEVICE_TREE_NODE_PROTOCOL *This, + IN CONST EFI_GUID *DeviceGuid, + IN UINT32 DeviceIndex, + OUT NVIDIA_DEVICE_TREE_NODE_PROTOCOL *DeviceTreeNode + ) +{ + NVIDIA_TEGRA_I2C_PRIVATE_DATA *Private = NULL; + + if (DeviceTreeNode == NULL || DeviceGuid == NULL) { + DEBUG ((DEBUG_ERROR, "%a: NULL DeviceTreeNode or DeviceGuid received\r\n", __FUNCTION__)); + return EFI_INVALID_PARAMETER; + } + + Private = TEGRA_I2C_PRIVATE_DATA_FROM_SLAVE_DEVICE_TREE_NODE(This); + for (INT32 i = 0; i < Private->NumberOfI2cDevices; i++) { + if (CompareGuid (Private->I2cDevices[i].DeviceGuid, DeviceGuid ) && Private->I2cDevices[i].DeviceIndex == DeviceIndex) { + DeviceTreeNode->DeviceTreeBase = Private->DeviceTreeBase; + DeviceTreeNode->NodeOffset = Private->SlaveDeviceTreeNodeOffsets[i]; + return EFI_SUCCESS; + } + } + return EFI_NOT_FOUND; +} + /** This routine is called to add an I2C device to the controller. - @param[in] Private Driver's private data. - @param[in] I2cAddress Address of the device. - @param[in] DeviceGuid GUID to identify the device type. - @param[in] I2cBusConfiguration The bus configuration (mux + switch settings) needed to access the device + @param[in] Private Driver's private data. + @param[in] I2cAddress Address of the device. + @param[in] DeviceGuid GUID to identify the device type. + @param[in] I2cBusConfiguration The bus configuration (mux + switch settings) needed to access the device + @param[in] DeviceTreeNodeOffset The offset of this device within the FDT - @retval EFI_SUCCESS Device added. - @retval other Some error occurs when device is being added. + @retval EFI_SUCCESS Device added. + @retval other Some error occurs when device is being added. **/ EFI_STATUS @@ -979,7 +1007,8 @@ TegraI2cAddDevice ( IN NVIDIA_TEGRA_I2C_PRIVATE_DATA *Private, IN UINT32 I2cAddress, IN EFI_GUID *DeviceGuid, - IN UINT32 I2cBusConfiguration + IN UINT32 I2cBusConfiguration, + IN UINT32 DeviceTreeNodeOffset ) { if (Private->NumberOfI2cDevices >= MAX_I2C_DEVICES) { @@ -995,6 +1024,8 @@ TegraI2cAddDevice ( Private->I2cDevices[Private->NumberOfI2cDevices].I2cBusConfiguration = I2cBusConfiguration; Private->I2cDevices[Private->NumberOfI2cDevices].SlaveAddressCount = 1; Private->I2cDevices[Private->NumberOfI2cDevices].SlaveAddressArray = &Private->SlaveAddressArray[Private->NumberOfI2cDevices * MAX_SLAVES_PER_DEVICE]; + Private->SlaveDeviceTreeNodeOffsets[Private->NumberOfI2cDevices] = DeviceTreeNodeOffset; + Private->NumberOfI2cDevices++; return EFI_SUCCESS; @@ -1074,6 +1105,8 @@ TegraI2CDriverBindingStart ( Private->I2cEnumerate.Enumerate = TegraI2cEnumerate; Private->I2cEnumerate.GetBusFrequency = TegraI2cGetBusFrequency; Private->I2CConfiguration.EnableI2cBusConfiguration = TegraI2cEnableI2cBusConfiguration; + Private->I2cSlaveDeviceTreeNode.LookupNode = TegraI2cLookupSlaveDeviceTreeNode; + Private->ProtocolsInstalled = FALSE; Private->DeviceTreeBase = DeviceTreeNode->DeviceTreeBase; Private->DeviceTreeNodeOffset = DeviceTreeNode->NodeOffset; @@ -1238,7 +1271,8 @@ TegraI2CDriverBindingStart ( Private, I2cAddress, DeviceGuid, - 0 + 0, + I2cNodeOffset ); if (EFI_ERROR (Status)) { goto ErrorExit; @@ -1261,7 +1295,8 @@ TegraI2CDriverBindingStart ( Private, I2cAddress, DeviceGuid, - 0 + 0, + I2cNodeOffset ); if (EFI_ERROR (Status)) { goto ErrorExit; @@ -1285,7 +1320,8 @@ TegraI2CDriverBindingStart ( Private, I2cAddress, DeviceGuid, - 0 + 0, + I2cNodeOffset ); if (EFI_ERROR (Status)) { goto ErrorExit; @@ -1329,7 +1365,8 @@ TegraI2CDriverBindingStart ( Private, I2cAddress, DeviceGuid, - 0 + 0, + I2cNodeOffset ); if (EFI_ERROR(Status)) { goto ErrorExit; @@ -1374,7 +1411,8 @@ TegraI2CDriverBindingStart ( Private, I2cAddressEeprom, DeviceGuid, - BusConfiguration.Value + BusConfiguration.Value, + I2cMuxSubNodeOffset ); if (EFI_ERROR(Status)) { @@ -1405,7 +1443,8 @@ TegraI2CDriverBindingStart ( Private, I2cAddress, DeviceGuid, - 0 + 0, + I2cNodeOffset ); if (EFI_ERROR (Status)) { goto ErrorExit; @@ -1427,7 +1466,8 @@ TegraI2CDriverBindingStart ( Private, I2cAddress, DeviceGuid, - 0 + 0, + I2cNodeOffset ); if (EFI_ERROR (Status)) { goto ErrorExit; @@ -1449,7 +1489,8 @@ TegraI2CDriverBindingStart ( Private, I2cAddress, DeviceGuid, - 0 + 0, + I2cNodeOffset ); if (EFI_ERROR (Status)) { goto ErrorExit; @@ -1489,7 +1530,8 @@ TegraI2CDriverBindingStart ( Private, I2cAddress, DeviceGuid, - 0 + 0, + EepromNodeOffset ); if (EFI_ERROR (Status)) { goto ErrorExit; @@ -1509,6 +1551,8 @@ TegraI2CDriverBindingStart ( &Private->I2cEnumerate, &gEfiI2cBusConfigurationManagementProtocolGuid, &Private->I2CConfiguration, + &gNVIDIAI2cSlaveDeviceTreeNodeProtocolGuid, + &Private->I2cSlaveDeviceTreeNode, NULL ); if (EFI_ERROR (Status)) { @@ -1529,6 +1573,8 @@ TegraI2CDriverBindingStart ( &Private->I2cEnumerate, &gEfiI2cBusConfigurationManagementProtocolGuid, &Private->I2CConfiguration, + &gNVIDIAI2cSlaveDeviceTreeNodeProtocolGuid, + &Private->I2cSlaveDeviceTreeNode, NULL ); } @@ -1590,6 +1636,8 @@ TegraI2CDriverBindingStop ( &Private->I2cEnumerate, &gEfiI2cBusConfigurationManagementProtocolGuid, &Private->I2CConfiguration, + &gNVIDIAI2cSlaveDeviceTreeNodeProtocolGuid, + &Private->I2cSlaveDeviceTreeNode, NULL ); if (EFI_ERROR (Status)) { diff --git a/Silicon/NVIDIA/Drivers/TegraI2c/TegraI2cDxe.inf b/Silicon/NVIDIA/Drivers/TegraI2c/TegraI2cDxe.inf index aaf0d4f968..457d62d8e3 100644 --- a/Silicon/NVIDIA/Drivers/TegraI2c/TegraI2cDxe.inf +++ b/Silicon/NVIDIA/Drivers/TegraI2c/TegraI2cDxe.inf @@ -46,6 +46,7 @@ gNVIDIADeviceTreeNodeProtocolGuid gNVIDIAPinControlProtocolGuid gNVIDIATegraI2cInitCompleteProtocolGuid + gNVIDIAI2cSlaveDeviceTreeNodeProtocolGuid [Guids] gNVIDIAI2cUnknown diff --git a/Silicon/NVIDIA/Include/Protocol/TegraI2cSlaveDeviceTreeNode.h b/Silicon/NVIDIA/Include/Protocol/TegraI2cSlaveDeviceTreeNode.h new file mode 100644 index 0000000000..d983b76132 --- /dev/null +++ b/Silicon/NVIDIA/Include/Protocol/TegraI2cSlaveDeviceTreeNode.h @@ -0,0 +1,21 @@ +#ifndef __NVIDIA_TEGRA_I2C_SLAVE_DEVICE_TREE_NODE_PROTOCOL_H__ +#define __NVIDIA_TEGRA_I2C_SLAVE_DEVICE_TREE_NODE_PROTOCOL_H__ + +#include + +typedef struct _NVIDIA_TEGRA_I2C_SLAVE_DEVICE_TREE_NODE_PROTOCOL NVIDIA_TEGRA_I2C_SLAVE_DEVICE_TREE_NODE_PROTOCOL; + +typedef EFI_STATUS + (*NVIDIA_TEGRA_I2C_SLAVE_DEVICE_TREE_NODE_PROTOCOL_LOOKUP_NODE)( + IN CONST NVIDIA_TEGRA_I2C_SLAVE_DEVICE_TREE_NODE_PROTOCOL *This, + IN CONST EFI_GUID *DeviceGuid, + IN UINT32 DeviceIndex, + OUT NVIDIA_DEVICE_TREE_NODE_PROTOCOL *DeviceTreeNode + ); + +struct _NVIDIA_TEGRA_I2C_SLAVE_DEVICE_TREE_NODE_PROTOCOL { + NVIDIA_TEGRA_I2C_SLAVE_DEVICE_TREE_NODE_PROTOCOL_LOOKUP_NODE LookupNode; +}; + +extern EFI_GUID gNVIDIAI2cSlaveDeviceTreeNodeProtocolGuid; +#endif//__NVIDIA_TEGRA_I2C_SLAVE_DEVICE_TREE_NODE_PROTOCOL_H__ diff --git a/Silicon/NVIDIA/NVIDIA.dec b/Silicon/NVIDIA/NVIDIA.dec index b9087ebb44..96d78d0c11 100644 --- a/Silicon/NVIDIA/NVIDIA.dec +++ b/Silicon/NVIDIA/NVIDIA.dec @@ -231,6 +231,7 @@ gNVIDIASubPinControlProtocolGuid = { 0x13fcda23, 0x1119, 0x4797, { 0xad, 0xed, 0x5a, 0xb4, 0xfe, 0x4c, 0x33, 0x1e } } gNVIDIABootChainProtocolGuid = { 0xbbed2514, 0x140b, 0x4176, { 0xa8, 0xf6, 0x51, 0x35, 0x8e, 0xbb, 0x21, 0xdf } } gNVIDIATegraI2cInitCompleteProtocolGuid = { 0xe7ebaff7, 0x3000, 0x4c79, { 0xa9, 0x2b, 0x8a, 0x4f, 0x63, 0x7a, 0xe3, 0x43 } } + gNVIDIAI2cSlaveDeviceTreeNodeProtocolGuid = { 0x7eff085a, 0x20a2, 0x480d, { 0xa7, 0x17, 0x7e, 0xd9, 0xdf, 0x15, 0x57, 0x76 } } gNVIDIAIpmiBlobTransferProtocolGuid = { 0x05837c75, 0x1d65, 0x468b, { 0xb1, 0xc2, 0x81, 0xaf, 0x9a, 0x31, 0x5b, 0x2c } } gNVIDIATegraCpuFrequencyProtocolGuid = { 0xa20bb97e, 0x4de7, 0x426e, { 0xac, 0xd6, 0x3a, 0x5e, 0xaa, 0x6a, 0xd6, 0xc5 } } gEfiApeiGetErrorSourcesGuid = { 0xb5aabe64, 0xf09a, 0x4b94, { 0x8e, 0xfa, 0x2e, 0x23, 0x4d, 0x00, 0x6d, 0x3d } } From 67a00fc6452b7b571214d4d8b8f72fd2cd501558 Mon Sep 17 00:00:00 2001 From: eh-steve <16373174+eh-steve@users.noreply.github.com> Date: Fri, 19 May 2023 19:27:25 +0100 Subject: [PATCH 4/5] feat: EEPROM CRC check can be skipped via "uefi-skip-crc" device-tree property NV UEFI currently skips the CRC check for Leopard camera eeproms whose product ID data starts with "LPRD" via a hardcoded exception. Other camera vendors may or may not set the last byte to the CRC8 value, so this exposes a device tree property "uefi-skip-crc" to be added to the eeprom device tree node, so hardcoded exceptions can be gradually removed and moved to device tree instead. This enables Framos camera support Signed-off-by: eh-steve <16373174+eh-steve@users.noreply.github.com> --- Silicon/NVIDIA/Drivers/EepromDxe/Eeprom.c | 40 +++++++++++++++++++-- Silicon/NVIDIA/Drivers/EepromDxe/Eeprom.inf | 2 ++ 2 files changed, 40 insertions(+), 2 deletions(-) diff --git a/Silicon/NVIDIA/Drivers/EepromDxe/Eeprom.c b/Silicon/NVIDIA/Drivers/EepromDxe/Eeprom.c index 720715d7eb..49843e5b9c 100644 --- a/Silicon/NVIDIA/Drivers/EepromDxe/Eeprom.c +++ b/Silicon/NVIDIA/Drivers/EepromDxe/Eeprom.c @@ -21,12 +21,14 @@ #include #include #include +#include #include #include #include #include #include +#include #define SERIAL_NUM_CMD_MAX_LEN 64 #define EEPROM_DATA_SIZE 256 @@ -316,6 +318,10 @@ EepromDxeDriverBindingStart ( TEGRA_EEPROM_BOARD_INFO *IdBoardInfo; BOOLEAN SkipEepromCRC; + NVIDIA_TEGRA_I2C_SLAVE_DEVICE_TREE_NODE_PROTOCOL *I2cSlaveDeviceTreeNode = NULL; + NVIDIA_DEVICE_TREE_NODE_PROTOCOL EepromDeviceTreeNode; + + RawData = NULL; CvmBoardInfo = NULL; IdBoardInfo = NULL; @@ -338,6 +344,36 @@ EepromDxeDriverBindingStart ( goto ErrorExit; } + EFI_HANDLE Handle[10]; // Usually only 8 Tegra I2C buses to choose from + UINTN HandleSize = sizeof(Handle); + Status = gBS->LocateHandle(ByProtocol, &gNVIDIAI2cSlaveDeviceTreeNodeProtocolGuid, NULL, &HandleSize, &Handle[0]); + + if ((EFI_ERROR (Status))) { + DEBUG ((DEBUG_ERROR, "%a: Unable to LocateHandle for I2cSlaveDeviceTreeNode Protocol (Status: %d)\n", __FUNCTION__, Status)); + return Status; + } + + + INT32 NumHandles = HandleSize/sizeof(Handle[0]); + for (INT32 i = 0; i < NumHandles; i++) { + Status = gBS->HandleProtocol (Handle[i], &gNVIDIAI2cSlaveDeviceTreeNodeProtocolGuid, (VOID **)&I2cSlaveDeviceTreeNode); + if ((EFI_ERROR (Status))) { + DEBUG ((DEBUG_ERROR, "%a: Unable to HandleProtocol for index %d I2cSlaveDeviceTreeNode Protocol (Status: %d)\n", __FUNCTION__, i, Status)); + return Status; + } + + Status = I2cSlaveDeviceTreeNode->LookupNode(I2cSlaveDeviceTreeNode, I2cIo->DeviceGuid, I2cIo->DeviceIndex, &EepromDeviceTreeNode); + if (!(EFI_ERROR (Status))) { + // found + break; + } + } + + if ((EFI_ERROR (Status))) { + DEBUG ((DEBUG_ERROR, "%a: Unable to LookupNode using any of the %d I2cSlaveDeviceTreeNode Protocols (device index %x, Status: %d)\n", __FUNCTION__, NumHandles, I2cIo->DeviceIndex, Status)); + return Status; + } + // Allocate EEPROM Data RawData = (UINT8 *)AllocateZeroPool (EEPROM_DATA_SIZE); if (RawData == NULL) { @@ -366,8 +402,8 @@ EepromDxeDriverBindingStart ( FreePool (Request); Request = NULL; - - if (0 == AsciiStrnCmp ( + if (fdt_get_property(EepromDeviceTreeNode.DeviceTreeBase, EepromDeviceTreeNode.NodeOffset, "uefi-skip-crc", NULL) != NULL || + 0 == AsciiStrnCmp ( (CHAR8 *)&RawData[CAMERA_EEPROM_PART_OFFSET], CAMERA_EEPROM_PART_NAME, AsciiStrLen (CAMERA_EEPROM_PART_NAME) diff --git a/Silicon/NVIDIA/Drivers/EepromDxe/Eeprom.inf b/Silicon/NVIDIA/Drivers/EepromDxe/Eeprom.inf index 7ed3db0b05..9a9ad6c440 100644 --- a/Silicon/NVIDIA/Drivers/EepromDxe/Eeprom.inf +++ b/Silicon/NVIDIA/Drivers/EepromDxe/Eeprom.inf @@ -38,6 +38,7 @@ PlatformResourceLib Crc8Lib PrintLib + FdtLib [Guids] gNVIDIAEeprom @@ -50,6 +51,7 @@ gNVIDIACvmEepromProtocolGuid ## SOMETIMES_PRODUCES gNVIDIACvbEepromProtocolGuid ## SOMETIMES_PRODUCES gNVIDIAEepromProtocolGuid ## SOMETIMES_PRODUCES + gNVIDIAI2cSlaveDeviceTreeNodeProtocolGuid ## CONSUMES [Depex] TRUE From badb4fb5fa01ffbab2badb1c2989b4be79cc0eae Mon Sep 17 00:00:00 2001 From: eh-steve <16373174+eh-steve@users.noreply.github.com> Date: Fri, 19 May 2023 19:36:01 +0100 Subject: [PATCH 5/5] feat: selective DT overlays applied to matching "ids" AND "eeprom-dt-paths" Adds support for specific overlay triggers based on a combination of EEPROM IDs and the exact device tree paths of those EEPROMS. The current behaviour of NV UEFI is that if any EEPROM product ID from any I2C bus matches any of the ids listed in board_config, the overlay is triggered. Effectively all EEPROMs add to a global list of Board IDs, regardless of which device they actually represent. This means a single camera plugged into a single port would make the whole board be treated as matching that camera. This is a problem, since: * it prevents plugging different camera models into different ports and have them work without re-flashing the whole device (any CSI camera port change requires a reflash, instead of supporting various models automatically, and having the interposer board partly populated) * Causes the kernel to create devnodes for devices that don't exist, (causing preventable error logs in nvargus-daemon) This PR adds support for a new, more controlled overlay behaviour via the new (optional) eeprom-dt-paths board_config property, where an overlay can be triggered based on a match of the ID in the EEPROM and a match of the exact I2C device tree path the EEPROM product ID came from. (Old behaviour with existing board_configs should be unchanged). This means that the presence of a single camera no longer adds this Product ID to the global list of board IDs, but that you can selectively apply an overlay only when a specific ID was read from a specific device tree path (i.e. individual CSI cameras can be enumerated during boot and the kernel never creates devnodes for missing devices). ``` board_config { ids = "framos-imx462-0"; eeprom-dt-paths = "/i2c@3180000/pca9547@70/i2c@1/eeprom@55", "/i2c@3180000/pca9547@70/i2c@1/eeprom@56"; sw-modules = "kernel"; }; ``` Signed-off-by: eh-steve <16373174+eh-steve@users.noreply.github.com> --- Silicon/NVIDIA/Drivers/EepromDxe/Eeprom.c | 9 ++ Silicon/NVIDIA/Include/Protocol/Eeprom.h | 2 + .../TegraDeviceTreeKernelOverlayLib.c | 4 +- .../TegraDeviceTreeOverlayLib.c | 1 + .../TegraDeviceTreeOverlayLibCommon.c | 129 +++++++++++++----- .../TegraDeviceTreeOverlayLibCommon.h | 1 + 6 files changed, 113 insertions(+), 33 deletions(-) diff --git a/Silicon/NVIDIA/Drivers/EepromDxe/Eeprom.c b/Silicon/NVIDIA/Drivers/EepromDxe/Eeprom.c index 49843e5b9c..91342c7a3a 100644 --- a/Silicon/NVIDIA/Drivers/EepromDxe/Eeprom.c +++ b/Silicon/NVIDIA/Drivers/EepromDxe/Eeprom.c @@ -317,6 +317,7 @@ EepromDxeDriverBindingStart ( TEGRA_EEPROM_BOARD_INFO *CvmBoardInfo; TEGRA_EEPROM_BOARD_INFO *IdBoardInfo; BOOLEAN SkipEepromCRC; + INT32 DeviceTreePathLen; NVIDIA_TEGRA_I2C_SLAVE_DEVICE_TREE_NODE_PROTOCOL *I2cSlaveDeviceTreeNode = NULL; NVIDIA_DEVICE_TREE_NODE_PROTOCOL EepromDeviceTreeNode; @@ -424,6 +425,14 @@ EepromDxeDriverBindingStart ( goto ErrorExit; } + DeviceTreePathLen = fdt_get_path(EepromDeviceTreeNode.DeviceTreeBase, EepromDeviceTreeNode.NodeOffset, IdBoardInfo->EepromDeviceTreePath, MAX_I2C_DEVICE_DT_PATH); + + if (DeviceTreePathLen != 0) { + DEBUG((DEBUG_ERROR, "%a: Failed to get device tree path length for I2c sub-device 0x%lx on I2c Bus 0x%lx (error: %d).\n", __FUNCTION__, I2cIo->DeviceIndex >> 16, I2cIo->DeviceIndex & 0xFFFF, DeviceTreePathLen)); + } else { + DEBUG ((DEBUG_INFO, "%a: Starting (TEGRA_PLATFORM_SILICON) Bus %x Device %x %a\r\n", __FUNCTION__, I2cIo->DeviceIndex >> 16, I2cIo->DeviceIndex & 0xFFFF, IdBoardInfo->EepromDeviceTreePath)); + } + Status = PopulateEepromData (RawData, IdBoardInfo); if (EFI_ERROR (Status)) { DEBUG ((DEBUG_ERROR, "Eeprom data population failed(%r)\r\n", Status)); diff --git a/Silicon/NVIDIA/Include/Protocol/Eeprom.h b/Silicon/NVIDIA/Include/Protocol/Eeprom.h index cc58fc2e93..4975916c3f 100644 --- a/Silicon/NVIDIA/Include/Protocol/Eeprom.h +++ b/Silicon/NVIDIA/Include/Protocol/Eeprom.h @@ -24,6 +24,7 @@ #define NVIDIA_EEPROM_BOARD_ID_PREFIX "699" #define CUSTOMER_EEPROM_BOARD_ID_MAGIC 0xcc +#define MAX_I2C_DEVICE_DT_PATH 64 /** * @brief The Product Part Number structure that is embedded into * EEPROM layout structure @@ -224,6 +225,7 @@ typedef struct { CHAR8 SerialNumber[TEGRA_SERIAL_NUM_LEN]; UINT8 MacAddr[NET_ETHER_ADDR_LEN]; UINT8 NumMacs; + CHAR8 EepromDeviceTreePath[MAX_I2C_DEVICE_DT_PATH]; } TEGRA_EEPROM_BOARD_INFO; static inline diff --git a/Silicon/NVIDIA/Library/TegraDeviceTreeOverlayLib/TegraDeviceTreeKernelOverlayLib.c b/Silicon/NVIDIA/Library/TegraDeviceTreeOverlayLib/TegraDeviceTreeKernelOverlayLib.c index 7dec51a05b..e9b688635e 100644 --- a/Silicon/NVIDIA/Library/TegraDeviceTreeOverlayLib/TegraDeviceTreeKernelOverlayLib.c +++ b/Silicon/NVIDIA/Library/TegraDeviceTreeOverlayLib/TegraDeviceTreeKernelOverlayLib.c @@ -68,6 +68,7 @@ ReadBoardInfo ( BoardInfo->IdCount = ProtocolCount; BoardInfo->ProductIds = (EEPROM_PART_NUMBER *)AllocateZeroPool (BoardInfo->IdCount * sizeof (EEPROM_PART_NUMBER)); + BoardInfo->EepromDeviceTreePaths = (CHAR8 **)AllocateZeroPool (BoardInfo->IdCount * sizeof (CHAR8 *)); for (i = 0; i < ProtocolCount; i++) { Status = gBS->HandleProtocol ( @@ -81,6 +82,7 @@ ReadBoardInfo ( } CopyMem ((VOID *)(&BoardInfo->ProductIds[i]), (VOID *)&Eeprom->ProductId, TEGRA_PRODUCT_ID_LEN); + BoardInfo->EepromDeviceTreePaths[i] = Eeprom->EepromDeviceTreePath; } if (BoardInfo->IdCount == 0) { @@ -90,7 +92,7 @@ ReadBoardInfo ( DEBUG ((DEBUG_INFO, "Eeprom product Ids: \n")); for (i = 0; i < BoardInfo->IdCount; i++) { - DEBUG ((DEBUG_INFO, "%d. %a \n", i+1, BoardInfo->ProductIds[i])); + DEBUG ((DEBUG_INFO, "%d. %a (%a)\n", i+1, BoardInfo->ProductIds[i], BoardInfo->EepromDeviceTreePaths[i])); } return EFI_SUCCESS; diff --git a/Silicon/NVIDIA/Library/TegraDeviceTreeOverlayLib/TegraDeviceTreeOverlayLib.c b/Silicon/NVIDIA/Library/TegraDeviceTreeOverlayLib/TegraDeviceTreeOverlayLib.c index 5458d7f679..eb5ad17d15 100644 --- a/Silicon/NVIDIA/Library/TegraDeviceTreeOverlayLib/TegraDeviceTreeOverlayLib.c +++ b/Silicon/NVIDIA/Library/TegraDeviceTreeOverlayLib/TegraDeviceTreeOverlayLib.c @@ -45,6 +45,7 @@ ReadBoardInfo ( BoardInfo->FuseCount = TegraBoardInfo->FuseCount; BoardInfo->IdCount = 2; /*CVM and CVB*/ BoardInfo->ProductIds = (EEPROM_PART_NUMBER *)AllocateZeroPool (BoardInfo->IdCount * sizeof (EEPROM_PART_NUMBER)); + BoardInfo->EepromDeviceTreePaths = (CHAR8 **)AllocateZeroPool (BoardInfo->IdCount * sizeof (CHAR8*)); CopyMem ((VOID *)&BoardInfo->ProductIds[0], (VOID *)TegraBoardInfo->CvmProductId, TEGRA_PRODUCT_ID_LEN); CopyMem ((VOID *)&BoardInfo->ProductIds[1], (VOID *)TegraBoardInfo->CvbProductId, TEGRA_PRODUCT_ID_LEN); diff --git a/Silicon/NVIDIA/Library/TegraDeviceTreeOverlayLib/TegraDeviceTreeOverlayLibCommon.c b/Silicon/NVIDIA/Library/TegraDeviceTreeOverlayLib/TegraDeviceTreeOverlayLibCommon.c index 568b9f6438..4a2773a505 100644 --- a/Silicon/NVIDIA/Library/TegraDeviceTreeOverlayLib/TegraDeviceTreeOverlayLibCommon.c +++ b/Silicon/NVIDIA/Library/TegraDeviceTreeOverlayLib/TegraDeviceTreeOverlayLibCommon.c @@ -32,12 +32,13 @@ typedef enum { } MATCH_OPERATOR; typedef struct { - CHAR8 Name[16]; - UINT32 Count; + CHAR8 Name[2][16]; + UINT32 Count[2]; MATCH_OPERATOR MatchOp; BOOLEAN (*IsMatch)( VOID *Fdt, CONST CHAR8 *Item, + CONST CHAR8 *DTPath, VOID *Param ); } DT_MATCH_INFO; @@ -55,6 +56,7 @@ STATIC BOOLEAN MatchId ( VOID *, CONST CHAR8 *, + CONST CHAR8 *, VOID * ); @@ -62,6 +64,7 @@ STATIC BOOLEAN MatchOdmData ( VOID *, CONST CHAR8 *, + CONST CHAR8 *, VOID * ); @@ -69,6 +72,7 @@ STATIC BOOLEAN MatchSWModule ( VOID *, CONST CHAR8 *, + CONST CHAR8 *, VOID * ); @@ -76,27 +80,28 @@ STATIC BOOLEAN MatchFuseInfo ( VOID *, CONST CHAR8 *, + CONST CHAR8 *, VOID * ); DT_MATCH_INFO MatchInfoArray[] = { { - .Name = "ids", + .Name = {"ids", "eeprom-dt-paths"}, .MatchOp = MATCH_OR, .IsMatch = MatchId, }, { - .Name = "odm-data", + .Name = {"odm-data"}, .MatchOp = MATCH_AND, .IsMatch = MatchOdmData, }, { - .Name = "sw-modules", + .Name = {"sw-modules"}, .MatchOp = MATCH_OR, .IsMatch = MatchSWModule, }, { - .Name = "fuse-info", + .Name = {"fuse-info"}, .MatchOp = MATCH_AND, .IsMatch = MatchFuseInfo, }, @@ -152,6 +157,7 @@ STATIC BOOLEAN MatchId ( VOID *Fdt, CONST CHAR8 *Id, + CONST CHAR8 *TargetDTPath, VOID *Param ) { @@ -162,6 +168,7 @@ MatchId ( INTN BoardIdLen; UINTN FabIdPrefixLen; CONST CHAR8 *BoardId = NULL; + CHAR8 *EeepromDeviceTreePath = NULL; BOOLEAN Matched = FALSE; @@ -222,15 +229,19 @@ MatchId ( } for (i = 0; i < BoardInfo->IdCount; i++) { + Matched = FALSE; BoardId = TegraBoardIdFromPartNumber (&BoardInfo->ProductIds[i]); + EeepromDeviceTreePath = BoardInfo->EepromDeviceTreePaths[i]; BoardIdLen = strlen (BoardId); BoardFabId = GetFabId (BoardId, NULL); DEBUG (( DEBUG_INFO, - "%a: check if overlay node id %a match with %a\n", + "%a: check if overlay node id %a match with %a (DT path %a, target DT path %a)\n", __FUNCTION__, Id, - BoardId + BoardId, + EeepromDeviceTreePath, + TargetDTPath )); switch (MatchType) { @@ -290,7 +301,20 @@ MatchId ( } if (Matched == TRUE) { - break; + if (TargetDTPath != NULL) { + if (EeepromDeviceTreePath == NULL) { + Matched = FALSE; + } else { + EeepromDeviceTreePath = BoardInfo->EepromDeviceTreePaths[i]; + INT32 TargetDTPathLen = strlen(TargetDTPath); + INT32 EepromPathLen = strlen(EeepromDeviceTreePath); + if (!(EepromPathLen == TargetDTPathLen && !CompareMem (TargetDTPath, EeepromDeviceTreePath, EepromPathLen))) { + Matched = FALSE; + } else { + break; + } + } + } } } @@ -303,6 +327,7 @@ STATIC BOOLEAN MatchOdmData ( VOID *Fdt, CONST CHAR8 *OdmData, + CONST CHAR8 *DTPath, VOID *Param ) { @@ -328,6 +353,7 @@ STATIC BOOLEAN MatchSWModule ( VOID *Fdt, CONST CHAR8 *ModuleStr, + CONST CHAR8 *DTPath, VOID *Param ) { @@ -342,6 +368,7 @@ STATIC BOOLEAN MatchFuseInfo ( VOID *Fdt, CONST CHAR8 *FuseStr, + CONST CHAR8 *DTPath, VOID *Param ) { @@ -375,18 +402,28 @@ PMGetPropertyCount ( UINTN AllCount = 0; UINTN Index; INTN PropCount; + INTN PropCount2; for (Index = 0; Index < ARRAY_SIZE (MatchInfoArray); MatchIter++, Index++) { - PropCount = fdt_stringlist_count (Fdt, Node, MatchIter->Name); + PropCount = fdt_stringlist_count (Fdt, Node, MatchIter->Name[0]); if (PropCount < 0) { DEBUG ((DEBUG_INFO, "%a: Node: %d, Property: %a: Not Found.\n", __FUNCTION__, Node, MatchIter->Name)); - MatchIter->Count = 0; + MatchIter->Count[0] = 0; } else { - MatchIter->Count = PropCount; + MatchIter->Count[0] = PropCount; DEBUG ((DEBUG_INFO, "%a: Node: %d, Property: %a: Count: %d.\n", __FUNCTION__, Node, MatchIter->Name, PropCount)); } - - AllCount += MatchIter->Count; + if (MatchIter->Name[1][0] != 0) { + PropCount2 = fdt_stringlist_count (Fdt, Node, MatchIter->Name[1]); + if (PropCount2 < 0) { + DEBUG ((DEBUG_INFO, "%a: Node: %d, Property: %a: Not Found.\n", __FUNCTION__, Node, MatchIter->Name[1])); + MatchIter->Count[1] = 0; + } else { + MatchIter->Count[1] = PropCount2; + DEBUG ((DEBUG_INFO, "%a: Node: %d, Property: %a: Count: %d.\n", __FUNCTION__, Node, MatchIter->Name[1], PropCount2)); + } + } + AllCount += MatchIter->Count[0] + MatchIter->Count[1]; } if (!AllCount) { @@ -593,6 +630,7 @@ ProcessOverlayDeviceTree ( CONST CHAR8 *NodeName; INTN ConfigNode; CONST CHAR8 *PropStr; + CONST CHAR8 *PropStr2; INTN PropCount; INT32 FdtErr; EFI_STATUS Status = EFI_SUCCESS; @@ -600,6 +638,7 @@ ProcessOverlayDeviceTree ( DT_MATCH_INFO *MatchIter; UINT32 Index; UINT32 Count; + UINT32 Count2; UINT32 NumberSubnodes; UINT32 FixupNodes = 0; @@ -637,27 +676,53 @@ ProcessOverlayDeviceTree ( MatchIter = MatchInfoArray; for (Index = 0; Index < ARRAY_SIZE (MatchInfoArray); Index++, MatchIter++) { - if ((MatchIter->Count > 0) && MatchIter->IsMatch) { + if ((MatchIter->Count[0] > 0) && MatchIter->IsMatch) { UINT32 Data = 0; - for (Count = 0, Found = FALSE; Count < MatchIter->Count; Count++) { - PropStr = fdt_stringlist_get (FdtOverlay, ConfigNode, MatchIter->Name, Count, NULL); - DEBUG (( - DEBUG_INFO, - "Check if property %a[%a] on /%a match\n", - MatchIter->Name, - PropStr, - FrName - )); - - Found = MatchIter->IsMatch (FdtBase, PropStr, &Data); - if (!Found && (MatchIter->MatchOp == MATCH_AND)) { - break; - } + for (Count = 0, Found = FALSE; Count < MatchIter->Count[0]; Count++) { + PropStr = fdt_stringlist_get (FdtOverlay, ConfigNode, MatchIter->Name[0], Count, NULL); + + if (MatchIter->Count[1] > 0) { + // This case should only be for "ids" and "eeprom-dt-paths" + for (Count2 = 0; Count2 < MatchIter->Count[1]; Count2++) { + PropStr2 = fdt_stringlist_get (FdtOverlay, ConfigNode, MatchIter->Name[1], Count2, NULL); + + DEBUG (( + DEBUG_INFO, + "Check if property %a[%a] with %a on /%a match \n", + MatchIter->Name, + PropStr, + PropStr2, + FrName + )); + + Found = MatchIter->IsMatch (FdtBase, PropStr, PropStr2, &Data); + if (Found && (MatchIter->MatchOp == MATCH_OR)) { + break; + } + + } + if (Found && (MatchIter->MatchOp == MATCH_OR)) { + break; + } + } else { + DEBUG (( + DEBUG_INFO, + "Check if property %a[%a] on /%a match\n", + MatchIter->Name, + PropStr, + FrName + )); + + Found = MatchIter->IsMatch (FdtBase, PropStr, NULL, &Data); + if (!Found && (MatchIter->MatchOp == MATCH_AND)) { + break; + } - if (Found && (MatchIter->MatchOp == MATCH_OR)) { - break; - } + if (Found && (MatchIter->MatchOp == MATCH_OR)) { + break; + } + } } if (!Found) { diff --git a/Silicon/NVIDIA/Library/TegraDeviceTreeOverlayLib/TegraDeviceTreeOverlayLibCommon.h b/Silicon/NVIDIA/Library/TegraDeviceTreeOverlayLib/TegraDeviceTreeOverlayLibCommon.h index 2141932ccc..daeb2819af 100644 --- a/Silicon/NVIDIA/Library/TegraDeviceTreeOverlayLib/TegraDeviceTreeOverlayLibCommon.h +++ b/Silicon/NVIDIA/Library/TegraDeviceTreeOverlayLib/TegraDeviceTreeOverlayLibCommon.h @@ -18,6 +18,7 @@ typedef struct { UINTN FuseCount; EEPROM_PART_NUMBER *ProductIds; UINTN IdCount; + CHAR8 **EepromDeviceTreePaths; } OVERLAY_BOARD_INFO; EFI_STATUS