On Mon, Sep 28, 2020 at 01:43:22PM -0500, Bjorn Andersson wrote:

On Mon 28 Sep 00:15 CDT 2020, Varadarajan Narayanan wrote:

quoted

diff --git a/drivers/pinctrl/qcom/pinctrl-ipq5018.c b/drivers/pinctrl/qcom/pinctrl-ipq5018.c

[..]

quoted

+static const struct msm_function ipq5018_functions[] = {

[..]

quoted

+	FUNCTION(qspi_clk),
+	FUNCTION(qspi_cs),
+	FUNCTION(qspi0),
+	FUNCTION(qspi1),
+	FUNCTION(qspi2),
+	FUNCTION(qspi3),

Instead of having one function name per pin it typically leads to
cleaner DT if you group these under the same name (i.e. "qspi")

Ok.

Same seems to apply to sdc, wci, xfem at least.

quoted

+	FUNCTION(reset_out),
+	FUNCTION(sdc1_clk),
+	FUNCTION(sdc1_cmd),
+	FUNCTION(sdc10),
+	FUNCTION(sdc11),
+	FUNCTION(sdc12),
+	FUNCTION(sdc13),
+	FUNCTION(wci0),
+	FUNCTION(wci1),
+	FUNCTION(wci2),
+	FUNCTION(wci3),
+	FUNCTION(wci4),
+	FUNCTION(wci5),
+	FUNCTION(wci6),
+	FUNCTION(wci7),
+	FUNCTION(wsa_swrm),
+	FUNCTION(wsi_clk3),
+	FUNCTION(wsi_data3),
+	FUNCTION(wsis_reset),
+	FUNCTION(xfem0),
+	FUNCTION(xfem1),
+	FUNCTION(xfem2),
+	FUNCTION(xfem3),
+	FUNCTION(xfem4),
+	FUNCTION(xfem5),
+	FUNCTION(xfem6),
+	FUNCTION(xfem7),
+};

Ok.

quoted

+static const struct msm_pingroup ipq5018_groups[] = {
+	PINGROUP(0, atest_char0, _, qdss_cti_trig_out_a0, wci0, wci0, xfem0,

What's up with wci0 being both function 4 and 5?

Will check this.

quoted

+		 _, _, _),
+	PINGROUP(1, atest_char1, _, qdss_cti_trig_in_a0, wci1, wci1, xfem1,
+		 _, _, _),

Please don't like break these, better blow the line length limit in
favor or readability.

quoted

+	PINGROUP(2, atest_char2, _, qdss_cti_trig_out_a1, wci2, wci2, xfem2,
+		 _, _, _),
+	PINGROUP(3, atest_char3, _, qdss_cti_trig_in_a1, wci3, wci3, xfem3,
+		 _, _, _),

Ok.

Regards,
Bjorn

Will post updated patches soon.

Thanks
Varada
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel