2022-09-19 17:41:08

by Yassine Oudjana

[permalink] [raw]
Subject: [PATCH 1/4] dt-bindings: pinctrl: Combine MediaTek MT67xx pinctrl binding docs

From: Yassine Oudjana <[email protected]>

Documents for MT6779, MT6795 and MT6797 that currently exist share
most properties, and each one has slightly differently worded
descriptions for those properties. Combine all three documents into
one common document for all MT67xx SoC pin controllers, picking a few
parts from each and accounting for differences such as items in reg
and reg-names properties. Also document the MT6765 pin controller
which currently has a driver but no DT binding documentation. It should
be possible to also include bindings for MT8183 and MT8188, but these
have some additional properties that might complicate things a bit,
so they are left alone for now.

Signed-off-by: Yassine Oudjana <[email protected]>
---
.../pinctrl/mediatek,mt6779-pinctrl.yaml | 207 ------------------
.../pinctrl/mediatek,mt6797-pinctrl.yaml | 176 ---------------
...6795.yaml => mediatek,mt67xx-pinctrl.yaml} | 181 +++++++++++----
MAINTAINERS | 2 +-
4 files changed, 135 insertions(+), 431 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/pinctrl/mediatek,mt6779-pinctrl.yaml
delete mode 100644 Documentation/devicetree/bindings/pinctrl/mediatek,mt6797-pinctrl.yaml
rename Documentation/devicetree/bindings/pinctrl/{mediatek,pinctrl-mt6795.yaml => mediatek,mt67xx-pinctrl.yaml} (65%)

