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

["Mathieu Dubois-Briand" <mathieu.dubois-briand@bootlin.com>]

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