lkml.org 
[lkml]   [2020]   [Sep]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 5/7] pinctrl: qcom: Add IPQ5018 pinctrl driver
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")

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),
> +};
> +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?

> + _, _, _),
> + 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,
> + _, _, _),

Regards,
Bjorn

\
 
 \ /
  Last update: 2020-09-28 20:48    [W:0.113 / U:2.804 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site