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

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

[Robert Jarzmik]

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

[PATCH 2/3] mtd: nand: pxa3xx_nand: fix early spurious interrupt

[Robert Jarzmik]

When the nand is first probe, and upon the first command start, the
status bits should be cleared before the interrupts are unmasked.

The bug is tricky : if the bootloader left a status bit set, the
unmasking of interrupts does trigger the interrupt handler before the
first command is issued, blocking the good behavior of the nand.

The same would happen if in pxa3xx_nand code flow a status bit is left,
and then a command is started.

Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr>
---
 drivers/mtd/nand/pxa3xx_nand.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c
index ed44bddcc43f..edfe329cc9db 100644
--- a/drivers/mtd/nand/pxa3xx_nand.c
+++ b/drivers/mtd/nand/pxa3xx_nand.c
@@ -451,8 +451,8 @@ static void pxa3xx_nand_start(struct pxa3xx_nand_info *info)
 	ndcr |= NDCR_ND_RUN;
 
 	/* clear status bits and run */
-	nand_writel(info, NDCR, 0);
 	nand_writel(info, NDSR, NDSR_MASK);
+	nand_writel(info, NDCR, 0);
 	nand_writel(info, NDCR, ndcr);
 }
 
-- 
2.1.4

[PATCH 3/3] mtd: nand: pxa3xx-nand: fix readid without keep_config

[Robert Jarzmik]

The cases of READID detection are broken on pxa3xx. The reason is that
in the early stages of nand probing, ie. at pxa3xx_nand_scan(), we
always have :
 - info->use_dma = 0 (regardless of dma support yet)
 - info->chunk_size = 0 (not yet detected)

The READID issued by pxa3xx_nand_scan() will therefore end up in
handle_data_pio(), and do_bytes will be 0, leading to not reading the
nand id, and blocking detection.

