[PATCH 0/2] pinctrl: zynqmp: Support muxing individual pins

[PATCH 0/2] pinctrl: zynqmp: Support muxing individual pins

[Sean Anderson]

This series adds support for muxing individual pins, instead of
requiring groups to be muxed together. See [1] for additional
discussion.

[1] https://lore.kernel.org/linux-arm-kernel/5bb0dc7e-4c89-4f3d-abc6-41ae9ded5ae9@linux.dev/


Sean Anderson (2):
  dt-bindings: pinctrl: xilinx: Add support for function with pins
  pinctrl: zynqmp: Support muxing individual pins

 .../bindings/pinctrl/xlnx,zynqmp-pinctrl.yaml | 344 +++++++++---------
 drivers/pinctrl/pinctrl-zynqmp.c              |  61 +++-
 2 files changed, 219 insertions(+), 186 deletions(-)

-- 
2.35.1.1320.gc452695387.dirty

[PATCH 0/2] pinctrl: zynqmp: Support muxing individual pins

[Sean Anderson]

This series adds support for muxing individual pins, instead of
requiring groups to be muxed together. See [1] for additional
discussion.

[1] https://lore.kernel.org/linux-arm-kernel/5bb0dc7e-4c89-4f3d-abc6-41ae9ded5ae9@linux.dev/


Sean Anderson (2):
  dt-bindings: pinctrl: xilinx: Add support for function with pins
  pinctrl: zynqmp: Support muxing individual pins

 .../bindings/pinctrl/xlnx,zynqmp-pinctrl.yaml | 344 +++++++++---------
 drivers/pinctrl/pinctrl-zynqmp.c              |  61 +++-
 2 files changed, 219 insertions(+), 186 deletions(-)

-- 
2.35.1.1320.gc452695387.dirty


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 1/2] dt-bindings: pinctrl: xilinx: Add support for function with pins

[Sean Anderson]

Support specifying the function per-pin. The driver doesn't care
whethern you use pins or groups for this purpose.

Signed-off-by: Sean Anderson <sean.anderson@linux.dev>
---

 .../bindings/pinctrl/xlnx,zynqmp-pinctrl.yaml | 344 +++++++++---------
 1 file changed, 176 insertions(+), 168 deletions(-)

diff --git a/Documentation/devicetree/bindings/pinctrl/xlnx,zynqmp-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/xlnx,zynqmp-pinctrl.yaml
index f13d315b5d5e..d3b258245e28 100644
--- a/Documentation/devicetree/bindings/pinctrl/xlnx,zynqmp-pinctrl.yaml
+++ b/Documentation/devicetree/bindings/pinctrl/xlnx,zynqmp-pinctrl.yaml
@@ -42,179 +42,187 @@ patternProperties:
         $ref: pinmux-node.yaml#
 
         properties:
+          pins:
+            description:
+              List of pins to select (either this or "groups" must be specified)
+            items:
+              pattern: '^MIO([0-9]|[1-6][0-9]|7[0-7])$'
+
           groups:
             description:
               List of groups to select (either this or "pins" must be
               specified), available groups for this subnode.
             items:
