[v4,12/20] mtd: spi-nor: Print debug message when the read back test fails
diff mbox series

Message ID 20191102112316.20715-13-tudor.ambarus@microchip.com
State Accepted
Delegated to: Ambarus Tudor
Headers show
Series
  • mtd: spi-nor: Quad Enable and (un)lock methods
Related show

Commit Message

Ambarus Tudor Nov. 2, 2019, 11:23 a.m. UTC
From: Tudor Ambarus <tudor.ambarus@microchip.com>

Demystify where the EIO error occurs.

Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
---
 drivers/mtd/spi-nor/spi-nor.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

Comments

Vignesh Raghavendra Nov. 5, 2019, 12:37 p.m. UTC | #1
On 02/11/19 4:53 PM, Tudor.Ambarus@microchip.com wrote:
> From: Tudor Ambarus <tudor.ambarus@microchip.com>
> 
> Demystify where the EIO error occurs.
> 
> Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
> ---

I think this is a small enough change that can be squashed into previous
patch itself

>  drivers/mtd/spi-nor/spi-nor.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> index 8f5e9833081b..725dab241271 100644
> --- a/drivers/mtd/spi-nor/spi-nor.c
> +++ b/drivers/mtd/spi-nor/spi-nor.c
> @@ -974,7 +974,12 @@ static int spi_nor_write_sr_and_check(struct spi_nor *nor, u8 status_new)
>  	if (ret)
>  		return ret;
>  
> -	return (nor->bouncebuf[0] != status_new) ? -EIO : 0;
> +	if (nor->bouncebuf[0] != status_new) {
> +		dev_dbg(nor->dev, "SR: read back test failed\n");
> +		return -EIO;
> +	}
> +
> +	return 0;
>  }
>  
>  /**
>
Ambarus Tudor Nov. 6, 2019, 7:24 a.m. UTC | #2
On 11/05/2019 02:37 PM, Vignesh Raghavendra wrote:
> On 02/11/19 4:53 PM, Tudor.Ambarus@microchip.com wrote:
>> From: Tudor Ambarus <tudor.ambarus@microchip.com>
>>
>> Demystify where the EIO error occurs.
>>
>> Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
>> ---
> I think this is a small enough change that can be squashed into previous
> patch itself
> 

I made separate patches because this is a separate logical change. The previous
patch extends the check on all bits of the Status Register, while this one
prints a debug message in case of EIO. Thus I tried to have a single logical
change contained in a single patch. I'm clearly no expert in this (Boris asked
me in v3 to split patches because I did too many things in one patch :) ), so I
would keep this as is, but if you still feel that it should be squashed, then
I'll do it. Please let me know.
Vignesh Raghavendra Nov. 6, 2019, 7:39 a.m. UTC | #3
On 06/11/19 12:54 PM, Tudor.Ambarus@microchip.com wrote:
> 
> 
> On 11/05/2019 02:37 PM, Vignesh Raghavendra wrote:
>> On 02/11/19 4:53 PM, Tudor.Ambarus@microchip.com wrote:
>>> From: Tudor Ambarus <tudor.ambarus@microchip.com>
>>>
>>> Demystify where the EIO error occurs.
>>>
>>> Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
>>> ---
>> I think this is a small enough change that can be squashed into previous
>> patch itself
>>
> 
> I made separate patches because this is a separate logical change. The previous
> patch extends the check on all bits of the Status Register, while this one
> prints a debug message in case of EIO. Thus I tried to have a single logical
> change contained in a single patch. I'm clearly no expert in this (Boris asked
> me in v3 to split patches because I did too many things in one patch :) ), so I
> would keep this as is, but if you still feel that it should be squashed, then
> I'll do it. Please let me know.
> 

I am fine either way. I don't have a strong preference...
Vignesh Raghavendra Nov. 7, 2019, 5:58 a.m. UTC | #4
On 06/11/19 1:09 PM, Vignesh Raghavendra wrote:
> 
> 
> On 06/11/19 12:54 PM, Tudor.Ambarus@microchip.com wrote:
>>
>>
>> On 11/05/2019 02:37 PM, Vignesh Raghavendra wrote:
>>> On 02/11/19 4:53 PM, Tudor.Ambarus@microchip.com wrote:
>>>> From: Tudor Ambarus <tudor.ambarus@microchip.com>
>>>>
>>>> Demystify where the EIO error occurs.
>>>>
>>>> Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
>>>> ---
>>> I think this is a small enough change that can be squashed into previous
>>> patch itself
>>>
>>
>> I made separate patches because this is a separate logical change. The previous
>> patch extends the check on all bits of the Status Register, while this one
>> prints a debug message in case of EIO. Thus I tried to have a single logical
>> change contained in a single patch. I'm clearly no expert in this (Boris asked
>> me in v3 to split patches because I did too many things in one patch :) ), so I
>> would keep this as is, but if you still feel that it should be squashed, then
>> I'll do it. Please let me know.
>>
> 
> I am fine either way. I don't have a strong preference...
> 

If you want to keep these separate:

Reviewed-by: Vignesh Raghavendra <vigneshr@ti.com>

Regards
Vignesh

Patch
diff mbox series

diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index 8f5e9833081b..725dab241271 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -974,7 +974,12 @@  static int spi_nor_write_sr_and_check(struct spi_nor *nor, u8 status_new)
 	if (ret)
 		return ret;
 
-	return (nor->bouncebuf[0] != status_new) ? -EIO : 0;
+	if (nor->bouncebuf[0] != status_new) {
+		dev_dbg(nor->dev, "SR: read back test failed\n");
+		return -EIO;
+	}
+
+	return 0;
 }
 
 /**