This doesn't happen if "keep_config" is used, which is probably the most
tested case.

Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr>
Fixes: 70ed85232a93 ("mtd: nand: pxa3xx: Introduce multiple page I/O
support")
---
 drivers/mtd/nand/pxa3xx_nand.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c
index edfe329cc9db..b0737aec7caf 100644
--- a/drivers/mtd/nand/pxa3xx_nand.c
+++ b/drivers/mtd/nand/pxa3xx_nand.c
@@ -1482,6 +1482,7 @@ static int pxa3xx_nand_scan(struct mtd_info *mtd)
 	int i, ret, num;
 	uint16_t ecc_strength, ecc_step;
 
+	info->chunk_size = 512;
 	if (pdata->keep_config && !pxa3xx_nand_detect_config(info))
 		goto KEEP_CONFIG;
 
-- 
2.1.4

Re: [PATCH 3/3] mtd: nand: pxa3xx-nand: fix readid without keep_config

[Ezequiel Garcia]

On 11 Aug 09:57 PM, Robert Jarzmik wrote:
> The cases of READID detection are broken on pxa3xx. The reason is that
> in the early stages of nand probing, ie. at pxa3xx_nand_scan(), we
> always have :
>  - info->use_dma = 0 (regardless of dma support yet)
>  - info->chunk_size = 0 (not yet detected)
> 
> The READID issued by pxa3xx_nand_scan() will therefore end up in
> handle_data_pio(), and do_bytes will be 0, leading to not reading the
> nand id, and blocking detection.
> 
> This doesn't happen if "keep_config" is used, which is probably the most
> tested case.
> 
> Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr>
> Fixes: 70ed85232a93 ("mtd: nand: pxa3xx: Introduce multiple page I/O
> support")

To be fair, Antoine submitted this a while ago:

http://lists.infradead.org/pipermail/linux-mtd/2015-April/058739.html

Not sure which one takes precedence in such a case (and yours
has a proper Fixes tag).

> ---
>  drivers/mtd/nand/pxa3xx_nand.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c
> index edfe329cc9db..b0737aec7caf 100644
> --- a/drivers/mtd/nand/pxa3xx_nand.c
> +++ b/drivers/mtd/nand/pxa3xx_nand.c
> @@ -1482,6 +1482,7 @@ static int pxa3xx_nand_scan(struct mtd_info *mtd)
>  	int i, ret, num;
>  	uint16_t ecc_strength, ecc_step;
>  
> +	info->chunk_size = 512;
>  	if (pdata->keep_config && !pxa3xx_nand_detect_config(info))
>  		goto KEEP_CONFIG;
>  
> -- 
> 2.1.4
> 
> 
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/

-- 
Ezequiel Garcia, VanguardiaSur
www.vanguardiasur.com.ar

Re: [PATCH 3/3] mtd: nand: pxa3xx-nand: fix readid without keep_config

[Robert Jarzmik]

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

> On 11 Aug 09:57 PM, Robert Jarzmik wrote:
>> The cases of READID detection are broken on pxa3xx. The reason is that
>> in the early stages of nand probing, ie. at pxa3xx_nand_scan(), we
>> always have :
>>  - info->use_dma = 0 (regardless of dma support yet)
>>  - info->chunk_size = 0 (not yet detected)
>> 
>> The READID issued by pxa3xx_nand_scan() will therefore end up in
>> handle_data_pio(), and do_bytes will be 0, leading to not reading the
>> nand id, and blocking detection.
>> 
>> This doesn't happen if "keep_config" is used, which is probably the most
>> tested case.
>> 
>> Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr>
>> Fixes: 70ed85232a93 ("mtd: nand: pxa3xx: Introduce multiple page I/O
>> support")
>
> To be fair, Antoine submitted this a while ago:
>
> http://lists.infradead.org/pipermail/linux-mtd/2015-April/058739.html
>
> Not sure which one takes precedence in such a case (and yours
> has a proper Fixes tag).
His has precedence. How is it that a fix patch is not yet merge since April ?
Is it because it's part of a still in review serie ?

Cheers.

--
Robert

Re: [PATCH 3/3] mtd: nand: pxa3xx-nand: fix readid without keep_config

[Ezequiel Garcia]

On 16 August 2015 at 19:22, Robert Jarzmik <robert.jarzmik@free.fr> wrote:
> Ezequiel Garcia <ezequiel@vanguardiasur.com.ar> writes:
>
>> On 11 Aug 09:57 PM, Robert Jarzmik wrote:
>>> The cases of READID detection are broken on pxa3xx. The reason is that
>>> in the early stages of nand probing, ie. at pxa3xx_nand_scan(), we
>>> always have :
>>>  - info->use_dma = 0 (regardless of dma support yet)
>>>  - info->chunk_size = 0 (not yet detected)
>>>
>>> The READID issued by pxa3xx_nand_scan() will therefore end up in
>>> handle_data_pio(), and do_bytes will be 0, leading to not reading the
>>> nand id, and blocking detection.
>>>
>>> This doesn't happen if "keep_config" is used, which is probably the most
>>> tested case.
>>>
>>> Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr>
>>> Fixes: 70ed85232a93 ("mtd: nand: pxa3xx: Introduce multiple page I/O
>>> support")
>>
>> To be fair, Antoine submitted this a while ago:
>>
>> http://lists.infradead.org/pipermail/linux-mtd/2015-April/058739.html
>>
>> Not sure which one takes precedence in such a case (and yours
>> has a proper Fixes tag).
> His has precedence. How is it that a fix patch is not yet merge since April ?
> Is it because it's part of a still in review serie ?
>

Exactly. And because I thought it was needed for Berlin support,
so it wasn't a regression.

-- 
Ezequiel García, VanguardiaSur
www.vanguardiasur.com.ar

Re: [PATCH 3/3] mtd: nand: pxa3xx-nand: fix readid without keep_config

[Robert Jarzmik]

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

> On 16 August 2015 at 19:22, Robert Jarzmik <robert.jarzmik@free.fr> wrote:
>> Ezequiel Garcia <ezequiel@vanguardiasur.com.ar> writes:
>>
>>> On 11 Aug 09:57 PM, Robert Jarzmik wrote:
>>>> The cases of READID detection are broken on pxa3xx. The reason is that
>>>> in the early stages of nand probing, ie. at pxa3xx_nand_scan(), we
>>>> always have :
>>>>  - info->use_dma = 0 (regardless of dma support yet)
>>>>  - info->chunk_size = 0 (not yet detected)
>>>>
>>>> The READID issued by pxa3xx_nand_scan() will therefore end up in
>>>> handle_data_pio(), and do_bytes will be 0, leading to not reading the
>>>> nand id, and blocking detection.
>>>>
>>>> This doesn't happen if "keep_config" is used, which is probably the most
>>>> tested case.
>>>>
>>>> Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr>
>>>> Fixes: 70ed85232a93 ("mtd: nand: pxa3xx: Introduce multiple page I/O
>>>> support")
>>>
>>> To be fair, Antoine submitted this a while ago:
>>>
>>> http://lists.infradead.org/pipermail/linux-mtd/2015-April/058739.html
>>>
>>> Not sure which one takes precedence in such a case (and yours
>>> has a proper Fixes tag).
>> His has precedence. How is it that a fix patch is not yet merge since April ?
>> Is it because it's part of a still in review serie ?

Antoine, could you resubmit this single patch with this as trailer please :
Acked-by: Robert Jarzmik <robert.jarzmik@free>
Fixes: 70ed85232a93 ("mtd: nand: pxa3xx: Introduce multiple page I/O
support")

This should get in Brian's fixes tree as soon as possible.

Thanks.

-- 
Robert

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

[Ezequiel Garcia]

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

Re: [PATCH 3/3] mtd: nand: pxa3xx-nand: fix readid without keep_config

[Antoine Tenart]

Hi,

On Mon, Aug 17, 2015 at 09:03:38PM +0200, Robert Jarzmik wrote:
> Ezequiel Garcia <ezequiel@vanguardiasur.com.ar> writes:
> 
> > On 16 August 2015 at 19:22, Robert Jarzmik <robert.jarzmik@free.fr> wrote:
> >> Ezequiel Garcia <ezequiel@vanguardiasur.com.ar> writes:
> >>
> >>> On 11 Aug 09:57 PM, Robert Jarzmik wrote:
> >>>> The cases of READID detection are broken on pxa3xx. The reason is that
> >>>> in the early stages of nand probing, ie. at pxa3xx_nand_scan(), we
> >>>> always have :
> >>>>  - info->use_dma = 0 (regardless of dma support yet)
> >>>>  - info->chunk_size = 0 (not yet detected)
> >>>>
> >>>> The READID issued by pxa3xx_nand_scan() will therefore end up in
> >>>> handle_data_pio(), and do_bytes will be 0, leading to not reading the
> >>>> nand id, and blocking detection.
> >>>>
> >>>> This doesn't happen if "keep_config" is used, which is probably the most
> >>>> tested case.
> >>>>
> >>>> Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr>
> >>>> Fixes: 70ed85232a93 ("mtd: nand: pxa3xx: Introduce multiple page I/O
> >>>> support")
> >>>
> >>> To be fair, Antoine submitted this a while ago:
> >>>
> >>> http://lists.infradead.org/pipermail/linux-mtd/2015-April/058739.html
> >>>
> >>> Not sure which one takes precedence in such a case (and yours
> >>> has a proper Fixes tag).
> >> His has precedence. How is it that a fix patch is not yet merge since April ?
> >> Is it because it's part of a still in review serie ?
> 
> Antoine, could you resubmit this single patch with this as trailer please :
> Acked-by: Robert Jarzmik <robert.jarzmik@free>
> Fixes: 70ed85232a93 ("mtd: nand: pxa3xx: Introduce multiple page I/O
> support")

I just resend it.

Antoine

-- 
Antoine Ténart, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

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

[Robert Jarzmik]

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

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

[Robert Jarzmik]

Robert Jarzmik <robert.jarzmik@free.fr> writes:

> 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.

Ezequiel, do you agree with my point ?

Cheers.

-- 
Robert

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

[Ezequiel Garcia]

On 23 August 2015 at 16:09, Robert Jarzmik <robert.jarzmik@free.fr> wrote:
> Robert Jarzmik <robert.jarzmik@free.fr> writes:
>
>> 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.
>>

So let's assume this person is not skilled enough to debug it, but it is skilled
enough to rebuild a kernel (to activate the verbose debug).

Then this person can also apply a simple patch (this patch), which seems
a pretty trivial step to do.

Since you might also need some other debug traces, I'm not convinced about
pushing this particular group to mainline.

>> 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.
>
> Ezequiel, do you agree with my point ?
>

I agree that the hack is useful for debugging purposes, it's just that I don't
see why we want it in mainline - where we usually avoid clutter.

Also, your argument applies to all the other drivers, but that doesn't mean
we will patch them all.

Anyway, this is just my opinion. If Brian thinks differently, I have no problem
with it.

Thanks!
-- 
Ezequiel García, VanguardiaSur
www.vanguardiasur.com.ar

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

[Brian Norris]

Garbage collecting some old patches before vacation...

On Mon, Aug 24, 2015 at 10:46:18AM -0300, Ezequiel Garcia wrote:
> I agree that the hack is useful for debugging purposes, it's just that I don't
> see why we want it in mainline - where we usually avoid clutter.

I'm not sure this is really that important of clutter. It's in a nicely
hidden abstraction (the local nand_{read,write}l() helpers), and it
doesn't add any compile time cost for most users.

> Also, your argument applies to all the other drivers, but that doesn't mean
> we will patch them all.
> 
> Anyway, this is just my opinion. If Brian thinks differently, I have no problem
> with it.

I don't have very strong opinions on this. It's kind of annoying to have
this sort of stuff duplicated for every driver, if it's really needed.
But I'll admit this kind of infrastructure is sometimes useful.

Anecdote: I recently found the regmap trace event infrastructure pretty
nice for debugging some other drivers. This would only require you to
have tracing enabled, and then no recompiles are necessary at all. Just
cmdline changes.

So, I could go with this patch, if Robert still desires it. Or you could
convert to using regmap for MMIO :)

Brian

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

[Robert Jarzmik]

Brian Norris <computersforpeace@gmail.com> writes:

> I don't have very strong opinions on this. It's kind of annoying to have
> this sort of stuff duplicated for every driver, if it's really needed.
> But I'll admit this kind of infrastructure is sometimes useful.
>
> Anecdote: I recently found the regmap trace event infrastructure pretty
> nice for debugging some other drivers. This would only require you to
> have tracing enabled, and then no recompiles are necessary at all. Just
> cmdline changes.
>
> So, I could go with this patch, if Robert still desires it. Or you could
> convert to using regmap for MMIO :)
I'm as you, I don't feel strong opinion about it, I'd like to have a debug
tracing tool, be that this patch or regmap MMIO.

If we all agree on a path I could even make the final patch, whichever solution
is chosen.

Cheers.

-- 
Robert

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

[Brian Norris]

On Sat, Dec 19, 2015 at 01:19:26PM +0100, Robert Jarzmik wrote:
> Brian Norris <computersforpeace@gmail.com> writes:
> 
> > I don't have very strong opinions on this. It's kind of annoying to have
> > this sort of stuff duplicated for every driver, if it's really needed.
> > But I'll admit this kind of infrastructure is sometimes useful.
> >
> > Anecdote: I recently found the regmap trace event infrastructure pretty
> > nice for debugging some other drivers. This would only require you to
> > have tracing enabled, and then no recompiles are necessary at all. Just
> > cmdline changes.
> >
> > So, I could go with this patch, if Robert still desires it. Or you could
> > convert to using regmap for MMIO :)
> I'm as you, I don't feel strong opinion about it, I'd like to have a debug
> tracing tool, be that this patch or regmap MMIO.

Regmap is probably overkill, and wouldn't do a lot to satisfy Ezequiel's
concern, I expect.

> If we all agree on a path I could even make the final patch, whichever solution
> is chosen.

I'm OK with this one, which still applies OK for me. But I do have one
comment, which I'll post at the top-level.

Brian

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

[Brian Norris]

On Tue, Aug 11, 2015 at 09:57:12PM +0200, 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.
> 
> 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);		\

The stringification of 'off' works for now, but I think that'd be a bit
restrictive in the future, if we ever want to (e.g.) do arithmetic to
compute the offset, like:

	nand_writel(info, SOME_REGISTER_MACRO + idx * 4, foo);

You might be better off just printing the hex value of the offset.

Regards,
Brian

> +		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
> 

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

[Robert Jarzmik]

Brian Norris <computersforpeace@gmail.com> writes:

> On Tue, Aug 11, 2015 at 09:57:12PM +0200, Robert Jarzmik wrote:
>> @@ -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);		\
>
> The stringification of 'off' works for now, but I think that'd be a bit
> restrictive in the future, if we ever want to (e.g.) do arithmetic to
> compute the offset, like:
>
> 	nand_writel(info, SOME_REGISTER_MACRO + idx * 4, foo);
>
> You might be better off just printing the hex value of the offset.
Sure. I can always have a post-processing script to recreate the register names.

I'll post a v2 soon.

Cheers.

--
Robert