Re: [PATCH 1/4] stmmac: replace all pr_xxx by their dev_xxx counterpart
On Wed, Sep 09, 2015 at 09:14:42AM -0700, Joe Perches wrote:
> On Wed, 2015-09-09 at 15:14 +0200, LABBE Corentin wrote:
> > The stmmac driver use lots of pr_xxx functions to print information.
> > This is bad since we cannot know which device logs the information.
> > (moreover if two stmmac device are present)
> []
> > So this patch replace all pr_xxx by their dev_xxx counterpart.
>
> Using
> netdev_(priv->dev, ...
> instead of
> dev_(priv->device,
>
> would be more consistent with other ethernet devices.
>
> > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> > b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> []
> > @@ -298,7 +298,7 @@ bool stmmac_eee_init(struct stmmac_priv *priv)
> > */
> > spin_lock_irqsave(&priv->lock, flags);
> > if (priv->eee_active) {
> > - pr_debug("stmmac: disable EEE\n");
> > + dev_dbg(priv->device, "disable EEE\n");
>
> netdev_dbg(priv->dev, ...)
>
> > @@ -657,10 +657,10 @@ static int stmmac_init_ptp(struct stmmac_priv *priv)
> > priv->adv_ts = 1;
> >
> > if (netif_msg_hw(priv) && priv->dma_cap.time_stamp)
> > - pr_debug("IEEE 1588-2002 Time Stamp supported\n");
> > + dev_dbg(priv->device, "IEEE 1588-2002 Time Stamp supported\n");
>
> And these netif_msg_ could be
>
> if (priv->dma_cap.timestamp)
> netif_dbg(priv, hw, priv->dev, ...);
>
>
Hello
My main goal is to improve logging from
[0.796804] stmmaceth 1c5.ethernet: no reset control found
[0.802635] Ring mode enabled
[0.805713] No HW DMA feature register supported
[0.810239] Normal descriptors
[0.813577] TX Checksum insertion supported
[ 23.615074] eth0: device MAC address aa:65:84:d5:a3:58
[ 23.704326] RX IPC Checksum Offload disabled
[ 23.704349] No MAC Management Counters available
to that:
[0.788147] sun7i-dwmac 1c5.ethernet (unnamed net_device)
(uninitialized): no reset control found
[0.797400] sun7i-dwmac 1c5.ethernet (unnamed net_device)
(uninitialized): Ring mode enabled
[0.806211] sun7i-dwmac 1c5.ethernet (unnamed net_device)
(uninitialized): No HW DMA feature register supported
[0.816658] sun7i-dwmac 1c5.ethernet (unnamed net_device)
(uninitialized): Normal descriptors
[0.825522] sun7i-dwmac 1c5.ethernet (unnamed net_device)
(uninitialized): TX Checksum insertion supported
[ 12.971725] sun7i-dwmac 1c5.ethernet eth0: device MAC address
3e:62:18:6f:c7:f4
[ 13.056902] sun7i-dwmac 1c5.ethernet eth0: RX IPC Checksum Offload
disabled
[ 13.056929] sun7i-dwmac 1c5.ethernet eth0: No MAC Management Counters
available
But by using the netdev_ functions the first five lines are not "pretty" with
the "(unnamed net_device) (uninitialized)"
Could I switch back do dev_xxx since they are "early device logging" and so
make it prettier ?
Best regards
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/4] stmmac: replace all pr_xxx by their dev_xxx counterpart
On Wed, Sep 09, 2015 at 09:14:42AM -0700, Joe Perches wrote:
> On Wed, 2015-09-09 at 15:14 +0200, LABBE Corentin wrote:
> > The stmmac driver use lots of pr_xxx functions to print information.
> > This is bad since we cannot know which device logs the information.
> > (moreover if two stmmac device are present)
> []
> > So this patch replace all pr_xxx by their dev_xxx counterpart.
>
> Using
> netdev_(priv->dev, ...
> instead of
> dev_(priv->device,
>
> would be more consistent with other ethernet devices.
>
Ok
> > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> > b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> []
> > @@ -298,7 +298,7 @@ bool stmmac_eee_init(struct stmmac_priv *priv)
> > */
> > spin_lock_irqsave(&priv->lock, flags);
> > if (priv->eee_active) {
> > - pr_debug("stmmac: disable EEE\n");
> > + dev_dbg(priv->device, "disable EEE\n");
>
> netdev_dbg(priv->dev, ...)
>
> > @@ -657,10 +657,10 @@ static int stmmac_init_ptp(struct stmmac_priv *priv)
> > priv->adv_ts = 1;
> >
> > if (netif_msg_hw(priv) && priv->dma_cap.time_stamp)
> > - pr_debug("IEEE 1588-2002 Time Stamp supported\n");
> > + dev_dbg(priv->device, "IEEE 1588-2002 Time Stamp supported\n");
>
> And these netif_msg_ could be
>
> if (priv->dma_cap.timestamp)
> netif_dbg(priv, hw, priv->dev, ...);
>
>
Thanks for the tip, I will add a patch for replacing that.
Regards
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/4] stmmac: replace all pr_xxx by their dev_xxx counterpart
On Wed, 2015-09-09 at 15:14 +0200, LABBE Corentin wrote:
> The stmmac driver use lots of pr_xxx functions to print information.
> This is bad since we cannot know which device logs the information.
> (moreover if two stmmac device are present)
[]
> So this patch replace all pr_xxx by their dev_xxx counterpart.
Using
netdev_(priv->dev, ...
instead of
dev_(priv->device,
would be more consistent with other ethernet devices.
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
[]
> @@ -298,7 +298,7 @@ bool stmmac_eee_init(struct stmmac_priv *priv)
>*/
> spin_lock_irqsave(&priv->lock, flags);
> if (priv->eee_active) {
> - pr_debug("stmmac: disable EEE\n");
> + dev_dbg(priv->device, "disable EEE\n");
netdev_dbg(priv->dev, ...)
> @@ -657,10 +657,10 @@ static int stmmac_init_ptp(struct stmmac_priv *priv)
> priv->adv_ts = 1;
>
> if (netif_msg_hw(priv) && priv->dma_cap.time_stamp)
> - pr_debug("IEEE 1588-2002 Time Stamp supported\n");
> + dev_dbg(priv->device, "IEEE 1588-2002 Time Stamp supported\n");
And these netif_msg_ could be
if (priv->dma_cap.timestamp)
netif_dbg(priv, hw, priv->dev, ...);
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/4] stmmac: replace all pr_xxx by their dev_xxx counterpart
The stmmac driver use lots of pr_xxx functions to print information.
This is bad since we cannot know which device logs the information.
(moreover if two stmmac device are present)
Furthermore, it seems that it assumes wrongly that all logs will always
be subsequent by using a dev_xxx then some indented pr_xxx like this:
kernel: sun7i-dwmac 1c5.ethernet: no reset control found
kernel: Ring mode enabled
kernel: No HW DMA feature register supported
kernel: Normal descriptors
kernel: TX Checksum insertion supported
So this patch replace all pr_xxx by their dev_xxx counterpart.
In the same time I remove some "stmmac:" print since
this will be a duplicate with that dev_xxx displays.
And this patch also change some pr_info by dev_err when
the word ERROR is printed.
Signed-off-by: LABBE Corentin
---
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 197 --
1 file changed, 112 insertions(+), 85 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 864b476..cb9efb3 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -298,7 +298,7 @@ bool stmmac_eee_init(struct stmmac_priv *priv)
*/
spin_lock_irqsave(&priv->lock, flags);
if (priv->eee_active) {
- pr_debug("stmmac: disable EEE\n");
+ dev_dbg(priv->device, "disable EEE\n");
del_timer_sync(&priv->eee_ctrl_timer);
priv->hw->mac->set_eee_timer(priv->hw, 0,
tx_lpi_timer);
@@ -327,7 +327,7 @@ bool stmmac_eee_init(struct stmmac_priv *priv)
ret = true;
spin_unlock_irqrestore(&priv->lock, flags);
- pr_debug("stmmac: Energy-Efficient Ethernet initialized\n");
+ dev_dbg(priv->device, "Energy-Efficient Ethernet
initialized\n");
}
out:
return ret;
@@ -448,8 +448,8 @@ static int stmmac_hwtstamp_ioctl(struct net_device *dev,
struct ifreq *ifr)
sizeof(struct hwtstamp_config)))
return -EFAULT;
- pr_debug("%s config flags:0x%x, tx_type:0x%x, rx_filter:0x%x\n",
-__func__, config.flags, config.tx_type, config.rx_filter);
+ dev_dbg(priv->device, "%s config flags:0x%x, tx_type:0x%x,
rx_filter:0x%x\n",
+ __func__, config.flags, config.tx_type, config.rx_filter);
/* reserved for future extensions */
if (config.flags)
@@ -657,10 +657,10 @@ static int stmmac_init_ptp(struct stmmac_priv *priv)
priv->adv_ts = 1;
if (netif_msg_hw(priv) && priv->dma_cap.time_stamp)
- pr_debug("IEEE 1588-2002 Time Stamp supported\n");
+ dev_dbg(priv->device, "IEEE 1588-2002 Time Stamp supported\n");
if (netif_msg_hw(priv) && priv->adv_ts)
- pr_debug("IEEE 1588-2008 Advanced Time Stamp supported\n");
+ dev_dbg(priv->device, "IEEE 1588-2008 Advanced Time Stamp
supported\n");
priv->hw->ptp = &stmmac_ptp;
priv->hwts_tx_en = 0;
@@ -740,8 +740,9 @@ static void stmmac_adjust_link(struct net_device *dev)
break;
default:
if (netif_msg_link(priv))
- pr_warn("%s: Speed (%d) not 10/100\n",
- dev->name, phydev->speed);
+ dev_warn(priv->device,
+"%s: Speed (%d) not 10/100\n",
+dev->name, phydev->speed);
break;
}
@@ -788,10 +789,10 @@ static void stmmac_check_pcs_mode(struct stmmac_priv
*priv)
(interface == PHY_INTERFACE_MODE_RGMII_ID) ||
(interface == PHY_INTERFACE_MODE_RGMII_RXID) ||
(interface == PHY_INTERFACE_MODE_RGMII_TXID)) {
- pr_debug("STMMAC: PCS RGMII support enable\n");
+ dev_dbg(priv->device, "PCS RGMII support enable\n");
priv->pcs = STMMAC_PCS_RGMII;
} else if (interface == PHY_INTERFACE_MODE_SGMII) {
- pr_debug("STMMAC: PCS SGMII support enable\n");
+ dev_dbg(priv->device, "PCS SGMII support enable\n");
priv->pcs = STMMAC_PCS_SGMII;
}
}
@@ -830,15 +831,16 @@ static int stmmac_init_phy(struct net_device *dev)
snprintf(phy_id_fmt, MII_BUS_ID_SIZE + 3, PHY_ID_FMT, bus_id,
priv->plat->phy_addr);
- pr_debug("stmmac_init_phy: trying to attach
