pin_config_32 doesn't make sense to me at the moment. Ian created it at a time when we could only configure the direction of pins and only for specific hardware (if I remember correctly, iMX51). Now there is no chance that it could be implemented for current hardware and pin flags. Configuration settings are almost never atomic for a single pin, let alone for multiple pins.

pin_access_32 has a chance of being implemented on current hardware, but only on a minimal amount of HW.

I'm not sure if it makes sense to implement something that can only be used on a minimal number of systems (i.e., something that can only be used for drivers specific to a given SoC).
Personally, I vote rather delete them than implement them.

I'd be hesitant to remove it from gpioc, we don't know if any userspace programs need it.

The range check is only for the index in the ivars. For example the ivar can only have two pins in it (say 2 and 7), then passing pin 0 to gpiobus_pin_access_32 would access pins 2 - 33 on the controller.
Unless you mean verify that the 32 pins are present in the child's ivars. That would be better, yes.

Sorry, but this is making less and less sense to me.

The gpiobus childern uses the giobus interface (as opposed to the gpio interface) and work with "virtual" pin numbers assigned by a third party (property in FDT, help or something). Gpiobus should convert these virtual pin numbers to real ones (gpio controller) using IVARS and can assume absolutely nothing about the ordering/sequencing of these translated numbers.

The controller (i.e. using gpio ifc) access_32/configure_32 expects to work on 32/npins consecutive pin numbers. This means that the corresponding gpiobus methods must only work on slaves with 32/npins or more consecutive pin mappings.
But will it still be usable?

Or have I missed something?

And sorry for the confusion, but I thought I remembered more about gpio. It wasn't entirely true and I need to reread the code.

In D51931#1189566, @mmel wrote:

Gpiobus should convert these virtual pin numbers to real ones (gpio controller) using IVARS and can assume absolutely nothing about the ordering/sequencing of these translated numbers.

Yeah I missed the ordering part, like the other review. I guess that means we can't check if they're all in the ivars before passing the call to the controller. All we can do in that case is make sure that devi->npins >= 32?

The controller (i.e. using gpio ifc) access_32/configure_32 expects to work on 32/npins consecutive pin numbers. This means that the corresponding gpiobus methods must only work on slaves with 32/npins or more consecutive pin mappings.
But will it still be usable?

The gpiobus pins don't necessarily need to be consecutive. I think it's fine if they are scattered in the ivars, as long as all 32 pins are there.

In D51931#1189574, @vexeduxr wrote:

In D51931#1189566, @mmel wrote:

Gpiobus should convert these virtual pin numbers to real ones (gpio controller) using IVARS and can assume absolutely nothing about the ordering/sequencing of these translated numbers.

Yeah I missed the ordering part, like the other review. I guess that means we can't check if they're all in the ivars before passing the call to the controller. All we can do in that case is make sure that devi->npins >= 32?

IMHO no, we should check if they are all in ivars, sequentially, starting with first_pin. See later.

The controller (i.e. using gpio ifc) access_32/configure_32 expects to work on 32/npins consecutive pin numbers. This means that the corresponding gpiobus methods must only work on slaves with 32/npins or more consecutive pin mappings.

But will it still be usable?

The gpiobus pins don't necessarily need to be consecutive. I think it's fine if they are scattered in the ivars, as long as all 32 pins are there.

Suppose we have one child device with an IVAR mapping from gpiobus numbers 0..31 to gpio numbers 0..31 and another with a mapping from 0..31 to 31..0. What bit order in the arguments (clear_pins, change_pins....) do you expect in these cases?

In D51931#1189620, @mmel wrote:

Suppose we have one child device with an IVAR mapping from gpiobus numbers 0..31 to gpio numbers 0..31 and another with a mapping from 0..31 to 31..0. What bit order in the arguments (clear_pins, change_pins....) do you expect in these cases?

I was about to say that they should have the same bit order, but now that I'm writing it out I see the issue... There's no reasonable way for the caller to figure out what bits map to what gpiobus pins.

That seems fine to me. Unfortunately, I just found that config32 has num_pins as an argument, so it may not work on all 32 bits :( I didn't notice that before, sorry.

Ah, I should've noticed that too...