Re: [edk2-devel] [PATCH v2 1/8] ArmPlatformPkg/NorFlashDxe: Remove unused debug print specifier

["Leif Lindholm" <quic_llindhol@quicinc.com>]

On Wed, Aug 24, 2022 at 23:48:17 -0400, Michael Kubacki wrote:
> From: Michael Kubacki <michael.kubacki@microsoft.com>
> 
> These debug messages are repeated in both NorFlashBlockIoReadBlocks()
> and NorFlashBlockIoWriteBlocks():
> 
>   "NorFlashBlockIoWriteBlocks(MediaId=0x%x, Lba=%ld, BufferSize=0x%x"
>   "bytes (%d kB), BufferPtr @ 0x%08x)\n"
> 
> Although this requires 5 arguments, only 4 are provided. The kilobyte
> value was never given.
> 
> This change removes that specifier so the 4 arguments match the 4
> specifiers in the debug macro.
>
> Cc: Leif Lindholm <quic_llindhol@quicinc.com>
> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
> Reviewed-by: Ard Biesheuvel <ardb@kernel.org>
> ---
>  ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashBlockIoDxe.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashBlockIoDxe.c b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashBlockIoDxe.c
> index 5afab0a79fa2..e671108e2bcf 100644
> --- a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashBlockIoDxe.c
> +++ b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashBlockIoDxe.c
> @@ -54,7 +54,7 @@ NorFlashBlockIoReadBlocks (
>    Instance = INSTANCE_FROM_BLKIO_THIS (This);
>    Media    = This->Media;
>  
> -  DEBUG ((DEBUG_BLKIO, "NorFlashBlockIoReadBlocks(MediaId=0x%x, Lba=%ld, BufferSize=0x%x bytes (%d kB), BufferPtr @ 0x%08x)\n", MediaId, Lba, BufferSizeInBytes, Buffer));
> +  DEBUG ((DEBUG_BLKIO, "NorFlashBlockIoReadBlocks(MediaId=0x%x, Lba=%ld, BufferSize=0x%x bytes, BufferPtr @ 0x%08x)\n", MediaId, Lba, BufferSizeInBytes, Buffer));
>

Line last touched 2013-01-25, which was a line ending fix on top of
the creation of ArmPlatformPkg on 2011-02-01. I mean on the positive
side, no one would appear to have needed to debug BLKIO since then.

I'm not going to bikeshed, but I'm going to mention the bikeshedding
point that the original mistake made was in not repeating
BufferSizeInBytes in the argument list.

Make of this what you will - either way
Reviewed-by: Leif Lindholm <quic_llindhol@quicinc.com>

>    if (!Media) {
>      Status = EFI_INVALID_PARAMETER;
> @@ -89,7 +89,7 @@ NorFlashBlockIoWriteBlocks (
>  
>    Instance = INSTANCE_FROM_BLKIO_THIS (This);
>  
> -  DEBUG ((DEBUG_BLKIO, "NorFlashBlockIoWriteBlocks(MediaId=0x%x, Lba=%ld, BufferSize=0x%x bytes (%d kB), BufferPtr @ 0x%08x)\n", MediaId, Lba, BufferSizeInBytes, Buffer));
> +  DEBUG ((DEBUG_BLKIO, "NorFlashBlockIoWriteBlocks(MediaId=0x%x, Lba=%ld, BufferSize=0x%x bytes, BufferPtr @ 0x%08x)\n", MediaId, Lba, BufferSizeInBytes, Buffer));
>  
>    if ( !This->Media->MediaPresent ) {
>      Status = EFI_NO_MEDIA;
> -- 
> 2.28.0.windows.1
> 
> 
> 
> 
> 
>