-              enum: [ethernet0_0_grp, ethernet1_0_grp, ethernet2_0_grp,
-                     ethernet3_0_grp, gemtsu0_0_grp, gemtsu0_1_grp,
-                     gemtsu0_2_grp, mdio0_0_grp, mdio1_0_grp,
-                     mdio1_1_grp, mdio2_0_grp, mdio3_0_grp,
-                     qspi0_0_grp, qspi_ss_0_grp, qspi_fbclk_0_grp,
-                     spi0_0_grp, spi0_ss_0_grp, spi0_ss_1_grp,
-                     spi0_ss_2_grp, spi0_1_grp, spi0_ss_3_grp,
-                     spi0_ss_4_grp, spi0_ss_5_grp, spi0_2_grp,
-                     spi0_ss_6_grp, spi0_ss_7_grp, spi0_ss_8_grp,
-                     spi0_3_grp, spi0_ss_9_grp, spi0_ss_10_grp,
-                     spi0_ss_11_grp, spi0_4_grp, spi0_ss_12_grp,
-                     spi0_ss_13_grp, spi0_ss_14_grp, spi0_5_grp,
-                     spi0_ss_15_grp, spi0_ss_16_grp, spi0_ss_17_grp,
-                     spi1_0_grp, spi1_ss_0_grp, spi1_ss_1_grp,
-                     spi1_ss_2_grp, spi1_1_grp, spi1_ss_3_grp,
-                     spi1_ss_4_grp, spi1_ss_5_grp, spi1_2_grp,
-                     spi1_ss_6_grp, spi1_ss_7_grp, spi1_ss_8_grp,
-                     spi1_3_grp, spi1_ss_9_grp, spi1_ss_10_grp,
-                     spi1_ss_11_grp, spi1_4_grp, spi1_ss_12_grp,
-                     spi1_ss_13_grp, spi1_ss_14_grp, spi1_5_grp,
-                     spi1_ss_15_grp, spi1_ss_16_grp, spi1_ss_17_grp,
-                     sdio0_0_grp, sdio0_1_grp, sdio0_2_grp,
-                     sdio0_3_grp, sdio0_4_grp, sdio0_5_grp,
-                     sdio0_6_grp, sdio0_7_grp, sdio0_8_grp,
-                     sdio0_9_grp, sdio0_10_grp, sdio0_11_grp,
-                     sdio0_12_grp, sdio0_13_grp, sdio0_14_grp,
-                     sdio0_15_grp, sdio0_16_grp, sdio0_17_grp,
-                     sdio0_18_grp, sdio0_19_grp, sdio0_20_grp,
-                     sdio0_21_grp, sdio0_22_grp, sdio0_23_grp,
-                     sdio0_24_grp, sdio0_25_grp, sdio0_26_grp,
-                     sdio0_27_grp, sdio0_28_grp, sdio0_29_grp,
-                     sdio0_30_grp, sdio0_31_grp, sdio0_32_grp,
-                     sdio0_pc_0_grp, sdio0_cd_0_grp, sdio0_wp_0_grp,
-                     sdio0_pc_1_grp, sdio0_cd_1_grp, sdio0_wp_1_grp,
-                     sdio0_pc_2_grp, sdio0_cd_2_grp, sdio0_wp_2_grp,
-                     sdio1_0_grp, sdio1_1_grp, sdio1_2_grp,
-                     sdio1_3_grp, sdio1_4_grp, sdio1_5_grp,
-                     sdio1_6_grp, sdio1_7_grp, sdio1_8_grp,
-                     sdio1_9_grp, sdio1_10_grp, sdio1_11_grp,
-                     sdio1_12_grp, sdio1_13_grp, sdio1_14_grp,
-                     sdio1_15_grp, sdio1_pc_0_grp, sdio1_cd_0_grp,
-                     sdio1_wp_0_grp, sdio1_pc_1_grp, sdio1_cd_1_grp,
-                     sdio1_wp_1_grp, nand0_0_grp, nand0_ce_0_grp,
-                     nand0_rb_0_grp, nand0_dqs_0_grp, nand0_ce_1_grp,
-                     nand0_rb_1_grp, nand0_dqs_1_grp, can0_0_grp,
-                     can0_1_grp, can0_2_grp, can0_3_grp,
-                     can0_4_grp, can0_5_grp, can0_6_grp,
-                     can0_7_grp, can0_8_grp, can0_9_grp,
-                     can0_10_grp, can0_11_grp, can0_12_grp,
-                     can0_13_grp, can0_14_grp, can0_15_grp,
-                     can0_16_grp, can0_17_grp, can0_18_grp,
-                     can1_0_grp, can1_1_grp, can1_2_grp,
-                     can1_3_grp, can1_4_grp, can1_5_grp,
-                     can1_6_grp, can1_7_grp, can1_8_grp,
-                     can1_9_grp, can1_10_grp, can1_11_grp,
-                     can1_12_grp, can1_13_grp, can1_14_grp,
-                     can1_15_grp, can1_16_grp, can1_17_grp,
-                     can1_18_grp, can1_19_grp, uart0_0_grp,
-                     uart0_1_grp, uart0_2_grp, uart0_3_grp,
-                     uart0_4_grp, uart0_5_grp, uart0_6_grp,
-                     uart0_7_grp, uart0_8_grp, uart0_9_grp,
-                     uart0_10_grp, uart0_11_grp, uart0_12_grp,
-                     uart0_13_grp, uart0_14_grp, uart0_15_grp,
-                     uart0_16_grp, uart0_17_grp, uart0_18_grp,
-                     uart1_0_grp, uart1_1_grp, uart1_2_grp,
-                     uart1_3_grp, uart1_4_grp, uart1_5_grp,
-                     uart1_6_grp, uart1_7_grp, uart1_8_grp,
-                     uart1_9_grp, uart1_10_grp, uart1_11_grp,
-                     uart1_12_grp, uart1_13_grp, uart1_14_grp,
-                     uart1_15_grp, uart1_16_grp, uart1_17_grp,
-                     uart1_18_grp, i2c0_0_grp, i2c0_1_grp,
-                     i2c0_2_grp, i2c0_3_grp, i2c0_4_grp,
-                     i2c0_5_grp, i2c0_6_grp, i2c0_7_grp,
-                     i2c0_8_grp, i2c0_9_grp, i2c0_10_grp,
-                     i2c0_11_grp, i2c0_12_grp, i2c0_13_grp,
-                     i2c0_14_grp, i2c0_15_grp, i2c0_16_grp,
-                     i2c0_17_grp, i2c0_18_grp, i2c1_0_grp,
-                     i2c1_1_grp, i2c1_2_grp, i2c1_3_grp,
-                     i2c1_4_grp, i2c1_5_grp, i2c1_6_grp,
-                     i2c1_7_grp, i2c1_8_grp, i2c1_9_grp,
-                     i2c1_10_grp, i2c1_11_grp, i2c1_12_grp,
-                     i2c1_13_grp, i2c1_14_grp, i2c1_15_grp,
-                     i2c1_16_grp, i2c1_17_grp, i2c1_18_grp,
-                     i2c1_19_grp, ttc0_clk_0_grp, ttc0_wav_0_grp,
-                     ttc0_clk_1_grp, ttc0_wav_1_grp, ttc0_clk_2_grp,
-                     ttc0_wav_2_grp, ttc0_clk_3_grp, ttc0_wav_3_grp,
-                     ttc0_clk_4_grp, ttc0_wav_4_grp, ttc0_clk_5_grp,
-                     ttc0_wav_5_grp, ttc0_clk_6_grp, ttc0_wav_6_grp,
-                     ttc0_clk_7_grp, ttc0_wav_7_grp, ttc0_clk_8_grp,
-                     ttc0_wav_8_grp, ttc1_clk_0_grp, ttc1_wav_0_grp,
-                     ttc1_clk_1_grp, ttc1_wav_1_grp, ttc1_clk_2_grp,
-                     ttc1_wav_2_grp, ttc1_clk_3_grp, ttc1_wav_3_grp,
-                     ttc1_clk_4_grp, ttc1_wav_4_grp, ttc1_clk_5_grp,
-                     ttc1_wav_5_grp, ttc1_clk_6_grp, ttc1_wav_6_grp,
-                     ttc1_clk_7_grp, ttc1_wav_7_grp, ttc1_clk_8_grp,
-                     ttc1_wav_8_grp, ttc2_clk_0_grp, ttc2_wav_0_grp,
-                     ttc2_clk_1_grp, ttc2_wav_1_grp, ttc2_clk_2_grp,
-                     ttc2_wav_2_grp, ttc2_clk_3_grp, ttc2_wav_3_grp,
-                     ttc2_clk_4_grp, ttc2_wav_4_grp, ttc2_clk_5_grp,
-                     ttc2_wav_5_grp, ttc2_clk_6_grp, ttc2_wav_6_grp,
-                     ttc2_clk_7_grp, ttc2_wav_7_grp, ttc2_clk_8_grp,
-                     ttc2_wav_8_grp, ttc3_clk_0_grp, ttc3_wav_0_grp,
-                     ttc3_clk_1_grp, ttc3_wav_1_grp, ttc3_clk_2_grp,
-                     ttc3_wav_2_grp, ttc3_clk_3_grp, ttc3_wav_3_grp,
-                     ttc3_clk_4_grp, ttc3_wav_4_grp, ttc3_clk_5_grp,
-                     ttc3_wav_5_grp, ttc3_clk_6_grp, ttc3_wav_6_grp,
-                     ttc3_clk_7_grp, ttc3_wav_7_grp, ttc3_clk_8_grp,
-                     ttc3_wav_8_grp, swdt0_clk_0_grp, swdt0_rst_0_grp,
-                     swdt0_clk_1_grp, swdt0_rst_1_grp, swdt0_clk_2_grp,
-                     swdt0_rst_2_grp, swdt0_clk_3_grp, swdt0_rst_3_grp,
-                     swdt0_clk_4_grp, swdt0_rst_4_grp, swdt0_clk_5_grp,
-                     swdt0_rst_5_grp, swdt0_clk_6_grp, swdt0_rst_6_grp,
-                     swdt0_clk_7_grp, swdt0_rst_7_grp, swdt0_clk_8_grp,
-                     swdt0_rst_8_grp, swdt0_clk_9_grp, swdt0_rst_9_grp,
-                     swdt0_clk_10_grp, swdt0_rst_10_grp, swdt0_clk_11_grp,
-                     swdt0_rst_11_grp, swdt0_clk_12_grp, swdt0_rst_12_grp,
-                     swdt1_clk_0_grp, swdt1_rst_0_grp, swdt1_clk_1_grp,
-                     swdt1_rst_1_grp, swdt1_clk_2_grp, swdt1_rst_2_grp,
-                     swdt1_clk_3_grp, swdt1_rst_3_grp, swdt1_clk_4_grp,
-                     swdt1_rst_4_grp, swdt1_clk_5_grp, swdt1_rst_5_grp,
-                     swdt1_clk_6_grp, swdt1_rst_6_grp, swdt1_clk_7_grp,
-                     swdt1_rst_7_grp, swdt1_clk_8_grp, swdt1_rst_8_grp,
-                     swdt1_clk_9_grp, swdt1_rst_9_grp, swdt1_clk_10_grp,
-                     swdt1_rst_10_grp, swdt1_clk_11_grp, swdt1_rst_11_grp,
-                     swdt1_clk_12_grp, swdt1_rst_12_grp, gpio0_0_grp,
-                     gpio0_1_grp, gpio0_2_grp, gpio0_3_grp,
-                     gpio0_4_grp, gpio0_5_grp, gpio0_6_grp,
-                     gpio0_7_grp, gpio0_8_grp, gpio0_9_grp,
-                     gpio0_10_grp, gpio0_11_grp, gpio0_12_grp,
-                     gpio0_13_grp, gpio0_14_grp, gpio0_15_grp,
-                     gpio0_16_grp, gpio0_17_grp, gpio0_18_grp,
-                     gpio0_19_grp, gpio0_20_grp, gpio0_21_grp,
-                     gpio0_22_grp, gpio0_23_grp, gpio0_24_grp,
-                     gpio0_25_grp, gpio0_26_grp, gpio0_27_grp,
-                     gpio0_28_grp, gpio0_29_grp, gpio0_30_grp,
-                     gpio0_31_grp, gpio0_32_grp, gpio0_33_grp,
-                     gpio0_34_grp, gpio0_35_grp, gpio0_36_grp,
-                     gpio0_37_grp, gpio0_38_grp, gpio0_39_grp,
-                     gpio0_40_grp, gpio0_41_grp, gpio0_42_grp,
-                     gpio0_43_grp, gpio0_44_grp, gpio0_45_grp,
-                     gpio0_46_grp, gpio0_47_grp, gpio0_48_grp,
-                     gpio0_49_grp, gpio0_50_grp, gpio0_51_grp,
-                     gpio0_52_grp, gpio0_53_grp, gpio0_54_grp,
-                     gpio0_55_grp, gpio0_56_grp, gpio0_57_grp,
-                     gpio0_58_grp, gpio0_59_grp, gpio0_60_grp,
-                     gpio0_61_grp, gpio0_62_grp, gpio0_63_grp,
-                     gpio0_64_grp, gpio0_65_grp, gpio0_66_grp,
-                     gpio0_67_grp, gpio0_68_grp, gpio0_69_grp,
-                     gpio0_70_grp, gpio0_71_grp, gpio0_72_grp,
-                     gpio0_73_grp, gpio0_74_grp, gpio0_75_grp,
-                     gpio0_76_grp, gpio0_77_grp, usb0_0_grp,
-                     usb1_0_grp, pmu0_0_grp, pmu0_1_grp,
-                     pmu0_2_grp, pmu0_3_grp, pmu0_4_grp,
-                     pmu0_5_grp, pmu0_6_grp, pmu0_7_grp,
-                     pmu0_8_grp, pmu0_9_grp, pmu0_10_grp,
-                     pmu0_11_grp, pcie0_0_grp, pcie0_1_grp,
-                     pcie0_2_grp, pcie0_3_grp, pcie0_4_grp,
-                     pcie0_5_grp, pcie0_6_grp, pcie0_7_grp,
-                     csu0_0_grp, csu0_1_grp, csu0_2_grp,
-                     csu0_3_grp, csu0_4_grp, csu0_5_grp,
-                     csu0_6_grp, csu0_7_grp, csu0_8_grp,
-                     csu0_9_grp, csu0_10_grp, csu0_11_grp,
-                     dpaux0_0_grp, dpaux0_1_grp, dpaux0_2_grp,
-                     dpaux0_3_grp, pjtag0_0_grp, pjtag0_1_grp,
-                     pjtag0_2_grp, pjtag0_3_grp, pjtag0_4_grp,
-                     pjtag0_5_grp, trace0_0_grp, trace0_clk_0_grp,
-                     trace0_1_grp, trace0_clk_1_grp, trace0_2_grp,
-                     trace0_clk_2_grp, testscan0_0_grp]
+              anyOf:
+                - pattern: '^MIO([0-9]|[1-6][0-9]|7[0-7])$'
+                - enum: [ethernet0_0_grp, ethernet1_0_grp, ethernet2_0_grp,
+                         ethernet3_0_grp, gemtsu0_0_grp, gemtsu0_1_grp,
+                         gemtsu0_2_grp, mdio0_0_grp, mdio1_0_grp,
+                         mdio1_1_grp, mdio2_0_grp, mdio3_0_grp,
+                         qspi0_0_grp, qspi_ss_0_grp, qspi_fbclk_0_grp,
+                         spi0_0_grp, spi0_ss_0_grp, spi0_ss_1_grp,
+                         spi0_ss_2_grp, spi0_1_grp, spi0_ss_3_grp,
+                         spi0_ss_4_grp, spi0_ss_5_grp, spi0_2_grp,
+                         spi0_ss_6_grp, spi0_ss_7_grp, spi0_ss_8_grp,
+                         spi0_3_grp, spi0_ss_9_grp, spi0_ss_10_grp,
+                         spi0_ss_11_grp, spi0_4_grp, spi0_ss_12_grp,
+                         spi0_ss_13_grp, spi0_ss_14_grp, spi0_5_grp,
+                         spi0_ss_15_grp, spi0_ss_16_grp, spi0_ss_17_grp,
+                         spi1_0_grp, spi1_ss_0_grp, spi1_ss_1_grp,
+                         spi1_ss_2_grp, spi1_1_grp, spi1_ss_3_grp,
+                         spi1_ss_4_grp, spi1_ss_5_grp, spi1_2_grp,
+                         spi1_ss_6_grp, spi1_ss_7_grp, spi1_ss_8_grp,
+                         spi1_3_grp, spi1_ss_9_grp, spi1_ss_10_grp,
+                         spi1_ss_11_grp, spi1_4_grp, spi1_ss_12_grp,
+                         spi1_ss_13_grp, spi1_ss_14_grp, spi1_5_grp,
+                         spi1_ss_15_grp, spi1_ss_16_grp, spi1_ss_17_grp,
+                         sdio0_0_grp, sdio0_1_grp, sdio0_2_grp,
+                         sdio0_3_grp, sdio0_4_grp, sdio0_5_grp,
+                         sdio0_6_grp, sdio0_7_grp, sdio0_8_grp,
+                         sdio0_9_grp, sdio0_10_grp, sdio0_11_grp,
+                         sdio0_12_grp, sdio0_13_grp, sdio0_14_grp,
+                         sdio0_15_grp, sdio0_16_grp, sdio0_17_grp,
+                         sdio0_18_grp, sdio0_19_grp, sdio0_20_grp,
+                         sdio0_21_grp, sdio0_22_grp, sdio0_23_grp,
+                         sdio0_24_grp, sdio0_25_grp, sdio0_26_grp,
+                         sdio0_27_grp, sdio0_28_grp, sdio0_29_grp,
+                         sdio0_30_grp, sdio0_31_grp, sdio0_32_grp,
+                         sdio0_pc_0_grp, sdio0_cd_0_grp, sdio0_wp_0_grp,
+                         sdio0_pc_1_grp, sdio0_cd_1_grp, sdio0_wp_1_grp,
+                         sdio0_pc_2_grp, sdio0_cd_2_grp, sdio0_wp_2_grp,
+                         sdio1_0_grp, sdio1_1_grp, sdio1_2_grp,
+                         sdio1_3_grp, sdio1_4_grp, sdio1_5_grp,
+                         sdio1_6_grp, sdio1_7_grp, sdio1_8_grp,
+                         sdio1_9_grp, sdio1_10_grp, sdio1_11_grp,
+                         sdio1_12_grp, sdio1_13_grp, sdio1_14_grp,
+                         sdio1_15_grp, sdio1_pc_0_grp, sdio1_cd_0_grp,
+                         sdio1_wp_0_grp, sdio1_pc_1_grp, sdio1_cd_1_grp,
+                         sdio1_wp_1_grp, nand0_0_grp, nand0_ce_0_grp,
+                         nand0_rb_0_grp, nand0_dqs_0_grp, nand0_ce_1_grp,
+                         nand0_rb_1_grp, nand0_dqs_1_grp, can0_0_grp,
+                         can0_1_grp, can0_2_grp, can0_3_grp,
+                         can0_4_grp, can0_5_grp, can0_6_grp,
+                         can0_7_grp, can0_8_grp, can0_9_grp,
+                         can0_10_grp, can0_11_grp, can0_12_grp,
+                         can0_13_grp, can0_14_grp, can0_15_grp,
+                         can0_16_grp, can0_17_grp, can0_18_grp,
+                         can1_0_grp, can1_1_grp, can1_2_grp,
+                         can1_3_grp, can1_4_grp, can1_5_grp,
+                         can1_6_grp, can1_7_grp, can1_8_grp,
+                         can1_9_grp, can1_10_grp, can1_11_grp,
+                         can1_12_grp, can1_13_grp, can1_14_grp,
+                         can1_15_grp, can1_16_grp, can1_17_grp,
+                         can1_18_grp, can1_19_grp, uart0_0_grp,
+                         uart0_1_grp, uart0_2_grp, uart0_3_grp,
+                         uart0_4_grp, uart0_5_grp, uart0_6_grp,
+                         uart0_7_grp, uart0_8_grp, uart0_9_grp,
+                         uart0_10_grp, uart0_11_grp, uart0_12_grp,
+                         uart0_13_grp, uart0_14_grp, uart0_15_grp,
+                         uart0_16_grp, uart0_17_grp, uart0_18_grp,
+                         uart1_0_grp, uart1_1_grp, uart1_2_grp,
+                         uart1_3_grp, uart1_4_grp, uart1_5_grp,
+                         uart1_6_grp, uart1_7_grp, uart1_8_grp,
+                         uart1_9_grp, uart1_10_grp, uart1_11_grp,
+                         uart1_12_grp, uart1_13_grp, uart1_14_grp,
+                         uart1_15_grp, uart1_16_grp, uart1_17_grp,
+                         uart1_18_grp, i2c0_0_grp, i2c0_1_grp,
+                         i2c0_2_grp, i2c0_3_grp, i2c0_4_grp,
+                         i2c0_5_grp, i2c0_6_grp, i2c0_7_grp,
+                         i2c0_8_grp, i2c0_9_grp, i2c0_10_grp,
+                         i2c0_11_grp, i2c0_12_grp, i2c0_13_grp,
+                         i2c0_14_grp, i2c0_15_grp, i2c0_16_grp,
+                         i2c0_17_grp, i2c0_18_grp, i2c1_0_grp,
+                         i2c1_1_grp, i2c1_2_grp, i2c1_3_grp,
+                         i2c1_4_grp, i2c1_5_grp, i2c1_6_grp,
+                         i2c1_7_grp, i2c1_8_grp, i2c1_9_grp,
+                         i2c1_10_grp, i2c1_11_grp, i2c1_12_grp,
+                         i2c1_13_grp, i2c1_14_grp, i2c1_15_grp,
+                         i2c1_16_grp, i2c1_17_grp, i2c1_18_grp,
+                         i2c1_19_grp, ttc0_clk_0_grp, ttc0_wav_0_grp,
+                         ttc0_clk_1_grp, ttc0_wav_1_grp, ttc0_clk_2_grp,
+                         ttc0_wav_2_grp, ttc0_clk_3_grp, ttc0_wav_3_grp,
+                         ttc0_clk_4_grp, ttc0_wav_4_grp, ttc0_clk_5_grp,
+                         ttc0_wav_5_grp, ttc0_clk_6_grp, ttc0_wav_6_grp,
+                         ttc0_clk_7_grp, ttc0_wav_7_grp, ttc0_clk_8_grp,
+                         ttc0_wav_8_grp, ttc1_clk_0_grp, ttc1_wav_0_grp,
+                         ttc1_clk_1_grp, ttc1_wav_1_grp, ttc1_clk_2_grp,
+                         ttc1_wav_2_grp, ttc1_clk_3_grp, ttc1_wav_3_grp,
+                         ttc1_clk_4_grp, ttc1_wav_4_grp, ttc1_clk_5_grp,
+                         ttc1_wav_5_grp, ttc1_clk_6_grp, ttc1_wav_6_grp,
+                         ttc1_clk_7_grp, ttc1_wav_7_grp, ttc1_clk_8_grp,
+                         ttc1_wav_8_grp, ttc2_clk_0_grp, ttc2_wav_0_grp,
+                         ttc2_clk_1_grp, ttc2_wav_1_grp, ttc2_clk_2_grp,
+                         ttc2_wav_2_grp, ttc2_clk_3_grp, ttc2_wav_3_grp,
+                         ttc2_clk_4_grp, ttc2_wav_4_grp, ttc2_clk_5_grp,
+                         ttc2_wav_5_grp, ttc2_clk_6_grp, ttc2_wav_6_grp,
+                         ttc2_clk_7_grp, ttc2_wav_7_grp, ttc2_clk_8_grp,
+                         ttc2_wav_8_grp, ttc3_clk_0_grp, ttc3_wav_0_grp,
+                         ttc3_clk_1_grp, ttc3_wav_1_grp, ttc3_clk_2_grp,
+                         ttc3_wav_2_grp, ttc3_clk_3_grp, ttc3_wav_3_grp,
+                         ttc3_clk_4_grp, ttc3_wav_4_grp, ttc3_clk_5_grp,
+                         ttc3_wav_5_grp, ttc3_clk_6_grp, ttc3_wav_6_grp,
+                         ttc3_clk_7_grp, ttc3_wav_7_grp, ttc3_clk_8_grp,
+                         ttc3_wav_8_grp, swdt0_clk_0_grp, swdt0_rst_0_grp,
+                         swdt0_clk_1_grp, swdt0_rst_1_grp, swdt0_clk_2_grp,
+                         swdt0_rst_2_grp, swdt0_clk_3_grp, swdt0_rst_3_grp,
+                         swdt0_clk_4_grp, swdt0_rst_4_grp, swdt0_clk_5_grp,
+                         swdt0_rst_5_grp, swdt0_clk_6_grp, swdt0_rst_6_grp,
+                         swdt0_clk_7_grp, swdt0_rst_7_grp, swdt0_clk_8_grp,
+                         swdt0_rst_8_grp, swdt0_clk_9_grp, swdt0_rst_9_grp,
+                         swdt0_clk_10_grp, swdt0_rst_10_grp, swdt0_clk_11_grp,
+                         swdt0_rst_11_grp, swdt0_clk_12_grp, swdt0_rst_12_grp,
+                         swdt1_clk_0_grp, swdt1_rst_0_grp, swdt1_clk_1_grp,
+                         swdt1_rst_1_grp, swdt1_clk_2_grp, swdt1_rst_2_grp,
+                         swdt1_clk_3_grp, swdt1_rst_3_grp, swdt1_clk_4_grp,
+                         swdt1_rst_4_grp, swdt1_clk_5_grp, swdt1_rst_5_grp,
+                         swdt1_clk_6_grp, swdt1_rst_6_grp, swdt1_clk_7_grp,
+                         swdt1_rst_7_grp, swdt1_clk_8_grp, swdt1_rst_8_grp,
+                         swdt1_clk_9_grp, swdt1_rst_9_grp, swdt1_clk_10_grp,
+                         swdt1_rst_10_grp, swdt1_clk_11_grp, swdt1_rst_11_grp,
+                         swdt1_clk_12_grp, swdt1_rst_12_grp, gpio0_0_grp,
+                         gpio0_1_grp, gpio0_2_grp, gpio0_3_grp,
+                         gpio0_4_grp, gpio0_5_grp, gpio0_6_grp,
+                         gpio0_7_grp, gpio0_8_grp, gpio0_9_grp,
+                         gpio0_10_grp, gpio0_11_grp, gpio0_12_grp,
+                         gpio0_13_grp, gpio0_14_grp, gpio0_15_grp,
+                         gpio0_16_grp, gpio0_17_grp, gpio0_18_grp,
+                         gpio0_19_grp, gpio0_20_grp, gpio0_21_grp,
+                         gpio0_22_grp, gpio0_23_grp, gpio0_24_grp,
+                         gpio0_25_grp, gpio0_26_grp, gpio0_27_grp,
+                         gpio0_28_grp, gpio0_29_grp, gpio0_30_grp,
+                         gpio0_31_grp, gpio0_32_grp, gpio0_33_grp,
+                         gpio0_34_grp, gpio0_35_grp, gpio0_36_grp,
+                         gpio0_37_grp, gpio0_38_grp, gpio0_39_grp,
+                         gpio0_40_grp, gpio0_41_grp, gpio0_42_grp,
+                         gpio0_43_grp, gpio0_44_grp, gpio0_45_grp,
+                         gpio0_46_grp, gpio0_47_grp, gpio0_48_grp,
+                         gpio0_49_grp, gpio0_50_grp, gpio0_51_grp,
+                         gpio0_52_grp, gpio0_53_grp, gpio0_54_grp,
+                         gpio0_55_grp, gpio0_56_grp, gpio0_57_grp,
+                         gpio0_58_grp, gpio0_59_grp, gpio0_60_grp,
+                         gpio0_61_grp, gpio0_62_grp, gpio0_63_grp,
+                         gpio0_64_grp, gpio0_65_grp, gpio0_66_grp,
+                         gpio0_67_grp, gpio0_68_grp, gpio0_69_grp,
+                         gpio0_70_grp, gpio0_71_grp, gpio0_72_grp,
+                         gpio0_73_grp, gpio0_74_grp, gpio0_75_grp,
+                         gpio0_76_grp, gpio0_77_grp, usb0_0_grp,
+                         usb1_0_grp, pmu0_0_grp, pmu0_1_grp,
+                         pmu0_2_grp, pmu0_3_grp, pmu0_4_grp,
+                         pmu0_5_grp, pmu0_6_grp, pmu0_7_grp,
+                         pmu0_8_grp, pmu0_9_grp, pmu0_10_grp,
+                         pmu0_11_grp, pcie0_0_grp, pcie0_1_grp,
+                         pcie0_2_grp, pcie0_3_grp, pcie0_4_grp,
+                         pcie0_5_grp, pcie0_6_grp, pcie0_7_grp,
+                         csu0_0_grp, csu0_1_grp, csu0_2_grp,
+                         csu0_3_grp, csu0_4_grp, csu0_5_grp,
+                         csu0_6_grp, csu0_7_grp, csu0_8_grp,
+                         csu0_9_grp, csu0_10_grp, csu0_11_grp,
+                         dpaux0_0_grp, dpaux0_1_grp, dpaux0_2_grp,
+                         dpaux0_3_grp, pjtag0_0_grp, pjtag0_1_grp,
+                         pjtag0_2_grp, pjtag0_3_grp, pjtag0_4_grp,
+                         pjtag0_5_grp, trace0_0_grp, trace0_clk_0_grp,
+                         trace0_1_grp, trace0_clk_1_grp, trace0_2_grp,
+                         trace0_clk_2_grp, testscan0_0_grp]
             maxItems: 78
 
           function:
-- 
2.35.1.1320.gc452695387.dirty

[PATCH 1/2] dt-bindings: pinctrl: xilinx: Add support for function with pins

[Sean Anderson]

Support specifying the function per-pin. The driver doesn't care
whethern you use pins or groups for this purpose.

Signed-off-by: Sean Anderson <sean.anderson@linux.dev>
---

 .../bindings/pinctrl/xlnx,zynqmp-pinctrl.yaml | 344 +++++++++---------
 1 file changed, 176 insertions(+), 168 deletions(-)

diff --git a/Documentation/devicetree/bindings/pinctrl/xlnx,zynqmp-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/xlnx,zynqmp-pinctrl.yaml
index f13d315b5d5e..d3b258245e28 100644
--- a/Documentation/devicetree/bindings/pinctrl/xlnx,zynqmp-pinctrl.yaml
+++ b/Documentation/devicetree/bindings/pinctrl/xlnx,zynqmp-pinctrl.yaml
@@ -42,179 +42,187 @@ patternProperties:
         $ref: pinmux-node.yaml#
 
         properties:
+          pins:
+            description:
+              List of pins to select (either this or "groups" must be specified)
+            items:
+              pattern: '^MIO([0-9]|[1-6][0-9]|7[0-7])$'
+
           groups:
             description:
               List of groups to select (either this or "pins" must be
               specified), available groups for this subnode.
             items:
