Re: [PATCH 1/5] MdePkg/SPI: Change function definitions to match their descriptions.

["Bi, Dandan" <dandan.bi@intel.com>]

Hi Marvin,

Thank you very much for your contribution. Could you hold this patch series?  Since current SPI header files follow PI1.6 spec.
For this case, we should submit an ECR to update the PI spec firstly. After the ECR has been approved by PIWG, then we can send patch to update them. 
Since you have found a lot of missing/incorrect parts in the SPI part of PI Spec. Could you help to submit an ECR to update them?


Thanks,
Dandan

-----Original Message-----
From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Marvin Häuser
Sent: Wednesday, February 28, 2018 12:49 AM
To: edk2-devel@lists.01.org
Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming <liming.gao@intel.com>
Subject: [edk2] [PATCH 1/5] MdePkg/SPI: Change function definitions to match their descriptions.

The PI specification is not continuous with function headers and descriptions for the SPI protocols. Correct and comment these mistakes.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Marvin Haeuser <Marvin.Haeuser@outlook.com>
---
 MdePkg/Include/Protocol/SpiConfiguration.h | 12 ++++++++----
 MdePkg/Include/Protocol/SpiHc.h            | 14 +++++++++-----
 MdePkg/Include/Protocol/SpiIo.h            | 15 ++++++++++-----
 3 files changed, 27 insertions(+), 14 deletions(-)

diff --git a/MdePkg/Include/Protocol/SpiConfiguration.h b/MdePkg/Include/Protocol/SpiConfiguration.h
index c0df183ec7f0..c36a809f4232 100644
--- a/MdePkg/Include/Protocol/SpiConfiguration.h
+++ b/MdePkg/Include/Protocol/SpiConfiguration.h
@@ -1,7 +1,7 @@
 /** @file
   This file defines the SPI Configuration Protocol.
 
-  Copyright (c) 2017, Intel Corporation. All rights reserved.<BR>
+  Copyright (c) 2017 - 2018, Intel Corporation. All rights 
+ 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 @@ -65,6 +65,10 @@ EFI_STATUS
   IN BOOLEAN                   PinValue
   );
 
+//
+// Note: In the PI specification, ClockHz is decorated as only 'IN', which is
+//       not conforming to the parameter description.
+//
 /**
   Set up the clock generator to produce the correct clock frequency, phase and
   polarity for a SPI chip.
@@ -78,7 +82,7 @@ EFI_STATUS
                             ClockPhase and ClockPolarity fields. The routine
                             also has access to the names for the SPI bus and
                             chip which can be used during debugging.
-  @param[in] ClockHz        Pointer to the requested clock frequency. The clock
+  @param[in,out] ClockHz    Pointer to the requested clock frequency. The clock
                             generator will choose a supported clock frequency
                             which is less then or equal to this value.
                             Specify zero to turn the clock generator off.
@@ -92,8 +96,8 @@ EFI_STATUS
 **/
 typedef EFI_STATUS
 (EFIAPI *EFI_SPI_CLOCK) (
-  IN CONST EFI_SPI_PERIPHERAL  *SpiPeripheral,
-  IN UINT32                    *ClockHz
+  IN     CONST EFI_SPI_PERIPHERAL  *SpiPeripheral,
+  IN OUT UINT32                    *ClockHz
   );
 
 ///
