[PATCH] pinctrl: sunxi: Use minimal debouncing period as default

[Linus Walleij]

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 at sys-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