From: Auke Kok <auke-jan.h.kok@intel.com>
To: "Robert P. J. Day" <rpjday@mindspring.com>
Cc: Linux kernel mailing list <linux-kernel@vger.kernel.org>,
trivial@kernel.org, NetDev <netdev@vger.kernel.org>,
Jesse Brandeburg <jesse.brandeburg@intel.com>
Subject: Re: [PATCH] ixgb: Delete IXGB_DBG() macro and call pr_debug() directly.
Date: Tue, 10 Oct 2006 11:18:36 -0700 [thread overview]
Message-ID: <452BE3FC.5010001@intel.com> (raw)
In-Reply-To: <Pine.LNX.4.64.0610101347270.10344@localhost.localdomain>
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
prev parent reply other threads:[~2006-10-10 18:18 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <Pine.LNX.4.64.0610100816440.7711@localhost.localdomain>
[not found] ` <452BC6C9.3050902@intel.com>
[not found] ` <Pine.LNX.4.64.0610101227170.9699@localhost.localdomain>
2006-10-10 16:50 ` [PATCH] ixgb: Delete IXGB_DBG() macro and call pr_debug() directly Auke Kok
2006-10-10 17:51 ` Robert P. J. Day
2006-10-10 18:18 ` Auke Kok [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=452BE3FC.5010001@intel.com \
--to=auke-jan.h.kok@intel.com \
--cc=jesse.brandeburg@intel.com \
--cc=linux-kernel@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=rpjday@mindspring.com \
--cc=trivial@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).