diff --git a/Documentation/devicetree/bindings/pinctrl/mediatek,mt6779-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/mediatek,mt6779-pinctrl.yaml
deleted file mode 100644
index 8c79fcef7c52..000000000000
--- a/Documentation/devicetree/bindings/pinctrl/mediatek,mt6779-pinctrl.yaml
+++ /dev/null
@@ -1,207 +0,0 @@
-# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
-%YAML 1.2
----
-$id: http://devicetree.org/schemas/pinctrl/mediatek,mt6779-pinctrl.yaml#
-$schema: http://devicetree.org/meta-schemas/core.yaml#
-
-title: Mediatek MT6779 Pin Controller
-
-maintainers:
- - Andy Teng <[email protected]>
-
-description: |+
- The pin controller node should be the child of a syscon node with the
- required property:
- - compatible: "syscon"
-
-properties:
- compatible:
- const: mediatek,mt6779-pinctrl
-
- 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"
-
- gpio-controller: true
-
- "#gpio-cells":
- const: 2
- description: |
- Number of cells in GPIO specifier. Since the generic GPIO
- binding is used, the amount of cells must be specified as 2. See the below
- mentioned gpio binding representation for description of particular cells.
-
- gpio-ranges:
- minItems: 1
- maxItems: 5
- description: |
- GPIO valid number range.
-
- interrupt-controller: true
-
- interrupts:
- maxItems: 1
- description: |
- Specifies the summary IRQ.
-
- "#interrupt-cells":
- const: 2
-
-allOf:
- - $ref: "pinctrl.yaml#"
-
-required:
- - compatible
- - reg
- - reg-names
- - gpio-controller
- - "#gpio-cells"
- - gpio-ranges
- - interrupt-controller
- - interrupts
- - "#interrupt-cells"
-
-patternProperties:
- '-[0-9]*$':
- type: object
- additionalProperties: false
-
- patternProperties:
- '-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"
-
- 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.
-
- bias-disable: true
-
- bias-pull-up: true
-
- bias-pull-down: true
-
- input-enable: true
-
- input-disable: true
-
- output-low: true
-
- output-high: true
-
- input-schmitt-enable: true
-
- input-schmitt-disable: true
-
- mediatek,pull-up-adv:
- description: |
- Pull up setings for 2 pull resistors, R0 and R1. User can
- configure those special pins. Valid arguments are described as below:
- 0: (R1, R0) = (0, 0) which means R1 disabled and R0 disabled.
- 1: (R1, R0) = (0, 1) which means R1 disabled and R0 enabled.
- 2: (R1, R0) = (1, 0) which means R1 enabled and R0 disabled.
- 3: (R1, R0) = (1, 1) which means R1 enabled and R0 enabled.
- $ref: /schemas/types.yaml#/definitions/uint32
- enum: [0, 1, 2, 3]
-
- mediatek,pull-down-adv:
- description: |
- Pull down settings for 2 pull resistors, R0 and R1. User can
- configure those special pins. Valid arguments are described as below:
- 0: (R1, R0) = (0, 0) which means R1 disabled and R0 disabled.
- 1: (R1, R0) = (0, 1) which means R1 disabled and R0 enabled.
- 2: (R1, R0) = (1, 0) which means R1 enabled and R0 disabled.
- 3: (R1, R0) = (1, 1) which means R1 enabled and R0 enabled.
- $ref: /schemas/types.yaml#/definitions/uint32
- enum: [0, 1, 2, 3]
-
- required:
- - pinmux
-
- additionalProperties: false
-
-additionalProperties: false
-
-examples:
- - |
- #include <dt-bindings/interrupt-controller/irq.h>
- #include <dt-bindings/interrupt-controller/arm-gic.h>
- #include <dt-bindings/pinctrl/mt6779-pinfunc.h>
-
- soc {
- #address-cells = <2>;
- #size-cells = <2>;
-
- pio: pinctrl@10005000 {
- compatible = "mediatek,mt6779-pinctrl";
- reg = <0 0x10005000 0 0x1000>,
- <0 0x11c20000 0 0x1000>,
- <0 0x11d10000 0 0x1000>,
- <0 0x11e20000 0 0x1000>,
- <0 0x11e70000 0 0x1000>,
- <0 0x11ea0000 0 0x1000>,
- <0 0x11f20000 0 0x1000>,
- <0 0x11f30000 0 0x1000>,
- <0 0x1000b000 0 0x1000>;
- reg-names = "gpio", "iocfg_rm",
- "iocfg_br", "iocfg_lm",
- "iocfg_lb", "iocfg_rt",
- "iocfg_lt", "iocfg_tl",
- "eint";
- gpio-controller;
- #gpio-cells = <2>;
- gpio-ranges = <&pio 0 0 210>;
- interrupt-controller;
- #interrupt-cells = <2>;
- interrupts = <GIC_SPI 204 IRQ_TYPE_LEVEL_HIGH>;
-
- mmc0_pins_default: mmc0-0 {
- cmd-dat-pins {
- pinmux = <PINMUX_GPIO168__FUNC_MSDC0_DAT0>,
- <PINMUX_GPIO172__FUNC_MSDC0_DAT1>,
- <PINMUX_GPIO169__FUNC_MSDC0_DAT2>,
- <PINMUX_GPIO177__FUNC_MSDC0_DAT3>,
- <PINMUX_GPIO170__FUNC_MSDC0_DAT4>,
- <PINMUX_GPIO173__FUNC_MSDC0_DAT5>,
- <PINMUX_GPIO171__FUNC_MSDC0_DAT6>,
- <PINMUX_GPIO174__FUNC_MSDC0_DAT7>,
- <PINMUX_GPIO167__FUNC_MSDC0_CMD>;
- input-enable;
- mediatek,pull-up-adv = <1>;
- };
- clk-pins {
- pinmux = <PINMUX_GPIO176__FUNC_MSDC0_CLK>;
- mediatek,pull-down-adv = <2>;
- };
- rst-pins {
- pinmux = <PINMUX_GPIO178__FUNC_MSDC0_RSTB>;
- mediatek,pull-up-adv = <0>;
- };
- };
- };
-
- mmc0 {
- pinctrl-0 = <&mmc0_pins_default>;
- pinctrl-names = "default";
- };
- };
diff --git a/Documentation/devicetree/bindings/pinctrl/mediatek,mt6797-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/mediatek,mt6797-pinctrl.yaml
deleted file mode 100644
index 637a8386e23e..000000000000
--- a/Documentation/devicetree/bindings/pinctrl/mediatek,mt6797-pinctrl.yaml
+++ /dev/null
@@ -1,176 +0,0 @@
-# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
-%YAML 1.2
----
-$id: http://devicetree.org/schemas/pinctrl/mediatek,mt6797-pinctrl.yaml#
-$schema: http://devicetree.org/meta-schemas/core.yaml#
-
-title: Mediatek MT6797 Pin Controller
-
-maintainers:
- - Sean Wang <[email protected]>
-
-description: |+
- The MediaTek's MT6797 Pin controller is used to control SoC pins.
-
-properties:
- compatible:
- const: mediatek,mt6797-pinctrl
-
- reg:
- minItems: 5
- maxItems: 5
-
- reg-names:
- items:
- - const: gpio
- - const: iocfgl
- - const: iocfgb
- - const: iocfgr
- - const: iocfgt
-
- gpio-controller: true
-
- "#gpio-cells":
- const: 2
- description: |
- Number of cells in GPIO specifier. Since the generic GPIO
- binding is used, the amount of cells must be specified as 2. See the below
- mentioned gpio binding representation for description of particular cells.
-
- interrupt-controller: true
-
- interrupts:
- maxItems: 1
-
- "#interrupt-cells":
- const: 2
-
-allOf:
- - $ref: "pinctrl.yaml#"
-
-required:
- - compatible
- - reg
- - reg-names
- - gpio-controller
- - "#gpio-cells"
-
-patternProperties:
- '-[0-9]+$':
- type: object
- additionalProperties: false
- patternProperties:
- 'pins':
- type: object
- additionalProperties: false
- 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"
-
- 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 <soc>-pinfunc.h directly.
-
- bias-disable: true
-
- bias-pull-up: true
-
- bias-pull-down: true
-
- input-enable: true
-
- input-disable: true
-
- output-enable: true
-
- output-low: true
-
- output-high: true
-
- input-schmitt-enable: true
-
- input-schmitt-disable: true
-
- drive-strength:
- enum: [2, 4, 8, 12, 16]
-
- slew-rate:
- enum: [0, 1]
-
- mediatek,pull-up-adv:
- description: |
- Pull up setings for 2 pull resistors, R0 and R1. User can
- configure those special pins. Valid arguments are described as below:
- 0: (R1, R0) = (0, 0) which means R1 disabled and R0 disabled.
- 1: (R1, R0) = (0, 1) which means R1 disabled and R0 enabled.
- 2: (R1, R0) = (1, 0) which means R1 enabled and R0 disabled.
- 3: (R1, R0) = (1, 1) which means R1 enabled and R0 enabled.
- $ref: /schemas/types.yaml#/definitions/uint32
- enum: [0, 1, 2, 3]
-
- mediatek,pull-down-adv:
- description: |
- Pull down settings for 2 pull resistors, R0 and R1. User can
- configure those special pins. Valid arguments are described as below:
- 0: (R1, R0) = (0, 0) which means R1 disabled and R0 disabled.
- 1: (R1, R0) = (0, 1) which means R1 disabled and R0 enabled.
- 2: (R1, R0) = (1, 0) which means R1 enabled and R0 disabled.
- 3: (R1, R0) = (1, 1) which means R1 enabled and R0 enabled.
- $ref: /schemas/types.yaml#/definitions/uint32
- enum: [0, 1, 2, 3]
-
- mediatek,tdsel:
- description: |
- An integer describing the steps for output level shifter duty
- cycle when asserted (high pulse width adjustment). Valid arguments
- are from 0 to 15.
- $ref: /schemas/types.yaml#/definitions/uint32
-
- mediatek,rdsel:
- description: |
- An integer describing the steps for input level shifter duty cycle
- when asserted (high pulse width adjustment). Valid arguments are
- from 0 to 63.
- $ref: /schemas/types.yaml#/definitions/uint32
-
- required:
- - pinmux
-
-additionalProperties: false
-
-examples:
- - |
- #include <dt-bindings/interrupt-controller/irq.h>
- #include <dt-bindings/interrupt-controller/arm-gic.h>
- #include <dt-bindings/pinctrl/mt6797-pinfunc.h>
-
- soc {
- #address-cells = <2>;
- #size-cells = <2>;
-
- pio: pinctrl@10005000 {
- compatible = "mediatek,mt6797-pinctrl";
- reg = <0 0x10005000 0 0x1000>,
- <0 0x10002000 0 0x400>,
- <0 0x10002400 0 0x400>,
- <0 0x10002800 0 0x400>,
- <0 0x10002C00 0 0x400>;
- reg-names = "gpio", "iocfgl", "iocfgb", "iocfgr", "iocfgt";
- gpio-controller;
- #gpio-cells = <2>;
-
- uart_pins_a: uart-0 {
- pins1 {
- pinmux = <MT6797_GPIO232__FUNC_URXD1>,
- <MT6797_GPIO233__FUNC_UTXD1>;
- };
- };
- };
- };
diff --git a/Documentation/devicetree/bindings/pinctrl/mediatek,pinctrl-mt6795.yaml b/Documentation/devicetree/bindings/pinctrl/mediatek,mt67xx-pinctrl.yaml
similarity index 65%
rename from Documentation/devicetree/bindings/pinctrl/mediatek,pinctrl-mt6795.yaml
rename to Documentation/devicetree/bindings/pinctrl/mediatek,mt67xx-pinctrl.yaml
index 73ae6e11410b..1a1a03cede3c 100644
--- a/Documentation/devicetree/bindings/pinctrl/mediatek,pinctrl-mt6795.yaml
+++ b/Documentation/devicetree/bindings/pinctrl/mediatek,mt67xx-pinctrl.yaml
@@ -1,53 +1,154 @@
# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
%YAML 1.2
---
-$id: http://devicetree.org/schemas/pinctrl/mediatek,pinctrl-mt6795.yaml#
+$id: http://devicetree.org/schemas/pinctrl/mediatek,mt67xx-pinctrl.yaml#
$schema: http://devicetree.org/meta-schemas/core.yaml#

