Re: [edk2-devel] [PATCH 02/17] OvmfPkg/PvScsiDxe: Install DriverBinding protocol

["Laszlo Ersek" <lersek@redhat.com>]

On 03/16/20 16:00, Liran Alon wrote:
> In order to probe and connect to the PvScsi device we need this
> protocol. Currently it does nothing.
> 
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2567
> Reviewed-by: Nikita Leshenko <nikita.leshchenko@oracle.com>
> Signed-off-by: Liran Alon <liran.alon@oracle.com>
> ---
>  OvmfPkg/PvScsiDxe/PvScsi.c   | 68 +++++++++++++++++++++++++++++++++++-
>  OvmfPkg/PvScsiDxe/PvScsi.inf |  1 +
>  2 files changed, 68 insertions(+), 1 deletion(-)
> 
> diff --git a/OvmfPkg/PvScsiDxe/PvScsi.c b/OvmfPkg/PvScsiDxe/PvScsi.c
> index a3f704d60d77..bf0c743bad15 100644
> --- a/OvmfPkg/PvScsiDxe/PvScsi.c
> +++ b/OvmfPkg/PvScsiDxe/PvScsi.c
> @@ -9,6 +9,65 @@
>  
>  **/
>  
> +#include <Library/UefiLib.h>
> +
> +//
> +// Higher versions will be used before lower, 0x10-0xffffffef is the version
> +// range for IVH (Indie Hardware Vendors)

(1) typo: s/IVH/IHV/

> +//
> +#define PVSCSI_BINDING_VERSION      0x10
> +
> +//
> +// Driver Binding
> +//
> +
> +STATIC
> +EFI_STATUS
> +EFIAPI
> +PvScsiDriverBindingSupported (
> +  IN EFI_DRIVER_BINDING_PROTOCOL *This,
> +  IN EFI_HANDLE                  ControllerHandle,
> +  IN EFI_DEVICE_PATH_PROTOCOL    *RemainingDevicePath OPTIONAL
> +  )
> +{
> +  return EFI_UNSUPPORTED;
> +}
> +
> +STATIC
> +EFI_STATUS
> +EFIAPI
> +PvScsiDriverBindingStart (
> +  IN EFI_DRIVER_BINDING_PROTOCOL *This,
> +  IN EFI_HANDLE                  ControllerHandle,
> +  IN EFI_DEVICE_PATH_PROTOCOL    *RemainingDevicePath OPTIONAL
> +  )
> +{
> +  return EFI_UNSUPPORTED;
> +}
> +
> +STATIC
> +EFI_STATUS
> +EFIAPI
> +PvScsiDriverBindingStop (
> +  IN EFI_DRIVER_BINDING_PROTOCOL *This,
> +  IN EFI_HANDLE                  ControllerHandle,
> +  IN UINTN                       NumberOfChildren,
> +  IN EFI_HANDLE                  *ChildHandleBuffer
> +  )
> +{
> +  return EFI_UNSUPPORTED;
> +}
> +
> +STATIC
> +EFI_DRIVER_BINDING_PROTOCOL mPvScsiDriverBinding = {

(2) For object declarations, we generally keep STATIC on the same line
as the type name.

> +  &PvScsiDriverBindingSupported,
> +  &PvScsiDriverBindingStart,
> +  &PvScsiDriverBindingStop,
> +  PVSCSI_BINDING_VERSION,
> +  NULL, // ImageHandle, filled by EfiLibInstallDriverBindingComponentName2()
> +  NULL  // DriverBindingHandle, filled as well
> +};
> +
>  //
>  // Entry Point
>  //
> @@ -20,5 +79,12 @@ PvScsiEntryPoint (
>    IN EFI_SYSTEM_TABLE *SystemTable
>    )
>  {
> -  return EFI_UNSUPPORTED;
> +  return EfiLibInstallDriverBindingComponentName2 (
> +           ImageHandle,
> +           SystemTable,
> +           &mPvScsiDriverBinding,
> +           ImageHandle,
> +           NULL, // TODO Component name
> +           NULL  // TODO Component name
> +           );
>  }
> diff --git a/OvmfPkg/PvScsiDxe/PvScsi.inf b/OvmfPkg/PvScsiDxe/PvScsi.inf
> index 093cc0171338..d1d0e963f96d 100644
> --- a/OvmfPkg/PvScsiDxe/PvScsi.inf
> +++ b/OvmfPkg/PvScsiDxe/PvScsi.inf
> @@ -25,3 +25,4 @@
>  
>  [LibraryClasses]
>    UefiDriverEntryPoint
> +  UefiLib
> 

With (1) and (2) addressed:

Reviewed-by: Laszlo Ersek <lersek@redhat.com>

(Please strip Nikita's R-b in v2, or please ask Nikita to post the R-b
in response to this v1 patch.)

Thanks,
Laszlo