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
