| Message ID | 20200914210128.7741-1-W_Armin@gmx.de |
|---|---|
| Headers | show |
| Series | 8390: core cleanups | expand |
From: Armin Wolf <W_Armin@gmx.de> Date: Mon, 14 Sep 2020 23:01:22 +0200 > The purpose of this patchset is to do some > cleanups in lib8390.c and 8390.c A lot of these changes are borderline beneficial, at best. You are adding include files to foo.c files that are already included by lib8390.c already (which the foo.c file includes). This is redundant and makes the compiler work harder. lib8390.c is designed to work like this. Your first patch mixes comment formatting with actual code changes (adding new curly braces and such). And so on and so forth... I honestly don't like this patch series at all. If you were about to add a big new feature to the 8390 code and wanted to clean it up first before doing so, maybe I'd be ok with these changes. But as a pure cleanup, sorry I'm not going to apply this stuff.
On Mon, 2020-09-14 at 23:01 +0200, Armin Wolf wrote: > Replace pr_cont() with SMP-safe construct. > > Signed-off-by: Armin Wolf <W_Armin@gmx.de> > --- > drivers/net/ethernet/8390/lib8390.c | 31 +++++++++++------------------ > 1 file changed, 12 insertions(+), 19 deletions(-) > > diff --git a/drivers/net/ethernet/8390/lib8390.c b/drivers/net/ethernet/8390/lib8390.c > index 3a2b1e33a47a..e8a323352c40 100644 > --- a/drivers/net/ethernet/8390/lib8390.c > +++ b/drivers/net/ethernet/8390/lib8390.c > @@ -518,25 +518,18 @@ static void ei_tx_err(struct net_device *dev) > { > unsigned long e8390_base = dev->base_addr; > /* ei_local is used on some platforms via the EI_SHIFT macro */ > - struct ei_device *ei_local __maybe_unused = netdev_priv(dev); > - unsigned char txsr = ei_inb_p(e8390_base+EN0_TSR); > - unsigned char tx_was_aborted = txsr & (ENTSR_ABT+ENTSR_FU); > - > -#ifdef VERBOSE_ERROR_DUMP > - netdev_dbg(dev, "transmitter error (%#2x):", txsr); > - if (txsr & ENTSR_ABT) > - pr_cont(" excess-collisions "); > - if (txsr & ENTSR_ND) > - pr_cont(" non-deferral "); > - if (txsr & ENTSR_CRS) > - pr_cont(" lost-carrier "); > - if (txsr & ENTSR_FU) > - pr_cont(" FIFO-underrun "); > - if (txsr & ENTSR_CDH) > - pr_cont(" lost-heartbeat "); > - pr_cont("\n"); > -#endif > - > + struct ei_device *ei_local = netdev_priv(dev); > + unsigned char txsr = ei_inb_p(e8390_base + EN0_TSR); > + unsigned char tx_was_aborted = txsr & (ENTSR_ABT + ENTSR_FU); > + > + if (netif_msg_tx_err(ei_local)) { > + netdev_err(dev, "Transmitter error %#2x ( %s%s%s%s%s)", txsr, > + (txsr & ENTSR_ABT) ? "excess-collisions " : "", > + (txsr & ENTSR_ND) ? "non-deferral " : "", > + (txsr & ENTSR_CRS) ? "lost-carrier " : "", > + (txsr & ENTSR_FU) ? "FIFO-underrun " : "", > + (txsr & ENTSR_CDH) ? "lost-heartbeat " : ""); > + } Still should use a terminating '\n' and likely this might be better as: netif_dbg(ei_local, tx_err, dev, "Transmitter error ...\n", etc...);