* [PATCH] ixgb: Delete IXGB_DBG() macro and call pr_debug() directly.
@ 2006-10-10 12:19 Robert P. J. Day
2006-10-10 16:14 ` Auke Kok
0 siblings, 1 reply; 9+ messages in thread
From: Robert P. J. Day @ 2006-10-10 12:19 UTC (permalink / raw)
To: Linux kernel mailing list; +Cc: trivial
Delete the minimally-useful IXGB_DBG() macro and call pr_debug()
directly from the main routine.
Signed-off-by: Robert P. J. Day <rpjday@mindspring.com>
---
diff --git a/drivers/net/ixgb/ixgb.h b/drivers/net/ixgb/ixgb.h
index 50ffe90..fb9fde5 100644
--- a/drivers/net/ixgb/ixgb.h
+++ b/drivers/net/ixgb/ixgb.h
@@ -77,12 +77,6 @@ #include "ixgb_hw.h"
#include "ixgb_ee.h"
#include "ixgb_ids.h"
-#ifdef _DEBUG_DRIVER_
-#define IXGB_DBG(args...) printk(KERN_DEBUG "ixgb: " args)
-#else
-#define IXGB_DBG(args...)
-#endif
-
#define PFX "ixgb: "
#define DPRINTK(nlevel, klevel, fmt, args...) \
(void)((NETIF_MSG_##nlevel & adapter->msg_enable) && \
diff --git a/drivers/net/ixgb/ixgb_main.c b/drivers/net/ixgb/ixgb_main.c
index e09f575..eada685 100644
--- a/drivers/net/ixgb/ixgb_main.c
+++ b/drivers/net/ixgb/ixgb_main.c
@@ -1948,7 +1948,7 @@ #endif
/* All receives must fit into a single buffer */
- IXGB_DBG("Receive packet consumed multiple buffers "
+ pr_debug("ixgb: Receive packet consumed multiple buffers "
"length<%x>\n", length);
dev_kfree_skb_irq(skb);
--
all right ... what did i mess up *this* time? :-) it's good
practice. that's my story and i'm sticking to it.
rday
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] ixgb: Delete IXGB_DBG() macro and call pr_debug() directly.
2006-10-10 12:19 [PATCH] ixgb: Delete IXGB_DBG() macro and call pr_debug() directly Robert P. J. Day
@ 2006-10-10 16:14 ` Auke Kok
2006-10-10 16:36 ` Robert P. J. Day
0 siblings, 1 reply; 9+ messages in thread
From: Auke Kok @ 2006-10-10 16:14 UTC (permalink / raw)
To: Robert P. J. Day; +Cc: Linux kernel mailing list, trivial
Robert P. J. Day wrote:
> Delete the minimally-useful IXGB_DBG() macro and call pr_debug()
> directly from the main routine.
>
> Signed-off-by: Robert P. J. Day <rpjday@mindspring.com>
> ---
> diff --git a/drivers/net/ixgb/ixgb.h b/drivers/net/ixgb/ixgb.h
> index 50ffe90..fb9fde5 100644
> --- a/drivers/net/ixgb/ixgb.h
> +++ b/drivers/net/ixgb/ixgb.h
> @@ -77,12 +77,6 @@ #include "ixgb_hw.h"
> #include "ixgb_ee.h"
> #include "ixgb_ids.h"
>
> -#ifdef _DEBUG_DRIVER_
> -#define IXGB_DBG(args...) printk(KERN_DEBUG "ixgb: " args)
> -#else
> -#define IXGB_DBG(args...)
> -#endif
> -
> #define PFX "ixgb: "
> #define DPRINTK(nlevel, klevel, fmt, args...) \
> (void)((NETIF_MSG_##nlevel & adapter->msg_enable) && \
> diff --git a/drivers/net/ixgb/ixgb_main.c b/drivers/net/ixgb/ixgb_main.c
> index e09f575..eada685 100644
> --- a/drivers/net/ixgb/ixgb_main.c
> +++ b/drivers/net/ixgb/ixgb_main.c
> @@ -1948,7 +1948,7 @@ #endif
>
> /* All receives must fit into a single buffer */
>
> - IXGB_DBG("Receive packet consumed multiple buffers "
> + pr_debug("ixgb: Receive packet consumed multiple buffers "
> "length<%x>\n", length);
>
> dev_kfree_skb_irq(skb);
>
> --
>
> all right ... what did i mess up *this* time? :-) it's good
> practice. that's my story and i'm sticking to it.
We should really use dev_dbg() instead, as it retains the 'ethX:' annotation afaics.
I'll have to see what looks best and also handle e100 and e1000.
Thanks,
Auke
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] ixgb: Delete IXGB_DBG() macro and call pr_debug() directly.
2006-10-10 16:14 ` Auke Kok
@ 2006-10-10 16:36 ` Robert P. J. Day
2006-10-10 16:50 ` Auke Kok
0 siblings, 1 reply; 9+ messages in thread
From: Robert P. J. Day @ 2006-10-10 16:36 UTC (permalink / raw)
To: Auke Kok; +Cc: Linux kernel mailing list, trivial
On Tue, 10 Oct 2006, Auke Kok wrote:
> Robert P. J. Day wrote:
> > Delete the minimally-useful IXGB_DBG() macro and call pr_debug()
> > directly from the main routine.
> >
> > Signed-off-by: Robert P. J. Day <rpjday@mindspring.com>
> > ---
> > diff --git a/drivers/net/ixgb/ixgb.h b/drivers/net/ixgb/ixgb.h
> > index 50ffe90..fb9fde5 100644
> > --- a/drivers/net/ixgb/ixgb.h
> > +++ b/drivers/net/ixgb/ixgb.h
> > @@ -77,12 +77,6 @@ #include "ixgb_hw.h"
> > #include "ixgb_ee.h"
> > #include "ixgb_ids.h"
> >
> > -#ifdef _DEBUG_DRIVER_
> > -#define IXGB_DBG(args...) printk(KERN_DEBUG "ixgb: " args)
> > -#else
> > -#define IXGB_DBG(args...)
> > -#endif
> > -
> > #define PFX "ixgb: "
> > #define DPRINTK(nlevel, klevel, fmt, args...) \
> > (void)((NETIF_MSG_##nlevel & adapter->msg_enable) && \
> > diff --git a/drivers/net/ixgb/ixgb_main.c b/drivers/net/ixgb/ixgb_main.c
> > index e09f575..eada685 100644
> > --- a/drivers/net/ixgb/ixgb_main.c
> > +++ b/drivers/net/ixgb/ixgb_main.c
> > @@ -1948,7 +1948,7 @@ #endif
> >
> > /* All receives must fit into a single buffer */
> >
> > - IXGB_DBG("Receive packet consumed multiple buffers "
> > + pr_debug("ixgb: Receive packet consumed multiple
> > buffers "
> > "length<%x>\n", length);
> >
> > dev_kfree_skb_irq(skb);
> >
> > --
> >
> > all right ... what did i mess up *this* time? :-) it's good
> > practice. that's my story and i'm sticking to it.
>
> We should really use dev_dbg() instead, as it retains the 'ethX:'
> annotation afaics.
i actually tried to use that first, but it wasn't clear to me what i
would use as that first argument to dev_dbg(), given the definitions
in include/linux/device.h:
#define dev_dbg(dev, format, arg...) \
dev_printk(KERN_DEBUG , dev , format , ## arg)
#define dev_printk(level, dev, format, arg...) \
printk(level "%s %s: " format , dev_driver_string(dev) ,
(dev)->bus_id , ## arg)
if someone wants to tell me what, in the context of ixgb_main.c, i
would use as that "dev" argument, i'm all for that.
rday
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] ixgb: Delete IXGB_DBG() macro and call pr_debug() directly.
2006-10-10 16:36 ` Robert P. J. Day
@ 2006-10-10 16:50 ` Auke Kok
2006-10-10 17:51 ` Robert P. J. Day
0 siblings, 1 reply; 9+ messages in thread
From: Auke Kok @ 2006-10-10 16:50 UTC (permalink / raw)
To: Robert P. J. Day
Cc: Linux kernel mailing list, trivial, NetDev, Jesse Brandeburg
Robert P. J. Day wrote:
> On Tue, 10 Oct 2006, Auke Kok wrote:
>
>> Robert P. J. Day wrote:
>>> Delete the minimally-useful IXGB_DBG() macro and call pr_debug()
>>> directly from the main routine.
>>>
>>> Signed-off-by: Robert P. J. Day <rpjday@mindspring.com>
>>> ---
>>> diff --git a/drivers/net/ixgb/ixgb.h b/drivers/net/ixgb/ixgb.h
>>> index 50ffe90..fb9fde5 100644
>>> --- a/drivers/net/ixgb/ixgb.h
>>> +++ b/drivers/net/ixgb/ixgb.h
>>> @@ -77,12 +77,6 @@ #include "ixgb_hw.h"
>>> #include "ixgb_ee.h"
>>> #include "ixgb_ids.h"
>>>
>>> -#ifdef _DEBUG_DRIVER_
>>> -#define IXGB_DBG(args...) printk(KERN_DEBUG "ixgb: " args)
>>> -#else
>>> -#define IXGB_DBG(args...)
>>> -#endif
>>> -
>>> #define PFX "ixgb: "
>>> #define DPRINTK(nlevel, klevel, fmt, args...) \
>>> (void)((NETIF_MSG_##nlevel & adapter->msg_enable) && \
>>> diff --git a/drivers/net/ixgb/ixgb_main.c b/drivers/net/ixgb/ixgb_main.c
>>> index e09f575..eada685 100644
>>> --- a/drivers/net/ixgb/ixgb_main.c
>>> +++ b/drivers/net/ixgb/ixgb_main.c
>>> @@ -1948,7 +1948,7 @@ #endif
>>>
>>> /* All receives must fit into a single buffer */
>>>
>>> - IXGB_DBG("Receive packet consumed multiple buffers "
>>> + pr_debug("ixgb: Receive packet consumed multiple
>>> buffers "
>>> "length<%x>\n", length);
>>>
>>> dev_kfree_skb_irq(skb);
>>>
>>> --
>>>
>>> all right ... what did i mess up *this* time? :-) it's good
>>> practice. that's my story and i'm sticking to it.
>>
>> We should really use dev_dbg() instead, as it retains the 'ethX:'
>> annotation afaics.
>
> i actually tried to use that first, but it wasn't clear to me what i
> would use as that first argument to dev_dbg(), given the definitions
> in include/linux/device.h:
>
> #define dev_dbg(dev, format, arg...) \
> dev_printk(KERN_DEBUG , dev , format , ## arg)
>
> #define dev_printk(level, dev, format, arg...) \
> printk(level "%s %s: " format , dev_driver_string(dev) ,
> (dev)->bus_id , ## arg)
>
> if someone wants to tell me what, in the context of ixgb_main.c, i
> would use as that "dev" argument, i'm all for that.
(CC netdev since it's a network driver topic).
all our macro's (e100, e1000, ixgb) use adapter->netdev->name inserted through the
DPRINTK macro.
if you'd really want to clean it all up, you'd have to replace all DPRINTK() calls with
dev_dbg(adapter->netdev->name, ....) which would just make it more lengthy and
uncomfortable to read.
which puts this in a bigger perspective. I suppose the nicest way to do program these is
to do something like this:
#define ixgb_dbg(args...) dev_dbg(adapter->netdev->name, args)
#define ixgb_err(args...) dev_err(adapter->netdev->name, args)
#define ixgb_info(args...) dev_info(adapter->netdev->name, args)
and use those consistently throughout the driver, ditto for e100/e1000.
I'll look into it and see what I can do.
Cheers,
Auke
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] ixgb: Delete IXGB_DBG() macro and call pr_debug() directly.
2006-10-10 16:50 ` Auke Kok
@ 2006-10-10 17:51 ` Robert P. J. Day
2006-10-10 18:18 ` Auke Kok
0 siblings, 1 reply; 9+ messages in thread
From: Robert P. J. Day @ 2006-10-10 17:51 UTC (permalink / raw)
To: Auke Kok; +Cc: Linux kernel mailing list, trivial, NetDev, Jesse Brandeburg
On Tue, 10 Oct 2006, Auke Kok wrote:
> Robert P. J. Day wrote:
... snip ...
> > if someone wants to tell me what, in the context of ixgb_main.c,
> > i would use as that "dev" argument [for dev_dbg], i'm all for
> > that.
>
> (CC netdev since it's a network driver topic).
>
> all our macro's (e100, e1000, ixgb) use adapter->netdev->name
> inserted through the DPRINTK macro.
>
> if you'd really want to clean it all up, you'd have to replace all
> DPRINTK() calls with dev_dbg(adapter->netdev->name, ....) which
> would just make it more lengthy and uncomfortable to read.
>
> which puts this in a bigger perspective. I suppose the nicest way to do
> program these is to do something like this:
>
> #define ixgb_dbg(args...) dev_dbg(adapter->netdev->name, args)
> #define ixgb_err(args...) dev_err(adapter->netdev->name, args)
> #define ixgb_info(args...) dev_info(adapter->netdev->name, args)
>
> and use those consistently throughout the driver, ditto for e100/e1000.
>
> I'll look into it and see what I can do.
um, yeah. i'm rapidly getting out of my comfort zone here. this
seemed like such a simple submission six hours ago. :-) live and
learn.
rday
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] ixgb: Delete IXGB_DBG() macro and call pr_debug() directly.
2006-10-10 17:51 ` Robert P. J. Day
@ 2006-10-10 18:18 ` Auke Kok
0 siblings, 0 replies; 9+ messages in thread
From: Auke Kok @ 2006-10-10 18:18 UTC (permalink / raw)
To: Robert P. J. Day
Cc: Linux kernel mailing list, trivial, NetDev, Jesse Brandeburg
Robert P. J. Day wrote:
> On Tue, 10 Oct 2006, Auke Kok wrote:
>
>> Robert P. J. Day wrote:
>
> ... snip ...
>
>>> if someone wants to tell me what, in the context of ixgb_main.c,
>>> i would use as that "dev" argument [for dev_dbg], i'm all for
>>> that.
>> (CC netdev since it's a network driver topic).
>>
>> all our macro's (e100, e1000, ixgb) use adapter->netdev->name
>> inserted through the DPRINTK macro.
>>
>> if you'd really want to clean it all up, you'd have to replace all
>> DPRINTK() calls with dev_dbg(adapter->netdev->name, ....) which
>> would just make it more lengthy and uncomfortable to read.
>>
>> which puts this in a bigger perspective. I suppose the nicest way to do
>> program these is to do something like this:
>>
>> #define ixgb_dbg(args...) dev_dbg(adapter->netdev->name, args)
>> #define ixgb_err(args...) dev_err(adapter->netdev->name, args)
>> #define ixgb_info(args...) dev_info(adapter->netdev->name, args)
>>
>> and use those consistently throughout the driver, ditto for e100/e1000.
>>
>> I'll look into it and see what I can do.
>
> um, yeah. i'm rapidly getting out of my comfort zone here. this
> seemed like such a simple submission six hours ago. :-) live and
> learn.
digging into it much deeper, dev_dbg seems horribly unsuited for our needs as it prints
the pci bus address and al. That's a lot of information when we really only care about
'eth0: ' perhaps with 'ixgb: ' prepended to it.
You end up with pretty much the same code that was there before - unless we make
something common for netdevices and use that instead:
#define netdev_dbg(netdev, args...) pr_debug(netdev->name, args)
and then use this stacked in the driver:
#define ixgb_dbg(args...) netdev_dbg(adapter->netdev, args)
that would seem clean and appropriate, and since this bit of infrastructure is missing
from netdevice.h, explains the current situation in all netdrivers (chaos).
of course, I'm completely not taking netif_msg_level into account yet here, which
probably makes it a bit more hairy.
Auke
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] ixgb: Delete IXGB_DBG() macro and call pr_debug() directly.
2006-10-10 12:12 ` Andreas Schwab
@ 2006-10-10 12:15 ` Robert P. J. Day
0 siblings, 0 replies; 9+ messages in thread
From: Robert P. J. Day @ 2006-10-10 12:15 UTC (permalink / raw)
To: Andreas Schwab; +Cc: Linux kernel mailing list, trivial
On Tue, 10 Oct 2006, Andreas Schwab wrote:
> "Robert P. J. Day" <rpjday@mindspring.com> writes:
>
> > Remove the minimally-useful definition of IXGB_DBG() and have
> > ixgb_main.c call pr_debug() directly.
> >
> > Signed-off-by: Robert P. J. Day <rpjday@mindspring.com>
> > ---
> > diff --git a/drivers/net/ixgb/ixgb.h b/drivers/net/ixgb/ixgb.h
> > index 50ffe90..fb9fde5 100644
> > --- a/drivers/net/ixgb/ixgb.h
> > +++ b/drivers/net/ixgb/ixgb.h
> > @@ -77,12 +77,6 @@ #include "ixgb_hw.h"
> > #include "ixgb_ee.h"
> > #include "ixgb_ids.h"
> >
> > -#ifdef _DEBUG_DRIVER_
> > -#define IXGB_DBG(args...) printk(KERN_DEBUG "ixgb: " args)
> ^^^^^^^^
> > -#else
> > -#define IXGB_DBG(args...)
> > -#endif
> > -
> > #define PFX "ixgb: "
> > #define DPRINTK(nlevel, klevel, fmt, args...) \
> > (void)((NETIF_MSG_##nlevel & adapter->msg_enable) && \
> > diff --git a/drivers/net/ixgb/ixgb_main.c b/drivers/net/ixgb/ixgb_main.c
> > index e09f575..d063e84 100644
> > --- a/drivers/net/ixgb/ixgb_main.c
> > +++ b/drivers/net/ixgb/ixgb_main.c
> > @@ -1948,7 +1948,7 @@ #endif
> >
> > /* All receives must fit into a single buffer */
> >
> > - IXGB_DBG("Receive packet consumed multiple buffers "
> > + pr_debug("Receive packet consumed multiple buffers "
>
> Would perhaps be useful to retain the "ixgb:" prefix.
crap, you're right. my bad. back into the breech ... argh. that's
what happens when there's only decaf left in the house.
rday
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] ixgb: Delete IXGB_DBG() macro and call pr_debug() directly.
2006-10-10 11:59 Robert P. J. Day
@ 2006-10-10 12:12 ` Andreas Schwab
2006-10-10 12:15 ` Robert P. J. Day
0 siblings, 1 reply; 9+ messages in thread
From: Andreas Schwab @ 2006-10-10 12:12 UTC (permalink / raw)
To: Robert P. J. Day; +Cc: Linux kernel mailing list, trivial
"Robert P. J. Day" <rpjday@mindspring.com> writes:
> Remove the minimally-useful definition of IXGB_DBG() and have
> ixgb_main.c call pr_debug() directly.
>
> Signed-off-by: Robert P. J. Day <rpjday@mindspring.com>
> ---
> diff --git a/drivers/net/ixgb/ixgb.h b/drivers/net/ixgb/ixgb.h
> index 50ffe90..fb9fde5 100644
> --- a/drivers/net/ixgb/ixgb.h
> +++ b/drivers/net/ixgb/ixgb.h
> @@ -77,12 +77,6 @@ #include "ixgb_hw.h"
> #include "ixgb_ee.h"
> #include "ixgb_ids.h"
>
> -#ifdef _DEBUG_DRIVER_
> -#define IXGB_DBG(args...) printk(KERN_DEBUG "ixgb: " args)
^^^^^^^^
> -#else
> -#define IXGB_DBG(args...)
> -#endif
> -
> #define PFX "ixgb: "
> #define DPRINTK(nlevel, klevel, fmt, args...) \
> (void)((NETIF_MSG_##nlevel & adapter->msg_enable) && \
> diff --git a/drivers/net/ixgb/ixgb_main.c b/drivers/net/ixgb/ixgb_main.c
> index e09f575..d063e84 100644
> --- a/drivers/net/ixgb/ixgb_main.c
> +++ b/drivers/net/ixgb/ixgb_main.c
> @@ -1948,7 +1948,7 @@ #endif
>
> /* All receives must fit into a single buffer */
>
> - IXGB_DBG("Receive packet consumed multiple buffers "
> + pr_debug("Receive packet consumed multiple buffers "
Would perhaps be useful to retain the "ixgb:" prefix.
Andreas.
--
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
PGP key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5
"And now for something completely different."
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH] ixgb: Delete IXGB_DBG() macro and call pr_debug() directly.
@ 2006-10-10 11:59 Robert P. J. Day
2006-10-10 12:12 ` Andreas Schwab
0 siblings, 1 reply; 9+ messages in thread
From: Robert P. J. Day @ 2006-10-10 11:59 UTC (permalink / raw)
To: Linux kernel mailing list; +Cc: trivial
Remove the minimally-useful definition of IXGB_DBG() and have
ixgb_main.c call pr_debug() directly.
Signed-off-by: Robert P. J. Day <rpjday@mindspring.com>
---
diff --git a/drivers/net/ixgb/ixgb.h b/drivers/net/ixgb/ixgb.h
index 50ffe90..fb9fde5 100644
--- a/drivers/net/ixgb/ixgb.h
+++ b/drivers/net/ixgb/ixgb.h
@@ -77,12 +77,6 @@ #include "ixgb_hw.h"
#include "ixgb_ee.h"
#include "ixgb_ids.h"
-#ifdef _DEBUG_DRIVER_
-#define IXGB_DBG(args...) printk(KERN_DEBUG "ixgb: " args)
-#else
-#define IXGB_DBG(args...)
-#endif
-
#define PFX "ixgb: "
#define DPRINTK(nlevel, klevel, fmt, args...) \
(void)((NETIF_MSG_##nlevel & adapter->msg_enable) && \
diff --git a/drivers/net/ixgb/ixgb_main.c b/drivers/net/ixgb/ixgb_main.c
index e09f575..d063e84 100644
--- a/drivers/net/ixgb/ixgb_main.c
+++ b/drivers/net/ixgb/ixgb_main.c
@@ -1948,7 +1948,7 @@ #endif
/* All receives must fit into a single buffer */
- IXGB_DBG("Receive packet consumed multiple buffers "
+ pr_debug("Receive packet consumed multiple buffers "
"length<%x>\n", length);
dev_kfree_skb_irq(skb);
--
ok, one more time ...
rday
^ permalink raw reply related [flat|nested] 9+ messages in thread
end of thread, other threads:[~2006-10-10 18:23 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2006-10-10 12:19 [PATCH] ixgb: Delete IXGB_DBG() macro and call pr_debug() directly Robert P. J. Day
2006-10-10 16:14 ` Auke Kok
2006-10-10 16:36 ` Robert P. J. Day
2006-10-10 16:50 ` Auke Kok
2006-10-10 17:51 ` Robert P. J. Day
2006-10-10 18:18 ` Auke Kok
-- strict thread matches above, loose matches on Subject: below --
2006-10-10 11:59 Robert P. J. Day
2006-10-10 12:12 ` Andreas Schwab
2006-10-10 12:15 ` Robert P. J. Day
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).