[PATCH 0/2] Add dbg_show functions to the Marvell GPIO drivers

[PATCH 0/2] Add dbg_show functions to the Marvell GPIO drivers

[Simon Guinot]

Here are two patches I have used to validate the patch series:
"refactoring for mask_cache".

They adds dedicated dbg_show functions to the Marvell GPIO drivers
mvebu-gpio and orion-gpio. This allows to display the SoC specific
GPIO informations in addition to the generic gpiolib debug output.

Regards,

Simon

Simon Guinot (2):
  gpio: mvebu: add dbg_show function
  ARM: Orion: add dbg_show function to gpio-orion driver

 arch/arm/plat-orion/gpio.c |   59 ++++++++++++++++++++++++++++++++++++++++++++
 drivers/gpio/gpio-mvebu.c  |   59 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 118 insertions(+)

-- 
1.7.10.4

[PATCH 1/2] gpio: mvebu: add dbg_show function

[Simon Guinot]

This patch adds a dedicated dbg_show function to the gpio-mvebu driver.
In addition to the generic gpiolib informations, this function displays
informations related with the specific Marvell registers (blink enable,
data in polarity, interrupt masks and cause).

Signed-off-by: Simon Guinot <simon.guinot@sequanux.org>
---
 drivers/gpio/gpio-mvebu.c |   59 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 59 insertions(+)

diff --git a/drivers/gpio/gpio-mvebu.c b/drivers/gpio/gpio-mvebu.c
index 61a6fde..c6a9f14 100644
--- a/drivers/gpio/gpio-mvebu.c
+++ b/drivers/gpio/gpio-mvebu.c
@@ -470,6 +470,64 @@ static void mvebu_gpio_irq_handler(unsigned int irq, struct irq_desc *desc)
 	}
 }
 
