[PATCH 2/2] pinctrl: add a generic control interface
linus.walleij at linaro.org
Thu Oct 20 10:24:56 UTC 2011
On Thu, Oct 20, 2011 at 1:04 AM, Stephen Warren <swarren at nvidia.com> wrote:
>> + * enum pin_config_param - possible pin configuration parameters
>> + * @PIN_CONFIG_BIAS_UNKNOWN: this bias mode is not known to us
> I'm not sure what use that would be; shouldn't we remove that, and add
> new PIN_CONFIG_BIAS_* values as/when they're needed?
Came from some idea that you could also pin_congif_get() to get the setting
of a pin, but that's overdesigned, so killed it now.
>> + * @PIN_CONFIG_BIAS_FLOAT: no specific bias, the pin will float or state
>> + * is not controlled by software
> "float" and "not controlled by SW" are very different things. How is float
> different to high impedance?
True. And the comment for BIAS_HIGH_IMPEDANCE even says so.
>> + * @PIN_CONFIG_BIAS_HIGH_IMPEDANCE: the pin will be set to a high impedance
>> + * mode, also know as "third-state" (tristate) or "high-Z" or "floating".
>> + * On output pins this effectively disconnects the pin, which is useful
>> + * if for example some other pin is going to drive the signal connected
>> + * to it for a while. Pins used for input are usually always high
>> + * impedance.
>> + * @PIN_CONFIG_BIAS_PULL_UP: the pin will be pulled up (usually with high
>> + * impedance to VDD), if the controller supports specifying a certain
>> + * pull-up resistance, this is given as an argument (in Ohms) when
>> + * setting this parameter
> What value should be used to disable a pull-up; 0?
A semantic question would also be if pull up is implicitly disabled
if you issue PIN_CONFIG_BIAS_PULL_DOWN when you are
in PULL_UP state.
I added PIN_CONFIG_BIAS_DISABLED to
So we can transition to a state of totally disabled pin bias.
(How to handled them is up to the driver...)
> What value should be
> used when requesting pull-up where the SoC doesn't have a configurable
> pull-up resistor; anything non-zero?
I think the argument should be ignored if it's simply on/off.
Then PIN_CONFIG_BIAS_DISABLE to turn it off.
> Tegra has separate configurations for
> pull-up/down strength (0..31) and pull-up/down enable (up/down/none), though
> I suppose we could map 0 to off, 1..32 to on with strength 0..31, although
> that'd prevent a driver from toggling the enable bit on/off without knowing
> what the strength code should be programmed to...
No a DISABLE enum is cleaner I think.
> Tegra's pull-up/down strengths aren't documented in terms of ohms, but
> rather a "drive strength code" on scale 0..31. I'm not sure what that
> truly maps to in hardware.
Hmmmmmmm can you check it with some hardware engineer?
I have a strong feeling that this is or should be very strictly specified
somewhere in an electrical datasheet. How else can users of the
circuit design the electronics around it? Trial-and-error? :-)
>> + * @PIN_CONFIG_BIAS_PULL_DOWN: the pin will be pulled down (usually with high
>> + * impedance to GROUND), if the controller supports specifying a certain
>> + * pull-down resistance, this is given as an argument (in Ohms) when
>> + * setting this parameter
>> + * @PIN_CONFIG_BIAS_HIGH: the pin will be wired high, connected to VDD
>> + * @PIN_CONFIG_BIAS_GROUND: the pin will be grounded, connected to GROUND
> At least some of these options appear mutually exclusive; I wonder if we
> shouldn't have a PIN_CONFIG_BIAS parameter, and have the legal values be
> HIGH/GROUND/NONE. Pull up/down are probably separate. HIGH_IMPEDANCE seems
> more like a PIN_CONFIG_DRIVE value than a PIN_CONFIG_BIAS value.
Yes some of them are. I think it should be up to the driver to handle
how the mutual exclusiveness of its supported configs are done,
returning the occasional -EINVAL.
Later we may consolidate this, say by letting the core keep track of
config states (it currently doesn't) and reject things that are mutually
exclusive. But I think that's overkill right now?
>> + * @PIN_CONFIG_DRIVE_UNKNOWN: we don't know the drive mode of this pin, for
>> + * example since it is controlled by hardware or the information is not
>> + * accessible but we need a meaningful enumerator in e.g. initialization
>> + * code
> Again, I'm not sure this is useful; if this is not a configurable parameter,
> surely initialization code would simply skip attempting to set this param?
>> + * @PIN_CONFIG_DRIVE_PUSH_PULL: the pin will be driven actively high and
>> + * low, this is the most typical case and is typically achieved with two
>> + * active transistors on the output. If the pin can support different
>> + * drive strengths for push/pull, the strength is given on a custom format
>> + * as argument when setting pins to this mode
>> + * @PIN_CONFIG_DRIVE_OPEN_DRAIN: the pin will be driven with open drain (open
>> + * collector) which means it is usually wired with other output ports
>> + * which are then pulled up with an external resistor. If the pin can
>> + * support different drive strengths for the open drain pin, the strength
>> + * is given on a custom format as argument when setting pins to this mode
>> + * @PIN_CONFIG_DRIVE_OPEN_SOURCE: the pin will be driven with open drain
>> + * (open emitter) which is the same as open drain mutatis mutandis but
>> + * pulled to ground. If the pin can support different drive strengths for
>> + * the open drain pin, the strength is given on a custom format as
>> + * argument when setting pins to this mode
>> + * @PIN_CONFIG_DRIVE_OFF: the pin is set to inactive drive mode, off
> These seem like mutually exclusive values for a PIN_CONFIG_DRIVE param.
They are. Driver needs to handle that, and say disable OPEN_DRAIN
if it is on when enabling OPEN_SOURCE.
>> + * @PIN_CONFIG_INPUT_SCHMITT: this will configure an input pin to run in
>> + * schmitt-trigger mode. If the schmitt-trigger has adjustable hysteresis,
>> + * the threshold value is given on a custom format as argument when
>> + * setting pins to this mode
>> + * @PIN_CONFIG_SLEW_RATE_RISING: this will configure the slew rate for rising
>> + * signals on the pin. The argument gives the rise time in nanoseconds
>> + * @PIN_CONFIG_SLEW_RATE_FALLING: this will configure the slew rate for falling
>> + * signals on the pin. The argument gives the fall time in nanoseconds
>> + * @PIN_CONFIG_LOAD_CAPACITANCE: some pins have inductive characteristics and
>> + * will deform waveforms when signals are transmitted on them, by
>> + * applying a load capacitance, the waveform can be rectified. The
>> + * argument gives the load capacitance in picofarad (pF)
>> + * @PIN_CONFIG_WAKEUP_ENABLE: this will configure an input pin such that if a
>> + * signal transition arrives at the pin when the pin controller/system
>> + * is sleeping, it will wake up the system
>> + * @PIN_CONFIG_END: this is the last enumerator for pin configurations, if
>> + * you need to pass in custom configurations to the pin controller, use
>> + * PIN_CONFIG_END+1 as the base offset
> Tegra needs at least some more:
> PIN_CONFIG_HIGH_SPEED_MODE (0..1) [tegra_drive_pingroup_config.hsm]
Is it only for output so it's actually PIN_CONFIG_DRIVE_HIGH_SPEED
or only for input so it's PIN_CONFIG_INPUT_HIGHSPEED?
So this is like the inverse of the slew rate or so?
> PIN_CONFIG_LOW_POWER_MODE (0..3) [tegra_drive_pingroup_config.drive]
OK added PIN_CONFIG_LOW_POWER_MODE, it could actually
map to say ground setting after a state transition in some cases.
>> @@ -113,6 +204,10 @@ extern struct pinctrl_dev *pinctrl_register(struct pinctrl_desc *pctldesc,
>> +extern int pin_config(struct pinctrl_dev *pctldev, int pin,
>> + enum pin_config_param param, unsigned long data);
>> +extern int pin_config_group(struct pinctrl_dev *pctldev, const char *pin_group,
>> + enum pin_config_param param, unsigned long data);
> Hmmm. Do we really want to expose these as public APIs? I suppose it
> will allow us to start configuring all these parameters ASAP, but all
> previous discussion has been aimed at having the pinctrl core set up an
> initial set of values at boot-time using a board-supplied table (so no
> external API), and then we were still talking about how to manipulate
> the values at run-time. Do we really want to encode all this information
> into drivers calling these APIs?
I already have such drivers: in the ABx500 there is a SIM card interface,
these set pull up/down and load capacitance (also voltage!) depending
on information obtained after actively communicating with the card
and asking about its electrical characteristics.
Same applies to some eMMC I think.
So we probably cannot avoid exposing this...
However, yes, the simple cases should be for default-config
and then state transitions to things like sleeping or different
More information about the linaro-dev