[PATCH] MdeModulePkg/EmmcDxe: demote DEBUG print to DEBUG_BLKIO

[PATCH] MdeModulePkg/EmmcDxe: demote DEBUG print to DEBUG_BLKIO

[Ard Biesheuvel]

Lower the priority of the DEBUG print in EmmcReadWrite(), which
is emitted for each read or write operation to the eMMC device,
which clutters up the log output of builds created with DEBUG_INFO
enabled.

Suggested-by: Pipat Methavanitpong <methavanitpong.pipat@socionext.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 MdeModulePkg/Bus/Sd/EmmcDxe/EmmcBlockIo.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/MdeModulePkg/Bus/Sd/EmmcDxe/EmmcBlockIo.c b/MdeModulePkg/Bus/Sd/EmmcDxe/EmmcBlockIo.c
index e1d0f394a954..f6b230514b71 100644
--- a/MdeModulePkg/Bus/Sd/EmmcDxe/EmmcBlockIo.c
+++ b/MdeModulePkg/Bus/Sd/EmmcDxe/EmmcBlockIo.c
@@ -901,7 +901,10 @@ EmmcReadWrite (
     if (EFI_ERROR (Status)) {
       return Status;
     }
-    DEBUG ((EFI_D_INFO, "Emmc%a(): Part %d Lba 0x%x BlkNo 0x%x Event %p with %r\n", IsRead ? "Read " : "Write", Partition->PartitionType, Lba, BlockNum, (Token != NULL) ? Token->Event : NULL, Status));
+    DEBUG ((DEBUG_BLKIO,
+      "Emmc%a(): Part %d Lba 0x%x BlkNo 0x%x Event %p with %r\n",
+      IsRead ? "Read " : "Write", Partition->PartitionType, Lba, BlockNum,
+      (Token != NULL) ? Token->Event : NULL, Status));
 
     Lba   += BlockNum;
     Buffer = (UINT8*)Buffer + BufferSize;
-- 
2.17.0

Re: [PATCH] MdeModulePkg/EmmcDxe: demote DEBUG print to DEBUG_BLKIO

[Laszlo Ersek]

On 06/07/18 11:10, Ard Biesheuvel wrote:
> Lower the priority of the DEBUG print in EmmcReadWrite(), which
> is emitted for each read or write operation to the eMMC device,
> which clutters up the log output of builds created with DEBUG_INFO
> enabled.
> 
> Suggested-by: Pipat Methavanitpong <methavanitpong.pipat@socionext.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  MdeModulePkg/Bus/Sd/EmmcDxe/EmmcBlockIo.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/MdeModulePkg/Bus/Sd/EmmcDxe/EmmcBlockIo.c b/MdeModulePkg/Bus/Sd/EmmcDxe/EmmcBlockIo.c
> index e1d0f394a954..f6b230514b71 100644
> --- a/MdeModulePkg/Bus/Sd/EmmcDxe/EmmcBlockIo.c
> +++ b/MdeModulePkg/Bus/Sd/EmmcDxe/EmmcBlockIo.c
> @@ -901,7 +901,10 @@ EmmcReadWrite (
>      if (EFI_ERROR (Status)) {
>        return Status;
>      }
> -    DEBUG ((EFI_D_INFO, "Emmc%a(): Part %d Lba 0x%x BlkNo 0x%x Event %p with %r\n", IsRead ? "Read " : "Write", Partition->PartitionType, Lba, BlockNum, (Token != NULL) ? Token->Event : NULL, Status));
> +    DEBUG ((DEBUG_BLKIO,
> +      "Emmc%a(): Part %d Lba 0x%x BlkNo 0x%x Event %p with %r\n",
> +      IsRead ? "Read " : "Write", Partition->PartitionType, Lba, BlockNum,
> +      (Token != NULL) ? Token->Event : NULL, Status));
>  
>      Lba   += BlockNum;
>      Buffer = (UINT8*)Buffer + BufferSize;
> 

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

Re: [PATCH] MdeModulePkg/EmmcDxe: demote DEBUG print to DEBUG_BLKIO

[Zeng, Star]

Good patch.

Another choice is to use DEBUG_VERBOSE.
We see other driver uses DEBUG_VERBOSE for BlockIo service (Hao can comment on that).
We'd better to align them for consistency.


Thanks,
Star
-----Original Message-----
From: Laszlo Ersek [mailto:lersek@redhat.com] 
Sent: Friday, June 8, 2018 1:23 AM
To: Ard Biesheuvel <ard.biesheuvel@linaro.org>; edk2-devel@lists.01.org
Cc: Zeng, Star <star.zeng@intel.com>
Subject: Re: [edk2] [PATCH] MdeModulePkg/EmmcDxe: demote DEBUG print to DEBUG_BLKIO

On 06/07/18 11:10, Ard Biesheuvel wrote:
> Lower the priority of the DEBUG print in EmmcReadWrite(), which is 
> emitted for each read or write operation to the eMMC device, which 
> clutters up the log output of builds created with DEBUG_INFO enabled.
> 
> Suggested-by: Pipat Methavanitpong 
> <methavanitpong.pipat@socionext.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  MdeModulePkg/Bus/Sd/EmmcDxe/EmmcBlockIo.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/MdeModulePkg/Bus/Sd/EmmcDxe/EmmcBlockIo.c 
> b/MdeModulePkg/Bus/Sd/EmmcDxe/EmmcBlockIo.c
> index e1d0f394a954..f6b230514b71 100644
> --- a/MdeModulePkg/Bus/Sd/EmmcDxe/EmmcBlockIo.c
> +++ b/MdeModulePkg/Bus/Sd/EmmcDxe/EmmcBlockIo.c
> @@ -901,7 +901,10 @@ EmmcReadWrite (
>      if (EFI_ERROR (Status)) {
>        return Status;
>      }
> -    DEBUG ((EFI_D_INFO, "Emmc%a(): Part %d Lba 0x%x BlkNo 0x%x Event %p with %r\n", IsRead ? "Read " : "Write", Partition->PartitionType, Lba, BlockNum, (Token != NULL) ? Token->Event : NULL, Status));
> +    DEBUG ((DEBUG_BLKIO,
> +      "Emmc%a(): Part %d Lba 0x%x BlkNo 0x%x Event %p with %r\n",
> +      IsRead ? "Read " : "Write", Partition->PartitionType, Lba, BlockNum,
> +      (Token != NULL) ? Token->Event : NULL, Status));
>  
>      Lba   += BlockNum;
>      Buffer = (UINT8*)Buffer + BufferSize;
> 

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

Re: [PATCH] MdeModulePkg/EmmcDxe: demote DEBUG print to DEBUG_BLKIO

[Ard Biesheuvel]

On 8 June 2018 at 05:15, Zeng, Star <star.zeng@intel.com> wrote:
> Good patch.
>
> Another choice is to use DEBUG_VERBOSE.
> We see other driver uses DEBUG_VERBOSE for BlockIo service (Hao can comment on that).
> We'd better to align them for consistency.
>

Hao,

Do you have any preference regarding the exact priority level we will
use for this particular DEBUG() print?

Thanks,
Ard.


> -----Original Message-----
> From: Laszlo Ersek [mailto:lersek@redhat.com]
> Sent: Friday, June 8, 2018 1:23 AM
> To: Ard Biesheuvel <ard.biesheuvel@linaro.org>; edk2-devel@lists.01.org
> Cc: Zeng, Star <star.zeng@intel.com>
> Subject: Re: [edk2] [PATCH] MdeModulePkg/EmmcDxe: demote DEBUG print to DEBUG_BLKIO
>
> On 06/07/18 11:10, Ard Biesheuvel wrote:
>> Lower the priority of the DEBUG print in EmmcReadWrite(), which is
>> emitted for each read or write operation to the eMMC device, which
>> clutters up the log output of builds created with DEBUG_INFO enabled.
>>
>> Suggested-by: Pipat Methavanitpong
>> <methavanitpong.pipat@socionext.com>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> ---
>>  MdeModulePkg/Bus/Sd/EmmcDxe/EmmcBlockIo.c | 5 ++++-
>>  1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/MdeModulePkg/Bus/Sd/EmmcDxe/EmmcBlockIo.c
>> b/MdeModulePkg/Bus/Sd/EmmcDxe/EmmcBlockIo.c
>> index e1d0f394a954..f6b230514b71 100644
>> --- a/MdeModulePkg/Bus/Sd/EmmcDxe/EmmcBlockIo.c
>> +++ b/MdeModulePkg/Bus/Sd/EmmcDxe/EmmcBlockIo.c
>> @@ -901,7 +901,10 @@ EmmcReadWrite (
>>      if (EFI_ERROR (Status)) {
>>        return Status;
>>      }
>> -    DEBUG ((EFI_D_INFO, "Emmc%a(): Part %d Lba 0x%x BlkNo 0x%x Event %p with %r\n", IsRead ? "Read " : "Write", Partition->PartitionType, Lba, BlockNum, (Token != NULL) ? Token->Event : NULL, Status));
>> +    DEBUG ((DEBUG_BLKIO,
>> +      "Emmc%a(): Part %d Lba 0x%x BlkNo 0x%x Event %p with %r\n",
>> +      IsRead ? "Read " : "Write", Partition->PartitionType, Lba, BlockNum,
>> +      (Token != NULL) ? Token->Event : NULL, Status));
>>
>>      Lba   += BlockNum;
>>      Buffer = (UINT8*)Buffer + BufferSize;
>>
>
> Reviewed-by: Laszlo Ersek <lersek@redhat.com>

Re: [PATCH] MdeModulePkg/EmmcDxe: demote DEBUG print to DEBUG_BLKIO

[Wu, Hao A]

Hi Ard,

After a quick check on the behavior of other storage device drivers, it seems
to me that they are not using the same debug levels for this kind of debug
message:

ATA and USB mass storage - BLKIO
NVM Express - VERBOSE
SD/eMMC - INFO
SCSI - actually no such debug message

My preference is to use the 'BLKIO' for the SD/eMMC case, since literally, it
seems the best fit and the majority of the drivers are using this level.
Or maybe we can use a combination of (DEBUG_BLKIO | DEBUG_VERBOSE).

Ard and Star, what's your thought?

Best Regards,
Hao Wu

> -----Original Message-----
> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
> Sent: Monday, June 11, 2018 4:15 PM
> To: Zeng, Star
> Cc: edk2-devel@lists.01.org; Laszlo Ersek; Wu, Hao A
> Subject: Re: [edk2] [PATCH] MdeModulePkg/EmmcDxe: demote DEBUG print to
> DEBUG_BLKIO
> 
> On 8 June 2018 at 05:15, Zeng, Star <star.zeng@intel.com> wrote:
> > Good patch.
> >
> > Another choice is to use DEBUG_VERBOSE.
> > We see other driver uses DEBUG_VERBOSE for BlockIo service (Hao can
> comment on that).
> > We'd better to align them for consistency.
> >
> 
> Hao,
> 
> Do you have any preference regarding the exact priority level we will
> use for this particular DEBUG() print?
> 
> Thanks,
> Ard.
> 
> 
> > -----Original Message-----
> > From: Laszlo Ersek [mailto:lersek@redhat.com]
> > Sent: Friday, June 8, 2018 1:23 AM
> > To: Ard Biesheuvel <ard.biesheuvel@linaro.org>; edk2-devel@lists.01.org
> > Cc: Zeng, Star <star.zeng@intel.com>
> > Subject: Re: [edk2] [PATCH] MdeModulePkg/EmmcDxe: demote DEBUG print
> to DEBUG_BLKIO
> >
> > On 06/07/18 11:10, Ard Biesheuvel wrote:
> >> Lower the priority of the DEBUG print in EmmcReadWrite(), which is
> >> emitted for each read or write operation to the eMMC device, which
> >> clutters up the log output of builds created with DEBUG_INFO enabled.
> >>
> >> Suggested-by: Pipat Methavanitpong
> >> <methavanitpong.pipat@socionext.com>
> >> Contributed-under: TianoCore Contribution Agreement 1.1
> >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> >> ---
> >>  MdeModulePkg/Bus/Sd/EmmcDxe/EmmcBlockIo.c | 5 ++++-
> >>  1 file changed, 4 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/MdeModulePkg/Bus/Sd/EmmcDxe/EmmcBlockIo.c
> >> b/MdeModulePkg/Bus/Sd/EmmcDxe/EmmcBlockIo.c
> >> index e1d0f394a954..f6b230514b71 100644
> >> --- a/MdeModulePkg/Bus/Sd/EmmcDxe/EmmcBlockIo.c
> >> +++ b/MdeModulePkg/Bus/Sd/EmmcDxe/EmmcBlockIo.c
> >> @@ -901,7 +901,10 @@ EmmcReadWrite (
> >>      if (EFI_ERROR (Status)) {
> >>        return Status;
> >>      }
> >> -    DEBUG ((EFI_D_INFO, "Emmc%a(): Part %d Lba 0x%x BlkNo 0x%x
> Event %p with %r\n", IsRead ? "Read " : "Write", Partition->PartitionType, Lba,
> BlockNum, (Token != NULL) ? Token->Event : NULL, Status));
> >> +    DEBUG ((DEBUG_BLKIO,
> >> +      "Emmc%a(): Part %d Lba 0x%x BlkNo 0x%x Event %p with %r\n",
> >> +      IsRead ? "Read " : "Write", Partition->PartitionType, Lba, BlockNum,
> >> +      (Token != NULL) ? Token->Event : NULL, Status));
> >>
> >>      Lba   += BlockNum;
> >>      Buffer = (UINT8*)Buffer + BufferSize;
> >>
> >
> > Reviewed-by: Laszlo Ersek <lersek@redhat.com>

Re: [PATCH] MdeModulePkg/EmmcDxe: demote DEBUG print to DEBUG_BLKIO

[Ard Biesheuvel]

On 11 June 2018 at 10:38, Wu, Hao A <hao.a.wu@intel.com> wrote:
> Hi Ard,
>
> After a quick check on the behavior of other storage device drivers, it seems
> to me that they are not using the same debug levels for this kind of debug
> message:
>
> ATA and USB mass storage - BLKIO
> NVM Express - VERBOSE
> SD/eMMC - INFO
> SCSI - actually no such debug message
>
> My preference is to use the 'BLKIO' for the SD/eMMC case, since literally, it
> seems the best fit and the majority of the drivers are using this level.
> Or maybe we can use a combination of (DEBUG_BLKIO | DEBUG_VERBOSE).
>
> Ard and Star, what's your thought?
>

I am happy to stick with the patch as I proposed it, i.e., DEBUG_BLKIO only

>> -----Original Message-----
>> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
>> Sent: Monday, June 11, 2018 4:15 PM
>> To: Zeng, Star
>> Cc: edk2-devel@lists.01.org; Laszlo Ersek; Wu, Hao A
>> Subject: Re: [edk2] [PATCH] MdeModulePkg/EmmcDxe: demote DEBUG print to
>> DEBUG_BLKIO
>>
>> On 8 June 2018 at 05:15, Zeng, Star <star.zeng@intel.com> wrote:
>> > Good patch.
>> >
>> > Another choice is to use DEBUG_VERBOSE.
>> > We see other driver uses DEBUG_VERBOSE for BlockIo service (Hao can
>> comment on that).
>> > We'd better to align them for consistency.
>> >
>>
>> Hao,
>>
>> Do you have any preference regarding the exact priority level we will
>> use for this particular DEBUG() print?
>>
>> Thanks,
>> Ard.
>>
>>
>> > -----Original Message-----
>> > From: Laszlo Ersek [mailto:lersek@redhat.com]
>> > Sent: Friday, June 8, 2018 1:23 AM
>> > To: Ard Biesheuvel <ard.biesheuvel@linaro.org>; edk2-devel@lists.01.org
>> > Cc: Zeng, Star <star.zeng@intel.com>
>> > Subject: Re: [edk2] [PATCH] MdeModulePkg/EmmcDxe: demote DEBUG print
>> to DEBUG_BLKIO
>> >
>> > On 06/07/18 11:10, Ard Biesheuvel wrote:
>> >> Lower the priority of the DEBUG print in EmmcReadWrite(), which is
>> >> emitted for each read or write operation to the eMMC device, which
>> >> clutters up the log output of builds created with DEBUG_INFO enabled.
>> >>
>> >> Suggested-by: Pipat Methavanitpong
>> >> <methavanitpong.pipat@socionext.com>
>> >> Contributed-under: TianoCore Contribution Agreement 1.1
>> >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> >> ---
>> >>  MdeModulePkg/Bus/Sd/EmmcDxe/EmmcBlockIo.c | 5 ++++-
>> >>  1 file changed, 4 insertions(+), 1 deletion(-)
>> >>
>> >> diff --git a/MdeModulePkg/Bus/Sd/EmmcDxe/EmmcBlockIo.c
>> >> b/MdeModulePkg/Bus/Sd/EmmcDxe/EmmcBlockIo.c
>> >> index e1d0f394a954..f6b230514b71 100644
>> >> --- a/MdeModulePkg/Bus/Sd/EmmcDxe/EmmcBlockIo.c
>> >> +++ b/MdeModulePkg/Bus/Sd/EmmcDxe/EmmcBlockIo.c
>> >> @@ -901,7 +901,10 @@ EmmcReadWrite (
>> >>      if (EFI_ERROR (Status)) {
>> >>        return Status;
>> >>      }
>> >> -    DEBUG ((EFI_D_INFO, "Emmc%a(): Part %d Lba 0x%x BlkNo 0x%x
>> Event %p with %r\n", IsRead ? "Read " : "Write", Partition->PartitionType, Lba,
>> BlockNum, (Token != NULL) ? Token->Event : NULL, Status));
>> >> +    DEBUG ((DEBUG_BLKIO,
>> >> +      "Emmc%a(): Part %d Lba 0x%x BlkNo 0x%x Event %p with %r\n",
>> >> +      IsRead ? "Read " : "Write", Partition->PartitionType, Lba, BlockNum,
>> >> +      (Token != NULL) ? Token->Event : NULL, Status));
>> >>
>> >>      Lba   += BlockNum;
>> >>      Buffer = (UINT8*)Buffer + BufferSize;
>> >>
>> >
>> > Reviewed-by: Laszlo Ersek <lersek@redhat.com>

Re: [PATCH] MdeModulePkg/EmmcDxe: demote DEBUG print to DEBUG_BLKIO

[Zeng, Star]

Let's go to use DEBUG_BLKIO to be consistent.

Ard, Reviewed-by: Star Zeng <star.zeng@intel.com>.
Hao, you can submit ticket on bugzilla and submit patch for NvmExpressDxe.

Thanks,
Star
-----Original Message-----
From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org] 
Sent: Monday, June 11, 2018 4:54 PM
To: Wu, Hao A <hao.a.wu@intel.com>
Cc: Zeng, Star <star.zeng@intel.com>; edk2-devel@lists.01.org; Laszlo Ersek <lersek@redhat.com>
Subject: Re: [edk2] [PATCH] MdeModulePkg/EmmcDxe: demote DEBUG print to DEBUG_BLKIO

On 11 June 2018 at 10:38, Wu, Hao A <hao.a.wu@intel.com> wrote:
> Hi Ard,
>
> After a quick check on the behavior of other storage device drivers, 
> it seems to me that they are not using the same debug levels for this 
> kind of debug
> message:
>
> ATA and USB mass storage - BLKIO
> NVM Express - VERBOSE
> SD/eMMC - INFO
> SCSI - actually no such debug message
>
> My preference is to use the 'BLKIO' for the SD/eMMC case, since 
> literally, it seems the best fit and the majority of the drivers are using this level.
> Or maybe we can use a combination of (DEBUG_BLKIO | DEBUG_VERBOSE).
>
> Ard and Star, what's your thought?
>

I am happy to stick with the patch as I proposed it, i.e., DEBUG_BLKIO only

>> -----Original Message-----
>> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
>> Sent: Monday, June 11, 2018 4:15 PM
>> To: Zeng, Star
>> Cc: edk2-devel@lists.01.org; Laszlo Ersek; Wu, Hao A
>> Subject: Re: [edk2] [PATCH] MdeModulePkg/EmmcDxe: demote DEBUG print 
>> to DEBUG_BLKIO
>>
>> On 8 June 2018 at 05:15, Zeng, Star <star.zeng@intel.com> wrote:
>> > Good patch.
>> >
>> > Another choice is to use DEBUG_VERBOSE.
>> > We see other driver uses DEBUG_VERBOSE for BlockIo service (Hao can
>> comment on that).
>> > We'd better to align them for consistency.
>> >
>>
>> Hao,
>>
>> Do you have any preference regarding the exact priority level we will 
>> use for this particular DEBUG() print?
>>
>> Thanks,
>> Ard.
>>
>>
>> > -----Original Message-----
>> > From: Laszlo Ersek [mailto:lersek@redhat.com]
>> > Sent: Friday, June 8, 2018 1:23 AM
>> > To: Ard Biesheuvel <ard.biesheuvel@linaro.org>; 
>> > edk2-devel@lists.01.org
>> > Cc: Zeng, Star <star.zeng@intel.com>
>> > Subject: Re: [edk2] [PATCH] MdeModulePkg/EmmcDxe: demote DEBUG 
>> > print
>> to DEBUG_BLKIO
>> >
>> > On 06/07/18 11:10, Ard Biesheuvel wrote:
>> >> Lower the priority of the DEBUG print in EmmcReadWrite(), which is 
>> >> emitted for each read or write operation to the eMMC device, which 
>> >> clutters up the log output of builds created with DEBUG_INFO enabled.
>> >>
>> >> Suggested-by: Pipat Methavanitpong 
>> >> <methavanitpong.pipat@socionext.com>
>> >> Contributed-under: TianoCore Contribution Agreement 1.1
>> >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> >> ---
>> >>  MdeModulePkg/Bus/Sd/EmmcDxe/EmmcBlockIo.c | 5 ++++-
>> >>  1 file changed, 4 insertions(+), 1 deletion(-)
>> >>
>> >> diff --git a/MdeModulePkg/Bus/Sd/EmmcDxe/EmmcBlockIo.c
>> >> b/MdeModulePkg/Bus/Sd/EmmcDxe/EmmcBlockIo.c
>> >> index e1d0f394a954..f6b230514b71 100644
>> >> --- a/MdeModulePkg/Bus/Sd/EmmcDxe/EmmcBlockIo.c
>> >> +++ b/MdeModulePkg/Bus/Sd/EmmcDxe/EmmcBlockIo.c
>> >> @@ -901,7 +901,10 @@ EmmcReadWrite (
>> >>      if (EFI_ERROR (Status)) {
>> >>        return Status;
>> >>      }
>> >> -    DEBUG ((EFI_D_INFO, "Emmc%a(): Part %d Lba 0x%x BlkNo 0x%x
>> Event %p with %r\n", IsRead ? "Read " : "Write", 
>> Partition->PartitionType, Lba, BlockNum, (Token != NULL) ? 
>> Token->Event : NULL, Status));
>> >> +    DEBUG ((DEBUG_BLKIO,
>> >> +      "Emmc%a(): Part %d Lba 0x%x BlkNo 0x%x Event %p with %r\n",
>> >> +      IsRead ? "Read " : "Write", Partition->PartitionType, Lba, BlockNum,
>> >> +      (Token != NULL) ? Token->Event : NULL, Status));
>> >>
>> >>      Lba   += BlockNum;
>> >>      Buffer = (UINT8*)Buffer + BufferSize;
>> >>
>> >
>> > Reviewed-by: Laszlo Ersek <lersek@redhat.com>

Re: [PATCH] MdeModulePkg/EmmcDxe: demote DEBUG print to DEBUG_BLKIO

[Ard Biesheuvel]

On 11 June 2018 at 11:12, Zeng, Star <star.zeng@intel.com> wrote:
> Let's go to use DEBUG_BLKIO to be consistent.
>
> Ard, Reviewed-by: Star Zeng <star.zeng@intel.com>.
> Hao, you can submit ticket on bugzilla and submit patch for NvmExpressDxe.
>

Thanks all

Pushed as 9dca2105ad96


> -----Original Message-----
> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
> Sent: Monday, June 11, 2018 4:54 PM
> To: Wu, Hao A <hao.a.wu@intel.com>
> Cc: Zeng, Star <star.zeng@intel.com>; edk2-devel@lists.01.org; Laszlo Ersek <lersek@redhat.com>
> Subject: Re: [edk2] [PATCH] MdeModulePkg/EmmcDxe: demote DEBUG print to DEBUG_BLKIO
>
> On 11 June 2018 at 10:38, Wu, Hao A <hao.a.wu@intel.com> wrote:
>> Hi Ard,
>>
>> After a quick check on the behavior of other storage device drivers,
>> it seems to me that they are not using the same debug levels for this
>> kind of debug
>> message:
>>
>> ATA and USB mass storage - BLKIO
>> NVM Express - VERBOSE
>> SD/eMMC - INFO
>> SCSI - actually no such debug message
>>
>> My preference is to use the 'BLKIO' for the SD/eMMC case, since
>> literally, it seems the best fit and the majority of the drivers are using this level.
>> Or maybe we can use a combination of (DEBUG_BLKIO | DEBUG_VERBOSE).
>>
>> Ard and Star, what's your thought?
>>
>
> I am happy to stick with the patch as I proposed it, i.e., DEBUG_BLKIO only
>
>>> -----Original Message-----
>>> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
>>> Sent: Monday, June 11, 2018 4:15 PM
>>> To: Zeng, Star
>>> Cc: edk2-devel@lists.01.org; Laszlo Ersek; Wu, Hao A
>>> Subject: Re: [edk2] [PATCH] MdeModulePkg/EmmcDxe: demote DEBUG print
>>> to DEBUG_BLKIO
>>>
>>> On 8 June 2018 at 05:15, Zeng, Star <star.zeng@intel.com> wrote:
>>> > Good patch.
>>> >
>>> > Another choice is to use DEBUG_VERBOSE.
>>> > We see other driver uses DEBUG_VERBOSE for BlockIo service (Hao can
>>> comment on that).
>>> > We'd better to align them for consistency.
>>> >
>>>
>>> Hao,
>>>
>>> Do you have any preference regarding the exact priority level we will
>>> use for this particular DEBUG() print?
>>>
>>> Thanks,
>>> Ard.
>>>
>>>
>>> > -----Original Message-----
>>> > From: Laszlo Ersek [mailto:lersek@redhat.com]
>>> > Sent: Friday, June 8, 2018 1:23 AM
>>> > To: Ard Biesheuvel <ard.biesheuvel@linaro.org>;
>>> > edk2-devel@lists.01.org
>>> > Cc: Zeng, Star <star.zeng@intel.com>
>>> > Subject: Re: [edk2] [PATCH] MdeModulePkg/EmmcDxe: demote DEBUG
>>> > print
>>> to DEBUG_BLKIO
>>> >
>>> > On 06/07/18 11:10, Ard Biesheuvel wrote:
>>> >> Lower the priority of the DEBUG print in EmmcReadWrite(), which is
>>> >> emitted for each read or write operation to the eMMC device, which
>>> >> clutters up the log output of builds created with DEBUG_INFO enabled.
>>> >>
>>> >> Suggested-by: Pipat Methavanitpong
>>> >> <methavanitpong.pipat@socionext.com>
>>> >> Contributed-under: TianoCore Contribution Agreement 1.1
>>> >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>> >> ---
>>> >>  MdeModulePkg/Bus/Sd/EmmcDxe/EmmcBlockIo.c | 5 ++++-
>>> >>  1 file changed, 4 insertions(+), 1 deletion(-)
>>> >>
>>> >> diff --git a/MdeModulePkg/Bus/Sd/EmmcDxe/EmmcBlockIo.c
>>> >> b/MdeModulePkg/Bus/Sd/EmmcDxe/EmmcBlockIo.c
>>> >> index e1d0f394a954..f6b230514b71 100644
>>> >> --- a/MdeModulePkg/Bus/Sd/EmmcDxe/EmmcBlockIo.c
>>> >> +++ b/MdeModulePkg/Bus/Sd/EmmcDxe/EmmcBlockIo.c
>>> >> @@ -901,7 +901,10 @@ EmmcReadWrite (
>>> >>      if (EFI_ERROR (Status)) {
>>> >>        return Status;
>>> >>      }
>>> >> -    DEBUG ((EFI_D_INFO, "Emmc%a(): Part %d Lba 0x%x BlkNo 0x%x
>>> Event %p with %r\n", IsRead ? "Read " : "Write",
>>> Partition->PartitionType, Lba, BlockNum, (Token != NULL) ?
>>> Token->Event : NULL, Status));
>>> >> +    DEBUG ((DEBUG_BLKIO,
>>> >> +      "Emmc%a(): Part %d Lba 0x%x BlkNo 0x%x Event %p with %r\n",
>>> >> +      IsRead ? "Read " : "Write", Partition->PartitionType, Lba, BlockNum,
>>> >> +      (Token != NULL) ? Token->Event : NULL, Status));
>>> >>
>>> >>      Lba   += BlockNum;
>>> >>      Buffer = (UINT8*)Buffer + BufferSize;
>>> >>
>>> >
>>> > Reviewed-by: Laszlo Ersek <lersek@redhat.com>

Re: [PATCH] MdeModulePkg/EmmcDxe: demote DEBUG print to DEBUG_BLKIO

[Wu, Hao A]

Yes, BZ 980 was filed, and I will propose a patch for it.

Best Regards,
Hao Wu


> -----Original Message-----
> From: Zeng, Star
> Sent: Monday, June 11, 2018 5:13 PM
> To: Ard Biesheuvel; Wu, Hao A
> Cc: edk2-devel@lists.01.org; Laszlo Ersek; Zeng, Star
> Subject: RE: [edk2] [PATCH] MdeModulePkg/EmmcDxe: demote DEBUG print to
> DEBUG_BLKIO
> 
> Let's go to use DEBUG_BLKIO to be consistent.
> 
> Ard, Reviewed-by: Star Zeng <star.zeng@intel.com>.
> Hao, you can submit ticket on bugzilla and submit patch for NvmExpressDxe.
> 
> Thanks,
> Star
> -----Original Message-----
> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
> Sent: Monday, June 11, 2018 4:54 PM
> To: Wu, Hao A <hao.a.wu@intel.com>
> Cc: Zeng, Star <star.zeng@intel.com>; edk2-devel@lists.01.org; Laszlo Ersek
> <lersek@redhat.com>
> Subject: Re: [edk2] [PATCH] MdeModulePkg/EmmcDxe: demote DEBUG print to
> DEBUG_BLKIO
> 
> On 11 June 2018 at 10:38, Wu, Hao A <hao.a.wu@intel.com> wrote:
> > Hi Ard,
> >
> > After a quick check on the behavior of other storage device drivers,
> > it seems to me that they are not using the same debug levels for this
> > kind of debug
> > message:
> >
> > ATA and USB mass storage - BLKIO
> > NVM Express - VERBOSE
> > SD/eMMC - INFO
> > SCSI - actually no such debug message
> >
> > My preference is to use the 'BLKIO' for the SD/eMMC case, since
> > literally, it seems the best fit and the majority of the drivers are using this
> level.
> > Or maybe we can use a combination of (DEBUG_BLKIO | DEBUG_VERBOSE).
> >
> > Ard and Star, what's your thought?
> >
> 
> I am happy to stick with the patch as I proposed it, i.e., DEBUG_BLKIO only
> 
> >> -----Original Message-----
> >> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
> >> Sent: Monday, June 11, 2018 4:15 PM
> >> To: Zeng, Star
> >> Cc: edk2-devel@lists.01.org; Laszlo Ersek; Wu, Hao A
> >> Subject: Re: [edk2] [PATCH] MdeModulePkg/EmmcDxe: demote DEBUG
> print
> >> to DEBUG_BLKIO
> >>
> >> On 8 June 2018 at 05:15, Zeng, Star <star.zeng@intel.com> wrote:
> >> > Good patch.
> >> >
> >> > Another choice is to use DEBUG_VERBOSE.
> >> > We see other driver uses DEBUG_VERBOSE for BlockIo service (Hao can
> >> comment on that).
> >> > We'd better to align them for consistency.
> >> >
> >>
> >> Hao,
> >>
> >> Do you have any preference regarding the exact priority level we will
> >> use for this particular DEBUG() print?
> >>
> >> Thanks,
> >> Ard.
> >>
> >>
> >> > -----Original Message-----
> >> > From: Laszlo Ersek [mailto:lersek@redhat.com]
> >> > Sent: Friday, June 8, 2018 1:23 AM
> >> > To: Ard Biesheuvel <ard.biesheuvel@linaro.org>;
> >> > edk2-devel@lists.01.org
> >> > Cc: Zeng, Star <star.zeng@intel.com>
> >> > Subject: Re: [edk2] [PATCH] MdeModulePkg/EmmcDxe: demote DEBUG
> >> > print
> >> to DEBUG_BLKIO
> >> >
> >> > On 06/07/18 11:10, Ard Biesheuvel wrote:
> >> >> Lower the priority of the DEBUG print in EmmcReadWrite(), which is
> >> >> emitted for each read or write operation to the eMMC device, which
> >> >> clutters up the log output of builds created with DEBUG_INFO enabled.
> >> >>
> >> >> Suggested-by: Pipat Methavanitpong
> >> >> <methavanitpong.pipat@socionext.com>
> >> >> Contributed-under: TianoCore Contribution Agreement 1.1
> >> >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> >> >> ---
> >> >>  MdeModulePkg/Bus/Sd/EmmcDxe/EmmcBlockIo.c | 5 ++++-
> >> >>  1 file changed, 4 insertions(+), 1 deletion(-)
> >> >>
> >> >> diff --git a/MdeModulePkg/Bus/Sd/EmmcDxe/EmmcBlockIo.c
> >> >> b/MdeModulePkg/Bus/Sd/EmmcDxe/EmmcBlockIo.c
> >> >> index e1d0f394a954..f6b230514b71 100644
> >> >> --- a/MdeModulePkg/Bus/Sd/EmmcDxe/EmmcBlockIo.c
> >> >> +++ b/MdeModulePkg/Bus/Sd/EmmcDxe/EmmcBlockIo.c
> >> >> @@ -901,7 +901,10 @@ EmmcReadWrite (
> >> >>      if (EFI_ERROR (Status)) {
> >> >>        return Status;
> >> >>      }
> >> >> -    DEBUG ((EFI_D_INFO, "Emmc%a(): Part %d Lba 0x%x BlkNo 0x%x
> >> Event %p with %r\n", IsRead ? "Read " : "Write",
> >> Partition->PartitionType, Lba, BlockNum, (Token != NULL) ?
> >> Token->Event : NULL, Status));
> >> >> +    DEBUG ((DEBUG_BLKIO,
> >> >> +      "Emmc%a(): Part %d Lba 0x%x BlkNo 0x%x Event %p with %r\n",
> >> >> +      IsRead ? "Read " : "Write", Partition->PartitionType, Lba, BlockNum,
> >> >> +      (Token != NULL) ? Token->Event : NULL, Status));
> >> >>
> >> >>      Lba   += BlockNum;
> >> >>      Buffer = (UINT8*)Buffer + BufferSize;
> >> >>
> >> >
> >> > Reviewed-by: Laszlo Ersek <lersek@redhat.com>