On 21/04/2023 11:43, Rohit Agarwal wrote:
> Add device tree binding Documentation details for Qualcomm SDX75
> pinctrl driver.
>
> Signed-off-by: Rohit Agarwal <quic_rohiagar@xxxxxxxxxxx>
Thank you for your patch. There is something to discuss/improve.
> +properties:
> + compatible:
> + const: qcom,sdx75-tlmm
> +
> + reg:
> + maxItems: 1
> +
> + interrupts: true
> + interrupt-controller: true
> + "#interrupt-cells": true
> + gpio-controller: true
> +
> + gpio-reserved-ranges:
> + minItems: 1
> + maxItems: 105
> +
> + gpio-line-names:
> + maxItems: 133
If you have 210 GPIOs, then this should be 210.
> +
> + "#gpio-cells": true
> + gpio-ranges: true
> + wakeup-parent: true
> +
> +patternProperties:
> + "-state$":
> + oneOf:
> + - $ref: "#/$defs/qcom-sdx75-tlmm-state"
> + - patternProperties:
> + "-pins$":
> + $ref: "#/$defs/qcom-sdx75-tlmm-state"
> + additionalProperties: false
> +
> +$defs:
> + qcom-sdx75-tlmm-state:
> + type: object
> + description:
> + Pinctrl node's client devices use subnodes for desired pin configuration.
> + Client device subnodes use below standard properties.
> + $ref: qcom,tlmm-common.yaml#/$defs/qcom-tlmm-state
unevaluatedProperties: false
> +
> + properties:
> + pins:
> + description:
> + List of gpio pins affected by the properties specified in this
> + subnode.
> + items:
> + oneOf:
> + - pattern: "^gpio([0-9]|[1-9][0-9]|1[0-9][0-9]|20[0-9])$"
This says you have 210 GPIOs.
> + - enum: [ ufs_reset, sdc2_clk, sdc2_cmd, sdc2_data ]
Keep these four enum values sorted alphabetically.
> + minItems: 1
> + maxItems: 36
> +
> + function:
> + description:
> + Specify the alternative function to be configured for the specified
> + pins.
> + enum: [ gpio, eth0_mdc, eth0_mdio, eth1_mdc, eth1_mdio,
> + qlink0_wmss_reset, qlink1_wmss_reset, rgmii_rxc, rgmii_rxd0,
> + rgmii_rxd1, rgmii_rxd2, rgmii_rxd3,rgmii_rx_ctl, rgmii_txc,
> + rgmii_txd0, rgmii_txd1, rgmii_txd2, rgmii_txd3, rgmii_tx_ctl,
> + adsp_ext_vfr, atest_char_start, atest_char_status0,
> + atest_char_status1, atest_char_status2, atest_char_status3,
> + audio_ref_clk, bimc_dte_test0, bimc_dte_test1,
> + char_exec_pending, char_exec_release, coex_uart2_rx,
> + coex_uart2_tx, coex_uart_rx, coex_uart_tx, cri_trng_rosc,
> + cri_trng_rosc0, cri_trng_rosc1, dbg_out_clk, ddr_bist_complete,
> + ddr_bist_fail, ddr_bist_start, ddr_bist_stop, ddr_pxi0_test,
> + ebi0_wrcdc_dq2, ebi0_wrcdc_dq3, ebi2_a_d, ebi2_lcd_cs,
> + ebi2_lcd_reset, ebi2_lcd_te, emac0_mcg_pst0, emac0_mcg_pst1,
> + emac0_mcg_pst2, emac0_mcg_pst3, emac0_ptp_aux, emac0_ptp_pps,
> + emac1_mcg_pst0, emac1_mcg_pst1, emac1_mcg_pst2, emac1_mcg_pst3,
> + emac1_ptp_aux0, emac1_ptp_aux1, emac1_ptp_aux2, emac1_ptp_aux3,
> + emac1_ptp_pps0, emac1_ptp_pps1, emac1_ptp_pps2, emac1_ptp_pps3,
> + emac_cdc_dtest0, emac_cdc_dtest1, emac_pps_in, ext_dbg_uart,
> + gcc_125_clk, gcc_gp1_clk, gcc_gp2_clk, gcc_gp3_clk,
> + gcc_plltest_bypassnl, gcc_plltest_resetn, i2s_mclk,
> + jitter_bist_ref, ldo_en, ldo_update, m_voc_ext, mgpi_clk_req,
> + native0, native1, native2, native3, native_char_start,
> + native_tsens_osc, native_tsense_pwm1, nav_dr_sync, nav_gpio_0,
> + nav_gpio_1, nav_gpio_2, nav_gpio_3, pa_indicator_1, pci_e_rst,
> + pcie0_clkreq_n, pcie1_clkreq_n, pcie2_clkreq_n, pll_bist_sync,
> + pll_clk_aux, pll_ref_clk, pri_mi2s_data0, pri_mi2s_data1,
> + pri_mi2s_sck, pri_mi2s_ws, prng_rosc_test0, prng_rosc_test1,
> + prng_rosc_test2, prng_rosc_test3, qdss_cti_trig0,
> + qdss_cti_trig1, qdss_gpio_traceclk, qdss_gpio_tracectl,
> + qdss_gpio_tracedata0, qdss_gpio_tracedata1,
> + qdss_gpio_tracedata10, qdss_gpio_tracedata11,
> + qdss_gpio_tracedata12, qdss_gpio_tracedata13,
> + qdss_gpio_tracedata14, qdss_gpio_tracedata15,
> + qdss_gpio_tracedata2, qdss_gpio_tracedata3,
> + qdss_gpio_tracedata4, qdss_gpio_tracedata5,
> + qdss_gpio_tracedata6, qdss_gpio_tracedata7,
> + qdss_gpio_tracedata8, qdss_gpio_tracedata9, qlink0_b_en,
> + qlink0_b_req, qlink0_l_en, qlink0_l_req, qlink1_l_en,
> + qlink1_l_req, qup_se0_l0, qup_se0_l1, qup_se0_l2, qup_se0_l3,
> + qup_se1_l2, qup_se1_l3, qup_se2_l0, qup_se2_l1, qup_se2_l2,
> + qup_se2_l3, qup_se3_l0, qup_se3_l1, qup_se3_l2, qup_se3_l3,
> + qup_se4_l2, qup_se4_l3, qup_se5_l0, qup_se5_l1, qup_se6_l0,
> + qup_se6_l1, qup_se6_l2, qup_se6_l3, qup_se7_l0, qup_se7_l1,
> + qup_se7_l2, qup_se7_l3, qup_se8_l2, qup_se8_l3, sdc1_tb_trig,
> + sdc2_tb_trig, sec_mi2s_data0, sec_mi2s_data1, sec_mi2s_sck,
> + sec_mi2s_ws, sgmii_phy_intr0, sgmii_phy_intr1, spmi_coex_clk,
> + spmi_coex_data, spmi_vgi_hwevent, tgu_ch0_trigout,
> + tri_mi2s_data0, tri_mi2s_data1, tri_mi2s_sck, tri_mi2s_ws,
> + uim1_clk, uim1_data, uim1_present, uim1_reset, uim2_clk,
> + uim2_data, uim2_present, uim2_reset, usb2phy_ac_en,
> + vsense_trigger_mirnat]
> +
> + bias-disable: true
> + bias-pull-down: true
> + bias-pull-up: true
> + drive-strength: true
> + input-enable: true
This is not allowed. Please rebase on pinctrl maintainer tree or next.
> + output-high: true
> + output-low: true
> +
> + required:
> + - pins
> +
> + additionalProperties: false
> +
> +required:
> + - compatible
> + - reg
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/interrupt-controller/arm-gic.h>
> + tlmm: pinctrl@f100000 {
> + compatible = "qcom,sdx75-tlmm";
> + reg = <0x0f100000 0x300000>;
> + gpio-controller;
> + #gpio-cells = <2>;
> + gpio-ranges = <&tlmm 0 0 134>;
Wrong number of pins. You have 210, right? This should be number of
GPIOs + optionally UFS reset.
Best regards,
Krzysztof
