lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <e34b35f4-4197-3973-0947-14ee577494e9@linaro.org>
Date:   Fri, 28 Oct 2022 16:01:58 -0400
From:   Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
To:     Yassine Oudjana <yassine.oudjana@...il.com>,
        Linus Walleij <linus.walleij@...aro.org>,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Matthias Brugger <matthias.bgg@...il.com>,
        Sean Wang <sean.wang@...nel.org>,
        Andy Teng <andy.teng@...iatek.com>,
        AngeloGioacchino Del Regno 
        <angelogioacchino.delregno@...labora.com>
Cc:     Yassine Oudjana <y.oudjana@...tonmail.com>,
        linux-mediatek@...ts.infradead.org, linux-gpio@...r.kernel.org,
        devicetree@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v4 09/13] dt-bindings: pinctrl: mediatek,mt6779-pinctrl:
 Add MT6795

On 28/10/2022 11:35, Yassine Oudjana wrote:
> From: Yassine Oudjana <y.oudjana@...tonmail.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@...tonmail.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@...iatek.com>
> +  - AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>
>    - Sean Wang <sean.wang@...nel.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

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