All of lore.kernel.org
 help / color / mirror / Atom feed
From: Coiby Xu <coiby.xu@gmail.com>
To: Hans de Goede <hdegoede@redhat.com>
Cc: Linus Walleij <linus.walleij@linaro.org>,
	"open list:GPIO SUBSYSTEM" <linux-gpio@vger.kernel.org>,
	wang jun <availa@outlook.com>,
	Nehal Shah <Nehal-bakulchandra.Shah@amd.com>,
	Shyam Sundar S K <Shyam-sundar.S-k@amd.com>,
	linux-kernel-mentees@lists.linuxfoundation.org
Subject: Re: Any other ways to debug GPIO interrupt controller (pinctrl-amd) for broken touchpads of a new laptop model?
Date: Thu, 15 Oct 2020 12:06:29 +0800	[thread overview]
Message-ID: <20201015040629.ycv5nc7vit6wcynu@Rk> (raw)
In-Reply-To: <df2c008b-e7b5-4fdd-42ea-4d1c62b52139@redhat.com>

On Wed, Oct 14, 2020 at 01:46:14PM +0200, Hans de Goede wrote:
>Hi,
>
[...]
>
>I've never seen this kinda glitch/debounce filter where
>you can filter out only one type of level before, so
>I wonder if the code maybe simply got it wrong, also for
>a level type irq I really see no objection to just
>use DB_TYPE_REMOVE_GLITCH instead of the weird "half"
>filters.
>
>So I just ran a git blame and the DB_TYPE_PRESERVE_HIGH_GLITCH
>has been there from the very first commit of this driver,
>I wonder if it has been wrong all this time and should be
>inverted (so DB_TYPE_PRESERVE_LOW_GLITCH instead).
>
>I think we may want to just play it safe though and simply
>switch to DB_TYPE_REMOVE_GLITCH as we already do for all
>edge types and when amd_gpio_set_config() gets called!
>
>Linus, what do you think about just switching to
>DB_TYPE_REMOVE_GLITCH for level type irqs (unifying them
>with all the other modes) and not mucking with this weird,
>undocumented "half" filter modes ?
>
>>Or can you point to some references? I've gain some
>>experience about how to configure the GPIO controller by studying the
>>code of pinctrl-amd and pinctrl-baytrail (I can't find the hardware
>>reference manual for baytrail either). I also tweaked the configuration
>>in pinctrl-amd, for example, setting the debounce timeout to 976 usec
>>and 3.9 msec without disabling the glitch filter could also save the
>>touchpad. But I need some knowledge to understand why this touchpad [1]
>>which also uses the buggy pinctrl-amd isn't affected.
>>
>>[1] https://github.com/Syniurge/i2c-amd-mp2/issues/11#issuecomment-707427095
>
>My guess would be that it uses edge types interrupts instead ?
>I have seen that quite a few times, even though it is weird
>to do that for i2c devices.
>
Actually it uses the level type interrupt according to the shared
DSDT.dsl,

         Device (TPDA)
         {
             Name (_HID, "SYNA2B3F")  // _HID: Hardware ID
             Name (_CID, "PNP0C50" /* HID Protocol Device (I2C bus) */)  // _CID: Compatible ID
             Name (_UID, 0x08)  // _UID: Unique ID
             Method (_CRS, 0, NotSerialized)  // _CRS: Current Resource Settings
             {
                 Name (RBUF, ResourceTemplate ()
                 {
                     I2cSerialBusV2 (0x002C, ControllerInitiated, 0x00061A80,
                         AddressingMode7Bit, "\\_SB.I2CD",
                         0x00, ResourceConsumer, , Exclusive,
                         )
                     GpioInt (Level, ActiveLow, ExclusiveAndWake, PullUp, 0x0000,
                         "\\_SB.GPIO", 0x00, ResourceConsumer, ,
                         )
                         {   // Pin list
                             0x0003
                         }
                 })
                 Return (RBUF) /* \_SB_.I2CD.TPDA._CRS.RBUF */
             }

>Regards,
>
>Hans
>

--
Best regards,
Coiby

WARNING: multiple messages have this Message-ID (diff)
From: Coiby Xu <coiby.xu@gmail.com>
To: Hans de Goede <hdegoede@redhat.com>
Cc: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>,
	Linus Walleij <linus.walleij@linaro.org>,
	wang jun <availa@outlook.com>,
	"open list:GPIO SUBSYSTEM" <linux-gpio@vger.kernel.org>,
	linux-kernel-mentees@lists.linuxfoundation.org,
	Nehal Shah <Nehal-bakulchandra.Shah@amd.com>
Subject: Re: [Linux-kernel-mentees] Any other ways to debug GPIO interrupt controller (pinctrl-amd) for broken touchpads of a new laptop model?
Date: Thu, 15 Oct 2020 12:06:29 +0800	[thread overview]
Message-ID: <20201015040629.ycv5nc7vit6wcynu@Rk> (raw)
In-Reply-To: <df2c008b-e7b5-4fdd-42ea-4d1c62b52139@redhat.com>

On Wed, Oct 14, 2020 at 01:46:14PM +0200, Hans de Goede wrote:
>Hi,
>
[...]
>
>I've never seen this kinda glitch/debounce filter where
>you can filter out only one type of level before, so
>I wonder if the code maybe simply got it wrong, also for
>a level type irq I really see no objection to just
>use DB_TYPE_REMOVE_GLITCH instead of the weird "half"
>filters.
>
>So I just ran a git blame and the DB_TYPE_PRESERVE_HIGH_GLITCH
>has been there from the very first commit of this driver,
>I wonder if it has been wrong all this time and should be
>inverted (so DB_TYPE_PRESERVE_LOW_GLITCH instead).
>
>I think we may want to just play it safe though and simply
>switch to DB_TYPE_REMOVE_GLITCH as we already do for all
>edge types and when amd_gpio_set_config() gets called!
>
>Linus, what do you think about just switching to
>DB_TYPE_REMOVE_GLITCH for level type irqs (unifying them
>with all the other modes) and not mucking with this weird,
>undocumented "half" filter modes ?
>
>>Or can you point to some references? I've gain some
>>experience about how to configure the GPIO controller by studying the
>>code of pinctrl-amd and pinctrl-baytrail (I can't find the hardware
>>reference manual for baytrail either). I also tweaked the configuration
>>in pinctrl-amd, for example, setting the debounce timeout to 976 usec
>>and 3.9 msec without disabling the glitch filter could also save the
>>touchpad. But I need some knowledge to understand why this touchpad [1]
>>which also uses the buggy pinctrl-amd isn't affected.
>>
>>[1] https://github.com/Syniurge/i2c-amd-mp2/issues/11#issuecomment-707427095
>
>My guess would be that it uses edge types interrupts instead ?
>I have seen that quite a few times, even though it is weird
>to do that for i2c devices.
>
Actually it uses the level type interrupt according to the shared
DSDT.dsl,

         Device (TPDA)
         {
             Name (_HID, "SYNA2B3F")  // _HID: Hardware ID
             Name (_CID, "PNP0C50" /* HID Protocol Device (I2C bus) */)  // _CID: Compatible ID
             Name (_UID, 0x08)  // _UID: Unique ID
             Method (_CRS, 0, NotSerialized)  // _CRS: Current Resource Settings
             {
                 Name (RBUF, ResourceTemplate ()
                 {
                     I2cSerialBusV2 (0x002C, ControllerInitiated, 0x00061A80,
                         AddressingMode7Bit, "\\_SB.I2CD",
                         0x00, ResourceConsumer, , Exclusive,
                         )
                     GpioInt (Level, ActiveLow, ExclusiveAndWake, PullUp, 0x0000,
                         "\\_SB.GPIO", 0x00, ResourceConsumer, ,
                         )
                         {   // Pin list
                             0x0003
                         }
                 })
                 Return (RBUF) /* \_SB_.I2CD.TPDA._CRS.RBUF */
             }

>Regards,
>
>Hans
>

--
Best regards,
Coiby
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

  parent reply	other threads:[~2020-10-15  4:06 UTC|newest]

Thread overview: 84+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-10-01 13:22 Any other ways to debug GPIO interrupt controller (pinctrl-amd) for broken touchpads of a new laptop model? Coiby Xu
2020-10-01 13:22 ` [Linux-kernel-mentees] " Coiby Xu
2020-10-01 20:57 ` Linus Walleij
2020-10-01 20:57   ` [Linux-kernel-mentees] " Linus Walleij
2020-10-02  9:40   ` Hans de Goede
2020-10-02  9:40     ` [Linux-kernel-mentees] " Hans de Goede
2020-10-02 12:42     ` Coiby Xu
2020-10-02 12:42       ` [Linux-kernel-mentees] " Coiby Xu
2020-10-02 13:36       ` Hans de Goede
2020-10-02 13:36         ` [Linux-kernel-mentees] " Hans de Goede
2020-10-02 14:51         ` Coiby Xu
2020-10-02 14:51           ` [Linux-kernel-mentees] " Coiby Xu
2020-10-02 19:44           ` Hans de Goede
2020-10-02 19:44             ` [Linux-kernel-mentees] " Hans de Goede
2020-10-02 22:45             ` Coiby Xu
2020-10-02 22:45               ` [Linux-kernel-mentees] " Coiby Xu
2020-10-03 13:22               ` Hans de Goede
2020-10-03 13:22                 ` [Linux-kernel-mentees] " Hans de Goede
2020-10-03 23:03                 ` Coiby Xu
2020-10-03 23:03                   ` [Linux-kernel-mentees] " Coiby Xu
2020-10-04  5:16                   ` Coiby Xu
2020-10-04  5:16                     ` [Linux-kernel-mentees] " Coiby Xu
2020-10-06  4:49                     ` Coiby Xu
2020-10-06  4:49                       ` [Linux-kernel-mentees] " Coiby Xu
2020-10-06  6:28                       ` Hans de Goede
2020-10-06  6:28                         ` [Linux-kernel-mentees] " Hans de Goede
2020-10-06  8:31                         ` Coiby Xu
2020-10-06  8:31                           ` [Linux-kernel-mentees] " Coiby Xu
2020-10-06  8:55                           ` Hans de Goede
2020-10-06  8:55                             ` [Linux-kernel-mentees] " Hans de Goede
2020-10-06  9:28                             ` Hans de Goede
2020-10-06  9:28                               ` [Linux-kernel-mentees] " Hans de Goede
2020-10-06  9:29                               ` Hans de Goede
2020-10-06  9:29                                 ` [Linux-kernel-mentees] " Hans de Goede
2020-10-08 16:32                                 ` Coiby Xu
2020-10-08 16:32                                   ` [Linux-kernel-mentees] " Coiby Xu
2020-10-14  4:24                                 ` Coiby Xu
2020-10-14  4:24                                   ` [Linux-kernel-mentees] " Coiby Xu
2020-10-14 11:34                                   ` Coiby Xu
2020-10-14 11:34                                     ` [Linux-kernel-mentees] " Coiby Xu
2020-10-14 11:46                                   ` Hans de Goede
2020-10-14 11:46                                     ` [Linux-kernel-mentees] " Hans de Goede
2020-10-15  3:27                                     ` Coiby Xu
2020-10-15  3:27                                       ` [Linux-kernel-mentees] " Coiby Xu
2020-10-15  4:06                                     ` Coiby Xu [this message]
2020-10-15  4:06                                       ` Coiby Xu
2020-10-26 22:54                                     ` Coiby Xu
2020-10-26 22:54                                       ` [Linux-kernel-mentees] " Coiby Xu
2020-10-27  9:52                                       ` Andy Shevchenko
2020-10-27  9:52                                         ` [Linux-kernel-mentees] " Andy Shevchenko
2020-10-30  4:58                                         ` Coiby Xu
2020-10-30  4:58                                           ` [Linux-kernel-mentees] " Coiby Xu
2020-10-27 10:09                                       ` Hans de Goede
2020-10-27 10:09                                         ` [Linux-kernel-mentees] " Hans de Goede
2020-10-27 15:13                                         ` Andy Shevchenko
2020-10-27 15:13                                           ` [Linux-kernel-mentees] " Andy Shevchenko
2020-10-27 16:00                                           ` Hans de Goede
2020-10-27 16:00                                             ` [Linux-kernel-mentees] " Hans de Goede
2020-10-27 16:09                                             ` Andy Shevchenko
2020-10-27 16:09                                               ` [Linux-kernel-mentees] " Andy Shevchenko
2020-10-29  8:04                                               ` Mika Westerberg
2020-10-29  8:04                                                 ` [Linux-kernel-mentees] " Mika Westerberg
2020-10-30  4:54                                               ` Coiby Xu
2020-10-30  4:54                                                 ` [Linux-kernel-mentees] " Coiby Xu
2020-11-02 19:06                                                 ` Andy Shevchenko
2020-11-02 19:06                                                   ` [Linux-kernel-mentees] " Andy Shevchenko
2020-11-02 22:56                                                   ` Coiby Xu
2020-11-02 22:56                                                     ` [Linux-kernel-mentees] " Coiby Xu
2020-11-03  0:05                                         ` Coiby Xu
2020-11-03  0:05                                           ` [Linux-kernel-mentees] " Coiby Xu
2020-11-03 10:12                                           ` Hans de Goede
2020-11-03 10:12                                             ` [Linux-kernel-mentees] " Hans de Goede
2020-11-03 10:49                                             ` Andy Shevchenko
2020-11-03 10:49                                               ` [Linux-kernel-mentees] " Andy Shevchenko
2020-11-03 11:00                                               ` Hans de Goede
2020-11-03 11:00                                                 ` [Linux-kernel-mentees] " Hans de Goede
2020-10-08 16:26                               ` Coiby Xu
2020-10-08 16:26                                 ` [Linux-kernel-mentees] " Coiby Xu
2020-10-06  9:16                           ` Linus Walleij
2020-10-06  9:16                             ` [Linux-kernel-mentees] " Linus Walleij
2020-10-08 16:40                             ` Coiby Xu
2020-10-08 16:40                               ` [Linux-kernel-mentees] " Coiby Xu
2020-10-02 10:59   ` Coiby Xu
2020-10-02 10:59     ` [Linux-kernel-mentees] " Coiby Xu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20201015040629.ycv5nc7vit6wcynu@Rk \
    --to=coiby.xu@gmail.com \
    --cc=Nehal-bakulchandra.Shah@amd.com \
    --cc=Shyam-sundar.S-k@amd.com \
    --cc=availa@outlook.com \
    --cc=hdegoede@redhat.com \
    --cc=linus.walleij@linaro.org \
    --cc=linux-gpio@vger.kernel.org \
    --cc=linux-kernel-mentees@lists.linuxfoundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.