On 07/10/2022 08:59, Yassine Oudjana wrote:
> From: Yassine Oudjana <y.oudjana@xxxxxxxxxxxxxx>
>
> Combine MT6795 pin controller document into MT6779 one. In the
> process, replace the current interrupts property description with
> the one from the MT6795 document since it makes more sense. Also
> amend property descriptions and examples with more detailed
> information that was available in the MT6795 document, and replace
> the current pinmux node name patterns with ones from it since they
> are more common across mediatek pin controller bindings.
>
> Signed-off-by: Yassine Oudjana <y.oudjana@xxxxxxxxxxxxxx>
> ---
> .../pinctrl/mediatek,mt6779-pinctrl.yaml | 94 ++++++--
> .../pinctrl/mediatek,pinctrl-mt6795.yaml | 227 ------------------
> 2 files changed, 77 insertions(+), 244 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 a2141eb0854e..cada3530dd0a 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,10 @@ properties:
> interrupt-controller: true
>
> interrupts:
> - maxItems: 1
> + minItems: 1
> + maxItems: 2
> description: |
> - Specifies the summary IRQ.
> + The interrupt outputs to sysirq.
I am not sure if description is relevant now for all variants... what is
the sysirq? You have two interrupts so both go to one sysirq?
>
> "#interrupt-cells":
> const: 2
> @@ -81,6 +84,30 @@ allOf:
> - const: iocfg_lt
> - const: iocfg_tl
> - const: eint
> +
> + interrupts:
> + items:
> + - description: EINT interrupt
> +
> + - if:
> + properties:
> + compatible:
> + contains:
> + const: mediatek,mt6795-pinctrl
> + then:
> + properties:
> + reg:
> + minItems: 2
What's the maxItems? You declared reg and reg-names in top-level
properties as accepting anything, therefore you cannot have loose
constraints here.
> +
> + reg-names:
> + items:
> + - const: base
> + - const: eint
> +
> + interrupts:
> + items:
> + - description: EINT interrupt
> + - description: EINT event_b interrupt
Blank line
> - if:
> properties:
> compatible:
> @@ -111,32 +138,50 @@ allOf:
> - "#interrupt-cells"
>
> patternProperties:
> - '-[0-9]*$':
> + '-pins$':
> type: object
> additionalProperties: false
>
> patternProperties:
> - '-pins*$':
> + '^pins':
> type: object
> description: |
> A pinctrl node should contain at least one subnodes representing the
> pinctrl groups available on the machine. Each subnode will list the
> pins it needs, and how they should be configured, with regard to muxer
> - configuration, pullups, drive strength, input enable/disable and input schmitt.
> - $ref: "/schemas/pinctrl/pincfg-node.yaml"
> + configuration, pullups, drive strength, input enable/disable and
> + input schmitt.
> + $ref: "pinmux-node.yaml"
Drop quotes
Why this one is not pincfg-node anymore? All your properties are not
valid then? You mix here so many changes it is a bit difficult to
understand the concept.
>
> properties:
> pinmux:
> description:
> - integer array, represents gpio pin number and mux setting.
> - Supported pin number and mux varies for different SoCs, and are defined
> - as macros in boot/dts/<soc>-pinfunc.h directly.
> + Integer array, represents gpio pin number and mux setting.
> + Supported pin number and mux varies for different SoCs, and are
> + defined as macros in dt-bindings/pinctrl/<soc>-pinfunc.h
> + directly.
>
> bias-disable: true
>
> - bias-pull-up: true
> -
> - bias-pull-down: true
> + bias-pull-up:
> + oneOf:
> + - type: boolean
> + - enum: [100, 101, 102, 103]
Missing ref
> + description: Pull up PUPD/R0/R1 type define value.
> + description: |
> + For normal pull up type, it is not necessary to specify R1R0
> + values; When pull up type is PUPD/R0/R1, adding R1R0 defines
> + will set different resistance values.
> +
> + bias-pull-down:
> + oneOf:
> + - type: boolean
> + - enum: [100, 101, 102, 103]
Missing ref
> + description: Pull down PUPD/R0/R1 type define value.
> + description: |
> + For normal pull down type, it is not necessary to specify R1R0
> + values; When pull down type is PUPD/R0/R1, adding R1R0 defines
> + will set different resistance values.
>
> input-enable: true
>
> @@ -151,7 +196,7 @@ patternProperties:
> input-schmitt-disable: true
>
> drive-strength:
> - enum: [2, 4, 8, 12, 16]
> + enum: [2, 4, 6, 8, 10, 12, 14, 16]
Now you are missing ref - you do not have a type now, because you
removed pincfg-node. Split the merging of different pinctrl bindings and
reorganization.
The drive strengths are also not valid for the other variant...
>
> slew-rate:
> enum: [0, 1]
> @@ -218,8 +263,9 @@ examples:
> #interrupt-cells = <2>;
> interrupts = <GIC_SPI 204 IRQ_TYPE_LEVEL_HIGH>;
>
> - mmc0_pins_default: mmc0-0 {
> - cmd-dat-pins {
> + /* GPIOs 167-174, 176-178 set as multifunction MSDC0 */
> + mmc0_pins_default: mmc0-pins {
> + pins-cmd-dat {
> pinmux = <PINMUX_GPIO168__FUNC_MSDC0_DAT0>,
> <PINMUX_GPIO172__FUNC_MSDC0_DAT1>,
> <PINMUX_GPIO169__FUNC_MSDC0_DAT2>,
> @@ -232,15 +278,29 @@ examples:
> input-enable;
> mediatek,pull-up-adv = <1>;
> };
> - clk-pins {
> + pins-clk {
> pinmux = <PINMUX_GPIO176__FUNC_MSDC0_CLK>;
> mediatek,pull-down-adv = <2>;
> };
> - rst-pins {
> + pins-rst {
> pinmux = <PINMUX_GPIO178__FUNC_MSDC0_RSTB>;
> mediatek,pull-up-adv = <0>;
> };
Best regards,
Krzysztof
