dt-bindings: pinctrl: tegra234: Add DT binding doc
Subsystems (4)
PATCH 1/3 dt-bindings: pinctrl: tegra234: Add DT binding doc
Add DT binding doc for Tegra234 pinmux driver.
--- .../pinctrl/nvidia,tegra234-pinmux.yaml | 232 ++++++++++++++++++ 1 file changed, 232 insertions(+) create mode 100644 Documentation/devicetree/bindings/pinctrl/nvidia,tegra234-pinmux.yaml
Documentation/devicetree/bindings/pinctrl/nvidia,tegra234-pinmux.yaml new file +232 -0
@@ -0,0 +1,232 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/pinctrl/nvidia,tegra234-pinmux.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: NVIDIA Tegra234 Pinmux Controller + +maintainers: + - Thierry Reding <[email protected]> + - Jon Hunter <[email protected]> + +properties: + compatible: + enum: + - nvidia,tegra234-pinmux + - nvidia,tegra234-pinmux-aon + + reg: + items: + - description: pinmux registers + +patternProperties: + "^pinmux(-[a-z0-9-_]+)?$": + type: object + properties: + phandle: true + + # pin groups + additionalProperties: + $ref: nvidia,tegra-pinmux-common.yaml + unevaluatedProperties: false + properties: + nvidia,function: + enum: [ gp, uartc, i2c8, spi2, i2c2, can1, can0, rsvd0, eth0, eth2, + eth1, dp, eth3, i2c4, i2c7, i2c9, eqos, pe2, pe1, pe0, pe3, + pe4, pe5, pe6, pe7, pe8, pe9, pe10, qspi0, qspi1, qpsi, + sdmmc1, sce, soc, gpio, hdmi, ufs0, spi3, spi1, uartb, uarte, + usb, extperiph2, extperiph1, i2c3, vi0, i2c5, uarta, uartd, + i2c1, i2s4, i2s6, aud, spi5, touch, uartj, rsvd1, wdt, tsc, + dmic3, led, vi0_alt, i2s5, nv, extperiph3, extperiph4, spi4, + ccla, i2s1, i2s2, i2s3, i2s8, rsvd2, dmic5, dca, displayb, + displaya, vi1, dcb, dmic1, dmic4, i2s7, dmic2, dspk0, rsvd3, + tsc_alt, istctrl, vi1_alt, dspk1, igpu ] + + nvidia,pull: true + nvidia,tristate: true + nvidia,schmitt: true + nvidia,enable-input: true + nvidia,open-drain: true + nvidia,lock: true + nvidia,drive-type: true + nvidia,io-hv: true + + required: + - nvidia,pins + +additionalProperties: false + +allOf: + - if: + properties: + compatible: + const: nvidia,tegra234-pinmux + then: + patternProperties: + "^pinmux(-[a-z0-9-_]+)?$": + type: object + additionalProperties: + properties: + nvidia,pins: + description: An array of strings. Each string contains the name + of a pin or group. Valid values for these names are listed + below. + + items: + enum: [ dap6_sclk_pa0, dap6_dout_pa1, dap6_din_pa2, + dap6_fs_pa3, dap4_sclk_pa4, dap4_dout_pa5, + dap4_din_pa6, dap4_fs_pa7, soc_gpio08_pb0, + qspi0_sck_pc0, qspi0_cs_n_pc1, + qspi0_io0_pc2, qspi0_io1_pc3, qspi0_io2_pc4, + qspi0_io3_pc5, qspi1_sck_pc6, qspi1_cs_n_pc7, + qspi1_io0_pd0, qspi1_io1_pd1, qspi1_io2_pd2, + qspi1_io3_pd3, eqos_txc_pe0, eqos_td0_pe1, + eqos_td1_pe2, eqos_td2_pe3, eqos_td3_pe4, + eqos_tx_ctl_pe5, eqos_rd0_pe6, eqos_rd1_pe7, + eqos_rd2_pf0, eqos_rd3_pf1, eqos_rx_ctl_pf2, + eqos_rxc_pf3, eqos_sma_mdio_pf4, eqos_sma_mdc_pf5, + soc_gpio13_pg0, soc_gpio14_pg1, soc_gpio15_pg2, + soc_gpio16_pg3, soc_gpio17_pg4, soc_gpio18_pg5, + soc_gpio19_pg6, soc_gpio20_pg7, soc_gpio21_ph0, + soc_gpio22_ph1, soc_gpio06_ph2, uart4_tx_ph3, + uart4_rx_ph4, uart4_rts_ph5, uart4_cts_ph6, + soc_gpio41_ph7, soc_gpio42_pi0, soc_gpio43_pi1, + soc_gpio44_pi2, gen1_i2c_scl_pi3, gen1_i2c_sda_pi4, + cpu_pwr_req_pi5, soc_gpio07_pi6, + sdmmc1_clk_pj0, sdmmc1_cmd_pj1, sdmmc1_dat0_pj2, + sdmmc1_dat1_pj3, sdmmc1_dat2_pj4, sdmmc1_dat3_pj5, + pex_l0_clkreq_n_pk0, pex_l0_rst_n_pk1, + pex_l1_clkreq_n_pk2, pex_l1_rst_n_pk3, + pex_l2_clkreq_n_pk4, pex_l2_rst_n_pk5, + pex_l3_clkreq_n_pk6, pex_l3_rst_n_pk7, + pex_l4_clkreq_n_pl0, pex_l4_rst_n_pl1, + pex_wake_n_pl2, soc_gpio34_pl3, dp_aux_ch0_hpd_pm0, + dp_aux_ch1_hpd_pm1, dp_aux_ch2_hpd_pm2, + dp_aux_ch3_hpd_pm3, soc_gpio55_pm4, soc_gpio36_pm5, + soc_gpio53_pm6, soc_gpio38_pm7, dp_aux_ch3_n_pn0, + soc_gpio39_pn1, soc_gpio40_pn2, dp_aux_ch1_p_pn3, + dp_aux_ch1_n_pn4, dp_aux_ch2_p_pn5, dp_aux_ch2_n_pn6, + dp_aux_ch3_p_pn7, extperiph1_clk_pp0, + extperiph2_clk_pp1, cam_i2c_scl_pp2, cam_i2c_sda_pp3, + soc_gpio23_pp4, soc_gpio24_pp5, soc_gpio25_pp6, + pwr_i2c_scl_pp7, pwr_i2c_sda_pq0, soc_gpio28_pq1, + soc_gpio29_pq2, soc_gpio30_pq3, soc_gpio31_pq4, + soc_gpio32_pq5, soc_gpio33_pq6, soc_gpio35_pq7, + soc_gpio37_pr0, soc_gpio56_pr1, uart1_tx_pr2, + uart1_rx_pr3, uart1_rts_pr4, uart1_cts_pr5, + soc_gpio61_pw0, soc_gpio62_pw1, gpu_pwr_req_px0, cv_pwr_req_px1, + gp_pwm2_px2, gp_pwm3_px3, uart2_tx_px4, uart2_rx_px5, + uart2_rts_px6, uart2_cts_px7, spi3_sck_py0, + spi3_miso_py1, spi3_mosi_py2, spi3_cs0_py3, + spi3_cs1_py4, uart5_tx_py5, uart5_rx_py6, + uart5_rts_py7, uart5_cts_pz0, usb_vbus_en0_pz1, + usb_vbus_en1_pz2, spi1_sck_pz3, spi1_miso_pz4, + spi1_mosi_pz5, spi1_cs0_pz6, spi1_cs1_pz7, + spi5_sck_pac0, spi5_miso_pac1, spi5_mosi_pac2, + spi5_cs0_pac3, soc_gpio57_pac4, soc_gpio58_pac5, + soc_gpio59_pac6, soc_gpio60_pac7, soc_gpio45_pad0, + soc_gpio46_pad1, soc_gpio47_pad2, soc_gpio48_pad3, + ufs0_ref_clk_pae0, ufs0_rst_n_pae1, + pex_l5_clkreq_n_paf0, pex_l5_rst_n_paf1, + pex_l6_clkreq_n_paf2, pex_l6_rst_n_paf3, + pex_l7_clkreq_n_pag0, pex_l7_rst_n_pag1, + pex_l8_clkreq_n_pag2, pex_l8_rst_n_pag3, + pex_l9_clkreq_n_pag4, pex_l9_rst_n_pag5, + pex_l10_clkreq_n_pag6, pex_l10_rst_n_pag7, + sdmmc1_comp, eqos_comp, qspi_comp, + # drive groups + drive_soc_gpio08_pb0, drive_soc_gpio36_pm5, + drive_soc_gpio53_pm6, drive_soc_gpio55_pm4, + drive_soc_gpio38_pm7, drive_soc_gpio39_pn1, + drive_soc_gpio40_pn2, drive_dp_aux_ch0_hpd_pm0, + drive_dp_aux_ch1_hpd_pm1, drive_dp_aux_ch2_hpd_pm2, + drive_dp_aux_ch3_hpd_pm3, drive_dp_aux_ch1_p_pn3, + drive_dp_aux_ch1_n_pn4, drive_dp_aux_ch2_p_pn5, + drive_dp_aux_ch2_n_pn6, drive_dp_aux_ch3_p_pn7, + drive_dp_aux_ch3_n_pn0, drive_pex_l2_clkreq_n_pk4, + drive_pex_wake_n_pl2, drive_pex_l1_clkreq_n_pk2, + drive_pex_l1_rst_n_pk3, drive_pex_l0_clkreq_n_pk0, + drive_pex_l0_rst_n_pk1, drive_pex_l2_rst_n_pk5, + drive_pex_l3_clkreq_n_pk6, drive_pex_l3_rst_n_pk7, + drive_pex_l4_clkreq_n_pl0, drive_pex_l4_rst_n_pl1, + drive_soc_gpio34_pl3, drive_pex_l5_clkreq_n_paf0, + drive_pex_l5_rst_n_paf1, drive_pex_l6_clkreq_n_paf2, + drive_pex_l6_rst_n_paf3, drive_pex_l10_clkreq_n_pag6, + drive_pex_l10_rst_n_pag7, drive_pex_l7_clkreq_n_pag0, + drive_pex_l7_rst_n_pag1, drive_pex_l8_clkreq_n_pag2, + drive_pex_l8_rst_n_pag3, drive_pex_l9_clkreq_n_pag4, + drive_pex_l9_rst_n_pag5, drive_sdmmc1_clk_pj0, + drive_sdmmc1_cmd_pj1, drive_sdmmc1_dat3_pj5, + drive_sdmmc1_dat2_pj4, drive_sdmmc1_dat1_pj3, + drive_sdmmc1_dat0_pj2 ] + + - if: + properties: + compatible: + const: nvidia,tegra234-pinmux-aon + then: + patternProperties: + "^pinmux(-[a-z0-9-_]+)?$": + type: object + additionalProperties: + properties: + nvidia,pins: + items: + enum: [ can0_dout_paa0, can0_din_paa1, can1_dout_paa2, + can1_din_paa3, can0_stb_paa4, can0_en_paa5, + soc_gpio49_paa6, can0_err_paa7, can1_stb_pbb0, + can1_en_pbb1, soc_gpio50_pbb2, can1_err_pbb3, + spi2_sck_pcc0, spi2_miso_pcc1, spi2_mosi_pcc2, + spi2_cs0_pcc3, touch_clk_pcc4, uart3_tx_pcc5, + uart3_rx_pcc6, gen2_i2c_scl_pcc7, gen2_i2c_sda_pdd0, + gen8_i2c_scl_pdd1, gen8_i2c_sda_pdd2, + sce_error_pee0, vcomp_alert_pee1, + ao_retention_n_pee2, batt_oc_pee3, power_on_pee4, + soc_gpio26_pee5, soc_gpio27_pee6, bootv_ctl_n_pee7, + hdmi_cec_pgg0, + # drive groups + drive_touch_clk_pcc4, drive_uart3_rx_pcc6, + drive_uart3_tx_pcc5, drive_gen8_i2c_sda_pdd2, + drive_gen8_i2c_scl_pdd1, drive_spi2_mosi_pcc2, + drive_gen2_i2c_scl_pcc7, drive_spi2_cs0_pcc3, + drive_gen2_i2c_sda_pdd0, drive_spi2_sck_pcc0, + drive_spi2_miso_pcc1, drive_can1_dout_paa2, + drive_can1_din_paa3, drive_can0_dout_paa0, + drive_can0_din_paa1, drive_can0_stb_paa4, + drive_can0_en_paa5, drive_soc_gpio49_paa6, + drive_can0_err_paa7, drive_can1_stb_pbb0, + drive_can1_en_pbb1, drive_soc_gpio50_pbb2, + drive_can1_err_pbb3, drive_sce_error_pee0, + drive_batt_oc_pee3, drive_bootv_ctl_n_pee7, + drive_power_on_pee4, drive_soc_gpio26_pee5, + drive_soc_gpio27_pee6, drive_ao_retention_n_pee2, + drive_vcomp_alert_pee1, drive_hdmi_cec_pgg0 ] + +required: + - compatible + - reg + +examples: + - | + #include <dt-bindings/pinctrl/pinctrl-tegra.h> + + pinmux@2430000 { + compatible = "nvidia,tegra234-pinmux"; + reg = <0x2430000 0x17000>; + + pinctrl-names = "pex_rst"; + pinctrl-0 = <&pex_rst_c5_out_state>; + + pex_rst_c5_out_state: pinmux-pex-rst-c5-out { + pex_rst { + nvidia,pins = "pex_l5_rst_n_pgg1"; + nvidia,schmitt = <TEGRA_PIN_DISABLE>; + nvidia,enable-input = <TEGRA_PIN_DISABLE>; + nvidia,io-hv = <TEGRA_PIN_ENABLE>; + nvidia,tristate = <TEGRA_PIN_DISABLE>; + nvidia,pull = <TEGRA_PIN_PULL_NONE>; + }; + }; + }; +...
Signature · 1 line
2.17.1
PATCH 2/3 pinctrl: tegra: Add Tegra234 pinmux driver
This change adds pinmux support for Tegra234
--- drivers/pinctrl/tegra/Kconfig | 4 + drivers/pinctrl/tegra/Makefile | 1 + drivers/pinctrl/tegra/pinctrl-tegra234.c | 1972 ++++++++++++++++++++++ drivers/soc/tegra/Kconfig | 1 + 4 files changed, 1978 insertions(+) create mode 100644 drivers/pinctrl/tegra/pinctrl-tegra234.c
drivers/pinctrl/tegra/Kconfig +4 -0
@@ -28,6 +28,10 @@ config PINCTRL_TEGRA194 bool select PINCTRL_TEGRA +config PINCTRL_TEGRA234 + bool + select PINCTRL_TEGRA + config PINCTRL_TEGRA_XUSB def_bool y if ARCH_TEGRA select GENERIC_PHY
drivers/pinctrl/tegra/Makefile +1 -0
@@ -6,4 +6,5 @@ obj-$(CONFIG_PINCTRL_TEGRA114) += pinctrl-tegra114.o obj-$(CONFIG_PINCTRL_TEGRA124) += pinctrl-tegra124.o obj-$(CONFIG_PINCTRL_TEGRA210) += pinctrl-tegra210.o obj-$(CONFIG_PINCTRL_TEGRA194) += pinctrl-tegra194.o +obj-$(CONFIG_PINCTRL_TEGRA234) += pinctrl-tegra234.o obj-$(CONFIG_PINCTRL_TEGRA_XUSB) += pinctrl-tegra-xusb.o
drivers/pinctrl/tegra/pinctrl-tegra234.c new file +1972 -0
1,972 lines not on this page · raw message
drivers/soc/tegra/Kconfig +1 -0
@@ -125,6 +125,7 @@ config ARCH_TEGRA_234_SOC bool "NVIDIA Tegra234 SoC" depends on !CPU_BIG_ENDIAN select MAILBOX + select PINCTRL_TEGRA234 select TEGRA_BPMP select TEGRA_HSP_MBOX select TEGRA_IVC
Signature · 1 line
2.17.1
PATCH 3/3 arm64: tegra: Add Tegra234 pinmux device
In reply to Prathamesh SheteThis change adds pinmux node for Tegra234.
--- arch/arm64/boot/dts/nvidia/tegra234.dtsi | 14 ++++++++++++++ 1 file changed, 14 insertions(+)
arch/arm64/boot/dts/nvidia/tegra234.dtsi +14 -0
@@ -701,6 +701,13 @@ interrupt-controller; #gpio-cells = <2>; gpio-controller; + gpio-ranges = <&pinmux 0 0 164>; + }; + + pinmux: pinmux@2430000 { + compatible = "nvidia,tegra234-pinmux"; + reg = <0x2430000 0x19100>; + status = "okay"; }; mc: memory-controller@2c00000 {
@@ -1664,6 +1671,13 @@ interrupt-controller; #gpio-cells = <2>; gpio-controller; + gpio-range = <&pinmux_aon 0 0 32>; + }; + + pinmux_aon: pinmux@c300000 { + compatible = "nvidia,tegra234-pinmux-aon"; + reg = <0xc300000 0x4000>; + status = "okay"; }; pwm4: pwm@c340000 {
Signature · 1 line
2.17.1
Re: [PATCH 3/3] arm64: tegra: Add Tegra234 pinmux device
Quoted 12 lines · Prathamesh Shete
This change adds pinmux node for Tegra234.
Signed-off-by Prathamesh Shete <[email protected]>--- arch/arm64/boot/dts/nvidia/tegra234.dtsi | 14 ++++++++++++++ 1 file changed, 14 insertions(+)arch/arm64/boot/dts/nvidia/tegra234.dtsi +0
-0
arch/arm64/boot/dts/nvidia/tegra234.dtsi line 710
Quoted 10 lines
arch/arm64/boot/dts/nvidia/tegra234.dtsi +7
-0@@ -701,6 +701,13 @@ interrupt-controller; #gpio-cells = <2>; gpio-controller; + gpio-ranges = <&pinmux 0 0 164>; + }; + + pinmux: pinmux@2430000 { + compatible = "nvidia,tegra234-pinmux"; + reg = <0x2430000 0x19100>; + status = "okay";
Why? Anything disabled it?
Quoted 4 lines
arch/arm64/boot/dts/nvidia/tegra234.dtsi +0
-0}; mc: memory-controller@2c00000 {
arch/arm64/boot/dts/nvidia/tegra234.dtsi line 1680
Quoted 10 lines
arch/arm64/boot/dts/nvidia/tegra234.dtsi +7
-0@@ -1664,6 +1671,13 @@ interrupt-controller; #gpio-cells = <2>; gpio-controller; + gpio-range = <&pinmux_aon 0 0 32>; + }; + + pinmux_aon: pinmux@c300000 { + compatible = "nvidia,tegra234-pinmux-aon"; + reg = <0xc300000 0x4000>; + status = "okay";
Also why?
arch/arm64/boot/dts/nvidia/tegra234.dtsi line 1683
Quoted 3 lines
arch/arm64/boot/dts/nvidia/tegra234.dtsi +0
-0}; pwm4: pwm@c340000 {
Best regards,
Krzysztof
Re: [PATCH 3/3] arm64: tegra: Add Tegra234 pinmux device
Quoted 28 lines · Krzysztof Kozlowski
Quoted 12 lines · Prathamesh Shete
This change adds pinmux node for Tegra234.
Signed-off-by Prathamesh Shete <[email protected]>--- arch/arm64/boot/dts/nvidia/tegra234.dtsi | 14 ++++++++++++++ 1 file changed, 14 insertions(+)arch/arm64/boot/dts/nvidia/tegra234.dtsi +0
-0arch/arm64/boot/dts/nvidia/tegra234.dtsi line 710
Quoted 10 lines
arch/arm64/boot/dts/nvidia/tegra234.dtsi +7
-0@@ -701,6 +701,13 @@ interrupt-controller; #gpio-cells = <2>; gpio-controller; + gpio-ranges = <&pinmux 0 0 164>; + }; + + pinmux: pinmux@2430000 { + compatible = "nvidia,tegra234-pinmux"; + reg = <0x2430000 0x19100>; + status = "okay";Why? Anything disabled it?
Quoted 2 lines
arch/arm64/boot/dts/nvidia/tegra234.dtsi +0
-0};
Quoted 14 lines
arch/arm64/boot/dts/nvidia/tegra234.dtsi line 1680
Quoted 12 lines
arch/arm64/boot/dts/nvidia/tegra234.dtsi +7
-0mc: memory-controller@2c00000 {@@ -1664,6 +1671,13 @@ interrupt-controller; #gpio-cells = <2>; gpio-controller; + gpio-range = <&pinmux_aon 0 0 32>; + }; + + pinmux_aon: pinmux@c300000 { + compatible = "nvidia,tegra234-pinmux-aon"; + reg = <0xc300000 0x4000>; + status = "okay";Also why?
These are probably copy-pasted from Tegra194 where these snuck in. I can drop those when applying. I'll also prepare a patch to drop these from the tegra194.dtsi.
I wonder if there's a good way to detect these. We'd have to run checks on the DT source files, so that's a bit difficult. I do have an experimental script that tries to capture some common pitfalls on sources but it's quite ugly and slow, but I guess I could add something like this. But perhaps there are better ways?
Thierry
Re: [PATCH 3/3] arm64: tegra: Add Tegra234 pinmux device
Quoted 48 lines · Thierry Reding
Quoted 28 lines · Krzysztof Kozlowski
Quoted 12 lines · Prathamesh Shete
This change adds pinmux node for Tegra234.
Signed-off-by Prathamesh Shete <[email protected]>--- arch/arm64/boot/dts/nvidia/tegra234.dtsi | 14 ++++++++++++++ 1 file changed, 14 insertions(+)arch/arm64/boot/dts/nvidia/tegra234.dtsi +0
-0arch/arm64/boot/dts/nvidia/tegra234.dtsi line 710
Quoted 10 lines
arch/arm64/boot/dts/nvidia/tegra234.dtsi +7
-0@@ -701,6 +701,13 @@ interrupt-controller; #gpio-cells = <2>; gpio-controller; + gpio-ranges = <&pinmux 0 0 164>; + }; + + pinmux: pinmux@2430000 { + compatible = "nvidia,tegra234-pinmux"; + reg = <0x2430000 0x19100>; + status = "okay";Why? Anything disabled it?
Quoted 2 lines
arch/arm64/boot/dts/nvidia/tegra234.dtsi +0
-0};Quoted 14 lines
arch/arm64/boot/dts/nvidia/tegra234.dtsi line 1680
Quoted 12 lines
arch/arm64/boot/dts/nvidia/tegra234.dtsi +7
-0mc: memory-controller@2c00000 {@@ -1664,6 +1671,13 @@ interrupt-controller; #gpio-cells = <2>; gpio-controller; + gpio-range = <&pinmux_aon 0 0 32>; + }; + + pinmux_aon: pinmux@c300000 { + compatible = "nvidia,tegra234-pinmux-aon"; + reg = <0xc300000 0x4000>; + status = "okay";Also why?
These are probably copy-pasted from Tegra194 where these snuck in. I can drop those when applying. I'll also prepare a patch to drop these from the
tegra194.dtsi.
Quoted 5 lines
I wonder if there's a good way to detect these. We'd have to run checks on the DT source files, so that's a bit difficult. I do have an experimental script that tries to capture some common pitfalls on sources but it's quite ugly and slow, but I guess I could add something like this. But perhaps there are better ways?
One way to easy spot them is to override always by label, thus every node defined like above is a new node. However I think we talked about this and you do not follow this practice, thus there is no way to tell - is the status reasonable or not.
Automated tools could help here as well - run fdtdump on DTB and look for status=okay.
Best regards,
Krzysztof
Re: [PATCH 3/3] arm64: tegra: Add Tegra234 pinmux device
Quoted 6 lines · Krzysztof Kozlowski
Quoted 5 lines
I wonder if there's a good way to detect these. We'd have to run checks on the DT source files, so that's a bit difficult. I do have an experimental script that tries to capture some common pitfalls on sources but it's quite ugly and slow, but I guess I could add something like this. But perhaps there are better ways?
Quoted 7 lines
One way to easy spot them is to override always by label, thus every node defined like above is a new node. However I think we talked about this and you do not follow this practice, thus there is no way to tell - is the status reasonable or not.
Automated tools could help here as well - run fdtdump on DTB and look for status=okay.
Eh, obviously it won't work - every node which was disabled in DTSI and enabled in DTS will have the status=okay...
Best regards,
Krzysztof
Re: [PATCH 3/3] arm64: tegra: Add Tegra234 pinmux device
Quoted 12 lines · Krzysztof Kozlowski
Quoted 11 lines · Krzysztof Kozlowski
Quoted 5 lines
I wonder if there's a good way to detect these. We'd have to run checks on the DT source files, so that's a bit difficult. I do have an experimental script that tries to capture some common pitfalls on sources but it's quite ugly and slow, but I guess I could add something like this. But perhaps there are better ways?
One way to easy spot them is to override always by label, thus every node defined like above is a new node. However I think we talked about this and you do not follow this practice, thus there is no way to tell - is the status reasonable or not.
Quoted 5 lines
Quoted 2 lines
Automated tools could help here as well - run fdtdump on DTB and look for status=okay.
Eh, obviously it won't work - every node which was disabled in DTSI and enabled in DTS will have the status=okay...
Yeah, I was originally thinking along the same lines, but when things are overridden that check no longer works. I suppose DTC could be taught to check for this when it merges nodes.
Thierry
Rob Herring
maintainerQuoted 8 lines · Prathamesh Shete
Add DT binding doc for Tegra234 pinmux driver.
Signed-off-by Prathamesh Shete <[email protected]>--- .../pinctrl/nvidia,tegra234-pinmux.yaml | 232 ++++++++++++++++++ 1 file changed, 232 insertions(+) create mode 100644 Documentation/devicetree/bindings/pinctrl/nvidia,tegra234-pinmux.yaml
My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' on your patch (DT_CHECKER_FLAGS is new in v5.13):
yamllint warnings/errors:
dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/pinctrl/nvidia,tegra234-pinmux.example.dtb: pinmux@2430000: pinmux-pex-rst-c5-out:pex_rst:nvidia,pins:0: 'pex_l5_rst_n_pgg1' is not one of ['dap6_sclk_pa0', 'dap6_dout_pa1', 'dap6_din_pa2', 'dap6_fs_pa3', 'dap4_sclk_pa4', 'dap4_dout_pa5', 'dap4_din_pa6', 'dap4_fs_pa7', 'soc_gpio08_pb0', 'qspi0_sck_pc0', 'qspi0_cs_n_pc1', 'qspi0_io0_pc2', 'qspi0_io1_pc3', 'qspi0_io2_pc4', 'qspi0_io3_pc5', 'qspi1_sck_pc6', 'qspi1_cs_n_pc7', 'qspi1_io0_pd0', 'qspi1_io1_pd1', 'qspi1_io2_pd2', 'qspi1_io3_pd3', 'eqos_txc_pe0', 'eqos_td0_pe1', 'eqos_td1_pe2', 'eqos_td2_pe3', 'eqos_td3_pe4', 'eqos_tx_ctl_pe5', 'eqos_rd0_pe6', 'eqos_rd1_pe7', 'eqos_rd2_pf0', 'eqos_rd3_pf1', 'eqos_rx_ctl_pf2', 'eqos_rxc_pf3', 'eqos_sma_mdio_pf4', 'eqos_sma_mdc_pf5', 'soc_gpio13_pg0', 'soc_gpio14_pg1', 'soc_gpio15_pg2', 'soc_gpio16_pg3', 'soc_gpio17_pg4', 'soc_gpio18_pg5', 'soc_gpio19_pg6', 'soc_gpio20_pg7', 'soc_gpio21_ph0', 'soc_gpio22_ph1', 'soc_gpio06_p
h2', 'uart4_tx_ph3', 'uart4_rx_ph4', 'uart4_rts_ph5', 'uart4_cts_ph6', 'soc_gpio41_ph7', 'soc_gpio42_pi0', 'soc_gpio43_pi1', 'soc_gpio44_pi2', 'gen1_i2c_scl_pi3', 'gen1_i2c_sda_pi4', 'cpu_pwr_req_pi5', 'soc_gpio07_pi6', 'sdmmc1_clk_pj0', 'sdmmc1_cmd_pj1', 'sdmmc1_dat0_pj2', 'sdmmc1_dat1_pj3', 'sdmmc1_dat2_pj4', 'sdmmc1_dat3_pj5', 'pex_l0_clkreq_n_pk0', 'pex_l0_rst_n_pk1', 'pex_l1_clkreq_n_pk2', 'pex_l1_rst_n_pk3', 'pex_l2_clkreq_n_pk4', 'pex_l2_rst_n_pk5', 'pex_l3_clkreq_n_pk6', 'pex_l3_rst_n_pk7', 'pex_l4_clkreq_n_pl0', 'pex_l4_rst_n_pl1', 'pex_wake_n_pl2', 'soc_gpio34_pl3', 'dp_aux_ch0_hpd_pm0', 'dp_aux_ch1_hpd_pm1', 'dp_aux_ch2_hpd_pm2', 'dp_aux_ch3_hpd_pm3', 'soc_gpio55_pm4', 'soc_gpio36_pm5', 'soc_gpio53_pm6', 'soc_gpio38_pm7', 'dp_aux_ch3_n_pn0', 'soc_gpio39_pn1', 'soc_gpio40_pn2', 'dp_aux_ch1_p_pn3', 'dp_aux_ch1_n_pn4', 'dp_aux_ch2_p_pn5', 'dp_aux_ch2_n_pn6', 'dp_aux_ch3_p_pn7', 'extperiph1_clk_pp0', 'extperiph2_clk_pp1', 'cam_i2c_scl_pp2', 'cam_i2c_sda_pp3', 'soc_gpio23_pp4' , 'soc_gpio24_pp5', 'soc_gpio25_pp6', 'pwr_i2c_scl_pp7', 'pwr_i2c_sda_pq0', 'soc_gpio28_pq1', 'soc_gpio29_pq2', 'soc_gpio30_pq3', 'soc_gpio31_pq4', 'soc_gpio32_pq5', 'soc_gpio33_pq6', 'soc_gpio35_pq7', 'soc_gpio37_pr0', 'soc_gpio56_pr1', 'uart1_tx_pr2', 'uart1_rx_pr3', 'uart1_rts_pr4', 'uart1_cts_pr5', 'soc_gpio61_pw0', 'soc_gpio62_pw1', 'gpu_pwr_req_px0', 'cv_pwr_req_px1', 'gp_pwm2_px2', 'gp_pwm3_px3', 'uart2_tx_px4', 'uart2_rx_px5', 'uart2_rts_px6', 'uart2_cts_px7', 'spi3_sck_py0', 'spi3_miso_py1', 'spi3_mosi_py2', 'spi3_cs0_py3', 'spi3_cs1_py4', 'uart5_tx_py5', 'uart5_rx_py6', 'uart5_rts_py7', 'uart5_cts_pz0', 'usb_vbus_en0_pz1', 'usb_vbus_en1_pz2', 'spi1_sck_pz3', 'spi1_miso_pz4', 'spi1_mosi_pz5', 'spi1_cs0_pz6', 'spi1_cs1_pz7', 'spi5_sck_pac0', 'spi5_miso_pac1', 'spi5_mosi_pac2', 'spi5_cs0_pac3', 'soc_gpio57_pac4', 'soc_gpio58_pac5', 'soc_gpio59_pac6', 'soc_gpio60_pac7', 'soc_gpio45_pad0', 'soc_gpio46_pad1', 'soc_gpio47_pad2', 'soc_gpio48_pad3', 'ufs0_ref_clk_pae0', 'ufs0_rst_n _pae1', 'pex_l5_clkreq_n_paf0', 'pex_l5_rst_n_paf1', 'pex_l6_clkreq_n_paf2', 'pex_l6_rst_n_paf3', 'pex_l7_clkreq_n_pag0', 'pex_l7_rst_n_pag1', 'pex_l8_clkreq_n_pag2', 'pex_l8_rst_n_pag3', 'pex_l9_clkreq_n_pag4', 'pex_l9_rst_n_pag5', 'pex_l10_clkreq_n_pag6', 'pex_l10_rst_n_pag7', 'sdmmc1_comp', 'eqos_comp', 'qspi_comp', 'drive_soc_gpio08_pb0', 'drive_soc_gpio36_pm5', 'drive_soc_gpio53_pm6', 'drive_soc_gpio55_pm4', 'drive_soc_gpio38_pm7', 'drive_soc_gpio39_pn1', 'drive_soc_gpio40_pn2', 'drive_dp_aux_ch0_hpd_pm0', 'drive_dp_aux_ch1_hpd_pm1', 'drive_dp_aux_ch2_hpd_pm2', 'drive_dp_aux_ch3_hpd_pm3', 'drive_dp_aux_ch1_p_pn3', 'drive_dp_aux_ch1_n_pn4', 'drive_dp_aux_ch2_p_pn5', 'drive_dp_aux_ch2_n_pn6', 'drive_dp_aux_ch3_p_pn7', 'drive_dp_aux_ch3_n_pn0', 'drive_pex_l2_clkreq_n_pk4', 'drive_pex_wake_n_pl2', 'drive_pex_l1_clkreq_n_pk2', 'drive_pex_l1_rst_n_pk3', 'drive_pex_l0_clkreq_n_pk0', 'drive_pex_l0_rst_n_pk1', 'drive_pex_l2_rst_n_pk5', 'drive_pex_l3_clkreq_n_pk6', 'drive_pex_l3_rst_n_pk 7', 'drive_pex_l4_clkreq_n_pl0', 'drive_pex_l4_rst_n_pl1', 'drive_soc_gpio34_pl3', 'drive_pex_l5_clkreq_n_paf0', 'drive_pex_l5_rst_n_paf1', 'drive_pex_l6_clkreq_n_paf2', 'drive_pex_l6_rst_n_paf3', 'drive_pex_l10_clkreq_n_pag6', 'drive_pex_l10_rst_n_pag7', 'drive_pex_l7_clkreq_n_pag0', 'drive_pex_l7_rst_n_pag1', 'drive_pex_l8_clkreq_n_pag2', 'drive_pex_l8_rst_n_pag3', 'drive_pex_l9_clkreq_n_pag4', 'drive_pex_l9_rst_n_pag5', 'drive_sdmmc1_clk_pj0', 'drive_sdmmc1_cmd_pj1', 'drive_sdmmc1_dat3_pj5', 'drive_sdmmc1_dat2_pj4', 'drive_sdmmc1_dat1_pj3', 'drive_sdmmc1_dat0_pj2']
From schema: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/pinctrl/nvidia,tegra234-pinmux.yaml
doc reference errors (make refcheckdocs):
See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/[email protected]
The base for the series is generally the latest rc1. A different dependency should be noted in *this* patch.
If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date:
pip3 install dtschema --upgrade
Please check and re-submit after running the above command yourself. Note that DT_SCHEMA_FILES can be set to your schema file to speed up checking your schema. However, it must be unset to test all examples with your schema.
Quoted 11 lines · Rob Herring
Quoted 8 lines · Prathamesh Shete
Add DT binding doc for Tegra234 pinmux driver.
Signed-off-by Prathamesh Shete <[email protected]>--- .../pinctrl/nvidia,tegra234-pinmux.yaml | 232 ++++++++++++++++++ 1 file changed, 232 insertions(+) create mode 100644 Documentation/devicetree/bindings/pinctrl/nvidia,tegra234-pinmux.yaml
Quoted 7 lines
My bot found errors running 'make DT_CHECKER_FLAGS=-m
dt_binding_check' on your patch (DT_CHECKER_FLAGSis new in v5.13):yamllint warnings/errors:
dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/pinctrl/nvidia,tegra234-pinmux.example.dtb: pinmux@2430000: pinmux-pex-rst-c5-out:pex_rst:nvidia,pins:0: 'pex_l5_rst_n_pgg1' is not one of
[...]
Prathamesh,
Looks like maybe you copied the example from Tegra194. On Tegra234 I think the corresponding GPIO is AF1, so the full pin name for this is pex_l5_rst_n_paf1.
Please fix and make sure to run the DT binding check before sending out v2.
Thierry
Quoted 1 line · Prathamesh Shete
Add DT binding doc for Tegra234 pinmux driver.
Subject: drop second/last, redundant "DT binding doc". The "dt-bindings" prefix is already stating that these are bindings. Which basically means your subject is quite empty...
Quoted 27 lines
Signed-off-by Prathamesh Shete <[email protected]>--- .../pinctrl/nvidia,tegra234-pinmux.yaml | 232 ++++++++++++++++++ 1 file changed, 232 insertions(+) create mode 100644 Documentation/devicetree/bindings/pinctrl/nvidia,tegra234-pinmux.yamlDocumentation/devicetree/bindings/pinctrl/nvidia,tegra234-pinmux.yaml new file +14
-0@@ -0,0 +1,232 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/pinctrl/nvidia,tegra234-pinmux.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: NVIDIA Tegra234 Pinmux Controller + +maintainers: + - Thierry Reding <[email protected]> + - Jon Hunter <[email protected]> + +properties: + compatible:
Documentation/devicetree/bindings/pinctrl/nvidia,tegra234-pinmux.yaml line 24
Quoted 10 lines
Documentation/devicetree/bindings/pinctrl/nvidia,tegra234-pinmux.yaml new file +10
-0+ enum: + - nvidia,tegra234-pinmux + - nvidia,tegra234-pinmux-aon + + reg: + items: + - description: pinmux registers + +patternProperties: + "^pinmux(-[a-z0-9-_]+)?$":
Underscore is not a valid character
Quoted 33 lines
Documentation/devicetree/bindings/pinctrl/nvidia,tegra234-pinmux.yaml new file +33
-0+ type: object + properties: + phandle: true + + # pin groups + additionalProperties: + $ref: nvidia,tegra-pinmux-common.yaml + unevaluatedProperties: false + properties: + nvidia,function: + enum: [ gp, uartc, i2c8, spi2, i2c2, can1, can0, rsvd0, eth0, eth2, + eth1, dp, eth3, i2c4, i2c7, i2c9, eqos, pe2, pe1, pe0, pe3, + pe4, pe5, pe6, pe7, pe8, pe9, pe10, qspi0, qspi1, qpsi, + sdmmc1, sce, soc, gpio, hdmi, ufs0, spi3, spi1, uartb, uarte, + usb, extperiph2, extperiph1, i2c3, vi0, i2c5, uarta, uartd, + i2c1, i2s4, i2s6, aud, spi5, touch, uartj, rsvd1, wdt, tsc, + dmic3, led, vi0_alt, i2s5, nv, extperiph3, extperiph4, spi4, + ccla, i2s1, i2s2, i2s3, i2s8, rsvd2, dmic5, dca, displayb, + displaya, vi1, dcb, dmic1, dmic4, i2s7, dmic2, dspk0, rsvd3, + tsc_alt, istctrl, vi1_alt, dspk1, igpu ] + + nvidia,pull: true + nvidia,tristate: true + nvidia,schmitt: true + nvidia,enable-input: true + nvidia,open-drain: true + nvidia,lock: true + nvidia,drive-type: true + nvidia,io-hv: true + + required: + - nvidia,pins +
Documentation/devicetree/bindings/pinctrl/nvidia,tegra234-pinmux.yaml line 67
Quoted 10 lines
Documentation/devicetree/bindings/pinctrl/nvidia,tegra234-pinmux.yaml new file +10
-0+additionalProperties: false + +allOf: + - if: + properties: + compatible: + const: nvidia,tegra234-pinmux + then: + patternProperties: + "^pinmux(-[a-z0-9-_]+)?$":
Underscore is not a valid character
Documentation/devicetree/bindings/pinctrl/nvidia,tegra234-pinmux.yaml line 74
Quoted 7 lines
Documentation/devicetree/bindings/pinctrl/nvidia,tegra234-pinmux.yaml new file +7
-0+ type: object + additionalProperties: + properties: + nvidia,pins: + description: An array of strings. Each string contains the name + of a pin or group. Valid values for these names are listed + below.
Define properties in top level, which points to the complexity of your if-else, thus probably this should be split into two bindings. Dunno, your other bindings repeat this pattern :(
Quoted 86 lines
Documentation/devicetree/bindings/pinctrl/nvidia,tegra234-pinmux.yaml new file +86
-0+ + items: + enum: [ dap6_sclk_pa0, dap6_dout_pa1, dap6_din_pa2, + dap6_fs_pa3, dap4_sclk_pa4, dap4_dout_pa5, + dap4_din_pa6, dap4_fs_pa7, soc_gpio08_pb0, + qspi0_sck_pc0, qspi0_cs_n_pc1, + qspi0_io0_pc2, qspi0_io1_pc3, qspi0_io2_pc4, + qspi0_io3_pc5, qspi1_sck_pc6, qspi1_cs_n_pc7, + qspi1_io0_pd0, qspi1_io1_pd1, qspi1_io2_pd2, + qspi1_io3_pd3, eqos_txc_pe0, eqos_td0_pe1, + eqos_td1_pe2, eqos_td2_pe3, eqos_td3_pe4, + eqos_tx_ctl_pe5, eqos_rd0_pe6, eqos_rd1_pe7, + eqos_rd2_pf0, eqos_rd3_pf1, eqos_rx_ctl_pf2, + eqos_rxc_pf3, eqos_sma_mdio_pf4, eqos_sma_mdc_pf5, + soc_gpio13_pg0, soc_gpio14_pg1, soc_gpio15_pg2, + soc_gpio16_pg3, soc_gpio17_pg4, soc_gpio18_pg5, + soc_gpio19_pg6, soc_gpio20_pg7, soc_gpio21_ph0, + soc_gpio22_ph1, soc_gpio06_ph2, uart4_tx_ph3, + uart4_rx_ph4, uart4_rts_ph5, uart4_cts_ph6, + soc_gpio41_ph7, soc_gpio42_pi0, soc_gpio43_pi1, + soc_gpio44_pi2, gen1_i2c_scl_pi3, gen1_i2c_sda_pi4, + cpu_pwr_req_pi5, soc_gpio07_pi6, + sdmmc1_clk_pj0, sdmmc1_cmd_pj1, sdmmc1_dat0_pj2, + sdmmc1_dat1_pj3, sdmmc1_dat2_pj4, sdmmc1_dat3_pj5, + pex_l0_clkreq_n_pk0, pex_l0_rst_n_pk1, + pex_l1_clkreq_n_pk2, pex_l1_rst_n_pk3, + pex_l2_clkreq_n_pk4, pex_l2_rst_n_pk5, + pex_l3_clkreq_n_pk6, pex_l3_rst_n_pk7, + pex_l4_clkreq_n_pl0, pex_l4_rst_n_pl1, + pex_wake_n_pl2, soc_gpio34_pl3, dp_aux_ch0_hpd_pm0, + dp_aux_ch1_hpd_pm1, dp_aux_ch2_hpd_pm2, + dp_aux_ch3_hpd_pm3, soc_gpio55_pm4, soc_gpio36_pm5, + soc_gpio53_pm6, soc_gpio38_pm7, dp_aux_ch3_n_pn0, + soc_gpio39_pn1, soc_gpio40_pn2, dp_aux_ch1_p_pn3, + dp_aux_ch1_n_pn4, dp_aux_ch2_p_pn5, dp_aux_ch2_n_pn6, + dp_aux_ch3_p_pn7, extperiph1_clk_pp0, + extperiph2_clk_pp1, cam_i2c_scl_pp2, cam_i2c_sda_pp3, + soc_gpio23_pp4, soc_gpio24_pp5, soc_gpio25_pp6, + pwr_i2c_scl_pp7, pwr_i2c_sda_pq0, soc_gpio28_pq1, + soc_gpio29_pq2, soc_gpio30_pq3, soc_gpio31_pq4, + soc_gpio32_pq5, soc_gpio33_pq6, soc_gpio35_pq7, + soc_gpio37_pr0, soc_gpio56_pr1, uart1_tx_pr2, + uart1_rx_pr3, uart1_rts_pr4, uart1_cts_pr5, + soc_gpio61_pw0, soc_gpio62_pw1, gpu_pwr_req_px0, cv_pwr_req_px1, + gp_pwm2_px2, gp_pwm3_px3, uart2_tx_px4, uart2_rx_px5, + uart2_rts_px6, uart2_cts_px7, spi3_sck_py0, + spi3_miso_py1, spi3_mosi_py2, spi3_cs0_py3, + spi3_cs1_py4, uart5_tx_py5, uart5_rx_py6, + uart5_rts_py7, uart5_cts_pz0, usb_vbus_en0_pz1, + usb_vbus_en1_pz2, spi1_sck_pz3, spi1_miso_pz4, + spi1_mosi_pz5, spi1_cs0_pz6, spi1_cs1_pz7, + spi5_sck_pac0, spi5_miso_pac1, spi5_mosi_pac2, + spi5_cs0_pac3, soc_gpio57_pac4, soc_gpio58_pac5, + soc_gpio59_pac6, soc_gpio60_pac7, soc_gpio45_pad0, + soc_gpio46_pad1, soc_gpio47_pad2, soc_gpio48_pad3, + ufs0_ref_clk_pae0, ufs0_rst_n_pae1, + pex_l5_clkreq_n_paf0, pex_l5_rst_n_paf1, + pex_l6_clkreq_n_paf2, pex_l6_rst_n_paf3, + pex_l7_clkreq_n_pag0, pex_l7_rst_n_pag1, + pex_l8_clkreq_n_pag2, pex_l8_rst_n_pag3, + pex_l9_clkreq_n_pag4, pex_l9_rst_n_pag5, + pex_l10_clkreq_n_pag6, pex_l10_rst_n_pag7, + sdmmc1_comp, eqos_comp, qspi_comp, + # drive groups + drive_soc_gpio08_pb0, drive_soc_gpio36_pm5, + drive_soc_gpio53_pm6, drive_soc_gpio55_pm4, + drive_soc_gpio38_pm7, drive_soc_gpio39_pn1, + drive_soc_gpio40_pn2, drive_dp_aux_ch0_hpd_pm0, + drive_dp_aux_ch1_hpd_pm1, drive_dp_aux_ch2_hpd_pm2, + drive_dp_aux_ch3_hpd_pm3, drive_dp_aux_ch1_p_pn3, + drive_dp_aux_ch1_n_pn4, drive_dp_aux_ch2_p_pn5, + drive_dp_aux_ch2_n_pn6, drive_dp_aux_ch3_p_pn7, + drive_dp_aux_ch3_n_pn0, drive_pex_l2_clkreq_n_pk4, + drive_pex_wake_n_pl2, drive_pex_l1_clkreq_n_pk2, + drive_pex_l1_rst_n_pk3, drive_pex_l0_clkreq_n_pk0, + drive_pex_l0_rst_n_pk1, drive_pex_l2_rst_n_pk5, + drive_pex_l3_clkreq_n_pk6, drive_pex_l3_rst_n_pk7, + drive_pex_l4_clkreq_n_pl0, drive_pex_l4_rst_n_pl1, + drive_soc_gpio34_pl3, drive_pex_l5_clkreq_n_paf0, + drive_pex_l5_rst_n_paf1, drive_pex_l6_clkreq_n_paf2, + drive_pex_l6_rst_n_paf3, drive_pex_l10_clkreq_n_pag6, + drive_pex_l10_rst_n_pag7, drive_pex_l7_clkreq_n_pag0, + drive_pex_l7_rst_n_pag1, drive_pex_l8_clkreq_n_pag2, + drive_pex_l8_rst_n_pag3, drive_pex_l9_clkreq_n_pag4, + drive_pex_l9_rst_n_pag5, drive_sdmmc1_clk_pj0, + drive_sdmmc1_cmd_pj1, drive_sdmmc1_dat3_pj5,
Documentation/devicetree/bindings/pinctrl/nvidia,tegra234-pinmux.yaml line 170
Quoted 10 lines
Documentation/devicetree/bindings/pinctrl/nvidia,tegra234-pinmux.yaml new file +10
-0+ drive_sdmmc1_dat2_pj4, drive_sdmmc1_dat1_pj3, + drive_sdmmc1_dat0_pj2 ] + + - if: + properties: + compatible: + const: nvidia,tegra234-pinmux-aon + then: + patternProperties: + "^pinmux(-[a-z0-9-_]+)?$":
No underscores
Quoted 42 lines
Documentation/devicetree/bindings/pinctrl/nvidia,tegra234-pinmux.yaml new file +42
-0+ type: object + additionalProperties: + properties: + nvidia,pins: + items: + enum: [ can0_dout_paa0, can0_din_paa1, can1_dout_paa2, + can1_din_paa3, can0_stb_paa4, can0_en_paa5, + soc_gpio49_paa6, can0_err_paa7, can1_stb_pbb0, + can1_en_pbb1, soc_gpio50_pbb2, can1_err_pbb3, + spi2_sck_pcc0, spi2_miso_pcc1, spi2_mosi_pcc2, + spi2_cs0_pcc3, touch_clk_pcc4, uart3_tx_pcc5, + uart3_rx_pcc6, gen2_i2c_scl_pcc7, gen2_i2c_sda_pdd0, + gen8_i2c_scl_pdd1, gen8_i2c_sda_pdd2, + sce_error_pee0, vcomp_alert_pee1, + ao_retention_n_pee2, batt_oc_pee3, power_on_pee4, + soc_gpio26_pee5, soc_gpio27_pee6, bootv_ctl_n_pee7, + hdmi_cec_pgg0, + # drive groups + drive_touch_clk_pcc4, drive_uart3_rx_pcc6, + drive_uart3_tx_pcc5, drive_gen8_i2c_sda_pdd2, + drive_gen8_i2c_scl_pdd1, drive_spi2_mosi_pcc2, + drive_gen2_i2c_scl_pcc7, drive_spi2_cs0_pcc3, + drive_gen2_i2c_sda_pdd0, drive_spi2_sck_pcc0, + drive_spi2_miso_pcc1, drive_can1_dout_paa2, + drive_can1_din_paa3, drive_can0_dout_paa0, + drive_can0_din_paa1, drive_can0_stb_paa4, + drive_can0_en_paa5, drive_soc_gpio49_paa6, + drive_can0_err_paa7, drive_can1_stb_pbb0, + drive_can1_en_pbb1, drive_soc_gpio50_pbb2, + drive_can1_err_pbb3, drive_sce_error_pee0, + drive_batt_oc_pee3, drive_bootv_ctl_n_pee7, + drive_power_on_pee4, drive_soc_gpio26_pee5, + drive_soc_gpio27_pee6, drive_ao_retention_n_pee2, + drive_vcomp_alert_pee1, drive_hdmi_cec_pgg0 ] + +required: + - compatible + - reg + +examples: + - | + #include <dt-bindings/pinctrl/pinctrl-tegra.h>
Documentation/devicetree/bindings/pinctrl/nvidia,tegra234-pinmux.yaml line 222
Quoted 10 lines
Documentation/devicetree/bindings/pinctrl/nvidia,tegra234-pinmux.yaml new file +10
-0+ + pinmux@2430000 { + compatible = "nvidia,tegra234-pinmux"; + reg = <0x2430000 0x17000>; + + pinctrl-names = "pex_rst"; + pinctrl-0 = <&pex_rst_c5_out_state>; + + pex_rst_c5_out_state: pinmux-pex-rst-c5-out { + pex_rst {
Underscores are not valid in node names.
Best regards,
Krzysztof
Quoted 38 lines · Krzysztof Kozlowski
Quoted 1 line · Prathamesh Shete
Add DT binding doc for Tegra234 pinmux driver.
Subject: drop second/last, redundant "DT binding doc". The "dt-bindings" prefix is already stating that these are bindings. Which basically means your subject is quite empty...
Quoted 31 lines
Signed-off-by Prathamesh Shete <[email protected]>--- .../pinctrl/nvidia,tegra234-pinmux.yaml | 232 ++++++++++++++++++ 1 file changed, 232 insertions(+) create mode 100644 Documentation/devicetree/bindings/pinctrl/nvidia,tegra234-pinmux.yamlDocumentation/devicetree/bindings/pinctrl/nvidia,tegra234-pinmux.yaml new file +18
-0@@ -0,0 +1,232 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/pinctrl/nvidia,tegra234-pinmux.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: NVIDIA Tegra234 Pinmux Controller + +maintainers: + - Thierry Reding <[email protected]> + - Jon Hunter <[email protected]> + +properties: + compatible: + enum: + - nvidia,tegra234-pinmux + - nvidia,tegra234-pinmux-aon +
Quoted 8 lines
Documentation/devicetree/bindings/pinctrl/nvidia,tegra234-pinmux.yaml line 24
Quoted 6 lines
Documentation/devicetree/bindings/pinctrl/nvidia,tegra234-pinmux.yaml new file +6
-0+ reg: + items: + - description: pinmux registers + +patternProperties: + "^pinmux(-[a-z0-9-_]+)?$":Underscore is not a valid character
This is again the same as for Tegra194. I don't think we use underscores in any pinmux node names at the moment, so we should be able to remove that one.
Quoted 55 lines
Quoted 34 lines
Documentation/devicetree/bindings/pinctrl/nvidia,tegra234-pinmux.yaml new file +33
-0+ type: object + properties: + phandle: true + + # pin groups + additionalProperties: + $ref: nvidia,tegra-pinmux-common.yaml + unevaluatedProperties: false + properties: + nvidia,function: + enum: [ gp, uartc, i2c8, spi2, i2c2, can1, can0, rsvd0, eth0, eth2, + eth1, dp, eth3, i2c4, i2c7, i2c9, eqos, pe2, pe1, pe0, pe3, + pe4, pe5, pe6, pe7, pe8, pe9, pe10, qspi0, qspi1, qpsi, + sdmmc1, sce, soc, gpio, hdmi, ufs0, spi3, spi1, uartb, uarte, + usb, extperiph2, extperiph1, i2c3, vi0, i2c5, uarta, uartd, + i2c1, i2s4, i2s6, aud, spi5, touch, uartj, rsvd1, wdt, tsc, + dmic3, led, vi0_alt, i2s5, nv, extperiph3, extperiph4, spi4, + ccla, i2s1, i2s2, i2s3, i2s8, rsvd2, dmic5, dca, displayb, + displaya, vi1, dcb, dmic1, dmic4, i2s7, dmic2, dspk0, rsvd3, + tsc_alt, istctrl, vi1_alt, dspk1, igpu ] + + nvidia,pull: true + nvidia,tristate: true + nvidia,schmitt: true + nvidia,enable-input: true + nvidia,open-drain: true + nvidia,lock: true + nvidia,drive-type: true + nvidia,io-hv: true + + required: + - nvidia,pins +Documentation/devicetree/bindings/pinctrl/nvidia,tegra234-pinmux.yaml line 67
Quoted 10 lines
Documentation/devicetree/bindings/pinctrl/nvidia,tegra234-pinmux.yaml new file +10
-0+additionalProperties: false + +allOf: + - if: + properties: + compatible: + const: nvidia,tegra234-pinmux + then: + patternProperties: + "^pinmux(-[a-z0-9-_]+)?$":Underscore is not a valid character
Documentation/devicetree/bindings/pinctrl/nvidia,tegra234-pinmux.yaml line 74
Quoted 7 lines
Documentation/devicetree/bindings/pinctrl/nvidia,tegra234-pinmux.yaml new file +7
-0+ type: object + additionalProperties: + properties: + nvidia,pins: + description: An array of strings. Each string contains the name + of a pin or group. Valid values for these names are listed + below.
Quoted 3 lines
Define properties in top level, which points to the complexity of your if-else, thus probably this should be split into two bindings. Dunno, your other bindings repeat this pattern :(
The property itself is already defined in the common schema found in nvidia,tegra-pinmux-common.yaml and we're overriding this here for each instance since each has its own set of pins.
This was a compromise to avoid too many bindings. Originally I attempted to roll all Tegra pinctrl bindings into a single dt-schema, but that turned out truly horrible =) Splitting this into per-SoC bindings is already causing a lot of duplication in these files, though splitting off the common bits into nvidi,tegra-pinmux-common.yaml helps a bit with that already. Splitting this into per-instance bindings would effectively duplicate everything but the pin array here, so we kind of settled on this compromise for Tegra194.
We're taking a bit of a shortcut here already, since technically not all pins support all the functions listed above. On the other hand, fully accurately describing per-pin functions would make this a total mess, so again, we use this simplified representation as a compromise.
Quoted 146 lines
Quoted 87 lines
Documentation/devicetree/bindings/pinctrl/nvidia,tegra234-pinmux.yaml new file +86
-0+ + items: + enum: [ dap6_sclk_pa0, dap6_dout_pa1, dap6_din_pa2, + dap6_fs_pa3, dap4_sclk_pa4, dap4_dout_pa5, + dap4_din_pa6, dap4_fs_pa7, soc_gpio08_pb0, + qspi0_sck_pc0, qspi0_cs_n_pc1, + qspi0_io0_pc2, qspi0_io1_pc3, qspi0_io2_pc4, + qspi0_io3_pc5, qspi1_sck_pc6, qspi1_cs_n_pc7, + qspi1_io0_pd0, qspi1_io1_pd1, qspi1_io2_pd2, + qspi1_io3_pd3, eqos_txc_pe0, eqos_td0_pe1, + eqos_td1_pe2, eqos_td2_pe3, eqos_td3_pe4, + eqos_tx_ctl_pe5, eqos_rd0_pe6, eqos_rd1_pe7, + eqos_rd2_pf0, eqos_rd3_pf1, eqos_rx_ctl_pf2, + eqos_rxc_pf3, eqos_sma_mdio_pf4, eqos_sma_mdc_pf5, + soc_gpio13_pg0, soc_gpio14_pg1, soc_gpio15_pg2, + soc_gpio16_pg3, soc_gpio17_pg4, soc_gpio18_pg5, + soc_gpio19_pg6, soc_gpio20_pg7, soc_gpio21_ph0, + soc_gpio22_ph1, soc_gpio06_ph2, uart4_tx_ph3, + uart4_rx_ph4, uart4_rts_ph5, uart4_cts_ph6, + soc_gpio41_ph7, soc_gpio42_pi0, soc_gpio43_pi1, + soc_gpio44_pi2, gen1_i2c_scl_pi3, gen1_i2c_sda_pi4, + cpu_pwr_req_pi5, soc_gpio07_pi6, + sdmmc1_clk_pj0, sdmmc1_cmd_pj1, sdmmc1_dat0_pj2, + sdmmc1_dat1_pj3, sdmmc1_dat2_pj4, sdmmc1_dat3_pj5, + pex_l0_clkreq_n_pk0, pex_l0_rst_n_pk1, + pex_l1_clkreq_n_pk2, pex_l1_rst_n_pk3, + pex_l2_clkreq_n_pk4, pex_l2_rst_n_pk5, + pex_l3_clkreq_n_pk6, pex_l3_rst_n_pk7, + pex_l4_clkreq_n_pl0, pex_l4_rst_n_pl1, + pex_wake_n_pl2, soc_gpio34_pl3, dp_aux_ch0_hpd_pm0, + dp_aux_ch1_hpd_pm1, dp_aux_ch2_hpd_pm2, + dp_aux_ch3_hpd_pm3, soc_gpio55_pm4, soc_gpio36_pm5, + soc_gpio53_pm6, soc_gpio38_pm7, dp_aux_ch3_n_pn0, + soc_gpio39_pn1, soc_gpio40_pn2, dp_aux_ch1_p_pn3, + dp_aux_ch1_n_pn4, dp_aux_ch2_p_pn5, dp_aux_ch2_n_pn6, + dp_aux_ch3_p_pn7, extperiph1_clk_pp0, + extperiph2_clk_pp1, cam_i2c_scl_pp2, cam_i2c_sda_pp3, + soc_gpio23_pp4, soc_gpio24_pp5, soc_gpio25_pp6, + pwr_i2c_scl_pp7, pwr_i2c_sda_pq0, soc_gpio28_pq1, + soc_gpio29_pq2, soc_gpio30_pq3, soc_gpio31_pq4, + soc_gpio32_pq5, soc_gpio33_pq6, soc_gpio35_pq7, + soc_gpio37_pr0, soc_gpio56_pr1, uart1_tx_pr2, + uart1_rx_pr3, uart1_rts_pr4, uart1_cts_pr5, + soc_gpio61_pw0, soc_gpio62_pw1, gpu_pwr_req_px0, cv_pwr_req_px1, + gp_pwm2_px2, gp_pwm3_px3, uart2_tx_px4, uart2_rx_px5, + uart2_rts_px6, uart2_cts_px7, spi3_sck_py0, + spi3_miso_py1, spi3_mosi_py2, spi3_cs0_py3, + spi3_cs1_py4, uart5_tx_py5, uart5_rx_py6, + uart5_rts_py7, uart5_cts_pz0, usb_vbus_en0_pz1, + usb_vbus_en1_pz2, spi1_sck_pz3, spi1_miso_pz4, + spi1_mosi_pz5, spi1_cs0_pz6, spi1_cs1_pz7, + spi5_sck_pac0, spi5_miso_pac1, spi5_mosi_pac2, + spi5_cs0_pac3, soc_gpio57_pac4, soc_gpio58_pac5, + soc_gpio59_pac6, soc_gpio60_pac7, soc_gpio45_pad0, + soc_gpio46_pad1, soc_gpio47_pad2, soc_gpio48_pad3, + ufs0_ref_clk_pae0, ufs0_rst_n_pae1, + pex_l5_clkreq_n_paf0, pex_l5_rst_n_paf1, + pex_l6_clkreq_n_paf2, pex_l6_rst_n_paf3, + pex_l7_clkreq_n_pag0, pex_l7_rst_n_pag1, + pex_l8_clkreq_n_pag2, pex_l8_rst_n_pag3, + pex_l9_clkreq_n_pag4, pex_l9_rst_n_pag5, + pex_l10_clkreq_n_pag6, pex_l10_rst_n_pag7, + sdmmc1_comp, eqos_comp, qspi_comp, + # drive groups + drive_soc_gpio08_pb0, drive_soc_gpio36_pm5, + drive_soc_gpio53_pm6, drive_soc_gpio55_pm4, + drive_soc_gpio38_pm7, drive_soc_gpio39_pn1, + drive_soc_gpio40_pn2, drive_dp_aux_ch0_hpd_pm0, + drive_dp_aux_ch1_hpd_pm1, drive_dp_aux_ch2_hpd_pm2, + drive_dp_aux_ch3_hpd_pm3, drive_dp_aux_ch1_p_pn3, + drive_dp_aux_ch1_n_pn4, drive_dp_aux_ch2_p_pn5, + drive_dp_aux_ch2_n_pn6, drive_dp_aux_ch3_p_pn7, + drive_dp_aux_ch3_n_pn0, drive_pex_l2_clkreq_n_pk4, + drive_pex_wake_n_pl2, drive_pex_l1_clkreq_n_pk2, + drive_pex_l1_rst_n_pk3, drive_pex_l0_clkreq_n_pk0, + drive_pex_l0_rst_n_pk1, drive_pex_l2_rst_n_pk5, + drive_pex_l3_clkreq_n_pk6, drive_pex_l3_rst_n_pk7, + drive_pex_l4_clkreq_n_pl0, drive_pex_l4_rst_n_pl1, + drive_soc_gpio34_pl3, drive_pex_l5_clkreq_n_paf0, + drive_pex_l5_rst_n_paf1, drive_pex_l6_clkreq_n_paf2, + drive_pex_l6_rst_n_paf3, drive_pex_l10_clkreq_n_pag6, + drive_pex_l10_rst_n_pag7, drive_pex_l7_clkreq_n_pag0, + drive_pex_l7_rst_n_pag1, drive_pex_l8_clkreq_n_pag2, + drive_pex_l8_rst_n_pag3, drive_pex_l9_clkreq_n_pag4, + drive_pex_l9_rst_n_pag5, drive_sdmmc1_clk_pj0, + drive_sdmmc1_cmd_pj1, drive_sdmmc1_dat3_pj5,Documentation/devicetree/bindings/pinctrl/nvidia,tegra234-pinmux.yaml line 170
Quoted 10 lines
Documentation/devicetree/bindings/pinctrl/nvidia,tegra234-pinmux.yaml new file +10
-0+ drive_sdmmc1_dat2_pj4, drive_sdmmc1_dat1_pj3, + drive_sdmmc1_dat0_pj2 ] + + - if: + properties: + compatible: + const: nvidia,tegra234-pinmux-aon + then: + patternProperties: + "^pinmux(-[a-z0-9-_]+)?$":No underscores
Quoted 46 lines
Documentation/devicetree/bindings/pinctrl/nvidia,tegra234-pinmux.yaml new file +46
-0+ type: object + additionalProperties: + properties: + nvidia,pins: + items: + enum: [ can0_dout_paa0, can0_din_paa1, can1_dout_paa2, + can1_din_paa3, can0_stb_paa4, can0_en_paa5, + soc_gpio49_paa6, can0_err_paa7, can1_stb_pbb0, + can1_en_pbb1, soc_gpio50_pbb2, can1_err_pbb3, + spi2_sck_pcc0, spi2_miso_pcc1, spi2_mosi_pcc2, + spi2_cs0_pcc3, touch_clk_pcc4, uart3_tx_pcc5, + uart3_rx_pcc6, gen2_i2c_scl_pcc7, gen2_i2c_sda_pdd0, + gen8_i2c_scl_pdd1, gen8_i2c_sda_pdd2, + sce_error_pee0, vcomp_alert_pee1, + ao_retention_n_pee2, batt_oc_pee3, power_on_pee4, + soc_gpio26_pee5, soc_gpio27_pee6, bootv_ctl_n_pee7, + hdmi_cec_pgg0, + # drive groups + drive_touch_clk_pcc4, drive_uart3_rx_pcc6, + drive_uart3_tx_pcc5, drive_gen8_i2c_sda_pdd2, + drive_gen8_i2c_scl_pdd1, drive_spi2_mosi_pcc2, + drive_gen2_i2c_scl_pcc7, drive_spi2_cs0_pcc3, + drive_gen2_i2c_sda_pdd0, drive_spi2_sck_pcc0, + drive_spi2_miso_pcc1, drive_can1_dout_paa2, + drive_can1_din_paa3, drive_can0_dout_paa0, + drive_can0_din_paa1, drive_can0_stb_paa4, + drive_can0_en_paa5, drive_soc_gpio49_paa6, + drive_can0_err_paa7, drive_can1_stb_pbb0, + drive_can1_en_pbb1, drive_soc_gpio50_pbb2, + drive_can1_err_pbb3, drive_sce_error_pee0, + drive_batt_oc_pee3, drive_bootv_ctl_n_pee7, + drive_power_on_pee4, drive_soc_gpio26_pee5, + drive_soc_gpio27_pee6, drive_ao_retention_n_pee2, + drive_vcomp_alert_pee1, drive_hdmi_cec_pgg0 ] + +required: + - compatible + - reg + +examples: + - | + #include <dt-bindings/pinctrl/pinctrl-tegra.h> + + pinmux@2430000 { + compatible = "nvidia,tegra234-pinmux"; + reg = <0x2430000 0x17000>;
Quoted 8 lines
Documentation/devicetree/bindings/pinctrl/nvidia,tegra234-pinmux.yaml line 222
Quoted 6 lines
Documentation/devicetree/bindings/pinctrl/nvidia,tegra234-pinmux.yaml new file +6
-0+ + pinctrl-names = "pex_rst"; + pinctrl-0 = <&pex_rst_c5_out_state>; + + pex_rst_c5_out_state: pinmux-pex-rst-c5-out { + pex_rst {Underscores are not valid in node names.
We have supported underscore in older bindings for historical reasons. But I suppose for these newer bindings we could disallow them.
Some of the older DTs have a large number of underscores, so I'm not sure it makes sense to go back and fix those. Maybe something to keep in mind for when we're done with all the conversions...
Thierry
Quoted 15 lines · Thierry Reding
Quoted 11 lines · Krzysztof Kozlowski
Documentation/devicetree/bindings/pinctrl/nvidia,tegra234-pinmux.yaml line 74
Quoted 7 lines
Documentation/devicetree/bindings/pinctrl/nvidia,tegra234-pinmux.yaml new file +7
-0+ type: object + additionalProperties: + properties: + nvidia,pins: + description: An array of strings. Each string contains the name + of a pin or group. Valid values for these names are listed + below.Define properties in top level, which points to the complexity of your if-else, thus probably this should be split into two bindings. Dunno, your other bindings repeat this pattern :(
Quoted 8 lines
The property itself is already defined in the common schema found in nvidia,tegra-pinmux-common.yaml and we're overriding this here for each instance since each has its own set of pins.
This was a compromise to avoid too many bindings. Originally I attempted to roll all Tegra pinctrl bindings into a single dt-schema, but that turned out truly horrible =) Splitting this into per-SoC bindings is already causing a lot of duplication in these files,
What would be duplicated? Almost eveerything should be coming from shared binding, so you will have only compatible, patternProperties(pinmux) and nvidia,pins. And an example. Maybe I miss something but I would say this would create many but very easy to read bindings, referencing common pieces.
Quoted 5 lines
though splitting
off the common bits into nvidi,tegra-pinmux-common.yaml helps a bit with that already. Splitting this into per-instance bindings would effectively duplicate everything but the pin array here, so we kind of settled on this compromise for Tegra194.
OK, but are you sure it is now readable? You have if:then: with
patternProperties: with additionalProperties: with properties: with nvidia,pins.
Quoted 5 lines
We're taking a bit of a shortcut here already, since technically not all pins support all the functions listed above. On the other hand, fully accurately describing per-pin functions would make this a total mess, so again, we use this simplified representation as a compromise.
That's okay, many platforms do the same way.
(...)
Quoted 4 lines
Quoted 4 lines
Quoted 3 lines
Documentation/devicetree/bindings/pinctrl/nvidia,tegra234-pinmux.yaml new file +3
-0+ + pex_rst_c5_out_state: pinmux-pex-rst-c5-out { + pex_rst {
Quoted 8 lines
Quoted 1 line
Underscores are not valid in node names.
We have supported underscore in older bindings for historical reasons. But I suppose for these newer bindings we could disallow them.
Some of the older DTs have a large number of underscores, so I'm not sure it makes sense to go back and fix those. Maybe something to keep in mind for when we're done with all the conversions...
I understand, up to you. I think that if such older platform is still supported/maintained/used, then such cleanups are positive. Underscores are reported by dtc at W=2, so it is not that critical. But many other nits like generic node names are being enforced by dtschema, thus if you want to achieve 0-warning state, at some point you will need to address these. Of course different question is on what tasks you want to spend your time. :)
Best regards,
Krzysztof
Quoted 53 lines
-----Original Message-----
From: Krzysztof Kozlowski <[email protected]>
Sent: Wednesday, February 8, 2023 5:28 PM
To: Thierry Reding <[email protected]>
Cc: Prathamesh Shete <[email protected]>; Jonathan Hunter <[email protected]>; [email protected]; [email protected]; [email protected]; [email protected]; linux- [email protected]; [email protected]; Suresh Mangipudi <[email protected]>
Subject: Re: [PATCH 1/3] dt-bindings: pinctrl: tegra234: Add DT binding docExternal email: Use caution opening links or attachments
Quoted 15 lines · Thierry Reding
Quoted 11 lines · Krzysztof Kozlowski
Documentation/devicetree/bindings/pinctrl/nvidia,tegra234-pinmux.yaml line 74
Quoted 7 lines
Documentation/devicetree/bindings/pinctrl/nvidia,tegra234-pinmux.yaml new file +7
-0+ type: object + additionalProperties: + properties: + nvidia,pins: + description: An array of strings. Each string contains the name + of a pin or group. Valid values for these names are listed + below.Define properties in top level, which points to the complexity of your if-else, thus probably this should be split into two bindings. Dunno, your other bindings repeat this pattern :(
Quoted 8 lines
The property itself is already defined in the common schema found in nvidia,tegra-pinmux-common.yaml and we're overriding this here for each instance since each has its own set of pins.
This was a compromise to avoid too many bindings. Originally I attempted to roll all Tegra pinctrl bindings into a single dt-schema, but that turned out truly horrible =) Splitting this into per-SoC bindings is already causing a lot of duplication in these files,
What would be duplicated? Almost eveerything should be coming from shared binding, so you will have only compatible, patternProperties(pinmux) and nvidia,pins. And an example. Maybe I miss something but I would say this would create many but very easy to read bindings, referencing common pieces.
Quoted 5 lines
though splitting
off the common bits into nvidi,tegra-pinmux-common.yaml helps a bit with that already. Splitting this into per-instance bindings would effectively duplicate everything but the pin array here, so we kind of settled on this compromise for Tegra194.
Quoted 3 lines
OK, but are you sure it is now readable? You have if:then: with
patternProperties: with additionalProperties: with properties: with nvidia,pins.
This is inline with the existing bindings and I think this is the compromise that was reached during review when the bindings were submitted,
offer to rework if a better alternative can be found, but that only makes sense if all the other bindings get changed as well, so I think it'd be good if we can merge in the same format as the existing bindings for now and change all of them later on. Sent a version v2 addressing all other comments.
Thanks
Prathamesh
Quoted 33 lines
Quoted 6 lines
We're taking a bit of a shortcut here already, since technically not all pins support all the functions listed above. On the other hand, fully accurately describing per-pin functions would make this a total mess, so again, we use this simplified representation as a compromise.
That's okay, many platforms do the same way.
(...)
Quoted 4 lines
Quoted 4 lines
Quoted 3 lines
Documentation/devicetree/bindings/pinctrl/nvidia,tegra234-pinmux.yaml new file +3
-0+ + pex_rst_c5_out_state: pinmux-pex-rst-c5-out { + pex_rst {Quoted 8 lines
Quoted 1 line
Underscores are not valid in node names.
We have supported underscore in older bindings for historical reasons. But I suppose for these newer bindings we could disallow them.
Some of the older DTs have a large number of underscores, so I'm not sure it makes sense to go back and fix those. Maybe something to keep in mind for when we're done with all the conversions...
I understand, up to you. I think that if such older platform is still supported/maintained/used, then such cleanups are positive. Underscores are reported by dtc at W=2, so it is not that critical. But many other nits like generic node names are being enforced by dtschema, thus if you want to achieve 0-warning state, at some point you will need to address these. Of course different question is on what tasks you want to spend your time. :)
Best regards,
Krzysztof
Quoted 53 lines · Prathamesh Shete
Quoted 53 lines
-----Original Message-----
From: Krzysztof Kozlowski <[email protected]>
Sent: Wednesday, February 8, 2023 5:28 PM
To: Thierry Reding <[email protected]>
Cc: Prathamesh Shete <[email protected]>; Jonathan Hunter <[email protected]>; [email protected]; [email protected]; [email protected]; [email protected]; linux- [email protected]; [email protected]; Suresh Mangipudi <[email protected]>
Subject: Re: [PATCH 1/3] dt-bindings: pinctrl: tegra234: Add DT binding docExternal email: Use caution opening links or attachments
Quoted 15 lines · Thierry Reding
Quoted 11 lines · Krzysztof Kozlowski
Documentation/devicetree/bindings/pinctrl/nvidia,tegra234-pinmux.yaml line 74
Quoted 7 lines
Documentation/devicetree/bindings/pinctrl/nvidia,tegra234-pinmux.yaml new file +7
-0+ type: object + additionalProperties: + properties: + nvidia,pins: + description: An array of strings. Each string contains the name + of a pin or group. Valid values for these names are listed + below.Define properties in top level, which points to the complexity of your if-else, thus probably this should be split into two bindings. Dunno, your other bindings repeat this pattern :(
Quoted 8 lines
The property itself is already defined in the common schema found in nvidia,tegra-pinmux-common.yaml and we're overriding this here for each instance since each has its own set of pins.
This was a compromise to avoid too many bindings. Originally I attempted to roll all Tegra pinctrl bindings into a single dt-schema, but that turned out truly horrible =) Splitting this into per-SoC bindings is already causing a lot of duplication in these files,
What would be duplicated? Almost eveerything should be coming from shared binding, so you will have only compatible, patternProperties(pinmux) and nvidia,pins. And an example. Maybe I miss something but I would say this would create many but very easy to read bindings, referencing common pieces.
Quoted 5 lines
though splitting
off the common bits into nvidi,tegra-pinmux-common.yaml helps a bit with that already. Splitting this into per-instance bindings would effectively duplicate everything but the pin array here, so we kind of settled on this compromise for Tegra194.
Quoted 4 lines
Quoted 3 lines
OK, but are you sure it is now readable? You have if:then: with
patternProperties: with additionalProperties: with properties: with nvidia,pins.This is inline with the existing bindings and I think this is the compromise that was reached during review when the bindings were submitted,
So the code might be totally unreadable, but it is inline with existing code, thus it should stay unreadable. Great.
Quoted 1 line
offer to rework if a better alternative can be found, but that only makes sense if all the other bindings get changed as well, so I think it'd be good if we can merge in the same format as the existing bindings for now and change all of them later on.
Cleanup should happen before adding new bindings.
Best regards,
Krzysztof
Quoted 54 lines · Krzysztof Kozlowski
Quoted 53 lines · Prathamesh Shete
Quoted 53 lines
-----Original Message-----
From: Krzysztof Kozlowski <[email protected]>
Sent: Wednesday, February 8, 2023 5:28 PM
To: Thierry Reding <[email protected]>
Cc: Prathamesh Shete <[email protected]>; Jonathan Hunter <[email protected]>; [email protected]; [email protected]; [email protected]; [email protected]; linux- [email protected]; [email protected]; Suresh Mangipudi <[email protected]>
Subject: Re: [PATCH 1/3] dt-bindings: pinctrl: tegra234: Add DT binding docExternal email: Use caution opening links or attachments
Quoted 15 lines · Thierry Reding
Quoted 11 lines · Krzysztof Kozlowski
Documentation/devicetree/bindings/pinctrl/nvidia,tegra234-pinmux.yaml line 74
Quoted 7 lines
Documentation/devicetree/bindings/pinctrl/nvidia,tegra234-pinmux.yaml new file +7
-0+ type: object + additionalProperties: + properties: + nvidia,pins: + description: An array of strings. Each string contains the name + of a pin or group. Valid values for these names are listed + below.Define properties in top level, which points to the complexity of your if-else, thus probably this should be split into two bindings. Dunno, your other bindings repeat this pattern :(
Quoted 8 lines
The property itself is already defined in the common schema found in nvidia,tegra-pinmux-common.yaml and we're overriding this here for each instance since each has its own set of pins.
This was a compromise to avoid too many bindings. Originally I attempted to roll all Tegra pinctrl bindings into a single dt-schema, but that turned out truly horrible =) Splitting this into per-SoC bindings is already causing a lot of duplication in these files,
What would be duplicated? Almost eveerything should be coming from shared binding, so you will have only compatible, patternProperties(pinmux) and nvidia,pins. And an example. Maybe I miss something but I would say this would create many but very easy to read bindings, referencing common pieces.
Quoted 5 lines
though splitting
off the common bits into nvidi,tegra-pinmux-common.yaml helps a bit with that already. Splitting this into per-instance bindings would effectively duplicate everything but the pin array here, so we kind of settled on this compromise for Tegra194.
Quoted 7 lines
Quoted 4 lines
Quoted 3 lines
OK, but are you sure it is now readable? You have if:then: with
patternProperties: with additionalProperties: with properties: with nvidia,pins.This is inline with the existing bindings and I think this is the compromise that was reached during review when the bindings were submitted,
So the code might be totally unreadable, but it is inline with existing code, thus it should stay unreadable. Great.
I'd say this is very subjective. I personally don't find the current version hard to read, but that's maybe because I wrote it... =)
Quoted 3 lines
Quoted 1 line
offer to rework if a better alternative can be found, but that only makes sense if all the other bindings get changed as well, so I think it'd be good if we can merge in the same format as the existing bindings for now and change all of them later on.
Cleanup should happen before adding new bindings.
I don't recall the exact problems that I ran into last time, but I do remember that pulling out the common bindings to the very top-level was the main issue.
If I understand correctly what you're saying, the main problem that makes this hard to read is the if and else constructs for AON/MAIN variants on Tegra194/Tegra234. These should be quite easy to pull out into separate bindings. I'll do that first and then see if there's anything that could be done to further improve things.
Thierry
Quoted 74 lines · Thierry Reding
Quoted 54 lines · Krzysztof Kozlowski
Quoted 53 lines · Prathamesh Shete
Quoted 53 lines
-----Original Message-----
From: Krzysztof Kozlowski <[email protected]>
Sent: Wednesday, February 8, 2023 5:28 PM
To: Thierry Reding <[email protected]>
Cc: Prathamesh Shete <[email protected]>; Jonathan Hunter <[email protected]>; [email protected]; [email protected]; [email protected]; [email protected]; linux- [email protected]; [email protected]; Suresh Mangipudi <[email protected]>
Subject: Re: [PATCH 1/3] dt-bindings: pinctrl: tegra234: Add DT binding docExternal email: Use caution opening links or attachments
Quoted 15 lines · Thierry Reding
Quoted 11 lines · Krzysztof Kozlowski
Review
Quoted 7 lines
(quoted diff) +7
-0+ type: object + additionalProperties: + properties: + nvidia,pins: + description: An array of strings. Each string contains the name + of a pin or group. Valid values for these names are listed + below.Define properties in top level, which points to the complexity of your if-else, thus probably this should be split into two bindings. Dunno, your other bindings repeat this pattern :(
Quoted 8 lines
The property itself is already defined in the common schema found in nvidia,tegra-pinmux-common.yaml and we're overriding this here for each instance since each has its own set of pins.
This was a compromise to avoid too many bindings. Originally I attempted to roll all Tegra pinctrl bindings into a single dt-schema, but that turned out truly horrible =) Splitting this into per-SoC bindings is already causing a lot of duplication in these files,
What would be duplicated? Almost eveerything should be coming from shared binding, so you will have only compatible, patternProperties(pinmux) and nvidia,pins. And an example. Maybe I miss something but I would say this would create many but very easy to read bindings, referencing common pieces.
Quoted 5 lines
though splitting
off the common bits into nvidi,tegra-pinmux-common.yaml helps a bit with that already. Splitting this into per-instance bindings would effectively duplicate everything but the pin array here, so we kind of settled on this compromise for Tegra194.Quoted 7 lines
Quoted 4 lines
Quoted 3 lines
OK, but are you sure it is now readable? You have if:then: with
patternProperties: with additionalProperties: with properties: with nvidia,pins.This is inline with the existing bindings and I think this is the compromise that was reached during review when the bindings were submitted,
So the code might be totally unreadable, but it is inline with existing code, thus it should stay unreadable. Great.
I'd say this is very subjective. I personally don't find the current version hard to read, but that's maybe because I wrote it... =)
Quoted 3 lines
Quoted 1 line
offer to rework if a better alternative can be found, but that only makes sense if all the other bindings get changed as well, so I think it'd be good if we can merge in the same format as the existing bindings for now and change all of them later on.
Cleanup should happen before adding new bindings.
I don't recall the exact problems that I ran into last time, but I do remember that pulling out the common bindings to the very top-level was the main issue.
Quoted 5 lines
If I understand correctly what you're saying, the main problem that makes this hard to read is the if and else constructs for AON/MAIN variants on Tegra194/Tegra234. These should be quite easy to pull out into separate bindings. I'll do that first and then see if there's anything that could be done to further improve things.
One problem is allowing characters here which are not allowed. Second problem is reluctance to change it with argument "existing bindings also have this problem". It's explanation like "there is already bug like this, so I am allowed to add similar one".
Now third is that defining properties in allOf is not the style we want to have, because it does not work with additionalProperties and is difficult to read. Again using argument "existing code also does like this" is a very poor argument.
Best regards,
Krzysztof
Quoted 81 lines · Krzysztof Kozlowski
Quoted 74 lines · Thierry Reding
Quoted 54 lines · Krzysztof Kozlowski
Quoted 53 lines · Prathamesh Shete
Quoted 53 lines
-----Original Message-----
From: Krzysztof Kozlowski <[email protected]>
Sent: Wednesday, February 8, 2023 5:28 PM
To: Thierry Reding <[email protected]>
Cc: Prathamesh Shete <[email protected]>; Jonathan Hunter <[email protected]>; [email protected]; [email protected]; [email protected]; [email protected]; linux- [email protected]; [email protected]; Suresh Mangipudi <[email protected]>
Subject: Re: [PATCH 1/3] dt-bindings: pinctrl: tegra234: Add DT binding docExternal email: Use caution opening links or attachments
Quoted 15 lines · Thierry Reding
Quoted 11 lines · Krzysztof Kozlowski
Review
Quoted 7 lines
(quoted diff) +7
-0+ type: object + additionalProperties: + properties: + nvidia,pins: + description: An array of strings. Each string contains the name + of a pin or group. Valid values for these names are listed + below.Define properties in top level, which points to the complexity of your if-else, thus probably this should be split into two bindings. Dunno, your other bindings repeat this pattern :(
Quoted 8 lines
The property itself is already defined in the common schema found in nvidia,tegra-pinmux-common.yaml and we're overriding this here for each instance since each has its own set of pins.
This was a compromise to avoid too many bindings. Originally I attempted to roll all Tegra pinctrl bindings into a single dt-schema, but that turned out truly horrible =) Splitting this into per-SoC bindings is already causing a lot of duplication in these files,
What would be duplicated? Almost eveerything should be coming from shared binding, so you will have only compatible, patternProperties(pinmux) and nvidia,pins. And an example. Maybe I miss something but I would say this would create many but very easy to read bindings, referencing common pieces.
Quoted 5 lines
though splitting
off the common bits into nvidi,tegra-pinmux-common.yaml helps a bit with that already. Splitting this into per-instance bindings would effectively duplicate everything but the pin array here, so we kind of settled on this compromise for Tegra194.Quoted 7 lines
Quoted 4 lines
Quoted 3 lines
OK, but are you sure it is now readable? You have if:then: with
patternProperties: with additionalProperties: with properties: with nvidia,pins.This is inline with the existing bindings and I think this is the compromise that was reached during review when the bindings were submitted,
So the code might be totally unreadable, but it is inline with existing code, thus it should stay unreadable. Great.
I'd say this is very subjective. I personally don't find the current version hard to read, but that's maybe because I wrote it... =)
Quoted 3 lines
Quoted 1 line
offer to rework if a better alternative can be found, but that only makes sense if all the other bindings get changed as well, so I think it'd be good if we can merge in the same format as the existing bindings for now and change all of them later on.
Cleanup should happen before adding new bindings.
I don't recall the exact problems that I ran into last time, but I do remember that pulling out the common bindings to the very top-level was the main issue.
Quoted 5 lines
If I understand correctly what you're saying, the main problem that makes this hard to read is the if and else constructs for AON/MAIN variants on Tegra194/Tegra234. These should be quite easy to pull out into separate bindings. I'll do that first and then see if there's anything that could be done to further improve things.
Quoted 4 lines
One problem is allowing characters here which are not allowed. Second problem is reluctance to change it with argument "existing bindings also have this problem". It's explanation like "there is already bug like this, so I am allowed to add similar one".
This is not a bug that we're trying to replicate. We're basing this binding on a existing bindings that were already reviewed upstream a long time ago. It uses a shared binding that's in use by these other bindings, so making any changes to this new binding means either the other ones need to be changed as well or we can't reuse the existing shared binding.
Quoted 4 lines
Now third is that defining properties in allOf is not the style we want to have, because it does not work with additionalProperties and is difficult to read. Again using argument "existing code also does like this" is a very poor argument.
As far as I can tell, it does work as expected in this case because we're not actually adding any *new* properties in the allOf/if branches. If we were, then yes, we would need to use unevaluatedProperties and that can get complicated. But again, in this case we're merely overriding existing properties with more specific values, which means that both the standard binding applies and then things are narrowed down by the values defined for each compatible.
Thierry
Quoted 87 lines · Thierry Reding
Quoted 81 lines · Krzysztof Kozlowski
Quoted 74 lines · Thierry Reding
Quoted 54 lines · Krzysztof Kozlowski
Quoted 53 lines · Prathamesh Shete
Quoted 53 lines
-----Original Message-----
From: Krzysztof Kozlowski <[email protected]>
Sent: Wednesday, February 8, 2023 5:28 PM
To: Thierry Reding <[email protected]>
Cc: Prathamesh Shete <[email protected]>; Jonathan Hunter <[email protected]>; [email protected]; [email protected]; [email protected]; [email protected]; linux- [email protected]; [email protected]; Suresh Mangipudi <[email protected]>
Subject: Re: [PATCH 1/3] dt-bindings: pinctrl: tegra234: Add DT binding docExternal email: Use caution opening links or attachments
Quoted 15 lines · Thierry Reding
Quoted 11 lines · Krzysztof Kozlowski
Review
Quoted 7 lines
(quoted diff) +7
-0+ type: object + additionalProperties: + properties: + nvidia,pins: + description: An array of strings. Each string contains the name + of a pin or group. Valid values for these names are listed + below.Define properties in top level, which points to the complexity of your if-else, thus probably this should be split into two bindings. Dunno, your other bindings repeat this pattern :(
Quoted 8 lines
The property itself is already defined in the common schema found in nvidia,tegra-pinmux-common.yaml and we're overriding this here for each instance since each has its own set of pins.
This was a compromise to avoid too many bindings. Originally I attempted to roll all Tegra pinctrl bindings into a single dt-schema, but that turned out truly horrible =) Splitting this into per-SoC bindings is already causing a lot of duplication in these files,
What would be duplicated? Almost eveerything should be coming from shared binding, so you will have only compatible, patternProperties(pinmux) and nvidia,pins. And an example. Maybe I miss something but I would say this would create many but very easy to read bindings, referencing common pieces.
Quoted 5 lines
though splitting
off the common bits into nvidi,tegra-pinmux-common.yaml helps a bit with that already. Splitting this into per-instance bindings would effectively duplicate everything but the pin array here, so we kind of settled on this compromise for Tegra194.Quoted 7 lines
Quoted 4 lines
Quoted 3 lines
OK, but are you sure it is now readable? You have if:then: with
patternProperties: with additionalProperties: with properties: with nvidia,pins.This is inline with the existing bindings and I think this is the compromise that was reached during review when the bindings were submitted,
So the code might be totally unreadable, but it is inline with existing code, thus it should stay unreadable. Great.
I'd say this is very subjective. I personally don't find the current version hard to read, but that's maybe because I wrote it... =)
Quoted 3 lines
Quoted 1 line
offer to rework if a better alternative can be found, but that only makes sense if all the other bindings get changed as well, so I think it'd be good if we can merge in the same format as the existing bindings for now and change all of them later on.
Cleanup should happen before adding new bindings.
I don't recall the exact problems that I ran into last time, but I do remember that pulling out the common bindings to the very top-level was the main issue.
Quoted 5 lines
If I understand correctly what you're saying, the main problem that makes this hard to read is the if and else constructs for AON/MAIN variants on Tegra194/Tegra234. These should be quite easy to pull out into separate bindings. I'll do that first and then see if there's anything that could be done to further improve things.
Quoted 4 lines
One problem is allowing characters here which are not allowed. Second problem is reluctance to change it with argument "existing bindings also have this problem". It's explanation like "there is already bug like this, so I am allowed to add similar one".
Quoted 6 lines
This is not a bug that we're trying to replicate. We're basing this binding on a existing bindings that were already reviewed upstream a long time ago. It uses a shared binding that's in use by these other bindings, so making any changes to this new binding means either the other ones need to be changed as well or we can't reuse the existing shared binding.
Are you sure? I did not see here conflict. The specific device binding can narrow the pattern defined in common binding.
What's more, where do you see this pattern at all in shared binding?
I am sorry, but this does not fit my arguments at all. This pattern is clearly wrong and argument to keep duplicating it because other (not common!) binding also has it is by design invalid.
Quoted 8 lines
Quoted 5 lines
Now third is that defining properties in allOf is not the style we want to have, because it does not work with additionalProperties and is difficult to read. Again using argument "existing code also does like this" is a very poor argument.
As far as I can tell, it does work as expected in this case because we're not actually adding any *new* properties in the allOf/if branches.
The if:else: defines type and additionalProperties, so I am sorry but this is not a readable solution.
Quoted 5 lines
If we were, then yes, we would need to use unevaluatedProperties and that can get complicated. But again, in this case we're merely overriding existing properties with more specific values, which means that both the standard binding applies and then things are narrowed down by the values defined for each compatible.
Best regards,
Krzysztof
Quoted 110 lines · Krzysztof Kozlowski
Quoted 87 lines · Thierry Reding
Quoted 81 lines · Krzysztof Kozlowski
Quoted 74 lines · Thierry Reding
Quoted 54 lines · Krzysztof Kozlowski
Quoted 53 lines · Prathamesh Shete
Quoted 53 lines
-----Original Message-----
From: Krzysztof Kozlowski <[email protected]>
Sent: Wednesday, February 8, 2023 5:28 PM
To: Thierry Reding <[email protected]>
Cc: Prathamesh Shete <[email protected]>; Jonathan Hunter <[email protected]>; [email protected]; [email protected]; [email protected]; [email protected]; linux- [email protected]; [email protected]; Suresh Mangipudi <[email protected]>
Subject: Re: [PATCH 1/3] dt-bindings: pinctrl: tegra234: Add DT binding docExternal email: Use caution opening links or attachments
Quoted 15 lines · Thierry Reding
Quoted 11 lines · Krzysztof Kozlowski
Review
Quoted 7 lines
(quoted diff) +7
-0+ type: object + additionalProperties: + properties: + nvidia,pins: + description: An array of strings. Each string contains the name + of a pin or group. Valid values for these names are listed + below.Define properties in top level, which points to the complexity of your if-else, thus probably this should be split into two bindings. Dunno, your other bindings repeat this pattern :(
Quoted 8 lines
The property itself is already defined in the common schema found in nvidia,tegra-pinmux-common.yaml and we're overriding this here for each instance since each has its own set of pins.
This was a compromise to avoid too many bindings. Originally I attempted to roll all Tegra pinctrl bindings into a single dt-schema, but that turned out truly horrible =) Splitting this into per-SoC bindings is already causing a lot of duplication in these files,
What would be duplicated? Almost eveerything should be coming from shared binding, so you will have only compatible, patternProperties(pinmux) and nvidia,pins. And an example. Maybe I miss something but I would say this would create many but very easy to read bindings, referencing common pieces.
Quoted 5 lines
though splitting
off the common bits into nvidi,tegra-pinmux-common.yaml helps a bit with that already. Splitting this into per-instance bindings would effectively duplicate everything but the pin array here, so we kind of settled on this compromise for Tegra194.Quoted 7 lines
Quoted 4 lines
Quoted 3 lines
OK, but are you sure it is now readable? You have if:then: with
patternProperties: with additionalProperties: with properties: with nvidia,pins.This is inline with the existing bindings and I think this is the compromise that was reached during review when the bindings were submitted,
So the code might be totally unreadable, but it is inline with existing code, thus it should stay unreadable. Great.
I'd say this is very subjective. I personally don't find the current version hard to read, but that's maybe because I wrote it... =)
Quoted 3 lines
Quoted 1 line
offer to rework if a better alternative can be found, but that only makes sense if all the other bindings get changed as well, so I think it'd be good if we can merge in the same format as the existing bindings for now and change all of them later on.
Cleanup should happen before adding new bindings.
I don't recall the exact problems that I ran into last time, but I do remember that pulling out the common bindings to the very top-level was the main issue.
Quoted 5 lines
If I understand correctly what you're saying, the main problem that makes this hard to read is the if and else constructs for AON/MAIN variants on Tegra194/Tegra234. These should be quite easy to pull out into separate bindings. I'll do that first and then see if there's anything that could be done to further improve things.
Quoted 4 lines
One problem is allowing characters here which are not allowed. Second problem is reluctance to change it with argument "existing bindings also have this problem". It's explanation like "there is already bug like this, so I am allowed to add similar one".
Quoted 6 lines
This is not a bug that we're trying to replicate. We're basing this binding on a existing bindings that were already reviewed upstream a long time ago. It uses a shared binding that's in use by these other bindings, so making any changes to this new binding means either the other ones need to be changed as well or we can't reuse the existing shared binding.
Are you sure? I did not see here conflict. The specific device binding can narrow the pattern defined in common binding.
What's more, where do you see this pattern at all in shared binding?
I am sorry, but this does not fit my arguments at all. This pattern is clearly wrong and argument to keep duplicating it because other (not common!) binding also has it is by design invalid.
Quoted 6 lines
Quoted 5 lines
Now third is that defining properties in allOf is not the style we want to have, because it does not work with additionalProperties and is difficult to read. Again using argument "existing code also does like this" is a very poor argument.
Quoted 11 lines
Quoted 2 lines
As far as I can tell, it does work as expected in this case because we're not actually adding any *new* properties in the allOf/if branches.
The if:else: defines type and additionalProperties, so I am sorry but this is not a readable solution.
Quoted 5 lines
If we were, then yes, we would need to use unevaluatedProperties and that can get complicated. But again, in this case we're merely overriding existing properties with more specific values, which means that both the standard binding applies and then things are narrowed down by the values defined for each compatible.
Okay, so I'm starting to get a bit lost here, so maybe it's time for another proposal. I've tried splitting this up more so that we avoid the if/else block. Would you mind taking another look to see if the patch below is any more readable to you?
Thierry
From 00cb909f6d8732680d65cdb67e0573c8e6dc7b7a Mon Sep 17 00:00:00 2001
From: Prathamesh Shete <[email protected]>
Date: Wed, 8 Mar 2023 17:14:30 +0530
Subject: [PATCH] dt-bindings: pinctrl: tegra234
Add DT binding doc for Tegra234 pinmux driver.
--- .../pinctrl/nvidia,tegra234-pinmux-aon.yaml | 61 ++++++++ .../nvidia,tegra234-pinmux-common.yaml | 65 ++++++++ .../pinctrl/nvidia,tegra234-pinmux.yaml | 141 ++++++++++++++++++ 3 files changed, 267 insertions(+) create mode 100644 Documentation/devicetree/bindings/pinctrl/nvidia,tegra234-pinmux-aon.yaml create mode 100644 Documentation/devicetree/bindings/pinctrl/nvidia,tegra234-pinmux-common.yaml create mode 100644 Documentation/devicetree/bindings/pinctrl/nvidia,tegra234-pinmux.yaml
Documentation/devicetree/bindings/pinctrl/nvidia,tegra234-pinmux-aon.yaml new file +61 -0
@@ -0,0 +1,61 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/pinctrl/nvidia,tegra234-pinmux-aon.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +$ref: nvidia,tegra234-pinmux-common.yaml + +title: NVIDIA Tegra234 AON Pinmux Controller + +maintainers: + - Thierry Reding <[email protected]> + - Jon Hunter <[email protected]> + +properties: + compatible: + const: nvidia,tegra234-pinmux-aon + + reg: true + +patternProperties: + "^pinmux(-[a-z0-9-]+)?$": + type: object + + # pin groups + additionalProperties: + properties: + nvidia,pins: + items: + enum: [ can0_dout_paa0, can0_din_paa1, can1_dout_paa2, + can1_din_paa3, can0_stb_paa4, can0_en_paa5, + soc_gpio49_paa6, can0_err_paa7, can1_stb_pbb0, + can1_en_pbb1, soc_gpio50_pbb2, can1_err_pbb3, + spi2_sck_pcc0, spi2_miso_pcc1, spi2_mosi_pcc2, + spi2_cs0_pcc3, touch_clk_pcc4, uart3_tx_pcc5, + uart3_rx_pcc6, gen2_i2c_scl_pcc7, gen2_i2c_sda_pdd0, + gen8_i2c_scl_pdd1, gen8_i2c_sda_pdd2, + sce_error_pee0, vcomp_alert_pee1, + ao_retention_n_pee2, batt_oc_pee3, power_on_pee4, + soc_gpio26_pee5, soc_gpio27_pee6, bootv_ctl_n_pee7, + hdmi_cec_pgg0, + # drive groups + drive_touch_clk_pcc4, drive_uart3_rx_pcc6, + drive_uart3_tx_pcc5, drive_gen8_i2c_sda_pdd2, + drive_gen8_i2c_scl_pdd1, drive_spi2_mosi_pcc2, + drive_gen2_i2c_scl_pcc7, drive_spi2_cs0_pcc3, + drive_gen2_i2c_sda_pdd0, drive_spi2_sck_pcc0, + drive_spi2_miso_pcc1, drive_can1_dout_paa2, + drive_can1_din_paa3, drive_can0_dout_paa0, + drive_can0_din_paa1, drive_can0_stb_paa4, + drive_can0_en_paa5, drive_soc_gpio49_paa6, + drive_can0_err_paa7, drive_can1_stb_pbb0, + drive_can1_en_pbb1, drive_soc_gpio50_pbb2, + drive_can1_err_pbb3, drive_sce_error_pee0, + drive_batt_oc_pee3, drive_bootv_ctl_n_pee7, + drive_power_on_pee4, drive_soc_gpio26_pee5, + drive_soc_gpio27_pee6, drive_ao_retention_n_pee2, + drive_vcomp_alert_pee1, drive_hdmi_cec_pgg0 ] + +additionalProperties: false +...
Documentation/devicetree/bindings/pinctrl/nvidia,tegra234-pinmux-common.yaml new file +65 -0
65 lines not on this page · raw message
Documentation/devicetree/bindings/pinctrl/nvidia,tegra234-pinmux.yaml new file +141 -0
141 lines not on this page · raw message
Signature · 1 line
2.40.0
Quoted 123 lines · Thierry Reding
Quoted 110 lines · Krzysztof Kozlowski
Quoted 87 lines · Thierry Reding
Quoted 81 lines · Krzysztof Kozlowski
Quoted 74 lines · Thierry Reding
Quoted 54 lines · Krzysztof Kozlowski
Quoted 53 lines · Prathamesh Shete
Quoted 53 lines
-----Original Message-----
From: Krzysztof Kozlowski <[email protected]>
Sent: Wednesday, February 8, 2023 5:28 PM
To: Thierry Reding <[email protected]>
Cc: Prathamesh Shete <[email protected]>; Jonathan Hunter <[email protected]>; [email protected]; [email protected]; [email protected]; [email protected]; linux- [email protected]; [email protected]; Suresh Mangipudi <[email protected]>
Subject: Re: [PATCH 1/3] dt-bindings: pinctrl: tegra234: Add DT binding docExternal email: Use caution opening links or attachments
Quoted 15 lines · Thierry Reding
Quoted 11 lines · Krzysztof Kozlowski
Documentation/devicetree/bindings/pinctrl/nvidia,tegra234-pinmux-common.yaml line 46
Quoted 7 lines
Documentation/devicetree/bindings/pinctrl/nvidia,tegra234-pinmux-common.yaml new file +7
-0+ type: object + additionalProperties: + properties: + nvidia,pins: + description: An array of strings. Each string contains the name + of a pin or group. Valid values for these names are listed + below.Define properties in top level, which points to the complexity of your if-else, thus probably this should be split into two bindings. Dunno, your other bindings repeat this pattern :(
Quoted 8 lines
The property itself is already defined in the common schema found in nvidia,tegra-pinmux-common.yaml and we're overriding this here for each instance since each has its own set of pins.
This was a compromise to avoid too many bindings. Originally I attempted to roll all Tegra pinctrl bindings into a single dt-schema, but that turned out truly horrible =) Splitting this into per-SoC bindings is already causing a lot of duplication in these files,
What would be duplicated? Almost eveerything should be coming from shared binding, so you will have only compatible, patternProperties(pinmux) and nvidia,pins. And an example. Maybe I miss something but I would say this would create many but very easy to read bindings, referencing common pieces.
Quoted 5 lines
though splitting
off the common bits into nvidi,tegra-pinmux-common.yaml helps a bit with that already. Splitting this into per-instance bindings would effectively duplicate everything but the pin array here, so we kind of settled on this compromise for Tegra194.Quoted 7 lines
Quoted 4 lines
Quoted 3 lines
OK, but are you sure it is now readable? You have if:then: with
patternProperties: with additionalProperties: with properties: with nvidia,pins.This is inline with the existing bindings and I think this is the compromise that was reached during review when the bindings were submitted,
So the code might be totally unreadable, but it is inline with existing code, thus it should stay unreadable. Great.
I'd say this is very subjective. I personally don't find the current version hard to read, but that's maybe because I wrote it... =)
Quoted 3 lines
Quoted 1 line
offer to rework if a better alternative can be found, but that only makes sense if all the other bindings get changed as well, so I think it'd be good if we can merge in the same format as the existing bindings for now and change all of them later on.
Cleanup should happen before adding new bindings.
I don't recall the exact problems that I ran into last time, but I do remember that pulling out the common bindings to the very top-level was the main issue.
Quoted 5 lines
If I understand correctly what you're saying, the main problem that makes this hard to read is the if and else constructs for AON/MAIN variants on Tegra194/Tegra234. These should be quite easy to pull out into separate bindings. I'll do that first and then see if there's anything that could be done to further improve things.
Quoted 4 lines
One problem is allowing characters here which are not allowed. Second problem is reluctance to change it with argument "existing bindings also have this problem". It's explanation like "there is already bug like this, so I am allowed to add similar one".
Quoted 6 lines
This is not a bug that we're trying to replicate. We're basing this binding on a existing bindings that were already reviewed upstream a long time ago. It uses a shared binding that's in use by these other bindings, so making any changes to this new binding means either the other ones need to be changed as well or we can't reuse the existing shared binding.
Are you sure? I did not see here conflict. The specific device binding can narrow the pattern defined in common binding.
What's more, where do you see this pattern at all in shared binding?
I am sorry, but this does not fit my arguments at all. This pattern is clearly wrong and argument to keep duplicating it because other (not common!) binding also has it is by design invalid.
Quoted 6 lines
Quoted 5 lines
Now third is that defining properties in allOf is not the style we want to have, because it does not work with additionalProperties and is difficult to read. Again using argument "existing code also does like this" is a very poor argument.
Quoted 11 lines
Quoted 2 lines
As far as I can tell, it does work as expected in this case because we're not actually adding any *new* properties in the allOf/if branches.
The if:else: defines type and additionalProperties, so I am sorry but this is not a readable solution.
Quoted 5 lines
If we were, then yes, we would need to use unevaluatedProperties and that can get complicated. But again, in this case we're merely overriding existing properties with more specific values, which means that both the standard binding applies and then things are narrowed down by the values defined for each compatible.
Quoted 6 lines
Okay, so I'm starting to get a bit lost here, so maybe it's time for another proposal. I've tried splitting this up more so that we avoid the if/else block. Would you mind taking another look to see if the patch below is any more readable to you?
Thierry
Krzysztof,
any thoughts on this proposal?
Thanks,
Thierry
Quoted 306 lines
From
00cb909f6d8732680d65cdb67e0573c8e6dc7b7aMon Sep 17 00:00:00 2001
From: Prathamesh Shete <[email protected]>
Date: Wed, 8 Mar 2023 17:14:30 +0530
Subject: [PATCH] dt-bindings: pinctrl: tegra234Add DT binding doc for Tegra234 pinmux driver.
Signed-off-by Prathamesh Shete <[email protected]>Signed-off-by Thierry Reding <[email protected]>--- .../pinctrl/nvidia,tegra234-pinmux-aon.yaml | 61 ++++++++ .../nvidia,tegra234-pinmux-common.yaml | 65 ++++++++ .../pinctrl/nvidia,tegra234-pinmux.yaml | 141 ++++++++++++++++++ 3 files changed, 267 insertions(+) create mode 100644 Documentation/devicetree/bindings/pinctrl/nvidia,tegra234-pinmux-aon.yaml create mode 100644 Documentation/devicetree/bindings/pinctrl/nvidia,tegra234-pinmux-common.yaml create mode 100644 Documentation/devicetree/bindings/pinctrl/nvidia,tegra234-pinmux.yamlDocumentation/devicetree/bindings/pinctrl/nvidia,tegra234-pinmux-aon.yaml new file +61
-0@@ -0,0 +1,61 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/pinctrl/nvidia,tegra234-pinmux-aon.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +$ref: nvidia,tegra234-pinmux-common.yaml + +title: NVIDIA Tegra234 AON Pinmux Controller + +maintainers: + - Thierry Reding <[email protected]> + - Jon Hunter <[email protected]> + +properties: + compatible: + const: nvidia,tegra234-pinmux-aon + + reg: true + +patternProperties: + "^pinmux(-[a-z0-9-]+)?$": + type: object + + # pin groups + additionalProperties: + properties: + nvidia,pins: + items: + enum: [ can0_dout_paa0, can0_din_paa1, can1_dout_paa2, + can1_din_paa3, can0_stb_paa4, can0_en_paa5, + soc_gpio49_paa6, can0_err_paa7, can1_stb_pbb0, + can1_en_pbb1, soc_gpio50_pbb2, can1_err_pbb3, + spi2_sck_pcc0, spi2_miso_pcc1, spi2_mosi_pcc2, + spi2_cs0_pcc3, touch_clk_pcc4, uart3_tx_pcc5, + uart3_rx_pcc6, gen2_i2c_scl_pcc7, gen2_i2c_sda_pdd0, + gen8_i2c_scl_pdd1, gen8_i2c_sda_pdd2, + sce_error_pee0, vcomp_alert_pee1, + ao_retention_n_pee2, batt_oc_pee3, power_on_pee4, + soc_gpio26_pee5, soc_gpio27_pee6, bootv_ctl_n_pee7, + hdmi_cec_pgg0, + # drive groups + drive_touch_clk_pcc4, drive_uart3_rx_pcc6, + drive_uart3_tx_pcc5, drive_gen8_i2c_sda_pdd2, + drive_gen8_i2c_scl_pdd1, drive_spi2_mosi_pcc2, + drive_gen2_i2c_scl_pcc7, drive_spi2_cs0_pcc3, + drive_gen2_i2c_sda_pdd0, drive_spi2_sck_pcc0, + drive_spi2_miso_pcc1, drive_can1_dout_paa2, + drive_can1_din_paa3, drive_can0_dout_paa0, + drive_can0_din_paa1, drive_can0_stb_paa4, + drive_can0_en_paa5, drive_soc_gpio49_paa6, + drive_can0_err_paa7, drive_can1_stb_pbb0, + drive_can1_en_pbb1, drive_soc_gpio50_pbb2, + drive_can1_err_pbb3, drive_sce_error_pee0, + drive_batt_oc_pee3, drive_bootv_ctl_n_pee7, + drive_power_on_pee4, drive_soc_gpio26_pee5, + drive_soc_gpio27_pee6, drive_ao_retention_n_pee2, + drive_vcomp_alert_pee1, drive_hdmi_cec_pgg0 ] + +additionalProperties: false +...Documentation/devicetree/bindings/pinctrl/nvidia,tegra234-pinmux-common.yaml new file +65
-0@@ -0,0 +1,65 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/pinctrl/nvidia,tegra234-pinmux-common.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: NVIDIA Tegra234 Pinmux Controller + +maintainers: + - Thierry Reding <[email protected]> + - Jon Hunter <[email protected]> + +properties: + compatible: true + + reg: + items: + - description: pinmux registers + +patternProperties: + "^pinmux(-[a-z0-9-]+)?$": + type: object + properties: + phandle: true + + # pin groups + additionalProperties: + $ref: nvidia,tegra-pinmux-common.yaml + unevaluatedProperties: false + properties: + nvidia,function: + enum: [ gp, uartc, i2c8, spi2, i2c2, can1, can0, rsvd0, eth0, eth2, + eth1, dp, eth3, i2c4, i2c7, i2c9, eqos, pe2, pe1, pe0, pe3, + pe4, pe5, pe6, pe7, pe8, pe9, pe10, qspi0, qspi1, qpsi, + sdmmc1, sce, soc, gpio, hdmi, ufs0, spi3, spi1, uartb, uarte, + usb, extperiph2, extperiph1, i2c3, vi0, i2c5, uarta, uartd, + i2c1, i2s4, i2s6, aud, spi5, touch, uartj, rsvd1, wdt, tsc, + dmic3, led, vi0_alt, i2s5, nv, extperiph3, extperiph4, spi4, + ccla, i2s1, i2s2, i2s3, i2s8, rsvd2, dmic5, dca, displayb, + displaya, vi1, dcb, dmic1, dmic4, i2s7, dmic2, dspk0, rsvd3, + tsc_alt, istctrl, vi1_alt, dspk1, igpu ] + + nvidia,pins: + description: An array of strings. Each string contains the name + of a pin or group. Valid values for these names are listed + below. + + nvidia,pull: true + nvidia,tristate: true + nvidia,schmitt: true + nvidia,enable-input: true + nvidia,open-drain: true + nvidia,lock: true + nvidia,drive-type: true + nvidia,io-hv: true + + required: + - nvidia,pins + +additionalProperties: false + +required: + - compatible + - reg +...Documentation/devicetree/bindings/pinctrl/nvidia,tegra234-pinmux.yaml new file +141
-0@@ -0,0 +1,141 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/pinctrl/nvidia,tegra234-pinmux.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +$ref: nvidia,tegra234-pinmux-common.yaml + +title: NVIDIA Tegra234 Pinmux Controller + +maintainers: + - Thierry Reding <[email protected]> + - Jon Hunter <[email protected]> + +properties: + compatible: + const: nvidia,tegra234-pinmux + + reg: true + +patternProperties: + "^pinmux(-[a-z0-9-]+)?$": + type: object + + # pin groups + additionalProperties: + properties: + nvidia,pins: + items: + enum: [ dap6_sclk_pa0, dap6_dout_pa1, dap6_din_pa2, + dap6_fs_pa3, dap4_sclk_pa4, dap4_dout_pa5, + dap4_din_pa6, dap4_fs_pa7, soc_gpio08_pb0, + qspi0_sck_pc0, qspi0_cs_n_pc1, + qspi0_io0_pc2, qspi0_io1_pc3, qspi0_io2_pc4, + qspi0_io3_pc5, qspi1_sck_pc6, qspi1_cs_n_pc7, + qspi1_io0_pd0, qspi1_io1_pd1, qspi1_io2_pd2, + qspi1_io3_pd3, eqos_txc_pe0, eqos_td0_pe1, + eqos_td1_pe2, eqos_td2_pe3, eqos_td3_pe4, + eqos_tx_ctl_pe5, eqos_rd0_pe6, eqos_rd1_pe7, + eqos_rd2_pf0, eqos_rd3_pf1, eqos_rx_ctl_pf2, + eqos_rxc_pf3, eqos_sma_mdio_pf4, eqos_sma_mdc_pf5, + soc_gpio13_pg0, soc_gpio14_pg1, soc_gpio15_pg2, + soc_gpio16_pg3, soc_gpio17_pg4, soc_gpio18_pg5, + soc_gpio19_pg6, soc_gpio20_pg7, soc_gpio21_ph0, + soc_gpio22_ph1, soc_gpio06_ph2, uart4_tx_ph3, + uart4_rx_ph4, uart4_rts_ph5, uart4_cts_ph6, + soc_gpio41_ph7, soc_gpio42_pi0, soc_gpio43_pi1, + soc_gpio44_pi2, gen1_i2c_scl_pi3, gen1_i2c_sda_pi4, + cpu_pwr_req_pi5, soc_gpio07_pi6, + sdmmc1_clk_pj0, sdmmc1_cmd_pj1, sdmmc1_dat0_pj2, + sdmmc1_dat1_pj3, sdmmc1_dat2_pj4, sdmmc1_dat3_pj5, + pex_l0_clkreq_n_pk0, pex_l0_rst_n_pk1, + pex_l1_clkreq_n_pk2, pex_l1_rst_n_pk3, + pex_l2_clkreq_n_pk4, pex_l2_rst_n_pk5, + pex_l3_clkreq_n_pk6, pex_l3_rst_n_pk7, + pex_l4_clkreq_n_pl0, pex_l4_rst_n_pl1, + pex_wake_n_pl2, soc_gpio34_pl3, dp_aux_ch0_hpd_pm0, + dp_aux_ch1_hpd_pm1, dp_aux_ch2_hpd_pm2, + dp_aux_ch3_hpd_pm3, soc_gpio55_pm4, soc_gpio36_pm5, + soc_gpio53_pm6, soc_gpio38_pm7, dp_aux_ch3_n_pn0, + soc_gpio39_pn1, soc_gpio40_pn2, dp_aux_ch1_p_pn3, + dp_aux_ch1_n_pn4, dp_aux_ch2_p_pn5, dp_aux_ch2_n_pn6, + dp_aux_ch3_p_pn7, extperiph1_clk_pp0, + extperiph2_clk_pp1, cam_i2c_scl_pp2, cam_i2c_sda_pp3, + soc_gpio23_pp4, soc_gpio24_pp5, soc_gpio25_pp6, + pwr_i2c_scl_pp7, pwr_i2c_sda_pq0, soc_gpio28_pq1, + soc_gpio29_pq2, soc_gpio30_pq3, soc_gpio31_pq4, + soc_gpio32_pq5, soc_gpio33_pq6, soc_gpio35_pq7, + soc_gpio37_pr0, soc_gpio56_pr1, uart1_tx_pr2, + uart1_rx_pr3, uart1_rts_pr4, uart1_cts_pr5, + soc_gpio61_pw0, soc_gpio62_pw1, gpu_pwr_req_px0, + cv_pwr_req_px1, gp_pwm2_px2, gp_pwm3_px3, uart2_tx_px4, + uart2_rx_px5, uart2_rts_px6, uart2_cts_px7, spi3_sck_py0, + spi3_miso_py1, spi3_mosi_py2, spi3_cs0_py3, + spi3_cs1_py4, uart5_tx_py5, uart5_rx_py6, + uart5_rts_py7, uart5_cts_pz0, usb_vbus_en0_pz1, + usb_vbus_en1_pz2, spi1_sck_pz3, spi1_miso_pz4, + spi1_mosi_pz5, spi1_cs0_pz6, spi1_cs1_pz7, + spi5_sck_pac0, spi5_miso_pac1, spi5_mosi_pac2, + spi5_cs0_pac3, soc_gpio57_pac4, soc_gpio58_pac5, + soc_gpio59_pac6, soc_gpio60_pac7, soc_gpio45_pad0, + soc_gpio46_pad1, soc_gpio47_pad2, soc_gpio48_pad3, + ufs0_ref_clk_pae0, ufs0_rst_n_pae1, + pex_l5_clkreq_n_paf0, pex_l5_rst_n_paf1, + pex_l6_clkreq_n_paf2, pex_l6_rst_n_paf3, + pex_l7_clkreq_n_pag0, pex_l7_rst_n_pag1, + pex_l8_clkreq_n_pag2, pex_l8_rst_n_pag3, + pex_l9_clkreq_n_pag4, pex_l9_rst_n_pag5, + pex_l10_clkreq_n_pag6, pex_l10_rst_n_pag7, + sdmmc1_comp, eqos_comp, qspi_comp, + # drive groups + drive_soc_gpio08_pb0, drive_soc_gpio36_pm5, + drive_soc_gpio53_pm6, drive_soc_gpio55_pm4, + drive_soc_gpio38_pm7, drive_soc_gpio39_pn1, + drive_soc_gpio40_pn2, drive_dp_aux_ch0_hpd_pm0, + drive_dp_aux_ch1_hpd_pm1, drive_dp_aux_ch2_hpd_pm2, + drive_dp_aux_ch3_hpd_pm3, drive_dp_aux_ch1_p_pn3, + drive_dp_aux_ch1_n_pn4, drive_dp_aux_ch2_p_pn5, + drive_dp_aux_ch2_n_pn6, drive_dp_aux_ch3_p_pn7, + drive_dp_aux_ch3_n_pn0, drive_pex_l2_clkreq_n_pk4, + drive_pex_wake_n_pl2, drive_pex_l1_clkreq_n_pk2, + drive_pex_l1_rst_n_pk3, drive_pex_l0_clkreq_n_pk0, + drive_pex_l0_rst_n_pk1, drive_pex_l2_rst_n_pk5, + drive_pex_l3_clkreq_n_pk6, drive_pex_l3_rst_n_pk7, + drive_pex_l4_clkreq_n_pl0, drive_pex_l4_rst_n_pl1, + drive_soc_gpio34_pl3, drive_pex_l5_clkreq_n_paf0, + drive_pex_l5_rst_n_paf1, drive_pex_l6_clkreq_n_paf2, + drive_pex_l6_rst_n_paf3, drive_pex_l10_clkreq_n_pag6, + drive_pex_l10_rst_n_pag7, drive_pex_l7_clkreq_n_pag0, + drive_pex_l7_rst_n_pag1, drive_pex_l8_clkreq_n_pag2, + drive_pex_l8_rst_n_pag3, drive_pex_l9_clkreq_n_pag4, + drive_pex_l9_rst_n_pag5, drive_sdmmc1_clk_pj0, + drive_sdmmc1_cmd_pj1, drive_sdmmc1_dat3_pj5, + drive_sdmmc1_dat2_pj4, drive_sdmmc1_dat1_pj3, + drive_sdmmc1_dat0_pj2 ] + +additionalProperties: false + +examples: + - | + #include <dt-bindings/pinctrl/pinctrl-tegra.h> + + pinmux@2430000 { + compatible = "nvidia,tegra234-pinmux"; + reg = <0x2430000 0x17000>; + + pinctrl-names = "pex_rst"; + pinctrl-0 = <&pex_rst_c5_out_state>; + + pex_rst_c5_out_state: pinmux-pex-rst-c5-out { + pexrst { + nvidia,pins = "pex_l5_rst_n_paf1"; + nvidia,schmitt = <TEGRA_PIN_DISABLE>; + nvidia,enable-input = <TEGRA_PIN_DISABLE>; + nvidia,io-hv = <TEGRA_PIN_ENABLE>; + nvidia,tristate = <TEGRA_PIN_DISABLE>; + nvidia,pull = <TEGRA_PIN_PULL_NONE>; + }; + }; + }; +...Signature · 1 line
2.40.0
Quoted 12 lines · Thierry Reding
Quoted 12 lines
Quoted 6 lines
Quoted 6 lines
If we were, then yes, we would need to use unevaluatedProperties and that can get complicated. But again, in this case we're merely overriding existing properties with more specific values, which means that both the standard binding applies and then things are narrowed down by the values defined for each compatible.
Okay, so I'm starting to get a bit lost here, so maybe it's time for another proposal. I've tried splitting this up more so that we avoid the if/else block. Would you mind taking another look to see if the patch below is any more readable to you?
Quoted 5 lines
Quoted 1 line
Thierry
Krzysztof,
any thoughts on this proposal?
I did not check the code thoroughly, but approach, assuming it works, looks fine.
Best regards,
Krzysztof