| Message ID | 1290887140.22971.16.camel@Joe-Laptop |
|---|---|
| State | RFC, archived |
| Delegated to: | David Miller |
| Headers | show |
2010/11/27 Joe Perches <joe@perches.com>: > On Sat, 2010-11-27 at 19:04 +0000, Ben Hutchings wrote: >> On Sat, 2010-11-27 at 19:39 +0100, Szymon Janc wrote: >> > diff --git a/drivers/net/forcedeth.c b/drivers/net/forcedeth.c >> > index 2f092d7..a2b6681 100644 >> > --- a/drivers/net/forcedeth.c >> > +++ b/drivers/net/forcedeth.c >> > @@ -958,7 +958,7 @@ static int reg_delay(struct net_device *dev, int offset, u32 mask, u32 target, >> > delaymax -= delay; >> > if (delaymax < 0) { >> > if (msg) >> > - printk("%s", msg); >> > + printk(KERN_WARNING "%s", msg); >> No, msg already includes a log level. > > True. The messages are still broken though. > Some have trailing newlines, others not. > > It'd be better to move the msg after the reg_delay > call and add the missing newlines. > [...] > diff --git a/drivers/net/forcedeth.c b/drivers/net/forcedeth.c > index 0fa1776..2d11028f 100644 > --- a/drivers/net/forcedeth.c > +++ b/drivers/net/forcedeth.c [...] > @@ -1547,9 +1544,9 @@ static void nv_stop_rx(struct net_device *dev) > else > rx_ctrl |= NVREG_RCVCTL_RX_PATH_EN; > writel(rx_ctrl, base + NvRegReceiverControl); > - reg_delay(dev, NvRegReceiverStatus, NVREG_RCVSTAT_BUSY, 0, > - NV_RXSTOP_DELAY1, NV_RXSTOP_DELAY1MAX, > - KERN_INFO "nv_stop_rx: ReceiverStatus remained busy"); > + if (reg_delay(dev, NvRegReceiverStatus, NVREG_RCVSTAT_BUSY, 0, > + NV_RXSTOP_DELAY1, NV_RXSTOP_DELAY1MAX)) > + printk(KERN_INFO "nv_stop_rx: ReceiverStatus remained busy\n"); You could change it to dev_info() and friends in one go. > udelay(NV_RXSTOP_DELAY2); > if (!np->mac_in_use) [...] Best Regards, Michał Mirosław -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sun, 2010-11-28 at 10:05 +0100, Michał Mirosław wrote: > 2010/11/27 Joe Perches <joe@perches.com>: > > On Sat, 2010-11-27 at 19:04 +0000, Ben Hutchings wrote: > >> On Sat, 2010-11-27 at 19:39 +0100, Szymon Janc wrote: > >> > diff --git a/drivers/net/forcedeth.c b/drivers/net/forcedeth.c > >> > index 2f092d7..a2b6681 100644 > >> > @@ -958,7 +958,7 @@ static int reg_delay(struct net_device *dev, int offset, u32 mask, u32 target, [] > >> > - printk("%s", msg); > >> > + printk(KERN_WARNING "%s", msg); > >> No, msg already includes a log level. > > True. The messages are still broken though. > > Some have trailing newlines, others not. > > It'd be better to move the msg after the reg_delay > > call and add the missing newlines. > [...] > > diff --git a/drivers/net/forcedeth.c b/drivers/net/forcedeth.c > > @@ -1547,9 +1544,9 @@ static void nv_stop_rx(struct net_device *dev) [] > > - reg_delay(dev, NvRegReceiverStatus, NVREG_RCVSTAT_BUSY, 0, > > - NV_RXSTOP_DELAY1, NV_RXSTOP_DELAY1MAX, > > - KERN_INFO "nv_stop_rx: ReceiverStatus remained busy"); > > + if (reg_delay(dev, NvRegReceiverStatus, NVREG_RCVSTAT_BUSY, 0, > > + NV_RXSTOP_DELAY1, NV_RXSTOP_DELAY1MAX)) > > + printk(KERN_INFO "nv_stop_rx: ReceiverStatus remained busy\n"); > You could change it to dev_info() and friends in one go. Yeah, I did that in a local tree for all logging calls, (dprintk->netdev_dbg, printk->netdev_<level> etc) but I'll wait until Szymon's patches are applied or nacked. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/net/forcedeth.c b/drivers/net/forcedeth.c index 0fa1776..2d11028f 100644 --- a/drivers/net/forcedeth.c +++ b/drivers/net/forcedeth.c @@ -948,7 +948,7 @@ static bool nv_optimized(struct fe_priv *np) } static int reg_delay(struct net_device *dev, int offset, u32 mask, u32 target, - int delay, int delaymax, const char *msg) + int delay, int delaymax) { u8 __iomem *base = get_hwbase(dev); @@ -956,11 +956,8 @@ static int reg_delay(struct net_device *dev, int offset, u32 mask, u32 target, do { udelay(delay); delaymax -= delay; - if (delaymax < 0) { - if (msg) - printk("%s", msg); + if (delaymax < 0) return 1; - } } while ((readl(base + offset) & mask) != target); return 0; } @@ -1145,7 +1142,7 @@ static int mii_rw(struct net_device *dev, int addr, int miireg, int value) writel(reg, base + NvRegMIIControl); if (reg_delay(dev, NvRegMIIControl, NVREG_MIICTL_INUSE, 0, - NV_MIIPHY_DELAY, NV_MIIPHY_DELAYMAX, NULL)) { + NV_MIIPHY_DELAY, NV_MIIPHY_DELAYMAX)) { dprintk(KERN_DEBUG "%s: mii_rw of reg %d at PHY %d timed out.\n", dev->name, miireg, addr); retval = -1; @@ -1547,9 +1544,9 @@ static void nv_stop_rx(struct net_device *dev) else rx_ctrl |= NVREG_RCVCTL_RX_PATH_EN; writel(rx_ctrl, base + NvRegReceiverControl); - reg_delay(dev, NvRegReceiverStatus, NVREG_RCVSTAT_BUSY, 0, - NV_RXSTOP_DELAY1, NV_RXSTOP_DELAY1MAX, - KERN_INFO "nv_stop_rx: ReceiverStatus remained busy"); + if (reg_delay(dev, NvRegReceiverStatus, NVREG_RCVSTAT_BUSY, 0, + NV_RXSTOP_DELAY1, NV_RXSTOP_DELAY1MAX)) + printk(KERN_INFO "nv_stop_rx: ReceiverStatus remained busy\n"); udelay(NV_RXSTOP_DELAY2); if (!np->mac_in_use) @@ -1582,9 +1579,9 @@ static void nv_stop_tx(struct net_device *dev) else tx_ctrl |= NVREG_XMITCTL_TX_PATH_EN; writel(tx_ctrl, base + NvRegTransmitterControl); - reg_delay(dev, NvRegTransmitterStatus, NVREG_XMITSTAT_BUSY, 0, - NV_TXSTOP_DELAY1, NV_TXSTOP_DELAY1MAX, - KERN_INFO "nv_stop_tx: TransmitterStatus remained busy"); + if (reg_delay(dev, NvRegTransmitterStatus, NVREG_XMITSTAT_BUSY, 0, + NV_TXSTOP_DELAY1, NV_TXSTOP_DELAY1MAX)) + printk(KERN_INFO "nv_stop_tx: TransmitterStatus remained busy\n"); udelay(NV_TXSTOP_DELAY2); if (!np->mac_in_use) @@ -5216,9 +5213,10 @@ static int nv_open(struct net_device *dev) writel(np->vlanctl_bits, base + NvRegVlanControl); pci_push(base); writel(NVREG_TXRXCTL_BIT1|np->txrxctl_bits, base + NvRegTxRxControl); - reg_delay(dev, NvRegUnknownSetupReg5, NVREG_UNKSETUP5_BIT31, NVREG_UNKSETUP5_BIT31, - NV_SETUP5_DELAY, NV_SETUP5_DELAYMAX, - KERN_INFO "open: SetupReg5, Bit 31 remained off\n"); + if (reg_delay(dev, NvRegUnknownSetupReg5, + NVREG_UNKSETUP5_BIT31, NVREG_UNKSETUP5_BIT31, + NV_SETUP5_DELAY, NV_SETUP5_DELAYMAX)) + printk(KERN_INFO "open: SetupReg5, Bit 31 remained off\n"); writel(0, base + NvRegMIIMask); writel(NVREG_IRQSTAT_MASK, base + NvRegIrqStatus);