[edk2-devel] [edk2-platforms: PATCH 1/4] Marvel/Drivers: Pp2Dxe: Basic support for Adapter Information Protocol
Marcin Wojtas
mw at semihalf.com
Fri May 3 07:49:30 UTC 2019
Hi Ard,
pt., 3 maj 2019 o 08:33 Ard Biesheuvel <ard.biesheuvel at linaro.org> napisał(a):
>
> On Fri, 3 May 2019 at 01:50, Marcin Wojtas <mw at semihalf.com> wrote:
> >
> > From: Tomasz Michalec <tm at 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 <tm at 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/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-
More information about the edk2-devel-archive
mailing list