Re: [lm-sensors] [PATCH v3] hwmon/mc13xxx-adc: add support for the MC13892 PMIC

[Guenter Roeck <guenter.roeck@ericsson.com>]

On Mon, Jan 30, 2012 at 03:45:02AM -0500, Jean Delvare wrote:
> Hi Uwe,
> 
> On Sat, 28 Jan 2012 21:55:07 +0100, Uwe Kleine-König wrote:
> > Based on a patch by David Jander that mostly did s/mc13783/mc13xxx/
> > 
> > Cc: David Jander <david.jander@protonic.nl>
> > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> > ---
> > Hello,
> > 
> > I didn't manage to send this "later today" as promised yesterday. So it
> > comes only today.
> 
> No problem :)
> 
> > changes since v2:
> >  - make code generating name attribute more robust and strip [-_]adc
> >  - consistenly use MC13783_ADC_16CHANS / mc13783_attr_16chans
> 
> Thanks for the update. I have a few more comments, but these are only
> minor things.
> 
> >  Documentation/hwmon/mc13783-adc |   48 ++++++++++++----
> >  drivers/hwmon/Kconfig           |    6 +-
> >  drivers/hwmon/mc13783-adc.c     |  116 +++++++++++++++++++++++++++++----------
> >  3 files changed, 126 insertions(+), 44 deletions(-)
> > 
> > diff --git a/Documentation/hwmon/mc13783-adc b/Documentation/hwmon/mc13783-adc
> > index 044531a..86f6da5 100644
> > --- a/Documentation/hwmon/mc13783-adc
> > +++ b/Documentation/hwmon/mc13783-adc
> > @@ -5,6 +5,9 @@ Supported chips:
> >    * Freescale Atlas MC13783
> >      Prefix: 'mc13783_adc'
> >      Datasheet: http://www.freescale.com/files/rf_if/doc/data_sheet/MC13783.pdf?fsrch=1
> > +  * Freescale Atlas MC13892
> > +    Prefix: 'mc13892_adc'
> > +    Datasheet: http://cache.freescale.com/files/analog/doc/data_sheet/MC13892.pdf?fsrch=1&sr=1
> 
> The _adc is gone from the prefixes with this version, so the
> documentation should be updated to reflect that.
> 
> >  static ssize_t mc13783_adc_show_name(struct device *dev, struct device_attribute
> >  			      *devattr, char *buf)
> >  {
> > -	return sprintf(buf, "mc13783_adc\n");
> > +	struct platform_device *pdev = to_platform_device(dev);
> > +	struct mc13783_adc_priv *priv = platform_get_drvdata(pdev);
> 
> When you don't need pdev itself, the above can be abbreviated as:
> 
> 	struct mc13783_adc_priv *priv = dev_get_drvdata(dev);
> 
> Same in function mc13783_adc_read(). This can be cleaned up later, gcc
> is apparently smart enough to optimize it anyway.
> 
> All the rest looks good, so assuming the prefixes get fixed in the documentation,
> 
> Acked-by: Jean Delvare <khali@linux-fr.org>
> 
> Guenter, how do we proceed? I see you have a small cleanup patch to the
> mc13783-adc driver in your tree, the same cleanup is included in Uwe's
> patch so they will conflict. Let me know if you want to take Uwe's
> patch and handle the conflict, or prefer to drop the cleanup patch from
> you tree and let me handle that driver for this cycle. Either way is
> fine with me, as long as it actually happens.
> 
I'll drop the cleanup patch. Isn't much anyway.

Guenter

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors