Re: [PATCH v4 09/13] dt-bindings: pinctrl: mediatek,mt6779-pinctrl: Add MT6795

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On Fri, Oct 28 2022 at 16:01:58 -04:00:00, Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxx> wrote:
On 28/10/2022 11:35, Yassine Oudjana wrote:
 From: Yassine Oudjana <y.oudjana@xxxxxxxxxxxxxx>

 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@xxxxxxxxxxxxxx>
 ---
  .../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@xxxxxxxxxxxx>
+ - AngeloGioacchino Del Regno <angelogioacchino.delregno@xxxxxxxxxxxxx>
    - Sean Wang <sean.wang@xxxxxxxxxx>

  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.

Fair point. I'll undo this.

I was thinking that MT6797 doesn't take any interrupts at the moment, but I'm certain the hardware does in fact have interrupts, so leaving the constraint should both keep the bindings more accurate and reduce the DT compatibility breakage when someone implements interrupts later.


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

The idea was that drive-strength was to be defined first then have its constraints set in the conditionals, so I moved this below the patternProperties block to accomplish that. Now I'm thinking of putting it here to begin with so that I wouldn't need to move it in this patch. What do you think?

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

Sorry, this was meant to be in the previous patch that changed the subnode name patterns. I'll move it there for the next revision.

Thanks,
Yassine






[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux