Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [PATCH] dt-bindings: gpio: gpio-davinci: Convert to json-schema
To:     Rob Herring <robh@kernel.org>
CC:     Vignesh Raghavendra <vigneshr@ti.com>,
        Lokesh Vutla <lokeshvutla@ti.com>,
        Grygorii Strashko <grygorii.strashko@ti.com>,
        Kishon Vijay Abraham I <kishon@ti.com>,
        Linus Walleij <linus.walleij@linaro.org>,
        Bartosz Golaszewski <bgolaszewski@baylibre.com>,
        Keerthy <j-keerthy@ti.com>, <linux-gpio@vger.kernel.org>,
        <devicetree@vger.kernel.org>, <linux-kernel@vger.kernel.org>
References: <20210511090122.6995-1-a-govindraju@ti.com>
 <20210517221513.GA3263368@robh.at.kernel.org>
From:   Aswath Govindraju <a-govindraju@ti.com>
Message-ID: <e239365e-35d7-694a-55cc-7dabfa66b108@ti.com>
Date:   Tue, 18 May 2021 19:43:38 +0530
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101
 Thunderbird/68.10.0
MIME-Version: 1.0
In-Reply-To: <20210517221513.GA3263368@robh.at.kernel.org>
Content-Type: text/plain; charset="utf-8"
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Precedence: bulk


Hi Rob,

On 18/05/21 3:45 am, Rob Herring wrote:
> On Tue, May 11, 2021 at 02:31:20PM +0530, Aswath Govindraju wrote:
>> Convert gpio-davinci dt-binding documentation from txt to yaml format.
>>
>> Signed-off-by: Aswath Govindraju <a-govindraju@ti.com>
>> ---
>>  .../devicetree/bindings/gpio/gpio-davinci.txt | 167 ---------------
>>  .../bindings/gpio/gpio-davinci.yaml           | 193 ++++++++++++++++++
>>  MAINTAINERS                                   |   2 +-
>>  3 files changed, 194 insertions(+), 168 deletions(-)
>>  delete mode 100644 Documentation/devicetree/bindings/gpio/gpio-davinci.txt
>>  create mode 100644 Documentation/devicetree/bindings/gpio/gpio-davinci.yaml
>>

[...]

>> +properties:
>> +  compatible:
>> +    oneOf:
>> +      - items:
>> +          - enum:
>> +              - ti,k2g-gpio
>> +              - ti,am654-gpio
>> +              - ti,j721e-gpio
>> +              - ti,am64-gpio
>> +          - const: ti,keystone-gpio
>> +
>> +      - items:
>> +          - const: ti,dm6441-gpio
>> +      - items:
>> +          - const: ti,keystone-gpio
> 
> These 2 can be expressed as an 'enum'.

I will change this.

> 
>> +
>> +  reg:
>> +    maxItems: 1
>> +    description:
>> +      Physical base address of the controller and the size of memory mapped registers.
> 
> Drop. That's every 'reg' property.
> 

I'll drop this.

>> +
>> +  gpio-controller: true
>> +
>> +  gpio-ranges: true
>> +
>> +  gpio-line-names:
>> +    description: strings describing the names of each gpio line.
> 
> Any constraints like min/max number of lines? 
> 

The max number of lines will be equal to ti,ngpio. Is there any way to
equate maxItems to the a property value in json schema ?

>> +
>> +  "#gpio-cells":
>> +    const: 2
>> +    description:
>> +      first cell is the pin number and second cell is used to specify optional parameters (unused).
>> +
>> +  interrupts:
>> +    description:
>> +      Array of GPIO interrupt number. Only banked or unbanked IRQs are supported at a time.
> 
> Needs constraints. How many items and what are they?

Here also the maximum number of interrupts is equal to ti,ngpio in
unbanked interrupts case. Same as above is there anyway to equate
maxItems to ti,ngpio property in json schma ? If not, then what would be
the best way to handle this ?