-title: Mediatek MT6795 Pin Controller
+title: Mediatek MT67xx Pin Controller

maintainers:
+ - Andy Teng <[email protected]>
- AngeloGioacchino Del Regno <[email protected]>
- Sean Wang <[email protected]>
+ - Yassine Oudjana <[email protected]>

-description: |
- The Mediatek's Pin controller is used to control SoC pins.
+description:
+ The MediaTek pin controller on MT67xx SoCs is used to control
+ pin functions, pull up/down resistance and drive strength options.

properties:
compatible:
- const: mediatek,mt6795-pinctrl
+ oneOf:
+ - enum:
+ - mediatek,mt6765-pinctrl
+ - mediatek,mt6795-pinctrl
+ - mediatek,mt6797-pinctrl
+ - items:
+ - const: mediatek,mt6779-pinctrl
+ - const: syscon
+
+ reg:
+ description: Physical addresses for GPIO base(s) and EINT registers.
+
+ reg-names: true

gpio-controller: true

- '#gpio-cells':
- description: |
- Number of cells in GPIO specifier. Since the generic GPIO binding is used,
- the amount of cells must be specified as 2. See the below
- mentioned gpio binding representation for description of particular cells.
+ "#gpio-cells":
const: 2
+ description: |
+ Number of cells in GPIO specifier. Since the generic GPIO binding
+ is used, the amount of cells must be specified as 2. See the below
+ mentioned gpio binding representation for description of particular
+ cells.

