From: Samuel Ortiz <sameo@linux.intel.com>
To: "J, KEERTHY" <j-keerthy@ti.com>
Cc: Mark Brown <broonie@opensource.wolfsonmicro.com>,
"amit.kucheria@canonical.com" <amit.kucheria@canonical.com>,
"mikko.k.ylinen@nokia.com" <mikko.k.ylinen@nokia.com>,
"lm-sensors@lm-sensors.org" <lm-sensors@lm-sensors.org>,
"linux-omap@vger.kernel.org" <linux-omap@vger.kernel.org>,
"balajitk@ti.com" <balajitk@ti.com>,
guenter.roeck@ericsson.com
Subject: Re: [lm-sensors] [PATCH v3] hwmon: twl4030: Driver for twl4030 madc module
Date: Mon, 31 Jan 2011 11:43:47 +0100 [thread overview]
Message-ID: <20110131104346.GA5954@sortiz-mobl> (raw)
In-Reply-To: <AANLkTinDVZe0eefrPZsMoUyzqfQOxZsDOaD0VG3OqXyT@mail.gmail.com>
Hi Keerthy,
On Sat, Jan 22, 2011 at 10:19:33PM +0530, J, KEERTHY wrote:
> On Fri, Jan 7, 2011 at 5:42 PM, J, KEERTHY <j-keerthy@ti.com> wrote:
> > On Fri, Jan 7, 2011 at 3:25 AM, Guenter Roeck
> > <guenter.roeck@ericsson.com> wrote:
> >> On Thu, 2011-01-06 at 15:21 -0500, Mark Brown wrote:
> >>> On Thu, Jan 06, 2011 at 07:04:30AM -0800, Guenter Roeck wrote:
> >>> > On Thu, Jan 06, 2011 at 07:07:13AM -0500, Mark Brown wrote:
> >>>
> >>> > > Why? It's not like hwmon has an unreasonably large core or similar.
> >>>
> >>> > Because it creates an unnecessary dependency, and because it is not hwmon's
> >>> > responsibility to provide infrastructure for other subsystems or drivers.
> >>>
> >>> hwmon isn't really doing anything, though. The *driver* is doing
> >>> something but it doesn't really impact the core that much. Not that I'm
> >>> particularly sold on putting the ADC core in here, but total NACK based
> >>> on that alone seems rather harsh.
> >>
> >> Possibly. However, I had suggested the following earlier (to the 1st
> >> version of the patch):
> >>
> >>> I commented on this a couple of times below - the driver mixes generic
> >>> ADC reading functions with hwmon functionality. Generic ADC reading
> >>> functionality should be moved into another driver, possibly to mfd.
> >>
> >> Obviously that was ignored. Maybe someone is willing to listen this time
> >> around.
> >>
> > I am sorry for not responding on the generic ADC handling part. Since many
> > other ADC drivers are part of hwmon i thought hwmon was the appropriate
> > place. However I can surely split the generic ADC handling part in mfd and
> > only hardware monitoring part in hwmon as suggested.
> >
> >> I won't let people break modularity just for convenience in a subsystem
> >> I am responsible for. And forcing the hwmon subsystem, and with it a
> >> specific hwmon driver, to exist just because the adc functionality it
> >> provides is needed by some other (most likely unrelated) subsystem /
> >> driver _does_ break modularity. Worse, it is completely unnecessary to
> >> do so. Other twl4030 functionality was extracted into generic code.
> >> twl-core.c, twl4030-codec.c, twl4030-irq.c, twl4030-power.c are all in
> >> mfd. I fail to see the problem with mfd/twl4030-adc.c.
> >>
> >> Guenter
> >>
> >>
> >>
> >
> > Hello Samuel,
> >
> > Is it ok to have the generic ADC functionality in mfd as a separate file?
> >
> > Regards,
> > Keerthy
> >
>
> Hello Samuel,
>
> We need your valuable inputs. Can generic ADC functionality can be in
> mfd as a separate file?
If it's really generic and doesn't mix hwmon stuff in the middle, then yes.
Cheers,
Samuel.
--
Intel Open Source Technology Centre
http://oss.intel.com/
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
next prev parent reply other threads:[~2011-01-31 10:43 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-01-06 4:17 [PATCH v3] hwmon: twl4030: Driver for twl4030 madc module Keerthy
2011-01-06 5:33 ` Guenter Roeck
2011-01-06 12:07 ` [lm-sensors] " Mark Brown
2011-01-06 15:04 ` Guenter Roeck
2011-01-06 20:21 ` Mark Brown
2011-01-06 21:55 ` Guenter Roeck
2011-01-07 12:12 ` J, KEERTHY
2011-01-07 14:45 ` Guenter Roeck
2011-01-22 16:49 ` J, KEERTHY
2011-01-31 10:43 ` Samuel Ortiz [this message]
2011-01-06 5:58 ` Kalliguddi, Hema
2011-01-07 8:40 ` J, KEERTHY
-- strict thread matches above, loose matches on Subject: below --
2011-01-06 3:56 Keerthy
2011-01-06 12:04 ` [lm-sensors] " Mark Brown
2011-01-07 9:25 ` J, KEERTHY
2011-01-07 10:44 ` Mark Brown
2011-01-07 11:20 ` J, KEERTHY
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20110131104346.GA5954@sortiz-mobl \
--to=sameo@linux.intel.com \
--cc=amit.kucheria@canonical.com \
--cc=balajitk@ti.com \
--cc=broonie@opensource.wolfsonmicro.com \
--cc=guenter.roeck@ericsson.com \
--cc=j-keerthy@ti.com \
--cc=linux-omap@vger.kernel.org \
--cc=lm-sensors@lm-sensors.org \
--cc=mikko.k.ylinen@nokia.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox