Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752507AbZI1S2L (ORCPT ); Mon, 28 Sep 2009 14:28:11 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1751901AbZI1S2L (ORCPT ); Mon, 28 Sep 2009 14:28:11 -0400 Received: from mail.perches.com ([173.55.12.10]:1240 "EHLO mail.perches.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751651AbZI1S2K (ORCPT ); Mon, 28 Sep 2009 14:28:10 -0400 Subject: Re: [PATCH] LED driver for Intel NAS SS4200 series (v3) From: Joe Perches To: Dave Hansen Cc: Andrew Morton , rpurdie@rpsys.net, linux-kernel@vger.kernel.org, rgirod@confocus.com, Arjan van de Ven In-Reply-To: <1254160310.17303.20180.camel@nimitz> References: <1253118732-11944-1-git-send-email-dave@sr71.net> <20090924171929.8ce89786.akpm@linux-foundation.org> <1254160310.17303.20180.camel@nimitz> Content-Type: text/plain; charset="UTF-8" Date: Mon, 28 Sep 2009 11:28:13 -0700 Message-Id: <1254162493.28232.235.camel@Joe-Laptop.home> Mime-Version: 1.0 X-Mailer: Evolution 2.28.0 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1695 Lines: 54 On Mon, 2009-09-28 at 10:51 -0700, Dave Hansen wrote: > On Thu, 2009-09-24 at 17:19 -0700, Andrew Morton wrote: Trivial logging comments: One of the printks is not prefaced and the printks and dev_printks are not consistently using either periods or no periods. Some possible substitutions: s/dev_printk(KERN_INFO/dev_info(/ s/dev_printk(KERN_DEBUG/dev_dbg(/ s/printk(KERN_INFO/pr_info(/ I think it would be better to use: #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt and remove the "%s: " ... KBUILD_MODNAME >+ printk(KERN_INFO KBUILD_MODNAME ": '%s' found\n", id->ident); Perhaps pr_info("Found a '%s'\n", id->ident); > + printk(KERN_DEBUG "setting blue off and amber on\n"); pr_debug? > + printk(KERN_INFO "%s: skipping hardware autodetection\n", > + KBUILD_MODNAME); > + printk(KERN_INFO "\tif this works, please send the output "); > + printk(KERN_INFO "of 'dmidecode' to dave@sr71.net\n"); I think this will look odd when printed. There's a tab on the second line, not on third. Perhaps: pr_info("Skipping hardware detection.\n"); pr_info("Please send 'dmidecode' output to dave@sr71.net\n"); > + printk(KERN_INFO "%s: no LED devices found\n", > + KBUILD_MODNAME); > + printk(KERN_INFO "registering %s PCI driver\n", KBUILD_MODNAME); > + printk(KERN_INFO "Unregistering %s driver\n", KBUILD_MODNAME); pr_info("No LED devices found\n"); pr_info("Registering PCI driver\n"); pr_info("Unregistering PCI driver\n"); -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/