-              enum: [ethernet0_0_grp, ethernet1_0_grp, ethernet2_0_grp,
-                     ethernet3_0_grp, gemtsu0_0_grp, gemtsu0_1_grp,
-                     gemtsu0_2_grp, mdio0_0_grp, mdio1_0_grp,
-                     mdio1_1_grp, mdio2_0_grp, mdio3_0_grp,
-                     qspi0_0_grp, qspi_ss_0_grp, qspi_fbclk_0_grp,
-                     spi0_0_grp, spi0_ss_0_grp, spi0_ss_1_grp,
-                     spi0_ss_2_grp, spi0_1_grp, spi0_ss_3_grp,
-                     spi0_ss_4_grp, spi0_ss_5_grp, spi0_2_grp,
-                     spi0_ss_6_grp, spi0_ss_7_grp, spi0_ss_8_grp,
-                     spi0_3_grp, spi0_ss_9_grp, spi0_ss_10_grp,
-                     spi0_ss_11_grp, spi0_4_grp, spi0_ss_12_grp,
-                     spi0_ss_13_grp, spi0_ss_14_grp, spi0_5_grp,
-                     spi0_ss_15_grp, spi0_ss_16_grp, spi0_ss_17_grp,
-                     spi1_0_grp, spi1_ss_0_grp, spi1_ss_1_grp,
-                     spi1_ss_2_grp, spi1_1_grp, spi1_ss_3_grp,
-                     spi1_ss_4_grp, spi1_ss_5_grp, spi1_2_grp,
-                     spi1_ss_6_grp, spi1_ss_7_grp, spi1_ss_8_grp,
-                     spi1_3_grp, spi1_ss_9_grp, spi1_ss_10_grp,
-                     spi1_ss_11_grp, spi1_4_grp, spi1_ss_12_grp,
-                     spi1_ss_13_grp, spi1_ss_14_grp, spi1_5_grp,
-                     spi1_ss_15_grp, spi1_ss_16_grp, spi1_ss_17_grp,
-                     sdio0_0_grp, sdio0_1_grp, sdio0_2_grp,
-                     sdio0_3_grp, sdio0_4_grp, sdio0_5_grp,
-                     sdio0_6_grp, sdio0_7_grp, sdio0_8_grp,
-                     sdio0_9_grp, sdio0_10_grp, sdio0_11_grp,
-                     sdio0_12_grp, sdio0_13_grp, sdio0_14_grp,
-                     sdio0_15_grp, sdio0_16_grp, sdio0_17_grp,
-                     sdio0_18_grp, sdio0_19_grp, sdio0_20_grp,
-                     sdio0_21_grp, sdio0_22_grp, sdio0_23_grp,
-                     sdio0_24_grp, sdio0_25_grp, sdio0_26_grp,
-                     sdio0_27_grp, sdio0_28_grp, sdio0_29_grp,
-                     sdio0_30_grp, sdio0_31_grp, sdio0_32_grp,
-                     sdio0_pc_0_grp, sdio0_cd_0_grp, sdio0_wp_0_grp,
-                     sdio0_pc_1_grp, sdio0_cd_1_grp, sdio0_wp_1_grp,
-                     sdio0_pc_2_grp, sdio0_cd_2_grp, sdio0_wp_2_grp,
-                     sdio1_0_grp, sdio1_1_grp, sdio1_2_grp,
-                     sdio1_3_grp, sdio1_4_grp, sdio1_5_grp,
-                     sdio1_6_grp, sdio1_7_grp, sdio1_8_grp,
-                     sdio1_9_grp, sdio1_10_grp, sdio1_11_grp,
-                     sdio1_12_grp, sdio1_13_grp, sdio1_14_grp,
-                     sdio1_15_grp, sdio1_pc_0_grp, sdio1_cd_0_grp,
-                     sdio1_wp_0_grp, sdio1_pc_1_grp, sdio1_cd_1_grp,
-                     sdio1_wp_1_grp, nand0_0_grp, nand0_ce_0_grp,
-                     nand0_rb_0_grp, nand0_dqs_0_grp, nand0_ce_1_grp,
-                     nand0_rb_1_grp, nand0_dqs_1_grp, can0_0_grp,
-                     can0_1_grp, can0_2_grp, can0_3_grp,
-                     can0_4_grp, can0_5_grp, can0_6_grp,
-                     can0_7_grp, can0_8_grp, can0_9_grp,
-                     can0_10_grp, can0_11_grp, can0_12_grp,
-                     can0_13_grp, can0_14_grp, can0_15_grp,
-                     can0_16_grp, can0_17_grp, can0_18_grp,
-                     can1_0_grp, can1_1_grp, can1_2_grp,
-                     can1_3_grp, can1_4_grp, can1_5_grp,
-                     can1_6_grp, can1_7_grp, can1_8_grp,
-                     can1_9_grp, can1_10_grp, can1_11_grp,
-                     can1_12_grp, can1_13_grp, can1_14_grp,
-                     can1_15_grp, can1_16_grp, can1_17_grp,
-                     can1_18_grp, can1_19_grp, uart0_0_grp,
-                     uart0_1_grp, uart0_2_grp, uart0_3_grp,
-                     uart0_4_grp, uart0_5_grp, uart0_6_grp,
-                     uart0_7_grp, uart0_8_grp, uart0_9_grp,
-                     uart0_10_grp, uart0_11_grp, uart0_12_grp,
-                     uart0_13_grp, uart0_14_grp, uart0_15_grp,
-                     uart0_16_grp, uart0_17_grp, uart0_18_grp,
-                     uart1_0_grp, uart1_1_grp, uart1_2_grp,
-                     uart1_3_grp, uart1_4_grp, uart1_5_grp,
-                     uart1_6_grp, uart1_7_grp, uart1_8_grp,
-                     uart1_9_grp, uart1_10_grp, uart1_11_grp,
-                     uart1_12_grp, uart1_13_grp, uart1_14_grp,
-                     uart1_15_grp, uart1_16_grp, uart1_17_grp,
-                     uart1_18_grp, i2c0_0_grp, i2c0_1_grp,
-                     i2c0_2_grp, i2c0_3_grp, i2c0_4_grp,
-                     i2c0_5_grp, i2c0_6_grp, i2c0_7_grp,
-                     i2c0_8_grp, i2c0_9_grp, i2c0_10_grp,
-                     i2c0_11_grp, i2c0_12_grp, i2c0_13_grp,
-                     i2c0_14_grp, i2c0_15_grp, i2c0_16_grp,
-                     i2c0_17_grp, i2c0_18_grp, i2c1_0_grp,
-                     i2c1_1_grp, i2c1_2_grp, i2c1_3_grp,
-                     i2c1_4_grp, i2c1_5_grp, i2c1_6_grp,
-                     i2c1_7_grp, i2c1_8_grp, i2c1_9_grp,
-                     i2c1_10_grp, i2c1_11_grp, i2c1_12_grp,
-                     i2c1_13_grp, i2c1_14_grp, i2c1_15_grp,
-                     i2c1_16_grp, i2c1_17_grp, i2c1_18_grp,
-                     i2c1_19_grp, ttc0_clk_0_grp, ttc0_wav_0_grp,
-                     ttc0_clk_1_grp, ttc0_wav_1_grp, ttc0_clk_2_grp,
-                     ttc0_wav_2_grp, ttc0_clk_3_grp, ttc0_wav_3_grp,
-                     ttc0_clk_4_grp, ttc0_wav_4_grp, ttc0_clk_5_grp,
-                     ttc0_wav_5_grp, ttc0_clk_6_grp, ttc0_wav_6_grp,
-                     ttc0_clk_7_grp, ttc0_wav_7_grp, ttc0_clk_8_grp,
-                     ttc0_wav_8_grp, ttc1_clk_0_grp, ttc1_wav_0_grp,
-                     ttc1_clk_1_grp, ttc1_wav_1_grp, ttc1_clk_2_grp,
-                     ttc1_wav_2_grp, ttc1_clk_3_grp, ttc1_wav_3_grp,
-                     ttc1_clk_4_grp, ttc1_wav_4_grp, ttc1_clk_5_grp,
-                     ttc1_wav_5_grp, ttc1_clk_6_grp, ttc1_wav_6_grp,
-                     ttc1_clk_7_grp, ttc1_wav_7_grp, ttc1_clk_8_grp,
-                     ttc1_wav_8_grp, ttc2_clk_0_grp, ttc2_wav_0_grp,
-                     ttc2_clk_1_grp, ttc2_wav_1_grp, ttc2_clk_2_grp,
-                     ttc2_wav_2_grp, ttc2_clk_3_grp, ttc2_wav_3_grp,
-                     ttc2_clk_4_grp, ttc2_wav_4_grp, ttc2_clk_5_grp,
-                     ttc2_wav_5_grp, ttc2_clk_6_grp, ttc2_wav_6_grp,
-                     ttc2_clk_7_grp, ttc2_wav_7_grp, ttc2_clk_8_grp,
-                     ttc2_wav_8_grp, ttc3_clk_0_grp, ttc3_wav_0_grp,
-                     ttc3_clk_1_grp, ttc3_wav_1_grp, ttc3_clk_2_grp,
-                     ttc3_wav_2_grp, ttc3_clk_3_grp, ttc3_wav_3_grp,
-                     ttc3_clk_4_grp, ttc3_wav_4_grp, ttc3_clk_5_grp,
-                     ttc3_wav_5_grp, ttc3_clk_6_grp, ttc3_wav_6_grp,
-                     ttc3_clk_7_grp, ttc3_wav_7_grp, ttc3_clk_8_grp,
-                     ttc3_wav_8_grp, swdt0_clk_0_grp, swdt0_rst_0_grp,
-                     swdt0_clk_1_grp, swdt0_rst_1_grp, swdt0_clk_2_grp,
-                     swdt0_rst_2_grp, swdt0_clk_3_grp, swdt0_rst_3_grp,
-                     swdt0_clk_4_grp, swdt0_rst_4_grp, swdt0_clk_5_grp,
-                     swdt0_rst_5_grp, swdt0_clk_6_grp, swdt0_rst_6_grp,
-                     swdt0_clk_7_grp, swdt0_rst_7_grp, swdt0_clk_8_grp,
-                     swdt0_rst_8_grp, swdt0_clk_9_grp, swdt0_rst_9_grp,
-                     swdt0_clk_10_grp, swdt0_rst_10_grp, swdt0_clk_11_grp,
-                     swdt0_rst_11_grp, swdt0_clk_12_grp, swdt0_rst_12_grp,
-                     swdt1_clk_0_grp, swdt1_rst_0_grp, swdt1_clk_1_grp,
-                     swdt1_rst_1_grp, swdt1_clk_2_grp, swdt1_rst_2_grp,
-                     swdt1_clk_3_grp, swdt1_rst_3_grp, swdt1_clk_4_grp,
-                     swdt1_rst_4_grp, swdt1_clk_5_grp, swdt1_rst_5_grp,
-                     swdt1_clk_6_grp, swdt1_rst_6_grp, swdt1_clk_7_grp,
-                     swdt1_rst_7_grp, swdt1_clk_8_grp, swdt1_rst_8_grp,
-                     swdt1_clk_9_grp, swdt1_rst_9_grp, swdt1_clk_10_grp,
-                     swdt1_rst_10_grp, swdt1_clk_11_grp, swdt1_rst_11_grp,
-                     swdt1_clk_12_grp, swdt1_rst_12_grp, gpio0_0_grp,
-                     gpio0_1_grp, gpio0_2_grp, gpio0_3_grp,
-                     gpio0_4_grp, gpio0_5_grp, gpio0_6_grp,
-                     gpio0_7_grp, gpio0_8_grp, gpio0_9_grp,
-                     gpio0_10_grp, gpio0_11_grp, gpio0_12_grp,
-                     gpio0_13_grp, gpio0_14_grp, gpio0_15_grp,
-                     gpio0_16_grp, gpio0_17_grp, gpio0_18_grp,
-                     gpio0_19_grp, gpio0_20_grp, gpio0_21_grp,
-                     gpio0_22_grp, gpio0_23_grp, gpio0_24_grp,
-                     gpio0_25_grp, gpio0_26_grp, gpio0_27_grp,
-                     gpio0_28_grp, gpio0_29_grp, gpio0_30_grp,
-                     gpio0_31_grp, gpio0_32_grp, gpio0_33_grp,
-                     gpio0_34_grp, gpio0_35_grp, gpio0_36_grp,
-                     gpio0_37_grp, gpio0_38_grp, gpio0_39_grp,
-                     gpio0_40_grp, gpio0_41_grp, gpio0_42_grp,
-                     gpio0_43_grp, gpio0_44_grp, gpio0_45_grp,
-                     gpio0_46_grp, gpio0_47_grp, gpio0_48_grp,
-                     gpio0_49_grp, gpio0_50_grp, gpio0_51_grp,
-                     gpio0_52_grp, gpio0_53_grp, gpio0_54_grp,
-                     gpio0_55_grp, gpio0_56_grp, gpio0_57_grp,
-                     gpio0_58_grp, gpio0_59_grp, gpio0_60_grp,
-                     gpio0_61_grp, gpio0_62_grp, gpio0_63_grp,
-                     gpio0_64_grp, gpio0_65_grp, gpio0_66_grp,
-                     gpio0_67_grp, gpio0_68_grp, gpio0_69_grp,
-                     gpio0_70_grp, gpio0_71_grp, gpio0_72_grp,
-                     gpio0_73_grp, gpio0_74_grp, gpio0_75_grp,
-                     gpio0_76_grp, gpio0_77_grp, usb0_0_grp,
-                     usb1_0_grp, pmu0_0_grp, pmu0_1_grp,
-                     pmu0_2_grp, pmu0_3_grp, pmu0_4_grp,
-                     pmu0_5_grp, pmu0_6_grp, pmu0_7_grp,
-                     pmu0_8_grp, pmu0_9_grp, pmu0_10_grp,
-                     pmu0_11_grp, pcie0_0_grp, pcie0_1_grp,
-                     pcie0_2_grp, pcie0_3_grp, pcie0_4_grp,
-                     pcie0_5_grp, pcie0_6_grp, pcie0_7_grp,
-                     csu0_0_grp, csu0_1_grp, csu0_2_grp,
-                     csu0_3_grp, csu0_4_grp, csu0_5_grp,
-                     csu0_6_grp, csu0_7_grp, csu0_8_grp,
-                     csu0_9_grp, csu0_10_grp, csu0_11_grp,
-                     dpaux0_0_grp, dpaux0_1_grp, dpaux0_2_grp,
-                     dpaux0_3_grp, pjtag0_0_grp, pjtag0_1_grp,
-                     pjtag0_2_grp, pjtag0_3_grp, pjtag0_4_grp,
-                     pjtag0_5_grp, trace0_0_grp, trace0_clk_0_grp,
-                     trace0_1_grp, trace0_clk_1_grp, trace0_2_grp,
-                     trace0_clk_2_grp, testscan0_0_grp]
+              anyOf:
+                - pattern: '^MIO([0-9]|[1-6][0-9]|7[0-7])$'
+                - enum: [ethernet0_0_grp, ethernet1_0_grp, ethernet2_0_grp,
+                         ethernet3_0_grp, gemtsu0_0_grp, gemtsu0_1_grp,
+                         gemtsu0_2_grp, mdio0_0_grp, mdio1_0_grp,
+                         mdio1_1_grp, mdio2_0_grp, mdio3_0_grp,
+                         qspi0_0_grp, qspi_ss_0_grp, qspi_fbclk_0_grp,
+                         spi0_0_grp, spi0_ss_0_grp, spi0_ss_1_grp,
+                         spi0_ss_2_grp, spi0_1_grp, spi0_ss_3_grp,
+                         spi0_ss_4_grp, spi0_ss_5_grp, spi0_2_grp,
+                         spi0_ss_6_grp, spi0_ss_7_grp, spi0_ss_8_grp,
+                         spi0_3_grp, spi0_ss_9_grp, spi0_ss_10_grp,
+                         spi0_ss_11_grp, spi0_4_grp, spi0_ss_12_grp,
+                         spi0_ss_13_grp, spi0_ss_14_grp, spi0_5_grp,
+                         spi0_ss_15_grp, spi0_ss_16_grp, spi0_ss_17_grp,
+                         spi1_0_grp, spi1_ss_0_grp, spi1_ss_1_grp,
+                         spi1_ss_2_grp, spi1_1_grp, spi1_ss_3_grp,
+                         spi1_ss_4_grp, spi1_ss_5_grp, spi1_2_grp,
+                         spi1_ss_6_grp, spi1_ss_7_grp, spi1_ss_8_grp,
+                         spi1_3_grp, spi1_ss_9_grp, spi1_ss_10_grp,
+                         spi1_ss_11_grp, spi1_4_grp, spi1_ss_12_grp,
+                         spi1_ss_13_grp, spi1_ss_14_grp, spi1_5_grp,
+                         spi1_ss_15_grp, spi1_ss_16_grp, spi1_ss_17_grp,
+                         sdio0_0_grp, sdio0_1_grp, sdio0_2_grp,
+                         sdio0_3_grp, sdio0_4_grp, sdio0_5_grp,
+                         sdio0_6_grp, sdio0_7_grp, sdio0_8_grp,
+                         sdio0_9_grp, sdio0_10_grp, sdio0_11_grp,
+                         sdio0_12_grp, sdio0_13_grp, sdio0_14_grp,
+                         sdio0_15_grp, sdio0_16_grp, sdio0_17_grp,
+                         sdio0_18_grp, sdio0_19_grp, sdio0_20_grp,
+                         sdio0_21_grp, sdio0_22_grp, sdio0_23_grp,
+                         sdio0_24_grp, sdio0_25_grp, sdio0_26_grp,
+                         sdio0_27_grp, sdio0_28_grp, sdio0_29_grp,
+                         sdio0_30_grp, sdio0_31_grp, sdio0_32_grp,
+                         sdio0_pc_0_grp, sdio0_cd_0_grp, sdio0_wp_0_grp,
+                         sdio0_pc_1_grp, sdio0_cd_1_grp, sdio0_wp_1_grp,
+                         sdio0_pc_2_grp, sdio0_cd_2_grp, sdio0_wp_2_grp,
+                         sdio1_0_grp, sdio1_1_grp, sdio1_2_grp,
+                         sdio1_3_grp, sdio1_4_grp, sdio1_5_grp,
+                         sdio1_6_grp, sdio1_7_grp, sdio1_8_grp,
+                         sdio1_9_grp, sdio1_10_grp, sdio1_11_grp,
+                         sdio1_12_grp, sdio1_13_grp, sdio1_14_grp,
+                         sdio1_15_grp, sdio1_pc_0_grp, sdio1_cd_0_grp,
+                         sdio1_wp_0_grp, sdio1_pc_1_grp, sdio1_cd_1_grp,
+                         sdio1_wp_1_grp, nand0_0_grp, nand0_ce_0_grp,
+                         nand0_rb_0_grp, nand0_dqs_0_grp, nand0_ce_1_grp,
+                         nand0_rb_1_grp, nand0_dqs_1_grp, can0_0_grp,
+                         can0_1_grp, can0_2_grp, can0_3_grp,
+                         can0_4_grp, can0_5_grp, can0_6_grp,
+                         can0_7_grp, can0_8_grp, can0_9_grp,
+                         can0_10_grp, can0_11_grp, can0_12_grp,
+                         can0_13_grp, can0_14_grp, can0_15_grp,
+                         can0_16_grp, can0_17_grp, can0_18_grp,
+                         can1_0_grp, can1_1_grp, can1_2_grp,
+                         can1_3_grp, can1_4_grp, can1_5_grp,
+                         can1_6_grp, can1_7_grp, can1_8_grp,
+                         can1_9_grp, can1_10_grp, can1_11_grp,
+                         can1_12_grp, can1_13_grp, can1_14_grp,
+                         can1_15_grp, can1_16_grp, can1_17_grp,
+                         can1_18_grp, can1_19_grp, uart0_0_grp,
+                         uart0_1_grp, uart0_2_grp, uart0_3_grp,
+                         uart0_4_grp, uart0_5_grp, uart0_6_grp,
+                         uart0_7_grp, uart0_8_grp, uart0_9_grp,
+                         uart0_10_grp, uart0_11_grp, uart0_12_grp,
+                         uart0_13_grp, uart0_14_grp, uart0_15_grp,
+                         uart0_16_grp, uart0_17_grp, uart0_18_grp,
+                         uart1_0_grp, uart1_1_grp, uart1_2_grp,
+                         uart1_3_grp, uart1_4_grp, uart1_5_grp,
+                         uart1_6_grp, uart1_7_grp, uart1_8_grp,
+                         uart1_9_grp, uart1_10_grp, uart1_11_grp,
+                         uart1_12_grp, uart1_13_grp, uart1_14_grp,
+                         uart1_15_grp, uart1_16_grp, uart1_17_grp,
+                         uart1_18_grp, i2c0_0_grp, i2c0_1_grp,
+                         i2c0_2_grp, i2c0_3_grp, i2c0_4_grp,
+                         i2c0_5_grp, i2c0_6_grp, i2c0_7_grp,
+                         i2c0_8_grp, i2c0_9_grp, i2c0_10_grp,
+                         i2c0_11_grp, i2c0_12_grp, i2c0_13_grp,
+                         i2c0_14_grp, i2c0_15_grp, i2c0_16_grp,
+                         i2c0_17_grp, i2c0_18_grp, i2c1_0_grp,
+                         i2c1_1_grp, i2c1_2_grp, i2c1_3_grp,
+                         i2c1_4_grp, i2c1_5_grp, i2c1_6_grp,
+                         i2c1_7_grp, i2c1_8_grp, i2c1_9_grp,
+                         i2c1_10_grp, i2c1_11_grp, i2c1_12_grp,
+                         i2c1_13_grp, i2c1_14_grp, i2c1_15_grp,
+                         i2c1_16_grp, i2c1_17_grp, i2c1_18_grp,
+                         i2c1_19_grp, ttc0_clk_0_grp, ttc0_wav_0_grp,
+                         ttc0_clk_1_grp, ttc0_wav_1_grp, ttc0_clk_2_grp,
+                         ttc0_wav_2_grp, ttc0_clk_3_grp, ttc0_wav_3_grp,
+                         ttc0_clk_4_grp, ttc0_wav_4_grp, ttc0_clk_5_grp,
+                         ttc0_wav_5_grp, ttc0_clk_6_grp, ttc0_wav_6_grp,
+                         ttc0_clk_7_grp, ttc0_wav_7_grp, ttc0_clk_8_grp,
+                         ttc0_wav_8_grp, ttc1_clk_0_grp, ttc1_wav_0_grp,
+                         ttc1_clk_1_grp, ttc1_wav_1_grp, ttc1_clk_2_grp,
+                         ttc1_wav_2_grp, ttc1_clk_3_grp, ttc1_wav_3_grp,
+                         ttc1_clk_4_grp, ttc1_wav_4_grp, ttc1_clk_5_grp,
+                         ttc1_wav_5_grp, ttc1_clk_6_grp, ttc1_wav_6_grp,
+                         ttc1_clk_7_grp, ttc1_wav_7_grp, ttc1_clk_8_grp,
+                         ttc1_wav_8_grp, ttc2_clk_0_grp, ttc2_wav_0_grp,
+                         ttc2_clk_1_grp, ttc2_wav_1_grp, ttc2_clk_2_grp,
+                         ttc2_wav_2_grp, ttc2_clk_3_grp, ttc2_wav_3_grp,
+                         ttc2_clk_4_grp, ttc2_wav_4_grp, ttc2_clk_5_grp,
+                         ttc2_wav_5_grp, ttc2_clk_6_grp, ttc2_wav_6_grp,
+                         ttc2_clk_7_grp, ttc2_wav_7_grp, ttc2_clk_8_grp,
+                         ttc2_wav_8_grp, ttc3_clk_0_grp, ttc3_wav_0_grp,
+                         ttc3_clk_1_grp, ttc3_wav_1_grp, ttc3_clk_2_grp,
+                         ttc3_wav_2_grp, ttc3_clk_3_grp, ttc3_wav_3_grp,
+                         ttc3_clk_4_grp, ttc3_wav_4_grp, ttc3_clk_5_grp,
+                         ttc3_wav_5_grp, ttc3_clk_6_grp, ttc3_wav_6_grp,
+                         ttc3_clk_7_grp, ttc3_wav_7_grp, ttc3_clk_8_grp,
+                         ttc3_wav_8_grp, swdt0_clk_0_grp, swdt0_rst_0_grp,
+                         swdt0_clk_1_grp, swdt0_rst_1_grp, swdt0_clk_2_grp,
+                         swdt0_rst_2_grp, swdt0_clk_3_grp, swdt0_rst_3_grp,
+                         swdt0_clk_4_grp, swdt0_rst_4_grp, swdt0_clk_5_grp,
+                         swdt0_rst_5_grp, swdt0_clk_6_grp, swdt0_rst_6_grp,
+                         swdt0_clk_7_grp, swdt0_rst_7_grp, swdt0_clk_8_grp,
+                         swdt0_rst_8_grp, swdt0_clk_9_grp, swdt0_rst_9_grp,
+                         swdt0_clk_10_grp, swdt0_rst_10_grp, swdt0_clk_11_grp,
+                         swdt0_rst_11_grp, swdt0_clk_12_grp, swdt0_rst_12_grp,
+                         swdt1_clk_0_grp, swdt1_rst_0_grp, swdt1_clk_1_grp,
+                         swdt1_rst_1_grp, swdt1_clk_2_grp, swdt1_rst_2_grp,
+                         swdt1_clk_3_grp, swdt1_rst_3_grp, swdt1_clk_4_grp,
+                         swdt1_rst_4_grp, swdt1_clk_5_grp, swdt1_rst_5_grp,
+                         swdt1_clk_6_grp, swdt1_rst_6_grp, swdt1_clk_7_grp,
+                         swdt1_rst_7_grp, swdt1_clk_8_grp, swdt1_rst_8_grp,
+                         swdt1_clk_9_grp, swdt1_rst_9_grp, swdt1_clk_10_grp,
+                         swdt1_rst_10_grp, swdt1_clk_11_grp, swdt1_rst_11_grp,
+                         swdt1_clk_12_grp, swdt1_rst_12_grp, gpio0_0_grp,
+                         gpio0_1_grp, gpio0_2_grp, gpio0_3_grp,
+                         gpio0_4_grp, gpio0_5_grp, gpio0_6_grp,
+                         gpio0_7_grp, gpio0_8_grp, gpio0_9_grp,
+                         gpio0_10_grp, gpio0_11_grp, gpio0_12_grp,
+                         gpio0_13_grp, gpio0_14_grp, gpio0_15_grp,
+                         gpio0_16_grp, gpio0_17_grp, gpio0_18_grp,
+                         gpio0_19_grp, gpio0_20_grp, gpio0_21_grp,
+                         gpio0_22_grp, gpio0_23_grp, gpio0_24_grp,
+                         gpio0_25_grp, gpio0_26_grp, gpio0_27_grp,
+                         gpio0_28_grp, gpio0_29_grp, gpio0_30_grp,
+                         gpio0_31_grp, gpio0_32_grp, gpio0_33_grp,
+                         gpio0_34_grp, gpio0_35_grp, gpio0_36_grp,
+                         gpio0_37_grp, gpio0_38_grp, gpio0_39_grp,
+                         gpio0_40_grp, gpio0_41_grp, gpio0_42_grp,
+                         gpio0_43_grp, gpio0_44_grp, gpio0_45_grp,
+                         gpio0_46_grp, gpio0_47_grp, gpio0_48_grp,
+                         gpio0_49_grp, gpio0_50_grp, gpio0_51_grp,
+                         gpio0_52_grp, gpio0_53_grp, gpio0_54_grp,
+                         gpio0_55_grp, gpio0_56_grp, gpio0_57_grp,
+                         gpio0_58_grp, gpio0_59_grp, gpio0_60_grp,
+                         gpio0_61_grp, gpio0_62_grp, gpio0_63_grp,
+                         gpio0_64_grp, gpio0_65_grp, gpio0_66_grp,
+                         gpio0_67_grp, gpio0_68_grp, gpio0_69_grp,
+                         gpio0_70_grp, gpio0_71_grp, gpio0_72_grp,
+                         gpio0_73_grp, gpio0_74_grp, gpio0_75_grp,
+                         gpio0_76_grp, gpio0_77_grp, usb0_0_grp,
+                         usb1_0_grp, pmu0_0_grp, pmu0_1_grp,
+                         pmu0_2_grp, pmu0_3_grp, pmu0_4_grp,
+                         pmu0_5_grp, pmu0_6_grp, pmu0_7_grp,
+                         pmu0_8_grp, pmu0_9_grp, pmu0_10_grp,
+                         pmu0_11_grp, pcie0_0_grp, pcie0_1_grp,
+                         pcie0_2_grp, pcie0_3_grp, pcie0_4_grp,
+                         pcie0_5_grp, pcie0_6_grp, pcie0_7_grp,
+                         csu0_0_grp, csu0_1_grp, csu0_2_grp,
+                         csu0_3_grp, csu0_4_grp, csu0_5_grp,
+                         csu0_6_grp, csu0_7_grp, csu0_8_grp,
+                         csu0_9_grp, csu0_10_grp, csu0_11_grp,
+                         dpaux0_0_grp, dpaux0_1_grp, dpaux0_2_grp,
+                         dpaux0_3_grp, pjtag0_0_grp, pjtag0_1_grp,
+                         pjtag0_2_grp, pjtag0_3_grp, pjtag0_4_grp,
+                         pjtag0_5_grp, trace0_0_grp, trace0_clk_0_grp,
+                         trace0_1_grp, trace0_clk_1_grp, trace0_2_grp,
+                         trace0_clk_2_grp, testscan0_0_grp]
             maxItems: 78
 
           function:
