On Mon, Sep 28, 2020 at 01:43:22PM -0500, Bjorn Andersson wrote:
> On Mon 28 Sep 00:15 CDT 2020, Varadarajan Narayanan wrote:
> > diff --git a/drivers/pinctrl/qcom/pinctrl-ipq5018.c b/drivers/pinctrl/qcom/pinctrl-ipq5018.c
> [..]
> > +static const struct msm_function ipq5018_functions[] = {
> [..]
> > + 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.
>
> > + 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.
> > +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.
> > + _, _, _),
> > + 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.
>
> > + 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
