pinctrl: qcom: Print high/low status of gpios in debugfs

Message ID 20180508001523.1264-1-swboyd@chromium.org
State New
Headers show
Series
  • pinctrl: qcom: Print high/low status of gpios in debugfs
Related show

Commit Message

Stephen Boyd May 8, 2018, 12:15 a.m.
I was debugging some gpio issues and I thought that the output of gpio
debugfs was telling me the high or low level of the gpios with a '1' or
a '0'. We saw a line like this though:

 gpio93  : in 4 2mA pull down

and I started to think that there may be a gas leak in the building
because '4' doesn't mean high or low, and other pins said '0' or '1'. It
turns out, '4' is the function selection for the pinmux of the gpio and
not the value on the pin. Reading code helps decipher what debugfs is
actually saying.

Add support to read the input or output pin depending on how the pin is
configured so we can easily see the high or low value of the pin in
debugfs. Now the output looks like

 gpio93  : in   low  func4 2mA pull down

which clearly shows that the pin is an input, low, with function 4 and a
2mA drive strength plus a pull down.

Cc: Bjorn Andersson <bjorn.andersson@linaro.org>
Cc: Alexandru M Stan <amstan@chromium.org>
Signed-off-by: Stephen Boyd <swboyd@chromium.org>
---

I have a patch to pretty print the function name, but I don't know how
useful that is given we have pinctrl debugfs which prints that and the
ACPI qcom driver doesn't have function names.

 drivers/pinctrl/qcom/pinctrl-msm.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

Comments

Linus Walleij May 16, 2018, 1:46 p.m. | #1
On Tue, May 8, 2018 at 2:15 AM, Stephen Boyd <swboyd@chromium.org> wrote:

> I was debugging some gpio issues and I thought that the output of gpio
> debugfs was telling me the high or low level of the gpios with a '1' or
> a '0'. We saw a line like this though:
>
>  gpio93  : in 4 2mA pull down
>
> and I started to think that there may be a gas leak in the building
> because '4' doesn't mean high or low, and other pins said '0' or '1'. It
> turns out, '4' is the function selection for the pinmux of the gpio and
> not the value on the pin. Reading code helps decipher what debugfs is
> actually saying.
>
> Add support to read the input or output pin depending on how the pin is
> configured so we can easily see the high or low value of the pin in
> debugfs. Now the output looks like
>
>  gpio93  : in   low  func4 2mA pull down
>
> which clearly shows that the pin is an input, low, with function 4 and a
> 2mA drive strength plus a pull down.
>
> Cc: Bjorn Andersson <bjorn.andersson@linaro.org>
> Cc: Alexandru M Stan <amstan@chromium.org>
> Signed-off-by: Stephen Boyd <swboyd@chromium.org>

Patch applied with some fuzzing. Please check the result!

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Stephen Boyd May 16, 2018, 3:15 p.m. | #2
Quoting Linus Walleij (2018-05-16 06:46:01)
> On Tue, May 8, 2018 at 2:15 AM, Stephen Boyd <swboyd@chromium.org> wrote:
> 
> > I was debugging some gpio issues and I thought that the output of gpio
> > debugfs was telling me the high or low level of the gpios with a '1' or
> > a '0'. We saw a line like this though:
> >
> >  gpio93  : in 4 2mA pull down
> >
> > and I started to think that there may be a gas leak in the building
> > because '4' doesn't mean high or low, and other pins said '0' or '1'. It
> > turns out, '4' is the function selection for the pinmux of the gpio and
> > not the value on the pin. Reading code helps decipher what debugfs is
> > actually saying.
> >
> > Add support to read the input or output pin depending on how the pin is
> > configured so we can easily see the high or low value of the pin in
> > debugfs. Now the output looks like
> >
> >  gpio93  : in   low  func4 2mA pull down
> >
> > which clearly shows that the pin is an input, low, with function 4 and a
> > 2mA drive strength plus a pull down.
> >
> > Cc: Bjorn Andersson <bjorn.andersson@linaro.org>
> > Cc: Alexandru M Stan <amstan@chromium.org>
> > Signed-off-by: Stephen Boyd <swboyd@chromium.org>
> 
> Patch applied with some fuzzing. Please check the result!
> 

Looks good visually. I'll pull it in shortly and test. Thanks!

--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Stephen Boyd May 22, 2018, 4:49 p.m. | #3
Quoting Stephen Boyd (2018-05-16 08:15:00)
> Quoting Linus Walleij (2018-05-16 06:46:01)
> > 
> > Patch applied with some fuzzing. Please check the result!
> > 
> 
> Looks good visually. I'll pull it in shortly and test. Thanks!
> 

In case you were wondering, it works out on my end so it's good to go.
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/drivers/pinctrl/qcom/pinctrl-msm.c b/drivers/pinctrl/qcom/pinctrl-msm.c
index ad80a17c9990..4c546b8052a7 100644
--- a/drivers/pinctrl/qcom/pinctrl-msm.c
+++ b/drivers/pinctrl/qcom/pinctrl-msm.c
@@ -506,7 +506,8 @@  static void msm_gpio_dbg_show_one(struct seq_file *s,
 	int is_out;
 	int drive;
 	int pull;
-	u32 ctl_reg;
+	int val;
+	u32 ctl_reg, io_reg;
 
 	static const char * const pulls[] = {
 		"no pull",
@@ -520,13 +521,20 @@  static void msm_gpio_dbg_show_one(struct seq_file *s,
 
 	g = &pctrl->soc->groups[offset];
 	ctl_reg = readl(pctrl->regs + g->ctl_reg);
+	io_reg = readl(pctrl->regs + g->io_reg);
 
 	is_out = !!(ctl_reg & BIT(g->oe_bit));
 	func = (ctl_reg >> g->mux_bit) & 7;
 	drive = (ctl_reg >> g->drv_bit) & 7;
 	pull = (ctl_reg >> g->pull_bit) & 3;
 
-	seq_printf(s, " %-8s: %-3s %d", g->name, is_out ? "out" : "in", func);
+	if (is_out)
+		val = !!(io_reg & BIT(g->out_bit));
+	else
+		val = !!(io_reg & BIT(g->in_bit));
+
+	seq_printf(s, " %-8s: %-3s", g->name, is_out ? "out" : "in");
+	seq_printf(s, " %-4s func%d", val ? "high" : "low", func);
 	seq_printf(s, " %dmA", msm_regval_to_drive(drive));
 	seq_printf(s, " %s", pulls[pull]);
 	seq_puts(s, "\n");