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

["Michael Kubacki" <mikuback@linux.microsoft.com>]

I updated V3 to repeat BufferSizeInBytes.

Regards,
Michael

On 8/25/2022 6:15 AM, Leif Lindholm wrote:
> 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
>>
>>
>>
>>
>>
>>
> 
> 
> 
>