> 
>> +
>> +  ti,ngpio:
>> +    $ref: /schemas/types.yaml#/definitions/uint32
>> +    description: The number of GPIO pins supported consecutively.
>> +    minimum: 1
>> +
>> +  ti,davinci-gpio-unbanked:
>> +    $ref: /schemas/types.yaml#/definitions/uint32
>> +    description: The number of GPIOs that have an individual interrupt line to processor.
>> +    minimum: 0
>> +
>> +  clocks:
>> +    maxItems: 1
>> +    description:
>> +      clock-specifier to represent input to the GPIO controller.
> 
> Drop description.
> 

Will drop this.

>> +
>> +  clock-names:
>> +    const: gpio
>> +
>> +  interrupt-controller: true
>> +
>> +  power-domains:
>> +    maxItems: 1
>> +    description:
>> +      Phandle to the power domain provider node.
> 
> Drop

Will drop this.
.
> 
>> +
>> +  "#interrupt-cells":
>> +    const: 2
>> +
>> +patternProperties:
>> +  "-hog$":
>> +    type: object
>> +    properties:
>> +      gpios: true
>> +      gpio-hog: true
>> +      input: true
>> +      output-high: true
>> +      output-low: true
>> +      line-name: true
>> +
>> +    required:
>> +      - gpio-hog
>> +      - gpios
>> +
>> +required:
>> +  - compatible
>> +  - reg
>> +  - gpio-controller
>> +  - "#gpio-cells"
>> +  - interrupts
>> +  - ti,ngpio
>> +  - ti,davinci-gpio-unbanked
>> +  - clocks
>> +  - clock-names
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> +  - |
>> +    #include<dt-bindings/interrupt-controller/arm-gic.h>
>> +
>> +    gpio0: gpio@2603000 {
>> +      compatible = "ti,k2g-gpio", "ti,keystone-gpio";
>> +      reg = <0x02603000 0x100>;
>> +      gpio-controller;
>> +      #gpio-cells = <2>;
>> +      interrupts = <GIC_SPI 432 IRQ_TYPE_EDGE_RISING>,
>> +                   <GIC_SPI 433 IRQ_TYPE_EDGE_RISING>,
>> +                   <GIC_SPI 434 IRQ_TYPE_EDGE_RISING>,
>> +                   <GIC_SPI 435 IRQ_TYPE_EDGE_RISING>,
>> +                   <GIC_SPI 436 IRQ_TYPE_EDGE_RISING>,
>> +                   <GIC_SPI 437 IRQ_TYPE_EDGE_RISING>,
>> +                   <GIC_SPI 438 IRQ_TYPE_EDGE_RISING>,
>> +                   <GIC_SPI 439 IRQ_TYPE_EDGE_RISING>,
>> +                   <GIC_SPI 440 IRQ_TYPE_EDGE_RISING>;
>> +      interrupt-controller;
>> +      #interrupt-cells = <2>;
>> +      ti,ngpio = <144>;
>> +      ti,davinci-gpio-unbanked = <0>;
>> +      clocks = <&k2g_clks 0x001b 0x0>;
>> +      clock-names = "gpio";
>> +    };
>> +
>> +  - |
>> +    #include<dt-bindings/interrupt-controller/arm-gic.h>
>> +
>> +    gpio1: gpio@260bf00 {
>> +      compatible = "ti,keystone-gpio";
>> +      reg = <0x0260bf00 0x100>;
>> +      gpio-controller;
>> +      #gpio-cells = <2>;
>> +      /* HW Interrupts mapped to GPIO pins */
>> +      interrupts = <GIC_SPI 120 IRQ_TYPE_EDGE_RISING>,
>> +                   <GIC_SPI 121 IRQ_TYPE_EDGE_RISING>,
>> +                   <GIC_SPI 122 IRQ_TYPE_EDGE_RISING>,
>> +                   <GIC_SPI 123 IRQ_TYPE_EDGE_RISING>,
>> +                   <GIC_SPI 124 IRQ_TYPE_EDGE_RISING>,
>> +                   <GIC_SPI 125 IRQ_TYPE_EDGE_RISING>,
>> +                   <GIC_SPI 126 IRQ_TYPE_EDGE_RISING>,
>> +                   <GIC_SPI 127 IRQ_TYPE_EDGE_RISING>,
>> +                   <GIC_SPI 128 IRQ_TYPE_EDGE_RISING>,
>> +                   <GIC_SPI 129 IRQ_TYPE_EDGE_RISING>,
>> +                   <GIC_SPI 130 IRQ_TYPE_EDGE_RISING>,
>> +                   <GIC_SPI 131 IRQ_TYPE_EDGE_RISING>,
>> +                   <GIC_SPI 132 IRQ_TYPE_EDGE_RISING>,
>> +                   <GIC_SPI 133 IRQ_TYPE_EDGE_RISING>,
>> +                   <GIC_SPI 134 IRQ_TYPE_EDGE_RISING>,
>> +                   <GIC_SPI 135 IRQ_TYPE_EDGE_RISING>,
>> +                   <GIC_SPI 136 IRQ_TYPE_EDGE_RISING>,
>> +                   <GIC_SPI 137 IRQ_TYPE_EDGE_RISING>,
>> +                   <GIC_SPI 138 IRQ_TYPE_EDGE_RISING>,
>> +                   <GIC_SPI 139 IRQ_TYPE_EDGE_RISING>,
>> +                   <GIC_SPI 140 IRQ_TYPE_EDGE_RISING>,
>> +                   <GIC_SPI 141 IRQ_TYPE_EDGE_RISING>,
>> +                   <GIC_SPI 142 IRQ_TYPE_EDGE_RISING>,
>> +                   <GIC_SPI 143 IRQ_TYPE_EDGE_RISING>,
>> +                   <GIC_SPI 144 IRQ_TYPE_EDGE_RISING>,
>> +                   <GIC_SPI 145 IRQ_TYPE_EDGE_RISING>,
>> +                   <GIC_SPI 146 IRQ_TYPE_EDGE_RISING>,
>> +                   <GIC_SPI 147 IRQ_TYPE_EDGE_RISING>,
>> +                   <GIC_SPI 148 IRQ_TYPE_EDGE_RISING>,
>> +                   <GIC_SPI 149 IRQ_TYPE_EDGE_RISING>,
>> +                   <GIC_SPI 150 IRQ_TYPE_EDGE_RISING>,
>> +                   <GIC_SPI 151 IRQ_TYPE_EDGE_RISING>;
>> +      clocks = <&clkgpio>;
>> +      clock-names = "gpio";
>> +      ti,ngpio = <32>;
>> +      ti,davinci-gpio-unbanked = <32>;
>> +    };
>> +
>> +  - |
>> +    wkup_gpio0: wkup_gpio0@42110000 {
> 
> gpio@...
> 

will change this in respin

Thanks,
Aswath

>> +      compatible = "ti,am654-gpio", "ti,keystone-gpio";
>> +      reg = <0x42110000 0x100>;
>> +      gpio-controller;
>> +      #gpio-cells = <2>;
>> +      interrupt-parent = <&intr_wkup_gpio>;
>> +      interrupts = <60>, <61>, <62>, <63>;
>> +      interrupt-controller;
>> +      #interrupt-cells = <2>;
>> +      ti,ngpio = <56>;
>> +      ti,davinci-gpio-unbanked = <0>;
>> +      clocks = <&k3_clks 59 0>;
>> +      clock-names = "gpio";
>> +    };
>> +
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 6e183abbbd2e..6b3519db8085 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -18269,7 +18269,7 @@ TI DAVINCI SERIES GPIO DRIVER
>>  M:	Keerthy <j-keerthy@ti.com>
>>  L:	linux-gpio@vger.kernel.org
>>  S:	Maintained
>> -F:	Documentation/devicetree/bindings/gpio/gpio-davinci.txt
>> +F:	Documentation/devicetree/bindings/gpio/gpio-davinci.yaml
>>  F:	drivers/gpio/gpio-davinci.c
>>  
>>  TI DAVINCI SERIES MEDIA DRIVER
>> -- 
>> 2.17.1
>>