gpio-ranges:
description: GPIO valid number range.
maxItems: 1

- reg:
- description:
- Physical address base for gpio base and eint registers.
- minItems: 2
-
- reg-names:
- items:
- - const: base
- - const: eint
+ interrupts:
+ description: The interrupt outputs to sysirq.

interrupt-controller: true

- '#interrupt-cells':
+ "#interrupt-cells":
const: 2

- interrupts:
- description: The interrupt outputs to sysirq.
- maxItems: 1
+required:
+ - compatible
+ - reg
+ - reg-names
+ - gpio-controller
+ - "#gpio-cells"
+
+allOf:
+ - $ref: "pinctrl.yaml#"
+ - if:
+ properties:
+ compatible:
+ contains:
+ const: mediatek,mt6765-pinctrl
+ then:
+ properties:
+ reg:
+ minItems: 9
+ maxItems: 9
+
+ reg-names:
+ items:
+ - const: iocfg0
+ - const: iocfg1
+ - const: iocfg2
+ - const: iocfg3
+ - const: iocfg4
+ - const: iocfg5
+ - const: iocfg6
+ - const: iocfg7
+ - const: eint
+ - 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
+ - if:
+ properties:
+ compatible:
+ contains:
+ const: mediatek,mt6795-pinctrl
+ then:
+ properties:
+ reg:
+ minItems: 2
+
+ reg-names:
+ items:
+ - const: base
+ - 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"

# PIN CONFIGURATION NODES
patternProperties:
@@ -59,24 +160,24 @@ patternProperties:
type: object
additionalProperties: false
description: |
- A pinctrl node should contain at least one subnodes representing the
+ A pinctrl node should contain at least one subnode 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.
+ pins it needs and how they should be configured, with regard to mux
+ configuration, pull-ups/downs, drive strength, input enable/disable
+ and schmitt input.
An example of using macro:
pincontroller {
/* GPIO0 set as multifunction GPIO0 */
gpio-pins {
pins {
pinmux = <PINMUX_GPIO0__FUNC_GPIO0>;
- }
+ };
};
/* GPIO45 set as multifunction SDA0 */
i2c0-pins {
pins {
pinmux = <PINMUX_GPIO45__FUNC_SDA0>;
- }
+ };
};
};
$ref: "pinmux-node.yaml"
@@ -96,20 +197,20 @@ patternProperties:
oneOf:
- type: boolean
- enum: [100, 101, 102, 103]
- description: mt6795 pull down PUPD/R0/R1 type define value.
+ 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
+ values. When pull down type is PUPD/R0/R1, adding R1R0 defines
will set different resistance values.

bias-pull-up:
oneOf:
- type: boolean
- enum: [100, 101, 102, 103]
- description: mt6795 pull up PUPD/R0/R1 type define value.
+ 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
+ values. When pull up type is PUPD/R0/R1, adding R1R0 defines
will set different resistance values.

bias-disable: true
@@ -151,20 +252,6 @@ patternProperties:
required:
- pinmux

-allOf:
- - $ref: "pinctrl.yaml#"
-
-required:
- - compatible
- - reg
- - reg-names
- - interrupts
- - interrupt-controller
- - '#interrupt-cells'
- - gpio-controller
- - '#gpio-cells'
- - gpio-ranges
-
additionalProperties: false

examples:
diff --git a/MAINTAINERS b/MAINTAINERS
index 677574d5c9a0..5aa093aaa85e 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -16213,7 +16213,7 @@ M: Sean Wang <[email protected]>
L: [email protected] (moderated for non-subscribers)
S: Maintained
F: Documentation/devicetree/bindings/pinctrl/mediatek,mt65xx-pinctrl.yaml
-F: Documentation/devicetree/bindings/pinctrl/mediatek,mt6797-pinctrl.yaml
+F: Documentation/devicetree/bindings/pinctrl/mediatek,mt67xx-pinctrl.yaml
F: Documentation/devicetree/bindings/pinctrl/mediatek,mt7622-pinctrl.yaml
F: Documentation/devicetree/bindings/pinctrl/mediatek,mt8183-pinctrl.yaml
F: drivers/pinctrl/mediatek/
--
2.37.3


Subject: Re: [PATCH 1/4] dt-bindings: pinctrl: Combine MediaTek MT67xx pinctrl binding docs

Il 19/09/22 19:01, Yassine Oudjana ha scritto:
> From: Yassine Oudjana <[email protected]>
>
> Documents for MT6779, MT6795 and MT6797 that currently exist share
> most properties, and each one has slightly differently worded
> descriptions for those properties. Combine all three documents into
> one common document for all MT67xx SoC pin controllers, picking a few
> parts from each and accounting for differences such as items in reg
> and reg-names properties. Also document the MT6765 pin controller
> which currently has a driver but no DT binding documentation. It should
> be possible to also include bindings for MT8183 and MT8188, but these
> have some additional properties that might complicate things a bit,
> so they are left alone for now.
>
> Signed-off-by: Yassine Oudjana <[email protected]>
> ---
> .../pinctrl/mediatek,mt6779-pinctrl.yaml | 207 ------------------
> .../pinctrl/mediatek,mt6797-pinctrl.yaml | 176 ---------------
> ...6795.yaml => mediatek,mt67xx-pinctrl.yaml} | 181 +++++++++++----

