All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v1 1/2] pinctrl: intel: Fix 2 kOhm bias which is 833 Ohm
@ 2020-10-14 10:46 Andy Shevchenko
  2020-10-14 10:46 ` [PATCH v1 2/2] pinctrl: intel: Set default bias in case no particular value given Andy Shevchenko
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Andy Shevchenko @ 2020-10-14 10:46 UTC (permalink / raw)
  To: Mika Westerberg, linux-gpio, Linus Walleij
  Cc: Andy Shevchenko, Jamie McClymont

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(-)

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.28.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v1 2/2] pinctrl: intel: Set default bias in case no particular value given
  2020-10-14 10:46 [PATCH v1 1/2] pinctrl: intel: Fix 2 kOhm bias which is 833 Ohm Andy Shevchenko
@ 2020-10-14 10:46 ` Andy Shevchenko
  2020-10-21  9:54   ` Mika Westerberg
  2020-10-21  9:51 ` [PATCH v1 1/2] pinctrl: intel: Fix 2 kOhm bias which is 833 Ohm Mika Westerberg
  2020-11-05  9:09 ` Linus Walleij
  2 siblings, 1 reply; 6+ messages in thread
From: Andy Shevchenko @ 2020-10-14 10:46 UTC (permalink / raw)
  To: Mika Westerberg, linux-gpio, Linus Walleij
  Cc: Andy Shevchenko, Jamie McClymont

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).

Fixes: e57725eabf87 ("pinctrl: intel: Add support for hardware debouncer")
Reported-by: Jamie McClymont <jamie@kwiius.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/pinctrl/intel/pinctrl-intel.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/pinctrl/intel/pinctrl-intel.c b/drivers/pinctrl/intel/pinctrl-intel.c
index df626643f9e4..6d23137489c1 100644
--- a/drivers/pinctrl/intel/pinctrl-intel.c
+++ b/drivers/pinctrl/intel/pinctrl-intel.c
@@ -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;
-- 
2.28.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v1 1/2] pinctrl: intel: Fix 2 kOhm bias which is 833 Ohm
  2020-10-14 10:46 [PATCH v1 1/2] pinctrl: intel: Fix 2 kOhm bias which is 833 Ohm Andy Shevchenko
  2020-10-14 10:46 ` [PATCH v1 2/2] pinctrl: intel: Set default bias in case no particular value given Andy Shevchenko
@ 2020-10-21  9:51 ` Mika Westerberg
  2020-11-05  9:09 ` Linus Walleij
  2 siblings, 0 replies; 6+ messages in thread
From: Mika Westerberg @ 2020-10-21  9:51 UTC (permalink / raw)
  To: Andy Shevchenko; +Cc: linux-gpio, Linus Walleij, Jamie McClymont

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>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v1 2/2] pinctrl: intel: Set default bias in case no particular value given
  2020-10-14 10:46 ` [PATCH v1 2/2] pinctrl: intel: Set default bias in case no particular value given Andy Shevchenko
@ 2020-10-21  9:54   ` Mika Westerberg
  0 siblings, 0 replies; 6+ messages in thread
From: Mika Westerberg @ 2020-10-21  9:54 UTC (permalink / raw)
  To: Andy Shevchenko; +Cc: linux-gpio, Linus Walleij, Jamie McClymont

On Wed, Oct 14, 2020 at 01:46:38PM +0300, Andy Shevchenko wrote:
> 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).
> 
> Fixes: e57725eabf87 ("pinctrl: intel: Add support for hardware debouncer")
> Reported-by: Jamie McClymont <jamie@kwiius.com>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Acked-by: Mika Westerberg <mika.westerberg@linux.intel.com>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v1 1/2] pinctrl: intel: Fix 2 kOhm bias which is 833 Ohm
  2020-10-14 10:46 [PATCH v1 1/2] pinctrl: intel: Fix 2 kOhm bias which is 833 Ohm Andy Shevchenko
  2020-10-14 10:46 ` [PATCH v1 2/2] pinctrl: intel: Set default bias in case no particular value given Andy Shevchenko
  2020-10-21  9:51 ` [PATCH v1 1/2] pinctrl: intel: Fix 2 kOhm bias which is 833 Ohm Mika Westerberg
@ 2020-11-05  9:09 ` Linus Walleij
  2020-11-05 16:22   ` Andy Shevchenko
  2 siblings, 1 reply; 6+ messages in thread
From: Linus Walleij @ 2020-11-05  9:09 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Mika Westerberg, open list:GPIO SUBSYSTEM, Jamie McClymont

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v1 1/2] pinctrl: intel: Fix 2 kOhm bias which is 833 Ohm
  2020-11-05  9:09 ` Linus Walleij
@ 2020-11-05 16:22   ` Andy Shevchenko
  0 siblings, 0 replies; 6+ messages in thread
From: Andy Shevchenko @ 2020-11-05 16:22 UTC (permalink / raw)
  To: Linus Walleij; +Cc: Mika Westerberg, open list:GPIO SUBSYSTEM, Jamie McClymont

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.

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2020-11-05 16:21 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-14 10:46 [PATCH v1 1/2] pinctrl: intel: Fix 2 kOhm bias which is 833 Ohm Andy Shevchenko
2020-10-14 10:46 ` [PATCH v1 2/2] pinctrl: intel: Set default bias in case no particular value given Andy Shevchenko
2020-10-21  9:54   ` Mika Westerberg
2020-10-21  9:51 ` [PATCH v1 1/2] pinctrl: intel: Fix 2 kOhm bias which is 833 Ohm Mika Westerberg
2020-11-05  9:09 ` Linus Walleij
2020-11-05 16:22   ` Andy Shevchenko

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.