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

[Sean Anderson <seanga2@gmail.com>]

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.

> 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.

> 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.

> Optionally, a GPIO controller may have a "ngpios" property. This property
> indicates the number of in-use slots of available slots for GPIOs. The
> typical example is something like this: the hardware register is 32 bits
> wide, but only 18 of the bits have a physical counterpart. The driver is
> generally written so that all 32 bits can be used, but the IP block is reused
> in a lot of designs, some using all 32 bits, some using 18 and some using
> 12. In this case, setting "ngpios = <18>;" informs the driver that only the
> first 18 GPIOs, at local offset 0 .. 17, are in use.

--Sean