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?
> +
> + 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 (#39942): https://edk2.groups.io/g/devel/message/39942
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]
-=-=-=-=-=-=-=-=-=-=-=-
