| Message ID | 201306141742.21808.heiko@sntech.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Jun 14, 2013 at 5:42 PM, Heiko Stübner <heiko@sntech.de> wrote: > The BIAS_DISABLE and BIAS_HIGH_IMPEDANCE generic pinconfig options were > missing information about their argument - which should be ignored. > > Also the BIAS_PULL_* options may have the pull strength as argument > when they are activated, while simpler hardware can use any > non-0 value for it. > > Update the kerneldoc to reflect this. > > Signed-off-by: Heiko Stuebner <heiko@sntech.de> I'm holding this patch off. > * @PIN_CONFIG_BIAS_PULL_UP: the pin will be pulled up (usually with high > - * impedance to VDD). If the argument is != 0 pull-up is enabled, > - * if it is 0, pull-up is disabled. > + * impedance to VDD). If the argument is != 0 pull-up is enabled. On > + * hardware supporting this, the argument should contain the strength of > + * the pull in Ohm. If it is 0, pull-up is disabled. As noted by Laurent, a pull-up of 0 Ohm is a short-circuit (bascially the TOTAL pull-up) and it is pretty counter-intuitive to have that mean "disable pull-up". Can we avoid this? > * @PIN_CONFIG_BIAS_PULL_DOWN: the pin will be pulled down (usually with high > - * impedance to GROUND). If the argument is != 0 pull-down is enabled, > - * if it is 0, pull-down is disabled. > + * impedance to GROUND). If the argument is != 0 pull-down is enabled. On > + * hardware supporting this, the argument should contain the strength of > + * the pull in Ohm. If it is 0, pull-down is disabled. Dito. > * @PIN_CONFIG_BIAS_PULL_PIN_DEFAULT: the pin will be pulled up or down based > * on embedded knowledge of the controller, like current mux function. > - * If the argument is != 0 pull up/down is enabled, if it is 0, > - * the pull is disabled. > + * If the argument is != 0 pull up/down is enabled. On hardware supporting > + * this, the argument should contain the strength of the pull in Ohm. > + * If it is 0, pull is disabled. Dito. Can't we rely on PIN_CONFIG_BIAS_DISABLE for all this? Yours, Linus Walleij
Am Sonntag, 16. Juni 2013, 12:26:38 schrieb Linus Walleij: > On Fri, Jun 14, 2013 at 5:42 PM, Heiko Stübner <heiko@sntech.de> wrote: > > The BIAS_DISABLE and BIAS_HIGH_IMPEDANCE generic pinconfig options were > > missing information about their argument - which should be ignored. > > > > Also the BIAS_PULL_* options may have the pull strength as argument > > when they are activated, while simpler hardware can use any > > non-0 value for it. > > > > Update the kerneldoc to reflect this. > > > > Signed-off-by: Heiko Stuebner <heiko@sntech.de> > > I'm holding this patch off. > > > * @PIN_CONFIG_BIAS_PULL_UP: the pin will be pulled up (usually with > > high > > > > - * impedance to VDD). If the argument is != 0 pull-up is enabled, > > - * if it is 0, pull-up is disabled. > > + * impedance to VDD). If the argument is != 0 pull-up is enabled. On > > + * hardware supporting this, the argument should contain the > > strength of + * the pull in Ohm. If it is 0, pull-up is disabled. > > As noted by Laurent, a pull-up of 0 Ohm is a short-circuit (bascially the > TOTAL pull-up) and it is pretty counter-intuitive to have that mean > "disable pull-up". Can we avoid this? > > > * @PIN_CONFIG_BIAS_PULL_DOWN: the pin will be pulled down (usually with > > high > > > > - * impedance to GROUND). If the argument is != 0 pull-down is > > enabled, - * if it is 0, pull-down is disabled. > > + * impedance to GROUND). If the argument is != 0 pull-down is > > enabled. On + * hardware supporting this, the argument should > > contain the strength of + * the pull in Ohm. If it is 0, pull-down > > is disabled. > > Dito. > > > * @PIN_CONFIG_BIAS_PULL_PIN_DEFAULT: the pin will be pulled up or down > > based * on embedded knowledge of the controller, like current mux > > function. > > > > - * If the argument is != 0 pull up/down is enabled, if it is 0, > > - * the pull is disabled. > > + * If the argument is != 0 pull up/down is enabled. On hardware > > supporting + * this, the argument should contain the strength of the > > pull in Ohm. + * If it is 0, pull is disabled. > > Dito. > > Can't we rely on PIN_CONFIG_BIAS_DISABLE for all this? you're the boss on this, I'll do whatever you say :-) I was going after the existing documentation of "bias-pull-X = <0>" being there to turn off this specific bias. So when reading the existing doc I was assuming somebody had needed this in the past. Actually I'm too more comfortable with only using bias-disable. So I'll redo this patch to remove the disabling of a pull via this method, if that's the right way to go. Same with the rockchip driver. Heiko
On Sun, Jun 16, 2013 at 12:45 PM, Heiko Stübner <heiko@sntech.de> wrote: > Am Sonntag, 16. Juni 2013, 12:26:38 schrieb Linus Walleij: >> Can't we rely on PIN_CONFIG_BIAS_DISABLE for all this? > > you're the boss on this, I'll do whatever you say :-) Oh that's not good. As can be seen from past experience I am often avoiding to do horrible design mistakes by other smart people telling me how wrong I am ... > Actually I'm too more comfortable with only using bias-disable. > > So I'll redo this patch to remove the disabling of a pull via this method, if > that's the right way to go. I sent some patch on how I'd like it defined, please check it. > Same with the rockchip driver. Hm I merged that patch actually, but you can send a v2 if you want me to replace it. Or a patch on top, if preferred. Yours, Linus Walleij
diff --git a/include/linux/pinctrl/pinconf-generic.h b/include/linux/pinctrl/pinconf-generic.h index d414a77..d1868bc 100644 --- a/include/linux/pinctrl/pinconf-generic.h +++ b/include/linux/pinctrl/pinconf-generic.h @@ -23,27 +23,31 @@ * @PIN_CONFIG_BIAS_DISABLE: disable any pin bias on the pin, a * transition from say pull-up to pull-down implies that you disable * pull-up in the process, this setting disables all biasing. + * The argument is ignored. * @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. + * impedance. The argument is ignored. * @PIN_CONFIG_BIAS_BUS_HOLD: the pin will be set to weakly latch so that it * weakly drives the last value on a tristate bus, also known as a "bus * holder", "bus keeper" or "repeater". This allows another device on the * bus to change the value by driving the bus high or low and switching to * tristate. The argument is ignored. * @PIN_CONFIG_BIAS_PULL_UP: the pin will be pulled up (usually with high - * impedance to VDD). If the argument is != 0 pull-up is enabled, - * if it is 0, pull-up is disabled. + * impedance to VDD). If the argument is != 0 pull-up is enabled. On + * hardware supporting this, the argument should contain the strength of + * the pull in Ohm. If it is 0, pull-up is disabled. * @PIN_CONFIG_BIAS_PULL_DOWN: the pin will be pulled down (usually with high - * impedance to GROUND). If the argument is != 0 pull-down is enabled, - * if it is 0, pull-down is disabled. + * impedance to GROUND). If the argument is != 0 pull-down is enabled. On + * hardware supporting this, the argument should contain the strength of + * the pull in Ohm. If it is 0, pull-down is disabled. * @PIN_CONFIG_BIAS_PULL_PIN_DEFAULT: the pin will be pulled up or down based * on embedded knowledge of the controller, like current mux function. - * If the argument is != 0 pull up/down is enabled, if it is 0, - * the pull is disabled. + * If the argument is != 0 pull up/down is enabled. On hardware supporting + * this, the argument should contain the strength of the pull in Ohm. + * If it is 0, pull is disabled. * @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. Setting this config will enable
The BIAS_DISABLE and BIAS_HIGH_IMPEDANCE generic pinconfig options were missing information about their argument - which should be ignored. Also the BIAS_PULL_* options may have the pull strength as argument when they are activated, while simpler hardware can use any non-0 value for it. Update the kerneldoc to reflect this. Signed-off-by: Heiko Stuebner <heiko@sntech.de> --- include/linux/pinctrl/pinconf-generic.h | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-)