On 26/02/2025 12:18, Linus Walleij wrote:
On Wed, Feb 26, 2025 at 7:09 AM Matti Vaittinen
<mazziesaccount@xxxxxxxxx> wrote:
On 25/02/2025 23:36, Linus Walleij wrote:
we can maybe move it to struct gpio_device in
drivers/gpio/gpiolib.h?
This struct exist for every gpio_chip but is entirely gpiolib-internal.
Then it becomes impossible to do it wrong...
True. I can try seeing what it'd require to do that. But ... If there
are any drivers out there altering the valid_mask _after_ registering
the driver to the gpio-core ... Then it may be a can of worms and I may
just keep the lid closed :)
That's easy to check with some git grep valid_mask
True. I just tried. It seems mostly Ok, but...
For example the drivers/gpio/gpio-rcar.c uses the contents of the
'valid_mask' in it's set_multiple callback to disallow setting the value
of masked GPIOs.
For uneducated person like me, it feels this check should be done and
enforced by the gpiolib and not left for untrustworthy driver writers
like me! (I am working on BD79124 driver and it didn't occur to me I
should check for the valid_mask in driver :) If gpiolib may call the
driver's set_multiple() with masked lines - then the bd79124 driver just
had one unknown bug less :rolleyes:) )
I tried looking at the gpiolib to see how this works... It seems to me:
gpio_chip_set_multiple() does not seem to check for valid_mask. This is
called from the gpiod_set_array_value_complex() - which gave me a
headache as it is, as name says, complex. Well, I didn't spot valid_mask
check but I may have missed a thing or 2...
If someone remembers straight away how this is supposed to work - I
appreciate any guidance. If not, then I try doing some testing when I
wire the BD79124 to my board for the next version of the BD79124 series.
and intuition. I think all calls actually changing the valid_mask
are in the init_valid_mask() callback as they should be.
Furthermore, I was not 100% sure the valid_mask was not intended to be
used directly by the drivers. I hoped you and Bart have an opinion on that.
Oh it was. First we just had .valid_mask and then it was
manipulated directly.
I still can't decide if hiding the valid_mask is the right thing to do,
or if we should just respect it if it is set by driver (as it was
originally intended).
Then we introduced init_valid_mask() and all users switched over
to using that.
So evolution, not intelligent design...
Like anything we actually get done ^_^;
Yours,
-- Matti
