Am 2022-07-06 22:46, schrieb Aidan MacDonald:
Am 2022-07-04 18:01, schrieb Aidan MacDonald:
Am 2022-07-03 13:10, schrieb Aidan MacDonald:
Some devices use a multi-bit register field to change the GPIO
input/output direction. Add the ->reg_field_xlate() callback to
support such devices in gpio-regmap.
->reg_field_xlate() builds on ->reg_mask_xlate() by allowing the
driver to return a mask and values to describe a register field.
gpio-regmap will use the mask to isolate the field and compare or
update it using the values to implement GPIO level and direction
get and set ops.
Thanks for working on this. Here are my thoughts on how to improve
it:
- I'm wary on the value translation of the set and get, you
don't need that at the moment, correct? I'd concentrate on
the direction for now.
- I'd add a xlate_direction(), see below for an example
Yeah, I only need direction, but there's no advantage to creating a
specific mechanism. I'm not opposed to doing that but I don't see
how it can be done cleanly. Being more general is more consistent
for the API and implementation -- even if the extra flexibility
probably won't be needed, it doesn't hurt.
I'd prefer to keep it to the current use case. I'm not sure if
there are many controllers which have more than one bit for
the input and output state. And if, we are still limited to
one register, what if the bits are distributed among multiple
registers..
I found three drivers (not exhaustive) that have fields for setting the
output level: gpio-amd8111, gpio-creg-snps, and gpio-lp3943. Admittedly
that's more than I expected, so maybe we shouldn't dismiss the idea of
multi-bit output fields.
I'm not dismissing it, but I want to wait for an actual user
for it :)
If you still think the API you're suggesting is better then I can go
with it, but IMHO it's more code and more special cases, even if only
a little bit.
What bothers me with your approach is that you are returning
an integer and in one case you are interpreting it as gpio
direction and in the other case you are interpreting it as
a gpio state, while mine is more explicit, i.e. a
xlate_direction() for the direction and if there will be
support for multi bit input/output then there would be a
xlate_state() or similar. Granted, it is more code, but
easier to understand IMHO.
-michael
