On 2019.11.27 15:24, Philippe Mathieu-Daudé wrote:
On 11/27/19 1:37 PM, Pete Batard wrote:From: Samer El-Haj-Mahmoud <samer.el-haj-mahm...@arm.com>The Raspberry Pi 4 has a new SD controller. As a result we must handle SD routing according to the model, which we perform in the Config driver by using the GetModelFamily () call that was recently introduced. Signed-off-by: Pete Batard <p...@akeo.ie> ---Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c | 137 ++++++++++++++------1 file changed, 96 insertions(+), 41 deletions(-)diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.cindex 98e58a560ed4..26bc92f28185 100644 --- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c +++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c @@ -1,6 +1,7 @@ /** @file * - * Copyright (c) 2018, Andrei Warkentin <andrey.warken...@gmail.com> + * Copyright (c) 2019, ARM Limited. All rights reserved."All rights reserved."?
To be honest, that's something that's been bothering me too in this codebase (and some other ones too, where you get to see the same), since there are only so many rights one can reserve when the code is actually governed by the Open Source license being used, and therefore asserting that you reserve "all rights" seems to be in direct conflict with that.
However, I am not a lawyer, and this seems to be standard boilerplate being imposed by large companies. For instance, you'll find plenty of instances of it in the existing codebase. E.g. https://github.com/tianocore/edk2/blob/master/ArmPkg/Include/AsmMacroIoLib.h has three separate entities that appear to state that each one holds all the rights to the source, which I can't help by find amusing.
I guess we're supposed to understand that each entity reserves all rights to the code they've actually written (including the right to do something that might go against the license, since "All rights" > "Rights to the extent being granted by the BSD"), and that it's up to legal departments to sort up the mess, if mess there is...
Then again, while I think I can wrap my head against what copyright entails, I'm not sure I completely get what these additional "rights" are supposed to mean in this context (my current take being that we're supposed to be believe that there exists an implicit grandfathered license, which gives all rights to the parent company, and that governs a virtual version of the source code containing only the changes that the developer applied, and therefore that the BSD licensed version of the source that is then made public is meant to be seen as a derivative of this virtual "All rights reserved" incomplete source, hence granting a partial "All rights" for said source to the company, if that makes any sense), so it may be good for someone with better understanding of this to clarify, or point to a place where this might be explained.
+ * Copyright (c) 2018 - 2019, Andrei Warkentin <andrey.warken...@gmail.com>* * SPDX-License-Identifier: BSD-2-Clause-Patent * @@ -9,10 +10,12 @@ #include <Uefi.h> #include <Library/HiiLib.h> #include <Library/DebugLib.h> +#include <Library/IoLib.h> #include <Library/UefiBootServicesTableLib.h> #include <Library/UefiRuntimeServicesTableLib.h> #include <Library/DevicePathLib.h> #include <IndustryStandard/RpiMbox.h> +#include <IndustryStandard/Bcm2836.h> #include <IndustryStandard/Bcm2836Gpio.h> #include <Library/GpioLib.h> #include <Protocol/RpiFirmware.h> @@ -212,6 +215,7 @@ ApplyVariables ( UINT32 CpuClock = PcdGet32 (PcdCpuClock); UINT32 CustomCpuClock = PcdGet32 (PcdCustomCpuClock); UINT32 Rate = 0; + UINT32 ModelFamily = 0; if (CpuClock != 0) { if (CpuClock == 2) { @@ -245,51 +249,102 @@ ApplyVariables ( DEBUG ((DEBUG_INFO, "Current CPU speed is %uHz\n", Rate)); } - /* - * Switching two groups around, so disable both first. - * - * No, I've not seen a problem, but having a group be - * routed to two sets of pins seems like asking for trouble. - */ - GpioPinFuncSet (34, GPIO_FSEL_INPUT); - GpioPinFuncSet (35, GPIO_FSEL_INPUT); - GpioPinFuncSet (36, GPIO_FSEL_INPUT); - GpioPinFuncSet (37, GPIO_FSEL_INPUT); - GpioPinFuncSet (38, GPIO_FSEL_INPUT); - GpioPinFuncSet (39, GPIO_FSEL_INPUT); - GpioPinFuncSet (48, GPIO_FSEL_INPUT); - GpioPinFuncSet (49, GPIO_FSEL_INPUT); - GpioPinFuncSet (50, GPIO_FSEL_INPUT); - GpioPinFuncSet (51, GPIO_FSEL_INPUT); - GpioPinFuncSet (52, GPIO_FSEL_INPUT); - GpioPinFuncSet (53, GPIO_FSEL_INPUT); - if (PcdGet32 (PcdSdIsArasan)) { - DEBUG ((DEBUG_INFO, "Routing SD to Arasan\n")); - Gpio48Group = GPIO_FSEL_ALT3; + Status = mFwProtocol->GetModelFamily (&ModelFamily); + if (Status != EFI_SUCCESS) {+ DEBUG ((DEBUG_ERROR, "Couldn't get the Raspberry Pi model family: %r\n", Status));+ } else {+ DEBUG ((DEBUG_INFO, "Current Raspberry Pi model family is 0x%x\n", ModelFamily));+ } + + + if (ModelFamily == 3) { /* - * Route SDIO to SdHost. + * Pi 3: either Arasan or SdHost goes to SD card. + * + * Switching two groups around, so disable both first. + * + * No, I've not seen a problem, but having a group be + * routed to two sets of pins seems like asking for trouble. */ - Gpio34Group = GPIO_FSEL_ALT0; - } else { - DEBUG ((DEBUG_INFO, "Routing SD to SdHost\n")); - Gpio48Group = GPIO_FSEL_ALT0; + GpioPinFuncSet (34, GPIO_FSEL_INPUT); + GpioPinFuncSet (35, GPIO_FSEL_INPUT); + GpioPinFuncSet (36, GPIO_FSEL_INPUT); + GpioPinFuncSet (37, GPIO_FSEL_INPUT); + GpioPinFuncSet (38, GPIO_FSEL_INPUT); + GpioPinFuncSet (39, GPIO_FSEL_INPUT); + GpioPinFuncSet (48, GPIO_FSEL_INPUT); + GpioPinFuncSet (49, GPIO_FSEL_INPUT); + GpioPinFuncSet (50, GPIO_FSEL_INPUT); + GpioPinFuncSet (51, GPIO_FSEL_INPUT); + GpioPinFuncSet (52, GPIO_FSEL_INPUT); + GpioPinFuncSet (53, GPIO_FSEL_INPUT); + + if (PcdGet32 (PcdSdIsArasan)) { + DEBUG ((DEBUG_INFO, "Routing SD to Arasan\n")); + Gpio48Group = GPIO_FSEL_ALT3; + /* + * Route SDIO to SdHost. + */ + Gpio34Group = GPIO_FSEL_ALT0; + } else { + DEBUG ((DEBUG_INFO, "Routing SD to SdHost\n")); + Gpio48Group = GPIO_FSEL_ALT0; + /* + * Route SDIO to Arasan. + */ + Gpio34Group = GPIO_FSEL_ALT3; + } + GpioPinFuncSet (34, Gpio34Group); + GpioPinFuncSet (35, Gpio34Group); + GpioPinFuncSet (36, Gpio34Group); + GpioPinFuncSet (37, Gpio34Group); + GpioPinFuncSet (38, Gpio34Group); + GpioPinFuncSet (39, Gpio34Group); + GpioPinFuncSet (48, Gpio48Group); + GpioPinFuncSet (49, Gpio48Group); + GpioPinFuncSet (50, Gpio48Group); + GpioPinFuncSet (51, Gpio48Group); + GpioPinFuncSet (52, Gpio48Group); + GpioPinFuncSet (53, Gpio48Group); + + } else if (ModelFamily == 4){Missing space before opening brackets.
Thanks for the review. Will fix that. Regards, /Pete
Patch looks good otherwise./* - * Route SDIO to Arasan. + * Pi 4: either Arasan or eMMC2 goes to SD card. */ - Gpio34Group = GPIO_FSEL_ALT3; + if (PcdGet32 (PcdSdIsArasan)) { + /* + * WiFi disabled. + */ + GpioPinFuncSet (34, GPIO_FSEL_INPUT); + GpioPinFuncSet (35, GPIO_FSEL_INPUT); + GpioPinFuncSet (36, GPIO_FSEL_INPUT); + GpioPinFuncSet (37, GPIO_FSEL_INPUT); + GpioPinFuncSet (38, GPIO_FSEL_INPUT); + GpioPinFuncSet (39, GPIO_FSEL_INPUT); + /* + * SD card pins go to Arasan. + */ + MmioWrite32((GPIO_BASE_ADDRESS + 0xD0), + MmioRead32(GPIO_BASE_ADDRESS + 0xD0) | 0x2); + } else { + /* + * SD card pins back to eMMC2. + */ + MmioWrite32((GPIO_BASE_ADDRESS + 0xD0), + MmioRead32(GPIO_BASE_ADDRESS + 0xD0) & ~0x2); + /* + * WiFi back to Arasan. + */ + GpioPinFuncSet (34, GPIO_FSEL_ALT3); + GpioPinFuncSet (35, GPIO_FSEL_ALT3); + GpioPinFuncSet (36, GPIO_FSEL_ALT3); + GpioPinFuncSet (37, GPIO_FSEL_ALT3); + GpioPinFuncSet (38, GPIO_FSEL_ALT3); + GpioPinFuncSet (39, GPIO_FSEL_ALT3); + } + } else {+ DEBUG ((DEBUG_ERROR, "Model Family %d not supported...\n", ModelFamily));} - GpioPinFuncSet (34, Gpio34Group); - GpioPinFuncSet (35, Gpio34Group); - GpioPinFuncSet (36, Gpio34Group); - GpioPinFuncSet (37, Gpio34Group); - GpioPinFuncSet (38, Gpio34Group); - GpioPinFuncSet (39, Gpio34Group); - GpioPinFuncSet (48, Gpio48Group); - GpioPinFuncSet (49, Gpio48Group); - GpioPinFuncSet (50, Gpio48Group); - GpioPinFuncSet (51, Gpio48Group); - GpioPinFuncSet (52, Gpio48Group); - GpioPinFuncSet (53, Gpio48Group); /* * JTAG pin JTAG sig GPIO Mode Header pin
-=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#51358): https://edk2.groups.io/g/devel/message/51358 Mute This Topic: https://groups.io/mt/62504750/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
