Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1205409 > unrolled thread

[PATCH 1/3] mtd: nand: pxa3xx_nand: add register access debug

Back to article view | Back to linux.kernel

Contents

  [PATCH 1/3] mtd: nand: pxa3xx_nand: add register access debug Robert Jarzmik <robert.jarzmik@free.fr> - 2015-08-11 22:10 +0200
    Re: [PATCH 1/3] mtd: nand: pxa3xx_nand: add register access debug Ezequiel Garcia <ezequiel@vanguardiasur.com.ar> - 2015-08-18 06:30 +0200
      Re: [PATCH 1/3] mtd: nand: pxa3xx_nand: add register access debug Robert Jarzmik <robert.jarzmik@free.fr> - 2015-08-18 20:40 +0200

#1205409 — [PATCH 1/3] mtd: nand: pxa3xx_nand: add register access debug

Add verbose debug for register accesses. This enables easier debugging
by following where and how hardware is stimulated, and how it answers.

Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr>
---
 drivers/mtd/nand/pxa3xx_nand.c | 22 +++++++++++++++++-----
 1 file changed, 17 insertions(+), 5 deletions(-)

diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c
index 1259cc558ce9..ed44bddcc43f 100644
--- a/drivers/mtd/nand/pxa3xx_nand.c
+++ b/drivers/mtd/nand/pxa3xx_nand.c
@@ -127,11 +127,23 @@
 #define EXT_CMD_TYPE_MONO	0 /* Monolithic read/write */
 
 /* macros for registers read/write */
-#define nand_writel(info, off, val)	\
-	writel_relaxed((val), (info)->mmio_base + (off))
-
-#define nand_readl(info, off)		\
-	readl_relaxed((info)->mmio_base + (off))
+#define nand_writel(info, off, val)					\
+	do {								\
+		dev_vdbg(&info->pdev->dev,				\
+			 "%s():%d nand_writel(0x%x, %s)\n",		\
+			 __func__, __LINE__, (val), #off);		\
+		writel_relaxed((val), (info)->mmio_base + (off));	\
+	} while (0)
+
+#define nand_readl(info, off)						\
+	({								\
+		unsigned int _v;					\
+		_v = readl_relaxed((info)->mmio_base + (off));		\
+		dev_vdbg(&info->pdev->dev,				\
+			 "%s():%d nand_readl(%s): 0x%x\n",		\
+			 __func__, __LINE__, #off, _v);			\
+		_v;							\
+	})
 
 /* error code and state */
 enum {
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1209032

On 11 Aug 09:57 PM, Robert Jarzmik wrote:
> Add verbose debug for register accesses. This enables easier debugging
> by following where and how hardware is stimulated, and how it answers.
> 

I really don't see why we want this patch. It's probably an useful hack to
use in some cases, but can't see why we would want it in mainline.

Feel free to prove me wrong.

> Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr>
> ---
>  drivers/mtd/nand/pxa3xx_nand.c | 22 +++++++++++++++++-----
>  1 file changed, 17 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c
> index 1259cc558ce9..ed44bddcc43f 100644
> --- a/drivers/mtd/nand/pxa3xx_nand.c
> +++ b/drivers/mtd/nand/pxa3xx_nand.c
> @@ -127,11 +127,23 @@
>  #define EXT_CMD_TYPE_MONO	0 /* Monolithic read/write */
>  
>  /* macros for registers read/write */
> -#define nand_writel(info, off, val)	\
> -	writel_relaxed((val), (info)->mmio_base + (off))
> -
> -#define nand_readl(info, off)		\
> -	readl_relaxed((info)->mmio_base + (off))
> +#define nand_writel(info, off, val)					\
> +	do {								\
> +		dev_vdbg(&info->pdev->dev,				\
> +			 "%s():%d nand_writel(0x%x, %s)\n",		\
> +			 __func__, __LINE__, (val), #off);		\
> +		writel_relaxed((val), (info)->mmio_base + (off));	\
> +	} while (0)
> +
> +#define nand_readl(info, off)						\
> +	({								\
> +		unsigned int _v;					\
> +		_v = readl_relaxed((info)->mmio_base + (off));		\
> +		dev_vdbg(&info->pdev->dev,				\
> +			 "%s():%d nand_readl(%s): 0x%x\n",		\
> +			 __func__, __LINE__, #off, _v);			\
> +		_v;							\
> +	})
>  
>  /* error code and state */
>  enum {
> -- 
> 2.1.4
> 
> 
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/

-- 
Ezequiel Garcia, VanguardiaSur
www.vanguardiasur.com.ar
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1209425

Ezequiel Garcia <ezequiel@vanguardiasur.com.ar> writes:

> On 11 Aug 09:57 PM, Robert Jarzmik wrote:
>> Add verbose debug for register accesses. This enables easier debugging
>> by following where and how hardware is stimulated, and how it answers.
>> 
>
> I really don't see why we want this patch. It's probably an useful hack to
> use in some cases, but can't see why we would want it in mainline.
>
> Feel free to prove me wrong.
Why not.

Imagine that there is a bug in the driver, such as the one with the status bits
clearing. You can't reproduce it, and the person who has the hardware to
reproduce it is not skilled enough to do any debug.

What are the tools and what will you ask this tester to do in order to debug and
solve his problem, as a good driver maintainer ?

Amongst the drivers I wrote in the past, this approach was the most reliable to
capture traces to debug remotely.

Cheers.

--
Robert
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web