diff --git a/MdePkg/Include/Protocol/SpiHc.h b/MdePkg/Include/Protocol/SpiHc.h index 12fe5d2dce0a..71c75431e4e8 100644
--- a/MdePkg/Include/Protocol/SpiHc.h
+++ b/MdePkg/Include/Protocol/SpiHc.h
@@ -1,7 +1,7 @@
 /** @file
   This file defines the SPI Host Controller Protocol.
 
-  Copyright (c) 2017, Intel Corporation. All rights reserved.<BR>
+  Copyright (c) 2017 - 2018, Intel Corporation. All rights 
+ 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 @@ -66,6 +66,10 @@ typedef EFI_STATUS
   IN BOOLEAN                    PinValue
   );
 
+//
+// Note: In the PI specification, ClockHz is decorated as only 'IN', which is
+//       not conforming to the parameter description.
+//
 /**
   Set up the clock generator to produce the correct clock frequency, phase and
   polarity for a SPI chip.
@@ -80,7 +84,7 @@ typedef EFI_STATUS
                             ClockPhase and ClockPolarity fields. The routine
                             also has access to the names for the SPI bus and
                             chip which can be used during debugging.
-  @param[in] ClockHz        Pointer to the requested clock frequency. The SPI
+  @param[in,out] ClockHz    Pointer to the requested clock frequency. The SPI
                             host controller will choose a supported clock
                             frequency which is less then or equal to this
                             value. Specify zero to turn the clock generator @@ -94,9 +98,9 @@ typedef EFI_STATUS  **/  typedef EFI_STATUS  (EFIAPI *EFI_SPI_HC_PROTOCOL_CLOCK) (
-  IN CONST EFI_SPI_HC_PROTOCOL  *This,
-  IN CONST EFI_SPI_PERIPHERAL   *SpiPeripheral,
-  IN UINT32                      *ClockHz
+  IN     CONST EFI_SPI_HC_PROTOCOL  *This,
+  IN     CONST EFI_SPI_PERIPHERAL   *SpiPeripheral,
+  IN OUT UINT32                     *ClockHz
   );
 
 /**
diff --git a/MdePkg/Include/Protocol/SpiIo.h b/MdePkg/Include/Protocol/SpiIo.h index 43e804518f8b..8c5d96bb04b2 100644
--- a/MdePkg/Include/Protocol/SpiIo.h
+++ b/MdePkg/Include/Protocol/SpiIo.h
@@ -1,7 +1,7 @@
 /** @file
   This file defines the SPI I/O Protocol.
 
-  Copyright (c) 2017, Intel Corporation. All rights reserved.<BR>
+  Copyright (c) 2017 - 2018, Intel Corporation. All rights 
+ 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 @@ -144,6 +144,11 @@ EFI_STATUS
   OUT UINT8                      *ReadBuffer
   );
 
+//
+// Note: In the PI specification, 'This' is decorated with 'IN' and 'CONST'.
+//       However, 'This' needs to be updated in order to reflect the
+//       Peripheral update.
+//
 /**
   Update the SPI peripheral associated with this SPI 10 instance.
 
@@ -152,8 +157,8 @@ EFI_STATUS
   SPI NOR flash parts, where the size and parameters change depending upon
   device is in the socket.
 
-  @param[in] This           Pointer to an EFI_SPI_IO_PROTOCOL structure.
-  @param[in] SpiPeripheral  Pointer to an EFI_SPI_PERIPHERAL structure.
+  @param[in,out] This           Pointer to an EFI_SPI_IO_PROTOCOL structure.
+  @param[in]     SpiPeripheral  Pointer to an EFI_SPI_PERIPHERAL structure.
 
   @retval EFI_SUCCESS            The SPI peripheral was updated successfully
   @retval EFI_INVALID_PARAMETER  The SpiPeripheral value is NULL, @@ -165,8 +170,8 @@ EFI_STATUS  **/  typedef EFI_STATUS  (EFIAPI *EFI_SPI_IO_PROTOCOL_UPDATE_SPI_PERIPHERAL) (
-  IN CONST EFI_SPI_IO_PROTOCOL  *This,
-  IN CONST EFI_SPI_PERIPHERAL   *SpiPeripheral
+  IN OUT EFI_SPI_IO_PROTOCOL       *This,
+  IN     CONST EFI_SPI_PERIPHERAL  *SpiPeripheral
   );
 
 ///
--
2.16.0.windows.2

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel