Hey Dandan, Thanks for your reply. I issued updates for the current headers because I saw activity in edk2-platforms to implement these incomplete protocols and because I haven't heard back for discussed changes in the specification for some time. Sure I could help submitting the ECR, however I'm not sure how. I'm not an UEFI Contributor and neither did I find any design flaws, just typos and missing definitions, which obviously must be bit values due to their non-exclusive nature. If you need me to do something concrete, I will try to do my best.
Regards, Marvin. > -----Original Message----- > From: Bi, Dandan <dandan...@intel.com> > Sent: Monday, March 5, 2018 3:15 AM > To: Marvin Häuser <marvin.haeu...@outlook.com>; edk2- > de...@lists.01.org > Cc: Kinney, Michael D <michael.d.kin...@intel.com>; Gao, Liming > <liming....@intel.com>; Bi, Dandan <dandan...@intel.com> > Subject: RE: [PATCH 1/5] MdePkg/SPI: Change function definitions to match > their descriptions. > > 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-boun...@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.kin...@intel.com>; Gao, Liming > <liming....@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.haeu...@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 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
