| Message ID | 1441608340-26983-2-git-send-email-saipava@xilinx.com |
|---|---|
| State | New |
| Headers | show |
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.
> -----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
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.
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
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.
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 >
> -----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 > >
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.
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(-)