linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] pinctrl: amd: print debounce filter info in debugfs
@ 2020-10-26 15:16 Coiby Xu
  2020-10-26 15:22 ` Andy Shevchenko
  0 siblings, 1 reply; 4+ messages in thread
From: Coiby Xu @ 2020-10-26 15:16 UTC (permalink / raw)
  To: linux-gpio; +Cc: Linus Walleij, Hans de Goede, Andy Shevchenko, open list

Print the status of debounce filter as follows,
$ cat /sys/kernel/debug/gpio
pin129          interrupt is disabled| interrupt is masked| disable wakeup in S0i3 state| disable wakeup in S3 state|
 disable wakeup in S4/S5 state| input is high|   pull-up is disabled| Pull-down is disabled|   output is disabled| debouncing filter disabled|   0x50000
pin130          interrupt is disabled| interrupt is masked| disable wakeup in S0i3 state| disable wakeup in S3 state|
 disable wakeup in S4/S5 state| input is high|   pull-up is disabled| Pull-down is disabled|   output is disabled| debouncing filter (high) enabled| debouncing timeout is 124800 (us)| 0x503c8
                                                                                                                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Signed-off-by: Coiby Xu <coiby.xu@gmail.com>
---
 drivers/pinctrl/pinctrl-amd.c | 43 +++++++++++++++++++++++++++++++++--
 1 file changed, 41 insertions(+), 2 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-amd.c b/drivers/pinctrl/pinctrl-amd.c
index 9a760f5cd7ed..77782d420967 100644
--- a/drivers/pinctrl/pinctrl-amd.c
+++ b/drivers/pinctrl/pinctrl-amd.c
@@ -197,10 +197,16 @@ static int amd_gpio_set_config(struct gpio_chip *gc, unsigned offset,
 static void amd_gpio_dbg_show(struct seq_file *s, struct gpio_chip *gc)
 {
 	u32 pin_reg;
+	u32 reg_db_mask;
 	unsigned long flags;
 	unsigned int bank, i, pin_num;
 	struct amd_gpio *gpio_dev = gpiochip_get_data(gc);

+	bool tmr_out_unit;
+	unsigned int time;
+	unsigned int unit;
+	bool tmr_large;
+
 	char *level_trig;
 	char *active_level;
 	char *interrupt_enable;
@@ -214,6 +220,8 @@ static void amd_gpio_dbg_show(struct seq_file *s, struct gpio_chip *gc)
 	char *pull_down_enable;
 	char *output_value;
 	char *output_enable;
+	char debounce_value[40];
+	char *debounce_enable;

 	for (bank = 0; bank < gpio_dev->hwbank_num; bank++) {
 		seq_printf(s, "GPIO bank%d\t", bank);
@@ -327,13 +335,44 @@ static void amd_gpio_dbg_show(struct seq_file *s, struct gpio_chip *gc)
 					pin_sts = "input is low|";
 			}

+			reg_db_mask = (DB_CNTRl_MASK << DB_CNTRL_OFF) & pin_reg;
+			if (reg_db_mask & pin_reg) {
+				tmr_out_unit = pin_reg & BIT(DB_TMR_OUT_UNIT_OFF);
+				tmr_large = pin_reg & BIT(DB_TMR_LARGE_OFF);
+				time = pin_reg & DB_TMR_OUT_MASK;
+				if (tmr_large) {
+					if (tmr_out_unit)
+						unit = 62500;
+					else
+						unit = 15600;
+				} else {
+					if (tmr_out_unit)
+						unit = 244;
+					else
+						unit = 61;
+				}
+				if ((DB_TYPE_REMOVE_GLITCH << DB_CNTRL_OFF) == reg_db_mask)
+					debounce_enable = "debouncing filter (high and low) enabled|";
+				else if ((DB_TYPE_PRESERVE_LOW_GLITCH << DB_CNTRL_OFF) == reg_db_mask)
+					debounce_enable = "debouncing filter (low) enabled|";
+				else
+					debounce_enable = "debouncing filter (high) enabled|";
+
+				snprintf(debounce_value, 40,
+					 "debouncing timeout is %u (us)|", time * unit);
+			} else {
+				debounce_enable = "debouncing filter disabled|";
+				snprintf(debounce_value, 40, " ");
+			}
+
 			seq_printf(s, "%s %s %s %s %s %s\n"
-				" %s %s %s %s %s %s %s 0x%x\n",
+				" %s %s %s %s %s %s %s %s %s 0x%x\n",
 				level_trig, active_level, interrupt_enable,
 				interrupt_mask, wake_cntrl0, wake_cntrl1,
 				wake_cntrl2, pin_sts, pull_up_sel,
 				pull_up_enable, pull_down_enable,
-				output_value, output_enable, pin_reg);
+				output_value, output_enable,
+				debounce_enable, debounce_value, pin_reg);
 		}
 	}
 }
--
2.28.0


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

* Re: [PATCH v2] pinctrl: amd: print debounce filter info in debugfs
  2020-10-26 15:16 [PATCH v2] pinctrl: amd: print debounce filter info in debugfs Coiby Xu
@ 2020-10-26 15:22 ` Andy Shevchenko
  2020-10-26 23:16   ` Coiby Xu
  0 siblings, 1 reply; 4+ messages in thread
From: Andy Shevchenko @ 2020-10-26 15:22 UTC (permalink / raw)
  To: Coiby Xu
  Cc: open list:GPIO SUBSYSTEM, Linus Walleij, Hans de Goede, open list

On Mon, Oct 26, 2020 at 5:16 PM Coiby Xu <coiby.xu@gmail.com> wrote:
>
> Print the status of debounce filter as follows,
> $ cat /sys/kernel/debug/gpio
> pin129          interrupt is disabled| interrupt is masked| disable wakeup in S0i3 state| disable wakeup in S3 state|
>  disable wakeup in S4/S5 state| input is high|   pull-up is disabled| Pull-down is disabled|   output is disabled| debouncing filter disabled|   0x50000
> pin130          interrupt is disabled| interrupt is masked| disable wakeup in S0i3 state| disable wakeup in S3 state|
>  disable wakeup in S4/S5 state| input is high|   pull-up is disabled| Pull-down is disabled|   output is disabled| debouncing filter (high) enabled| debouncing timeout is 124800 (us)| 0x503c8
>                                                                                                                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Thanks for an update!
In general looks good, one nit below (sorry, missed it in v1 round)

...

> +       char debounce_value[40];

(1)

...

> +                               if (tmr_large) {
> +                                       if (tmr_out_unit)
> +                                               unit = 62500;
> +                                       else

> +                                               unit = 15600;

Side note: Hmm... Shouldn't be 15625? As 1/4.

> +                               } else {
> +                                       if (tmr_out_unit)
> +                                               unit = 244;
> +                                       else
> +                                               unit = 61;

...


> +                               snprintf(debounce_value, 40,
> +                                        "debouncing timeout is %u (us)|", time * unit);

(2)

...

> +                               snprintf(debounce_value, 40, " ");

(3)

Because of definition (1) can you in (2) and (3) use sizeof() ?

-- 
With Best Regards,
Andy Shevchenko

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

* Re: [PATCH v2] pinctrl: amd: print debounce filter info in debugfs
  2020-10-26 15:22 ` Andy Shevchenko
