Re: [PATCH 2/6] dt-bindings: pinctrl: mt7620: add proper function muxing binding

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

 



On 14.12.2022 14:55, Krzysztof Kozlowski wrote:
On 13/12/2022 14:04, Arınç ÜNAL wrote:
Not every function can be muxed to a group. Add proper binding which
documents which function can be muxed to a group or set of groups.

Signed-off-by: Arınç ÜNAL <arinc.unal@xxxxxxxxxx>
---
  .../pinctrl/ralink,mt7620-pinctrl.yaml        | 632 +++++++++++++++++-
  1 file changed, 596 insertions(+), 36 deletions(-)

diff --git a/Documentation/devicetree/bindings/pinctrl/ralink,mt7620-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/ralink,mt7620-pinctrl.yaml
index 6f17f3991640..06880c80ba80 100644
--- a/Documentation/devicetree/bindings/pinctrl/ralink,mt7620-pinctrl.yaml
+++ b/Documentation/devicetree/bindings/pinctrl/ralink,mt7620-pinctrl.yaml
@@ -29,47 +29,608 @@ patternProperties:
          $ref: pinmux-node.yaml#
properties:
-          groups:
-            description: The pin group to select.
-            enum: [
-              # common
-              i2c, spi, wdt,
-
-              # For MT7620 SoC
-              ephy, mdio, nd_sd, pa, pcie, rgmii1, rgmii2, spi refclk,
-              uartf, uartlite, wled,
-
-              # For MT7628 and MT7688 SoCs
-              gpio, i2s, p0led_an, p0led_kn, p1led_an, p1led_kn, p2led_an,
-              p2led_kn, p3led_an, p3led_kn, p4led_an, p4led_kn, perst, pwm0,
-              pwm1, refclk, sdmode, spi cs1, spis, uart0, uart1, uart2,
-              wled_an, wled_kn,
-            ]
-
            function:
-            description: The mux function to select.
-            enum: [
-              # common
-              gpio, i2c, refclk, spi,
-
-              # For MT7620 SoC
-              ephy, gpio i2s, gpio uartf, i2s uartf, mdio, nand, pa,
-              pcie refclk, pcie rst, pcm gpio, pcm i2s, pcm uartf,
-              rgmii1, rgmii2, sd, spi refclk, uartf, uartlite, wdt refclk,
-              wdt rst, wled,
-
-              # For MT7628 and MT7688 SoCs
-              antenna, debug, i2s, jtag, p0led_an, p0led_kn,
-              p1led_an, p1led_kn, p2led_an, p2led_kn, p3led_an, p3led_kn,
-              p4led_an, p4led_kn, pcie, pcm, perst, pwm, pwm0, pwm1, pwm_uart2,
-              rsvd, sdxc, sdxc d5 d4, sdxc d6, sdxc d7, spi cs1,
-              spis, sw_r, uart0, uart1, uart2, utif, wdt, wled_an, wled_kn, -,
-            ]
+            description:
+              A string containing the name of the function to mux to the group.
+            anyOf:
+              - description: For MT7620 SoC
+                enum: [ephy, gpio, gpio i2s, gpio uartf, i2c, i2s uartf, mdio, nand, pa,
+                       pcie refclk, pcie rst, pcm gpio, pcm i2s, pcm uartf, refclk,
+                       rgmii1, rgmii2, sd, spi, spi refclk, uartf, uartlite, wdt refclk,
+                       wdt rst, wled]
+
+              - description: For MT7628 and MT7688 SoCs
+                enum: [antenna, debug, gpio, i2c, i2s, jtag, p0led_an, p0led_kn,
+                       p1led_an, p1led_kn, p2led_an, p2led_kn, p3led_an, p3led_kn,
+                       p4led_an, p4led_kn, pcie, pcm, perst, pwm, pwm0, pwm1, pwm_uart2,
+                       refclk, rsvd, sdxc, sdxc d5 d4, sdxc d6, sdxc d7, spi, spi cs1,
+                       spis, sw_r, uart0, uart1, uart2, utif, wdt, wled_an, wled_kn, -]
+
+          groups:
+            description:
+              An array of strings. Each string contains the name of a group.
required:
            - groups
            - function
+ allOf:
+          - if:
+              properties:
+                function:
+                  const: antenna
+            then:
+              properties:
+                groups:
+                  enum: [i2s]

I have doubts such setup is maintainable and readable. I would suggest
to leave just few - maybe for gpio, jtag, refclk, utif.

These bindings are not going to change once all properly defined and I'm here as a maintainer so I don't see an issue with maintaining the binding.

It's the whole pin configuration of an SoC squashed under a single document. I guess this is the fate of the pinctrl bindings. The bindings for mt7622 is not so different:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/pinctrl/mediatek%2Cmt7622-pinctrl.yaml#n63

It's still much better than reading the code:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/pinctrl/ralink/pinctrl-mt7620.c

Arınç



[Index of Archives]     [LKML Archive]     [Linux ARM Kernel]     [Linux ARM]     [Git]     [Yosemite News]     [Linux SCSI]     [Linux Hams]

  Powered by Linux