[v3,1/2] dt-bindings: pinctrl: qcom: Add SM6350 pinctrl bindings

Add device tree binding Documentation details for Qualcomm SM6350
pinctrl driver.

Signed-off-by: Konrad Dybcio <konrad.dybcio@somainline.org>
---
Changes since v2:
- Tweak the regex to match gpio0-gpio157

 .../bindings/pinctrl/qcom,sm6350-pinctrl.yaml | 148 ++++++++++++++++++
 1 file changed, 148 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pinctrl/qcom,sm6350-pinctrl.yaml

On Sat 28 Aug 12:23 CDT 2021, Konrad Dybcio wrote:

> Add device tree binding Documentation details for Qualcomm SM6350
> pinctrl driver.
> 
> Signed-off-by: Konrad Dybcio <konrad.dybcio@somainline.org>

Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>

Regards,
Bjorn

> ---
> Changes since v2:
> - Tweak the regex to match gpio0-gpio157
> 
>  .../bindings/pinctrl/qcom,sm6350-pinctrl.yaml | 148 ++++++++++++++++++
>  1 file changed, 148 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/pinctrl/qcom,sm6350-pinctrl.yaml
> 
> diff --git a/Documentation/devicetree/bindings/pinctrl/qcom,sm6350-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/qcom,sm6350-pinctrl.yaml
> new file mode 100644
> index 000000000000..554992a681f3
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pinctrl/qcom,sm6350-pinctrl.yaml
> @@ -0,0 +1,148 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/pinctrl/qcom,sm6350-pinctrl.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Qualcomm Technologies, Inc. SM6350 TLMM block
> +
> +maintainers:
> +  - Konrad Dybcio <konrad.dybcio@somainline.org>
> +
> +description: |
> +  This binding describes the Top Level Mode Multiplexer (TLMM) block found
> +  in the SM6350 platform.
> +
> +allOf:
> +  - $ref: /schemas/pinctrl/qcom,tlmm-common.yaml#
> +
> +properties:
> +  compatible:
> +    const: qcom,sm6350-tlmm
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts: true
> +  interrupt-controller: true
> +  '#interrupt-cells': true
> +  gpio-controller: true
> +  gpio-reserved-ranges: true
> +  '#gpio-cells': true
> +  gpio-ranges: true
> +  wakeup-parent: true
> +
> +required:
> +  - compatible
> +  - reg
> +
> +additionalProperties: false
> +
> +patternProperties:
> +  '-state$':
> +    oneOf:
> +      - $ref: "#/$defs/qcom-sm6350-tlmm-state"
> +      - patternProperties:
> +          ".*":
> +            $ref: "#/$defs/qcom-sm6350-tlmm-state"
> +
> +$defs:
> +  qcom-sm6350-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"
> +
> +    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-4][0-9]|15[0-7])$"
> +            - enum: [ sdc1_clk, sdc1_cmd, sdc1_data, sdc2_clk, sdc2_cmd, sdc2_data ]
> +        minItems: 1
> +        maxItems: 36
> +
> +      function:
> +        description:
> +          Specify the alternative function to be configured for the specified
> +          pins.
> +
> +        enum: [ adsp_ext, agera_pll, atest_char, atest_char0, atest_char1, atest_char2,
> +                atest_char3, atest_tsens, atest_tsens2, atest_usb1, atest_usb10, atest_usb11,
> +                atest_usb12, atest_usb13, atest_usb2, atest_usb20, atest_usb21, atest_usb22,
> +                atest_usb23, audio_ref, btfm_slimbus, cam_mclk0, cam_mclk1, cam_mclk2, cam_mclk3,
> +                cam_mclk4, cci_async, cci_i2c, cci_timer0, cci_timer1, cci_timer2, cci_timer3,
> +                cci_timer4, cri_trng, dbg_out, ddr_bist, ddr_pxi0, ddr_pxi1, ddr_pxi2, ddr_pxi3,
> +                dp_hot, edp_lcd, gcc_gp1, gcc_gp2, gcc_gp3, gp_pdm0, gp_pdm1, gp_pdm2, gpio,
> +                gps_tx, ibi_i3c, jitter_bist, ldo_en, ldo_update, lpass_ext, m_voc, mclk,
> +                mdp_vsync, mdp_vsync0, mdp_vsync1, mdp_vsync2, mdp_vsync3, mi2s_0, mi2s_1, mi2s_2,
> +                mss_lte, nav_gpio, nav_pps, pa_indicator, pcie0_clk, phase_flag0, phase_flag1,
> +                phase_flag10, phase_flag11, phase_flag12, phase_flag13, phase_flag14, phase_flag15,
> +                phase_flag16, phase_flag17, phase_flag18, phase_flag19, phase_flag2, phase_flag20,
> +                phase_flag21, phase_flag22, phase_flag23, phase_flag24, phase_flag25, phase_flag26,
> +                phase_flag27, phase_flag28, phase_flag29, phase_flag3, phase_flag30, phase_flag31,
> +                phase_flag4, phase_flag5, phase_flag6, phase_flag7, phase_flag8, phase_flag9,
> +                pll_bist, pll_bypassnl, pll_reset, prng_rosc, qdss_cti, qdss_gpio, qdss_gpio0,
> +                qdss_gpio1, qdss_gpio10, qdss_gpio11, qdss_gpio12, qdss_gpio13, qdss_gpio14,
> +                qdss_gpio15, qdss_gpio2, qdss_gpio3, qdss_gpio4, qdss_gpio5, qdss_gpio6,
> +                qdss_gpio7, qdss_gpio8, qdss_gpio9, qlink0_enable, qlink0_request, qlink0_wmss,
> +                qlink1_enable, qlink1_request, qlink1_wmss, qup00, qup01, qup02, qup10, qup11,
> +                qup12, qup13_f1, qup13_f2, qup14, rffe0_clk, rffe0_data, rffe1_clk, rffe1_data,
> +                rffe2_clk, rffe2_data, rffe3_clk, rffe3_data, rffe4_clk, rffe4_data, sd_write,
> +                sdc1_tb, sdc2_tb, sp_cmu, tgu_ch0, tgu_ch1, tgu_ch2, tgu_ch3, tsense_pwm1,
> +                tsense_pwm2, uim1_clk, uim1_data, uim1_present, uim1_reset, uim2_clk, uim2_data,
> +                uim2_present, uim2_reset, usb_phy, vfr_1, vsense_trigger, wlan1_adc0, wlan1_adc1,
> +                wlan2_adc0, wlan2_adc1, ]
> +
> +
> +      bias-disable: true
> +      bias-pull-down: true
> +      bias-pull-up: true
> +      drive-strength: true
> +      input-enable: true
> +      output-high: true
> +      output-low: true
> +
> +    required:
> +      - pins
> +      - function
> +
> +    additionalProperties: false
> +
> +examples:
> +  - |
> +        #include <dt-bindings/interrupt-controller/arm-gic.h>
> +        pinctrl@f100000 {
> +                compatible = "qcom,sm6350-tlmm";
> +                reg = <0x0f100000 0x300000>;
> +                interrupts = <GIC_SPI 208 IRQ_TYPE_LEVEL_HIGH>;
> +                gpio-controller;
> +                #gpio-cells = <2>;
> +                interrupt-controller;
> +                #interrupt-cells = <2>;
> +                gpio-ranges = <&tlmm 0 0 157>;
> +
> +                gpio-wo-subnode-state {
> +                        pins = "gpio1";
> +                        function = "gpio";
> +                };
> +
> +                uart-w-subnodes-state {
> +                        rx {
> +                                pins = "gpio25";
> +                                function = "qup13_f2";
> +                                bias-disable;
> +                        };
> +
> +                        tx {
> +                                pins = "gpio26";
> +                                function = "qup13_f2";
> +                                bias-disable;
> +                        };
> +                };
> +        };
> +...
> -- 
> 2.33.0
>

