diff mbox series

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

Message ID 20180607091003.8687-1-ard.biesheuvel@linaro.org
State Accepted
Commit 9dca2105ad960c9946d7cc2ece40f65e1999dac7
Headers show
Series [edk2] MdeModulePkg/EmmcDxe: demote DEBUG print to DEBUG_BLKIO | expand

Commit Message

Ard Biesheuvel June 7, 2018, 9:10 a.m. UTC 
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(-)

-- 
2.17.0

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Comments

Laszlo Ersek June 7, 2018, 5:23 p.m. UTC | #1 
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>

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Zeng, Star June 8, 2018, 3:15 a.m. UTC | #2 
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>

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Ard Biesheuvel June 11, 2018, 8:14 a.m. UTC | #3 
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>

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Wu, Hao A June 11, 2018, 8:38 a.m. UTC | #4 
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>

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Ard Biesheuvel June 11, 2018, 8:54 a.m. UTC | #5 
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>

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Zeng, Star June 11, 2018, 9:12 a.m. UTC | #6 
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>

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Ard Biesheuvel June 11, 2018, 9:40 a.m. UTC | #7 
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>

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Wu, Hao A June 12, 2018, 2:04 a.m. UTC | #8 
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>

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
diff mbox series

Patch

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;