[prev in list] [next in list] [prev in thread] [next in thread]
List: lm-sensors
Subject: Re: [lm-sensors] [PATCH 1/2] hwmon: twl4030: Driver for twl4030
From: Keerthy <a0393675 () ti ! com>
Date: 2010-09-27 9:31:06
Message-ID: 4CA0618A.9030207 () ti ! com
[Download RAW message or body]
Hello Sameo,
twl4030-madc driver patch can be found here:
http://www.mail-archive.com/linux-omap@vger.kernel.org/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@ericsson.com]
>>>> Sent: Thursday, September 16, 2010 8:48 PM
>>>> To: J, KEERTHY
>>>> Cc: linux-omap@vger.kernel.org; lm-sensors@lm-sensors.org; 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
>
_______________________________________________
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]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic