Re: [lm-sensors] [PATCH 1/2] hwmon: twl4030: Driver for twl4030 madc module

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hello Sameo,

twl4030-madc  driver patch can be found here:
http://www.mail-archive.com/linux-omap@xxxxxxxxxxxxxxx/msg34947.html

Based on the received inputs.
Can the twl4030-madc driver or part of the driver reside under mfd?

Regards,
Keerthy





On Monday 20 September 2010 08:08 PM, Guenter Roeck wrote:
On Mon, Sep 20, 2010 at 10:09:05AM -0400, Guenter Roeck wrote:
Hi,
On Mon, Sep 20, 2010 at 06:34:24AM -0400, J, KEERTHY wrote:

-----Original Message-----
From: Guenter Roeck [mailto:guenter.roeck@xxxxxxxxxxxx]
Sent: Thursday, September 16, 2010 8:48 PM
To: J, KEERTHY
Cc: linux-omap@xxxxxxxxxxxxxxx; lm-sensors@xxxxxxxxxxxxxx; Krishnamoorthy,
Balaji T
Subject: Re: [lm-sensors] [PATCH 1/2] hwmon: twl4030: Driver for twl4030
madc module

[...]
+EXPORT_SYMBOL(twl4030_madc_conversion);
+
If this function is going to be called  from external code, it should not
really be defined here. I would suggest to move it to a global location
such as
mfd instead, including all related functions.

The existence of this function export indicates that another non-hwmon
driver depends on this one, which should not really be the case. Another
reason to have a separate common driver instead, and mfd might just be the
place for it.
Few kernel modules need to perform ADC conversion to measure battery
voltage, battery temperature, VBUS voltage via twl4030_madc_conversion.
the_madc is needed as those drivers will not have this device pointer.

The point I was trying to make is that this function (as well as the ioctl)
should not be in this driver in the first place. hwmon is about providing
hwmon information, not about providing adc readings to another driver.

Or, in other words, hwmon should be a consumer of information from other drivers,
not a producer of information to other drivers.

There should be a higher level driver (presumably a mfd driver) to provide
adc readings to all consumers, ie to all callers of twl4030_madc_conversion().
This driver can provide all data and information needed by more than one driver,
and would also be the logical place for the ioctl providing raw adc readings
to the user.

I just noticed that there already is a mfd core driver for tw4030. You might want
to consider moving the functionality to read adc values into that driver.

Guenter

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Arm (vger)]     [ARM Kernel]     [ARM MSM]     [Linux Tegra]     [Linux WPAN Networking]     [Linux Wireless Networking]     [Maemo Users]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux