On 06/05/2025 11:11, luyulin wrote: > This commit adds Device Tree binding documentation for the
Please do not use "This commit/patch/change", but imperative mood. See longer explanation here: https://elixir.bootlin.com/linux/v5.17.1/source/Documentation/process/submitting-patches.rst#L95 > ESWIN EIC7700 pinctrl controller module. The document describes > the required properties, compatible strings, and usage examples > in the device tree configuration when applying this module. Do not explain what DT binding is. We all know. Explain the hardware. See also other comits as examples. > > Co-developed-by: Samuel Holland <[email protected]> > Signed-off-by: Samuel Holland <[email protected]> > Signed-off-by: luyulin <[email protected]> Looks like you used login name as full name. Use rather latin transcription or your name in your native language. > --- > .../pinctrl/eswin,eic7700-pinctrl.yaml | 156 ++++++++++++++++++ This was already send and reviewed. Implement previous feedback https://lore.kernel.org/all/20250326-owl-of-algebraic-wealth-61aeda@krzk-bin/ Then please version your patches correctly, e.g. use b4 or git format-patch -vX, and add changelog in cover letter or under '---' of individual patches describing changes from previous version. > 1 file changed, 156 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/pinctrl/eswin,eic7700-pinctrl.yaml > > diff --git > a/Documentation/devicetree/bindings/pinctrl/eswin,eic7700-pinctrl.yaml > b/Documentation/devicetree/bindings/pinctrl/eswin,eic7700-pinctrl.yaml > new file mode 100644 > index 000000000000..d8811a8e0a51 > --- /dev/null > +++ b/Documentation/devicetree/bindings/pinctrl/eswin,eic7700-pinctrl.yaml > @@ -0,0 +1,156 @@ > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/pinctrl/eswin,eic7700-pinctrl.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Eswin Eic7700 Pinctrl > + > +maintainers: > + - LuYuLin <[email protected]> > + > +description: | > + Please refer to pinctrl-bindings.txt in this directory for details of the > + common pinctrl bindings used by client devices, including the meaning of > the > + phrase "pin configuration node". Drop this paragraph, redundant. > + > + eic7700 pin configuration nodes act as a container for an arbitrary number > of > + subnodes. Each of these subnodes represents some desired configuration for > one or > + more pins. This configuration can include the mux function to select on > those pin(s), > + and various pin configuration parameters, such as input-enable, pull-up, > etc. > + > +properties: > + compatible: > + const: eswin,eic7700-pinctrl > + > + reg: > + description: Specifies the base address and size of the SLCR space. Drop description, redundant. > + maxItems: 1 > + > + "vrgmii-supply": Open existing bindings. Do you see any property written like that? No, therefore do not come with own syntax, Drop quotes, > + description: > + Regulator supply for the RGMII interface IO power domain. > + This property should reference a regulator that provides either 1.8V > or 3.3V, > + depending on the board-level voltage configuration required by the > RGMII interface. > + > +patternProperties: > + '^(.*-)?(pins)$': Weirdly complicated pattern. Why not just "-pins$"? For which case do you need "pins" name? > + type: object > + description: > + Pinctrl node's client devices use subnodes for pin muxes, > + which in turn use below standard properties. > + > + properties: > + pins: > + description: > + For eic7700, specifies the name(s) of one or more pins to be > configured by > + this node. > + items: > + enum: [ chip_mode, mode_set0, mode_set1, mode_set2, mode_set3, xin, > + rst_out_n, key_reset_n, gpio0, por_sel, jtag0_tck, > jtag0_tms, > + jtag0_tdi, jtag0_tdo, gpio5, spi2_cs0_n, jtag1_tck, > jtag1_tms, > + jtag1_tdi, jtag1_tdo, gpio11, spi2_cs1_n, pcie_clkreq_n, > + pcie_wake_n, pcie_perst_n, hdmi_scl, hdmi_sda, hdmi_cec, > + jtag2_trst, rgmii0_clk_125, rgmii0_txen, rgmii0_txclk, > + rgmii0_txd0, rgmii0_txd1, rgmii0_txd2, rgmii0_txd3, > i2s0_bclk, > + i2s0_wclk, i2s0_sdi, i2s0_sdo, i2s_mclk, rgmii0_rxclk, > + rgmii0_rxdv, rgmii0_rxd0, rgmii0_rxd1, rgmii0_rxd2, > rgmii0_rxd3, > + i2s2_bclk, i2s2_wclk, i2s2_sdi, i2s2_sdo, gpio27, gpio28, > gpio29, > + rgmii0_mdc, rgmii0_mdio, rgmii0_intb, rgmii1_clk_125, > rgmii1_txen, > + rgmii1_txclk, rgmii1_txd0, rgmii1_txd1, rgmii1_txd2, > rgmii1_txd3, > + i2s1_bclk, i2s1_wclk, i2s1_sdi, i2s1_sdo, gpio34, > rgmii1_rxclk, > + rgmii1_rxdv, rgmii1_rxd0, rgmii1_rxd1, rgmii1_rxd2, > rgmii1_rxd3, > + spi1_cs0_n, spi1_clk, spi1_d0, spi1_d1, spi1_d2, spi1_d3, > spi1_cs1_n, > + rgmii1_mdc, rgmii1_mdio, rgmii1_intb, usb0_pwren, > usb1_pwren, > + i2c0_scl, i2c0_sda, i2c1_scl, i2c1_sda, i2c2_scl, i2c2_sda, > + i2c3_scl, i2c3_sda, i2c4_scl, i2c4_sda, i2c5_scl, i2c5_sda, > + uart0_tx, uart0_rx, uart1_tx, uart1_rx, uart1_cts, > uart1_rts, > + uart2_tx, uart2_rx, jtag2_tck, jtag2_tms, jtag2_tdi, > jtag2_tdo, > + fan_pwm, fan_tach, mipi_csi0_xvs, mipi_csi0_xhs, > mipi_csi0_mclk, > + mipi_csi1_xvs, mipi_csi1_xhs, mipi_csi1_mclk, > mipi_csi2_xvs, > + mipi_csi2_xhs, mipi_csi2_mclk, mipi_csi3_xvs, > mipi_csi3_xhs, > + mipi_csi3_mclk, mipi_csi4_xvs, mipi_csi4_xhs, > mipi_csi4_mclk, > + mipi_csi5_xvs, mipi_csi5_xhs, mipi_csi5_mclk, spi3_cs_n, > spi3_clk, > + spi3_di, spi3_do, gpio92, gpio93, s_mode, gpio95, > spi0_cs_n, > + spi0_clk, spi0_d0, spi0_d1, spi0_d2, spi0_d3, i2c10_scl, > + i2c10_sda, i2c11_scl, i2c11_sda, gpio106, boot_sel0, > boot_sel1, > + boot_sel2, boot_sel3, gpio111, lpddr_ref_clk ] > + > + function: > + description: > + Specify the alternative function to be configured for the > + given pins. > + enum: [ disabled, boot_sel, chip_mode, emmc, fan_tach, > + gpio, hdmi, i2c, i2s, jtag, ddr_ref_clk_sel, > + lpddr_ref_clk, mipi_csi, osc, pcie, pwm, > + rgmii, reset, sata, sdio, spi, s_mode, uart, usb ] > + > + input-schmitt-enable: true > + > + input-schmitt-disable: true > + > + bias-disable: true > + > + bias-pull-down: true > + > + bias-pull-up: true > + > + input-enable: true > + > + input-disable: true > + > + drive-strength-microamp: true > + > + allOf: > + - $ref: pincfg-node.yaml# > + - $ref: pinmux-node.yaml# > + > + - if: > + properties: > + pins: > + anyOf: > + - pattern: '^rgmii' > + - const: lpddr_ref_clk > + then: > + properties: > + drive-strength-microamp: > + enum: [3000, 6000, 9000, 12000, 15000, 18000, 21000, 24000] > + else: > + properties: > + drive-strength-microamp: > + enum: [6000, 9000, 12000, 15000, 18000, 21000, 24000, 27000] > + > + required: > + - pins > + > + additionalProperties: false > + > +allOf: > + - $ref: pinctrl.yaml# > + > +required: > + - compatible > + - reg > + > +additionalProperties: false > + > +examples: > + - | > + pinctrl@51600080 { > + compatible = "eswin,eic7700-pinctrl"; > + reg = <0x51600080 0x1fff80>; > + vrgmii-supply = <&vcc_1v8>; > + gpio10_pins: gpio10-pins { Drop label. > + pins = "jtag1_tdo"; > + function = "gpio"; > + input-enable; > + bias-pull-up; > + }; > + }; > + > + i2c2 { Drop this node, not relevant. Best regards, Krzysztof
