Hi Ard,
pt., 3 maj 2019 o 08:33 Ard Biesheuvel <ard.biesheu...@linaro.org> napisał(a):
>
> On Fri, 3 May 2019 at 01:50, Marcin Wojtas <m...@semihalf.com> wrote:
> >
> > From: Tomasz Michalec <t...@semihalf.com>
> >
> > Add implementation of Adapter Information Protocol in Armada 7k8k
> > PP2 NIC driver. Support retrieving information about media state
> > which allows to use NetLibDetectMediaWaitTimeout on network interface.
> > This patch fixes possible hangs when attempting to PXE boot on
> > unconnected interfaces.
> >
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Tomasz Michalec <t...@semihalf.com>
>
> Please put your own signoff here. You can retain the one from Tomasz
> if you like.
>
> > ---
> > Silicon/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.inf | 1 +
> > Silicon/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.h | 3 +
> > Silicon/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.c | 157 ++++++++++++++++++++
> > 3 files changed, 161 insertions(+)
> >
> > diff --git a/Silicon/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.inf
> > b/Silicon/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.inf
> > index be536ab..73aa413 100644
> > --- a/Silicon/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.inf
> > +++ b/Silicon/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.inf
> > @@ -64,6 +64,7 @@
> > CacheMaintenanceLib
> >
> > [Protocols]
> > + gEfiAdapterInformationProtocolGuid
> > gEfiSimpleNetworkProtocolGuid
> > gEfiDevicePathProtocolGuid
> > gEfiCpuArchProtocolGuid
> > diff --git a/Silicon/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.h
> > b/Silicon/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.h
> > index b8a5dae..66bd46a 100644
> > --- a/Silicon/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.h
> > +++ b/Silicon/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.h
> > @@ -35,6 +35,7 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH
> > DAMAGE.
> > #ifndef __PP2_DXE_H__
> > #define __PP2_DXE_H__
> >
> > +#include <Protocol/AdapterInformation.h>
> > #include <Protocol/Cpu.h>
> > #include <Protocol/DevicePath.h>
> > #include <Protocol/DriverBinding.h>
> > @@ -59,6 +60,7 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH
> > DAMAGE.
> > #define MVPP2_MAX_PORT 3
> >
> > #define PP2DXE_SIGNATURE SIGNATURE_32('P', 'P', '2',
> > 'D')
> > +#define INSTANCE_FROM_AIP(a) CR((a), PP2DXE_CONTEXT, Aip,
> > PP2DXE_SIGNATURE)
> > #define INSTANCE_FROM_SNP(a) CR((a), PP2DXE_CONTEXT, Snp,
> > PP2DXE_SIGNATURE)
> >
> > /* OS API */
> > @@ -365,6 +367,7 @@ typedef struct {
> > UINTN CompletionQueueTail;
> > EFI_EVENT EfiExitBootServicesEvent;
> > PP2_DEVICE_PATH *DevicePath;
> > + EFI_ADAPTER_INFORMATION_PROTOCOL Aip;
> > } PP2DXE_CONTEXT;
> >
> > /* Inline helpers */
> > diff --git a/Silicon/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.c
> > b/Silicon/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.c
> > index 02b2798..473a2a1 100644
> > --- a/Silicon/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.c
> > +++ b/Silicon/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.c
> > @@ -1129,6 +1129,7 @@ Pp2DxeSnpInstall (
> > &Handle,
> > &gEfiSimpleNetworkProtocolGuid, &Pp2Context->Snp,
> > &gEfiDevicePathProtocolGuid, Pp2DevicePath,
> > + &gEfiAdapterInformationProtocolGuid, &Pp2Context->Aip,
> > NULL
> > );
> >
> > @@ -1139,6 +1140,157 @@ Pp2DxeSnpInstall (
> > return Status;
> > }
> >
> > +/**
> > + Returns the current state information for the adapter.
> > +
> > + This function returns information of type InformationType from the
> > adapter.
> > + If an adapter does not support the requested informational type, then
> > + EFI_UNSUPPORTED is returned.
> > +
> > + @param[in] This A pointer to the
> > EFI_ADAPTER_INFORMATION_PROTOCOL instance.
> > + @param[in] InformationType A pointer to an EFI_GUID that defines
> > the contents of InformationBlock.
> > + @param[out] InforamtionBlock The service returns a pointer to the
> > buffer with the InformationBlock
> > + structure which contains details
> > about the data specific to InformationType.
> > + @param[out] InforamtionBlockSize The driver returns the size of the
> > InformationBlock in bytes.
>
> Please fix these typos. I know they were copy/pasted from the protocol
> definition, but I'd still prefer them to be fixed.
>
> > +
> > + @retval EFI_SUCCESS The InformationType information was
> > retrieved.
> > + @retval EFI_UNSUPPORTED The InformationType is not known.
> > + @retval EFI_DEVICE_ERROR The device reported an error.
> > + @retval EFI_OUT_OF_RESOURCES The request could not be completed
> > due to a lack of resources.
> > + @retval EFI_INVALID_PARAMETER This is NULL.
> > + @retval EFI_INVALID_PARAMETER InformationBlock is NULL.
> > + @retval EFI_INVALID_PARAMETER InformationBlockSize is NULL.
> > +
> > +**/
>
> STATIC?
>
> > +EFI_STATUS
> > +EFIAPI
> > +Pp2AipGetInformation (
> > + IN EFI_ADAPTER_INFORMATION_PROTOCOL *This,
> > + IN EFI_GUID *InformationType,
> > + OUT VOID **InformationBlock,
> > + OUT UINTN *InformationBlockSize
> > + )
> > +{
> > + EFI_ADAPTER_INFO_MEDIA_STATE *AdapterInfo;
> > + PP2DXE_CONTEXT *Pp2Context;
> > + EFI_STATUS Status;
> > +
> > + if (This == NULL || InformationBlock == NULL || InformationBlockSize ==
> > NULL) {
> > + return EFI_INVALID_PARAMETER;
> > + }
> > +
> > + if (!CompareGuid (InformationType, &gEfiAdapterInfoMediaStateGuid)) {
> > + return EFI_UNSUPPORTED;
> > + }
> > +
> > + AdapterInfo = AllocateZeroPool (sizeof (EFI_ADAPTER_INFO_MEDIA_STATE));
> > + if (AdapterInfo == NULL) {
> > + return EFI_OUT_OF_RESOURCES;
> > + }
> > +
> > + *InformationBlock = AdapterInfo;
> > + *InformationBlockSize = sizeof (EFI_ADAPTER_INFO_MEDIA_STATE);
> > +
> > + Pp2Context = INSTANCE_FROM_AIP (This);
> > +
> > + Status = Pp2Context->Snp.GetStatus (&(Pp2Context->Snp), NULL, NULL);
> > + if (Status == EFI_NOT_READY){
> > + AdapterInfo->MediaState = EFI_NOT_READY;
> > + return EFI_SUCCESS;
> > + }
>
> What happens if GetStatus returns something other than EFI_SUCCESS or
> EFI_NOT_READY?
>
In Pp2Dxe those are the only 2 possible return values, but despite
that I'll add handling for:
} else if (EFI_ERROR(Status) {
> > +
> > + AdapterInfo->MediaState = (Pp2Context->Snp.Mode->MediaPresent ?
> > + EFI_SUCCESS : EFI_NOT_READY);
> > +
>
> Please get rid of the ternary expression, just fold the condition into
> the preceding if()
>
>
> > + return EFI_SUCCESS;
> > +}
> > +
> > +/**
> > + Sets state information for an adapter.
> > +
> > + This function sends information of type InformationType for an adapter.
> > + If an adapter does not support the requested information type, then
> > EFI_UNSUPPORTED
> > + is returned.
> > +
> > + @param[in] This A pointer to the
> > EFI_ADAPTER_INFORMATION_PROTOCOL instance.
> > + @param[in] InformationType A pointer to an EFI_GUID that defines
> > the contents of InformationBlock.
> > + @param[in] InforamtionBlock A pointer to the InformationBlock
> > structure which contains details
> > + about the data specific to
> > InformationType.
> > + @param[in] InforamtionBlockSize The size of the InformationBlock in
> > bytes.
> > +
>
> More typos
>
> > + @retval EFI_SUCCESS The information was received and
> > interpreted successfully.
> > + @retval EFI_UNSUPPORTED The InformationType is not known.
> > + @retval EFI_DEVICE_ERROR The device reported an error.
> > + @retval EFI_INVALID_PARAMETER This is NULL.
> > + @retval EFI_INVALID_PARAMETER InformationBlock is NULL.
> > + @retval EFI_WRITE_PROTECTED The InformationType cannot be
> > modified using EFI_ADAPTER_INFO_SET_INFO().
> > +
> > +**/
>
> STATIC?
>
> > +EFI_STATUS
> > +EFIAPI
> > +Pp2AipSetInformation (
> > + IN EFI_ADAPTER_INFORMATION_PROTOCOL *This,
> > + IN EFI_GUID *InformationType,
> > + IN VOID *InformationBlock,
> > + IN UINTN InformationBlockSize
> > + )
> > +{
> > + if (This == NULL || InformationBlock == NULL) {
> > + return EFI_INVALID_PARAMETER;
> > + }
> > +
> > + if (CompareGuid (InformationType, &gEfiAdapterInfoMediaStateGuid)) {
> > + return EFI_WRITE_PROTECTED;
> > + }
> > +
> > + return EFI_UNSUPPORTED;
> > +}
> > +
> > +/**
> > + Get a list of supported information types for this instance of the
> > protocol.
> > +
> > + This function returns a list of InformationType GUIDs that are supported
> > on an
> > + adapter with this instance of EFI_ADAPTER_INFORMATION_PROTOCOL. The list
> > is returned
> > + in InfoTypesBuffer, and the number of GUID pointers in InfoTypesBuffer
> > is returned in
> > + InfoTypesBufferCount.
> > +
> > + @param[in] This A pointer to the
> > EFI_ADAPTER_INFORMATION_PROTOCOL instance.
> > + @param[out] InfoTypesBuffer A pointer to the array of
> > InformationType GUIDs that are supported
> > + by This.
> > + @param[out] InfoTypesBufferCount A pointer to the number of GUIDs
> > present in InfoTypesBuffer.
> > +
> > + @retval EFI_SUCCESS The list of information type GUIDs
> > that are supported on this adapter was
> > + returned in InfoTypesBuffer. The
> > number of information type GUIDs was
> > + returned in InfoTypesBufferCount.
> > + @retval EFI_INVALID_PARAMETER This is NULL.
> > + @retval EFI_INVALID_PARAMETER InfoTypesBuffer is NULL.
> > + @retval EFI_INVALID_PARAMETER InfoTypesBufferCount is NULL.
> > + @retval EFI_OUT_OF_RESOURCES There is not enough pool memory to
> > store the results.
> > +
> > +**/
>
> STATIC
>
> > +EFI_STATUS
> > +EFIAPI
> > +Pp2AipGetSupportedTypes (
> > + IN EFI_ADAPTER_INFORMATION_PROTOCOL *This,
> > + OUT EFI_GUID **InfoTypesBuffer,
> > + OUT UINTN *InfoTypesBufferCount
> > + )
> > +{
> > + if (This == NULL || InfoTypesBuffer == NULL || InfoTypesBufferCount ==
> > NULL) {
> > + return EFI_INVALID_PARAMETER;
> > + }
> > +
> > + *InfoTypesBuffer = AllocateZeroPool (sizeof (EFI_GUID));
>
> No need to zero if you assign the whole thing immediately
>
> > + if (*InfoTypesBuffer == NULL) {
> > + return EFI_OUT_OF_RESOURCES;
> > + }
> > +
> > + *InfoTypesBufferCount = 1;
> > + (*InfoTypesBuffer)[0] = gEfiAdapterInfoMediaStateGuid;
> > +
>
> Use CopyGuid() not struct assignment
>
> > + return EFI_SUCCESS;
> > +}
> > +
> > STATIC
> > VOID
> > Pp2DxeParsePortPcd (
> > @@ -1290,6 +1442,11 @@ Pp2DxeInitialiseController (
> > Pp2Context->Instance = DeviceInstance;
> > DeviceInstance++;
> >
> > + /* Prepare AIP Protocol */
> > + Pp2Context->Aip.GetInformation = Pp2AipGetInformation;
> > + Pp2Context->Aip.SetInformation = Pp2AipSetInformation;
> > + Pp2Context->Aip.GetSupportedTypes = Pp2AipGetSupportedTypes;
> > +
> > /* Install SNP protocol */
> > Status = Pp2DxeSnpInstall(Pp2Context);
> > if (EFI_ERROR(Status)) {
> > --
> > 2.7.4
> >
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#39948): https://edk2.groups.io/g/devel/message/39948
Mute This Topic: https://groups.io/mt/31476852/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
