| Message ID | 20241016-gpio-ngpios-v1-0-f16cf154f715@linaro.org |
|---|---|
| Headers | show |
| Series | gpio: mmio: Support ngpios property | expand |
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>
I thought this generic property was already supported by the generic MMIO bindings and code, but no. It's a pretty obvious usecase to just use some from 0..n of a MMIO GPIO bank, so add support for this. Signed-off-by: Linus Walleij <linus.walleij@linaro.org> --- Linus Walleij (2): dt-bindings: gpio-mmio: Add ngpios property gpio: mmio: Parse ngpios property Documentation/devicetree/bindings/gpio/gpio-mmio.yaml | 13 ++++++++++++- drivers/gpio/gpio-mmio.c | 4 ++++ 2 files changed, 16 insertions(+), 1 deletion(-) --- base-commit: 9852d85ec9d492ebef56dc5f229416c925758edc change-id: 20241016-gpio-ngpios-0cad57f0a757 Best regards,