| Message ID | 20241016-gpio-ngpios-v1-1-f16cf154f715@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | gpio: mmio: Support ngpios property | expand |
On Wed, Oct 16, 2024 at 2:21 AM Linus Walleij <linus.walleij@linaro.org> wrote: > > This adds the ngpios property to MMIO GPIO. We restrict the > property to 1..63 since there is no point in 0 GPIO lines and > we support up to 64bits wide registers for now. Why does it need to be restricted? Is something bad going to happen if for some reason someone tries to control a non-existent GPIO? If there is, maybe there should be a specific compatible as the h/w is not so generic. Rob
On Wed, Oct 16, 2024 at 5:47 PM Rob Herring <robh@kernel.org> wrote: > On Wed, Oct 16, 2024 at 2:21 AM Linus Walleij <linus.walleij@linaro.org> wrote: > > > > This adds the ngpios property to MMIO GPIO. We restrict the > > property to 1..63 since there is no point in 0 GPIO lines and > > we support up to 64bits wide registers for now. > > Why does it need to be restricted? Is something bad going to happen if > for some reason someone tries to control a non-existent GPIO? Unlikely. But the biggest inconvenience is that non-existing GPIO lines gets exposed to userspace which causes confusion. It's a bit like saying you have 32 harddisks on your system just because the register has 32 bits. > If there > is, maybe there should be a specific compatible as the h/w is not so > generic. The gpio-mmio is quite generic, it's the most generic GPIO driver we have. The ngpios property is also generic, it is in: Documentation/devicetree/bindings/gpio/gpio.txt (commit aacaffd1d9a6f8e2c7369d83c21d41c3b53e2edc) At the time (2015) I just documented the already widespread use of this property. It is also reflected in several of the new yaml bindings, a git grep ngpios will show you. Yours, Linus Walleij
On Wed, Oct 16, 2024 at 1:32 PM Linus Walleij <linus.walleij@linaro.org> wrote: > > On Wed, Oct 16, 2024 at 5:47 PM Rob Herring <robh@kernel.org> wrote: > > On Wed, Oct 16, 2024 at 2:21 AM Linus Walleij <linus.walleij@linaro.org> wrote: > > > > > > This adds the ngpios property to MMIO GPIO. We restrict the > > > property to 1..63 since there is no point in 0 GPIO lines and > > > we support up to 64bits wide registers for now. > > > > Why does it need to be restricted? Is something bad going to happen if > > for some reason someone tries to control a non-existent GPIO? > > Unlikely. But the biggest inconvenience is that non-existing GPIO lines > gets exposed to userspace which causes confusion. It's a bit like > saying you have 32 harddisks on your system just because the register > has 32 bits. I would love to have 32 harddisks. My analogy is we don't define how many interrupts an interrupt controller has. That's generally either implicit or you just don't need to know (other than validating used interrupt numbers). Of course interrupts aren't exposed to userspace. This property has shortcomings if you want to prevent exposing non-existent GPIOs to userspace. You really need a mask because what if GPIO0 doesn't exist? > > If there > > is, maybe there should be a specific compatible as the h/w is not so > > generic. > > The gpio-mmio is quite generic, it's the most generic GPIO > driver we have. > > The ngpios property is also generic, it is in: > Documentation/devicetree/bindings/gpio/gpio.txt > (commit aacaffd1d9a6f8e2c7369d83c21d41c3b53e2edc) > > At the time (2015) I just documented the already widespread use > of this property. > > It is also reflected in several of the new yaml bindings, a git grep > ngpios will show you. Yes, I know. You can also find push back on using it. Anyway, I did my push back and what's one more user (sigh), so: Acked-by: Rob Herring (Arm) <robh@kernel.org>
On Wed, Oct 16, 2024 at 8:59 PM Rob Herring <robh@kernel.org> wrote: > This property has shortcomings if you want to prevent exposing > non-existent GPIOs to userspace. You really need a mask because what > if GPIO0 doesn't exist? That does happen in $OS (DT hat on), a famous OS has such sparse masks for GPIO chips. They have not become DT bindings (yet). As it turns out they are mostly used for cases where firmware/BIOS/secure world want to "steal" some GPIOs out of the pool and as such has nothing to do with hardware description. It has been shown that in practice what HW engineers do is line up as many IP instances they need and just truncate the last one, rendering the upper bits unused. They are practical and least effort-oriented people who have yet to be seen doing something crazy like e.g. connect every second bit to an actual pin. > > It is also reflected in several of the new yaml bindings, a git grep > > ngpios will show you. > > Yes, I know. You can also find push back on using it. > > Anyway, I did my push back and what's one more user (sigh), so: > > Acked-by: Rob Herring (Arm) <robh@kernel.org> Thanks Rob, some push back is always good! Yours, Linus Walleij
diff --git a/Documentation/devicetree/bindings/gpio/gpio-mmio.yaml b/Documentation/devicetree/bindings/gpio/gpio-mmio.yaml index b394e058256e..87e986386f32 100644 --- a/Documentation/devicetree/bindings/gpio/gpio-mmio.yaml +++ b/Documentation/devicetree/bindings/gpio/gpio-mmio.yaml @@ -37,7 +37,8 @@ properties: description: A list of registers in the controller. The width of each register is determined by its size. All registers must have the same width. The number - of GPIOs is set by the width, with bit 0 corresponding to GPIO 0. + of GPIOs is set by the width, with bit 0 corresponding to GPIO 0, unless + the ngpios property further restricts the number of used lines. items: - description: Register to READ the value of the GPIO lines. If GPIO line is high, @@ -74,6 +75,15 @@ properties: native-endian: true + ngpios: + minimum: 1 + maximum: 63 + description: + If this property is present the number of usable GPIO lines are restricted + to the first 0 .. ngpios lines. This is useful when the GPIO MMIO register + has 32 bits for GPIO but only the first 12 are actually connected to + real electronics, and then we set ngpios to 12. + no-output: $ref: /schemas/types.yaml#/definitions/flag description: @@ -111,6 +121,7 @@ examples: compatible = "brcm,bcm6345-gpio"; reg-names = "dirout", "dat"; reg = <0xfffe0406 2>, <0xfffe040a 2>; + ngpios = <15>; native-endian; gpio-controller; #gpio-cells = <2>;
This adds the ngpios property to MMIO GPIO. We restrict the property to 1..63 since there is no point in 0 GPIO lines and we support up to 64bits wide registers for now. Signed-off-by: Linus Walleij <linus.walleij@linaro.org> --- Documentation/devicetree/bindings/gpio/gpio-mmio.yaml | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-)