@ 2020-10-26 23:16   ` Coiby Xu
  2020-10-27  9:42     ` Andy Shevchenko
  0 siblings, 1 reply; 4+ messages in thread
From: Coiby Xu @ 2020-10-26 23:16 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: open list:GPIO SUBSYSTEM, Linus Walleij, Hans de Goede, open list

On Mon, Oct 26, 2020 at 05:22:45PM +0200, Andy Shevchenko wrote:
>On Mon, Oct 26, 2020 at 5:16 PM Coiby Xu <coiby.xu@gmail.com> wrote:
>>
>> Print the status of debounce filter as follows,
>> $ cat /sys/kernel/debug/gpio
>> pin129          interrupt is disabled| interrupt is masked| disable wakeup in S0i3 state| disable wakeup in S3 state|
>>  disable wakeup in S4/S5 state| input is high|   pull-up is disabled| Pull-down is disabled|   output is disabled| debouncing filter disabled|   0x50000
>> pin130          interrupt is disabled| interrupt is masked| disable wakeup in S0i3 state| disable wakeup in S3 state|
>>  disable wakeup in S4/S5 state| input is high|   pull-up is disabled| Pull-down is disabled|   output is disabled| debouncing filter (high) enabled| debouncing timeout is 124800 (us)| 0x503c8
>>                                                                                                                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>
>Thanks for an update!
>In general looks good, one nit below (sorry, missed it in v1 round)
>
Thank you for the feedbacks!
>...
>
>> +       char debounce_value[40];
>
>(1)
>
>...
>
>> +                               if (tmr_large) {
>> +                                       if (tmr_out_unit)
>> +                                               unit = 62500;
>> +                                       else
>
>> +                                               unit = 15600;
>
>Side note: Hmm... Shouldn't be 15625? As 1/4.

Thank you for discovering the inconsistency! I wrote these code based on
amd_gpio_set_debounce. I'll send an email to the original author to
confirm it.

static int amd_gpio_set_debounce(struct gpio_chip *gc, unsigned offset,
		unsigned debounce)
{
     ...
	if (debounce) {
		pin_reg |= DB_TYPE_REMOVE_GLITCH << DB_CNTRL_OFF;
		pin_reg &= ~DB_TMR_OUT_MASK;
		/*
		Debounce	Debounce	Timer	Max
		TmrLarge	TmrOutUnit	Unit	Debounce
							Time
		0	0	61 usec (2 RtcClk)	976 usec
		0	1	244 usec (8 RtcClk)	3.9 msec
		1	0	15.6 msec (512 RtcClk)	250 msec
		1	1	62.5 msec (2048 RtcClk)	1 sec
		*/

		if (debounce < 61) {
			pin_reg |= 1;
			pin_reg &= ~BIT(DB_TMR_OUT_UNIT_OFF);
			pin_reg &= ~BIT(DB_TMR_LARGE_OFF);
		} else if (debounce < 976) {
			time = debounce / 61;
			pin_reg |= time & DB_TMR_OUT_MASK;
			pin_reg &= ~BIT(DB_TMR_OUT_UNIT_OFF);
			pin_reg &= ~BIT(DB_TMR_LARGE_OFF);
         ...
>
>> +                               } else {
>> +                                       if (tmr_out_unit)
>> +                                               unit = 244;
>> +                                       else
>> +                                               unit = 61;
>
>...
>
>
>> +                               snprintf(debounce_value, 40,
>> +                                        "debouncing timeout is %u (us)|", time * unit);
>
>(2)
>
>...
>
>> +                               snprintf(debounce_value, 40, " ");
>
>(3)
>
>Because of definition (1) can you in (2) and (3) use sizeof() ?
>
I've considered defining a constant. Obviously sizeof is a better
idea:)

>--
>With Best Regards,
>Andy Shevchenko

--
Best regards,
Coiby

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

* Re: [PATCH v2] pinctrl: amd: print debounce filter info in debugfs
  2020-10-26 23:16   ` Coiby Xu
@ 2020-10-27  9:42     ` Andy Shevchenko
  0 siblings, 0 replies; 4+ messages in thread
From: Andy Shevchenko @ 2020-10-27  9:42 UTC (permalink / raw)
  To: Coiby Xu
  Cc: open list:GPIO SUBSYSTEM, Linus Walleij, Hans de Goede, open list

On Tue, Oct 27, 2020 at 1:16 AM Coiby Xu <coiby.xu@gmail.com> wrote:
> On Mon, Oct 26, 2020 at 05:22:45PM +0200, Andy Shevchenko wrote:
> >On Mon, Oct 26, 2020 at 5:16 PM Coiby Xu <coiby.xu@gmail.com> wrote:

...

> >> +                               if (tmr_large) {
> >> +                                       if (tmr_out_unit)
> >> +                                               unit = 62500;
> >> +                                       else
> >
> >> +                                               unit = 15600;
> >
> >Side note: Hmm... Shouldn't be 15625? As 1/4.
>
> Thank you for discovering the inconsistency! I wrote these code based on
> amd_gpio_set_debounce. I'll send an email to the original author to
> confirm it.
>
> static int amd_gpio_set_debounce(struct gpio_chip *gc, unsigned offset,
>                 unsigned debounce)
> {
>      ...
>         if (debounce) {
>                 pin_reg |= DB_TYPE_REMOVE_GLITCH << DB_CNTRL_OFF;
>                 pin_reg &= ~DB_TMR_OUT_MASK;
>                 /*
>                 Debounce        Debounce        Timer   Max
>                 TmrLarge        TmrOutUnit      Unit    Debounce
>                                                         Time
>                 0       0       61 usec (2 RtcClk)      976 usec
>                 0       1       244 usec (8 RtcClk)     3.9 msec
>                 1       0       15.6 msec (512 RtcClk)  250 msec
>                 1       1       62.5 msec (2048 RtcClk) 1 sec
>                 */

What the heck with HW companies! (Just an emotion based on the experience)
They like to use really bad precision when it's clear that the numbers
should be different (note the cycles, it's 1/4 sharp ratio).

>                 if (debounce < 61) {
>                         pin_reg |= 1;
>                         pin_reg &= ~BIT(DB_TMR_OUT_UNIT_OFF);
>                         pin_reg &= ~BIT(DB_TMR_LARGE_OFF);
>                 } else if (debounce < 976) {
>                         time = debounce / 61;
>                         pin_reg |= time & DB_TMR_OUT_MASK;
>                         pin_reg &= ~BIT(DB_TMR_OUT_UNIT_OFF);
>                         pin_reg &= ~BIT(DB_TMR_LARGE_OFF);
>          ...
> >
> >> +                               } else {
> >> +                                       if (tmr_out_unit)
> >> +                                               unit = 244;
> >> +                                       else
> >> +                                               unit = 61;

-- 
With Best Regards,
Andy Shevchenko

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

end of thread, other threads:[~2020-10-27  9:42 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-26 15:16 [PATCH v2] pinctrl: amd: print debounce filter info in debugfs Coiby Xu
2020-10-26 15:22 ` Andy Shevchenko
2020-10-26 23:16   ` Coiby Xu
2020-10-27  9:42     ` Andy Shevchenko

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).