[cgl_discussion] Re: [PATCH-RFC] README 1ST - New problem logging macros (2.5.38)
rusty at rustcorp.com.au
Mon Sep 23 22:55:42 PDT 2002
In message <3D8FD0A9.1010906 at pobox.com> you write:
> > @@ -325,7 +326,8 @@
> > while(inb(cmd_ioaddr) && --wait >= 0);
> > #ifndef final_version
> > if (wait < 0)
> > - printk(KERN_ALERT "eepro100: wait_for_cmd_done timeout!\n");
> > + problem(LOG_ALERT, "eepro100: wait_for_cmd_done timeout!",
> > + detail(ioaddr, "%lx", cmd_ioaddr));
> bloat, the ioaddr can easily be deduced
No! That's *exactly* the problem: you see:
eepro100: wait_for_cmd_done timeout!
in your logs, now *which* of the 5 eepro100 cards was it?
wait_for_cmd_done(long cmd_ioaddr) should take a 'struct net_device *'
and use net_problem, then no details needed.
> > + pci_introduce(pdev);
> bloat, we don't need foo_introduce() functions for every subsystem, when
> every subsystem always has an attach-new-device function.
Yes, this should be moved up into the generic pci/networking code, but I
guess they wanted a self-contained example.
> > - printk (KERN_ERR "eepro100: cannot reserve MMIO region\n");
> > + pci_problem(LOG_ERR, pdev, "eepro100: cannot reserve MMIO regio
> bloat, no advantage over printk
Now, which of those 5 cards was it again?
> > - printk(KERN_WARNING "%s: Invalid EEPROM checksum %#4.4x
> > - "check settings before activating this devic
> > - dev->name, sum);
> > + net_pci_problem(LOG_WARNING, dev, pdev, "Invalid EEPROM
> > + "check settings before activating this devic
> > + detail(checksum, "%#4.4x", sum));
> bloat, checksum is purely informational, and can be obtained through
> other means
It's a direct translation from the printk ferchissakes!
Anyone who quotes me in their sig is an idiot. -- Rusty Russell.
More information about the cgl_discussion