| Message ID | 20200517003354.233373-21-marex@denx.de |
|---|---|
| State | Changes Requested |
| Delegated to: | David Miller |
| Headers | show |
| Series | net: ks8851: Unify KS8851 SPI and MLL drivers | expand |
On Sun, 2020-05-17 at 02:33 +0200, Marek Vasut wrote: > Drop those debug statements from both drivers. They were there since > at least 2011 and enabled by default, but that's likely wrong. [] > diff --git a/drivers/net/ethernet/micrel/ks8851_par.c b/drivers/net/ethernet/micrel/ks8851_par.c [] > -#define DEBUG Dropping the #define DEBUG lines will cause a behavior change for the netdev/netif_dbg uses as these messages will no longer be output by default.
On 5/17/20 4:01 AM, Joe Perches wrote: > On Sun, 2020-05-17 at 02:33 +0200, Marek Vasut wrote: >> Drop those debug statements from both drivers. They were there since >> at least 2011 and enabled by default, but that's likely wrong. > [] >> diff --git a/drivers/net/ethernet/micrel/ks8851_par.c b/drivers/net/ethernet/micrel/ks8851_par.c > [] >> -#define DEBUG > > Dropping the #define DEBUG lines will cause a behavior > change for the netdev/netif_dbg uses as these messages > will no longer be output by default. Is that a problem ?
On Sun, 2020-05-17 at 04:28 +0200, Marek Vasut wrote: > On 5/17/20 4:01 AM, Joe Perches wrote: > > On Sun, 2020-05-17 at 02:33 +0200, Marek Vasut wrote: > > > Drop those debug statements from both drivers. They were there since > > > at least 2011 and enabled by default, but that's likely wrong. > > [] > > > diff --git a/drivers/net/ethernet/micrel/ks8851_par.c b/drivers/net/ethernet/micrel/ks8851_par.c > > [] > > > -#define DEBUG > > > > Dropping the #define DEBUG lines will cause a behavior > > change for the netdev/netif_dbg uses as these messages > > will no longer be output by default. > > Is that a problem ? Dunno. I don't use nor debug these drivers. You just say that's likely wrong, but I wonder if that's really true. You also don't mention in your patch commit message that the output logging actually does change as if the DEBUG define has no effect. Prior to this change these were output at KERN_DEBUG $ git grep -A1 _dbg drivers/net/ethernet/micrel/ks8851.c drivers/net/ethernet/micrel/ks8851.c: netif_dbg(ks, hw, ks->netdev, "setting power mode %d\n", pwrmode); drivers/net/ethernet/micrel/ks8851.c- -- drivers/net/ethernet/micrel/ks8851.c: netif_dbg(ks, rx_status, ks->netdev, drivers/net/ethernet/micrel/ks8851.c- "%s: %d@%p\n", __func__, len, buff); -- drivers/net/ethernet/micrel/ks8851.c: * ks8851_dbg_dumpkkt - dump initial packet contents to debug drivers/net/ethernet/micrel/ks8851.c- * @ks: The device state -- drivers/net/ethernet/micrel/ks8851.c: * Dump the initial data from the packet to dev_dbg(). drivers/net/ethernet/micrel/ks8851.c-*/ drivers/net/ethernet/micrel/ks8851.c:static void ks8851_dbg_dumpkkt(struct ks8851_net *ks, u8 *rxpkt) drivers/net/ethernet/micrel/ks8851.c-{ drivers/net/ethernet/micrel/ks8851.c: netdev_dbg(ks->netdev, drivers/net/ethernet/micrel/ks8851.c- "pkt %02x%02x%02x%02x %02x%02x%02x%02x %02x%02x%02x%02x\n", -- drivers/net/ethernet/micrel/ks8851.c: netif_dbg(ks, rx_status, ks->netdev, drivers/net/ethernet/micrel/ks8851.c- "%s: %d packets\n", __func__, rxfc); -- drivers/net/ethernet/micrel/ks8851.c: netif_dbg(ks, rx_status, ks->netdev, drivers/net/ethernet/micrel/ks8851.c- "rx: stat 0x%04x, len 0x%04x\n", rxstat, rxlen); -- drivers/net/ethernet/micrel/ks8851.c: ks8851_dbg_dumpkkt(ks, rxpkt); drivers/net/ethernet/micrel/ks8851.c- -- drivers/net/ethernet/micrel/ks8851.c: netif_dbg(ks, intr, ks->netdev, drivers/net/ethernet/micrel/ks8851.c- "%s: status 0x%04x\n", __func__, status); -- drivers/net/ethernet/micrel/ks8851.c: netif_dbg(ks, intr, ks->netdev, drivers/net/ethernet/micrel/ks8851.c- "%s: txspace %d\n", __func__, ks->tx_space); -- drivers/net/ethernet/micrel/ks8851.c: netif_dbg(ks, tx_queued, ks->netdev, "%s: skb %p, %d@%p, irq %d\n", drivers/net/ethernet/micrel/ks8851.c- __func__, txp, txp->len, txp->data, irq); -- drivers/net/ethernet/micrel/ks8851.c: netif_dbg(ks, ifup, ks->netdev, "opening\n"); drivers/net/ethernet/micrel/ks8851.c- -- drivers/net/ethernet/micrel/ks8851.c: netif_dbg(ks, ifup, ks->netdev, "network device up\n"); drivers/net/ethernet/micrel/ks8851.c- -- drivers/net/ethernet/micrel/ks8851.c: netif_dbg(ks, ifdown, ks->netdev, drivers/net/ethernet/micrel/ks8851.c- "%s: freeing txb %p\n", __func__, txb); -- drivers/net/ethernet/micrel/ks8851.c: netif_dbg(ks, tx_queued, ks->netdev, drivers/net/ethernet/micrel/ks8851.c- "%s: skb %p, %d@%p\n", __func__, skb, skb->len, skb->data);
On 5/17/20 4:37 AM, Joe Perches wrote: > On Sun, 2020-05-17 at 04:28 +0200, Marek Vasut wrote: >> On 5/17/20 4:01 AM, Joe Perches wrote: >>> On Sun, 2020-05-17 at 02:33 +0200, Marek Vasut wrote: >>>> Drop those debug statements from both drivers. They were there since >>>> at least 2011 and enabled by default, but that's likely wrong. >>> [] >>>> diff --git a/drivers/net/ethernet/micrel/ks8851_par.c b/drivers/net/ethernet/micrel/ks8851_par.c >>> [] >>>> -#define DEBUG >>> >>> Dropping the #define DEBUG lines will cause a behavior >>> change for the netdev/netif_dbg uses as these messages >>> will no longer be output by default. >> >> Is that a problem ? > > Dunno. I don't use nor debug these drivers. > I don't use those debug messages either, so it's not a problem for me.
On Sun, 2020-05-17 at 04:47 +0200, Marek Vasut wrote: > On 5/17/20 4:37 AM, Joe Perches wrote: > > On Sun, 2020-05-17 at 04:28 +0200, Marek Vasut wrote: > > > On 5/17/20 4:01 AM, Joe Perches wrote: > > > > On Sun, 2020-05-17 at 02:33 +0200, Marek Vasut wrote: > > > > > Drop those debug statements from both drivers. They were there since > > > > > at least 2011 and enabled by default, but that's likely wrong. > > > > [] > > > > > diff --git a/drivers/net/ethernet/micrel/ks8851_par.c b/drivers/net/ethernet/micrel/ks8851_par.c > > > > [] > > > > > -#define DEBUG > > > > > > > > Dropping the #define DEBUG lines will cause a behavior > > > > change for the netdev/netif_dbg uses as these messages > > > > will no longer be output by default. > > > > > > Is that a problem ? > > > > Dunno. I don't use nor debug these drivers. > > > > I don't use those debug messages either, so it's not a problem for me. Just mention it in the changelog please.
On 5/17/20 4:55 AM, Joe Perches wrote: > On Sun, 2020-05-17 at 04:47 +0200, Marek Vasut wrote: >> On 5/17/20 4:37 AM, Joe Perches wrote: >>> On Sun, 2020-05-17 at 04:28 +0200, Marek Vasut wrote: >>>> On 5/17/20 4:01 AM, Joe Perches wrote: >>>>> On Sun, 2020-05-17 at 02:33 +0200, Marek Vasut wrote: >>>>>> Drop those debug statements from both drivers. They were there since >>>>>> at least 2011 and enabled by default, but that's likely wrong. >>>>> [] >>>>>> diff --git a/drivers/net/ethernet/micrel/ks8851_par.c b/drivers/net/ethernet/micrel/ks8851_par.c >>>>> [] >>>>>> -#define DEBUG >>>>> >>>>> Dropping the #define DEBUG lines will cause a behavior >>>>> change for the netdev/netif_dbg uses as these messages >>>>> will no longer be output by default. >>>> >>>> Is that a problem ? >>> >>> Dunno. I don't use nor debug these drivers. >>> >> >> I don't use those debug messages either, so it's not a problem for me. > > Just mention it in the changelog please. Or let's just drop this patch for now. I am still waiting for a clear direction on the indirect accessors.
diff --git a/drivers/net/ethernet/micrel/ks8851_par.c b/drivers/net/ethernet/micrel/ks8851_par.c index 949c7a876f9a..ae9943f37408 100644 --- a/drivers/net/ethernet/micrel/ks8851_par.c +++ b/drivers/net/ethernet/micrel/ks8851_par.c @@ -6,10 +6,6 @@ * Ben Dooks <ben@simtec.co.uk> */ -#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt - -#define DEBUG - #include <linux/interrupt.h> #include <linux/module.h> #include <linux/kernel.h> diff --git a/drivers/net/ethernet/micrel/ks8851_spi.c b/drivers/net/ethernet/micrel/ks8851_spi.c index 4ec7f1615977..f7ceebba7db0 100644 --- a/drivers/net/ethernet/micrel/ks8851_spi.c +++ b/drivers/net/ethernet/micrel/ks8851_spi.c @@ -6,10 +6,6 @@ * Ben Dooks <ben@simtec.co.uk> */ -#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt - -#define DEBUG - #include <linux/interrupt.h> #include <linux/module.h> #include <linux/kernel.h>
Drop those debug statements from both drivers. They were there since at least 2011 and enabled by default, but that's likely wrong. Signed-off-by: Marek Vasut <marex@denx.de> Cc: David S. Miller <davem@davemloft.net> Cc: Lukas Wunner <lukas@wunner.de> Cc: Petr Stetiar <ynezz@true.cz> Cc: YueHaibing <yuehaibing@huawei.com> --- V6: New patch --- drivers/net/ethernet/micrel/ks8851_par.c | 4 ---- drivers/net/ethernet/micrel/ks8851_spi.c | 4 ---- 2 files changed, 8 deletions(-)