Leif,
This remained unsent. Please see below.
2016-11-29 13:25 GMT+01:00 Leif Lindholm leif.lindholm@linaro.org:
On Sun, Nov 27, 2016 at 10:55:45PM +0100, Marcin Wojtas wrote:
From: Jan Dąbroś jsd@semihalf.com
Because Xenon SD/MMC controller isn't fully compatible with SDHC, it demands some quirks on generic driver during initialization, i.e. limit bus width to 4, no voltage switch and hence disable highest speed modes, etc. Moreover, Xenon-specific init sequence is required, because it configures custom set of registers of Sd/Mmc PHY.
Two files containing Xenon-specific functions and defines were added.
Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Jan Dabros jsd@semihalf.com Signed-off-by: Marcin Wojtas mw@semihalf.com
Drivers/SdMmc/XenonDxe/MvComponentName.c | 4 +- Drivers/SdMmc/XenonDxe/MvEmmcDevice.c | 4 +- Drivers/SdMmc/XenonDxe/MvSdDevice.c | 4 +- Drivers/SdMmc/XenonDxe/MvSdMmcPciHcDxe.c | 122 +++--- Drivers/SdMmc/XenonDxe/MvSdMmcPciHcDxe.h | 4 +- Drivers/SdMmc/XenonDxe/MvSdMmcPciHcDxe.inf | 46 +- Drivers/SdMmc/XenonDxe/MvSdMmcPciHci.c | 5 +- Drivers/SdMmc/XenonDxe/XenonSdhci.c | 666 +++++++++++++++++++++++++++++ Drivers/SdMmc/XenonDxe/XenonSdhci.h | 346 +++++++++++++++ 9 files changed, 1107 insertions(+), 94 deletions(-) create mode 100755 Drivers/SdMmc/XenonDxe/XenonSdhci.c create mode 100644 Drivers/SdMmc/XenonDxe/XenonSdhci.h
diff --git a/Drivers/SdMmc/XenonDxe/MvComponentName.c b/Drivers/SdMmc/XenonDxe/MvComponentName.c index 3329929..112be3a 100644 --- a/Drivers/SdMmc/XenonDxe/MvComponentName.c +++ b/Drivers/SdMmc/XenonDxe/MvComponentName.c @@ -2,6 +2,8 @@ UEFI Component Name(2) protocol implementation for SD/MMC host controller driver.
Copyright (c) 2015, Intel Corporation. All rights reserved.<BR>
- Copyright (C) 2016 Marvell International Ltd. All rigths reserved.<BR>
- This program and the accompanying materials are licensed and made available under the terms and conditions of the BSD License which accompanies this distribution. The full text of the license may be found at
@@ -12,7 +14,7 @@
**/
-#include "SdMmcPciHcDxe.h" +#include "XenonSdhci.h"
So, OK, I've already given you reviewed-by on the import (not thinking through properly), but was the file renaming (as opposed to the copy) really necessary?
Since this component is not exporting global symbols, all global disambiguation happens via GUID. The only effect the rename has is to grow this (now much smaller) diff against original.
4 files are modified compared to the original only to deal with the rename.
Note that this also breaks bisect of something attempting to build this component standalone (while not breaking the actual tree).
Ok, original names will be restored.
diff --git a/Drivers/SdMmc/XenonDxe/MvSdMmcPciHci.c b/Drivers/SdMmc/XenonDxe/MvSdMmcPciHci.c index ccbf355..47be381 100644 --- a/Drivers/SdMmc/XenonDxe/MvSdMmcPciHci.c +++ b/Drivers/SdMmc/XenonDxe/MvSdMmcPciHci.c @@ -5,6 +5,8 @@ It would expose EFI_SD_MMC_PASS_THRU_PROTOCOL for upper layer use.
Copyright (c) 2015 - 2016, Intel Corporation. All rights reserved.<BR>
- Copyright (C) 2016 Marvell International Ltd. All rigths reserved.<BR>
- This program and the accompanying materials are licensed and made available under the terms and conditions of the BSD License which accompanies this distribution. The full text of the license may be found at
@@ -15,7 +17,7 @@
**/
-#include "SdMmcPciHcDxe.h" +#include "XenonSdhci.h"
/** Dump the content of SD/MMC host controller's Capability Register. @@ -1636,6 +1638,7 @@ SdMmcExecTrb ( break; } }
Now now - that needlessly added newline wasn't in v2. Seeing things like this makes me nervous other things may have been changed since my previous review, forcing me to spend time going away diffing the two versions...
Sorry for that. It remained unnoticed after squashing the DMA debugs:/
OK, so apart from this one it doesn't look too bad ... but it does show that you have changed/added many copyright statements - correctly, but this should have been mentioned in 0/7.
Please address, and please take more care in future.
Ok, thank you.
Best regards, Marcin