Re: [lm-sensors] [PATCH] hwmon: Add WM835x PMIC hardwaremonitoring driver

[Mark Brown]

On Sat, Jul 18, 2009 at 11:17:12AM +0200, Jean Delvare wrote:

> > +WM8350_NAMED_VOLTAGE(0, USB);
> > +WM8350_NAMED_VOLTAGE(1, BATT);
> > +WM8350_NAMED_VOLTAGE(2, LINE);

> I would suggest passing the whole WM8350_AUXADC_USB, etc. string as the
> 2nd parameter. This makes grepping the source code for users (as LXR
> does for example) possible.

The reason it's formatted like that is that the AUXADC input names match
the names of the supplies that are being monitored so the second parameter
is also used to provide a label to userspace.

> > +static struct platform_driver wm8350_hwmon_driver = {
> > +	.probe = wm8350_hwmon_probe,
> > +	.remove = __devexit_p(wm8350_hwmon_remove),
> > +	.driver = {
> > +		.name = "wm8350-hwmon",

> I think you are supposed to set .owner to THIS_MODULE?

Hrm, yeah.  I need to figure out where I cut'n'pasted this from since I
suspect the error came along with that.

> When you are done, what is your merge plan? I would be happy to take
> the patch in my unofficial hwmon tree and push it in 2.6.32, but due to
> wm8350-core dependencies, you may prefer it to be merged differently?

I'm happy to go with whatever is easiest for the people actually doing
the merging.  wm8350 is fairly static at the minute - it's much more
likely that there would be merge issues from the build infrastructure.
It might be a little easier to merge via mfd.

I'll fix the rest of your comments, thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/