Hello Yassine,
nice cleanup over here!

There's a catch though: as far as I know, wildcards are not permitted... so you
should, at this point, merge all of these in mediatek,mt6779-pinctrl.yaml instead.

Before jumping to that, though... Krzysztof, can you please confirm (or deny)?

Regards,
Angelo

> MAINTAINERS | 2 +-
> 4 files changed, 135 insertions(+), 431 deletions(-)
> delete mode 100644 Documentation/devicetree/bindings/pinctrl/mediatek,mt6779-pinctrl.yaml
> delete mode 100644 Documentation/devicetree/bindings/pinctrl/mediatek,mt6797-pinctrl.yaml
> rename Documentation/devicetree/bindings/pinctrl/{mediatek,pinctrl-mt6795.yaml => mediatek,mt67xx-pinctrl.yaml} (65%)
>

2022-09-21 07:44:08

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 1/4] dt-bindings: pinctrl: Combine MediaTek MT67xx pinctrl binding docs

On 19/09/2022 19:01, Yassine Oudjana wrote:
> From: Yassine Oudjana <[email protected]>
>
> Documents for MT6779, MT6795 and MT6797 that currently exist share
> most properties, and each one has slightly differently worded
> descriptions for those properties. Combine all three documents into
> one common document for all MT67xx SoC pin controllers, picking a few
> parts from each and accounting for differences such as items in reg
> and reg-names properties. Also document the MT6765 pin controller
> which currently has a driver but no DT binding documentation. It should
> be possible to also include bindings for MT8183 and MT8188, but these
> have some additional properties that might complicate things a bit,
> so they are left alone for now.
>

> properties:
> compatible:
> - const: mediatek,mt6795-pinctrl
> + oneOf:
> + - enum:
> + - mediatek,mt6765-pinctrl
> + - mediatek,mt6795-pinctrl
> + - mediatek,mt6797-pinctrl
> + - items:
> + - const: mediatek,mt6779-pinctrl
> + - const: syscon

No, this is not like old bindings at all. It's not merging, it's a
change sneaked inside huge diff. Also - probably totally untested on DTS
(or old bindings were broken).

That's a no-go.

Best regards,
Krzysztof

2022-09-21 07:45:41

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 1/4] dt-bindings: pinctrl: Combine MediaTek MT67xx pinctrl binding docs

On 20/09/2022 10:06, AngeloGioacchino Del Regno wrote:
> Il 19/09/22 19:01, Yassine Oudjana ha scritto:
>> From: Yassine Oudjana <[email protected]>
>>
>> Documents for MT6779, MT6795 and MT6797 that currently exist share
>> most properties, and each one has slightly differently worded
>> descriptions for those properties. Combine all three documents into
>> one common document for all MT67xx SoC pin controllers, picking a few
>> parts from each and accounting for differences such as items in reg
>> and reg-names properties. Also document the MT6765 pin controller
>> which currently has a driver but no DT binding documentation. It should
>> be possible to also include bindings for MT8183 and MT8188, but these
>> have some additional properties that might complicate things a bit,
>> so they are left alone for now.
>>
>> Signed-off-by: Yassine Oudjana <[email protected]>
>> ---
>> .../pinctrl/mediatek,mt6779-pinctrl.yaml | 207 ------------------
>> .../pinctrl/mediatek,mt6797-pinctrl.yaml | 176 ---------------
>> ...6795.yaml => mediatek,mt67xx-pinctrl.yaml} | 181 +++++++++++----
>
> Hello Yassine,
> nice cleanup over here!
>
> There's a catch though: as far as I know, wildcards are not permitted... so you
> should, at this point, merge all of these in mediatek,mt6779-pinctrl.yaml instead.
>
> Before jumping to that, though... Krzysztof, can you please confirm (or deny)?

Wildcards are not allowed in compatibles. In filename wildcards or
family name could work if they are really going to match the devices. I
have doubts here. 67xx is quite a lot of different devices, so I am not
sure this will cover them all.

I would prefer one name (oldest SoC or lowest number).

Best regards,
Krzysztof

2022-09-21 09:45:23

by Yassine Oudjana

[permalink] [raw]
Subject: Re: [PATCH 1/4] dt-bindings: pinctrl: Combine MediaTek MT67xx pinctrl binding docs


