Re: [PATCH v14 07/16] dt-bindings: fix sifive gpio properties

[Rob Herring <robh@kernel.org>]

On Tue, Feb 2, 2021 at 6:01 PM Sean Anderson <seanga2@gmail.com> wrote:
>
> On 2/2/21 2:02 PM, Rob Herring wrote:
> > On Tue, Feb 2, 2021 at 4:36 AM Damien Le Moal <damien.lemoal@wdc.com> wrote:
> >>
> >> The sifive gpio IP block supports up to 32 GPIOs. Reflect that in the
> >> interrupts property description and maxItems. Also add the standard
> >> ngpios property to describe the number of GPIOs available on the
> >> implementation.
> >>
> >> Also add the "canaan,k210-gpiohs" compatible string to indicate the use
> >> of this gpio controller in the Canaan Kendryte K210 SoC. If this
> >> compatible string is used, do not define the clocks property as
> >> required as the K210 SoC does not have a software controllable clock
> >> for the Sifive gpio IP block.
> >>
> >> Cc: Paul Walmsley <paul.walmsley@sifive.com>
> >> Cc: Rob Herring <robh@kernel.org>
> >> Cc: devicetree@vger.kernel.org
> >> Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
> >> ---
> >>   .../devicetree/bindings/gpio/sifive,gpio.yaml | 21 ++++++++++++++++---
> >>   1 file changed, 18 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/Documentation/devicetree/bindings/gpio/sifive,gpio.yaml b/Documentation/devicetree/bindings/gpio/sifive,gpio.yaml
> >> index ab22056f8b44..2cef18ca737c 100644
> >> --- a/Documentation/devicetree/bindings/gpio/sifive,gpio.yaml
> >> +++ b/Documentation/devicetree/bindings/gpio/sifive,gpio.yaml
> >> @@ -16,6 +16,7 @@ properties:
> >>         - enum:
> >>             - sifive,fu540-c000-gpio
> >>             - sifive,fu740-c000-gpio
> >> +          - canaan,k210-gpiohs
> >>         - const: sifive,gpio0
> >>
> >>     reg:
> >> @@ -23,9 +24,9 @@ properties:
> >>
> >>     interrupts:
> >>       description:
> >> -      interrupt mapping one per GPIO. Maximum 16 GPIOs.
> >> +      interrupt mapping one per GPIO. Maximum 32 GPIOs.
> >>       minItems: 1
> >> -    maxItems: 16
> >> +    maxItems: 32
> >>
> >>     interrupt-controller: true
> >>
> >> @@ -38,6 +39,10 @@ properties:
> >>     "#gpio-cells":
> >>       const: 2
> >>
> >> +  ngpios:
> >> +    minimum: 1
> >> +    maximum: 32
> >
> > What's the default as obviously drivers already assume something.
>
> The driver currently assumes 16. However, as noted in reply to Atish,
> the number of GPIOs is configurable.

So you need a 'default: 16' here.

> > Does a driver actually need to know this? For example, does the
> > register stride change or something?
>
> No. I believe that the number of GPIOs sets which bits in the control
> registers are valid. So the maximum number of GPIOs is the word width of
> the bus.

So like register access size (e.g. readw vs readl)? If so, we have
'reg-io-width' for that purpose.

> > Please don't add it if the only purpose is error check your DT (IOW,
> > if it just checks the max cell value in gpios phandles).
>
> Why not? This seems like exactly the situation this property was
> designed for.

Because it is redundant. All the GPIO lines you want to use should be
connected to something with a *-gpios property. If not, then that's a
failure to describe part of the h/w.

For comparison, we generally don't define how many interrupts an
interrupt controller has. Or how many power-domains a power domain
provider has. I can go on with every provider/consumer binding...

Rob