Re: [PATCH v3 1/2] dt-bindings: pinctrl: Add bindings for Intel Thunderbay pinctrl driver

[Rob Herring <robh@xxxxxxxxxx>]

On Thu, Dec 16, 2021 at 08:30:59PM +0530, lakshmi.sowjanya.d@xxxxxxxxx wrote:
> From: Lakshmi Sowjanya D <lakshmi.sowjanya.d@xxxxxxxxx>
> 
> Add Device Tree bindings documentation and an entry in MAINTAINERS file
> for Intel Thunder Bay SoC's pin controller.

Seems Linus already applied this... You may need to send incremental 
changes.

> 
> Signed-off-by: Lakshmi Sowjanya D <lakshmi.sowjanya.d@xxxxxxxxx>
> ---
>  .../pinctrl/intel,pinctrl-thunderbay.yaml     | 122 ++++++++++++++++++
>  MAINTAINERS                                   |   5 +
>  2 files changed, 127 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/pinctrl/intel,pinctrl-thunderbay.yaml
> 
> diff --git a/Documentation/devicetree/bindings/pinctrl/intel,pinctrl-thunderbay.yaml b/Documentation/devicetree/bindings/pinctrl/intel,pinctrl-thunderbay.yaml
> new file mode 100644
> index 000000000000..528f0d8445dd
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pinctrl/intel,pinctrl-thunderbay.yaml
> @@ -0,0 +1,122 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/pinctrl/intel,pinctrl-thunderbay.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Intel Thunder Bay pin controller Device Tree Bindings
> +
> +maintainers:
> +  - Lakshmi Sowjanya D <lakshmi.sowjanya.d@xxxxxxxxx>
> +
> +description: |
> +  Intel Thunder Bay SoC integrates a pin controller which enables control
> +  of pin directions, input/output values and configuration
> +  for a total of 67 pins.
> +
> +properties:
> +  compatible:
> +    const: intel,thunderbay-pinctrl
> +
> +  reg:
> +    maxItems: 1
> +
> +  gpio-controller: true
> +
> +  '#gpio-cells':
> +    const: 2
> +
> +  gpio-ranges:
> +    maxItems: 1
> +
> +  interrupts:
> +    description:
> +      Specifies the interrupt lines to be used by the controller.
> +    maxItems: 2

Need to define what each one is.

> +
> +  interrupt-controller: true
> +
> +  '#interrupt-cells':
> +    const: 2
> +
> +patternProperties:
> +  '^gpio@[0-9a-f]*$':
> +    type: object
> +
> +    description:
> +      Child nodes can be specified to contain pin configuration information,
> +      which can then be utilized by pinctrl client devices.
> +      The following properties are supported.
> +    $ref: pincfg-node.yaml#
> +
> +    properties:
> +      pins:
> +        description: |
> +          The name(s) of the pins to be configured in the child node.
> +          Supported pin names are "GPIO0" up to "GPIO66".

items:
  pattern: '^GPIO[1-6]?[0-9]$'

> +
> +      bias-disable: true
> +
> +      bias-pull-down: true
> +
> +      bias-pull-up: true
> +
> +      drive-strength:
> +        description: Drive strength for the pad.
> +        enum: [0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15]
> +
> +      bias-bus-hold:
> +        type: boolean
> +
> +      input-schmitt-enable:
> +        type: boolean
> +
> +      slew-rate:
> +        description: GPIO slew rate control.
> +                      0 - Slow
> +                      1 - Fast

You need to use '|' if you want the formatting to be preserved.

> +        enum: [0, 1]
> +
> +    additionalProperties: false
> +
> +required:
> +  - compatible
> +  - reg
> +  - gpio-controller
> +  - '#gpio-cells'
> +  - gpio-ranges
> +  - interrupts
> +  - interrupt-controller
> +  - '#interrupt-cells'
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +    // Example 1
> +    pinctrl0: gpio@0 {

unit address is wrong

> +        compatible = "intel,thunderbay-pinctrl";
> +        reg = <0x600b0000 0x88>;
> +        gpio-controller;
> +        #gpio-cells = <0x2>;
> +        gpio-ranges = <&pinctrl0 0 0 67>;
> +        interrupts = <GIC_SPI 94 IRQ_TYPE_LEVEL_HIGH>,
> +                     <GIC_SPI 95 IRQ_TYPE_LEVEL_HIGH>;
> +        interrupt-controller;
> +        #interrupt-cells = <2>;

How about showing a pin node.

> +    };
> +
> +    // Example 2
> +    pinctrl1: gpio@1 {

I don't see the point of the 2nd example. It is the same for purposes of 
examples.

> +        compatible = "intel,thunderbay-pinctrl";
> +        reg = <0x600c0000 0x88>;
> +        gpio-controller;
> +        #gpio-cells = <0x2>;
> +        gpio-ranges = <&pinctrl1 0 0 53>;
> +        interrupts = <GIC_SPI 94 IRQ_TYPE_LEVEL_HIGH>,
> +                     <GIC_SPI 95 IRQ_TYPE_LEVEL_HIGH>;
> +        interrupt-controller;
> +        #interrupt-cells = <2>;
> +    };
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 13f9a84a617e..db744ba259e4 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -15127,6 +15127,11 @@ L:	linux-omap@xxxxxxxxxxxxxxx
>  S:	Maintained
>  F:	drivers/pinctrl/pinctrl-single.c
>  
> +PIN CONTROLLER - THUNDERBAY
> +M:	Lakshmi Sowjanya D <lakshmi.sowjanya.d@xxxxxxxxx>
> +S:	Supported
> +F:	drivers/pinctrl/pinctrl-thunderbay.c
> +
>  PKTCDVD DRIVER
>  M:	linux-block@xxxxxxxxxxxxxxx
>  S:	Orphan
> -- 
> 2.17.1
> 
>