linux-kernel - Re: [PATCH 2/2] pinctrl: add a generic control interface

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: <20111020131042.GH32007@S2100-06.ap.freescale.net>
Date:	Thu, 20 Oct 2011 21:10:43 +0800
From:	Shawn Guo <shawn.guo@...escale.com>
To:	Barry Song <21cnbao@...il.com>
CC:	Linus Walleij <linus.walleij@...ricsson.com>,
	<linux-kernel@...r.kernel.org>,
	Stephen Warren <swarren@...dia.com>,
	Linaro Dev <linaro-dev@...ts.linaro.org>,
	Sascha Hauer <s.hauer@...gutronix.de>,
	David Brown <davidb@...eaurora.org>,
	Grant Likely <grant.likely@...retlab.ca>,
	Linus Walleij <linus.walleij@...aro.org>
Subject: Re: [PATCH 2/2] pinctrl: add a generic control interface

On Thu, Oct 20, 2011 at 05:17:08PM +0800, Barry Song wrote:
> >> +enum pin_config_param {
> >> +     PIN_CONFIG_BIAS_UNKNOWN,
> >> +     PIN_CONFIG_BIAS_FLOAT,
> >> +     PIN_CONFIG_BIAS_HIGH_IMPEDANCE,
> >> +     PIN_CONFIG_BIAS_PULL_UP,
> >> +     PIN_CONFIG_BIAS_PULL_DOWN,
> >> +     PIN_CONFIG_BIAS_HIGH,
> >> +     PIN_CONFIG_BIAS_GROUND,
> >> +     PIN_CONFIG_DRIVE_UNKNOWN,
> >> +     PIN_CONFIG_DRIVE_PUSH_PULL,
> >> +     PIN_CONFIG_DRIVE_OPEN_DRAIN,
> >> +     PIN_CONFIG_DRIVE_OPEN_SOURCE,
> >> +     PIN_CONFIG_DRIVE_OFF,
> >> +     PIN_CONFIG_INPUT_SCHMITT,
> >> +     PIN_CONFIG_SLEW_RATE_RISING,
> >> +     PIN_CONFIG_SLEW_RATE_FALLING,
> >> +     PIN_CONFIG_LOAD_CAPACITANCE,
> >> +     PIN_CONFIG_WAKEUP_ENABLE,
> >> +     PIN_CONFIG_END,
> >> +};
> >> +
> >
> > IMO, trying to enumerate all possible pin_config options is just to
> > make everyone's life harder.  Firstly, this enumeration is far from
> > completion, for imx6 iomuxc example, we have 3 options for pull-up,
> > 22, 47 and 100 kOhm, and 7 options for driver-strength, 34, 40, 48,
> > 60, 80, 120, and 240 Ohm.  It's just so hard to make this enumeration
> > complete.  Secondly, defining this param as enum requires the API
> > user to call the function multiple times to configure one pin.  For
> > example, if I want to configure pin_foo as slow-slew, open-drain,
> > 100 kOhm pull-up and 120 Ohm driver-strength, I will have to call
> > pin_config(pctldev, pin_foo, ...) 4 times to achieve that.
> >
> > I like Stephen's idea about having 'u32 param' and let pinctrl drivers
> > to encode/decode this u32 for their pinctrl controller.  It makes
> > people's life much easier.
> 
> A multifunctional API is actually  a bad and hard-to-use API. i agree
> we can make param u32 instead of enum and let specific driver
> customizes the param by itself.
> 
> But if there are some common params, for example, PULL, OC/OD,
> WAKEUP_ENABLE, which almost all platforms need,  why don't we make
> them have common definitions like:
> 
> #define PIN_CONFIG_PULL                             0
> #define PIN_CONFIG_OPEN_DRAIN                1
> ....
> #define PIN_CONFIG_USER                            5 (in case)
> 
> if one platform has config not in the up list, then:
> 
> #define PIN_CONFIG_TERGA_XXX                PIN_CONFIG_USER
> #define PIN_CONFIG_TERGA_YYY                (PIN_CONFIG_USER + 1)
> 
> without the common definition from  PIN_CONFIG_PULL to
> PIN_CONFIG_USER, many platforms will need to definite them repeatedly.
> that is what we hate.
> 
I prefer to completely leave the encoding of this 'u32 param' to pinctrl
drivers, so that they can map the bit definition of 'param' to the
controller register as much as they can.  On imx, we have one register
accommodating all pin configuration options for one pin, so we can
write 'param' directly into register with exactly same bit definition
as register.

-- 
Regards,
Shawn

--
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/