On Sat, 28 Aug 2021 19:23:13 +0200, Konrad Dybcio wrote:
> Add device tree binding Documentation details for Qualcomm SM6350
> pinctrl driver.
> 
> Signed-off-by: Konrad Dybcio <konrad.dybcio@somainline.org>
> ---
> Changes since v2:
> - Tweak the regex to match gpio0-gpio157
> 
>  .../bindings/pinctrl/qcom,sm6350-pinctrl.yaml | 148 ++++++++++++++++++
>  1 file changed, 148 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/pinctrl/qcom,sm6350-pinctrl.yaml
> 

Reviewed-by: Rob Herring <robh@kernel.org>

Hi Konrad,

based on other reviews on the mailing list/IRC I have some comments here

On Samstag, 28. August 2021 19:23:14 CEST Konrad Dybcio wrote:
> This adds pincontrol driver for tlmm block found in SM6350 SoC

> 

> This patch is based on downstream copyleft code.

> 

> Reviewed-by: AngeloGioacchino Del Regno

> <angelogioacchino.delregno@somainline.org> Signed-off-by: Konrad Dybcio

> <konrad.dybcio@somainline.org>

> Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>

> ---

> Changes since v2:

> - Trim the forgotten-about comments

> - Add Bjorn's r-b

> 

>  drivers/pinctrl/qcom/Kconfig          |    9 +

