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 for Android: free password hash cracker in your pocket

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

Message-ID: <Z9PikuvAR-XsYhPF@surfacebook.localdomain>
Date: Fri, 14 Mar 2025 10:02:26 +0200
From: Andy Shevchenko <andy.shevchenko@...il.com>
To: Mathieu Dubois-Briand <mathieu.dubois-briand@...tlin.com>
Cc: Andy Shevchenko <andriy.shevchenko@...el.com>,
	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

Thu, Mar 13, 2025 at 05:43:00PM +0100, Mathieu Dubois-Briand kirjoitti:
> On Mon Feb 17, 2025 at 9:08 PM CET, Andy Shevchenko wrote:
> > On Mon, Feb 17, 2025 at 12:20:13PM +0100, Mathieu Dubois-Briand wrote:

...

> > > A datasheet is available on https://www.analog.com/en/products/max7360.html
> >
> > Thank you for this good elaboration!
> > I will check on the datasheet later on, having one week off.

Note, I have only briefly looked at it, not a deep study and TBH I am not sure
I will have time to invest into that.

> Thanks for your feedback! Sorry I haven't been able to work on this
> series for the last few weeks, but I finally had the opportunity to
> integrate your comments.

No rush, this will miss v6.15 anyway, so we still have a couple of months.

> > But what I have read above sounds to me like the following:
> >
> > 1) the PORT0-PORT7 should be just a regular pin control with the respective
> > function being provided (see pinctrl-cy8c95x0.c as an example);
> 
> Ok, so I created a pin control driver for the PORT pins. This will
> effectively help to prevent concurrent use of pins in place of the
> request()/free() callbacks.
> 
> My only concern is: as there is no real pin muxing on the chip, my
> .set_mux callabck in pinmux_ops structure is not doing anything. It
> looks like I'm not the only one
> (drivers/pinctrl/pinctrl-microchip-sgpio.c does the same thing), but I
> hope this is OK.

Hmm... This is strange. The PWM/GPIO block has 3 functions (GPIO/PWM/rotary),
How comes you have no switch between them?

As far as I read in the datasheet this is controlled by register 0x40
(and seems implicitly by other registers when it's in PWM mode).

> > 2) the COL2 COL7 case can be modeled as a simplest GPIO (GPO) driver with
> > reserved lines property (this will set valid mask and let GPIOLIB to refuse any
> > use of the keypad connected pins.
> 
> I mostly went that way, just a few notes.
> 
> I chose to not use the reserved lines property in the device tree, but
> instead implemented a gpiolib init_valid_mask() callback. In believe
> this is better, as:
> - We can automatically generate the valid gpios mask, based on the
>   number of columns used.
> - It allows to get rid of the compatibility check between the number of
>   columns and the number of GPIOs provided by the device tree: DT
>   provides the number of columns, we deduct the number of GPIOs.

If I understood it correctly it should work as well. But let's discuss that
when you issue a new version.

> I chose to number GPIOs from 0 to 7.
> - This might be a bit questionable, as GPIO 0 and 1 will always be
>   invalid: pins 0 and 1 of the chip cannot be used as GPIOs. I'm
>   definitely open to discussion on this point.
> - Yet I believe it simplifies everything for the user: pin numbers and
>   GPIO numbers are the same instead of having an offset of 2.
> - It also simplifies a bit the GPIO driver code.

In general you should follow the datasheet and mask the GPIOs that may not be
uses as a such due to HW limitation / specific configuration.

-- 
With Best Regards,
Andy Shevchenko