| Message ID | 1441444376-4947-1-git-send-email-saipava@xilinx.com |
|---|---|
| State | New |
| Headers | show |
On 09/05/2015 03:12 AM, Sai Pavan Boddu wrote: > Fix compile time warnings, because of type mismatch for unsigned long > long type. > > Signed-off-by: Sai Pavan Boddu <saipava@xilinx.com> > --- > Changes for V2: > Fix commit message. > Correct line lenght. > --- > +#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); Please also fix the real problem. This sort of bitrot will keep occurring unless DPRINT_L1() is fixed to unconditionally compile its arguments. In other words, do something like: #define DPRINT_L1(fmt, ...) \ do { \ if (SDHC_DEBUG) { \ fprintf(stderr, "QEMU SDHC: " fmt, ##__VA_ARGS__); \ } \ } while (0) #define DPRINT_L2(fmt, ...) \ do { \ if (SDHC_DEBUG > 1) { \ fprintf(stderr, "QEMU SDHC: " fmt, ##__VA_ARGS__); \ } \ } while (0) #define ERRPRINT(fmt, ...) \ do { if (SDHC_DEBUG) { \ fprintf(stderr, "QEMU SDHC ERROR: " fmt, ##__VA_ARGS__); \ } \ } while (0) rather than the current junk that eliminates the fprintf entirely when SDHC_DEBUG is at its default of 0.
On Sat, Sep 5, 2015 at 2:12 AM, Sai Pavan Boddu <sai.pavan.boddu@xilinx.com> wrote: > 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 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 >
On Sat, Sep 5, 2015 at 5:41 AM, Eric Blake <eblake@redhat.com> wrote: > On 09/05/2015 03:12 AM, Sai Pavan Boddu wrote: >> Fix compile time warnings, because of type mismatch for unsigned long >> long type. >> >> Signed-off-by: Sai Pavan Boddu <saipava@xilinx.com> >> --- >> Changes for V2: >> Fix commit message. >> Correct line lenght. >> --- > >> +#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); > > Please also fix the real problem. This sort of bitrot will keep > occurring unless DPRINT_L1() is fixed to unconditionally compile its > arguments. In other words, do something like: > This isn't strictly needed for this patch, but I see the point. Pavan do you want to respin as a 2P series or do this as follow up? Regards, Peter > #define DPRINT_L1(fmt, ...) \ > do { \ > if (SDHC_DEBUG) { \ > fprintf(stderr, "QEMU SDHC: " fmt, ##__VA_ARGS__); \ > } \ > } while (0) > #define DPRINT_L2(fmt, ...) \ > do { \ > if (SDHC_DEBUG > 1) { \ > fprintf(stderr, "QEMU SDHC: " fmt, ##__VA_ARGS__); \ > } \ > } while (0) > #define ERRPRINT(fmt, ...) \ > do { > if (SDHC_DEBUG) { \ > fprintf(stderr, "QEMU SDHC ERROR: " fmt, ##__VA_ARGS__); \ > } \ > } while (0) > > rather than the current junk that eliminates the fprintf entirely when > SDHC_DEBUG is at its default of 0. > > > -- > Eric Blake eblake redhat com +1-919-301-3266 > Libvirt virtualization library http://libvirt.org >
> -----Original Message----- > From: Eric Blake [mailto:eblake@redhat.com] > Sent: Saturday, September 05, 2015 6:12 PM > To: Sai Pavan Boddu; qemu-devel@nongnu.org; > crosthwaitepeter@gmail.com; peter.maydell@linaro.org > Cc: Sai Pavan Boddu; Edgar Iglesias; Alistair Francis > Subject: Re: [Qemu-devel] [PATCH V2] sdhci: use PRIx64 for uint64_t type > > On 09/05/2015 03:12 AM, Sai Pavan Boddu wrote: > > Fix compile time warnings, because of type mismatch for unsigned long > > long type. > > > > Signed-off-by: Sai Pavan Boddu <saipava@xilinx.com> > > --- > > Changes for V2: > > Fix commit message. > > Correct line lenght. > > --- > > > +#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); > > Please also fix the real problem. This sort of bitrot will keep > occurring unless DPRINT_L1() is fixed to unconditionally compile its > arguments. In other words, do something like: > > #define DPRINT_L1(fmt, ...) \ > do { \ > if (SDHC_DEBUG) { \ > fprintf(stderr, "QEMU SDHC: " fmt, ##__VA_ARGS__); \ > } \ > } while (0) > #define DPRINT_L2(fmt, ...) \ > do { \ > if (SDHC_DEBUG > 1) { \ > fprintf(stderr, "QEMU SDHC: " fmt, ##__VA_ARGS__); \ > } \ > } while (0) > #define ERRPRINT(fmt, ...) \ > do { > if (SDHC_DEBUG) { \ > fprintf(stderr, "QEMU SDHC ERROR: " fmt, ##__VA_ARGS__); \ > } \ > } while (0) > > rather than the current junk that eliminates the fprintf entirely when > SDHC_DEBUG is at its default of 0. [Sai Pavan ] Yeah, never thought about this. Your suggestion will help in preventing many issue like the same. I will fix this!! Thanks, Sai Pavan. > > > -- > Eric Blake eblake redhat com +1-919-301-3266 > Libvirt virtualization library http://libvirt.org
Hi Peter, > -----Original Message----- > From: Peter Crosthwaite [mailto:crosthwaitepeter@gmail.com] > Sent: Sunday, September 06, 2015 12:03 AM > To: Sai Pavan Boddu > Cc: qemu-devel@nongnu.org Developers; Peter Maydell; Alistair Francis; > Edgar Iglesias; Sai Pavan Boddu > Subject: Re: [PATCH V2] sdhci: use PRIx64 for uint64_t type > > On Sat, Sep 5, 2015 at 2:12 AM, Sai Pavan Boddu > <sai.pavan.boddu@xilinx.com> wrote: > > 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> [Sai Pavan ] Thanks :), I will even send out V3_2P, closing on Eric comments. Regards, Sai Pavan > > > --- > > 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 > >
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; }
Fix compile time warnings, because of type mismatch for unsigned long long type. Signed-off-by: Sai Pavan Boddu <saipava@xilinx.com> --- Changes for V2: Fix commit message. Correct line lenght. --- hw/sd/sdhci.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)