| Message ID | 1602831532-24818-1-git-send-email-rnayak@codeaurora.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [1/2] dt-bindings: pinctrl: qcom: Add sc7280 pinctrl bindings | expand |
On Fri, 16 Oct 2020 12:28:51 +0530, Rajendra Nayak wrote: > Add device tree binding Documentation details for Qualcomm SC7280 > TLMM block. > > Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org> > --- > .../bindings/pinctrl/qcom,sc7280-pinctrl.yaml | 170 +++++++++++++++++++++ > 1 file changed, 170 insertions(+) > create mode 100644 Documentation/devicetree/bindings/pinctrl/qcom,sc7280-pinctrl.yaml > Reviewed-by: Rob Herring <robh@kernel.org>
On Fri, Oct 16, 2020 at 8:59 AM Rajendra Nayak <rnayak@codeaurora.org> wrote: > Add initial pinctrl driver to support pin configuration with > pinctrl framework for SC7280 SoC > > Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org> This came in during the merge window I think, waiting for Bjorn to have a look at it. Yours, Linus Walleij
On Fri 16 Oct 01:58 CDT 2020, Rajendra Nayak wrote: > diff --git a/drivers/pinctrl/qcom/pinctrl-sc7280.c b/drivers/pinctrl/qcom/pinctrl-sc7280.c [..] > +static const struct msm_function sc7280_functions[] = { [..] > + FUNCTION(phase_flag0), > + FUNCTION(phase_flag1), > + FUNCTION(phase_flag10), > + FUNCTION(phase_flag11), > + FUNCTION(phase_flag12), > + FUNCTION(phase_flag13), > + FUNCTION(phase_flag14), > + FUNCTION(phase_flag15), > + FUNCTION(phase_flag16), > + FUNCTION(phase_flag17), > + FUNCTION(phase_flag18), > + FUNCTION(phase_flag19), > + FUNCTION(phase_flag2), > + FUNCTION(phase_flag20), > + FUNCTION(phase_flag21), > + FUNCTION(phase_flag22), > + FUNCTION(phase_flag23), > + FUNCTION(phase_flag24), > + FUNCTION(phase_flag25), > + FUNCTION(phase_flag26), > + FUNCTION(phase_flag27), > + FUNCTION(phase_flag28), > + FUNCTION(phase_flag29), > + FUNCTION(phase_flag3), > + FUNCTION(phase_flag30), > + FUNCTION(phase_flag31), > + FUNCTION(phase_flag4), > + FUNCTION(phase_flag5), > + FUNCTION(phase_flag6), > + FUNCTION(phase_flag7), > + FUNCTION(phase_flag8), > + FUNCTION(phase_flag9), I prefer when we squash these into a single function. > + FUNCTION(pll_bist), > + FUNCTION(pll_bypassnl), > + FUNCTION(pll_clk), > + FUNCTION(pll_reset), > + FUNCTION(pri_mi2s), > + FUNCTION(prng_rosc), > + FUNCTION(qdss_cti), > + FUNCTION(qdss_gpio), > + FUNCTION(qdss_gpio0), > + FUNCTION(qdss_gpio1), > + FUNCTION(qdss_gpio10), > + FUNCTION(qdss_gpio11), > + FUNCTION(qdss_gpio12), > + FUNCTION(qdss_gpio13), > + FUNCTION(qdss_gpio14), > + FUNCTION(qdss_gpio15), > + FUNCTION(qdss_gpio2), > + FUNCTION(qdss_gpio3), > + FUNCTION(qdss_gpio4), > + FUNCTION(qdss_gpio5), > + FUNCTION(qdss_gpio6), > + FUNCTION(qdss_gpio7), > + FUNCTION(qdss_gpio8), > + FUNCTION(qdss_gpio9), Ditto. > + FUNCTION(qlink0_enable), > + FUNCTION(qlink0_request), > + FUNCTION(qlink0_wmss), > + FUNCTION(qlink1_enable), > + FUNCTION(qlink1_request), > + FUNCTION(qlink1_wmss), > + FUNCTION(qspi_clk), > + FUNCTION(qspi_cs), > + FUNCTION(qspi_data), > + FUNCTION(qup00), > + FUNCTION(qup01), > + FUNCTION(qup02), > + FUNCTION(qup03), > + FUNCTION(qup04), > + FUNCTION(qup05), > + FUNCTION(qup06), > + FUNCTION(qup07), > + FUNCTION(qup10), > + FUNCTION(qup11), > + FUNCTION(qup12), > + FUNCTION(qup13), > + FUNCTION(qup14), > + FUNCTION(qup15), > + FUNCTION(qup16), > + FUNCTION(qup17), > + FUNCTION(sdc40), > + FUNCTION(sdc41), > + FUNCTION(sdc42), > + FUNCTION(sdc43), > + FUNCTION(sdc4_clk), > + FUNCTION(sdc4_cmd), > + FUNCTION(sd_write), > + FUNCTION(sec_mi2s), > + FUNCTION(tb_trig), > + FUNCTION(tgu_ch0), > + FUNCTION(tgu_ch1), > + FUNCTION(tsense_pwm1), > + FUNCTION(tsense_pwm2), > + FUNCTION(uim0_clk), > + FUNCTION(uim0_data), > + FUNCTION(uim0_present), > + FUNCTION(uim0_reset), > + FUNCTION(uim1_clk), > + FUNCTION(uim1_data), > + FUNCTION(uim1_present), > + FUNCTION(uim1_reset), > + FUNCTION(usb2phy_ac), > + FUNCTION(usb_phy), > + FUNCTION(vfr_0), > + FUNCTION(vfr_1), > + FUNCTION(vsense_trigger), > +}; > + > +/* Every pin is maintained as a single group, and missing or non-existing pin > + * would be maintained as dummy group to synchronize pin group index with > + * pin descriptor registered with pinctrl core. > + * Clients would not be able to request these dummy pin groups. > + */ > +static const struct msm_pingroup sc7280_groups[] = { [..] > + [174] = PINGROUP(174, qdss_gpio15, _, _, _, _, _, _, _, _), > + [175] = SDC_QDSD_PINGROUP(sdc1_rclk, 0x1b3000, 15, 0), > + [176] = SDC_QDSD_PINGROUP(sdc1_clk, 0x1b3000, 13, 6), > + [177] = SDC_QDSD_PINGROUP(sdc1_cmd, 0x1b3000, 11, 3), > + [178] = SDC_QDSD_PINGROUP(sdc1_data, 0x1b3000, 9, 0), > + [179] = SDC_QDSD_PINGROUP(sdc2_clk, 0x1b4000, 14, 6), > + [180] = SDC_QDSD_PINGROUP(sdc2_cmd, 0x1b4000, 11, 3), > + [181] = SDC_QDSD_PINGROUP(sdc2_data, 0x1b4000, 9, 0), > + [182] = UFS_RESET(ufs_reset, 0x1be000), Rather than fiddling ufs reset using pinconf we expose it as a gp(i)o pin upstream. So please move this to be the 175th item in the list and bump ngpios to 176 below. > +}; > + > +static const struct msm_pinctrl_soc_data sc7280_pinctrl = { > + .pins = sc7280_pins, > + .npins = ARRAY_SIZE(sc7280_pins), > + .functions = sc7280_functions, > + .nfunctions = ARRAY_SIZE(sc7280_functions), > + .groups = sc7280_groups, > + .ngroups = ARRAY_SIZE(sc7280_groups), > + .ngpios = 175, > +}; > + Apart from that it looks good. Regards, Bjorn
On 10/29/2020 7:50 PM, Bjorn Andersson wrote: > On Fri 16 Oct 01:58 CDT 2020, Rajendra Nayak wrote: >> diff --git a/drivers/pinctrl/qcom/pinctrl-sc7280.c b/drivers/pinctrl/qcom/pinctrl-sc7280.c > [..] >> +static const struct msm_function sc7280_functions[] = { > [..] >> + FUNCTION(phase_flag0), >> + FUNCTION(phase_flag1), >> + FUNCTION(phase_flag10), >> + FUNCTION(phase_flag11), >> + FUNCTION(phase_flag12), >> + FUNCTION(phase_flag13), >> + FUNCTION(phase_flag14), >> + FUNCTION(phase_flag15), >> + FUNCTION(phase_flag16), >> + FUNCTION(phase_flag17), >> + FUNCTION(phase_flag18), >> + FUNCTION(phase_flag19), >> + FUNCTION(phase_flag2), >> + FUNCTION(phase_flag20), >> + FUNCTION(phase_flag21), >> + FUNCTION(phase_flag22), >> + FUNCTION(phase_flag23), >> + FUNCTION(phase_flag24), >> + FUNCTION(phase_flag25), >> + FUNCTION(phase_flag26), >> + FUNCTION(phase_flag27), >> + FUNCTION(phase_flag28), >> + FUNCTION(phase_flag29), >> + FUNCTION(phase_flag3), >> + FUNCTION(phase_flag30), >> + FUNCTION(phase_flag31), >> + FUNCTION(phase_flag4), >> + FUNCTION(phase_flag5), >> + FUNCTION(phase_flag6), >> + FUNCTION(phase_flag7), >> + FUNCTION(phase_flag8), >> + FUNCTION(phase_flag9), > > I prefer when we squash these into a single function. > >> + FUNCTION(pll_bist), >> + FUNCTION(pll_bypassnl), >> + FUNCTION(pll_clk), >> + FUNCTION(pll_reset), >> + FUNCTION(pri_mi2s), >> + FUNCTION(prng_rosc), >> + FUNCTION(qdss_cti), >> + FUNCTION(qdss_gpio), >> + FUNCTION(qdss_gpio0), >> + FUNCTION(qdss_gpio1), >> + FUNCTION(qdss_gpio10), >> + FUNCTION(qdss_gpio11), >> + FUNCTION(qdss_gpio12), >> + FUNCTION(qdss_gpio13), >> + FUNCTION(qdss_gpio14), >> + FUNCTION(qdss_gpio15), >> + FUNCTION(qdss_gpio2), >> + FUNCTION(qdss_gpio3), >> + FUNCTION(qdss_gpio4), >> + FUNCTION(qdss_gpio5), >> + FUNCTION(qdss_gpio6), >> + FUNCTION(qdss_gpio7), >> + FUNCTION(qdss_gpio8), >> + FUNCTION(qdss_gpio9), > > Ditto. > >> + FUNCTION(qlink0_enable), >> + FUNCTION(qlink0_request), >> + FUNCTION(qlink0_wmss), >> + FUNCTION(qlink1_enable), >> + FUNCTION(qlink1_request), >> + FUNCTION(qlink1_wmss), >> + FUNCTION(qspi_clk), >> + FUNCTION(qspi_cs), >> + FUNCTION(qspi_data), >> + FUNCTION(qup00), >> + FUNCTION(qup01), >> + FUNCTION(qup02), >> + FUNCTION(qup03), >> + FUNCTION(qup04), >> + FUNCTION(qup05), >> + FUNCTION(qup06), >> + FUNCTION(qup07), >> + FUNCTION(qup10), >> + FUNCTION(qup11), >> + FUNCTION(qup12), >> + FUNCTION(qup13), >> + FUNCTION(qup14), >> + FUNCTION(qup15), >> + FUNCTION(qup16), >> + FUNCTION(qup17), >> + FUNCTION(sdc40), >> + FUNCTION(sdc41), >> + FUNCTION(sdc42), >> + FUNCTION(sdc43), >> + FUNCTION(sdc4_clk), >> + FUNCTION(sdc4_cmd), >> + FUNCTION(sd_write), >> + FUNCTION(sec_mi2s), >> + FUNCTION(tb_trig), >> + FUNCTION(tgu_ch0), >> + FUNCTION(tgu_ch1), >> + FUNCTION(tsense_pwm1), >> + FUNCTION(tsense_pwm2), >> + FUNCTION(uim0_clk), >> + FUNCTION(uim0_data), >> + FUNCTION(uim0_present), >> + FUNCTION(uim0_reset), >> + FUNCTION(uim1_clk), >> + FUNCTION(uim1_data), >> + FUNCTION(uim1_present), >> + FUNCTION(uim1_reset), >> + FUNCTION(usb2phy_ac), >> + FUNCTION(usb_phy), >> + FUNCTION(vfr_0), >> + FUNCTION(vfr_1), >> + FUNCTION(vsense_trigger), >> +}; >> + >> +/* Every pin is maintained as a single group, and missing or non-existing pin >> + * would be maintained as dummy group to synchronize pin group index with >> + * pin descriptor registered with pinctrl core. >> + * Clients would not be able to request these dummy pin groups. >> + */ >> +static const struct msm_pingroup sc7280_groups[] = { > [..] >> + [174] = PINGROUP(174, qdss_gpio15, _, _, _, _, _, _, _, _), >> + [175] = SDC_QDSD_PINGROUP(sdc1_rclk, 0x1b3000, 15, 0), >> + [176] = SDC_QDSD_PINGROUP(sdc1_clk, 0x1b3000, 13, 6), >> + [177] = SDC_QDSD_PINGROUP(sdc1_cmd, 0x1b3000, 11, 3), >> + [178] = SDC_QDSD_PINGROUP(sdc1_data, 0x1b3000, 9, 0), >> + [179] = SDC_QDSD_PINGROUP(sdc2_clk, 0x1b4000, 14, 6), >> + [180] = SDC_QDSD_PINGROUP(sdc2_cmd, 0x1b4000, 11, 3), >> + [181] = SDC_QDSD_PINGROUP(sdc2_data, 0x1b4000, 9, 0), >> + [182] = UFS_RESET(ufs_reset, 0x1be000), > > Rather than fiddling ufs reset using pinconf we expose it as a gp(i)o > pin upstream. So please move this to be the 175th item in the list and > bump ngpios to 176 below. > >> +}; >> + >> +static const struct msm_pinctrl_soc_data sc7280_pinctrl = { >> + .pins = sc7280_pins, >> + .npins = ARRAY_SIZE(sc7280_pins), >> + .functions = sc7280_functions, >> + .nfunctions = ARRAY_SIZE(sc7280_functions), >> + .groups = sc7280_groups, >> + .ngroups = ARRAY_SIZE(sc7280_groups), >> + .ngpios = 175, >> +}; >> + > > Apart from that it looks good. thanks Bjorn, I'll fix-up and re-post.
diff --git a/Documentation/devicetree/bindings/pinctrl/qcom,sc7280-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/qcom,sc7280-pinctrl.yaml new file mode 100644 index 0000000..971cfaf --- /dev/null +++ b/Documentation/devicetree/bindings/pinctrl/qcom,sc7280-pinctrl.yaml @@ -0,0 +1,170 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/pinctrl/qcom,sc7280-pinctrl.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Qualcomm Technologies, Inc. SC7280 TLMM block + +maintainers: + - Rajendra Nayak <rnayak@codeaurora.org> + +description: | + This binding describes the Top Level Mode Multiplexer block found in the + SC7280 platform. + +properties: + compatible: + const: qcom,sc7280-pinctrl + + reg: + maxItems: 1 + + interrupts: + description: Specifies the TLMM summary IRQ + maxItems: 1 + + interrupt-controller: true + + '#interrupt-cells': + description: + Specifies the PIN numbers and Flags, as defined in defined in + include/dt-bindings/interrupt-controller/irq.h + const: 2 + + gpio-controller: true + + '#gpio-cells': + description: Specifying the pin number and flags, as defined in + include/dt-bindings/gpio/gpio.h + const: 2 + + gpio-ranges: + maxItems: 1 + + wakeup-parent: + maxItems: 1 + +#PIN CONFIGURATION NODES +patternProperties: + '-pins$': + type: object + description: + Pinctrl node's client devices use subnodes for desired pin configuration. + Client device subnodes use below standard properties. + $ref: "/schemas/pinctrl/pincfg-node.yaml" + + 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-7][0-4])$" + - enum: [ sdc1_rclk, sdc1_clk, sdc1_cmd, sdc1_data, sdc2_clk, + sdc2_cmd, sdc2_data, ufs_reset ] + minItems: 1 + maxItems: 16 + + function: + description: + Specify the alternative function to be configured for the specified + pins. + + enum: [ atest_char, atest_char0, atest_char1, atest_char2, + atest_char3, atest_usb0, atest_usb00, atest_usb01, + atest_usb02, atest_usb03, atest_usb1, atest_usb10, + atest_usb11, atest_usb12, atest_usb13, audio_ref, + cam_mclk, cci_async, cci_i2c, cci_timer0, cci_timer1, + cci_timer2, cci_timer3, cci_timer4, cmu_rng0, cmu_rng1, + cmu_rng2, cmu_rng3, coex_uart1, cri_trng, cri_trng0, + cri_trng1, dbg_out, ddr_bist, ddr_pxi0, ddr_pxi1, dp_hot, + dp_lcd, edp_hot, edp_lcd, gcc_gp1, gcc_gp2, gcc_gp3, + gpio, host2wlan_sol, ibi_i3c, jitter_bist, lpass_slimbus, + mdp_vsync, mdp_vsync0, mdp_vsync1, mdp_vsync2, mdp_vsync3, + mdp_vsync4, mdp_vsync5, mi2s0_data0, mi2s0_data1, mi2s0_sck, + mi2s0_ws, mi2s1_data0, mi2s1_data1, mi2s1_sck, mi2s1_ws, + mi2s2_data0, mi2s2_data1, mi2s2_sck, mi2s2_ws, mss_grfc0, + mss_grfc1, mss_grfc10, mss_grfc11, mss_grfc12, mss_grfc2, + mss_grfc3, mss_grfc4, mss_grfc5, mss_grfc6, mss_grfc7, + mss_grfc8, mss_grfc9, nav_gpio0, nav_gpio1, nav_gpio2, + pa_indicator, pcie0_clkreqn, pcie1_clkreqn, 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_clk, pll_reset, pri_mi2s, 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, qspi_clk, qspi_cs, qspi_data, + qup00, qup01, qup02, qup03, qup04, qup05, qup06, qup07, + qup10, qup11, qup12, qup13, qup14, qup15, qup16, qup17, + sdc40, sdc41, sdc42, sdc43, sdc4_clk, sdc4_cmd, sd_write, + sec_mi2s, tb_trig, tgu_ch0, tgu_ch1, tsense_pwm1, + tsense_pwm2, uim0_clk, uim0_data, uim0_present, uim0_reset, + uim1_clk, uim1_data, uim1_present, uim1_reset, usb2phy_ac, + usb_phy, vfr_0, vfr_1, vsense_trigger ] + + drive-strength: + enum: [2, 4, 6, 8, 10, 12, 14, 16] + default: 2 + description: + Selects the drive strength for the specified pins, in mA. + + bias-pull-down: true + + bias-pull-up: true + + bias-disable: true + + output-high: true + + output-low: true + + required: + - pins + - function + + additionalProperties: false + +required: + - compatible + - reg + - interrupts + - interrupt-controller + - '#interrupt-cells' + - gpio-controller + - '#gpio-cells' + - gpio-ranges + +additionalProperties: false + +examples: + - | + #include <dt-bindings/interrupt-controller/arm-gic.h> + tlmm: pinctrl@f000000 { + compatible = "qcom,sc7280-pinctrl"; + reg = <0xf000000 0x1000000>; + interrupts = <GIC_SPI 208 IRQ_TYPE_LEVEL_HIGH>; + gpio-controller; + #gpio-cells = <2>; + interrupt-controller; + #interrupt-cells = <2>; + gpio-ranges = <&tlmm 0 0 175>; + wakeup-parent = <&pdc>; + + qup_uart5_default: qup-uart5-pins { + pins = "gpio46", "gpio47"; + function = "qup13"; + drive-strength = <2>; + bias-disable; + }; + };
Add device tree binding Documentation details for Qualcomm SC7280 TLMM block. Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org> --- .../bindings/pinctrl/qcom,sc7280-pinctrl.yaml | 170 +++++++++++++++++++++ 1 file changed, 170 insertions(+) create mode 100644 Documentation/devicetree/bindings/pinctrl/qcom,sc7280-pinctrl.yaml