-- 
2.35.1.1320.gc452695387.dirty


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 2/2] pinctrl: zynqmp: Support muxing individual pins

[Sean Anderson]

While muxing groups of pins at once can be convenient for large
interfaces, it can also be rigid. This is because the group is set to
all pins which support a particular function, even though not all pins
may be used. For example, the sdhci0 function may be used with a 8-bit
eMMC, 4-bit SD card, or even a 1-bit SD card. In these cases, the extra
pins may be repurposed for other uses, but this is not currently
allowed.

Add a new group for each pin which can be muxed. These groups are part
of each function the pin can be muxed to. We treat group selectors
beyond the number of groups as "pin" groups. To set this up, we
initialize groups before functions, and then create a bitmap of used
pins for each function. These used pins are appended to the function's
list of groups.

Signed-off-by: Sean Anderson <sean.anderson@linux.dev>
---

 drivers/pinctrl/pinctrl-zynqmp.c | 61 ++++++++++++++++++++++----------
 1 file changed, 43 insertions(+), 18 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-zynqmp.c b/drivers/pinctrl/pinctrl-zynqmp.c
index 5c46b7d7ebcb..4829150ab069 100644
--- a/drivers/pinctrl/pinctrl-zynqmp.c
+++ b/drivers/pinctrl/pinctrl-zynqmp.c
@@ -10,6 +10,7 @@
 
 #include <dt-bindings/pinctrl/pinctrl-zynqmp.h>
 
+#include <linux/bitmap.h>
 #include <linux/init.h>
 #include <linux/module.h>
 #include <linux/of_address.h>
