linux-kernel - Re: [PATCH] mfd: ab8500-gpadc Add new GPADC driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20110201113617.GB10128@sortiz-mobl>
Date:	Tue, 1 Feb 2011 12:36:18 +0100
From:	Samuel Ortiz <sameo@...ux.intel.com>
To:	Mattias Wallin <mattias.wallin@...ricsson.com>
Cc:	Arun MURTHY <arun.murthy@...ricsson.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Linus WALLEIJ <linus.walleij@...ricsson.com>,
	Srinidhi KASAGAR <srinidhi.kasagar@...ricsson.com>
Subject: Re: [PATCH] mfd: ab8500-gpadc Add new GPADC driver

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/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/