[Qemu-devel] [PATCH V3 1/2] sdhci: use PRIx64 for uint64_t type

[Qemu-devel] [PATCH V3 1/2] sdhci: use PRIx64 for uint64_t type

[Sai Pavan Boddu]

Fix compile time warnings, because of type mismatch for unsigned long
long type.

Signed-off-by: Sai Pavan Boddu <saipava@xilinx.com>
Reviewed-by: Peter Crosthwaite <crosthwaite.peter@gmail.com>
---
Changes for V3:
    Same as V2.
Changes for V2:
    Fix commit message.
    Correct line lenght.
---
 hw/sd/sdhci.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
index 2469077..639feee 100644
--- a/hw/sd/sdhci.c
+++ b/hw/sd/sdhci.c
@@ -22,6 +22,7 @@
  * with this program; if not, see <http://www.gnu.org/licenses/>.
  */
 
+#include <inttypes.h>
 #include "hw/hw.h"
 #include "sysemu/block-backend.h"
 #include "sysemu/blockdev.h"
@@ -719,7 +720,8 @@ static void sdhci_do_adma(SDHCIState *s)
             break;
         case SDHC_ADMA_ATTR_ACT_LINK:   /* link to next descriptor table */
             s->admasysaddr = dscr.addr;
-            DPRINT_L1("ADMA link: admasysaddr=0x%lx\n", s->admasysaddr);
+            DPRINT_L1("ADMA link: admasysaddr=0x%" PRIx64 "\n",
+                      s->admasysaddr);
             break;
         default:
             s->admasysaddr += dscr.incr;