On Wed, Sep 21 2022 at 09:20:43 AM +0200, Krzysztof Kozlowski
<[email protected]> wrote:
> On 19/09/2022 19:01, Yassine Oudjana wrote:
>> From: Yassine Oudjana <[email protected]>
>>
>> Documents for MT6779, MT6795 and MT6797 that currently exist share
>> most properties, and each one has slightly differently worded
>> descriptions for those properties. Combine all three documents into
>> one common document for all MT67xx SoC pin controllers, picking a
>> few
>> parts from each and accounting for differences such as items in reg
>> and reg-names properties. Also document the MT6765 pin controller
>> which currently has a driver but no DT binding documentation. It
>> should
>> be possible to also include bindings for MT8183 and MT8188, but
>> these
>> have some additional properties that might complicate things a bit,
>> so they are left alone for now.
>>
>
>> properties:
>> compatible:
>> - const: mediatek,mt6795-pinctrl
>> + oneOf:
>> + - enum:
>> + - mediatek,mt6765-pinctrl
>> + - mediatek,mt6795-pinctrl
>> + - mediatek,mt6797-pinctrl
>> + - items:
>> + - const: mediatek,mt6779-pinctrl
>> + - const: syscon
>
> No, this is not like old bindings at all. It's not merging, it's a
> change sneaked inside huge diff. Also - probably totally untested on
> DTS
> (or old bindings were broken).


Actually this change was made specifically so that it remains (probably
becomes?) compatible with existing DTS and passes checks. mt6779.dtsi
currently has the syscon compatible string but it wasn't listed along
with mediatek,mt6779-pinctrl in the old document, but instead there was
something in the description about putting the pinctrl node under a
syscon node, which isn't the case in the existing DTS. This patch
passed both dt_binding_check and dtbs_check. Anyway, I see how I failed
to describe this change, so I'll go through the patch again and try to
find any other small changes I might've made and forgotten about, and
either put them in separate patches or describe them in the commit
message, whichever one you think is better. Also, do I make those
changes in the original documents then combine or combine first then
make them in the new one?

Thanks,
Yassine

(Sorry for the spam, my client was misconfigured so it previously sent
HTML instead of plain text.)

>
> That's a no-go.
>
> Best regards,
> Krzysztof


Subject: Re: [PATCH 1/4] dt-bindings: pinctrl: Combine MediaTek MT67xx pinctrl binding docs

Il 21/09/22 11:30, [email protected] ha scritto:
>
>
> On Wed, Sep 21 2022 at 09:11:12 AM +0200, Krzysztof Kozlowski
> <[email protected]> wrote:
>> On 20/09/2022 10:06, AngeloGioacchino Del Regno wrote:
>>>  Il 19/09/22 19:01, Yassine Oudjana ha scritto:
>>>>  From: Yassine Oudjana <[email protected]>
>>>>
>>>>  Documents for MT6779, MT6795 and MT6797 that currently exist share
>>>>  most properties, and each one has slightly differently worded
>>>>  descriptions for those properties. Combine all three documents into
>>>>  one common document for all MT67xx SoC pin controllers, picking a few
>>>>  parts from each and accounting for differences such as items in reg
>>>>  and reg-names properties. Also document the MT6765 pin controller
>>>>  which currently has a driver but no DT binding documentation. It should
>>>>  be possible to also include bindings for MT8183 and MT8188, but these
>>>>  have some additional properties that might complicate things a bit,
>>>>  so they are left alone for now.
>>>>
>>>>  Signed-off-by: Yassine Oudjana <[email protected]>
>>>>  ---
>>>>    .../pinctrl/mediatek,mt6779-pinctrl.yaml      | 207 ------------------
>>>>    .../pinctrl/mediatek,mt6797-pinctrl.yaml      | 176 ---------------
>>>>    ...6795.yaml => mediatek,mt67xx-pinctrl.yaml} | 181 +++++++++++----
>>>
>>>  Hello Yassine,
>>>  nice cleanup over here!
>>>
>>>  There's a catch though: as far as I know, wildcards are not permitted... so you
>>>  should, at this point, merge all of these in mediatek,mt6779-pinctrl.yaml instead.
>>>
>>>  Before jumping to that, though... Krzysztof, can you please confirm (or deny)?
>>
>> Wildcards are not allowed in compatibles. In filename wildcards or
>> family name could work if they are really going to match the devices. I
>> have doubts here. 67xx is quite a lot of different devices, so I am not
>> sure this will cover them all.
>>
>> I would prefer one name (oldest SoC or lowest number).
>
> Lowest number (and probably oldest too but not sure since mediatek naming
> conventions are a bit weird) currently documented is mt6779, but mt6765 gets
> documented in this patch and mt6735 (this one I know for sure is older than the
> rest) in a following patch, so do I just stick with mt6779 or do I change it in the
> following patches documenting mt6765 and mt6735?
>

I see the sequence as:

1. You merge mediatek,mt6797-pinctrl.yaml into mediatek,mt6779-pinctrl.yaml; then
2. Adding MT6765 documentation to mediatek,mt6779-pinctrl.yaml; then
3. Adding support for MT6735, documentation goes again to 6779-pinctrl.

This means that you're working with mediatek,mt6779-pinctrl.yaml :-)

P.S.: That was also a suggestion about how to split things per-commit!

Cheers,
Angelo

> Thanks,
> Yassine
>
>>
>> Best regards,
>> Krzysztof
>
>

2022-09-21 10:42:27

by Yassine Oudjana

[permalink] [raw]
Subject: Re: [PATCH 1/4] dt-bindings: pinctrl: Combine MediaTek MT67xx pinctrl binding docs



