From mboxrd@z Thu Jan 1 00:00:00 1970 From: Keerthy Subject: Re: [lm-sensors] [PATCH 1/2] hwmon: twl4030: Driver for twl4030 madc module Date: Mon, 27 Sep 2010 14:49:06 +0530 Message-ID: <4CA0618A.9030207@ti.com> References: <1284632604-25303-1-git-send-email-j-keerthy@ti.com> <20100916151736.GC14165@ericsson.com> <20100920140905.GB1306@ericsson.com> <20100920143834.GA1500@ericsson.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Return-path: Received: from bear.ext.ti.com ([192.94.94.41]:58810 "EHLO bear.ext.ti.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932298Ab0I0JTv (ORCPT ); Mon, 27 Sep 2010 05:19:51 -0400 In-Reply-To: <20100920143834.GA1500@ericsson.com> Sender: linux-omap-owner@vger.kernel.org List-Id: linux-omap@vger.kernel.org To: sameo@linux.intel.com Cc: "J, KEERTHY" , "linux-omap@vger.kernel.org" , "Krishnamoorthy, Balaji T" , "lm-sensors@lm-sensors.org" , Guenter Roeck 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 > From mboxrd@z Thu Jan 1 00:00:00 1970 From: Keerthy Date: Mon, 27 Sep 2010 09:31:06 +0000 Subject: Re: [lm-sensors] [PATCH 1/2] hwmon: twl4030: Driver for twl4030 Message-Id: <4CA0618A.9030207@ti.com> List-Id: References: <1284632604-25303-1-git-send-email-j-keerthy@ti.com> <20100916151736.GC14165@ericsson.com> <20100920140905.GB1306@ericsson.com> <20100920143834.GA1500@ericsson.com> In-Reply-To: <20100920143834.GA1500@ericsson.com> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: sameo@linux.intel.com Cc: "J, KEERTHY" , "linux-omap@vger.kernel.org" , "Krishnamoorthy, Balaji T" , "lm-sensors@lm-sensors.org" , Guenter Roeck 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