linux-kernel - Re: [PATCH v4 07/10] gpio: max7360: Add MAX7360 gpio support

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: <Z9hGKCdR7NHqfRmC@smile.fi.intel.com>
Date: Mon, 17 Mar 2025 17:56:24 +0200
From: Andy Shevchenko <andy.shevchenko@...il.com>
To: Mathieu Dubois-Briand <mathieu.dubois-briand@...tlin.com>
Cc: Lee Jones <lee@...nel.org>, Rob Herring <robh@...nel.org>,
	Krzysztof Kozlowski <krzk+dt@...nel.org>,
	Conor Dooley <conor+dt@...nel.org>,
	Kamel Bouhara <kamel.bouhara@...tlin.com>,
	Linus Walleij <linus.walleij@...aro.org>,
	Bartosz Golaszewski <brgl@...ev.pl>,
	Dmitry Torokhov <dmitry.torokhov@...il.com>,
	Uwe Kleine-König <ukleinek@...nel.org>,
	Michael Walle <mwalle@...nel.org>, Mark Brown <broonie@...nel.org>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	"Rafael J. Wysocki" <rafael@...nel.org>,
	Danilo Krummrich <dakr@...nel.org>, devicetree@...r.kernel.org,
	linux-kernel@...r.kernel.org, linux-gpio@...r.kernel.org,
	linux-input@...r.kernel.org, linux-pwm@...r.kernel.org,
	Grégory Clement <gregory.clement@...tlin.com>,
	Thomas Petazzoni <thomas.petazzoni@...tlin.com>
Subject: Re: [PATCH v4 07/10] gpio: max7360: Add MAX7360 gpio support

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