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