>  drivers/pinctrl/qcom/Makefile         |    1 +

>  drivers/pinctrl/qcom/pinctrl-sm6350.c | 1593 +++++++++++++++++++++++++

>  3 files changed, 1603 insertions(+)

>  create mode 100644 drivers/pinctrl/qcom/pinctrl-sm6350.c

> 

[SNIP]
>

> +DECLARE_MSM_GPIO_PINS(128);

> +DECLARE_MSM_GPIO_PINS(129);

> +DECLARE_MSM_GPIO_PINS(130);

> +DECLARE_MSM_GPIO_PINS(131);

> +DECLARE_MSM_GPIO_PINS(132);

> +DECLARE_MSM_GPIO_PINS(133);

> +DECLARE_MSM_GPIO_PINS(134);

> +DECLARE_MSM_GPIO_PINS(135);

> +DECLARE_MSM_GPIO_PINS(136);

> +DECLARE_MSM_GPIO_PINS(137);

> +DECLARE_MSM_GPIO_PINS(138);

> +DECLARE_MSM_GPIO_PINS(139);

> +DECLARE_MSM_GPIO_PINS(140);

> +DECLARE_MSM_GPIO_PINS(141);

> +DECLARE_MSM_GPIO_PINS(142);

> +DECLARE_MSM_GPIO_PINS(143);

> +DECLARE_MSM_GPIO_PINS(144);

> +DECLARE_MSM_GPIO_PINS(145);

> +DECLARE_MSM_GPIO_PINS(146);

> +DECLARE_MSM_GPIO_PINS(147);

> +DECLARE_MSM_GPIO_PINS(148);

> +DECLARE_MSM_GPIO_PINS(149);

> +DECLARE_MSM_GPIO_PINS(150);

> +DECLARE_MSM_GPIO_PINS(151);

> +DECLARE_MSM_GPIO_PINS(152);

> +DECLARE_MSM_GPIO_PINS(153);

> +DECLARE_MSM_GPIO_PINS(154);

> +DECLARE_MSM_GPIO_PINS(155);

> +

> +static const unsigned int sdc1_rclk_pins[] = { 156 };

> +static const unsigned int sdc1_clk_pins[] = { 157 };

> +static const unsigned int sdc1_cmd_pins[] = { 158 };

> +static const unsigned int sdc1_data_pins[] = { 159 };

> +static const unsigned int sdc2_clk_pins[] = { 160 };

> +static const unsigned int sdc2_cmd_pins[] = { 161 };

> +static const unsigned int sdc2_data_pins[] = { 162 };

> +static const unsigned int ufs_reset_pins[] = { 163 };


All these numbers don't match anymore after moving ufs_reset to 156

