linux-kernel - Re: [PATCH] pinctrl: sunxi: Use minimal debouncing period as default

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CACRpkdbqufNVq4Ai6GQpgK2OY0rxLnp1wLQNmRoCv44T0xmFkQ@mail.gmail.com>
Date: Tue, 17 Dec 2024 14:39:03 +0100
From: Linus Walleij <linus.walleij@...aro.org>
To: Paul Kocialkowski <paulk@...-base.io>
Cc: Maxime Ripard <mripard@...nel.org>, linux-arm-kernel@...ts.infradead.org, 
	linux-sunxi@...ts.linux.dev, linux-kernel@...r.kernel.org, 
	Uwe Kleine-König <ukleinek@...nel.org>, 
	Chen-Yu Tsai <wens@...e.org>, Jernej Skrabec <jernej.skrabec@...il.com>, 
	Samuel Holland <samuel@...lland.org>, Paul Kocialkowski <contact@...lk.fr>
Subject: Re: [PATCH] pinctrl: sunxi: Use minimal debouncing period as default

Some discussion here, and some emotions involved.

I can't seem to follow the technical matter because of all the
social matters :/

On Tue, Nov 19, 2024 at 4:00 PM Paul Kocialkowski <paulk@...-base.io> wrote:

> My use-case here was hooking up a sparkfun sensor board by the way,
> not some very advanced corner-case.

Are you adding this as a DT overlay or by modifying an existing device
tree? Does this sensor have an established device tree binding?

Are you using that sparkfun sensor by a kernel driver or from userspace
using the GPIO character device?

I noticed that the sunxi GPIO driver is implementing the
.set_config() callback calling gpiochip_generic_config,
which makes it call down to the pin control driver to set up
the pin config.

which would in turn make it eligible to use
the gpiod_set_debounce() callback to push down the debounce
period.

But pinctrl-sunxi.c's sunxi_pconf_set() does *NOT* implement
support for setting up the debounce, because it is (as I understand
it) not part of the pin config hardware, but part of the interrupt
generator hardware, correct?

In that case I think we the gpiochip .set_config() callback should
be modified something like this (pseudo code):

sunxi_pinctrl_gpio_set_config()
{
    if (irq_is_in_use && param_is_debounce) {
        modify_irq_debounce_according_to_param()
        return 0;
    }
    gpiochip_generic_config()
}

pctl->chip->set_config = sunxi_pinctrl_gpio_set_config()

Maybe the debounce can also be set even if the line is not used
for IRQ? I'm not sure.

In either case the latter would give the GPIO driver a handle
on the debounce, which is good because the irqchip
generally does not.

There is a way it is possible to use the interrupt with desired debounce
setting by first getting the GPIO descriptor and modify the debounce
setting and then getting the interrupt from the GPIO descriptor:

struct gpio_desc *g;
int irq;

g = gpiod_get(dev, "dataready", GPIOD_IN);
gpiod_set_debounce(g, 1);
irq = gpiod_to_irq(g);
...request irq...

Here I assume the line out from the sensor is named "dataready"
the actual component likely has a name like that for the line.

This requires changes in the device tree as well, so that a GPIO
line is assigned to the sensor instead of "just" an interrupt:

sensor {
  dataready-gpios = <&gpio 14>;
  ...
};

instead of:

sensor {
  interrupt-parent = <&gpio>;
  interrupts = <14 IRQ_TYPE_EDGE_RISING>;
};

Unfortunately this is one of the areas where DT bindings are
a bit ambiguous. Some just assign the GPIO line as an interrupt
as both APIs are available, sometimes a proper GPIO line is
preferred for the above reasons.

Yours,
Linus Walleij