Re: [edk2-devel] [PATCH v5 03/12] OvmfPkg/MptScsiDxe: Report name of driver

["Carsey, Jaben" <jaben.carsey@intel.com>]

I don't remember reviewing this previously (not recently at least), maybe the RB should be CC?

One comment inline below.

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Nikita
> Leshenko
> Sent: Friday, April 24, 2020 10:59 AM
> To: devel@edk2.groups.io
> Cc: Nikita Leshenko <nikita.leshchenko@oracle.com>;
> liran.alon@oracle.com; aaron.young@oracle.com; Justen, Jordan L
> <jordan.l.justen@intel.com>; Laszlo Ersek <lersek@redhat.com>; Ard
> Biesheuvel <ard.biesheuvel@arm.com>; Carsey, Jaben
> <jaben.carsey@intel.com>
> Subject: [edk2-devel] [PATCH v5 03/12] OvmfPkg/MptScsiDxe: Report name
> of driver
> 
> Install Component Name protocols to have a nice display name for the driver
> in places such as UEFI shell.
> 
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2390
> Signed-off-by: Nikita Leshenko <nikita.leshchenko@oracle.com>
> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
> Reviewed-by: Jaben Carsey <jaben.carsey@intel.com>
> Reviewed-by: Liran Alon <liran.alon@oracle.com>
> ---
>  OvmfPkg/MptScsiDxe/MptScsi.c | 61
> ++++++++++++++++++++++++++++++++++--
>  1 file changed, 59 insertions(+), 2 deletions(-)
> 
> diff --git a/OvmfPkg/MptScsiDxe/MptScsi.c b/OvmfPkg/MptScsiDxe/MptScsi.c
> index 581d3909b84d..64949a809022 100644
> --- a/OvmfPkg/MptScsiDxe/MptScsi.c
> +++ b/OvmfPkg/MptScsiDxe/MptScsi.c
> @@ -69,6 +69,63 @@ EFI_DRIVER_BINDING_PROTOCOL
> mMptScsiDriverBinding = {
>    NULL, // DriverBindingHandle, filled as well  };
> 
> +//
> +// Component Name
> +//
> +
> +STATIC
> +EFI_UNICODE_STRING_TABLE mDriverNameTable[] = {
> +  { "eng;en", L"LSI Fusion MPT SCSI Driver" },
> +  { NULL,     NULL                   }
> +};
> +
> +STATIC
> +EFI_COMPONENT_NAME_PROTOCOL mComponentName;
> +
> +EFI_STATUS
> +EFIAPI
> +MptScsiGetDriverName (
> +  IN  EFI_COMPONENT_NAME_PROTOCOL *This,
> +  IN  CHAR8                       *Language,
> +  OUT CHAR16                      **DriverName
> +  )
> +{
> +  return LookupUnicodeString2 (
> +           Language,
> +           This->SupportedLanguages,
> +           mDriverNameTable,
> +           DriverName,
> +           (BOOLEAN)(This == &mComponentName) // Iso639Language
> +           );
> +}
> +
> +EFI_STATUS
> +EFIAPI
> +MptScsiGetDeviceName (
> +  IN  EFI_COMPONENT_NAME_PROTOCOL *This,
> +  IN  EFI_HANDLE                  DeviceHandle,
> +  IN  EFI_HANDLE                  ChildHandle,
> +  IN  CHAR8                       *Language,
> +  OUT CHAR16                      **ControllerName
> +  )
> +{
> +  return EFI_UNSUPPORTED;
> +}
> +
> +STATIC
> +EFI_COMPONENT_NAME_PROTOCOL mComponentName = {
> +  &MptScsiGetDriverName,
> +  &MptScsiGetDeviceName,
> +  "eng" // SupportedLanguages, ISO 639-2 language codes };
> +
> +STATIC
> +EFI_COMPONENT_NAME2_PROTOCOL mComponentName2 = {
> +  (EFI_COMPONENT_NAME2_GET_DRIVER_NAME)
> &MptScsiGetDriverName,
> +  (EFI_COMPONENT_NAME2_GET_CONTROLLER_NAME)
> &MptScsiGetDeviceName,
> +  "en" // SupportedLanguages, RFC 4646 language codes };

I think that the }; needs to be outside of the comment for both of these structures.

> +
>  //
>  // Entry Point
>  //
> @@ -85,7 +142,7 @@ MptScsiEntryPoint (
>             SystemTable,
>             &mMptScsiDriverBinding,
>             ImageHandle, // The handle to install onto
> -           NULL, // TODO Component name
> -           NULL // TODO Component name
> +           &mComponentName,
> +           &mComponentName2
>             );
>  }
> --
> 2.20.1
> 
> 
>