| Message ID | 20110810203340.21204.45508.stgit@shuttle2.etheralp.ch |
|---|---|
| State | Changes Requested |
| Delegated to: | Stefano Babic |
| Headers | show |
Dear Eric Jarrige, On 08/10/2011 10:33 PM, Eric Jarrige wrote: > Signed-off-by: Eric Jarrige<eric.jarrige@armadeus.org> > Cc: Ben Warren<biggerbadderben@gmail.com> > --- > drivers/net/dm9000x.c | 8 ++++---- > 1 files changed, 4 insertions(+), 4 deletions(-) > > diff --git a/drivers/net/dm9000x.c b/drivers/net/dm9000x.c > index b5c5573..9cd0195 100644 > --- a/drivers/net/dm9000x.c > +++ b/drivers/net/dm9000x.c > @@ -232,7 +232,7 @@ dm9000_probe(void) > id_val |= DM9000_ior(DM9000_PIDL)<< 16; > id_val |= DM9000_ior(DM9000_PIDH)<< 24; > if (id_val == DM9000_ID) { > - printf("dm9000 i/o: 0x%x, id: 0x%x \n", CONFIG_DM9000_BASE, > + DM9000_DBG("dm9000 i/o: 0x%x, id: 0x%x \n", CONFIG_DM9000_BASE, > id_val); > return 0; > } else { > @@ -298,19 +298,19 @@ static int dm9000_init(struct eth_device *dev, bd_t *bd) > > switch (io_mode) { > case 0x0: /* 16-bit mode */ > - printf("DM9000: running in 16 bit mode\n"); > + DM9000_DBG("DM9000: running in 16 bit mode\n"); <snip> I'am just wondering: I see that DM9000_DBG is used all over dm9000 code - do you know the reason why not just use debug()? Regards Simon
Hi Simon, > Dear Eric Jarrige, > > On 08/10/2011 10:33 PM, Eric Jarrige wrote: >> Signed-off-by: Eric Jarrige<eric.jarrige@armadeus.org> >> Cc: Ben Warren<biggerbadderben@gmail.com> >> --- >> drivers/net/dm9000x.c | 8 ++++---- >> 1 files changed, 4 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/net/dm9000x.c b/drivers/net/dm9000x.c >> index b5c5573..9cd0195 100644 >> --- a/drivers/net/dm9000x.c >> +++ b/drivers/net/dm9000x.c >> @@ -232,7 +232,7 @@ dm9000_probe(void) >> id_val |= DM9000_ior(DM9000_PIDL)<< 16; >> id_val |= DM9000_ior(DM9000_PIDH)<< 24; >> if (id_val == DM9000_ID) { >> - printf("dm9000 i/o: 0x%x, id: 0x%x \n", CONFIG_DM9000_BASE, >> + DM9000_DBG("dm9000 i/o: 0x%x, id: 0x%x \n", CONFIG_DM9000_BASE, >> id_val); >> return 0; >> } else { >> @@ -298,19 +298,19 @@ static int dm9000_init(struct eth_device *dev, bd_t *bd) >> >> switch (io_mode) { >> case 0x0: /* 16-bit mode */ >> - printf("DM9000: running in 16 bit mode\n"); >> + DM9000_DBG("DM9000: running in 16 bit mode\n"); > <snip> > > I'am just wondering: I see that DM9000_DBG is used all over dm9000 code > - do you know the reason why not just use debug()? Very likely only historical reasons as the code predates the DEBUG best practice. Bow that you've identified it, we should change it ;) Cheers Detlev
On 08/10/2011 10:33 PM, Eric Jarrige wrote: > Signed-off-by: Eric Jarrige <eric.jarrige@armadeus.org> > Cc: Ben Warren <biggerbadderben@gmail.com> > --- > drivers/net/dm9000x.c | 8 ++++---- > 1 files changed, 4 insertions(+), 4 deletions(-) Hi Eric, > - printf("dm9000 i/o: 0x%x, id: 0x%x \n", CONFIG_DM9000_BASE, > + DM9000_DBG("dm9000 i/o: 0x%x, id: 0x%x \n", CONFIG_DM9000_BASE, > id_val); This is a good choice to get rid of nasty DM9000_DBG replacing it with the general debug() ! Best regards, Stefano Babic
Hi Simon, Hi Detlev, > Hi Simon, > >> Dear Eric Jarrige, >> >> On 08/10/2011 10:33 PM, Eric Jarrige wrote: >>> Signed-off-by: Eric Jarrige<eric.jarrige@armadeus.org> >>> Cc: Ben Warren<biggerbadderben@gmail.com> >>> --- >>> drivers/net/dm9000x.c | 8 ++++---- >>> 1 files changed, 4 insertions(+), 4 deletions(-) >>> >>> diff --git a/drivers/net/dm9000x.c b/drivers/net/dm9000x.c >>> index b5c5573..9cd0195 100644 >>> --- a/drivers/net/dm9000x.c >>> +++ b/drivers/net/dm9000x.c >>> @@ -232,7 +232,7 @@ dm9000_probe(void) >>> id_val |= DM9000_ior(DM9000_PIDL)<< 16; >>> id_val |= DM9000_ior(DM9000_PIDH)<< 24; >>> if (id_val == DM9000_ID) { >>> - printf("dm9000 i/o: 0x%x, id: 0x%x \n", CONFIG_DM9000_BASE, >>> + DM9000_DBG("dm9000 i/o: 0x%x, id: 0x%x \n", CONFIG_DM9000_BASE, >>> id_val); >>> return 0; >>> } else { >>> @@ -298,19 +298,19 @@ static int dm9000_init(struct eth_device *dev, bd_t *bd) >>> >>> switch (io_mode) { >>> case 0x0: /* 16-bit mode */ >>> - printf("DM9000: running in 16 bit mode\n"); >>> + DM9000_DBG("DM9000: running in 16 bit mode\n"); >> <snip> >> >> I'am just wondering: I see that DM9000_DBG is used all over dm9000 code >> - do you know the reason why not just use debug()? dm9000 does not use debug() at all. That's the reason to fix the printf issues as a first priority by using the DM9000_DBG. > > Very likely only historical reasons as the code predates the DEBUG best > practice. Bow that you've identified it, we should change it ;) > Cheers, Eric
Dear Eric Jarrige, In message <2B613989-ADDB-43D6-A8C3-A30D5245262A@armadeus.org> you wrote: > > >> I'am just wondering: I see that DM9000_DBG is used all over dm9000 code > >> - do you know the reason why not just use debug()? > > dm9000 does not use debug() at all. > That's the reason to fix the printf issues as a first priority by using the DM9000_DBG. NAK. If you touch that, then please do it right: us debug() instead, and remove DM9000_DBG. Thanks. Best regards, Wolfgang Denk
Dear Wolfgang, On 24 août 2011, at 22:20, Wolfgang Denk wrote: > Dear Eric Jarrige, > > In message <2B613989-ADDB-43D6-A8C3-A30D5245262A@armadeus.org> you wrote: >> >>>> I'am just wondering: I see that DM9000_DBG is used all over dm9000 code >>>> - do you know the reason why not just use debug()? >> >> dm9000 does not use debug() at all. >> That's the reason to fix the printf issues as a first priority by using the DM9000_DBG. > > NAK. > > If you touch that, then please do it right: us debug() instead, and > remove DM9000_DBG. Thanks. Do you mean both changes in one patch? Best regards, Eric
On Thursday, August 25, 2011 01:04:17 AM Eric Jarrige wrote: > Dear Wolfgang, > > On 24 août 2011, at 22:20, Wolfgang Denk wrote: > > Dear Eric Jarrige, > > > > In message <2B613989-ADDB-43D6-A8C3-A30D5245262A@armadeus.org> you wrote: > >>>> I'am just wondering: I see that DM9000_DBG is used all over dm9000 > >>>> code - do you know the reason why not just use debug()? > >> > >> dm9000 does not use debug() at all. > >> That's the reason to fix the printf issues as a first priority by using > >> the DM9000_DBG. > > > > NAK. > > > > If you touch that, then please do it right: us debug() instead, and > > remove DM9000_DBG. Thanks. > > Do you mean both changes in one patch? My wild guess would be to FIRST fix the DM9000_DBG and NEXT convert these printf()s to debug()s. Cheers > > Best regards, > Eric > _______________________________________________ > U-Boot mailing list > U-Boot@lists.denx.de > http://lists.denx.de/mailman/listinfo/u-boot
Dear Eric Jarrige, In message <683E266C-CDFB-4310-8145-1926C6FCCBD8@armadeus.org> you wrote: > > > If you touch that, then please do it right: us debug() instead, and > > remove DM9000_DBG. Thanks. > > Do you mean both changes in one patch? Yes, please. Best regards, Wolfgang Denk
diff --git a/drivers/net/dm9000x.c b/drivers/net/dm9000x.c index b5c5573..9cd0195 100644 --- a/drivers/net/dm9000x.c +++ b/drivers/net/dm9000x.c @@ -232,7 +232,7 @@ dm9000_probe(void) id_val |= DM9000_ior(DM9000_PIDL) << 16; id_val |= DM9000_ior(DM9000_PIDH) << 24; if (id_val == DM9000_ID) { - printf("dm9000 i/o: 0x%x, id: 0x%x \n", CONFIG_DM9000_BASE, + DM9000_DBG("dm9000 i/o: 0x%x, id: 0x%x \n", CONFIG_DM9000_BASE, id_val); return 0; } else { @@ -298,19 +298,19 @@ static int dm9000_init(struct eth_device *dev, bd_t *bd) switch (io_mode) { case 0x0: /* 16-bit mode */ - printf("DM9000: running in 16 bit mode\n"); + DM9000_DBG("DM9000: running in 16 bit mode\n"); db->outblk = dm9000_outblk_16bit; db->inblk = dm9000_inblk_16bit; db->rx_status = dm9000_rx_status_16bit; break; case 0x01: /* 32-bit mode */ - printf("DM9000: running in 32 bit mode\n"); + DM9000_DBG("DM9000: running in 32 bit mode\n"); db->outblk = dm9000_outblk_32bit; db->inblk = dm9000_inblk_32bit; db->rx_status = dm9000_rx_status_32bit; break; case 0x02: /* 8 bit mode */ - printf("DM9000: running in 8 bit mode\n"); + DM9000_DBG("DM9000: running in 8 bit mode\n"); db->outblk = dm9000_outblk_8bit; db->inblk = dm9000_inblk_8bit; db->rx_status = dm9000_rx_status_8bit;
Signed-off-by: Eric Jarrige <eric.jarrige@armadeus.org> Cc: Ben Warren <biggerbadderben@gmail.com> --- drivers/net/dm9000x.c | 8 ++++---- 1 files changed, 4 insertions(+), 4 deletions(-)