Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <e34b35f4-4197-3973-0947-14ee577494e9@linaro.org>
Date:   Fri, 28 Oct 2022 16:01:58 -0400
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.4.1
Subject: Re: [PATCH v4 09/13] dt-bindings: pinctrl: mediatek,mt6779-pinctrl:
 Add MT6795
Content-Language: en-US
To:     Yassine Oudjana <yassine.oudjana@gmail.com>,
        Linus Walleij <linus.walleij@linaro.org>,
        Rob Herring <robh+dt@kernel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        Matthias Brugger <matthias.bgg@gmail.com>,
        Sean Wang <sean.wang@kernel.org>,
        Andy Teng <andy.teng@mediatek.com>,
        AngeloGioacchino Del Regno 
        <angelogioacchino.delregno@collabora.com>
Cc:     Yassine Oudjana <y.oudjana@protonmail.com>,
        linux-mediatek@lists.infradead.org, linux-gpio@vger.kernel.org,
        devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
        linux-kernel@vger.kernel.org
References: <20221028153505.23741-1-y.oudjana@protonmail.com>
 <20221028153505.23741-10-y.oudjana@protonmail.com>
From:   Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
In-Reply-To: <20221028153505.23741-10-y.oudjana@protonmail.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 28/10/2022 11:35, Yassine Oudjana wrote:
> From: Yassine Oudjana <y.oudjana@protonmail.com>
> 
> Combine MT6795 pin controller document into MT6779 one. In the
> process, amend the example with comments and additional pinctrl
> nodes from the MT6795 example, replace the current interrupts
> property description with the one from the MT6795 document since
> it makes more sense and define its items using conditionals
> as they now vary between variants. Also use conditionals to define
> valid values for the drive-strength property for each variant.
> 
> Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com>
> ---
>  .../pinctrl/mediatek,mt6779-pinctrl.yaml      | 189 ++++++++++-----
>  .../pinctrl/mediatek,pinctrl-mt6795.yaml      | 227 ------------------
>  2 files changed, 127 insertions(+), 289 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/pinctrl/mediatek,pinctrl-mt6795.yaml
> 
> diff --git a/Documentation/devicetree/bindings/pinctrl/mediatek,mt6779-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/mediatek,mt6779-pinctrl.yaml
> index 70e4ffa2d897..6f2cffe50b11 100644
> --- a/Documentation/devicetree/bindings/pinctrl/mediatek,mt6779-pinctrl.yaml
> +++ b/Documentation/devicetree/bindings/pinctrl/mediatek,mt6779-pinctrl.yaml
> @@ -8,6 +8,7 @@ title: Mediatek MT6779 Pin Controller
>  
>  maintainers:
>    - Andy Teng <andy.teng@mediatek.com>
> +  - AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
>    - Sean Wang <sean.wang@kernel.org>
>  
>  description:
> @@ -18,6 +19,7 @@ properties:
>    compatible:
>      enum:
>        - mediatek,mt6779-pinctrl
> +      - mediatek,mt6795-pinctrl
>        - mediatek,mt6797-pinctrl
>  
>    reg:
> @@ -43,9 +45,7 @@ properties:
>    interrupt-controller: true
>  
>    interrupts:
> -    maxItems: 1

Leave the constraints.

Why? Because now you dropped it for mt6797... You bring here some random
changes and it is difficult to review it.

> -    description: |
> -      Specifies the summary IRQ.
> +    description: Interrupt outputs to the system interrupt controller (sysirq).
>  
>    "#interrupt-cells":
>      const: 2
> @@ -57,59 +57,6 @@ required:
>    - gpio-controller
>    - "#gpio-cells"
>  
> -allOf:
> -  - $ref: "pinctrl.yaml#"
> -  - if:
> -      properties:

Make the move of this hunk in your description cleanup patch. Don't mix
functional changes and some cleanups.

