On Mon, Mar 17, 2025 at 03:13:07PM +0100, Mathieu Dubois-Briand wrote:
> On Fri Mar 14, 2025 at 9:14 AM CET, Andy Shevchenko wrote:
> > Thu, Mar 13, 2025 at 06:07:03PM +0100, Mathieu Dubois-Briand kirjoitti:
> > > On Fri Feb 14, 2025 at 4:59 PM CET, Andy Shevchenko wrote:
> > > > On Fri, Feb 14, 2025 at 12:49:57PM +0100, Mathieu Dubois-Briand wrote:
...
> > > > > + /*
> > > > > + * MAX7360_REG_DEBOUNCE contains configuration both for keypad debounce
> > > > > + * timings and gpos/keypad columns repartition. Only the later is
> > > > > + * modified here.
> > > > > + */
> > > > > + val = FIELD_PREP(MAX7360_PORTS, ngpios);
> > > > > + ret = regmap_write_bits(regmap, MAX7360_REG_DEBOUNCE, MAX7360_PORTS, val);
> > > > > + if (ret) {
> > > > > + dev_err(dev, "Failed to write max7360 columns/gpos configuration");
> > > > > + return ret;
> > > > > + }
> > > >
> > > > Shouldn't this be configured via ->set_config() callback?
> > >
> > > I believe this comment has been a bit outdated by our discussion on
> > > using GPIO valid mask, but I believe we could not use the ->set_config()
> > > callback here: this callback is made to configure a single pin while the
> > > gpos/keypad columns repartition is global.
> >
> > Yeah, we have similar desing in Intel Bay Trail (see pinctrl-baytrail.c) and it
> > requires some software driven heuristics on how individual setting may affect
> > the global one. But the Q here is is the debounce affects only keypad? Then it
> > should be configured via keypad matrix driver. Btw, have you checked
> > drivers/input/keyboard/matrix_keypad.c? Is there anything that can be useful
> > here?
> >
>
> Hum, maybe the comment is not clear enough? Not sure, but please tell
> me.
I see it now, yes, the comment seems point too much attention on the register
(and hence its name) then content.
I would start this comment with something like:
"Configure which GPIOs will be used for keypad."
> So yes, this register is named "debounce" but controls two different
> things:
> - The keypad debounce: we do not touch it here.
> - The partition between keypad columns and gpos. This is the value we do
> modify here.
--
With Best Regards,
Andy Shevchenko
