Hi Jean, Removing the printk is OK for me. Thank you very much. BRs Xie Xiaobo > -----Original Message----- > From: Jean Delvare [mailto:khali@xxxxxxxxxxxx] > Sent: 2012年2月23日 2:00 > To: guenter.roeck@xxxxxxxxxxxx > Cc: Xie Xiaobo-R63061; lm-sensors@xxxxxxxxxxxxxx; 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@xxxxxxxxxxxxx> > > > Signed-off-by: Xie Xiaobo <X.Xie@xxxxxxxxxxxxx> > > > > [ ... ] > > > > > + > > > + 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@xxxxxxxxxxxxxx http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
