pinctrl: intel: Fix 2 kOhm bias which is 833 Ohm
Subsystems (3)
PATCH 1/2 pinctrl: intel: Fix 2 kOhm bias which is 833 Ohm
2 kOhm bias was never an option in Intel GPIO hardware, the available matrix is:
000 none 001 1 kOhm (if available) 010 5 kOhm 100 20 kOhm
As easy to get the 3 resistors are gated separately and according to parallel circuits calculations we may get combinations of the above where the result is always strictly less than minimal resistance. Hence, additional values can be:
011 ~833.3 Ohm 101 ~952.4 Ohm 110 ~4 kOhm 111 ~800 Ohm
That said, convert TERM definitions to be the bit masks to reflect the above.
While at it, enable the same setting for pull down case.
--- drivers/pinctrl/intel/pinctrl-intel.c | 32 ++++++++++++++++++--------- 1 file changed, 22 insertions(+), 10 deletions(-)
drivers/pinctrl/intel/pinctrl-intel.c +22 -10
@@ -72,10 +72,10 @@#define PADCFG1_TERM_UP BIT(13)#define PADCFG1_TERM_SHIFT 10#define PADCFG1_TERM_MASK GENMASK(12, 10) -#define PADCFG1_TERM_20K 4 -#define PADCFG1_TERM_2K 3 -#define PADCFG1_TERM_5K 2 -#define PADCFG1_TERM_1K 1 +#define PADCFG1_TERM_20K BIT(2) +#define PADCFG1_TERM_5K BIT(1) +#define PADCFG1_TERM_1K BIT(0) +#define PADCFG1_TERM_833 (BIT(1) | BIT(0))#define PADCFG2 0x008#define PADCFG2_DEBEN BIT(0)
@@ -559,12 +559,12 @@ static int intel_config_get_pull(struct intel_pinctrl *pctrl, unsigned int pin,return -EINVAL ;switch (term) { +case PADCFG1_TERM_833 : + *arg =833 ; +break ;case PADCFG1_TERM_1K : *arg =1000 ;break ; -case PADCFG1_TERM_2K : - *arg =2000 ; -break ;case PADCFG1_TERM_5K : *arg =5000 ;break ;
@@ -580,6 +580,11 @@ static int intel_config_get_pull(struct intel_pinctrl *pctrl, unsigned int pin,return -EINVAL ;switch (term) { +case PADCFG1_TERM_833 : +if (!(community->features &PINCTRL_FEATURE_1K_PD )) +return -EINVAL ; + *arg =833 ; +break ;case PADCFG1_TERM_1K :if (!(community->features &PINCTRL_FEATURE_1K_PD ))return -EINVAL ;
@@ -695,12 +700,12 @@ static int intel_config_set_pull(struct intel_pinctrl *pctrl, unsigned int pin,case 5000 : value |=PADCFG1_TERM_5K <<PADCFG1_TERM_SHIFT ;break ; -case 2000 : - value |=PADCFG1_TERM_2K <<PADCFG1_TERM_SHIFT ; -break ;case 1000 : value |=PADCFG1_TERM_1K <<PADCFG1_TERM_SHIFT ;break ; +case 833 : + value |=PADCFG1_TERM_833 <<PADCFG1_TERM_SHIFT ; +break ;default : ret = -EINVAL ; }
@@ -724,6 +729,13 @@ static int intel_config_set_pull(struct intel_pinctrl *pctrl, unsigned int pin, } value |=PADCFG1_TERM_1K <<PADCFG1_TERM_SHIFT ;break ; +case 833 : +if (!(community->features &PINCTRL_FEATURE_1K_PD )) { + ret = -EINVAL ; +break ; + } + value |=PADCFG1_TERM_833 <<PADCFG1_TERM_SHIFT ; +break ;default : ret = -EINVAL ; }
Signature · 1 line
2.28.0
PATCH 2/2 pinctrl: intel: Set default bias in case no particular value given
When GPIO library asks pin control to set the bias, it doesn't pass any value of it and argument is considered boolean (and this is true for ACPI GpioIo() / GpioInt() resources, by the way). Thus, individual drivers must behave well, when they got the resistance value of 1 Ohm, i.e. transforming it to sane default.
In case of Intel pin control hardware the 5 kOhm sounds plausible because on one hand it's a minimum of resistors present in all hardware generations and at the same time it's high enough to minimize leakage current (will be only 200 uA with the above choice).
--- drivers/pinctrl/intel/pinctrl-intel.c | 8 ++++++++ 1 file changed, 8 insertions(+)
drivers/pinctrl/intel/pinctrl-intel.c +8 -0
@@ -693,6 +693,10 @@ static int intel_config_set_pull(struct intel_pinctrl *pctrl, unsigned int pin, value |=PADCFG1_TERM_UP ; +/* Set default strength value in case none is given */ +if (arg ==1 ) + arg =5000 ; +switch (arg) {case 20000 : value |=PADCFG1_TERM_20K <<PADCFG1_TERM_SHIFT ;
@@ -715,6 +719,10 @@ static int intel_config_set_pull(struct intel_pinctrl *pctrl, unsigned int pin,case PIN_CONFIG_BIAS_PULL_DOWN : value &= ~(PADCFG1_TERM_UP |PADCFG1_TERM_MASK ); +/* Set default strength value in case none is given */ +if (arg ==1 ) + arg =5000 ; +switch (arg) {case 20000 : value |=PADCFG1_TERM_20K <<PADCFG1_TERM_SHIFT ;
Signature · 1 line
2.28.0
Re: [PATCH v1 2/2] pinctrl: intel: Set default bias in case no particular value given
Quoted 6 lines · Andy Shevchenko
When GPIO library asks pin control to set the bias, it doesn't pass any value of it and argument is considered boolean (and this is true for ACPI
GpioIo()/GpioInt()resources, by the way). Thus, individual drivers must behave well, when they got the resistance value of 1 Ohm, i.e. transforming it to sane default.
Quoted 8 lines
In case of Intel pin control hardware the 5 kOhm sounds plausible because on one hand it's a minimum of resistors present in all hardware generations and at the same time it's high enough to minimize leakage current (will be only 200 uA with the above choice).
Fixes e57725eabf87 ("pinctrl: intel: Add support for hardware debouncer")Reported-by Jamie McClymont <[email protected]>Signed-off-by Andy Shevchenko <[email protected]>
Quoted 18 lines · Andy Shevchenko
2 kOhm bias was never an option in Intel GPIO hardware, the available matrix is:
000 none 001 1 kOhm (if available) 010 5 kOhm 100 20 kOhmAs easy to get the 3 resistors are gated separately and according to parallel circuits calculations we may get combinations of the above where the result is always strictly less than minimal resistance. Hence, additional values can be:
011 ~833.3 Ohm 101 ~952.4 Ohm 110 ~4 kOhm 111 ~800 Ohm
Quoted 7 lines
That said, convert TERM definitions to be the bit masks to reflect the above.
While at it, enable the same setting for pull down case.
Fixes 7981c0015af2 ("pinctrl: intel: Add Intel Sunrisepoint pin controller and GPIO support")Cc Jamie McClymont <[email protected]>Signed-off-by Andy Shevchenko <[email protected]>
Quoted 18 lines · Andy Shevchenko
2 kOhm bias was never an option in Intel GPIO hardware, the available matrix is:
000 none 001 1 kOhm (if available) 010 5 kOhm 100 20 kOhmAs easy to get the 3 resistors are gated separately and according to parallel circuits calculations we may get combinations of the above where the result is always strictly less than minimal resistance. Hence, additional values can be:
011 ~833.3 Ohm 101 ~952.4 Ohm 110 ~4 kOhm 111 ~800 Ohm
Quoted 7 lines
That said, convert TERM definitions to be the bit masks to reflect the above.
While at it, enable the same setting for pull down case.
Fixes 7981c0015af2 ("pinctrl: intel: Add Intel Sunrisepoint pin controller and GPIO support")Cc Jamie McClymont <[email protected]>Signed-off-by Andy Shevchenko <[email protected]>
Good research!
I expect this as part of a pull request for fixes or devel.
Yours,
Linus Walleij
Quoted 25 lines · Linus Walleij
Quoted 22 lines · Andy Shevchenko
2 kOhm bias was never an option in Intel GPIO hardware, the available matrix is:
000 none 001 1 kOhm (if available) 010 5 kOhm 100 20 kOhmAs easy to get the 3 resistors are gated separately and according to parallel circuits calculations we may get combinations of the above where the result is always strictly less than minimal resistance. Hence, additional values can be:
011 ~833.3 Ohm 101 ~952.4 Ohm 110 ~4 kOhm 111 ~800 OhmThat said, convert TERM definitions to be the bit masks to reflect the above.
While at it, enable the same setting for pull down case.
Quoted 7 lines
Quoted 3 lines
Fixes 7981c0015af2 ("pinctrl: intel: Add Intel Sunrisepoint pin controller and GPIO support")Cc Jamie McClymont <[email protected]>Signed-off-by Andy Shevchenko <[email protected]>Good research!
I expect this as part of a pull request for fixes or devel.
for-next, but yes, I don't want to be in hurry of backporting this, better to test it carefully.
Signature · 2 lines
With Best Regards, Andy Shevchenko