| Message ID | 20201014104638.84043-1-andriy.shevchenko@linux.intel.com |
|---|---|
| State | Accepted |
| Commit | dd26209bc56886cacdbd828571e54a6bca251e55 |
| Headers | show |
| Series | [v1,1/2] pinctrl: intel: Fix 2 kOhm bias which is 833 Ohm | expand |
On Wed, Oct 14, 2020 at 01:46:37PM +0300, Andy Shevchenko wrote: > 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. > > Fixes: 7981c0015af2 ("pinctrl: intel: Add Intel Sunrisepoint pin controller and GPIO support") > Cc: Jamie McClymont <jamie@kwiius.com> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> Acked-by: Mika Westerberg <mika.westerberg@linux.intel.com>
On Wed, Oct 14, 2020 at 12:46 PM Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > 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. > > Fixes: 7981c0015af2 ("pinctrl: intel: Add Intel Sunrisepoint pin controller and GPIO support") > Cc: Jamie McClymont <jamie@kwiius.com> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> Good research! I expect this as part of a pull request for fixes or devel. Yours, Linus Walleij
On Thu, Nov 05, 2020 at 10:09:45AM +0100, Linus Walleij wrote: > On Wed, Oct 14, 2020 at 12:46 PM Andy Shevchenko > <andriy.shevchenko@linux.intel.com> wrote: > > > 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. > > > > Fixes: 7981c0015af2 ("pinctrl: intel: Add Intel Sunrisepoint pin controller and GPIO support") > > Cc: Jamie McClymont <jamie@kwiius.com> > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > > 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.
diff --git a/drivers/pinctrl/intel/pinctrl-intel.c b/drivers/pinctrl/intel/pinctrl-intel.c index 80b4c5cdc3f6..df626643f9e4 100644 --- a/drivers/pinctrl/intel/pinctrl-intel.c +++ b/drivers/pinctrl/intel/pinctrl-intel.c @@ -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; }
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. Fixes: 7981c0015af2 ("pinctrl: intel: Add Intel Sunrisepoint pin controller and GPIO support") Cc: Jamie McClymont <jamie@kwiius.com> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> --- drivers/pinctrl/intel/pinctrl-intel.c | 32 ++++++++++++++++++--------- 1 file changed, 22 insertions(+), 10 deletions(-)