On Wed, Sep 21 2022 at 09:11:12 AM +0200, Krzysztof Kozlowski
<[email protected]> wrote:
> On 20/09/2022 10:06, AngeloGioacchino Del Regno wrote:
>> Il 19/09/22 19:01, Yassine Oudjana ha scritto:
>>> From: Yassine Oudjana <[email protected]>
>>>
>>> Documents for MT6779, MT6795 and MT6797 that currently exist share
>>> most properties, and each one has slightly differently worded
>>> descriptions for those properties. Combine all three documents into
>>> one common document for all MT67xx SoC pin controllers, picking a
>>> few
>>> parts from each and accounting for differences such as items in reg
>>> and reg-names properties. Also document the MT6765 pin controller
>>> which currently has a driver but no DT binding documentation. It
>>> should
>>> be possible to also include bindings for MT8183 and MT8188, but
>>> these
>>> have some additional properties that might complicate things a bit,
>>> so they are left alone for now.
>>>
>>> Signed-off-by: Yassine Oudjana <[email protected]>
>>> ---
>>> .../pinctrl/mediatek,mt6779-pinctrl.yaml | 207
>>> ------------------
>>> .../pinctrl/mediatek,mt6797-pinctrl.yaml | 176
>>> ---------------
>>> ...6795.yaml => mediatek,mt67xx-pinctrl.yaml} | 181
>>> +++++++++++----
>>
>> Hello Yassine,
>> nice cleanup over here!
>>
>> There's a catch though: as far as I know, wildcards are not
>> permitted... so you
>> should, at this point, merge all of these in
>> mediatek,mt6779-pinctrl.yaml instead.
>>
>> Before jumping to that, though... Krzysztof, can you please confirm
>> (or deny)?
>
> Wildcards are not allowed in compatibles. In filename wildcards or
> family name could work if they are really going to match the devices.
> I
> have doubts here. 67xx is quite a lot of different devices, so I am
> not
> sure this will cover them all.
>
> I would prefer one name (oldest SoC or lowest number).

Lowest number (and probably oldest too but not sure since mediatek
naming conventions are a bit weird) currently documented is mt6779, but
mt6765 gets documented in this patch and mt6735 (this one I know for
sure is older than the rest) in a following patch, so do I just stick
with mt6779 or do I change it in the following patches documenting
mt6765 and mt6735?

Thanks,
Yassine

>
> Best regards,
> Krzysztof


2022-09-21 10:42:54

by Yassine Oudjana

[permalink] [raw]
Subject: Re: [PATCH 1/4] dt-bindings: pinctrl: Combine MediaTek MT67xx pinctrl binding docs



On Wed, Sep 21 2022 at 11:45:41 AM +0200, AngeloGioacchino Del Regno
<[email protected]> wrote:
> Il 21/09/22 11:30, [email protected] ha scritto:
>>
>>
>> On Wed, Sep 21 2022 at 09:11:12 AM +0200, Krzysztof Kozlowski
>> <[email protected]> wrote:
>>> On 20/09/2022 10:06, AngeloGioacchino Del Regno wrote:
>>>> Il 19/09/22 19:01, Yassine Oudjana ha scritto:
>>>>> From: Yassine Oudjana <[email protected]>
>>>>>
>>>>> Documents for MT6779, MT6795 and MT6797 that currently exist
>>>>> share
>>>>> most properties, and each one has slightly differently worded
>>>>> descriptions for those properties. Combine all three documents
>>>>> into
>>>>> one common document for all MT67xx SoC pin controllers, picking
>>>>> a few
>>>>> parts from each and accounting for differences such as items in
>>>>> reg
>>>>> and reg-names properties. Also document the MT6765 pin controller
>>>>> which currently has a driver but no DT binding documentation. It
>>>>> should
>>>>> be possible to also include bindings for MT8183 and MT8188, but
>>>>> these
>>>>> have some additional properties that might complicate things a
>>>>> bit,
>>>>> so they are left alone for now.
>>>>>
>>>>> Signed-off-by: Yassine Oudjana <[email protected]>
>>>>> ---
>>>>> .../pinctrl/mediatek,mt6779-pinctrl.yaml | 207
>>>>> ------------------
>>>>> .../pinctrl/mediatek,mt6797-pinctrl.yaml | 176
>>>>> ---------------
>>>>> ...6795.yaml => mediatek,mt67xx-pinctrl.yaml} | 181
>>>>> +++++++++++----
>>>>
>>>> Hello Yassine,
>>>> nice cleanup over here!
>>>>
>>>> There's a catch though: as far as I know, wildcards are not
>>>> permitted... so you
>>>> should, at this point, merge all of these in
>>>> mediatek,mt6779-pinctrl.yaml instead.
>>>>
>>>> Before jumping to that, though... Krzysztof, can you please
>>>> confirm (or deny)?
>>>
>>> Wildcards are not allowed in compatibles. In filename wildcards or
>>> family name could work if they are really going to match the
>>> devices. I
>>> have doubts here. 67xx is quite a lot of different devices, so I am
>>> not
>>> sure this will cover them all.
>>>
>>> I would prefer one name (oldest SoC or lowest number).
>>
>> Lowest number (and probably oldest too but not sure since mediatek
>> naming conventions are a bit weird) currently documented is mt6779,
>> but mt6765 gets documented in this patch and mt6735 (this one I
>> know for sure is older than the rest) in a following patch, so do I
>> just stick with mt6779 or do I change it in the following patches
>> documenting mt6765 and mt6735?
>>
>
> I see the sequence as:
>
> 1. You merge mediatek,mt6797-pinctrl.yaml into
> mediatek,mt6779-pinctrl.yaml; then

