[prev in list] [next in list] [prev in thread] [next in thread] 

List:       lm-sensors
Subject:    Re: [lm-sensors] [PATCH V5] hwmon: Add MCP3021 ADC driver
From:       Xie Xiaobo-R63061 <r63061 () freescale ! com>
Date:       2012-02-23 6:48:51
Message-ID: 69EC9ED88E3CC04094A78F8074A7986D2209E3 () 039-SN1MPN1-004 ! 039d ! mgd ! msft ! net
[Download RAW message or body]

Hi Jean,

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

BRs
Xie Xiaobo

> -----Original Message-----
> From: Jean Delvare [mailto:khali@linux-fr.org]
> Sent: 2012Äê2Ô 23ÈÕ 2:00
> To: guenter.roeck@ericsson.com
> Cc: Xie Xiaobo-R63061; lm-sensors@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@freescale.com>
> > > Signed-off-by: Xie Xiaobo <X.Xie@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

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
[prev in list] [next in list] [prev in thread] [next in thread]