LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Samuel Ortiz <sameo@linux.intel.com>
To: Mattias Wallin <mattias.wallin@stericsson.com>
Cc: Arun MURTHY <arun.murthy@stericsson.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Linus WALLEIJ <linus.walleij@stericsson.com>,
	Srinidhi KASAGAR <srinidhi.kasagar@stericsson.com>
Subject: Re: [PATCH] mfd: ab8500-gpadc Add new GPADC driver
Date: Tue, 1 Feb 2011 12:36:18 +0100	[thread overview]
Message-ID: <20110201113617.GB10128@sortiz-mobl> (raw)
In-Reply-To: <4D39770C.20208@stericsson.com>

Hi Mattias,

On Fri, Jan 21, 2011 at 01:07:40PM +0100, Mattias Wallin wrote:
> On 01/21/2011 11:31 AM, Arun MURTHY wrote:
> > The only clients for GPADC is the battery driver and the Audio Acc
> > detection. Both of these are sub-modules/clients of ab8500.
> > None other than these can use the GPADC.
> > Inputs to GPADC can only be one among the following:
> > /* GPADC source: From datasheet(ADCSwSel[4:0] in GPADCCtrl2) */
> > #define BAT_CTRL        0x01
> > #define BTEMP_BALL      0x02
> > #define MAIN_CHARGER_V  0x03
> > #define ACC_DETECT1     0x04
> > #define ACC_DETECT2     0x05
> > #define ADC_AUX1		0x06
> > #define ADC_AUX2		0x07
> > #define MAIN_BAT_V      0x08
> > #define VBUS_V          0x09
> > #define MAIN_CHARGER_C  0x0A
> > #define USB_CHARGER_C   0x0B
> > #define BK_BAT_V        0x0C
> > #define DIE_TEMP        0x0D
> > 
> > Henceforth in order to secure the usage of GPADC, and in order to
> > restrict it to only EM and AUDIO sub-module, the gpadc device struct
> > was added to ab8500 struct. Also that the exported function
> > ab8500_gpadc_convert has an argument struct ab8500_gpadc, which can
> > be obtained be dereferencing the struct ab8500. This is possible only
> > with the ab8500 and its clients, thereby securing the usage to
> > battery driver and audio acc detect.
> Yes and I would like to remove this restriction and have a simpler more open api.
> First I don't like that users needs to do a lot of pointer dereferencing in their call like ab8500_gpadc_convert(dev->parent->ab8500->gpadc, USB_CHARGER) (an example).
>
As subdevices, I expect users to have an ab8500 pointer. So it would just be
one dereference.


> I prefer a get function that returns a handle that should be used as first argument in the convert calls.
> It also makes sense if this driver will be extended to use more channels.
> Second this driver will be used by for example accessories which likely will be called by 3 party drivers
> and to make their life easier I don't want to force them to this ab8500-core dependency.
>
As I'm not familiar with your HW architecture, could you please describe how
those accessories would wire into the ab8500 core ?
If those devices really are independent drivers (i.e. not subdevices) needing
to get an A/D conversion from the ab8500 adc (I don't see how that can happen,
hence my above question), then it might make sense to use a conversion API
independent from any ab8500 pointer. But otherwise, I prefer this API rather
than the one in v2 of this patch.

Cheers,
Samuel.

-- 
Intel Open Source Technology Centre
http://oss.intel.com/

  parent reply	other threads:[~2011-02-01 11:36 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-01-20 10:28 Arun Murthy
2011-01-20 12:37 ` Mattias Wallin
2011-01-21 10:31   ` Arun MURTHY
2011-01-21 12:07     ` Mattias Wallin
2011-01-24  3:37       ` Arun MURTHY
2011-02-01 11:36       ` Samuel Ortiz [this message]
2011-02-02  8:15         ` Mattias Wallin

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=20110201113617.GB10128@sortiz-mobl \
    --to=sameo@linux.intel.com \
    --cc=arun.murthy@stericsson.com \
    --cc=linus.walleij@stericsson.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mattias.wallin@stericsson.com \
    --cc=srinidhi.kasagar@stericsson.com \
    --subject='Re: [PATCH] mfd: ab8500-gpadc Add new GPADC driver' \
    /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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).