(ref: https://lore.kernel.org/lkml/YNTYvKYDWFxUcb+Y@yoga/ )

> +

> +enum sm6350_functions {

> +	msm_mux_adsp_ext,

> +	msm_mux_agera_pll,

> +	msm_mux_atest_char,

> +	msm_mux_atest_char0,

> +	msm_mux_atest_char1,

> +	msm_mux_atest_char2,

> +	msm_mux_atest_char3,

> +	msm_mux_atest_tsens,

> +	msm_mux_atest_tsens2,

> +	msm_mux_atest_usb1,

> +	msm_mux_atest_usb10,

> +	msm_mux_atest_usb11,

> +	msm_mux_atest_usb12,

> +	msm_mux_atest_usb13,

> +	msm_mux_atest_usb2,

> +	msm_mux_atest_usb20,

> +	msm_mux_atest_usb21,

> +	msm_mux_atest_usb22,

> +	msm_mux_atest_usb23,


Bjorn mentioned to merge all the atest_usb* functions into a single one.

> +	msm_mux_audio_ref,

> +	msm_mux_btfm_slimbus,

> +	msm_mux_cam_mclk0,

> +	msm_mux_cam_mclk1,

> +	msm_mux_cam_mclk2,

> +	msm_mux_cam_mclk3,

> +	msm_mux_cam_mclk4,

> +	msm_mux_cci_async,

> +	msm_mux_cci_i2c,

> +	msm_mux_cci_timer0,

> +	msm_mux_cci_timer1,

> +	msm_mux_cci_timer2,

> +	msm_mux_cci_timer3,

> +	msm_mux_cci_timer4,

> +	msm_mux_cri_trng,

> +	msm_mux_dbg_out,

> +	msm_mux_ddr_bist,

> +	msm_mux_ddr_pxi0,

> +	msm_mux_ddr_pxi1,

> +	msm_mux_ddr_pxi2,

> +	msm_mux_ddr_pxi3,

> +	msm_mux_dp_hot,

> +	msm_mux_edp_lcd,

> +	msm_mux_gcc_gp1,

> +	msm_mux_gcc_gp2,

> +	msm_mux_gcc_gp3,

> +	msm_mux_gp_pdm0,

> +	msm_mux_gp_pdm1,

> +	msm_mux_gp_pdm2,

> +	msm_mux_gpio,

> +	msm_mux_gps_tx,

> +	msm_mux_ibi_i3c,

> +	msm_mux_jitter_bist,

> +	msm_mux_ldo_en,

> +	msm_mux_ldo_update,

> +	msm_mux_lpass_ext,

> +	msm_mux_m_voc,

> +	msm_mux_mclk,

> +	msm_mux_mdp_vsync,

> +	msm_mux_mdp_vsync0,

> +	msm_mux_mdp_vsync1,

> +	msm_mux_mdp_vsync2,

> +	msm_mux_mdp_vsync3,

> +	msm_mux_mi2s_0,

> +	msm_mux_mi2s_1,

> +	msm_mux_mi2s_2,

> +	msm_mux_mss_lte,

> +	msm_mux_nav_gpio,

> +	msm_mux_nav_pps,

> +	msm_mux_pa_indicator,

> +	msm_mux_pcie0_clk,

> +	msm_mux_phase_flag0,

> +	msm_mux_phase_flag1,

> +	msm_mux_phase_flag10,

> +	msm_mux_phase_flag11,

> +	msm_mux_phase_flag12,

> +	msm_mux_phase_flag13,

> +	msm_mux_phase_flag14,

> +	msm_mux_phase_flag15,

> +	msm_mux_phase_flag16,

> +	msm_mux_phase_flag17,

> +	msm_mux_phase_flag18,

> +	msm_mux_phase_flag19,

> +	msm_mux_phase_flag2,

> +	msm_mux_phase_flag20,

> +	msm_mux_phase_flag21,

> +	msm_mux_phase_flag22,

> +	msm_mux_phase_flag23,

> +	msm_mux_phase_flag24,

> +	msm_mux_phase_flag25,

> +	msm_mux_phase_flag26,

> +	msm_mux_phase_flag27,

> +	msm_mux_phase_flag28,

> +	msm_mux_phase_flag29,

> +	msm_mux_phase_flag3,

> +	msm_mux_phase_flag30,

> +	msm_mux_phase_flag31,

> +	msm_mux_phase_flag4,

> +	msm_mux_phase_flag5,

> +	msm_mux_phase_flag6,

> +	msm_mux_phase_flag7,

> +	msm_mux_phase_flag8,

> +	msm_mux_phase_flag9,


.. and all the phase_flag* ones.

> +	msm_mux_pll_bist,

> +	msm_mux_pll_bypassnl,

> +	msm_mux_pll_reset,

> +	msm_mux_prng_rosc,

> +	msm_mux_qdss_cti,

> +	msm_mux_qdss_gpio,

> +	msm_mux_qdss_gpio0,

> +	msm_mux_qdss_gpio1,

> +	msm_mux_qdss_gpio10,

> +	msm_mux_qdss_gpio11,

> +	msm_mux_qdss_gpio12,

> +	msm_mux_qdss_gpio13,

> +	msm_mux_qdss_gpio14,

> +	msm_mux_qdss_gpio15,

> +	msm_mux_qdss_gpio2,

> +	msm_mux_qdss_gpio3,

> +	msm_mux_qdss_gpio4,

> +	msm_mux_qdss_gpio5,

> +	msm_mux_qdss_gpio6,

> +	msm_mux_qdss_gpio7,

> +	msm_mux_qdss_gpio8,

> +	msm_mux_qdss_gpio9,

> +	msm_mux_qlink0_enable,

> +	msm_mux_qlink0_request,

> +	msm_mux_qlink0_wmss,

> +	msm_mux_qlink1_enable,

> +	msm_mux_qlink1_request,

> +	msm_mux_qlink1_wmss,

> +	msm_mux_qup00,

> +	msm_mux_qup01,

> +	msm_mux_qup02,

> +	msm_mux_qup10,

> +	msm_mux_qup11,

> +	msm_mux_qup12,

> +	msm_mux_qup13_f1,

> +	msm_mux_qup13_f2,

> +	msm_mux_qup14,

> +	msm_mux_rffe0_clk,

> +	msm_mux_rffe0_data,

> +	msm_mux_rffe1_clk,

> +	msm_mux_rffe1_data,

> +	msm_mux_rffe2_clk,

> +	msm_mux_rffe2_data,

> +	msm_mux_rffe3_clk,

> +	msm_mux_rffe3_data,

> +	msm_mux_rffe4_clk,

> +	msm_mux_rffe4_data,

> +	msm_mux_sd_write,

> +	msm_mux_sdc1_tb,

> +	msm_mux_sdc2_tb,

> +	msm_mux_sp_cmu,

> +	msm_mux_tgu_ch0,

> +	msm_mux_tgu_ch1,

> +	msm_mux_tgu_ch2,

> +	msm_mux_tgu_ch3,

> +	msm_mux_tsense_pwm1,

> +	msm_mux_tsense_pwm2,

> +	msm_mux_uim1_clk,

> +	msm_mux_uim1_data,

> +	msm_mux_uim1_present,

> +	msm_mux_uim1_reset,


maybe even uim1_* into uim1?

> +	msm_mux_uim2_clk,

> +	msm_mux_uim2_data,

> +	msm_mux_uim2_present,

> +	msm_mux_uim2_reset,


.. and uim2?

> +	msm_mux_usb_phy,

> +	msm_mux_vfr_1,

> +	msm_mux_vsense_trigger,

> +	msm_mux_wlan1_adc0,

> +	msm_mux_wlan1_adc1,

> +	msm_mux_wlan2_adc0,

> +	msm_mux_wlan2_adc1,

> +	msm_mux__,

> +};

> +

>

[SNIP]
>

> +

> +static const struct msm_pinctrl_soc_data sm6350_pinctrl = {

> +	.pins = sm6350_pins,

> +	.npins = ARRAY_SIZE(sm6350_pins),

> +	.functions = sm6350_functions,

> +	.nfunctions = ARRAY_SIZE(sm6350_functions),

> +	.groups = sm6350_groups,

> +	.ngroups = ARRAY_SIZE(sm6350_groups),

> +	.ngpios = 157,

> +	.wakeirq_map = sm6350_pdc_map,

> +	.nwakeirq_map = ARRAY_SIZE(sm6350_pdc_map),

> +	.wakeirq_dual_edge_errata = true,

> +};

> +

> +static int sm6350_pinctrl_probe(struct platform_device *pdev)

> +{

> +	return msm_pinctrl_probe(pdev, &sm6350_pinctrl);

> +}

> +

> +static const struct of_device_id sm6350_pinctrl_of_match[] = {

> +	{ .compatible = "qcom,sm6350-tlmm", },

> +	{ },


No need for a trailing comma here ;)

> +};

> +

> +static struct platform_driver sm6350_pinctrl_driver = {

> +	.driver = {

> +		.name = "sm6350-pinctrl",

> +		.of_match_table = sm6350_pinctrl_of_match,

> +	},

> +	.probe = sm6350_pinctrl_probe,

> +	.remove = msm_pinctrl_remove,

> +};

> +

> +static int __init sm6350_pinctrl_init(void)

> +{

> +	return platform_driver_register(&sm6350_pinctrl_driver);

> +}

> +arch_initcall(sm6350_pinctrl_init);

> +

> +static void __exit sm6350_pinctrl_exit(void)

> +{

> +	platform_driver_unregister(&sm6350_pinctrl_driver);

> +}

> +module_exit(sm6350_pinctrl_exit);

> +

> +MODULE_DESCRIPTION("QTI sm6350 pinctrl driver");

> +MODULE_LICENSE("GPL v2");

> +MODULE_DEVICE_TABLE(of, sm6350_pinctrl_of_match);


Some/most(?) newer drivers also use the name tlmm instead of pinctrl in the 
function names and in the .name of the driver.

Regards,
Luca