+#ifdef CONFIG_DEBUG_FS
+#include <linux/seq_file.h>
+
+static void mvebu_gpio_dbg_show(struct seq_file *s, struct gpio_chip *chip)
+{
+	struct mvebu_gpio_chip *mvchip =
+		container_of(chip, struct mvebu_gpio_chip, chip);
+	u32 out, io_conf, blink, in_pol, data_in, cause, edg_msk, lvl_msk;
+	int i;
+
+	out	= readl_relaxed(mvebu_gpioreg_out(mvchip));
+	io_conf	= readl_relaxed(mvebu_gpioreg_io_conf(mvchip));
+	blink	= readl_relaxed(mvebu_gpioreg_blink(mvchip));
+	in_pol	= readl_relaxed(mvebu_gpioreg_in_pol(mvchip));
+	data_in	= readl_relaxed(mvebu_gpioreg_data_in(mvchip));
+	cause	= readl_relaxed(mvebu_gpioreg_edge_cause(mvchip));
+	edg_msk	= readl_relaxed(mvebu_gpioreg_edge_mask(mvchip));
+	lvl_msk	= readl_relaxed(mvebu_gpioreg_level_mask(mvchip));
+
+	for (i = 0; i < chip->ngpio; i++) {
+		const char *label;
+		int msk;
+		bool is_out;
+
+		label = gpiochip_is_requested(chip, i);
+		if (!label)
+			continue;
+
+		msk = 1 << i;
+		is_out = !(io_conf & msk);
+
+		seq_printf(s, " gpio-%-3d (%-20.20s)", chip->base + i, label);
+
+		if (is_out) {
+			seq_printf(s, " out %s %s\n",
+				   out & msk ? "hi" : "lo",
+				   blink & msk ? "(blink )" : "");
+			continue;
+		}
+
+		seq_printf(s, " in  %s (act %s) - IRQ",
+			   (data_in ^ in_pol) & msk  ? "hi" : "lo",
+			   in_pol & msk ? "lo" : "hi");
+		if (!((edg_msk | lvl_msk) & msk)) {
+			seq_printf(s, " disabled\n");
+			continue;
+		}
+		if (edg_msk & msk)
+			seq_printf(s, " edge ");
+		if (lvl_msk & msk)
+			seq_printf(s, " level");
+		seq_printf(s, " (%s)\n", cause & msk ? "pending" : "clear  ");
+	}
+}
+#else
+#define mvebu_gpio_dbg_show NULL
+#endif
+
 static struct of_device_id mvebu_gpio_of_match[] = {
 	{
 		.compatible = "marvell,orion-gpio",
@@ -550,6 +608,7 @@ static int mvebu_gpio_probe(struct platform_device *pdev)
 	mvchip->chip.ngpio = ngpios;
 	mvchip->chip.can_sleep = 0;
 	mvchip->chip.of_node = np;
+	mvchip->chip.dbg_show = mvebu_gpio_dbg_show;
 
 	spin_lock_init(&mvchip->lock);
 	mvchip->membase = devm_ioremap_resource(&pdev->dev, res);
-- 
1.7.10.4

[PATCH 1/2] gpio: mvebu: add dbg_show function

[Jason Cooper]

Linus,

On Fri, Mar 22, 2013 at 07:49:47PM +0100, Simon Guinot wrote:
> This patch adds a dedicated dbg_show function to the gpio-mvebu driver.
> In addition to the generic gpiolib informations, this function displays
> informations related with the specific Marvell registers (blink enable,
> data in polarity, interrupt masks and cause).
> 
> Signed-off-by: Simon Guinot <simon.guinot@sequanux.org>
> ---
>  drivers/gpio/gpio-mvebu.c |   59 +++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 59 insertions(+)

It'd probably be easiest to split this series up since the two patches
don't really depend on one another at all.

If you agree, I'll pull in the plat-orion/ patch.

thx,

Jason.

[PATCH 1/2] gpio: mvebu: add dbg_show function

[Thomas Petazzoni]

Dear Simon Guinot,

On Fri, 22 Mar 2013 19:49:47 +0100, Simon Guinot wrote:
> +	for (i = 0; i < chip->ngpio; i++) {
> +		const char *label;
> +		int msk;
> +		bool is_out;
> +
> +		label = gpiochip_is_requested(chip, i);
> +		if (!label)
> +			continue;
> +
> +		msk = 1 << i;
> +		is_out = !(io_conf & msk);

Maybe instead of using 'msk' you could use test_bit() ?

		is_out = !test_bit(i, io_conf);

> +		seq_printf(s, " gpio-%-3d (%-20.20s)", chip->base + i, label);
> +
> +		if (is_out) {
> +			seq_printf(s, " out %s %s\n",
> +				   out & msk ? "hi" : "lo",

				   test_bit(i, out) ? "hi" : "lo",

> +				   blink & msk ? "(blink )" : "");

				   test_bit(i, blink) ? "(blink )" : ""

etc.

Thomas
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

[PATCH 1/2] gpio: mvebu: add dbg_show function

[Simon Guinot]

On Sat, Mar 23, 2013 at 04:21:18PM +0100, Thomas Petazzoni wrote:
> Dear Simon Guinot,

Hi Thomas,

> 
> On Fri, 22 Mar 2013 19:49:47 +0100, Simon Guinot wrote:
> > +	for (i = 0; i < chip->ngpio; i++) {
> > +		const char *label;
> > +		int msk;
> > +		bool is_out;
> > +
> > +		label = gpiochip_is_requested(chip, i);
> > +		if (!label)
> > +			continue;
> > +
> > +		msk = 1 << i;
> > +		is_out = !(io_conf & msk);
> 
> Maybe instead of using 'msk' you could use test_bit() ?

test_bit implies more extra bit shifting which are not needed.
But maybe it makes the function more readable.

Your call :)

Simon

> 
> 		is_out = !test_bit(i, io_conf);
> 
> > +		seq_printf(s, " gpio-%-3d (%-20.20s)", chip->base + i, label);
> > +
> > +		if (is_out) {
> > +			seq_printf(s, " out %s %s\n",
> > +				   out & msk ? "hi" : "lo",
> 
> 				   test_bit(i, out) ? "hi" : "lo",
> 
> > +				   blink & msk ? "(blink )" : "");
> 
> 				   test_bit(i, blink) ? "(blink )" : ""
> 
> etc.
> 
> Thomas
> -- 
> Thomas Petazzoni, Free Electrons
> Kernel, drivers, real-time and embedded Linux
> development, consulting, training and support.
> http://free-electrons.com
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130323/969ef853/attachment.sig>

[PATCH 1/2] gpio: mvebu: add dbg_show function

[Russell King - ARM Linux]

On Sat, Mar 23, 2013 at 04:21:18PM +0100, Thomas Petazzoni wrote:
> Dear Simon Guinot,
> 
> On Fri, 22 Mar 2013 19:49:47 +0100, Simon Guinot wrote:
> > +	for (i = 0; i < chip->ngpio; i++) {
> > +		const char *label;
> > +		int msk;
> > +		bool is_out;
> > +
> > +		label = gpiochip_is_requested(chip, i);
> > +		if (!label)
> > +			continue;
> > +
> > +		msk = 1 << i;
> > +		is_out = !(io_conf & msk);
> 
> Maybe instead of using 'msk' you could use test_bit() ?
> 
> 		is_out = !test_bit(i, io_conf);
> 
> > +		seq_printf(s, " gpio-%-3d (%-20.20s)", chip->base + i, label);
> > +
> > +		if (is_out) {
> > +			seq_printf(s, " out %s %s\n",
> > +				   out & msk ? "hi" : "lo",
> 
> 				   test_bit(i, out) ? "hi" : "lo",
> 
> > +				   blink & msk ? "(blink )" : "");
> 
> 				   test_bit(i, blink) ? "(blink )" : ""

I don't think this is a good idea - and it's wrong.  test_bit() operates
on an array of bits, and needs to be passed an unsigned long pointer.
blink etc are "u32"s which aren't unsigned long.

Moreover, what if ngpio becomes greater than 32 (for whatever reason)?
test_bit(), when used correctly, will access the following word, which
is probably a bad thing.  The original won't have that behaviour,
which is safer.

Last point on the original - msk wants to also be of type 'u32' as well
to match io_conf/out/blink.

[PATCH 1/2] gpio: mvebu: add dbg_show function

[Thomas Petazzoni]

Russell,

On Sat, 23 Mar 2013 15:43:18 +0000, Russell King - ARM Linux wrote:

> I don't think this is a good idea - and it's wrong.  test_bit() operates
> on an array of bits, and needs to be passed an unsigned long pointer.
> blink etc are "u32"s which aren't unsigned long.
> 
> Moreover, what if ngpio becomes greater than 32 (for whatever reason)?
> test_bit(), when used correctly, will access the following word, which
> is probably a bad thing.  The original won't have that behaviour,
> which is safer.

You're right, indeed.

Thomas
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

[PATCH 2/2] ARM: Orion: add dbg_show function to gpio-orion driver

[Simon Guinot]

This patch adds a dedicated dbg_show function to the gpio-mvebu driver.
In addition to the generic gpiolib informations, this function displays
informations related with the specific Marvell registers (blink enable,
data in polarity, interrupt masks and cause).

Signed-off-by: Simon Guinot <simon.guinot@sequanux.org>
---
 arch/arm/plat-orion/gpio.c |   59 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 59 insertions(+)

diff --git a/arch/arm/plat-orion/gpio.c b/arch/arm/plat-orion/gpio.c
index c29ee7e..cc27824 100644
--- a/arch/arm/plat-orion/gpio.c
+++ b/arch/arm/plat-orion/gpio.c
@@ -439,6 +439,64 @@ static void gpio_irq_handler(unsigned irq, struct irq_desc *desc)
 	}
 }
 
+#ifdef CONFIG_DEBUG_FS
+#include <linux/seq_file.h>
+
+static void orion_gpio_dbg_show(struct seq_file *s, struct gpio_chip *chip)
+{
+	struct orion_gpio_chip *ochip =
+		container_of(chip, struct orion_gpio_chip, chip);
+	u32 out, io_conf, blink, in_pol, data_in, cause, edg_msk, lvl_msk;
+	int i;
+
+	out	= readl_relaxed(GPIO_OUT(ochip));
+	io_conf	= readl_relaxed(GPIO_IO_CONF(ochip));
+	blink	= readl_relaxed(GPIO_BLINK_EN(ochip));
+	in_pol	= readl_relaxed(GPIO_IN_POL(ochip));
+	data_in	= readl_relaxed(GPIO_DATA_IN(ochip));
+	cause	= readl_relaxed(GPIO_EDGE_CAUSE(ochip));
+	edg_msk	= readl_relaxed(GPIO_EDGE_MASK(ochip));
+	lvl_msk	= readl_relaxed(GPIO_LEVEL_MASK(ochip));
+
+	for (i = 0; i < chip->ngpio; i++) {
+		const char *label;
+		int msk;
+		bool is_out;
+
+		label = gpiochip_is_requested(chip, i);
+		if (!label)
+			continue;
+
+		msk = 1 << i;
+		is_out = !(io_conf & msk);
+
+		seq_printf(s, " gpio-%-3d (%-20.20s)", chip->base + i, label);
+
+		if (is_out) {
+			seq_printf(s, " out %s %s\n",
+				   out & msk ? "hi" : "lo",
+				   blink & msk ? "(blink )" : "");
+			continue;
+		}
+
+		seq_printf(s, " in  %s (act %s) - IRQ",
+			   (data_in ^ in_pol) & msk  ? "hi" : "lo",
+			   in_pol & msk ? "lo" : "hi");
+		if (!((edg_msk | lvl_msk) & msk)) {
+			seq_printf(s, " disabled\n");
+			continue;
+		}
+		if (edg_msk & msk)
+			seq_printf(s, " edge ");
+		if (lvl_msk & msk)
+			seq_printf(s, " level");
+		seq_printf(s, " (%s)\n", cause & msk ? "pending" : "clear  ");
+	}
+}
+#else
+#define orion_gpio_dbg_show NULL
+#endif
+
 void __init orion_gpio_init(struct device_node *np,
 			    int gpio_base, int ngpio,
 			    void __iomem *base, int mask_offset,
@@ -471,6 +529,7 @@ void __init orion_gpio_init(struct device_node *np,
 #ifdef CONFIG_OF
 	ochip->chip.of_node = np;
 #endif
+	ochip->chip.dbg_show = orion_gpio_dbg_show;
 
 	spin_lock_init(&ochip->lock);
 	ochip->base = (void __iomem *)base;
-- 
1.7.10.4

[PATCH 2/2] ARM: Orion: add dbg_show function to gpio-orion driver

[Jason Cooper]

On Fri, Mar 22, 2013 at 07:49:48PM +0100, Simon Guinot wrote:
> This patch adds a dedicated dbg_show function to the gpio-mvebu driver.
> In addition to the generic gpiolib informations, this function displays
> informations related with the specific Marvell registers (blink enable,
> data in polarity, interrupt masks and cause).
> 
> Signed-off-by: Simon Guinot <simon.guinot@sequanux.org>
> ---
>  arch/arm/plat-orion/gpio.c |   59 ++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 59 insertions(+)

Applied to mvebu/soc

thx,

Jason.