And mediatek,pinctrl-mt6795 gets merged here too I assume?

> 2. Adding MT6765 documentation to mediatek,mt6779-pinctrl.yaml; then
> 3. Adding support for MT6735, documentation goes again to
> 6779-pinctrl.
>
> This means that you're working with mediatek,mt6779-pinctrl.yaml :-)
>
> P.S.: That was also a suggestion about how to split things per-commit!
>
> Cheers,
> Angelo
>
>> Thanks,
>> Yassine
>>
>>>
>>> Best regards,
>>> Krzysztof
>>
>>
>


Subject: Re: [PATCH 1/4] dt-bindings: pinctrl: Combine MediaTek MT67xx pinctrl binding docs

Il 21/09/22 12:24, [email protected] ha scritto:
>
>
> On Wed, Sep 21 2022 at 11:45:41 AM +0200, AngeloGioacchino Del Regno
> <[email protected]> wrote:
>> Il 21/09/22 11:30, [email protected] ha scritto:
>>>
>>>
>>> On Wed, Sep 21 2022 at 09:11:12 AM +0200, Krzysztof Kozlowski
>>> <[email protected]> wrote:
>>>> On 20/09/2022 10:06, AngeloGioacchino Del Regno wrote:
>>>>>  Il 19/09/22 19:01, Yassine Oudjana ha scritto:
>>>>>>  From: Yassine Oudjana <[email protected]>
>>>>>>
>>>>>>  Documents for MT6779, MT6795 and MT6797 that currently exist share
>>>>>>  most properties, and each one has slightly differently worded
>>>>>>  descriptions for those properties. Combine all three documents into
>>>>>>  one common document for all MT67xx SoC pin controllers, picking a few
>>>>>>  parts from each and accounting for differences such as items in reg
>>>>>>  and reg-names properties. Also document the MT6765 pin controller
>>>>>>  which currently has a driver but no DT binding documentation. It should
>>>>>>  be possible to also include bindings for MT8183 and MT8188, but these
>>>>>>  have some additional properties that might complicate things a bit,
>>>>>>  so they are left alone for now.
>>>>>>
>>>>>>  Signed-off-by: Yassine Oudjana <[email protected]>
>>>>>>  ---
>>>>>>    .../pinctrl/mediatek,mt6779-pinctrl.yaml      | 207 ------------------
>>>>>>    .../pinctrl/mediatek,mt6797-pinctrl.yaml      | 176 ---------------
>>>>>>    ...6795.yaml => mediatek,mt67xx-pinctrl.yaml} | 181 +++++++++++----
>>>>>
>>>>>  Hello Yassine,
>>>>>  nice cleanup over here!
>>>>>
>>>>>  There's a catch though: as far as I know, wildcards are not permitted... so you
>>>>>  should, at this point, merge all of these in mediatek,mt6779-pinctrl.yaml
>>>>> instead.
>>>>>
>>>>>  Before jumping to that, though... Krzysztof, can you please confirm (or deny)?
>>>>
>>>> Wildcards are not allowed in compatibles. In filename wildcards or
>>>> family name could work if they are really going to match the devices. I
>>>> have doubts here. 67xx is quite a lot of different devices, so I am not
>>>> sure this will cover them all.
>>>>
>>>> I would prefer one name (oldest SoC or lowest number).
>>>
>>> Lowest number (and probably oldest too but not sure since mediatek naming
>>> conventions are a bit weird) currently documented is mt6779, but mt6765 gets
>>> documented in this patch and mt6735 (this one I know for sure is older than the
>>> rest) in a following patch, so do I just stick with mt6779 or do I change it in
>>> the following patches documenting mt6765 and mt6735?
>>>
>>
>> I see the sequence as:
>>
>> 1. You merge mediatek,mt6797-pinctrl.yaml into mediatek,mt6779-pinctrl.yaml; then
>
> And mediatek,pinctrl-mt6795 gets merged here too I assume?
>

Yeah sorry about forgetting that one. Anyway, obviously, do one merge operation
per commit!


>> 2. Adding MT6765 documentation to mediatek,mt6779-pinctrl.yaml; then
>> 3. Adding support for MT6735, documentation goes again to 6779-pinctrl.
>>
>> This means that you're working with mediatek,mt6779-pinctrl.yaml :-)
>>
>> P.S.: That was also a suggestion about how to split things per-commit!
>>
>> Cheers,
>> Angelo
>>
>>> Thanks,
>>> Yassine
>>>
>>>>
>>>> Best regards,
>>>> Krzysztof
>>>
>>>
>>
>
>