Am 2023-05-03 10:37, schrieb 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. :)
That's what GPIO_REGMAP_ADDR(addr) is for :)
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 can't follow you here exactly. But I've briefly looked at the
datasheet
and the output is an open drain one. Just like the one we are currently
discussing in [1]. Here too, there seems to be no direction register,
although it is mentioned in the datasheet. But Table 1. Register Address
Map
is super confusing, so please correct me if I'm wrong.
Since we now have already two different chips with this OD output and
always
active input buffer, it makes sense to add support to gpio-regmap for
these
kind of devices. To me it is still unclear wether we need the direction
at
all. Linus answered my question yesterday, but I haven't found time to
dig
into that topic myself. Please go ahead and make some suggestions :)
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)
-michael
[1] https://lore.kernel.org/r/20230502084406.3529645-1-michael@xxxxxxxx/
