Re: [PATCH 10/18] pinctrl: qcom: sa8775p: add the pinctrl driver for the sa8775p platform
From: Bartosz Golaszewski <hidden>
Date: 2023-01-19 08:43:52
Also in:
linux-arm-kernel, linux-arm-msm, linux-clk, linux-devicetree, linux-gpio, linux-iommu, linux-pm, lkml
On Tue, Jan 10, 2023 at 5:26 PM Bjorn Andersson [off-list ref] wrote:
On Mon, Jan 09, 2023 at 06:45:03PM +0100, Bartosz Golaszewski wrote: [..]quoted
+enum sa8775p_functions { + msm_mux_gpio, + msm_mux_atest_char, + msm_mux_atest_char0, + msm_mux_atest_char1, + msm_mux_atest_char2, + msm_mux_atest_char3, + msm_mux_atest_usb2, + msm_mux_atest_usb20, + msm_mux_atest_usb21, + msm_mux_atest_usb22, + msm_mux_atest_usb23,Please squash these to a single msm_mux_atest.
How about staying consistent with the sc8280xp which is the closest platform to sa8775p and do a group for atest_char, a separate group for atest_usb2...
quoted
+ msm_mux_audio_ref, + msm_mux_cam_mclk, + 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_cci_timer5, + msm_mux_cci_timer6, + msm_mux_cci_timer7, + msm_mux_cci_timer8, + msm_mux_cci_timer9, + msm_mux_cri_trng, + msm_mux_cri_trng0, + msm_mux_cri_trng1, + 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_ddr_pxi4, + msm_mux_ddr_pxi5, + msm_mux_edp0_hot, + msm_mux_edp0_lcd, + msm_mux_edp1_hot, + msm_mux_edp1_lcd, + msm_mux_edp2_hot, + msm_mux_edp2_lcd, + msm_mux_edp3_hot, + msm_mux_edp3_lcd, + msm_mux_emac0_mcg0, + msm_mux_emac0_mcg1, + msm_mux_emac0_mcg2, + msm_mux_emac0_mcg3, + msm_mux_emac0_mdc, + msm_mux_emac0_mdio, + msm_mux_emac0_ptp,msm_mux_emac0quoted
+ msm_mux_emac1_mcg0, + msm_mux_emac1_mcg1, + msm_mux_emac1_mcg2, + msm_mux_emac1_mcg3, + msm_mux_emac1_mdc, + msm_mux_emac1_mdio, + msm_mux_emac1_ptp,msm_mux_emac1
...leave these two here as is...
quoted
+ msm_mux_gcc_gp1, + msm_mux_gcc_gp2, + msm_mux_gcc_gp3, + msm_mux_gcc_gp4, + msm_mux_gcc_gp5, + msm_mux_hs0_mi2s, + msm_mux_hs1_mi2s, + msm_mux_hs2_mi2s, + msm_mux_ibi_i3c, + msm_mux_jitter_bist, + msm_mux_mdp0_vsync0, + msm_mux_mdp0_vsync1, + msm_mux_mdp0_vsync2, + msm_mux_mdp0_vsync3, + msm_mux_mdp0_vsync4, + msm_mux_mdp0_vsync5, + msm_mux_mdp0_vsync6, + msm_mux_mdp0_vsync7, + msm_mux_mdp0_vsync8, + msm_mux_mdp1_vsync0, + msm_mux_mdp1_vsync1, + msm_mux_mdp1_vsync2, + msm_mux_mdp1_vsync3, + msm_mux_mdp1_vsync4, + msm_mux_mdp1_vsync5, + msm_mux_mdp1_vsync6, + msm_mux_mdp1_vsync7, + msm_mux_mdp1_vsync8, + msm_mux_mdp_vsync, + msm_mux_mi2s1_data0, + msm_mux_mi2s1_data1, + msm_mux_mi2s1_sck, + msm_mux_mi2s1_ws, + msm_mux_mi2s2_data0, + msm_mux_mi2s2_data1, + msm_mux_mi2s2_sck, + msm_mux_mi2s2_ws, + msm_mux_mi2s_mclk0, + msm_mux_mi2s_mclk1, + msm_mux_pcie0_clkreq, + msm_mux_pcie1_clkreq, + 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,msm_mux_phase_flag
... change this one as you suggest...
quoted
+ msm_mux_pll_bist, + msm_mux_pll_clk, + msm_mux_prng_rosc0, + msm_mux_prng_rosc1, + msm_mux_prng_rosc2, + msm_mux_prng_rosc3, + 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_qdss
... and have these as qdss_cti and qdss_gpio.
quoted
+ msm_mux_qup0_se0, + msm_mux_qup0_se1, + msm_mux_qup0_se2, + msm_mux_qup0_se3, + msm_mux_qup0_se4, + msm_mux_qup0_se5, + msm_mux_qup1_se0, + msm_mux_qup1_se1, + msm_mux_qup1_se2, + msm_mux_qup1_se3, + msm_mux_qup1_se4, + msm_mux_qup1_se5, + msm_mux_qup1_se6, + msm_mux_qup2_se0, + msm_mux_qup2_se1, + msm_mux_qup2_se2, + msm_mux_qup2_se3, + msm_mux_qup2_se4, + msm_mux_qup2_se5, + msm_mux_qup2_se6, + msm_mux_qup3_se0, + msm_mux_sail_top, + msm_mux_sailss_emac0, + msm_mux_sailss_ospi, + msm_mux_sgmii_phy, + msm_mux_tb_trig, + msm_mux_tgu_ch0, + msm_mux_tgu_ch1, + msm_mux_tgu_ch2, + msm_mux_tgu_ch3, + msm_mux_tgu_ch4, + msm_mux_tgu_ch5, + msm_mux_tsense_pwm1, + msm_mux_tsense_pwm2, + msm_mux_tsense_pwm3, + msm_mux_tsense_pwm4, + msm_mux_usb2phy_ac, + msm_mux_vsense_trigger, + msm_mux__, +}; +[..]quoted
+static const struct msm_pingroup sa8775p_groups[] = { + [0] = PINGROUP(0, _, _, _, _, _, _, _, _, _), + [1] = PINGROUP(1, pcie0_clkreq, _, _, _, _, _, _, _, _), + [2] = PINGROUP(2, _, _, _, _, _, _, _, _, _), + [3] = PINGROUP(3, pcie1_clkreq, _, _, _, _, _, _, _, _), + [4] = PINGROUP(4, _, _, _, _, _, _, _, _, _), + [5] = PINGROUP(5, _, _, _, _, _, _, _, _, _), + [6] = PINGROUP(6, emac0_ptp, emac0_ptp, emac1_ptp, emac1_ptp, _, _, _, _, _),How is it possible to select the first or the second one of these functions when they are named the same?
I think Prasad and Yadu (the original authors of the driver) followed
the example from sc8280xp:
c0e4c71a9e7ce (Bjorn Andersson 2022-03-08 14:11:32 -0800 1804) [156] =
PINGROUP(156, qup6, emac0_ptp, emac0_ptp, _, _, _, _),
Do you remember what your original intention was? I also see that the
GPIOs repeat in the groups definitions:
980 static const char * const emac0_ptp_groups[] = {
981 "gpio130", "gpio130", "gpio131", "gpio131", "gpio156", "gpio156",
982 "gpio157", "gpio157", "gpio158", "gpio158", "gpio159", "gpio159",
983 };
[...]
Bart