diff mbox series

[1/2] mtd: rawnand: Fix the nand_read_data_op() early check

Message ID 20240507160546.130255-2-miquel.raynal@bootlin.com
State New
Headers show
Series mtd: rawnand: NAND early identification fixes | expand

Commit Message

Miquel Raynal May 7, 2024, 4:05 p.m. UTC
The nand_read_data_op() operation, which only consists in DATA_IN
cycles, is sadly not supported by all controllers despite being very
basic. The core, for some time, supposed all drivers would support
it. An improvement to this situation for supporting more constrained
controller added a check to verify if the operation was supported before
attempting it by running the function with the check_only boolean set
first, and then possibly falling back to another (possibly slightly less
optimized) alternative.

An even newer addition moved that check very early and probe time, in
order to perform the check only once. The content of the operation was
not so important, as long as the controller driver would tell whether
such operation on the NAND bus would be possible or not. In practice, no
buffer was provided (no fake buffer or whatever) as it is anyway not
relevant for the "check_only" condition. Unfortunately, early in the
function, there is an if statement verifying that the input parameters
are right for normal use, making the early check always unsuccessful.

Fixes: 9f820fc0651c ("mtd: rawnand: Check the data only read pattern only once")
Cc: stable@vger.kernel.org
Reported-by: Alexander Dahl <ada@thorsis.com>
Closes: https://lore.kernel.org/linux-mtd/20240306-shaky-bunion-d28b65ea97d7@thorsis.com/
Reported-by: Steven Seeger <steven.seeger@flightsystems.net>
Closes: https://lore.kernel.org/linux-mtd/DM6PR05MB4506554457CF95191A670BDEF7062@DM6PR05MB4506.namprd05.prod.outlook.com/
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/mtd/nand/raw/nand_base.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Alexander Dahl May 8, 2024, 6:36 a.m. UTC | #1
Hello Miquel,

Am Tue, May 07, 2024 at 06:05:45PM +0200 schrieb Miquel Raynal:
> The nand_read_data_op() operation, which only consists in DATA_IN
> cycles, is sadly not supported by all controllers despite being very
> basic. The core, for some time, supposed all drivers would support
> it. An improvement to this situation for supporting more constrained
> controller added a check to verify if the operation was supported before
> attempting it by running the function with the check_only boolean set
> first, and then possibly falling back to another (possibly slightly less
> optimized) alternative.
> 
> An even newer addition moved that check very early and probe time, in
> order to perform the check only once. The content of the operation was
> not so important, as long as the controller driver would tell whether
> such operation on the NAND bus would be possible or not. In practice, no
> buffer was provided (no fake buffer or whatever) as it is anyway not
> relevant for the "check_only" condition. Unfortunately, early in the
> function, there is an if statement verifying that the input parameters
> are right for normal use, making the early check always unsuccessful.
> 
> Fixes: 9f820fc0651c ("mtd: rawnand: Check the data only read pattern only once")
> Cc: stable@vger.kernel.org
> Reported-by: Alexander Dahl <ada@thorsis.com>
> Closes: https://lore.kernel.org/linux-mtd/20240306-shaky-bunion-d28b65ea97d7@thorsis.com/
> Reported-by: Steven Seeger <steven.seeger@flightsystems.net>
> Closes: https://lore.kernel.org/linux-mtd/DM6PR05MB4506554457CF95191A670BDEF7062@DM6PR05MB4506.namprd05.prod.outlook.com/
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
>  drivers/mtd/nand/raw/nand_base.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
> index acd137dd0957..248e654ecefd 100644
> --- a/drivers/mtd/nand/raw/nand_base.c
> +++ b/drivers/mtd/nand/raw/nand_base.c
> @@ -2173,7 +2173,7 @@ EXPORT_SYMBOL_GPL(nand_reset_op);
>  int nand_read_data_op(struct nand_chip *chip, void *buf, unsigned int len,
>  		      bool force_8bit, bool check_only)
>  {
> -	if (!len || !buf)
> +	if (!len || (!check_only && !buf))
>  		return -EINVAL;
>  
>  	if (nand_has_exec_op(chip)) {

Thanks for tacking care of this.

Reviewed-by: Alexander Dahl <ada@thorsis.com>

Greets
Alex
diff mbox series

Patch

diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
index acd137dd0957..248e654ecefd 100644
--- a/drivers/mtd/nand/raw/nand_base.c
+++ b/drivers/mtd/nand/raw/nand_base.c
@@ -2173,7 +2173,7 @@  EXPORT_SYMBOL_GPL(nand_reset_op);
 int nand_read_data_op(struct nand_chip *chip, void *buf, unsigned int len,
 		      bool force_8bit, bool check_only)
 {
-	if (!len || !buf)
+	if (!len || (!check_only && !buf))
 		return -EINVAL;
 
 	if (nand_has_exec_op(chip)) {