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: <D8FAEPI26C8F.397VN87KK9VIO@bootlin.com>
Date: Thu, 13 Mar 2025 17:43:00 +0100
From: "Mathieu Dubois-Briand" <mathieu.dubois-briand@...tlin.com>
To: "Andy Shevchenko" <andriy.shevchenko@...el.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 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:
> > To provide a bit more details, there is basically two set of pins usable
> > as GPIOs.
> > 
> > On one side we have what I refer to as GPIOs:
> >   - PORT0 to PORT7 pins of the chip.
> >   - Shared with PWM and rotary encoder functionalities. Functionality
> >     selection can be made independently for each pin. We have to ensure
> >     the same pin is not used by two drivers at the same time. E.g. we
> >     cannot have at the same time GPIO4 and PWM4.
> >   - Supports input and interrupts.
> >   - Outputs may be configured as constant current.
> >   - 8 GPIOS supported, so ngpios is fixed to MAX7360_MAX_GPIO.
> >   - maxim,max7360-gpio compatible, gpio_function == MAX7360_GPIO_PORT.
> > 
> > On the other side, we have what I refer to as GPOs:
> >   - COL2 to COL7 pins of the chip.
> >   - Shared with the keypad functionality. Selections is made by
> >     partitioning the pins: first pins for keypad columns, last pins for
> >     GPOs. Partition is described by the ngpios property.
> >   - Only support outputs.
> >   - maxim,max7360-gpo compatible, gpio_function == MAX7360_GPIO_COL.
> > 
> > > Or you mean that there output only GPIO lines in HW after all?
> > > Is there a link to the datasheet?
> > 
> > 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.
>

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.

> 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.

> 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.

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.

-- 
Mathieu Dubois-Briand, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com