@@ -727,7 +729,8 @@ static void sdhci_do_adma(SDHCIState *s)
         }
 
         if (dscr.attr & SDHC_ADMA_ATTR_INT) {
-            DPRINT_L1("ADMA interrupt: admasysaddr=0x%lx\n", s->admasysaddr);
+            DPRINT_L1("ADMA interrupt: admasysaddr=0x%" PRIx64 "\n",
+                      s->admasysaddr);
             if (s->norintstsen & SDHC_NISEN_DMA) {
                 s->norintsts |= SDHC_NIS_DMA;
             }
-- 
1.9.1

[Qemu-devel] [PATCH V3 2/2] sdhci: Change debug prints to compile unconditionally

[Sai Pavan Boddu]

Conditionaly compilation hides few type mismatch warnings, fix it to
compile unconditioinal.

Signed-off-by: Sai Pavan Boddu <saipava@xilinx.com>
Suggested-by: Eric Blake <eblake@redhat.com>
---
 hw/sd/sdhci.c | 36 ++++++++++++++++++------------------
 1 file changed, 18 insertions(+), 18 deletions(-)

diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
index 639feee..3ad6c76 100644
--- a/hw/sd/sdhci.c
+++ b/hw/sd/sdhci.c
@@ -37,24 +37,24 @@
 #define SDHC_DEBUG                        0
 #endif
 
-#if SDHC_DEBUG == 0
-    #define DPRINT_L1(fmt, args...)       do { } while (0)
-    #define DPRINT_L2(fmt, args...)       do { } while (0)
-    #define ERRPRINT(fmt, args...)        do { } while (0)
-#elif SDHC_DEBUG == 1
-    #define DPRINT_L1(fmt, args...)       \
-        do {fprintf(stderr, "QEMU SDHC: "fmt, ## args); } while (0)
-    #define DPRINT_L2(fmt, args...)       do { } while (0)
-    #define ERRPRINT(fmt, args...)        \
-        do {fprintf(stderr, "QEMU SDHC ERROR: "fmt, ## args); } while (0)
-#else
-    #define DPRINT_L1(fmt, args...)       \
-        do {fprintf(stderr, "QEMU SDHC: "fmt, ## args); } while (0)
-    #define DPRINT_L2(fmt, args...)       \
-        do {fprintf(stderr, "QEMU SDHC: "fmt, ## args); } while (0)
-    #define ERRPRINT(fmt, args...)        \
-        do {fprintf(stderr, "QEMU SDHC ERROR: "fmt, ## args); } while (0)
-#endif
+#define DPRINT_L1(fmt, args...) \
+    do { \
+        if (SDHC_DEBUG) { \
+            fprintf(stderr, "QEMU SDHC: " fmt, ## args); \
+        } \
+    } while (0)
+#define DPRINT_L2(fmt, args...) \
+    do { \
+        if (SDHC_DEBUG > 1) { \
+            fprintf(stderr, "QEMU SDHC: " fmt, ## args); \
+        } \
+    } while (0)
+#define ERRPRINT(fmt, args...) \
+    do { \
+        if (SDHC_DEBUG) { \
+            fprintf(stderr, "QEMU SDHC ERROR: " fmt, ## args); \
+        } \
+    } while (0)
 
 /* Default SD/MMC host controller features information, which will be
  * presented in CAPABILITIES register of generic SD host controller at reset.
-- 
1.9.1

Re: [Qemu-devel] [PATCH V3 2/2] sdhci: Change debug prints to compile unconditionally

[Markus Armbruster]

Sai Pavan Boddu <sai.pavan.boddu@xilinx.com> writes:

> Conditionaly compilation hides few type mismatch warnings, fix it to
> compile unconditioinal.
>
> Signed-off-by: Sai Pavan Boddu <saipava@xilinx.com>
> Suggested-by: Eric Blake <eblake@redhat.com>

No objection to this patch, but have you considered tracepoints?
See docs/tracing.txt.

Re: [Qemu-devel] [PATCH V3 2/2] sdhci: Change debug prints to compile unconditionally

[Sai Pavan Boddu]

> -----Original Message-----
> From: Markus Armbruster [mailto:armbru@redhat.com]
> Sent: Monday, September 07, 2015 1:05 PM
> To: Sai Pavan Boddu
> Cc: qemu-devel@nongnu.org; crosthwaitepeter@gmail.com;
> eblake@redhat.com; peter.maydell@linaro.org; Sai Pavan Boddu; Edgar
> Iglesias; Alistair Francis
> Subject: Re: [Qemu-devel] [PATCH V3 2/2] sdhci: Change debug prints to
> compile unconditionally
> 
> Sai Pavan Boddu <sai.pavan.boddu@xilinx.com> writes:
> 
> > Conditionaly compilation hides few type mismatch warnings, fix it to
> > compile unconditioinal.
> >
> > Signed-off-by: Sai Pavan Boddu <saipava@xilinx.com>
> > Suggested-by: Eric Blake <eblake@redhat.com>
> 
> No objection to this patch, but have you considered tracepoints?
> See docs/tracing.txt.
[Sai Pavan ] No, I didn’t do that. Actually no knowledge on that. Let me figure it out what it does. Even better if you can just explain in few lines, for what trace tool is used?

Thanks,
Sai Pavan

Re: [Qemu-devel] [PATCH V3 2/2] sdhci: Change debug prints to compile unconditionally

[Markus Armbruster]

Sai Pavan Boddu <sai.pavan.boddu@xilinx.com> writes:

>> -----Original Message-----
>> From: Markus Armbruster [mailto:armbru@redhat.com]
>> Sent: Monday, September 07, 2015 1:05 PM
>> To: Sai Pavan Boddu
>> Cc: qemu-devel@nongnu.org; crosthwaitepeter@gmail.com;
>> eblake@redhat.com; peter.maydell@linaro.org; Sai Pavan Boddu; Edgar
>> Iglesias; Alistair Francis
>> Subject: Re: [Qemu-devel] [PATCH V3 2/2] sdhci: Change debug prints to
>> compile unconditionally
>> 
>> Sai Pavan Boddu <sai.pavan.boddu@xilinx.com> writes:
>> 
>> > Conditionaly compilation hides few type mismatch warnings, fix it to
>> > compile unconditioinal.
>> >
>> > Signed-off-by: Sai Pavan Boddu <saipava@xilinx.com>
>> > Suggested-by: Eric Blake <eblake@redhat.com>
>> 
>> No objection to this patch, but have you considered tracepoints?
>> See docs/tracing.txt.
>
> [Sai Pavan ] No, I didn’t do that. Actually no knowledge on that. Let
> me figure it out what it does. Even better if you can just explain in
> few lines, for what trace tool is used?

For a simple example of how to convert from debug prints to tracepoints,
check out commit f6e3534.

Perhaps the simplest way to get the debugging output then is the
"stderr" backend you get with "configure --enable-trace-backend=stderr".
Run QEMU with "-trace events=foo", where file foo contains a list of
events to enable.  To get all the tracepoints added by the commit above,
try this one-element list:

    fw_cfg_*

Hope this suffices to get you started.

Re: [Qemu-devel] [PATCH V3 2/2] sdhci: Change debug prints to compile unconditionally

[Sai Pavan Boddu]

Hi Markus,

> -----Original Message-----
> From: Markus Armbruster [mailto:armbru@redhat.com]
> Sent: Monday, September 07, 2015 2:35 PM
> To: Sai Pavan Boddu
> Cc: peter.maydell@linaro.org; Alistair Francis; crosthwaitepeter@gmail.com;
> qemu-devel@nongnu.org; Edgar Iglesias
> Subject: Re: [Qemu-devel] [PATCH V3 2/2] sdhci: Change debug prints to
> compile unconditionally
> 
> Sai Pavan Boddu <sai.pavan.boddu@xilinx.com> writes:
> 
> >> -----Original Message-----
> >> From: Markus Armbruster [mailto:armbru@redhat.com]
> >> Sent: Monday, September 07, 2015 1:05 PM
> >> To: Sai Pavan Boddu
> >> Cc: qemu-devel@nongnu.org; crosthwaitepeter@gmail.com;
> >> eblake@redhat.com; peter.maydell@linaro.org; Sai Pavan Boddu; Edgar
> >> Iglesias; Alistair Francis
> >> Subject: Re: [Qemu-devel] [PATCH V3 2/2] sdhci: Change debug prints to
> >> compile unconditionally
> >>
> >> Sai Pavan Boddu <sai.pavan.boddu@xilinx.com> writes:
> >>
> >> > Conditionaly compilation hides few type mismatch warnings, fix it to
> >> > compile unconditioinal.
> >> >
> >> > Signed-off-by: Sai Pavan Boddu <saipava@xilinx.com>
> >> > Suggested-by: Eric Blake <eblake@redhat.com>
> >>
> >> No objection to this patch, but have you considered tracepoints?
> >> See docs/tracing.txt.
> >
> > [Sai Pavan ] No, I didn’t do that. Actually no knowledge on that. Let
> > me figure it out what it does. Even better if you can just explain in
> > few lines, for what trace tool is used?
> 
> For a simple example of how to convert from debug prints to tracepoints,
> check out commit f6e3534.
> 
> Perhaps the simplest way to get the debugging output then is the
> "stderr" backend you get with "configure --enable-trace-backend=stderr".
> Run QEMU with "-trace events=foo", where file foo contains a list of
> events to enable.  To get all the tracepoints added by the commit above,
> try this one-element list:
> 
>     fw_cfg_*
> 
> Hope this suffices to get you started.
[Sai Pavan ] Thanks, this was useful. And I even tried to apply the same to sdhci. And the problems I faced in using this are as below.

-> Present sdhci debug prints are not event oriented. i.e they are generic,  so the number of arguments are changing for most of the DPRINT markers. Is there a way trace-events can manage variable arguments rather that fixed debug string and fixed number of arguments.

i.e consider two prints like below, and convert them to use  trace_debug

DPRINT("Read From %lx",READ_REG);
DPRINT("Write to %lx with mask %lx",WRITE_REG, WRITE_MASK);

We need to use two trace_debug* fuctions, as these are two different events. We cannot generically  use the same for both.

-> And even the format specifiers cannot be changed as everything here is happening at run time.

Yes, that was a very good idea to use trace tool. But much changes are needed in present code to get the same implementation. Will create the patches implementing the same later.

Anyways that was a good intro to the tracer. Thanks!

Regards,
Sai Pavan

Re: [Qemu-devel] [PATCH V3 2/2] sdhci: Change debug prints to compile unconditionally

[Markus Armbruster]

Sai Pavan Boddu <sai.pavan.boddu@xilinx.com> writes:

> Hi Markus,
>
>> -----Original Message-----
>> From: Markus Armbruster [mailto:armbru@redhat.com]
>> Sent: Monday, September 07, 2015 2:35 PM
>> To: Sai Pavan Boddu
>> Cc: peter.maydell@linaro.org; Alistair Francis; crosthwaitepeter@gmail.com;
>> qemu-devel@nongnu.org; Edgar Iglesias
>> Subject: Re: [Qemu-devel] [PATCH V3 2/2] sdhci: Change debug prints to
>> compile unconditionally
>> 
>> Sai Pavan Boddu <sai.pavan.boddu@xilinx.com> writes:
>> 
>> >> -----Original Message-----
>> >> From: Markus Armbruster [mailto:armbru@redhat.com]
>> >> Sent: Monday, September 07, 2015 1:05 PM
>> >> To: Sai Pavan Boddu
>> >> Cc: qemu-devel@nongnu.org; crosthwaitepeter@gmail.com;
>> >> eblake@redhat.com; peter.maydell@linaro.org; Sai Pavan Boddu; Edgar
>> >> Iglesias; Alistair Francis
>> >> Subject: Re: [Qemu-devel] [PATCH V3 2/2] sdhci: Change debug prints to
>> >> compile unconditionally
>> >>
>> >> Sai Pavan Boddu <sai.pavan.boddu@xilinx.com> writes:
>> >>
>> >> > Conditionaly compilation hides few type mismatch warnings, fix it to
>> >> > compile unconditioinal.
>> >> >
>> >> > Signed-off-by: Sai Pavan Boddu <saipava@xilinx.com>
>> >> > Suggested-by: Eric Blake <eblake@redhat.com>
>> >>
>> >> No objection to this patch, but have you considered tracepoints?
>> >> See docs/tracing.txt.
>> >
>> > [Sai Pavan ] No, I didn’t do that. Actually no knowledge on that. Let
>> > me figure it out what it does. Even better if you can just explain in
>> > few lines, for what trace tool is used?
>> 
>> For a simple example of how to convert from debug prints to tracepoints,
>> check out commit f6e3534.
>> 
>> Perhaps the simplest way to get the debugging output then is the
>> "stderr" backend you get with "configure --enable-trace-backend=stderr".
>> Run QEMU with "-trace events=foo", where file foo contains a list of
>> events to enable.  To get all the tracepoints added by the commit above,
>> try this one-element list:
>> 
>>     fw_cfg_*
>> 
>> Hope this suffices to get you started.
>
> [Sai Pavan ] Thanks, this was useful. And I even tried to apply the
> same to sdhci. And the problems I faced in using this are as below.
>
> -> Present sdhci debug prints are not event oriented. i.e they are
> generic, so the number of arguments are changing for most of the
> DPRINT markers. Is there a way trace-events can manage variable
> arguments rather that fixed debug string and fixed number of
> arguments.
>
> i.e consider two prints like below, and convert them to use  trace_debug
>
> DPRINT("Read From %lx",READ_REG);
> DPRINT("Write to %lx with mask %lx",WRITE_REG, WRITE_MASK);
>
> We need to use two trace_debug* fuctions, as these are two different
> events. We cannot generically use the same for both.

Yes.  As in commit f6e3534, every DPRINT() becomes a distinct
tracepoint.

> -> And even the format specifiers cannot be changed as everything here
> is happening at run time.
>
> Yes, that was a very good idea to use trace tool. But much changes are
> needed in present code to get the same implementation. Will create the
> patches implementing the same later.
>
> Anyways that was a good intro to the tracer. Thanks!

You're welcome.

Re: [Qemu-devel] [PATCH V3 2/2] sdhci: Change debug prints to compile unconditionally

[Peter Crosthwaite]

On Sun, Sep 6, 2015 at 11:45 PM, Sai Pavan Boddu
<sai.pavan.boddu@xilinx.com> wrote:
> Conditionaly compilation hides few type mismatch warnings, fix it to

"conditional"

> compile unconditioinal.
>

"unconditionally"

> Signed-off-by: Sai Pavan Boddu <saipava@xilinx.com>
> Suggested-by: Eric Blake <eblake@redhat.com>

Otherwise:

Reviewed-by: Peter Crosthwaite <crosthwaite.peter@gmail.com>

Can trivial take these two?

Regards,
Peter

> ---
>  hw/sd/sdhci.c | 36 ++++++++++++++++++------------------
>  1 file changed, 18 insertions(+), 18 deletions(-)
>
> diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
> index 639feee..3ad6c76 100644
> --- a/hw/sd/sdhci.c
> +++ b/hw/sd/sdhci.c
> @@ -37,24 +37,24 @@
>  #define SDHC_DEBUG                        0
>  #endif
>
> -#if SDHC_DEBUG == 0
> -    #define DPRINT_L1(fmt, args...)       do { } while (0)
> -    #define DPRINT_L2(fmt, args...)       do { } while (0)
> -    #define ERRPRINT(fmt, args...)        do { } while (0)
> -#elif SDHC_DEBUG == 1
> -    #define DPRINT_L1(fmt, args...)       \
> -        do {fprintf(stderr, "QEMU SDHC: "fmt, ## args); } while (0)
> -    #define DPRINT_L2(fmt, args...)       do { } while (0)
> -    #define ERRPRINT(fmt, args...)        \
> -        do {fprintf(stderr, "QEMU SDHC ERROR: "fmt, ## args); } while (0)
> -#else
> -    #define DPRINT_L1(fmt, args...)       \
> -        do {fprintf(stderr, "QEMU SDHC: "fmt, ## args); } while (0)
> -    #define DPRINT_L2(fmt, args...)       \
> -        do {fprintf(stderr, "QEMU SDHC: "fmt, ## args); } while (0)
> -    #define ERRPRINT(fmt, args...)        \
> -        do {fprintf(stderr, "QEMU SDHC ERROR: "fmt, ## args); } while (0)
> -#endif
> +#define DPRINT_L1(fmt, args...) \
> +    do { \
> +        if (SDHC_DEBUG) { \
> +            fprintf(stderr, "QEMU SDHC: " fmt, ## args); \
> +        } \
> +    } while (0)
> +#define DPRINT_L2(fmt, args...) \
> +    do { \
> +        if (SDHC_DEBUG > 1) { \
> +            fprintf(stderr, "QEMU SDHC: " fmt, ## args); \
> +        } \
> +    } while (0)
> +#define ERRPRINT(fmt, args...) \
> +    do { \
> +        if (SDHC_DEBUG) { \
> +            fprintf(stderr, "QEMU SDHC ERROR: " fmt, ## args); \
> +        } \
> +    } while (0)
>
>  /* Default SD/MMC host controller features information, which will be
>   * presented in CAPABILITIES register of generic SD host controller at reset.
> --
> 1.9.1
>

Re: [Qemu-devel] [PATCH V3 2/2] sdhci: Change debug prints to compile unconditionally

[Sai Pavan Boddu]

> -----Original Message-----
> From: Peter Crosthwaite [mailto:crosthwaitepeter@gmail.com]
> Sent: Monday, September 07, 2015 10:52 PM
> To: Sai Pavan Boddu; qemu-trivial@nongnu.org
> Cc: qemu-devel@nongnu.org Developers; Eric Blake; Peter Maydell; Alistair
> Francis; Edgar Iglesias; Sai Pavan Boddu
> Subject: Re: [PATCH V3 2/2] sdhci: Change debug prints to compile
> unconditionally
> 
> On Sun, Sep 6, 2015 at 11:45 PM, Sai Pavan Boddu
> <sai.pavan.boddu@xilinx.com> wrote:
> > Conditionaly compilation hides few type mismatch warnings, fix it to
> 
> "conditional"
> 
> > compile unconditioinal.
> >
> 
> "unconditionally"
> 
> > Signed-off-by: Sai Pavan Boddu <saipava@xilinx.com>
> > Suggested-by: Eric Blake <eblake@redhat.com>
> 
> Otherwise:
> 
> Reviewed-by: Peter Crosthwaite <crosthwaite.peter@gmail.com>
Thanks! , Send V4 correcting the commit.

Regards,
Sai Pavan
> 
> Can trivial take these two?
> 
> Regards,
> Peter
> 
> > ---
> >  hw/sd/sdhci.c | 36 ++++++++++++++++++------------------
> >  1 file changed, 18 insertions(+), 18 deletions(-)
> >
> > diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
> > index 639feee..3ad6c76 100644
> > --- a/hw/sd/sdhci.c
> > +++ b/hw/sd/sdhci.c
> > @@ -37,24 +37,24 @@
> >  #define SDHC_DEBUG                        0
> >  #endif
> >
> > -#if SDHC_DEBUG == 0
> > -    #define DPRINT_L1(fmt, args...)       do { } while (0)
> > -    #define DPRINT_L2(fmt, args...)       do { } while (0)
> > -    #define ERRPRINT(fmt, args...)        do { } while (0)
> > -#elif SDHC_DEBUG == 1
> > -    #define DPRINT_L1(fmt, args...)       \
> > -        do {fprintf(stderr, "QEMU SDHC: "fmt, ## args); } while (0)
> > -    #define DPRINT_L2(fmt, args...)       do { } while (0)
> > -    #define ERRPRINT(fmt, args...)        \
> > -        do {fprintf(stderr, "QEMU SDHC ERROR: "fmt, ## args); } while (0)
> > -#else
> > -    #define DPRINT_L1(fmt, args...)       \
> > -        do {fprintf(stderr, "QEMU SDHC: "fmt, ## args); } while (0)
> > -    #define DPRINT_L2(fmt, args...)       \
> > -        do {fprintf(stderr, "QEMU SDHC: "fmt, ## args); } while (0)
> > -    #define ERRPRINT(fmt, args...)        \
> > -        do {fprintf(stderr, "QEMU SDHC ERROR: "fmt, ## args); } while (0)
> > -#endif
> > +#define DPRINT_L1(fmt, args...) \
> > +    do { \
> > +        if (SDHC_DEBUG) { \
> > +            fprintf(stderr, "QEMU SDHC: " fmt, ## args); \
> > +        } \
> > +    } while (0)
> > +#define DPRINT_L2(fmt, args...) \
> > +    do { \
> > +        if (SDHC_DEBUG > 1) { \
> > +            fprintf(stderr, "QEMU SDHC: " fmt, ## args); \
> > +        } \
> > +    } while (0)
> > +#define ERRPRINT(fmt, args...) \
> > +    do { \
> > +        if (SDHC_DEBUG) { \
> > +            fprintf(stderr, "QEMU SDHC ERROR: " fmt, ## args); \
> > +        } \
> > +    } while (0)
> >
> >  /* Default SD/MMC host controller features information, which will be
> >   * presented in CAPABILITIES register of generic SD host controller at reset.
> > --
> > 1.9.1
> >