linux-kernel - Re: [PATCH v6 4/4] drivers/hwmon/menf21bmc_hwmon: introduce MEN14F021P00 BMC HWMON driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20140828081643.GA27707@awelinux>
Date:	Thu, 28 Aug 2014 10:16:43 +0200
From:	Andreas Werner <andreas.werner@....de>
To:	Guenter Roeck <linux@...ck-us.net>
CC:	Andreas Werner <andreas.werner@....de>,
	<linux-kernel@...r.kernel.org>, <sameo@...ux.intel.com>,
	<lee.jones@...aro.org>, <lm-sensors@...sensors.org>,
	<wim@...ana.be>, <linux-watchdog@...r.kernel.org>,
	<cooloney@...il.com>, <rpurdie@...ys.net>,
	<linux-leds@...r.kernel.org>
Subject: Re: [PATCH v6 4/4] drivers/hwmon/menf21bmc_hwmon: introduce
 MEN14F021P00 BMC HWMON driver

On Wed, Aug 27, 2014 at 11:38:43AM -0700, Guenter Roeck wrote:
> On Wed, Aug 27, 2014 at 07:53:06PM +0200, Andreas Werner wrote:
> > Added driver to support the 14F021P00 BMC Hardware Monitoring.
> > The BMC is a Board Management Controller including monitoring of the
> > board voltages.
> > 
> > Signed-off-by: Andreas Werner <andreas.werner@....de>
> 
> Looks good, though I do have one comment ...
> 
> > +
> > +	hwmon_dev = devm_hwmon_device_register_with_groups(&pdev->dev,
> > +						   "menf21bmc", drv_data,
> > +						   menf21bmc_hwmon_groups);
> > +	if (IS_ERR(hwmon_dev))
> > +		return PTR_ERR(hwmon_dev);
> > +
> > +	dev_info(&pdev->dev, "MEN 14F021P00 BMC hwmon device enabled");
> > +
> You removed the dev_info from the mfd driver but left it in the client
> drivers.  If you'd drop it here as well, you could use
> 'return PTR_ERR_OR_ZERO(hwmon_dev);'.  Just a suggestion, not mandatory.
> 
> Either case,

It is a nice info for every slave driver which are the drivers for accessing
the features in the BMC. The mfd is just the driver who will instantiate the
slaves. For the mfd the most important info is if there was an error.

Anyway good idea with PTR_ERR_OR_ZERO.

Thanks for the nice review and the comments.

Regards
Andy

> 
> Reviewed-by: Guenter Roeck <linux@...ck-us.net>
> 
> I assume this will go in through the mfd tree.
> 
> Thanks,
> Guenter
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/