| Message ID | 20220308100956.2750295-6-wenst@chromium.org |
|---|---|
| State | Accepted |
| Commit | fcde2a3fc86c5974b34ba5cfb1b532fc00fefb8d |
| Headers | show |
| Series | pinctrl: mediatek: Fixes and minor improvements | expand |
On Tue, 2022-03-08 at 18:09 +0800, Chen-Yu Tsai wrote: > The caller of mtk_pctrl_show_one_pin() is responsible for printing the > full line. mtk_pctrl_show_one_pin(), called through mtk_pctrl_dbg_show(), > should only produce a string containing the extra information the driver > wants included. Does this function have another caller? It looks as if this function doesn't need to be EXPORT_SYMBOL_GPL and could just be declared static in the file instead. btw: using %1d is mostly senseless as a control, but has some documentary use as a value < 0 or >= 10 will be always be fully emitted anyway. > diff --git a/drivers/pinctrl/mediatek/pinctrl-paris.c b/drivers/pinctrl/mediatek/pinctrl-paris.c [] > @@ -634,14 +634,10 @@ ssize_t mtk_pctrl_show_one_pin(struct mtk_pinctrl *hw, > pullen, > pullup); > > - if (r1 != -1) { > - len += scnprintf(buf + len, buf_len - len, " (%1d %1d)\n", > - r1, r0); > - } else if (rsel != -1) { > - len += scnprintf(buf + len, buf_len - len, " (%1d)\n", rsel); > - } else { > - len += scnprintf(buf + len, buf_len - len, "\n"); > - } > + if (r1 != -1) > + len += scnprintf(buf + len, buf_len - len, " (%1d %1d)", r1, r0); > + else if (rsel != -1) > + len += scnprintf(buf + len, buf_len - len, " (%1d)", rsel); > > return len; > }
On Thu, Mar 10, 2022 at 4:01 AM Joe Perches <joe@perches.com> wrote: > > On Tue, 2022-03-08 at 18:09 +0800, Chen-Yu Tsai wrote: > > The caller of mtk_pctrl_show_one_pin() is responsible for printing the > > full line. mtk_pctrl_show_one_pin(), called through mtk_pctrl_dbg_show(), > > should only produce a string containing the extra information the driver > > wants included. > > Does this function have another caller? > > It looks as if this function doesn't need to be EXPORT_SYMBOL_GPL > and could just be declared static in the file instead. Judging from the commit history, I suspect there might have been plans for using this in another module? I can do a patch to make it static until that ever happens. > btw: using %1d is mostly senseless as a control, but has some > documentary use as a value < 0 or >= 10 will be always be > fully emitted anyway. Ack. But we could just leave it be. Regards ChenYu > > diff --git a/drivers/pinctrl/mediatek/pinctrl-paris.c b/drivers/pinctrl/mediatek/pinctrl-paris.c > [] > > @@ -634,14 +634,10 @@ ssize_t mtk_pctrl_show_one_pin(struct mtk_pinctrl *hw, > > pullen, > > pullup); > > > > - if (r1 != -1) { > > - len += scnprintf(buf + len, buf_len - len, " (%1d %1d)\n", > > - r1, r0); > > - } else if (rsel != -1) { > > - len += scnprintf(buf + len, buf_len - len, " (%1d)\n", rsel); > > - } else { > > - len += scnprintf(buf + len, buf_len - len, "\n"); > > - } > > + if (r1 != -1) > > + len += scnprintf(buf + len, buf_len - len, " (%1d %1d)", r1, r0); > > + else if (rsel != -1) > > + len += scnprintf(buf + len, buf_len - len, " (%1d)", rsel); > > > > return len; > > } > >
diff --git a/drivers/pinctrl/mediatek/pinctrl-paris.c b/drivers/pinctrl/mediatek/pinctrl-paris.c index 3bda1aac650b..38a00a906daf 100644 --- a/drivers/pinctrl/mediatek/pinctrl-paris.c +++ b/drivers/pinctrl/mediatek/pinctrl-paris.c @@ -634,14 +634,10 @@ ssize_t mtk_pctrl_show_one_pin(struct mtk_pinctrl *hw, pullen, pullup); - if (r1 != -1) { - len += scnprintf(buf + len, buf_len - len, " (%1d %1d)\n", - r1, r0); - } else if (rsel != -1) { - len += scnprintf(buf + len, buf_len - len, " (%1d)\n", rsel); - } else { - len += scnprintf(buf + len, buf_len - len, "\n"); - } + if (r1 != -1) + len += scnprintf(buf + len, buf_len - len, " (%1d %1d)", r1, r0); + else if (rsel != -1) + len += scnprintf(buf + len, buf_len - len, " (%1d)", rsel); return len; }
The caller of mtk_pctrl_show_one_pin() is responsible for printing the full line. mtk_pctrl_show_one_pin(), called through mtk_pctrl_dbg_show(), should only produce a string containing the extra information the driver wants included. Drop the extra newlines. Also unbreak the line that is only slightly over 80 characters to make it easier on the eye, and get rid of the braces now that each block in the conditionals is just one line. Fixes: 184d8e13f9b1 ("pinctrl: mediatek: Add support for pin configuration dump via debugfs.") Fixes: fb34a9ae383a ("pinctrl: mediatek: support rsel feature") Signed-off-by: Chen-Yu Tsai <wenst@chromium.org> Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> --- drivers/pinctrl/mediatek/pinctrl-paris.c | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-)