linux-kernel - RE: [PATCH v5 3/7] iio: Add support for DA9150 GPADC

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 for Android: free password hash cracker in your pocket

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

Message-ID: <2E89032DDAA8B9408CB92943514A0337AB528C82@SW-EX-MBX01.diasemi.com>
Date:	Wed, 21 Jan 2015 11:02:07 +0000
From:	"Opensource [Adam Thomson]" <Adam.Thomson.Opensource@...semi.com>
To:	Jonathan Cameron <jic23@...nel.org>,
	"Opensource [Adam Thomson]" <Adam.Thomson.Opensource@...semi.com>,
	Lee Jones <lee.jones@...aro.org>,
	Samuel Ortiz <sameo@...ux.intel.com>,
	Hartmut Knaack <knaack.h@....de>,
	"linux-iio@...r.kernel.org" <linux-iio@...r.kernel.org>,
	Sebastian Reichel <sre@...nel.org>,
	Dmitry Eremin-Solenikov <dbaryshkov@...il.com>,
	"David Woodhouse" <dwmw2@...radead.org>,
	"linux-pm@...r.kernel.org" <linux-pm@...r.kernel.org>,
	Rob Herring <robh+dt@...nel.org>,
	Pawel Moll <pawel.moll@....com>,
	Mark Rutland <mark.rutland@....com>,
	Ian Campbell <ijc+devicetree@...lion.org.uk>,
	Kumar Gala <galak@...eaurora.org>,
	"Grant Likely" <grant.likely@...aro.org>,
	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
	Andrew Morton <akpm@...ux-foundation.org>,
	"Joe Perches" <joe@...ches.com>
CC:	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"Support Opensource" <Support.Opensource@...semi.com>
Subject: RE: [PATCH v5 3/7] iio: Add support for DA9150 GPADC

On January 20, 2015 20:49, Jonathan Cameron wrote:

> >>>>> +#define DA9150_GPADC_CHANNEL_PROCESSED(_id, _hw_id, _type,
> >> _ext_name)
> >>>> 	\
> >>>>> +	DA9150_GPADC_CHANNEL(_id, _hw_id, _type,			\
> >>>>> +			     BIT(IIO_CHAN_INFO_PROCESSED), _ext_name)
> >>>>> +
> >>>>> +/* Supported channels */
> >>>>> +static const struct iio_chan_spec da9150_gpadc_channels[] = {
> >>>>> +	DA9150_GPADC_CHANNEL_PROCESSED(GPIOA, GPIOA_6V, IIO_VOLTAGE,
> >>>> "GPIOA"),
> >>>>> +	DA9150_GPADC_CHANNEL_PROCESSED(GPIOB, GPIOB_6V, IIO_VOLTAGE,
> >>>> "GPIOB"),
> >>>>> +	DA9150_GPADC_CHANNEL_PROCESSED(GPIOC, GPIOC_6V, IIO_VOLTAGE,
> >>>> "GPIOC"),
> >>>>> +	DA9150_GPADC_CHANNEL_PROCESSED(GPIOD, GPIOD_6V, IIO_VOLTAGE,
> >>>> "GPIOD"),
> >>>> I'm not sure some of these really deserve extended names.  Those are usually
> >>>> reserved for naming strange internal adc channels etc.  These first 4 are
> >>>> presumably just for input pins?  Those should just be channels 0..3
> >>>> On another note, unless you want really weird sysfs attribute names, the
> >>>> extended names want to be lowercase.
> >>>>
> >>>
> >>> I'd prefer to keep the names because the input pins are muxed with GPIOs of the
> >>> chip, so thought it sensible to show that this is the case. Am happy to change
> >>> to lower-case to follow convention.
> >> Hmm.  It's a bit of an oddity as the point of the naming
> >> is about the uses, not which pins they are on.  If we exposed the
> >> 'datasheet_name' parameter directly rather than just using it internally
> >> I'd suggest relying on that - but clearly you want it to be apparent
> >> in the interface.  Whether that is useful is the question I'd raise
> >> here (and is the reason datasheet_name is not exposed.
> >>
> >> The obvious question is does userspace care?  Answer is probably not.
> >>
> >> It cares what is being measured but this is about what pins it is
> >> on and doesn't provide any information on what is connected to them.
> >>
> >
> > Surely it helps when using sysfs to access whatever is connected to one of
> > those pins if we label the pin with something meaningful? If say you have a
> > device connected to GPIC of the charger IC, it's easier to work out which ADC
> > channel you need to access through sysfs if the naming is as I have now, rather
> > than some arbitrary number which doesn't necessarily tally to the channel in the
> > datasheet. You'd then need to look at the code and work out which channel
> number
> > GPIOC actually was. Or am I just missing something here? :)
> Not really for the vast majority of users.  They tend not to have a detailed
> board layout in front of them.  It's more interesting if you know 'what' they
> are measuring (hence we do use these names when that is true - such as
> internal voltage measurements).
> 
> The numbers almost never tally with the datasheet, but then datasheet numbering
> has a habit of being inconsistent as well!

Can't say I agree that this won't be useful/informative to many users, but don't
want to make this a sticking point so will update.