@@ -97,7 +98,7 @@ static int zynqmp_pctrl_get_groups_count(struct pinctrl_dev *pctldev)
 {
 	struct zynqmp_pinctrl *pctrl = pinctrl_dev_get_drvdata(pctldev);
 
-	return pctrl->ngroups;
+	return pctrl->ngroups + zynqmp_desc.npins;
 }
 
 static const char *zynqmp_pctrl_get_group_name(struct pinctrl_dev *pctldev,
@@ -105,7 +106,10 @@ static const char *zynqmp_pctrl_get_group_name(struct pinctrl_dev *pctldev,
 {
 	struct zynqmp_pinctrl *pctrl = pinctrl_dev_get_drvdata(pctldev);
 
-	return pctrl->groups[selector].name;
+	if (selector < pctrl->ngroups)
+		return pctrl->groups[selector].name;
+
+	return zynqmp_desc.pins[selector - pctrl->ngroups].name;
 }
 
 static int zynqmp_pctrl_get_group_pins(struct pinctrl_dev *pctldev,
@@ -115,8 +119,13 @@ static int zynqmp_pctrl_get_group_pins(struct pinctrl_dev *pctldev,
 {
 	struct zynqmp_pinctrl *pctrl = pinctrl_dev_get_drvdata(pctldev);
 
-	*pins = pctrl->groups[selector].pins;
-	*npins = pctrl->groups[selector].npins;
+	if (selector < pctrl->ngroups) {
+		*pins = pctrl->groups[selector].pins;
+		*npins = pctrl->groups[selector].npins;
+	} else {
+		*pins = &zynqmp_desc.pins[selector - pctrl->ngroups].number;
+		*npins = 1;
+	}
 
 	return 0;
 }
@@ -560,10 +569,12 @@ static int zynqmp_pinctrl_prepare_func_groups(struct device *dev, u32 fid,
 {
 	u16 resp[NUM_GROUPS_PER_RESP] = {0};
 	const char **fgroups;
-	int ret, index, i;
+	int ret, index, i, pin;
+	unsigned int npins;
+	unsigned long *used_pins __free(bitmap) =
+		bitmap_zalloc(zynqmp_desc.npins, GFP_KERNEL);
 
-	fgroups = devm_kcalloc(dev, func->ngroups, sizeof(*fgroups), GFP_KERNEL);
-	if (!fgroups)
+	if (!used_pins)
 		return -ENOMEM;
 
 	for (index = 0; index < func->ngroups; index += NUM_GROUPS_PER_RESP) {
@@ -578,23 +589,37 @@ static int zynqmp_pinctrl_prepare_func_groups(struct device *dev, u32 fid,
 			if (resp[i] == RESERVED_GROUP)
 				continue;
 
-			fgroups[index + i] = devm_kasprintf(dev, GFP_KERNEL,
-							    "%s_%d_grp",
-							    func->name,
-							    index + i);
-			if (!fgroups[index + i])
-				return -ENOMEM;
-
 			groups[resp[i]].name = devm_kasprintf(dev, GFP_KERNEL,
 							      "%s_%d_grp",
 							      func->name,
 							      index + i);
 			if (!groups[resp[i]].name)
 				return -ENOMEM;
+
+			for (pin = 0; pin < groups[resp[i]].npins; pin++)
+				set_bit(groups[resp[i]].pins[pin], used_pins);
 		}
 	}
 done:
+	npins = bitmap_weight(used_pins, zynqmp_desc.npins);
+	fgroups = devm_kcalloc(dev, func->ngroups + npins, sizeof(*fgroups),
+			       GFP_KERNEL);
+	if (!fgroups)
+		return -ENOMEM;
+
+	for (i = 0; i < func->ngroups; i++) {
+		fgroups[i] = devm_kasprintf(dev, GFP_KERNEL, "%s_%d_grp",
+					    func->name, i);
+		if (!fgroups[i])
+			return -ENOMEM;
+	}
+
+	pin = 0;
+	for_each_set_bit(pin, used_pins, zynqmp_desc.npins)
+		fgroups[i++] = zynqmp_desc.pins[pin].name;
+
 	func->groups = fgroups;
+	func->ngroups += npins;
 
 	return 0;
 }
@@ -772,6 +797,10 @@ static int zynqmp_pinctrl_prepare_function_info(struct device *dev,
 	if (!groups)
 		return -ENOMEM;
 
+	ret = zynqmp_pinctrl_prepare_group_pins(dev, groups, pctrl->ngroups);
+	if (ret)
+		return ret;
+
 	for (i = 0; i < pctrl->nfuncs; i++) {
 		ret = zynqmp_pinctrl_prepare_func_groups(dev, i, &funcs[i],
 							 groups);
@@ -779,10 +808,6 @@ static int zynqmp_pinctrl_prepare_function_info(struct device *dev,
 			return ret;
 	}
 
-	ret = zynqmp_pinctrl_prepare_group_pins(dev, groups, pctrl->ngroups);
-	if (ret)
-		return ret;
-
 	pctrl->funcs = funcs;
 	pctrl->groups = groups;
 
-- 
2.35.1.1320.gc452695387.dirty

[PATCH 2/2] pinctrl: zynqmp: Support muxing individual pins

[Sean Anderson]

While muxing groups of pins at once can be convenient for large
interfaces, it can also be rigid. This is because the group is set to
all pins which support a particular function, even though not all pins
may be used. For example, the sdhci0 function may be used with a 8-bit
eMMC, 4-bit SD card, or even a 1-bit SD card. In these cases, the extra
pins may be repurposed for other uses, but this is not currently
allowed.

Add a new group for each pin which can be muxed. These groups are part
of each function the pin can be muxed to. We treat group selectors
beyond the number of groups as "pin" groups. To set this up, we
initialize groups before functions, and then create a bitmap of used
pins for each function. These used pins are appended to the function's
list of groups.

Signed-off-by: Sean Anderson <sean.anderson@linux.dev>
---

 drivers/pinctrl/pinctrl-zynqmp.c | 61 ++++++++++++++++++++++----------
 1 file changed, 43 insertions(+), 18 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-zynqmp.c b/drivers/pinctrl/pinctrl-zynqmp.c
index 5c46b7d7ebcb..4829150ab069 100644
--- a/drivers/pinctrl/pinctrl-zynqmp.c
+++ b/drivers/pinctrl/pinctrl-zynqmp.c
@@ -10,6 +10,7 @@
 
 #include <dt-bindings/pinctrl/pinctrl-zynqmp.h>
 
+#include <linux/bitmap.h>
 #include <linux/init.h>
 #include <linux/module.h>
 #include <linux/of_address.h>
@@ -97,7 +98,7 @@ static int zynqmp_pctrl_get_groups_count(struct pinctrl_dev *pctldev)
 {
 	struct zynqmp_pinctrl *pctrl = pinctrl_dev_get_drvdata(pctldev);
 
-	return pctrl->ngroups;
+	return pctrl->ngroups + zynqmp_desc.npins;
 }
 
 static const char *zynqmp_pctrl_get_group_name(struct pinctrl_dev *pctldev,
@@ -105,7 +106,10 @@ static const char *zynqmp_pctrl_get_group_name(struct pinctrl_dev *pctldev,
 {
 	struct zynqmp_pinctrl *pctrl = pinctrl_dev_get_drvdata(pctldev);
 
-	return pctrl->groups[selector].name;
+	if (selector < pctrl->ngroups)
+		return pctrl->groups[selector].name;
+
+	return zynqmp_desc.pins[selector - pctrl->ngroups].name;
 }
 
 static int zynqmp_pctrl_get_group_pins(struct pinctrl_dev *pctldev,
@@ -115,8 +119,13 @@ static int zynqmp_pctrl_get_group_pins(struct pinctrl_dev *pctldev,
 {
 	struct zynqmp_pinctrl *pctrl = pinctrl_dev_get_drvdata(pctldev);
 
-	*pins = pctrl->groups[selector].pins;
-	*npins = pctrl->groups[selector].npins;
+	if (selector < pctrl->ngroups) {
+		*pins = pctrl->groups[selector].pins;
+		*npins = pctrl->groups[selector].npins;
+	} else {
+		*pins = &zynqmp_desc.pins[selector - pctrl->ngroups].number;
+		*npins = 1;
+	}
 
 	return 0;
 }
@@ -560,10 +569,12 @@ static int zynqmp_pinctrl_prepare_func_groups(struct device *dev, u32 fid,
 {
 	u16 resp[NUM_GROUPS_PER_RESP] = {0};
 	const char **fgroups;
-	int ret, index, i;
+	int ret, index, i, pin;
+	unsigned int npins;
+	unsigned long *used_pins __free(bitmap) =
+		bitmap_zalloc(zynqmp_desc.npins, GFP_KERNEL);
 
-	fgroups = devm_kcalloc(dev, func->ngroups, sizeof(*fgroups), GFP_KERNEL);
-	if (!fgroups)
+	if (!used_pins)
 		return -ENOMEM;
 
 	for (index = 0; index < func->ngroups; index += NUM_GROUPS_PER_RESP) {
@@ -578,23 +589,37 @@ static int zynqmp_pinctrl_prepare_func_groups(struct device *dev, u32 fid,
 			if (resp[i] == RESERVED_GROUP)
 				continue;
 
-			fgroups[index + i] = devm_kasprintf(dev, GFP_KERNEL,
-							    "%s_%d_grp",
-							    func->name,
-							    index + i);
-			if (!fgroups[index + i])
-				return -ENOMEM;
-
 			groups[resp[i]].name = devm_kasprintf(dev, GFP_KERNEL,
 							      "%s_%d_grp",
 							      func->name,
 							      index + i);
 			if (!groups[resp[i]].name)
 				return -ENOMEM;
+
+			for (pin = 0; pin < groups[resp[i]].npins; pin++)
+				set_bit(groups[resp[i]].pins[pin], used_pins);
 		}
 	}
 done:
+	npins = bitmap_weight(used_pins, zynqmp_desc.npins);
+	fgroups = devm_kcalloc(dev, func->ngroups + npins, sizeof(*fgroups),
+			       GFP_KERNEL);
+	if (!fgroups)
+		return -ENOMEM;
+
+	for (i = 0; i < func->ngroups; i++) {
+		fgroups[i] = devm_kasprintf(dev, GFP_KERNEL, "%s_%d_grp",
+					    func->name, i);
+		if (!fgroups[i])
+			return -ENOMEM;
+	}
+
+	pin = 0;
+	for_each_set_bit(pin, used_pins, zynqmp_desc.npins)
+		fgroups[i++] = zynqmp_desc.pins[pin].name;
+
 	func->groups = fgroups;
+	func->ngroups += npins;
 
 	return 0;
 }
@@ -772,6 +797,10 @@ static int zynqmp_pinctrl_prepare_function_info(struct device *dev,
 	if (!groups)
 		return -ENOMEM;
 
+	ret = zynqmp_pinctrl_prepare_group_pins(dev, groups, pctrl->ngroups);
+	if (ret)
+		return ret;
+
 	for (i = 0; i < pctrl->nfuncs; i++) {
 		ret = zynqmp_pinctrl_prepare_func_groups(dev, i, &funcs[i],
 							 groups);
@@ -779,10 +808,6 @@ static int zynqmp_pinctrl_prepare_function_info(struct device *dev,
 			return ret;
 	}
 
-	ret = zynqmp_pinctrl_prepare_group_pins(dev, groups, pctrl->ngroups);
-	if (ret)
-		return ret;
-
 	pctrl->funcs = funcs;
 	pctrl->groups = groups;
 
-- 
2.35.1.1320.gc452695387.dirty


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/2] pinctrl: zynqmp: Support muxing individual pins

[Andy Shevchenko]

Fri, May 03, 2024 at 12:22:17PM -0400, Sean Anderson kirjoitti:
> While muxing groups of pins at once can be convenient for large
> interfaces, it can also be rigid. This is because the group is set to
> all pins which support a particular function, even though not all pins
> may be used. For example, the sdhci0 function may be used with a 8-bit
> eMMC, 4-bit SD card, or even a 1-bit SD card. In these cases, the extra
> pins may be repurposed for other uses, but this is not currently
> allowed.
> 
> Add a new group for each pin which can be muxed. These groups are part
> of each function the pin can be muxed to. We treat group selectors
> beyond the number of groups as "pin" groups. To set this up, we
> initialize groups before functions, and then create a bitmap of used
> pins for each function. These used pins are appended to the function's
> list of groups.

...

> +			for (pin = 0; pin < groups[resp[i]].npins; pin++)
> +				set_bit(groups[resp[i]].pins[pin], used_pins);

Why atomic bit operation?

...

> +	fgroups = devm_kcalloc(dev, func->ngroups + npins, sizeof(*fgroups),

size_add() from overflow.h.

> +			       GFP_KERNEL);
> +	if (!fgroups)
> +		return -ENOMEM;

...

> +	for (i = 0; i < func->ngroups; i++) {
> +		fgroups[i] = devm_kasprintf(dev, GFP_KERNEL, "%s_%d_grp",
> +					    func->name, i);
> +		if (!fgroups[i])
> +			return -ENOMEM;
> +	}

Hmm... Can this benefit from devm_kasprintf_strarray()?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 2/2] pinctrl: zynqmp: Support muxing individual pins

[Andy Shevchenko]

Fri, May 03, 2024 at 12:22:17PM -0400, Sean Anderson kirjoitti:
> While muxing groups of pins at once can be convenient for large
> interfaces, it can also be rigid. This is because the group is set to
> all pins which support a particular function, even though not all pins
> may be used. For example, the sdhci0 function may be used with a 8-bit
> eMMC, 4-bit SD card, or even a 1-bit SD card. In these cases, the extra
> pins may be repurposed for other uses, but this is not currently
> allowed.
> 
> Add a new group for each pin which can be muxed. These groups are part
> of each function the pin can be muxed to. We treat group selectors
> beyond the number of groups as "pin" groups. To set this up, we
> initialize groups before functions, and then create a bitmap of used
> pins for each function. These used pins are appended to the function's
> list of groups.

...

> +			for (pin = 0; pin < groups[resp[i]].npins; pin++)
> +				set_bit(groups[resp[i]].pins[pin], used_pins);

Why atomic bit operation?

...

> +	fgroups = devm_kcalloc(dev, func->ngroups + npins, sizeof(*fgroups),

size_add() from overflow.h.

> +			       GFP_KERNEL);
> +	if (!fgroups)
> +		return -ENOMEM;

...

> +	for (i = 0; i < func->ngroups; i++) {
> +		fgroups[i] = devm_kasprintf(dev, GFP_KERNEL, "%s_%d_grp",
> +					    func->name, i);
> +		if (!fgroups[i])
> +			return -ENOMEM;
> +	}

Hmm... Can this benefit from devm_kasprintf_strarray()?

-- 
With Best Regards,
Andy Shevchenko



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/2] pinctrl: zynqmp: Support muxing individual pins

[Sean Anderson]

On 5/6/24 15:26, Andy Shevchenko wrote:
> Fri, May 03, 2024 at 12:22:17PM -0400, Sean Anderson kirjoitti:
>> While muxing groups of pins at once can be convenient for large
>> interfaces, it can also be rigid. This is because the group is set to
>> all pins which support a particular function, even though not all pins
>> may be used. For example, the sdhci0 function may be used with a 8-bit
>> eMMC, 4-bit SD card, or even a 1-bit SD card. In these cases, the extra
>> pins may be repurposed for other uses, but this is not currently
>> allowed.
>> 
>> Add a new group for each pin which can be muxed. These groups are part
>> of each function the pin can be muxed to. We treat group selectors
>> beyond the number of groups as "pin" groups. To set this up, we
>> initialize groups before functions, and then create a bitmap of used
>> pins for each function. These used pins are appended to the function's
>> list of groups.
> 
> ...
> 
>> +			for (pin = 0; pin < groups[resp[i]].npins; pin++)
>> +				set_bit(groups[resp[i]].pins[pin], used_pins);
> 
> Why atomic bit operation?

The name was easier to remember. I can make it non-atomic.

> ...
> 
>> +	fgroups = devm_kcalloc(dev, func->ngroups + npins, sizeof(*fgroups),
> 
> size_add() from overflow.h.

OK

>> +			       GFP_KERNEL);
>> +	if (!fgroups)
>> +		return -ENOMEM;
> 
> ...
> 
>> +	for (i = 0; i < func->ngroups; i++) {
>> +		fgroups[i] = devm_kasprintf(dev, GFP_KERNEL, "%s_%d_grp",
>> +					    func->name, i);
>> +		if (!fgroups[i])
>> +			return -ENOMEM;
>> +	}
> 
> Hmm... Can this benefit from devm_kasprintf_strarray()?
> 

I don't think so, since the prefix is different for each group.

Thanks for the suggestions.

--Sean

Re: [PATCH 2/2] pinctrl: zynqmp: Support muxing individual pins

[Sean Anderson]

On 5/6/24 15:26, Andy Shevchenko wrote:
> Fri, May 03, 2024 at 12:22:17PM -0400, Sean Anderson kirjoitti:
>> While muxing groups of pins at once can be convenient for large
>> interfaces, it can also be rigid. This is because the group is set to
>> all pins which support a particular function, even though not all pins
>> may be used. For example, the sdhci0 function may be used with a 8-bit
>> eMMC, 4-bit SD card, or even a 1-bit SD card. In these cases, the extra
>> pins may be repurposed for other uses, but this is not currently
>> allowed.
>> 
>> Add a new group for each pin which can be muxed. These groups are part
>> of each function the pin can be muxed to. We treat group selectors
>> beyond the number of groups as "pin" groups. To set this up, we
>> initialize groups before functions, and then create a bitmap of used
>> pins for each function. These used pins are appended to the function's
>> list of groups.
> 
> ...
> 
>> +			for (pin = 0; pin < groups[resp[i]].npins; pin++)
>> +				set_bit(groups[resp[i]].pins[pin], used_pins);
> 
> Why atomic bit operation?

The name was easier to remember. I can make it non-atomic.

> ...
> 
>> +	fgroups = devm_kcalloc(dev, func->ngroups + npins, sizeof(*fgroups),
> 
> size_add() from overflow.h.

OK

>> +			       GFP_KERNEL);
>> +	if (!fgroups)
>> +		return -ENOMEM;
> 
> ...
> 
>> +	for (i = 0; i < func->ngroups; i++) {
>> +		fgroups[i] = devm_kasprintf(dev, GFP_KERNEL, "%s_%d_grp",
>> +					    func->name, i);
>> +		if (!fgroups[i])
>> +			return -ENOMEM;
>> +	}
> 
> Hmm... Can this benefit from devm_kasprintf_strarray()?
> 

I don't think so, since the prefix is different for each group.

Thanks for the suggestions.

--Sean

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/2] pinctrl: zynqmp: Support muxing individual pins

[Sean Anderson]

On 5/6/24 15:38, Sean Anderson wrote:
> On 5/6/24 15:26, Andy Shevchenko wrote:
>> Fri, May 03, 2024 at 12:22:17PM -0400, Sean Anderson kirjoitti:
>>> While muxing groups of pins at once can be convenient for large
>>> interfaces, it can also be rigid. This is because the group is set to
>>> all pins which support a particular function, even though not all pins
>>> may be used. For example, the sdhci0 function may be used with a 8-bit
>>> eMMC, 4-bit SD card, or even a 1-bit SD card. In these cases, the extra
>>> pins may be repurposed for other uses, but this is not currently
>>> allowed.
>>> 
>>> Add a new group for each pin which can be muxed. These groups are part
>>> of each function the pin can be muxed to. We treat group selectors
>>> beyond the number of groups as "pin" groups. To set this up, we
>>> initialize groups before functions, and then create a bitmap of used
>>> pins for each function. These used pins are appended to the function's
>>> list of groups.
>> 
>> ...
>> 
>>> +			for (pin = 0; pin < groups[resp[i]].npins; pin++)
>>> +				set_bit(groups[resp[i]].pins[pin], used_pins);
>> 
>> Why atomic bit operation?
> 
> The name was easier to remember. I can make it non-atomic.
> 
>> ...
>> 
>>> +	fgroups = devm_kcalloc(dev, func->ngroups + npins, sizeof(*fgroups),
>> 
>> size_add() from overflow.h.
> 
> OK
> 
>>> +			       GFP_KERNEL);
>>> +	if (!fgroups)
>>> +		return -ENOMEM;
>> 
>> ...
>> 
>>> +	for (i = 0; i < func->ngroups; i++) {
>>> +		fgroups[i] = devm_kasprintf(dev, GFP_KERNEL, "%s_%d_grp",
>>> +					    func->name, i);
>>> +		if (!fgroups[i])
>>> +			return -ENOMEM;
>>> +	}
>> 
>> Hmm... Can this benefit from devm_kasprintf_strarray()?
>> 
> 
> I don't think so, since the prefix is different for each group.

Sorry, the prefix is the same, but we have to use this format as to not
break the devicetree ABI.

--Sean

Re: [PATCH 2/2] pinctrl: zynqmp: Support muxing individual pins

[Sean Anderson]

On 5/6/24 15:38, Sean Anderson wrote:
> On 5/6/24 15:26, Andy Shevchenko wrote:
>> Fri, May 03, 2024 at 12:22:17PM -0400, Sean Anderson kirjoitti:
>>> While muxing groups of pins at once can be convenient for large
>>> interfaces, it can also be rigid. This is because the group is set to
>>> all pins which support a particular function, even though not all pins
>>> may be used. For example, the sdhci0 function may be used with a 8-bit
>>> eMMC, 4-bit SD card, or even a 1-bit SD card. In these cases, the extra
>>> pins may be repurposed for other uses, but this is not currently
>>> allowed.
>>> 
>>> Add a new group for each pin which can be muxed. These groups are part
>>> of each function the pin can be muxed to. We treat group selectors
>>> beyond the number of groups as "pin" groups. To set this up, we
>>> initialize groups before functions, and then create a bitmap of used
>>> pins for each function. These used pins are appended to the function's
>>> list of groups.
>> 
>> ...
>> 
>>> +			for (pin = 0; pin < groups[resp[i]].npins; pin++)
>>> +				set_bit(groups[resp[i]].pins[pin], used_pins);
>> 
>> Why atomic bit operation?
> 
> The name was easier to remember. I can make it non-atomic.
> 
>> ...
>> 
>>> +	fgroups = devm_kcalloc(dev, func->ngroups + npins, sizeof(*fgroups),
>> 
>> size_add() from overflow.h.
> 
> OK
> 
>>> +			       GFP_KERNEL);
>>> +	if (!fgroups)
>>> +		return -ENOMEM;
>> 
>> ...
>> 
>>> +	for (i = 0; i < func->ngroups; i++) {
>>> +		fgroups[i] = devm_kasprintf(dev, GFP_KERNEL, "%s_%d_grp",
>>> +					    func->name, i);
>>> +		if (!fgroups[i])
>>> +			return -ENOMEM;
>>> +	}
>> 
>> Hmm... Can this benefit from devm_kasprintf_strarray()?
>> 
> 
> I don't think so, since the prefix is different for each group.

Sorry, the prefix is the same, but we have to use this format as to not
break the devicetree ABI.

--Sean


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 0/2] pinctrl: zynqmp: Support muxing individual pins

[Linus Walleij]

On Fri, May 3, 2024 at 6:22 PM Sean Anderson <sean.anderson@linux.dev> wrote:

> This series adds support for muxing individual pins, instead of
> requiring groups to be muxed together. See [1] for additional
> discussion.
>
> [1] https://lore.kernel.org/linux-arm-kernel/5bb0dc7e-4c89-4f3d-abc6-41ae9ded5ae9@linux.dev/

The way I usually would recommend to solve this would be to
define new subgroups, so e.g. for a UARTS:

uart0_grp = pin_rx, pin_tx, pin_cts, pin_dts, pin_dcd;

And today this would be used like that:

mux0:
    function = "uart0";
    groups = "uart0_grp";

Then we realize that not everyone need all the modem
control signals provided. What to do. Well this:

uart0_rxtx_grp = pin_rx, pin_tx:
uart0_modem_grp = pin_cts, pin_dts, pin_dcd;

mux0:
    function = "uart0";
    groups = "uart0_rxtx_grp";

Now the CTS, DTS, DCD pins can be reused for something
else such as GPIO.

I *know* that this breaks ABI: the driver group definitions change
and the device tree needs to be changed too.

This only matters if the users have a habit of distributing the
kernel and DTB separately so a new kernel needs to support
and old DTB. This varies in how much control we have but I
think for Xilinx systems the kernel and DTB are always updated
in lockstep, so it really does not matter?

Yours,
Linus Walleij

Re: [PATCH 0/2] pinctrl: zynqmp: Support muxing individual pins

[Linus Walleij]

On Fri, May 3, 2024 at 6:22 PM Sean Anderson <sean.anderson@linux.dev> wrote:

> This series adds support for muxing individual pins, instead of
> requiring groups to be muxed together. See [1] for additional
> discussion.
>
> [1] https://lore.kernel.org/linux-arm-kernel/5bb0dc7e-4c89-4f3d-abc6-41ae9ded5ae9@linux.dev/

The way I usually would recommend to solve this would be to
define new subgroups, so e.g. for a UARTS:

uart0_grp = pin_rx, pin_tx, pin_cts, pin_dts, pin_dcd;

And today this would be used like that:

mux0:
    function = "uart0";
    groups = "uart0_grp";

Then we realize that not everyone need all the modem
control signals provided. What to do. Well this:

uart0_rxtx_grp = pin_rx, pin_tx:
uart0_modem_grp = pin_cts, pin_dts, pin_dcd;

mux0:
    function = "uart0";
    groups = "uart0_rxtx_grp";

Now the CTS, DTS, DCD pins can be reused for something
else such as GPIO.

I *know* that this breaks ABI: the driver group definitions change
and the device tree needs to be changed too.

This only matters if the users have a habit of distributing the
kernel and DTB separately so a new kernel needs to support
and old DTB. This varies in how much control we have but I
think for Xilinx systems the kernel and DTB are always updated
in lockstep, so it really does not matter?

Yours,
Linus Walleij

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 0/2] pinctrl: zynqmp: Support muxing individual pins

[Sean Anderson]

On 5/6/24 02:43, Linus Walleij wrote:
> On Fri, May 3, 2024 at 6:22 PM Sean Anderson <sean.anderson@linux.dev> wrote:
> 
>> This series adds support for muxing individual pins, instead of
>> requiring groups to be muxed together. See [1] for additional
>> discussion.
>>
>> [1] https://lore.kernel.org/linux-arm-kernel/5bb0dc7e-4c89-4f3d-abc6-41ae9ded5ae9@linux.dev/
> 
> The way I usually would recommend to solve this would be to
> define new subgroups, so e.g. for a UARTS:
> 
> uart0_grp = pin_rx, pin_tx, pin_cts, pin_dts, pin_dcd;
> 
> And today this would be used like that:
> 
> mux0:
>     function = "uart0";
>     groups = "uart0_grp";
> 
> Then we realize that not everyone need all the modem
> control signals provided. What to do. Well this:
> 
> uart0_rxtx_grp = pin_rx, pin_tx:
> uart0_modem_grp = pin_cts, pin_dts, pin_dcd;
> 
> mux0:
>     function = "uart0";
>     groups = "uart0_rxtx_grp";
> 
> Now the CTS, DTS, DCD pins can be reused for something
> else such as GPIO.
> 
> I *know* that this breaks ABI: the driver group definitions change
> and the device tree needs to be changed too.
> 
> This only matters if the users have a habit of distributing the
> kernel and DTB separately so a new kernel needs to support
> and old DTB. This varies in how much control we have but I
> think for Xilinx systems the kernel and DTB are always updated
> in lockstep, so it really does not matter?

Well, the pin groups are actually defined in the PMU firmware. And
frankly, I don't see the point of pin "groups" when there are not actual
pin groups at the hardware level. The pins can all be muxed
individually, so there's no point in adding artificial groups on top.
Just mux the pins like the hardware allows and everything is easy. Cuts
down on the absurd number of strings too.

--Sean

Re: [PATCH 0/2] pinctrl: zynqmp: Support muxing individual pins

[Sean Anderson]

On 5/6/24 02:43, Linus Walleij wrote:
> On Fri, May 3, 2024 at 6:22 PM Sean Anderson <sean.anderson@linux.dev> wrote:
> 
>> This series adds support for muxing individual pins, instead of
>> requiring groups to be muxed together. See [1] for additional
>> discussion.
>>
>> [1] https://lore.kernel.org/linux-arm-kernel/5bb0dc7e-4c89-4f3d-abc6-41ae9ded5ae9@linux.dev/
> 
> The way I usually would recommend to solve this would be to
> define new subgroups, so e.g. for a UARTS:
> 
> uart0_grp = pin_rx, pin_tx, pin_cts, pin_dts, pin_dcd;
> 
> And today this would be used like that:
> 
> mux0:
>     function = "uart0";
>     groups = "uart0_grp";
> 
> Then we realize that not everyone need all the modem
> control signals provided. What to do. Well this:
> 
> uart0_rxtx_grp = pin_rx, pin_tx:
> uart0_modem_grp = pin_cts, pin_dts, pin_dcd;
> 
> mux0:
>     function = "uart0";
>     groups = "uart0_rxtx_grp";
> 
> Now the CTS, DTS, DCD pins can be reused for something
> else such as GPIO.
> 
> I *know* that this breaks ABI: the driver group definitions change
> and the device tree needs to be changed too.
> 
> This only matters if the users have a habit of distributing the
> kernel and DTB separately so a new kernel needs to support
> and old DTB. This varies in how much control we have but I
> think for Xilinx systems the kernel and DTB are always updated
> in lockstep, so it really does not matter?

Well, the pin groups are actually defined in the PMU firmware. And
frankly, I don't see the point of pin "groups" when there are not actual
pin groups at the hardware level. The pins can all be muxed
individually, so there's no point in adding artificial groups on top.
Just mux the pins like the hardware allows and everything is easy. Cuts
down on the absurd number of strings too.

--Sean

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 0/2] pinctrl: zynqmp: Support muxing individual pins

[Michal Simek]

On 5/6/24 16:45, Sean Anderson wrote:
> On 5/6/24 02:43, Linus Walleij wrote:
>> On Fri, May 3, 2024 at 6:22 PM Sean Anderson <sean.anderson@linux.dev> wrote:
>>
>>> This series adds support for muxing individual pins, instead of
>>> requiring groups to be muxed together. See [1] for additional
>>> discussion.
>>>
>>> [1] https://lore.kernel.org/linux-arm-kernel/5bb0dc7e-4c89-4f3d-abc6-41ae9ded5ae9@linux.dev/
>>
>> The way I usually would recommend to solve this would be to
>> define new subgroups, so e.g. for a UARTS:
>>
>> uart0_grp = pin_rx, pin_tx, pin_cts, pin_dts, pin_dcd;
>>
>> And today this would be used like that:
>>
>> mux0:
>>      function = "uart0";
>>      groups = "uart0_grp";
>>
>> Then we realize that not everyone need all the modem
>> control signals provided. What to do. Well this:
>>
>> uart0_rxtx_grp = pin_rx, pin_tx:
>> uart0_modem_grp = pin_cts, pin_dts, pin_dcd;
>>
>> mux0:
>>      function = "uart0";
>>      groups = "uart0_rxtx_grp";
>>
>> Now the CTS, DTS, DCD pins can be reused for something
>> else such as GPIO.
>>
>> I *know* that this breaks ABI: the driver group definitions change
>> and the device tree needs to be changed too.
>>
>> This only matters if the users have a habit of distributing the
>> kernel and DTB separately so a new kernel needs to support
>> and old DTB. This varies in how much control we have but I
>> think for Xilinx systems the kernel and DTB are always updated
>> in lockstep, so it really does not matter?
> 
> Well, the pin groups are actually defined in the PMU firmware. And
> frankly, I don't see the point of pin "groups" when there are not actual
> pin groups at the hardware level. The pins can all be muxed
> individually, so there's no point in adding artificial groups on top.
> Just mux the pins like the hardware allows and everything is easy. Cuts
> down on the absurd number of strings too.

That Linus example is split which would make sense but as Sean said HW is not 
really working like this. Because you can actually take tx from group0 and rx 
from group5. You can't configure it in design tools but you can configure via 
registers and it will just work fine.

Thanks,
Michal

Re: [PATCH 0/2] pinctrl: zynqmp: Support muxing individual pins

[Michal Simek]

On 5/6/24 16:45, Sean Anderson wrote:
> On 5/6/24 02:43, Linus Walleij wrote:
>> On Fri, May 3, 2024 at 6:22 PM Sean Anderson <sean.anderson@linux.dev> wrote:
>>
>>> This series adds support for muxing individual pins, instead of
>>> requiring groups to be muxed together. See [1] for additional
>>> discussion.
>>>
>>> [1] https://lore.kernel.org/linux-arm-kernel/5bb0dc7e-4c89-4f3d-abc6-41ae9ded5ae9@linux.dev/
>>
>> The way I usually would recommend to solve this would be to
>> define new subgroups, so e.g. for a UARTS:
>>
>> uart0_grp = pin_rx, pin_tx, pin_cts, pin_dts, pin_dcd;
>>
>> And today this would be used like that:
>>
>> mux0:
>>      function = "uart0";
>>      groups = "uart0_grp";
>>
>> Then we realize that not everyone need all the modem
>> control signals provided. What to do. Well this:
>>
>> uart0_rxtx_grp = pin_rx, pin_tx:
>> uart0_modem_grp = pin_cts, pin_dts, pin_dcd;
>>
>> mux0:
>>      function = "uart0";
>>      groups = "uart0_rxtx_grp";
>>
>> Now the CTS, DTS, DCD pins can be reused for something
>> else such as GPIO.
>>
>> I *know* that this breaks ABI: the driver group definitions change
>> and the device tree needs to be changed too.
>>
>> This only matters if the users have a habit of distributing the
>> kernel and DTB separately so a new kernel needs to support
>> and old DTB. This varies in how much control we have but I
>> think for Xilinx systems the kernel and DTB are always updated
>> in lockstep, so it really does not matter?
> 
> Well, the pin groups are actually defined in the PMU firmware. And
> frankly, I don't see the point of pin "groups" when there are not actual
> pin groups at the hardware level. The pins can all be muxed
> individually, so there's no point in adding artificial groups on top.
> Just mux the pins like the hardware allows and everything is easy. Cuts
> down on the absurd number of strings too.

That Linus example is split which would make sense but as Sean said HW is not 
really working like this. Because you can actually take tx from group0 and rx 
from group5. You can't configure it in design tools but you can configure via 
registers and it will just work fine.

Thanks,
Michal


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 0/2] pinctrl: zynqmp: Support muxing individual pins

[Linus Walleij]

On Mon, May 6, 2024 at 4:45 PM Sean Anderson <sean.anderson@linux.dev> wrote:

> > Then we realize that not everyone need all the modem
> > control signals provided. What to do. Well this:
> >
> > uart0_rxtx_grp = pin_rx, pin_tx:
> > uart0_modem_grp = pin_cts, pin_dts, pin_dcd;
> >
> > mux0:
> >     function = "uart0";
> >     groups = "uart0_rxtx_grp";
> >
> > Now the CTS, DTS, DCD pins can be reused for something
> > else such as GPIO.
> >
> > I *know* that this breaks ABI: the driver group definitions change
> > and the device tree needs to be changed too.

Actually I didn't think that over, it is possible to add new groups
and retain the old ones.

I.e. retain uart0_grp, but additionally add and use
uart0_rxtx and uart0_modem_grp and use one or the
other approach.

> Well, the pin groups are actually defined in the PMU firmware.

Is that firmware written in such an helpful way that the groups
can be extracted from the firmware then, as with SCMI? Or is it
a matter of duplicating the info from the PMU in the software-defined
groups.

> And
> frankly, I don't see the point of pin "groups" when there are not actual
> pin groups at the hardware level. The pins can all be muxed
> individually, so there's no point in adding artificial groups on top.
> Just mux the pins like the hardware allows and everything is easy. Cuts
> down on the absurd number of strings too.

So are you going to switch all of Xilinx devicetrees over to using exclusively
the new method (muxing individual pins)?

I'm fine with one (string identified groups) which I encourage, but I
let individual pin control pass as well on several occasions.

What I don't want to see is a Franken-solution that mixes the two
approaches, even less so on the same system. Someone is going to
have to maintain the resulting mess. And this looks like exactly that.

If you want to mux individual pins instead of groups and functions, by
all means, but please do not mix the two approaches in the same
driver, I'm just trying to save Xilinx from themselves here.

Yours,
Linus Walleij

Re: [PATCH 0/2] pinctrl: zynqmp: Support muxing individual pins

[Linus Walleij]

On Mon, May 6, 2024 at 4:45 PM Sean Anderson <sean.anderson@linux.dev> wrote:

> > Then we realize that not everyone need all the modem
> > control signals provided. What to do. Well this:
> >
> > uart0_rxtx_grp = pin_rx, pin_tx:
> > uart0_modem_grp = pin_cts, pin_dts, pin_dcd;
> >
> > mux0:
> >     function = "uart0";
> >     groups = "uart0_rxtx_grp";
> >
> > Now the CTS, DTS, DCD pins can be reused for something
> > else such as GPIO.
> >
> > I *know* that this breaks ABI: the driver group definitions change
> > and the device tree needs to be changed too.

Actually I didn't think that over, it is possible to add new groups
and retain the old ones.

I.e. retain uart0_grp, but additionally add and use
uart0_rxtx and uart0_modem_grp and use one or the
other approach.

> Well, the pin groups are actually defined in the PMU firmware.

Is that firmware written in such an helpful way that the groups
can be extracted from the firmware then, as with SCMI? Or is it
a matter of duplicating the info from the PMU in the software-defined
groups.

> And
> frankly, I don't see the point of pin "groups" when there are not actual
> pin groups at the hardware level. The pins can all be muxed
> individually, so there's no point in adding artificial groups on top.
> Just mux the pins like the hardware allows and everything is easy. Cuts
> down on the absurd number of strings too.

So are you going to switch all of Xilinx devicetrees over to using exclusively
the new method (muxing individual pins)?

I'm fine with one (string identified groups) which I encourage, but I
let individual pin control pass as well on several occasions.

What I don't want to see is a Franken-solution that mixes the two
approaches, even less so on the same system. Someone is going to
have to maintain the resulting mess. And this looks like exactly that.

If you want to mux individual pins instead of groups and functions, by
all means, but please do not mix the two approaches in the same
driver, I'm just trying to save Xilinx from themselves here.

Yours,
Linus Walleij

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 0/2] pinctrl: zynqmp: Support muxing individual pins

[Sean Anderson]

On 5/27/24 09:15, Linus Walleij wrote:
> On Mon, May 6, 2024 at 4:45 PM Sean Anderson <sean.anderson@linux.dev> wrote:
> 
>> > Then we realize that not everyone need all the modem
>> > control signals provided. What to do. Well this:
>> >
>> > uart0_rxtx_grp = pin_rx, pin_tx:
>> > uart0_modem_grp = pin_cts, pin_dts, pin_dcd;
>> >
>> > mux0:
>> >     function = "uart0";
>> >     groups = "uart0_rxtx_grp";
>> >
>> > Now the CTS, DTS, DCD pins can be reused for something
>> > else such as GPIO.
>> >
>> > I *know* that this breaks ABI: the driver group definitions change
>> > and the device tree needs to be changed too.
> 
> Actually I didn't think that over, it is possible to add new groups
> and retain the old ones.
> 
> I.e. retain uart0_grp, but additionally add and use
> uart0_rxtx and uart0_modem_grp and use one or the
> other approach.

That is what this patch does.

>> Well, the pin groups are actually defined in the PMU firmware.
> 
> Is that firmware written in such an helpful way that the groups
> can be extracted from the firmware then, as with SCMI? Or is it
> a matter of duplicating the info from the PMU in the software-defined
> groups.

Fundamentally, the pin muxings are known a priori from the reference
manual. Because pinmuxing itself has been delegated to the PMU firmware,
we defer to it when determining what muxings are available. The PMU
firmware describes muxings in terms of pins and functions; groups are a
Linux-only concept.

>> And
>> frankly, I don't see the point of pin "groups" when there are not actual
>> pin groups at the hardware level. The pins can all be muxed
>> individually, so there's no point in adding artificial groups on top.
>> Just mux the pins like the hardware allows and everything is easy. Cuts
>> down on the absurd number of strings too.
> 
> So are you going to switch all of Xilinx devicetrees over to using exclusively
> the new method (muxing individual pins)?

No. We have to support it anyway for compatibility, so there is no point
in changing everything for no reason.

> I'm fine with one (string identified groups) which I encourage, but I
> let individual pin control pass as well on several occasions.
> 
> What I don't want to see is a Franken-solution that mixes the two
> approaches, even less so on the same system. Someone is going to
> have to maintain the resulting mess. And this looks like exactly that.

Well, perhaps you should have reviewed the original driver more
closely.

> If you want to mux individual pins instead of groups and functions, by
> all means, but please do not mix the two approaches in the same
> driver, I'm just trying to save Xilinx from themselves here.

I see no point in creating thousands of groups for every combination of
pin muxings when we could just switch to the solution in this (or v2 of)
patch. For compatibility we cannot be rid of the old situation, but we
can at least fix it. There is no technical problem with them coexisting.

--Sean

Re: [PATCH 0/2] pinctrl: zynqmp: Support muxing individual pins

[Sean Anderson]

On 5/27/24 09:15, Linus Walleij wrote:
> On Mon, May 6, 2024 at 4:45 PM Sean Anderson <sean.anderson@linux.dev> wrote:
> 
>> > Then we realize that not everyone need all the modem
>> > control signals provided. What to do. Well this:
>> >
>> > uart0_rxtx_grp = pin_rx, pin_tx:
>> > uart0_modem_grp = pin_cts, pin_dts, pin_dcd;
>> >
>> > mux0:
>> >     function = "uart0";
>> >     groups = "uart0_rxtx_grp";
>> >
>> > Now the CTS, DTS, DCD pins can be reused for something
>> > else such as GPIO.
>> >
>> > I *know* that this breaks ABI: the driver group definitions change
>> > and the device tree needs to be changed too.
> 
> Actually I didn't think that over, it is possible to add new groups
> and retain the old ones.
> 
> I.e. retain uart0_grp, but additionally add and use
> uart0_rxtx and uart0_modem_grp and use one or the
> other approach.

That is what this patch does.

>> Well, the pin groups are actually defined in the PMU firmware.
> 
> Is that firmware written in such an helpful way that the groups
> can be extracted from the firmware then, as with SCMI? Or is it
> a matter of duplicating the info from the PMU in the software-defined
> groups.

Fundamentally, the pin muxings are known a priori from the reference
manual. Because pinmuxing itself has been delegated to the PMU firmware,
we defer to it when determining what muxings are available. The PMU
firmware describes muxings in terms of pins and functions; groups are a
Linux-only concept.

>> And
>> frankly, I don't see the point of pin "groups" when there are not actual
>> pin groups at the hardware level. The pins can all be muxed
>> individually, so there's no point in adding artificial groups on top.
>> Just mux the pins like the hardware allows and everything is easy. Cuts
>> down on the absurd number of strings too.
> 
> So are you going to switch all of Xilinx devicetrees over to using exclusively
> the new method (muxing individual pins)?

No. We have to support it anyway for compatibility, so there is no point
in changing everything for no reason.

> I'm fine with one (string identified groups) which I encourage, but I
> let individual pin control pass as well on several occasions.
> 
> What I don't want to see is a Franken-solution that mixes the two
> approaches, even less so on the same system. Someone is going to
> have to maintain the resulting mess. And this looks like exactly that.

Well, perhaps you should have reviewed the original driver more
closely.

> If you want to mux individual pins instead of groups and functions, by
> all means, but please do not mix the two approaches in the same
> driver, I'm just trying to save Xilinx from themselves here.

I see no point in creating thousands of groups for every combination of
pin muxings when we could just switch to the solution in this (or v2 of)
patch. For compatibility we cannot be rid of the old situation, but we
can at least fix it. There is no technical problem with them coexisting.

--Sean

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 0/2] pinctrl: zynqmp: Support muxing individual pins

[Linus Walleij]

On Tue, May 28, 2024 at 4:28 PM Sean Anderson <sean.anderson@linux.dev> wrote:

> Well, perhaps you should have reviewed the original driver more
> closely.

Do you want to push me down and increase my work related
stress? Because that is the effect of such statements.

It looks like criticism of me as a person, so explain yourself.

Writing this kind of things looks to me like some kind of abusive way
to express your desire and that is what burns maintainers out, so
if that is what you are doing, stop doing that, adjust your behaviour
and focus on technical issues.

> > If you want to mux individual pins instead of groups and functions, by
> > all means, but please do not mix the two approaches in the same
> > driver, I'm just trying to save Xilinx from themselves here.
>
> I see no point in creating thousands of groups

Please share your calculations for figures like "thousands".

In my experience, groups are usually in the tens, perhaps
hundreds, physically restricted by the number of pins
underneath a BGA. A Micro-FCBGA has 479 balls and many
are GND and power, so that sets a ballpark figure.

> for every combination of pin musings

It is clear from the documentation that the point if the pinmux
groups and pins are not to present all possible options (known as
a "phone exchange" solution) but those that are used in practice,
i.e. these representing real world use cases. See below.

> when we could just switch to the solution in this (or v2 of)
> patch. For compatibility we cannot be rid of the old situation, but we
> can at least fix it. There is no technical problem with them coexisting.

Historically there are  ~2 camps:

- One camp want to use groups and
functions to combine pins in groups with functions to form usecases.

In some cases (such as pinctrl-gemini.c or the very latest
pinctrl-scmi.c merged for v6.10) this reflects how the hardware
actually looks: it does not make individual pins available for muxing,
but you poke bits or send messages to change entire
groups-to-function mappings, so it is necessary for some hardware.

So when you write that "groups are a Linux-only concept" this
is because you probably haven't seen this part of the world.
Groups exist in hardware, and in the SCMI specification.

There are systems with individual control of the muxing
of every pin, such that e.g. every pin has a muxing register.

These are again not really phone exchanges: I am yet to see
a system where any function can be mapped to any pin. These
just do not exist. What exists in practice is that each pin can be
mapped to 2-4 functions, in extreme cases some more. Often
these functions are mapped to adjacent pins, and the "chessboard"
picture in the documentation for the subsystem reflects this.

For this reason, it is often helpful for driver writers to group
adjacent pins into groups, so an iterator can walk over the
pins and poke their registers in order, instead of treating each
pin as a unique entity.

- Then there is the camp that just by habit *want* to control
each pin individually. The extreme example is pinctrl-single.c
which is named like such because each pin is controlled by
a single register. TI wanted this solution mainly because their
hardware wasn't described in manuals, but in other HW
description files, and they needed to process large volumes
of data into DT-form.

I didn't like this solution initially because it makes it hard for
people without datasheets to understand what is going on.
But I was convinced to let this coexist with the group and function
mapping, which is fine: maybe one size doesn't fit all.

i.MX and others also do this approach but with large sets of
defines in the <dt-bindings/*> files.

Combining these two approaches is not something I recommend.

Yours,
Linus Walleij

Re: [PATCH 0/2] pinctrl: zynqmp: Support muxing individual pins

[Linus Walleij]

On Tue, May 28, 2024 at 4:28 PM Sean Anderson <sean.anderson@linux.dev> wrote:

> Well, perhaps you should have reviewed the original driver more
> closely.

Do you want to push me down and increase my work related
stress? Because that is the effect of such statements.

It looks like criticism of me as a person, so explain yourself.

Writing this kind of things looks to me like some kind of abusive way
to express your desire and that is what burns maintainers out, so
if that is what you are doing, stop doing that, adjust your behaviour
and focus on technical issues.

> > If you want to mux individual pins instead of groups and functions, by
> > all means, but please do not mix the two approaches in the same
> > driver, I'm just trying to save Xilinx from themselves here.
>
> I see no point in creating thousands of groups

Please share your calculations for figures like "thousands".

In my experience, groups are usually in the tens, perhaps
hundreds, physically restricted by the number of pins
underneath a BGA. A Micro-FCBGA has 479 balls and many
are GND and power, so that sets a ballpark figure.

> for every combination of pin musings

It is clear from the documentation that the point if the pinmux
groups and pins are not to present all possible options (known as
a "phone exchange" solution) but those that are used in practice,
i.e. these representing real world use cases. See below.

> when we could just switch to the solution in this (or v2 of)
> patch. For compatibility we cannot be rid of the old situation, but we
> can at least fix it. There is no technical problem with them coexisting.

Historically there are  ~2 camps:

- One camp want to use groups and
functions to combine pins in groups with functions to form usecases.

In some cases (such as pinctrl-gemini.c or the very latest
pinctrl-scmi.c merged for v6.10) this reflects how the hardware
actually looks: it does not make individual pins available for muxing,
but you poke bits or send messages to change entire
groups-to-function mappings, so it is necessary for some hardware.

So when you write that "groups are a Linux-only concept" this
is because you probably haven't seen this part of the world.
Groups exist in hardware, and in the SCMI specification.

There are systems with individual control of the muxing
of every pin, such that e.g. every pin has a muxing register.

These are again not really phone exchanges: I am yet to see
a system where any function can be mapped to any pin. These
just do not exist. What exists in practice is that each pin can be
mapped to 2-4 functions, in extreme cases some more. Often
these functions are mapped to adjacent pins, and the "chessboard"
picture in the documentation for the subsystem reflects this.

For this reason, it is often helpful for driver writers to group
adjacent pins into groups, so an iterator can walk over the
pins and poke their registers in order, instead of treating each
pin as a unique entity.

- Then there is the camp that just by habit *want* to control
each pin individually. The extreme example is pinctrl-single.c
which is named like such because each pin is controlled by
a single register. TI wanted this solution mainly because their
hardware wasn't described in manuals, but in other HW
description files, and they needed to process large volumes
of data into DT-form.

I didn't like this solution initially because it makes it hard for
people without datasheets to understand what is going on.
But I was convinced to let this coexist with the group and function
mapping, which is fine: maybe one size doesn't fit all.

i.MX and others also do this approach but with large sets of
defines in the <dt-bindings/*> files.

Combining these two approaches is not something I recommend.

Yours,
Linus Walleij

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 0/2] pinctrl: zynqmp: Support muxing individual pins

[Sean Anderson]

On 5/29/24 04:38, Linus Walleij wrote:
> On Tue, May 28, 2024 at 4:28 PM Sean Anderson <sean.anderson@linux.dev> wrote:
> 
>> Well, perhaps you should have reviewed the original driver more
>> closely.
> 
> Do you want to push me down and increase my work related
> stress? Because that is the effect of such statements.
> 
> It looks like criticism of me as a person, so explain yourself.
> 
> Writing this kind of things looks to me like some kind of abusive way
> to express your desire and that is what burns maintainers out, so
> if that is what you are doing, stop doing that, adjust your behaviour
> and focus on technical issues.

The technical issue is that the driver does not match the hardware. We
must maintain the existing set of groups for backwards-compatibility.
But this should not prevent improvement.

Saying that we cannot have both group styles means that the driver is
permanently stuck with whatever was picked when it was submitted. Hence,
if you want to have only one style you had better review new drivers
very carefully.

>> > If you want to mux individual pins instead of groups and functions, by
>> > all means, but please do not mix the two approaches in the same
>> > driver, I'm just trying to save Xilinx from themselves here.
>>
>> I see no point in creating thousands of groups
> 
> Please share your calculations for figures like "thousands".
> 
> In my experience, groups are usually in the tens, perhaps
> hundreds, physically restricted by the number of pins
> underneath a BGA. A Micro-FCBGA has 479 balls and many
> are GND and power, so that sets a ballpark figure.

There are 78 muxable pins on this hardware, and around 40 groups, each
with signals that can be muxed to each pin. If we were to create groups
for each combination of signals and pins, there would literally be
thousands of groups.

>> for every combination of pin musings
> 
> It is clear from the documentation that the point if the pinmux
> groups and pins are not to present all possible options (known as
> a "phone exchange" solution) but those that are used in practice,
> i.e. these representing real world use cases. See below.
> 
>> when we could just switch to the solution in this (or v2 of)
>> patch. For compatibility we cannot be rid of the old situation, but we
>> can at least fix it. There is no technical problem with them coexisting.
> 
> Historically there are  ~2 camps:
> 
> - One camp want to use groups and
> functions to combine pins in groups with functions to form usecases.
> 
> In some cases (such as pinctrl-gemini.c or the very latest
> pinctrl-scmi.c merged for v6.10) this reflects how the hardware
> actually looks: it does not make individual pins available for muxing,
> but you poke bits or send messages to change entire
> groups-to-function mappings, so it is necessary for some hardware.
> 
> So when you write that "groups are a Linux-only concept" this
> is because you probably haven't seen this part of the world.
> Groups exist in hardware, and in the SCMI specification.

What I mean is that, for this hardware, groups are a Linux only concept.
Neither the firmware nor the hardware itself has a concept of groups.
While other hardware may have this concept, it does not apply here.

I do not object to groups where that is the hardware reality, but they
are unnecessarily constraining for this part.

> There are systems with individual control of the muxing
> of every pin, such that e.g. every pin has a muxing register.
> 
> These are again not really phone exchanges: I am yet to see
> a system where any function can be mapped to any pin. These
> just do not exist.

Canaan K210.

> What exists in practice is that each pin can be mapped to 2-4
> functions, in extreme cases some more. Often these functions are
> mapped to adjacent pins, and the "chessboard" picture in the
> documentation for the subsystem reflects this.
> 
> For this reason, it is often helpful for driver writers to group
> adjacent pins into groups, so an iterator can walk over the
> pins and poke their registers in order, instead of treating each
> pin as a unique entity.
> 
> - Then there is the camp that just by habit *want* to control
> each pin individually. The extreme example is pinctrl-single.c
> which is named like such because each pin is controlled by
> a single register. TI wanted this solution mainly because their
> hardware wasn't described in manuals, but in other HW
> description files, and they needed to process large volumes
> of data into DT-form.
> 
> I didn't like this solution initially because it makes it hard for
> people without datasheets to understand what is going on.
> But I was convinced to let this coexist with the group and function
> mapping, which is fine: maybe one size doesn't fit all.
> 
> i.MX and others also do this approach but with large sets of
> defines in the <dt-bindings/*> files.
> 
> Combining these two approaches is not something I recommend.

Well, the former approach is wrong for this hardware, but we must
support it for backwards-compatibility. A combination is the obvious
solution.

--Sean

Re: [PATCH 0/2] pinctrl: zynqmp: Support muxing individual pins

[Sean Anderson]

On 5/29/24 04:38, Linus Walleij wrote:
> On Tue, May 28, 2024 at 4:28 PM Sean Anderson <sean.anderson@linux.dev> wrote:
> 
>> Well, perhaps you should have reviewed the original driver more
>> closely.
> 
> Do you want to push me down and increase my work related
> stress? Because that is the effect of such statements.
> 
> It looks like criticism of me as a person, so explain yourself.
> 
> Writing this kind of things looks to me like some kind of abusive way
> to express your desire and that is what burns maintainers out, so
> if that is what you are doing, stop doing that, adjust your behaviour
> and focus on technical issues.

The technical issue is that the driver does not match the hardware. We
must maintain the existing set of groups for backwards-compatibility.
But this should not prevent improvement.

Saying that we cannot have both group styles means that the driver is
permanently stuck with whatever was picked when it was submitted. Hence,
if you want to have only one style you had better review new drivers
very carefully.

>> > If you want to mux individual pins instead of groups and functions, by
>> > all means, but please do not mix the two approaches in the same
>> > driver, I'm just trying to save Xilinx from themselves here.
>>
>> I see no point in creating thousands of groups
> 
> Please share your calculations for figures like "thousands".
> 
> In my experience, groups are usually in the tens, perhaps
> hundreds, physically restricted by the number of pins
> underneath a BGA. A Micro-FCBGA has 479 balls and many
> are GND and power, so that sets a ballpark figure.

There are 78 muxable pins on this hardware, and around 40 groups, each
with signals that can be muxed to each pin. If we were to create groups
for each combination of signals and pins, there would literally be
thousands of groups.

>> for every combination of pin musings
> 
> It is clear from the documentation that the point if the pinmux
> groups and pins are not to present all possible options (known as
> a "phone exchange" solution) but those that are used in practice,
> i.e. these representing real world use cases. See below.
> 
>> when we could just switch to the solution in this (or v2 of)
>> patch. For compatibility we cannot be rid of the old situation, but we
>> can at least fix it. There is no technical problem with them coexisting.
> 
> Historically there are  ~2 camps:
> 
> - One camp want to use groups and
> functions to combine pins in groups with functions to form usecases.
> 
> In some cases (such as pinctrl-gemini.c or the very latest
> pinctrl-scmi.c merged for v6.10) this reflects how the hardware
> actually looks: it does not make individual pins available for muxing,
> but you poke bits or send messages to change entire
> groups-to-function mappings, so it is necessary for some hardware.
> 
> So when you write that "groups are a Linux-only concept" this
> is because you probably haven't seen this part of the world.
> Groups exist in hardware, and in the SCMI specification.

What I mean is that, for this hardware, groups are a Linux only concept.
Neither the firmware nor the hardware itself has a concept of groups.
While other hardware may have this concept, it does not apply here.

I do not object to groups where that is the hardware reality, but they
are unnecessarily constraining for this part.

> There are systems with individual control of the muxing
> of every pin, such that e.g. every pin has a muxing register.
> 
> These are again not really phone exchanges: I am yet to see
> a system where any function can be mapped to any pin. These
> just do not exist.

Canaan K210.

> What exists in practice is that each pin can be mapped to 2-4
> functions, in extreme cases some more. Often these functions are
> mapped to adjacent pins, and the "chessboard" picture in the
> documentation for the subsystem reflects this.
> 
> For this reason, it is often helpful for driver writers to group
> adjacent pins into groups, so an iterator can walk over the
> pins and poke their registers in order, instead of treating each
> pin as a unique entity.
> 
> - Then there is the camp that just by habit *want* to control
> each pin individually. The extreme example is pinctrl-single.c
> which is named like such because each pin is controlled by
> a single register. TI wanted this solution mainly because their
> hardware wasn't described in manuals, but in other HW
> description files, and they needed to process large volumes
> of data into DT-form.
> 
> I didn't like this solution initially because it makes it hard for
> people without datasheets to understand what is going on.
> But I was convinced to let this coexist with the group and function
> mapping, which is fine: maybe one size doesn't fit all.
> 
> i.MX and others also do this approach but with large sets of
> defines in the <dt-bindings/*> files.
> 
> Combining these two approaches is not something I recommend.

Well, the former approach is wrong for this hardware, but we must
support it for backwards-compatibility. A combination is the obvious
solution.

--Sean

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 0/2] pinctrl: zynqmp: Support muxing individual pins

[Linus Walleij]

On Thu, May 30, 2024 at 7:08 PM Sean Anderson <sean.anderson@linux.dev> wrote:
> On 5/29/24 04:38, Linus Walleij wrote:
> > On Tue, May 28, 2024 at 4:28 PM Sean Anderson <sean.anderson@linux.dev> wrote:
> >
> >> Well, perhaps you should have reviewed the original driver more
> >> closely.
> >
> > Do you want to push me down and increase my work related
> > stress? Because that is the effect of such statements.
> >
> > It looks like criticism of me as a person, so explain yourself.
> >
> > Writing this kind of things looks to me like some kind of abusive way
> > to express your desire and that is what burns maintainers out, so
> > if that is what you are doing, stop doing that, adjust your behaviour
> > and focus on technical issues.
>
> The technical issue is that the driver does not match the hardware. We
> must maintain the existing set of groups for backwards-compatibility.
> But this should not prevent improvement.
>
> Saying that we cannot have both group styles means that the driver is
> permanently stuck with whatever was picked when it was submitted. Hence,
> if you want to have only one style you had better review new drivers
> very carefully.

Actually I did say you can rewrite it to the other style, it's just work.

If the previous approach was wrong, just redo it as it should be,
and rewrite the DT bindings and the existing device trees. If
backward-compatibility is so important, add a new driver with a new
unique Kconfig CONFIG_PINCTRL_ZYNQMP_V2 and new bindings
on the side and select one from a new compatible such as
"xlnx,zynqmp-pinctrl-v2", problem solved:
new driver new bindings, can be used on a per-board basis,
can be compiled into the same kernel image.

It may be embarrassing to have to tell the device tree maintainers
that the bindings got wrong three years ago and now we need to roll
a v2, but worse things have happened.

I don't like the approach
"this was done so we cannot redo it", we can always redo things,
it is even expected as proven by Fred Brooks timeless statement
in "The Mythical Man-Month": any team *will* always design
a throw-away system whether they intend it or not, there will be
a second version.

This approach will be more clean, I think? Also it will be
possible to phase over more boards and perhaps eventually
drop the old driver and the old bindings.

I'd like to hear from Xilinx/AMD how they want to solve this
going forward.

Yours,
Linus Walleij

Re: [PATCH 0/2] pinctrl: zynqmp: Support muxing individual pins

[Linus Walleij]

On Thu, May 30, 2024 at 7:08 PM Sean Anderson <sean.anderson@linux.dev> wrote:
> On 5/29/24 04:38, Linus Walleij wrote:
> > On Tue, May 28, 2024 at 4:28 PM Sean Anderson <sean.anderson@linux.dev> wrote:
> >
> >> Well, perhaps you should have reviewed the original driver more
> >> closely.
> >
> > Do you want to push me down and increase my work related
> > stress? Because that is the effect of such statements.
> >
> > It looks like criticism of me as a person, so explain yourself.
> >
> > Writing this kind of things looks to me like some kind of abusive way
> > to express your desire and that is what burns maintainers out, so
> > if that is what you are doing, stop doing that, adjust your behaviour
> > and focus on technical issues.
>
> The technical issue is that the driver does not match the hardware. We
> must maintain the existing set of groups for backwards-compatibility.
> But this should not prevent improvement.
>
> Saying that we cannot have both group styles means that the driver is
> permanently stuck with whatever was picked when it was submitted. Hence,
> if you want to have only one style you had better review new drivers
> very carefully.

Actually I did say you can rewrite it to the other style, it's just work.

If the previous approach was wrong, just redo it as it should be,
and rewrite the DT bindings and the existing device trees. If
backward-compatibility is so important, add a new driver with a new
unique Kconfig CONFIG_PINCTRL_ZYNQMP_V2 and new bindings
on the side and select one from a new compatible such as
"xlnx,zynqmp-pinctrl-v2", problem solved:
new driver new bindings, can be used on a per-board basis,
can be compiled into the same kernel image.

It may be embarrassing to have to tell the device tree maintainers
that the bindings got wrong three years ago and now we need to roll
a v2, but worse things have happened.

I don't like the approach
"this was done so we cannot redo it", we can always redo things,
it is even expected as proven by Fred Brooks timeless statement
in "The Mythical Man-Month": any team *will* always design
a throw-away system whether they intend it or not, there will be
a second version.

This approach will be more clean, I think? Also it will be
possible to phase over more boards and perhaps eventually
drop the old driver and the old bindings.

I'd like to hear from Xilinx/AMD how they want to solve this
going forward.

Yours,
Linus Walleij

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 0/2] pinctrl: zynqmp: Support muxing individual pins

[Michal Simek]

On 6/3/24 11:02, Linus Walleij wrote:
> On Thu, May 30, 2024 at 7:08 PM Sean Anderson <sean.anderson@linux.dev> wrote:
>> On 5/29/24 04:38, Linus Walleij wrote:
>>> On Tue, May 28, 2024 at 4:28 PM Sean Anderson <sean.anderson@linux.dev> wrote:
>>>
>>>> Well, perhaps you should have reviewed the original driver more
>>>> closely.
>>>
>>> Do you want to push me down and increase my work related
>>> stress? Because that is the effect of such statements.
>>>
>>> It looks like criticism of me as a person, so explain yourself.
>>>
>>> Writing this kind of things looks to me like some kind of abusive way
>>> to express your desire and that is what burns maintainers out, so
>>> if that is what you are doing, stop doing that, adjust your behaviour
>>> and focus on technical issues.
>>
>> The technical issue is that the driver does not match the hardware. We
>> must maintain the existing set of groups for backwards-compatibility.
>> But this should not prevent improvement.
>>
>> Saying that we cannot have both group styles means that the driver is
>> permanently stuck with whatever was picked when it was submitted. Hence,
>> if you want to have only one style you had better review new drivers
>> very carefully.
> 
> Actually I did say you can rewrite it to the other style, it's just work.
> 
> If the previous approach was wrong, just redo it as it should be,
> and rewrite the DT bindings and the existing device trees. If
> backward-compatibility is so important, add a new driver with a new
> unique Kconfig CONFIG_PINCTRL_ZYNQMP_V2 and new bindings
> on the side and select one from a new compatible such as
> "xlnx,zynqmp-pinctrl-v2", problem solved:
> new driver new bindings, can be used on a per-board basis,
> can be compiled into the same kernel image.
> 
> It may be embarrassing to have to tell the device tree maintainers
> that the bindings got wrong three years ago and now we need to roll
> a v2, but worse things have happened.
> 
> I don't like the approach
> "this was done so we cannot redo it", we can always redo things,
> it is even expected as proven by Fred Brooks timeless statement
> in "The Mythical Man-Month": any team *will* always design
> a throw-away system whether they intend it or not, there will be
> a second version.
> 
> This approach will be more clean, I think? Also it will be
> possible to phase over more boards and perhaps eventually
> drop the old driver and the old bindings.
> 
> I'd like to hear from Xilinx/AMD how they want to solve this
> going forward.

Sorry for delay jumping to this long discussion. Groups were chosen because 
that's how design tool (Vivado) allow you to configure it. That's why these 
groups are described in TF-A. That's something what fits the need for most of 
use cases.
But then there is technical side of things what can be setup via registers.

If you look at uart0. There are 19 groups in Vivado. uart0_grp0 MIO2-3, etc 
uart0_grp19 MIO74-75 and that's what it is described today.
But it is completely valid if you take MIO 2 from grp0 and MIO75 from grp19.
It will work without any issue but likely we can't described it today.

Uart is interesting one because you can configure one RX channel and up to 19 
(number of groups) TX and it will work properly from HW perspective. I don't 
think you can describe it via DT today.

In Sean's use case with SD we don't have groups with less then 4 data pins but 
SD cards should be still working and that's what we can't really describe too.

I don't think Xilinx recommends these descriptions and normally customers are 
not design it like this but from HW perspective it should be working properly.


Regarding driver. I agree that describing all possible configurations via groups 
is not a good idea because describing all combinations is very hard. It is not 
unlimited number of them but that number is very very high and it won't serve 
the purpose.

TF-A should be capable to provide all information about configurations already 
it is just up to OS to use it properly.
If there is something missing we can take a look but we have already issue with 
TF-A running out of space.

 From my perspective allowing functionality per single MIO pins is not a bad way 
to go.
Having it in separate driver is possible but don't think it is worth of effort.

I have asked Sai (driver owner) to take a look at the patch more closely.

Thanks,
Michal

Re: [PATCH 0/2] pinctrl: zynqmp: Support muxing individual pins

[Michal Simek]

On 6/3/24 11:02, Linus Walleij wrote:
> On Thu, May 30, 2024 at 7:08 PM Sean Anderson <sean.anderson@linux.dev> wrote:
>> On 5/29/24 04:38, Linus Walleij wrote:
>>> On Tue, May 28, 2024 at 4:28 PM Sean Anderson <sean.anderson@linux.dev> wrote:
>>>
>>>> Well, perhaps you should have reviewed the original driver more
>>>> closely.
>>>
>>> Do you want to push me down and increase my work related
>>> stress? Because that is the effect of such statements.
>>>
>>> It looks like criticism of me as a person, so explain yourself.
>>>
>>> Writing this kind of things looks to me like some kind of abusive way
>>> to express your desire and that is what burns maintainers out, so
>>> if that is what you are doing, stop doing that, adjust your behaviour
>>> and focus on technical issues.
>>
>> The technical issue is that the driver does not match the hardware. We
>> must maintain the existing set of groups for backwards-compatibility.
>> But this should not prevent improvement.
>>
>> Saying that we cannot have both group styles means that the driver is
>> permanently stuck with whatever was picked when it was submitted. Hence,
>> if you want to have only one style you had better review new drivers
>> very carefully.
> 
> Actually I did say you can rewrite it to the other style, it's just work.
> 
> If the previous approach was wrong, just redo it as it should be,
> and rewrite the DT bindings and the existing device trees. If
> backward-compatibility is so important, add a new driver with a new
> unique Kconfig CONFIG_PINCTRL_ZYNQMP_V2 and new bindings
> on the side and select one from a new compatible such as
> "xlnx,zynqmp-pinctrl-v2", problem solved:
> new driver new bindings, can be used on a per-board basis,
> can be compiled into the same kernel image.
> 
> It may be embarrassing to have to tell the device tree maintainers
> that the bindings got wrong three years ago and now we need to roll
> a v2, but worse things have happened.
> 
> I don't like the approach
> "this was done so we cannot redo it", we can always redo things,
> it is even expected as proven by Fred Brooks timeless statement
> in "The Mythical Man-Month": any team *will* always design
> a throw-away system whether they intend it or not, there will be
> a second version.
> 
> This approach will be more clean, I think? Also it will be
> possible to phase over more boards and perhaps eventually
> drop the old driver and the old bindings.
> 
> I'd like to hear from Xilinx/AMD how they want to solve this
> going forward.

Sorry for delay jumping to this long discussion. Groups were chosen because 
that's how design tool (Vivado) allow you to configure it. That's why these 
groups are described in TF-A. That's something what fits the need for most of 
use cases.
But then there is technical side of things what can be setup via registers.

If you look at uart0. There are 19 groups in Vivado. uart0_grp0 MIO2-3, etc 
uart0_grp19 MIO74-75 and that's what it is described today.
But it is completely valid if you take MIO 2 from grp0 and MIO75 from grp19.
It will work without any issue but likely we can't described it today.

Uart is interesting one because you can configure one RX channel and up to 19 
(number of groups) TX and it will work properly from HW perspective. I don't 
think you can describe it via DT today.

In Sean's use case with SD we don't have groups with less then 4 data pins but 
SD cards should be still working and that's what we can't really describe too.

I don't think Xilinx recommends these descriptions and normally customers are 
not design it like this but from HW perspective it should be working properly.


Regarding driver. I agree that describing all possible configurations via groups 
is not a good idea because describing all combinations is very hard. It is not 
unlimited number of them but that number is very very high and it won't serve 
the purpose.

TF-A should be capable to provide all information about configurations already 
it is just up to OS to use it properly.
If there is something missing we can take a look but we have already issue with 
TF-A running out of space.

 From my perspective allowing functionality per single MIO pins is not a bad way 
to go.
Having it in separate driver is possible but don't think it is worth of effort.

I have asked Sai (driver owner) to take a look at the patch more closely.

Thanks,
Michal





_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 0/2] pinctrl: zynqmp: Support muxing individual pins

[Linus Walleij]

On Mon, Jun 3, 2024 at 2:37 PM Michal Simek <michal.simek@amd.com> wrote:

> I have asked Sai (driver owner) to take a look at the patch more closely.

OK if you and Sai both provide your Reviewed-by tags so I know this is what you
want to happen, then I'll apply the patch.

Thanks Michal!

Yours,
Linus Walleij

Re: [PATCH 0/2] pinctrl: zynqmp: Support muxing individual pins

[Linus Walleij]

On Mon, Jun 3, 2024 at 2:37 PM Michal Simek <michal.simek@amd.com> wrote:

> I have asked Sai (driver owner) to take a look at the patch more closely.

OK if you and Sai both provide your Reviewed-by tags so I know this is what you
want to happen, then I'll apply the patch.

Thanks Michal!

Yours,
Linus Walleij

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel