[lm-sensors] [PATCH V5] hwmon: Add MCP3021 ADC driver

Xie Xiaobo-R63061 r63061 at freescale.com
Thu Feb 23 07:48:51 CET 2012


Hi Jean,

Removing the printk is OK for me. Thank you very much.

BRs
Xie Xiaobo

> -----Original Message-----
> From: Jean Delvare [mailto:khali at linux-fr.org]
> Sent: 2012年2月23日 2:00
> To: guenter.roeck at ericsson.com
> Cc: Xie Xiaobo-R63061; lm-sensors at lm-sensors.org; Hu Mingkai-B21284
> Subject: Re: [PATCH V5] hwmon: Add MCP3021 ADC driver
> 
> On Wed, 22 Feb 2012 09:46:20 -0800, Guenter Roeck wrote:
> > On Wed, 2012-02-22 at 02:55 -0500, Xie Xiaobo wrote:
> > > Add I2C driver for MCP3021 that is an ADC chip from Microchip.
> > > The MCP3021 is a successive approximation A/D converter (ADC) with
> > > 10-bit resolution.
> > > The driver export the value of Vin to sysfs, the voltage unit is mV.
> > > Through the sysfs interface, lm-sensors tool can also display Vin
> > > voltage.
> > >
> > > Signed-off-by: Mingkai Hu <Mingkai.hu at freescale.com>
> > > Signed-off-by: Xie Xiaobo <X.Xie at freescale.com>
> >
> > [ ... ]
> >
> > > +
> > > +	if (reg < 0) {
> > > +		printk(KERN_ERR "%s encountered error %d\n", __func__, reg);
> >
> > You should use dev_err() here.
> 
> I don't think there's any point in printing an error message in the first place. I/O
> errors over I2C can happen, and the caller will get an error when it happens.
> "sensors" will display an error message and other monitoring applications would
> do too. Logging an error message is redundant IMHO.
> 
> If Xie agrees, I'll just remove the message, no need to resent. We've gone
> through 4 reviews, it's probably enough for such a small driver, it's about time to
> accept it for the next merge window. Patch applied (with minor changes), thanks.
> 
> --
> Jean Delvare



More information about the lm-sensors mailing list