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
