Re: [PATCH v2 2/2] gpio: max7317: Add gpio expander driver

[Edmund Berenson]

On 4/4/23 16:05, Linus Walleij wrote:
> On Mon, Apr 3, 2023 at 1:41 PM Edmund Berenson
> <edmund.berenson@xxxxxxxxx> wrote:
> 
>> Add driver for maxim MAX7317 SPI-Interfaced 10 Port
>> GPIO Expander.
>>
>> v2: adjust driver to use regmap
>>
>> Co-developed-by: Lukasz Zemla <Lukasz.Zemla@xxxxxxxxxxxx>
>> Signed-off-by: Lukasz Zemla <Lukasz.Zemla@xxxxxxxxxxxx>
>> Signed-off-by: Edmund Berenson <edmund.berenson@xxxxxxxxx>
> 
> Notwithstanding the other comments from Bartosz, this seems like
> a driver that should be using the regmap GPIO helper library.
> git grep GPIO_REGMAP will show you examples of other drivers
> that use this and how it is used.
> 
> Yours,
> Linus Walleij

Hi,

thanks for the review and suggestion. I tried following your suggestion and use
GPIO_REGMAP to implement the driver.

Unfortunately I ran into two issues
1. reg_set_base == 0: for the devcie reg_set base is 0x0. In gpio-regmap there
are several tests for !reg_set_base. There doesn't seem a way to distinguish
between is set to 0 and is not set. :)

2. input/output direction: to set a gpio pin to input one has to write 0x1 to
the corresponding output register. The issue starts when I configure a port to
be an output, set output to 0x1, check the direction of the pin, doing so trough
sysfs the system will now assume the pin is an input and I can't set its values
anymore. Avoiding this I would like to track the direction of the pin separately
from the device register, which is atm done in the corresponding bespoke in/out
functions.

I could probably solve both of these issues trough the reg_mask_xlate function
but I believe this would introduce unneeded obscurity in the driver.

I do not believe there are any other easy obvious/better fixes for this. (or
maybe you prove me wrong :))
Would you be okay for this driver to stick with direct regmap usage? (obviously
fixing the review suggestions)

BR
Edmund

-- 
Edmund Berenson, emlix GmbH, http://www.emlix.com
Fon +49 551 30664-0, Fax +49 551 30664-11
Gothaer Platz 3, 37083 Göttingen, Germany
Sitz der Gesellschaft: Göttingen, Amtsgericht Göttingen HR B 3160
Geschäftsführung: Heike Jordan, Dr. Uwe Kracke – Ust-IdNr.: DE 205 198 055

emlix - smart embedded open source