> -        compatible:
> -          contains:
> -            const: mediatek,mt6779-pinctrl
> -    then:
> -      properties:
> -        reg:
> -          minItems: 9
> -          maxItems: 9
> -
> -        reg-names:
> -          items:
> -            - const: gpio
> -            - const: iocfg_rm
> -            - const: iocfg_br
> -            - const: iocfg_lm
> -            - const: iocfg_lb
> -            - const: iocfg_rt
> -            - const: iocfg_lt
> -            - const: iocfg_tl
> -            - const: eint
> -  - if:
> -      properties:
> -        compatible:
> -          contains:
> -            const: mediatek,mt6797-pinctrl
> -    then:
> -      properties:
> -        reg:
> -          minItems: 5
> -          maxItems: 5
> -
> -        reg-names:
> -          items:
> -            - const: gpio
> -            - const: iocfgl
> -            - const: iocfgb
> -            - const: iocfgr
> -            - const: iocfgt
> -  - if:
> -      properties:
> -        reg-names:
> -          contains:
> -            const: eint
> -    then:
> -      required:
> -        - interrupts
> -        - interrupt-controller
> -        - "#interrupt-cells"
> -
>  patternProperties:
>    '-pins$':
>      type: object
> @@ -169,8 +116,7 @@ patternProperties:
>  
>            input-schmitt-disable: true
>  
> -          drive-strength:
> -            enum: [2, 4, 8, 12, 16]
> +          drive-strength: true
>  
>            slew-rate:
>              enum: [0, 1]
> @@ -202,6 +148,110 @@ patternProperties:
>  
>          additionalProperties: false
>  
> +allOf:
> +  - $ref: "pinctrl.yaml#"
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: mediatek,mt6779-pinctrl
> +    then:
> +      properties:
> +        reg:
> +          minItems: 9
> +          maxItems: 9
> +
> +        reg-names:
> +          items:
> +            - const: gpio
> +            - const: iocfg_rm
> +            - const: iocfg_br
> +            - const: iocfg_lm
> +            - const: iocfg_lb
> +            - const: iocfg_rt
> +            - const: iocfg_lt
> +            - const: iocfg_tl
> +            - const: eint
> +
> +        interrupts:
> +          items:
> +            - description: EINT interrupt
> +
> +      patternProperties:
> +        '-pins$':
> +          patternProperties:
> +            '^pins':
> +              properties:
> +                drive-strength:
> +                  enum: [2, 4, 8, 12, 16]
> +
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: mediatek,mt6795-pinctrl
> +    then:
> +      properties:
> +        reg:
> +          minItems: 2
> +          maxItems: 2
> +
> +        reg-names:
> +          items:
> +            - const: base
> +            - const: eint
> +
> +        interrupts:
> +          items:
> +            - description: EINT interrupt
> +            - description: EINT event_b interrupt
> +
> +      patternProperties:
> +        '-pins$':
> +          patternProperties:
> +            '^pins':
> +              properties:
> +                drive-strength:
> +                  enum: [2, 4, 6, 8, 10, 12, 14, 16]
> +
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: mediatek,mt6797-pinctrl
> +    then:
> +      properties:
> +        reg:
> +          minItems: 5
> +          maxItems: 5
> +
> +        reg-names:
> +          items:
> +            - const: gpio
> +            - const: iocfgl
> +            - const: iocfgb
> +            - const: iocfgr
> +            - const: iocfgt
> +
> +      patternProperties:
> +        '-pins$':
> +          patternProperties:
> +            '^pins':
> +              properties:
> +                drive-strength:
> +                  enum: [2, 4, 8, 12, 16]
> +
> +  - if:
> +      properties:
> +        reg-names:
> +          contains:
> +            const: eint
> +    then:
> +      required:
> +        - interrupts
> +        - interrupt-controller
> +        - "#interrupt-cells"
> +
>  additionalProperties: false
>  
>  examples:
> @@ -237,8 +287,9 @@ examples:
>              #interrupt-cells = <2>;
>              interrupts = <GIC_SPI 204 IRQ_TYPE_LEVEL_HIGH>;
>  
> -            mmc0_pins_default: mmc0-0 {
> -                cmd-dat-pins {

How this is related to the patch?

Organize the patches so they are easy for review.

Best regards,
Krzysztof