In order to enable modification of dynamic PCD's for the libraries
and DXE drivers, this patch introduces new driver. It is
executed prior to other drivers. Mpp, ComPhy and Utmi libraries
initialization were moved from PrePi stage to DXE.
To force the correct driver dispatch sequence, introduce a protocol GUID
and install the protocol as a NULL protocol when PlatInitDxe executes.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Marcin Wojtas <mw@semihalf.com>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
Platform/Marvell/Armada/Armada.dsc.inc | 3 ++
Platform/Marvell/Armada/Armada70x0.fdf | 5 +++
Platform/Marvell/Armada/Drivers/PlatInitDxe/PlatInitDxe.c | 44 ++++++++++++++++++++
Platform/Marvell/Armada/Drivers/PlatInitDxe/PlatInitDxe.inf | 44 ++++++++++++++++++++
Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0Lib.c | 11 -----
Platform/Marvell/Marvell.dec | 5 +++
6 files changed, 101 insertions(+), 11 deletions(-)
diff --git a/Platform/Marvell/Armada/Armada.dsc.inc b/Platform/Marvell/Armada/Armada.dsc.inc
index 89fb7e7..417bb0c 100644
--- a/Platform/Marvell/Armada/Armada.dsc.inc
+++ b/Platform/Marvell/Armada/Armada.dsc.inc
@@ -378,6 +378,9 @@
ArmPkg/Drivers/TimerDxe/TimerDxe.inf
ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.inf
+ # Platform Initialization
+ Platform/Marvell/Armada/Drivers/PlatInitDxe/PlatInitDxe.inf
+
# Platform drivers
Platform/Marvell/Drivers/I2c/MvI2cDxe/MvI2cDxe.inf
MdeModulePkg/Bus/I2c/I2cDxe/I2cDxe.inf
diff --git a/Platform/Marvell/Armada/Armada70x0.fdf b/Platform/Marvell/Armada/Armada70x0.fdf
index c861e78..763d76a 100644
--- a/Platform/Marvell/Armada/Armada70x0.fdf
+++ b/Platform/Marvell/Armada/Armada70x0.fdf
@@ -89,6 +89,11 @@ FvNameGuid = 5eda4200-2c5f-43cb-9da3-0baf74b1b30c
INF MdeModulePkg/Core/Dxe/DxeMain.inf
+ #
+ # Platform Initialization
+ #
+ INF Platform/Marvell/Armada/Drivers/PlatInitDxe/PlatInitDxe.inf
+
# PI DXE Drivers producing Architectural Protocols (EFI Services)
INF ArmPkg/Drivers/CpuDxe/CpuDxe.inf
INF ArmPkg/Drivers/ArmGic/ArmGicDxe.inf
diff --git a/Platform/Marvell/Armada/Drivers/PlatInitDxe/PlatInitDxe.c b/Platform/Marvell/Armada/Drivers/PlatInitDxe/PlatInitDxe.c
new file mode 100644
index 0000000..919454b
--- /dev/null
+++ b/Platform/Marvell/Armada/Drivers/PlatInitDxe/PlatInitDxe.c
@@ -0,0 +1,44 @@
+/** @file
+ Copyright (C) Marvell International Ltd. and its affiliates
+
+ 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
+ http://opensource.org/licenses/bsd-license.php
+
+ THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+
+**/
+
+#include <Library/DebugLib.h>
+#include <Library/MppLib.h>
+#include <Library/MvComPhyLib.h>
+#include <Library/PcdLib.h>
+#include <Library/UefiDriverEntryPoint.h>
+#include <Library/UefiBootServicesTableLib.h>
+#include <Library/UtmiPhyLib.h>
+
+EFI_STATUS
+EFIAPI
+ArmadaPlatInitDxeEntryPoint (
+ IN EFI_HANDLE ImageHandle,
+ IN EFI_SYSTEM_TABLE *SystemTable
+ )
+{
+ EFI_STATUS Status;
+
+ DEBUG ((DEBUG_ERROR, "\nArmada Platform Init\n\n"));
+
+ Status = gBS->InstallProtocolInterface (&ImageHandle,
+ &gMarvellPlatformInitCompleteProtocolGuid,
+ EFI_NATIVE_INTERFACE,
+ NULL);
+ ASSERT_EFI_ERROR (Status);
+
+ MvComPhyInit ();
+ UtmiPhyInit ();
+ MppInitialize ();
+
+ return EFI_SUCCESS;
+}
diff --git a/Platform/Marvell/Armada/Drivers/PlatInitDxe/PlatInitDxe.inf b/Platform/Marvell/Armada/Drivers/PlatInitDxe/PlatInitDxe.inf
new file mode 100644
index 0000000..29abcaf
--- /dev/null
+++ b/Platform/Marvell/Armada/Drivers/PlatInitDxe/PlatInitDxe.inf
@@ -0,0 +1,44 @@
+#/* @file
+# Copyright (C) Marvell International Ltd. and its affiliates
+#
+# 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
+# http://opensource.org/licenses/bsd-license.php
+#
+# THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+# WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+#
+#*/
+
+[Defines]
+ INF_VERSION = 0x00010019
+ BASE_NAME = PlatInitDxe
+ FILE_GUID = 8c66f65b-08a6-4c91-b993-ff81e0adf818
+ MODULE_TYPE = DXE_DRIVER
+ VERSION_STRING = 1.0
+
+ ENTRY_POINT = ArmadaPlatInitDxeEntryPoint
+
+[Sources]
+ PlatInitDxe.c
+
+[Packages]
+ MdeModulePkg/MdeModulePkg.dec
+ MdePkg/MdePkg.dec
+ Platform/Marvell/Marvell.dec
+
+[LibraryClasses]
+ ComPhyLib
+ DebugLib
+ MppLib
+ PcdLib
+ TimerLib
+ UefiDriverEntryPoint
+ UtmiPhyLib
+
+[Protocols]
+ gMarvellPlatformInitCompleteProtocolGuid ## PRODUCES
+
+[Depex]
+ TRUE
diff --git a/Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0Lib.c b/Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0Lib.c
index 0ed310f..968d28f 100644
--- a/Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0Lib.c
+++ b/Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0Lib.c
@@ -15,12 +15,8 @@
#include <Library/ArmLib.h>
#include <Library/ArmPlatformLib.h>
-#include <Library/MppLib.h>
-#include <Library/MvComPhyLib.h>
-#include <Library/UtmiPhyLib.h>
#include <Ppi/ArmMpCoreInfo.h>
-
ARM_CORE_INFO mArmada7040MpCoreInfoTable[] = {
{
// Cluster 0, Core 0
@@ -90,13 +86,6 @@ ArmPlatformInitialize (
IN UINTN MpId
)
{
- if (!ArmPlatformIsPrimaryCore (MpId)) {
- return RETURN_SUCCESS;
- }
-
- MvComPhyInit ();
- UtmiPhyInit ();
- MppInitialize ();
return RETURN_SUCCESS;
}
diff --git a/Platform/Marvell/Marvell.dec b/Platform/Marvell/Marvell.dec
index 0902086..e7d7c2c 100644
--- a/Platform/Marvell/Marvell.dec
+++ b/Platform/Marvell/Marvell.dec
@@ -56,6 +56,11 @@
gShellFUpdateHiiGuid = { 0x9b5d2176, 0x590a, 0x49db, { 0x89, 0x5d, 0x4a, 0x70, 0xfe, 0xad, 0xbe, 0x24 } }
gShellSfHiiGuid = { 0x03a67756, 0x8cde, 0x4638, { 0x82, 0x34, 0x4a, 0x0f, 0x6d, 0x58, 0x81, 0x39 } }
+[Protocols]
+ # installed as a protocol by PlatInitDxe to force ordering between DXE drivers
+ # that depend on the lowlevel platform initialization having been completed
+ gMarvellPlatformInitCompleteProtocolGuid = { 0x465b8cf7, 0x016f, 0x4ba6, { 0xbe, 0x6b, 0x28, 0x0e, 0x3a, 0x7d, 0x38, 0x6f } }
+
[PcdsFixedAtBuild.common]
#MPP
gMarvellTokenSpaceGuid.PcdMppChipCount|0|UINT32|0x30000001
--
1.8.3.1
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
On Mon, Oct 09, 2017 at 07:00:50PM +0200, Marcin Wojtas wrote:
> In order to enable modification of dynamic PCD's for the libraries
> and DXE drivers, this patch introduces new driver. It is
> executed prior to other drivers. Mpp, ComPhy and Utmi libraries
> initialization were moved from PrePi stage to DXE.
>
> To force the correct driver dispatch sequence, introduce a protocol GUID
> and install the protocol as a NULL protocol when PlatInitDxe executes.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Marcin Wojtas <mw@semihalf.com>
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
What does Ard's Signed-off-by signify here?
(I know the authorship on some of these is a bit blurred, since you've
been working together, but I'd like to be clear.)
> ---
> Platform/Marvell/Armada/Armada.dsc.inc | 3 ++
> Platform/Marvell/Armada/Armada70x0.fdf | 5 +++
> Platform/Marvell/Armada/Drivers/PlatInitDxe/PlatInitDxe.c | 44 ++++++++++++++++++++
> Platform/Marvell/Armada/Drivers/PlatInitDxe/PlatInitDxe.inf | 44 ++++++++++++++++++++
> Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0Lib.c | 11 -----
> Platform/Marvell/Marvell.dec | 5 +++
> 6 files changed, 101 insertions(+), 11 deletions(-)
>
> diff --git a/Platform/Marvell/Armada/Armada.dsc.inc b/Platform/Marvell/Armada/Armada.dsc.inc
> index 89fb7e7..417bb0c 100644
> --- a/Platform/Marvell/Armada/Armada.dsc.inc
> +++ b/Platform/Marvell/Armada/Armada.dsc.inc
> @@ -378,6 +378,9 @@
> ArmPkg/Drivers/TimerDxe/TimerDxe.inf
> ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.inf
>
> + # Platform Initialization
> + Platform/Marvell/Armada/Drivers/PlatInitDxe/PlatInitDxe.inf
> +
> # Platform drivers
> Platform/Marvell/Drivers/I2c/MvI2cDxe/MvI2cDxe.inf
> MdeModulePkg/Bus/I2c/I2cDxe/I2cDxe.inf
> diff --git a/Platform/Marvell/Armada/Armada70x0.fdf b/Platform/Marvell/Armada/Armada70x0.fdf
> index c861e78..763d76a 100644
> --- a/Platform/Marvell/Armada/Armada70x0.fdf
> +++ b/Platform/Marvell/Armada/Armada70x0.fdf
> @@ -89,6 +89,11 @@ FvNameGuid = 5eda4200-2c5f-43cb-9da3-0baf74b1b30c
>
> INF MdeModulePkg/Core/Dxe/DxeMain.inf
>
> + #
> + # Platform Initialization
> + #
> + INF Platform/Marvell/Armada/Drivers/PlatInitDxe/PlatInitDxe.inf
> +
> # PI DXE Drivers producing Architectural Protocols (EFI Services)
> INF ArmPkg/Drivers/CpuDxe/CpuDxe.inf
> INF ArmPkg/Drivers/ArmGic/ArmGicDxe.inf
> diff --git a/Platform/Marvell/Armada/Drivers/PlatInitDxe/PlatInitDxe.c b/Platform/Marvell/Armada/Drivers/PlatInitDxe/PlatInitDxe.c
> new file mode 100644
> index 0000000..919454b
> --- /dev/null
> +++ b/Platform/Marvell/Armada/Drivers/PlatInitDxe/PlatInitDxe.c
> @@ -0,0 +1,44 @@
> +/** @file
> + Copyright (C) Marvell International Ltd. and its affiliates
We normally need a year here as well.
If Ard has co-authored parts, I guess we should have Linaro copyright
notice on affected files as well.
Content of the patch is fine.
/
Leif
> +
> + 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
> + http://opensource.org/licenses/bsd-license.php
> +
> + THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> + WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> +
> +**/
> +
> +#include <Library/DebugLib.h>
> +#include <Library/MppLib.h>
> +#include <Library/MvComPhyLib.h>
> +#include <Library/PcdLib.h>
> +#include <Library/UefiDriverEntryPoint.h>
> +#include <Library/UefiBootServicesTableLib.h>
> +#include <Library/UtmiPhyLib.h>
> +
> +EFI_STATUS
> +EFIAPI
> +ArmadaPlatInitDxeEntryPoint (
> + IN EFI_HANDLE ImageHandle,
> + IN EFI_SYSTEM_TABLE *SystemTable
> + )
> +{
> + EFI_STATUS Status;
> +
> + DEBUG ((DEBUG_ERROR, "\nArmada Platform Init\n\n"));
> +
> + Status = gBS->InstallProtocolInterface (&ImageHandle,
> + &gMarvellPlatformInitCompleteProtocolGuid,
> + EFI_NATIVE_INTERFACE,
> + NULL);
> + ASSERT_EFI_ERROR (Status);
> +
> + MvComPhyInit ();
> + UtmiPhyInit ();
> + MppInitialize ();
> +
> + return EFI_SUCCESS;
> +}
> diff --git a/Platform/Marvell/Armada/Drivers/PlatInitDxe/PlatInitDxe.inf b/Platform/Marvell/Armada/Drivers/PlatInitDxe/PlatInitDxe.inf
> new file mode 100644
> index 0000000..29abcaf
> --- /dev/null
> +++ b/Platform/Marvell/Armada/Drivers/PlatInitDxe/PlatInitDxe.inf
> @@ -0,0 +1,44 @@
> +#/* @file
> +# Copyright (C) Marvell International Ltd. and its affiliates
> +#
> +# 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
> +# http://opensource.org/licenses/bsd-license.php
> +#
> +# THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +# WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> +#
> +#*/
> +
> +[Defines]
> + INF_VERSION = 0x00010019
> + BASE_NAME = PlatInitDxe
> + FILE_GUID = 8c66f65b-08a6-4c91-b993-ff81e0adf818
> + MODULE_TYPE = DXE_DRIVER
> + VERSION_STRING = 1.0
> +
> + ENTRY_POINT = ArmadaPlatInitDxeEntryPoint
> +
> +[Sources]
> + PlatInitDxe.c
> +
> +[Packages]
> + MdeModulePkg/MdeModulePkg.dec
> + MdePkg/MdePkg.dec
> + Platform/Marvell/Marvell.dec
> +
> +[LibraryClasses]
> + ComPhyLib
> + DebugLib
> + MppLib
> + PcdLib
> + TimerLib
> + UefiDriverEntryPoint
> + UtmiPhyLib
> +
> +[Protocols]
> + gMarvellPlatformInitCompleteProtocolGuid ## PRODUCES
> +
> +[Depex]
> + TRUE
> diff --git a/Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0Lib.c b/Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0Lib.c
> index 0ed310f..968d28f 100644
> --- a/Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0Lib.c
> +++ b/Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0Lib.c
> @@ -15,12 +15,8 @@
>
> #include <Library/ArmLib.h>
> #include <Library/ArmPlatformLib.h>
> -#include <Library/MppLib.h>
> -#include <Library/MvComPhyLib.h>
> -#include <Library/UtmiPhyLib.h>
> #include <Ppi/ArmMpCoreInfo.h>
>
> -
> ARM_CORE_INFO mArmada7040MpCoreInfoTable[] = {
> {
> // Cluster 0, Core 0
> @@ -90,13 +86,6 @@ ArmPlatformInitialize (
> IN UINTN MpId
> )
> {
> - if (!ArmPlatformIsPrimaryCore (MpId)) {
> - return RETURN_SUCCESS;
> - }
> -
> - MvComPhyInit ();
> - UtmiPhyInit ();
> - MppInitialize ();
> return RETURN_SUCCESS;
> }
>
> diff --git a/Platform/Marvell/Marvell.dec b/Platform/Marvell/Marvell.dec
> index 0902086..e7d7c2c 100644
> --- a/Platform/Marvell/Marvell.dec
> +++ b/Platform/Marvell/Marvell.dec
> @@ -56,6 +56,11 @@
> gShellFUpdateHiiGuid = { 0x9b5d2176, 0x590a, 0x49db, { 0x89, 0x5d, 0x4a, 0x70, 0xfe, 0xad, 0xbe, 0x24 } }
> gShellSfHiiGuid = { 0x03a67756, 0x8cde, 0x4638, { 0x82, 0x34, 0x4a, 0x0f, 0x6d, 0x58, 0x81, 0x39 } }
>
> +[Protocols]
> + # installed as a protocol by PlatInitDxe to force ordering between DXE drivers
> + # that depend on the lowlevel platform initialization having been completed
> + gMarvellPlatformInitCompleteProtocolGuid = { 0x465b8cf7, 0x016f, 0x4ba6, { 0xbe, 0x6b, 0x28, 0x0e, 0x3a, 0x7d, 0x38, 0x6f } }
> +
> [PcdsFixedAtBuild.common]
> #MPP
> gMarvellTokenSpaceGuid.PcdMppChipCount|0|UINT32|0x30000001
> --
> 1.8.3.1
>
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Hi Leif, 2017-10-10 16:37 GMT+02:00 Leif Lindholm <leif.lindholm@linaro.org>: > On Mon, Oct 09, 2017 at 07:00:50PM +0200, Marcin Wojtas wrote: >> In order to enable modification of dynamic PCD's for the libraries >> and DXE drivers, this patch introduces new driver. It is >> executed prior to other drivers. Mpp, ComPhy and Utmi libraries >> initialization were moved from PrePi stage to DXE. >> >> To force the correct driver dispatch sequence, introduce a protocol GUID >> and install the protocol as a NULL protocol when PlatInitDxe executes. >> >> Contributed-under: TianoCore Contribution Agreement 1.1 >> Signed-off-by: Marcin Wojtas <mw@semihalf.com> >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > > What does Ard's Signed-off-by signify here? > (I know the authorship on some of these is a bit blurred, since you've > been working together, but I'd like to be clear.) These were the lines, introducing/installing protocol GUID stuff. It was in a small separate patch, but I squashed it into bigger one. > >> --- >> Platform/Marvell/Armada/Armada.dsc.inc | 3 ++ >> Platform/Marvell/Armada/Armada70x0.fdf | 5 +++ >> Platform/Marvell/Armada/Drivers/PlatInitDxe/PlatInitDxe.c | 44 ++++++++++++++++++++ >> Platform/Marvell/Armada/Drivers/PlatInitDxe/PlatInitDxe.inf | 44 ++++++++++++++++++++ >> Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0Lib.c | 11 ----- >> Platform/Marvell/Marvell.dec | 5 +++ >> 6 files changed, 101 insertions(+), 11 deletions(-) >> >> diff --git a/Platform/Marvell/Armada/Armada.dsc.inc b/Platform/Marvell/Armada/Armada.dsc.inc >> index 89fb7e7..417bb0c 100644 >> --- a/Platform/Marvell/Armada/Armada.dsc.inc >> +++ b/Platform/Marvell/Armada/Armada.dsc.inc >> @@ -378,6 +378,9 @@ >> ArmPkg/Drivers/TimerDxe/TimerDxe.inf >> ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.inf >> >> + # Platform Initialization >> + Platform/Marvell/Armada/Drivers/PlatInitDxe/PlatInitDxe.inf >> + >> # Platform drivers >> Platform/Marvell/Drivers/I2c/MvI2cDxe/MvI2cDxe.inf >> MdeModulePkg/Bus/I2c/I2cDxe/I2cDxe.inf >> diff --git a/Platform/Marvell/Armada/Armada70x0.fdf b/Platform/Marvell/Armada/Armada70x0.fdf >> index c861e78..763d76a 100644 >> --- a/Platform/Marvell/Armada/Armada70x0.fdf >> +++ b/Platform/Marvell/Armada/Armada70x0.fdf >> @@ -89,6 +89,11 @@ FvNameGuid = 5eda4200-2c5f-43cb-9da3-0baf74b1b30c >> >> INF MdeModulePkg/Core/Dxe/DxeMain.inf >> >> + # >> + # Platform Initialization >> + # >> + INF Platform/Marvell/Armada/Drivers/PlatInitDxe/PlatInitDxe.inf >> + >> # PI DXE Drivers producing Architectural Protocols (EFI Services) >> INF ArmPkg/Drivers/CpuDxe/CpuDxe.inf >> INF ArmPkg/Drivers/ArmGic/ArmGicDxe.inf >> diff --git a/Platform/Marvell/Armada/Drivers/PlatInitDxe/PlatInitDxe.c b/Platform/Marvell/Armada/Drivers/PlatInitDxe/PlatInitDxe.c >> new file mode 100644 >> index 0000000..919454b >> --- /dev/null >> +++ b/Platform/Marvell/Armada/Drivers/PlatInitDxe/PlatInitDxe.c >> @@ -0,0 +1,44 @@ >> +/** @file >> + Copyright (C) Marvell International Ltd. and its affiliates > > We normally need a year here as well. > If Ard has co-authored parts, I guess we should have Linaro copyright > notice on affected files as well. I have no problem with that, if you find it appropriate, given my explanation above. > > Content of the patch is fine. > Ok, thanks! Marcin _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
On Tue, Oct 10, 2017 at 04:45:10PM +0200, Marcin Wojtas wrote:
> Hi Leif,
>
> 2017-10-10 16:37 GMT+02:00 Leif Lindholm <leif.lindholm@linaro.org>:
> > On Mon, Oct 09, 2017 at 07:00:50PM +0200, Marcin Wojtas wrote:
> >> In order to enable modification of dynamic PCD's for the libraries
> >> and DXE drivers, this patch introduces new driver. It is
> >> executed prior to other drivers. Mpp, ComPhy and Utmi libraries
> >> initialization were moved from PrePi stage to DXE.
> >>
> >> To force the correct driver dispatch sequence, introduce a protocol GUID
> >> and install the protocol as a NULL protocol when PlatInitDxe executes.
> >>
> >> Contributed-under: TianoCore Contribution Agreement 1.1
> >> Signed-off-by: Marcin Wojtas <mw@semihalf.com>
> >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> >
> > What does Ard's Signed-off-by signify here?
> > (I know the authorship on some of these is a bit blurred, since you've
> > been working together, but I'd like to be clear.)
>
> These were the lines, introducing/installing protocol GUID stuff. It
> was in a small separate patch, but I squashed it into bigger one.
Personally, I would in this instance do:
<me>
Ard
<me>
It's verbose, but reasonably clear.
> >
> >> ---
> >> Platform/Marvell/Armada/Armada.dsc.inc | 3 ++
> >> Platform/Marvell/Armada/Armada70x0.fdf | 5 +++
> >> Platform/Marvell/Armada/Drivers/PlatInitDxe/PlatInitDxe.c | 44 ++++++++++++++++++++
> >> Platform/Marvell/Armada/Drivers/PlatInitDxe/PlatInitDxe.inf | 44 ++++++++++++++++++++
> >> Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0Lib.c | 11 -----
> >> Platform/Marvell/Marvell.dec | 5 +++
> >> 6 files changed, 101 insertions(+), 11 deletions(-)
> >>
> >> diff --git a/Platform/Marvell/Armada/Armada.dsc.inc b/Platform/Marvell/Armada/Armada.dsc.inc
> >> index 89fb7e7..417bb0c 100644
> >> --- a/Platform/Marvell/Armada/Armada.dsc.inc
> >> +++ b/Platform/Marvell/Armada/Armada.dsc.inc
> >> @@ -378,6 +378,9 @@
> >> ArmPkg/Drivers/TimerDxe/TimerDxe.inf
> >> ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.inf
> >>
> >> + # Platform Initialization
> >> + Platform/Marvell/Armada/Drivers/PlatInitDxe/PlatInitDxe.inf
> >> +
> >> # Platform drivers
> >> Platform/Marvell/Drivers/I2c/MvI2cDxe/MvI2cDxe.inf
> >> MdeModulePkg/Bus/I2c/I2cDxe/I2cDxe.inf
> >> diff --git a/Platform/Marvell/Armada/Armada70x0.fdf b/Platform/Marvell/Armada/Armada70x0.fdf
> >> index c861e78..763d76a 100644
> >> --- a/Platform/Marvell/Armada/Armada70x0.fdf
> >> +++ b/Platform/Marvell/Armada/Armada70x0.fdf
> >> @@ -89,6 +89,11 @@ FvNameGuid = 5eda4200-2c5f-43cb-9da3-0baf74b1b30c
> >>
> >> INF MdeModulePkg/Core/Dxe/DxeMain.inf
> >>
> >> + #
> >> + # Platform Initialization
> >> + #
> >> + INF Platform/Marvell/Armada/Drivers/PlatInitDxe/PlatInitDxe.inf
> >> +
> >> # PI DXE Drivers producing Architectural Protocols (EFI Services)
> >> INF ArmPkg/Drivers/CpuDxe/CpuDxe.inf
> >> INF ArmPkg/Drivers/ArmGic/ArmGicDxe.inf
> >> diff --git a/Platform/Marvell/Armada/Drivers/PlatInitDxe/PlatInitDxe.c b/Platform/Marvell/Armada/Drivers/PlatInitDxe/PlatInitDxe.c
> >> new file mode 100644
> >> index 0000000..919454b
> >> --- /dev/null
> >> +++ b/Platform/Marvell/Armada/Drivers/PlatInitDxe/PlatInitDxe.c
> >> @@ -0,0 +1,44 @@
> >> +/** @file
> >> + Copyright (C) Marvell International Ltd. and its affiliates
> >
> > We normally need a year here as well.
> > If Ard has co-authored parts, I guess we should have Linaro copyright
> > notice on affected files as well.
>
> I have no problem with that, if you find it appropriate, given my
> explanation above.
Personally I dont mind much, but I think it would be more correct.
/
Leif
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
2017-10-10 17:03 GMT+02:00 Leif Lindholm <leif.lindholm@linaro.org>: > On Tue, Oct 10, 2017 at 04:45:10PM +0200, Marcin Wojtas wrote: >> Hi Leif, >> >> 2017-10-10 16:37 GMT+02:00 Leif Lindholm <leif.lindholm@linaro.org>: >> > On Mon, Oct 09, 2017 at 07:00:50PM +0200, Marcin Wojtas wrote: >> >> In order to enable modification of dynamic PCD's for the libraries >> >> and DXE drivers, this patch introduces new driver. It is >> >> executed prior to other drivers. Mpp, ComPhy and Utmi libraries >> >> initialization were moved from PrePi stage to DXE. >> >> >> >> To force the correct driver dispatch sequence, introduce a protocol GUID >> >> and install the protocol as a NULL protocol when PlatInitDxe executes. >> >> >> >> Contributed-under: TianoCore Contribution Agreement 1.1 >> >> Signed-off-by: Marcin Wojtas <mw@semihalf.com> >> >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> >> > >> > What does Ard's Signed-off-by signify here? >> > (I know the authorship on some of these is a bit blurred, since you've >> > been working together, but I'd like to be clear.) >> >> These were the lines, introducing/installing protocol GUID stuff. It >> was in a small separate patch, but I squashed it into bigger one. > > Personally, I would in this instance do: > <me> > Ard > <me> > > It's verbose, but reasonably clear. > How about: In order to enable modification of dynamic PCD's for the libraries and DXE drivers, this patch introduces new driver. It is executed prior to other drivers. Mpp, ComPhy and Utmi libraries initialization were moved from PrePi stage to DXE. Signed-off-by: Marcin Wojtas <mw@semihalf.com> To force the correct driver dispatch sequence, introduce a protocol GUID and install the protocol as a NULL protocol when PlatInitDxe executes. Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> Signed-off-by: Marcin Wojtas <mw@semihalf.com> ? Was that, what you meant? >> > >> >> --- >> >> Platform/Marvell/Armada/Armada.dsc.inc | 3 ++ >> >> Platform/Marvell/Armada/Armada70x0.fdf | 5 +++ >> >> Platform/Marvell/Armada/Drivers/PlatInitDxe/PlatInitDxe.c | 44 ++++++++++++++++++++ >> >> Platform/Marvell/Armada/Drivers/PlatInitDxe/PlatInitDxe.inf | 44 ++++++++++++++++++++ >> >> Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0Lib.c | 11 ----- >> >> Platform/Marvell/Marvell.dec | 5 +++ >> >> 6 files changed, 101 insertions(+), 11 deletions(-) >> >> >> >> diff --git a/Platform/Marvell/Armada/Armada.dsc.inc b/Platform/Marvell/Armada/Armada.dsc.inc >> >> index 89fb7e7..417bb0c 100644 >> >> --- a/Platform/Marvell/Armada/Armada.dsc.inc >> >> +++ b/Platform/Marvell/Armada/Armada.dsc.inc >> >> @@ -378,6 +378,9 @@ >> >> ArmPkg/Drivers/TimerDxe/TimerDxe.inf >> >> ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.inf >> >> >> >> + # Platform Initialization >> >> + Platform/Marvell/Armada/Drivers/PlatInitDxe/PlatInitDxe.inf >> >> + >> >> # Platform drivers >> >> Platform/Marvell/Drivers/I2c/MvI2cDxe/MvI2cDxe.inf >> >> MdeModulePkg/Bus/I2c/I2cDxe/I2cDxe.inf >> >> diff --git a/Platform/Marvell/Armada/Armada70x0.fdf b/Platform/Marvell/Armada/Armada70x0.fdf >> >> index c861e78..763d76a 100644 >> >> --- a/Platform/Marvell/Armada/Armada70x0.fdf >> >> +++ b/Platform/Marvell/Armada/Armada70x0.fdf >> >> @@ -89,6 +89,11 @@ FvNameGuid = 5eda4200-2c5f-43cb-9da3-0baf74b1b30c >> >> >> >> INF MdeModulePkg/Core/Dxe/DxeMain.inf >> >> >> >> + # >> >> + # Platform Initialization >> >> + # >> >> + INF Platform/Marvell/Armada/Drivers/PlatInitDxe/PlatInitDxe.inf >> >> + >> >> # PI DXE Drivers producing Architectural Protocols (EFI Services) >> >> INF ArmPkg/Drivers/CpuDxe/CpuDxe.inf >> >> INF ArmPkg/Drivers/ArmGic/ArmGicDxe.inf >> >> diff --git a/Platform/Marvell/Armada/Drivers/PlatInitDxe/PlatInitDxe.c b/Platform/Marvell/Armada/Drivers/PlatInitDxe/PlatInitDxe.c >> >> new file mode 100644 >> >> index 0000000..919454b >> >> --- /dev/null >> >> +++ b/Platform/Marvell/Armada/Drivers/PlatInitDxe/PlatInitDxe.c >> >> @@ -0,0 +1,44 @@ >> >> +/** @file >> >> + Copyright (C) Marvell International Ltd. and its affiliates >> > >> > We normally need a year here as well. >> > If Ard has co-authored parts, I guess we should have Linaro copyright >> > notice on affected files as well. >> >> I have no problem with that, if you find it appropriate, given my >> explanation above. > > Personally I dont mind much, but I think it would be more correct. > Ok, will add it in v2. _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
On Tue, Oct 10, 2017 at 05:06:42PM +0200, Marcin Wojtas wrote:
> 2017-10-10 17:03 GMT+02:00 Leif Lindholm <leif.lindholm@linaro.org>:
> > On Tue, Oct 10, 2017 at 04:45:10PM +0200, Marcin Wojtas wrote:
> >> Hi Leif,
> >>
> >> 2017-10-10 16:37 GMT+02:00 Leif Lindholm <leif.lindholm@linaro.org>:
> >> > On Mon, Oct 09, 2017 at 07:00:50PM +0200, Marcin Wojtas wrote:
> >> >> In order to enable modification of dynamic PCD's for the libraries
> >> >> and DXE drivers, this patch introduces new driver. It is
> >> >> executed prior to other drivers. Mpp, ComPhy and Utmi libraries
> >> >> initialization were moved from PrePi stage to DXE.
> >> >>
> >> >> To force the correct driver dispatch sequence, introduce a protocol GUID
> >> >> and install the protocol as a NULL protocol when PlatInitDxe executes.
> >> >>
> >> >> Contributed-under: TianoCore Contribution Agreement 1.1
> >> >> Signed-off-by: Marcin Wojtas <mw@semihalf.com>
> >> >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> >> >
> >> > What does Ard's Signed-off-by signify here?
> >> > (I know the authorship on some of these is a bit blurred, since you've
> >> > been working together, but I'd like to be clear.)
> >>
> >> These were the lines, introducing/installing protocol GUID stuff. It
> >> was in a small separate patch, but I squashed it into bigger one.
> >
> > Personally, I would in this instance do:
> > <me>
> > Ard
> > <me>
> >
> > It's verbose, but reasonably clear.
> >
>
> How about:
>
> In order to enable modification of dynamic PCD's for the libraries
> and DXE drivers, this patch introduces new driver. It is
> executed prior to other drivers. Mpp, ComPhy and Utmi libraries
> initialization were moved from PrePi stage to DXE.
>
> Signed-off-by: Marcin Wojtas <mw@semihalf.com>
>
> To force the correct driver dispatch sequence, introduce a protocol GUID
> and install the protocol as a NULL protocol when PlatInitDxe executes.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Signed-off-by: Marcin Wojtas <mw@semihalf.com>
>
> ?
>
> Was that, what you meant?
I think Contibuted-under: still needs to come first.
I don't think we have an explicit policy for how to deal with
multi-contributor patches. The ones we do see tend to just keep a
single commit message and list the contributors.
In Linux. it would be something like
Signed-off-by: Marcin Wojtas <mw@semihalf.com>
[Introduce protocol GUID to force correct driver dispatch order]
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Signed-off-by: Marcin Wojtas <mw@semihalf.com>
I would be quite happy to use the same format here.
/
Leif
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
On 10 October 2017 at 16:26, Leif Lindholm <leif.lindholm@linaro.org> wrote: > On Tue, Oct 10, 2017 at 05:06:42PM +0200, Marcin Wojtas wrote: >> 2017-10-10 17:03 GMT+02:00 Leif Lindholm <leif.lindholm@linaro.org>: >> > On Tue, Oct 10, 2017 at 04:45:10PM +0200, Marcin Wojtas wrote: >> >> Hi Leif, >> >> >> >> 2017-10-10 16:37 GMT+02:00 Leif Lindholm <leif.lindholm@linaro.org>: >> >> > On Mon, Oct 09, 2017 at 07:00:50PM +0200, Marcin Wojtas wrote: >> >> >> In order to enable modification of dynamic PCD's for the libraries >> >> >> and DXE drivers, this patch introduces new driver. It is >> >> >> executed prior to other drivers. Mpp, ComPhy and Utmi libraries >> >> >> initialization were moved from PrePi stage to DXE. >> >> >> >> >> >> To force the correct driver dispatch sequence, introduce a protocol GUID >> >> >> and install the protocol as a NULL protocol when PlatInitDxe executes. >> >> >> >> >> >> Contributed-under: TianoCore Contribution Agreement 1.1 >> >> >> Signed-off-by: Marcin Wojtas <mw@semihalf.com> >> >> >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> >> >> > >> >> > What does Ard's Signed-off-by signify here? >> >> > (I know the authorship on some of these is a bit blurred, since you've >> >> > been working together, but I'd like to be clear.) >> >> >> >> These were the lines, introducing/installing protocol GUID stuff. It >> >> was in a small separate patch, but I squashed it into bigger one. >> > >> > Personally, I would in this instance do: >> > <me> >> > Ard >> > <me> >> > >> > It's verbose, but reasonably clear. >> > >> >> How about: >> >> In order to enable modification of dynamic PCD's for the libraries >> and DXE drivers, this patch introduces new driver. It is >> executed prior to other drivers. Mpp, ComPhy and Utmi libraries >> initialization were moved from PrePi stage to DXE. >> >> Signed-off-by: Marcin Wojtas <mw@semihalf.com> >> >> To force the correct driver dispatch sequence, introduce a protocol GUID >> and install the protocol as a NULL protocol when PlatInitDxe executes. >> >> Contributed-under: TianoCore Contribution Agreement 1.1 >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> >> Signed-off-by: Marcin Wojtas <mw@semihalf.com> >> >> ? >> >> Was that, what you meant? > > I think Contibuted-under: still needs to come first. > > I don't think we have an explicit policy for how to deal with > multi-contributor patches. The ones we do see tend to just keep a > single commit message and list the contributors. > > In Linux. it would be something like > Signed-off-by: Marcin Wojtas <mw@semihalf.com> > [Introduce protocol GUID to force correct driver dispatch order] > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > Signed-off-by: Marcin Wojtas <mw@semihalf.com> > > I would be quite happy to use the same format here. > Well, Tianocore still conflates authorship with a statement regarding the origin of the contribution. I wonder how this is supposed to work when Linaro engineers such as myself contribute code that was authored by engineers working in member companies, e.g., Socionext. The license and the contract that company has with Linaro give me the right to contribute that code, but that does not make me the author, and I cannot add a Signed-off-by that wasn't present when we received the code (even if I knew the name of the author) _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
2017-10-10 22:36 GMT+02:00 Ard Biesheuvel <ard.biesheuvel@linaro.org>: > On 10 October 2017 at 16:26, Leif Lindholm <leif.lindholm@linaro.org> wrote: >> On Tue, Oct 10, 2017 at 05:06:42PM +0200, Marcin Wojtas wrote: >>> 2017-10-10 17:03 GMT+02:00 Leif Lindholm <leif.lindholm@linaro.org>: >>> > On Tue, Oct 10, 2017 at 04:45:10PM +0200, Marcin Wojtas wrote: >>> >> Hi Leif, >>> >> >>> >> 2017-10-10 16:37 GMT+02:00 Leif Lindholm <leif.lindholm@linaro.org>: >>> >> > On Mon, Oct 09, 2017 at 07:00:50PM +0200, Marcin Wojtas wrote: >>> >> >> In order to enable modification of dynamic PCD's for the libraries >>> >> >> and DXE drivers, this patch introduces new driver. It is >>> >> >> executed prior to other drivers. Mpp, ComPhy and Utmi libraries >>> >> >> initialization were moved from PrePi stage to DXE. >>> >> >> >>> >> >> To force the correct driver dispatch sequence, introduce a protocol GUID >>> >> >> and install the protocol as a NULL protocol when PlatInitDxe executes. >>> >> >> >>> >> >> Contributed-under: TianoCore Contribution Agreement 1.1 >>> >> >> Signed-off-by: Marcin Wojtas <mw@semihalf.com> >>> >> >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> >>> >> > >>> >> > What does Ard's Signed-off-by signify here? >>> >> > (I know the authorship on some of these is a bit blurred, since you've >>> >> > been working together, but I'd like to be clear.) >>> >> >>> >> These were the lines, introducing/installing protocol GUID stuff. It >>> >> was in a small separate patch, but I squashed it into bigger one. >>> > >>> > Personally, I would in this instance do: >>> > <me> >>> > Ard >>> > <me> >>> > >>> > It's verbose, but reasonably clear. >>> > >>> >>> How about: >>> >>> In order to enable modification of dynamic PCD's for the libraries >>> and DXE drivers, this patch introduces new driver. It is >>> executed prior to other drivers. Mpp, ComPhy and Utmi libraries >>> initialization were moved from PrePi stage to DXE. >>> >>> Signed-off-by: Marcin Wojtas <mw@semihalf.com> >>> >>> To force the correct driver dispatch sequence, introduce a protocol GUID >>> and install the protocol as a NULL protocol when PlatInitDxe executes. >>> >>> Contributed-under: TianoCore Contribution Agreement 1.1 >>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> >>> Signed-off-by: Marcin Wojtas <mw@semihalf.com> >>> >>> ? >>> >>> Was that, what you meant? >> >> I think Contibuted-under: still needs to come first. >> >> I don't think we have an explicit policy for how to deal with >> multi-contributor patches. The ones we do see tend to just keep a >> single commit message and list the contributors. >> >> In Linux. it would be something like >> Signed-off-by: Marcin Wojtas <mw@semihalf.com> >> [Introduce protocol GUID to force correct driver dispatch order] >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> >> Signed-off-by: Marcin Wojtas <mw@semihalf.com> >> >> I would be quite happy to use the same format here. >> > > Well, Tianocore still conflates authorship with a statement regarding > the origin of the contribution. I wonder how this is supposed to work > when Linaro engineers such as myself contribute code that was authored > by engineers working in member companies, e.g., Socionext. The license > and the contract that company has with Linaro give me the right to > contribute that code, but that does not make me the author, and I > cannot add a Signed-off-by that wasn't present when we received the > code (even if I knew the name of the author) I think it's fairly easy thing, needlessly twisted... How does above reflect the requirement to add contributor sign-off to someone else's patch (with his authorship and original sign-off - should they be removed?)? Anyway, let's make a quick decision here - should I submit patch with linux-like signatures and description? Or should I split the patches? Best regards, Marcin _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
On Wed, Oct 11, 2017 at 06:53:05AM +0200, Marcin Wojtas wrote:
> >> I think Contibuted-under: still needs to come first.
> >>
> >> I don't think we have an explicit policy for how to deal with
> >> multi-contributor patches. The ones we do see tend to just keep a
> >> single commit message and list the contributors.
> >>
> >> In Linux. it would be something like
> >> Signed-off-by: Marcin Wojtas <mw@semihalf.com>
> >> [Introduce protocol GUID to force correct driver dispatch order]
> >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> >> Signed-off-by: Marcin Wojtas <mw@semihalf.com>
> >>
> >> I would be quite happy to use the same format here.
> >>
> >
> > Well, Tianocore still conflates authorship with a statement regarding
> > the origin of the contribution. I wonder how this is supposed to work
> > when Linaro engineers such as myself contribute code that was authored
> > by engineers working in member companies, e.g., Socionext. The license
> > and the contract that company has with Linaro give me the right to
> > contribute that code, but that does not make me the author, and I
> > cannot add a Signed-off-by that wasn't present when we received the
> > code (even if I knew the name of the author)
>
> I think it's fairly easy thing, needlessly twisted... How does above
> reflect the requirement to add contributor sign-off to someone else's
> patch (with his authorship and original sign-off - should they be
> removed?)?
Well, we're not debating this because it's critical for this one
patch, but because it would be useful to have a precedent.
> Anyway, let's make a quick decision here - should I submit patch with
> linux-like signatures and description? Or should I split the patches?
Let's put it this way - if you split the patches, you remove this
series from abovementioned discussion :)
/
Leif
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Leif, 2017-10-11 10:32 GMT+02:00 Leif Lindholm <leif.lindholm@linaro.org>: > On Wed, Oct 11, 2017 at 06:53:05AM +0200, Marcin Wojtas wrote: >> >> I think Contibuted-under: still needs to come first. >> >> >> >> I don't think we have an explicit policy for how to deal with >> >> multi-contributor patches. The ones we do see tend to just keep a >> >> single commit message and list the contributors. >> >> >> >> In Linux. it would be something like >> >> Signed-off-by: Marcin Wojtas <mw@semihalf.com> >> >> [Introduce protocol GUID to force correct driver dispatch order] >> >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> >> >> Signed-off-by: Marcin Wojtas <mw@semihalf.com> >> >> >> >> I would be quite happy to use the same format here. >> >> >> > >> > Well, Tianocore still conflates authorship with a statement regarding >> > the origin of the contribution. I wonder how this is supposed to work >> > when Linaro engineers such as myself contribute code that was authored >> > by engineers working in member companies, e.g., Socionext. The license >> > and the contract that company has with Linaro give me the right to >> > contribute that code, but that does not make me the author, and I >> > cannot add a Signed-off-by that wasn't present when we received the >> > code (even if I knew the name of the author) >> >> I think it's fairly easy thing, needlessly twisted... How does above >> reflect the requirement to add contributor sign-off to someone else's >> patch (with his authorship and original sign-off - should they be >> removed?)? > > Well, we're not debating this because it's critical for this one > patch, but because it would be useful to have a precedent. > I'm totally fine with precedences, it's rather your call, whether it's accepted or not :) My three arugments are: - I have still a lot patches ahead and it's very likely such situation may occur again. - Needless to say, it may happen again in the development of other platforms. - Artificially splitting patches seems to me as not really needed and I'm not convinced to its justification. >> Anyway, let's make a quick decision here - should I submit patch with >> linux-like signatures and description? Or should I split the patches? > > Let's put it this way - if you split the patches, you remove this > series from abovementioned discussion :) > If you're ok with it, I'd go with single patch, but I can do it either way - I think I'm not to decide, what's best from maintainers' point of view :) Best regards, Marcin _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
On Wed, Oct 11, 2017 at 10:43:14AM +0200, Marcin Wojtas wrote:
> >> I think it's fairly easy thing, needlessly twisted... How does above
> >> reflect the requirement to add contributor sign-off to someone else's
> >> patch (with his authorship and original sign-off - should they be
> >> removed?)?
> >
> > Well, we're not debating this because it's critical for this one
> > patch, but because it would be useful to have a precedent.
>
> I'm totally fine with precedences, it's rather your call, whether it's
> accepted or not :) My three arugments are:
> - I have still a lot patches ahead and it's very likely such situation
> may occur again.
> - Needless to say, it may happen again in the development of other platforms.
> - Artificially splitting patches seems to me as not really needed and
> I'm not convinced to its justification.
>
> >> Anyway, let's make a quick decision here - should I submit patch with
> >> linux-like signatures and description? Or should I split the patches?
> >
> > Let's put it this way - if you split the patches, you remove this
> > series from abovementioned discussion :)
>
> If you're ok with it, I'd go with single patch, but I can do it either
> way - I think I'm not to decide, what's best from maintainers' point
> of view :)
For now, I would take the single patch with Linux-style description,
like the example I sent earlier.
/
Leif
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
2017-10-11 11:14 GMT+02:00 Leif Lindholm <leif.lindholm@linaro.org>: > On Wed, Oct 11, 2017 at 10:43:14AM +0200, Marcin Wojtas wrote: >> >> I think it's fairly easy thing, needlessly twisted... How does above >> >> reflect the requirement to add contributor sign-off to someone else's >> >> patch (with his authorship and original sign-off - should they be >> >> removed?)? >> > >> > Well, we're not debating this because it's critical for this one >> > patch, but because it would be useful to have a precedent. >> >> I'm totally fine with precedences, it's rather your call, whether it's >> accepted or not :) My three arugments are: >> - I have still a lot patches ahead and it's very likely such situation >> may occur again. >> - Needless to say, it may happen again in the development of other platforms. >> - Artificially splitting patches seems to me as not really needed and >> I'm not convinced to its justification. >> >> >> Anyway, let's make a quick decision here - should I submit patch with >> >> linux-like signatures and description? Or should I split the patches? >> > >> > Let's put it this way - if you split the patches, you remove this >> > series from abovementioned discussion :) >> >> If you're ok with it, I'd go with single patch, but I can do it either >> way - I think I'm not to decide, what's best from maintainers' point >> of view :) > > For now, I would take the single patch with Linux-style description, > like the example I sent earlier. > Great, will send it in v2. Thanks, Marcin _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
© 2016 - 2026 Red Hat, Inc.