Re: [PATCH 2/2] staging: brcm80211: align message upon driver load with Kconfig driver name

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sun, Jan 23, 2011 at 08:30:39AM +0100, Arend van Spriel wrote:
> Both SoftMAC and FullMAC driver print information upon loading containing
> description and version number. The descriptive text has been aligned with
> the descriptive text used in Kconfig.

Better, but:

> @@ -2103,7 +2097,8 @@ dhd_pub_t *dhd_attach(struct osl_info *osh, struct dhd_bus *bus,
>  	dhd->early_suspend.resume = dhd_late_resume;
>  	register_early_suspend(&dhd->early_suspend);
>  #endif
> -
> +	printk(KERN_DEFAULT "Broadcom IEEE802.11n embedded FullMAC WLAN ");
> +	printk(KERN_DEFAULT "Driver, version " EPI_VERSION_STR "\n");

Why print this at all?  And even worse, why make it two lines long?  Can
the kernel log somehow not handle longer-than-60-characters lines?  What
happens if you get a different kernel thread printing a line inbetween
those two lines?  It's a mess, that's what.

Oh, and I'm guessing that you never even tested this as it really does
not print out what you think it does.  Which makes this patch even worse
on so many other levels.

Please, just drop this entirely.  Drivers don't need to advertise
themselves to the world in the kernel log.  Ones that do need to get
that line removed as no one cares about it (seriously, no one does).
There are easier, better, and standardized ways of getting the version
number of the loaded drivers from the system without having to spelunk
through the kernel log files.

> --- a/drivers/staging/brcm80211/brcmsmac/wl_mac80211.c
> +++ b/drivers/staging/brcm80211/brcmsmac/wl_mac80211.c
> @@ -738,18 +738,13 @@ static struct wl_info *wl_attach(u16 vendor, u16 device, unsigned long regs,
>  		WL_ERROR("%s: regulatory_hint failed, status %d\n",
>  			 __func__, err);
>  	}
> -	WL_ERROR("wl%d: Broadcom BCM43xx 802.11 MAC80211 Driver (" PHY_VERSION_STR ")",
> -		 unit);
> -
> -#ifdef BCMDBG
> -	printf(" (Compiled at " __TIME__ " on " __DATE__ ")");
> -#endif				/* BCMDBG */
> -	printf("\n");
> +	printk(KERN_DEFAULT "wl%d: Broadcom IEEE802.11n PCIe SoftMAC WLAN "
> +	       "Driver, version " PHY_VERSION_STR "\n", unit);

Same here, just don't.  At least you didn't print it out on 2 lines, but
still, it's not needed at all.

thanks,

greg k-h
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/devel


[Index of Archives]     [Linux Driver Backports]     [DMA Engine]     [Linux GPIO]     [Linux SPI]     [Video for Linux]     [Linux USB Devel]     [Linux Coverity]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux