[v5,11/12] mtd: rawnand: brcmnand: exec_op helper functions return type fixes

  From: David Regan <dregan@broadcom.com>

fix return type for exec_op reset and status detect helper functions

Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
Closes: http://lists.infradead.org/pipermail/linux-mtd/2023-December/102423.html
Fixes: 3c8260ce7663 ("mtd: rawnand: brcmnand: exec_op implementation")
Signed-off-by: David Regan <dregan@broadcom.com>
Signed-off-by: William Zhang <william.zhang@broadcom.com>
Reviewed-by: William Zhang <william.zhang@broadcom.com>

---

Changes in v5: None
Changes in v4:
- Fix the commit id in the fixes tag

Changes in v3: None
Changes in v2:
- Added to patch series

 drivers/mtd/nand/raw/brcmnand/brcmnand.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)
  

Hi,

william.zhang@broadcom.com wrote on Wed,  7 Feb 2024 12:22:56 -0800:

> From: David Regan <dregan@broadcom.com>
> 
> fix return type for exec_op reset and status detect helper functions

Please make a correct sentence (Fic, detect?, '.').

> Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
> Closes: http://lists.infradead.org/pipermail/linux-mtd/2023-December/102423.html
> Fixes: 3c8260ce7663 ("mtd: rawnand: brcmnand: exec_op implementation")
> Signed-off-by: David Regan <dregan@broadcom.com>
> Signed-off-by: William Zhang <william.zhang@broadcom.com>
> Reviewed-by: William Zhang <william.zhang@broadcom.com>
> 
> ---
> 
> Changes in v5: None
> Changes in v4:
> - Fix the commit id in the fixes tag
> 
> Changes in v3: None
> Changes in v2:
> - Added to patch series
> 
>  drivers/mtd/nand/raw/brcmnand/brcmnand.c | 15 +++++++++------
>  1 file changed, 9 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
> index 8c1489ff7bd6..7ce2b267676f 100644
> --- a/drivers/mtd/nand/raw/brcmnand/brcmnand.c
> +++ b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
> @@ -625,7 +625,7 @@ enum {
>  /* Only for v7.2 */
>  #define	ACC_CONTROL_ECC_EXT_SHIFT		13
>  
> -static u8 brcmnand_status(struct brcmnand_host *host);
> +static int brcmnand_status(struct brcmnand_host *host);
>  
>  static inline bool brcmnand_non_mmio_ops(struct brcmnand_controller *ctrl)
>  {
> @@ -1749,7 +1749,7 @@ static int brcmnand_waitfunc(struct nand_chip *chip)
>  				 INTFC_FLASH_STATUS;
>  }
>  
> -static u8 brcmnand_status(struct brcmnand_host *host)
> +static int brcmnand_status(struct brcmnand_host *host)
>  {
>  	struct nand_chip *chip = &host->chip;
>  	struct mtd_info *mtd = nand_to_mtd(chip);
> @@ -1760,7 +1760,7 @@ static u8 brcmnand_status(struct brcmnand_host *host)
>  	return brcmnand_waitfunc(chip);
>  }
>  
> -static u8 brcmnand_reset(struct brcmnand_host *host)
> +static int brcmnand_reset(struct brcmnand_host *host)
>  {
>  	struct nand_chip *chip = &host->chip;
>  
> @@ -2492,11 +2492,14 @@ static int brcmnand_exec_op(struct nand_chip *chip,
>  
>  	if (brcmnand_op_is_status(op)) {
>  		status = op->instrs[1].ctx.data.buf.in;
> -		*status = brcmnand_status(host);
> +		ret = brcmnand_status(host);
> +		if (ret < 0)
> +			return ret;
> +
> +		*status = ret & 0xFF;
>  
>  		return 0;
> -	}
> -	else if (brcmnand_op_is_reset(op)) {
> +	} else if (brcmnand_op_is_reset(op)) {

This is another change, please make it in another patch. Also while you
are at it, there are probably other checkpatch warnings that can be
fixed, if you want.

>  		ret = brcmnand_reset(host);
>  		if (ret < 0)
>  			return ret;


Thanks,
Miquèl
  

william.zhang@broadcom.com wrote on Wed,  7 Feb 2024 12:22:56 -0800:

> From: David Regan <dregan@broadcom.com>
> 
> fix return type for exec_op reset and status detect helper functions
> 
> Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
> Closes: http://lists.infradead.org/pipermail/linux-mtd/2023-December/102423.html
> Fixes: 3c8260ce7663 ("mtd: rawnand: brcmnand: exec_op implementation")
> Signed-off-by: David Regan <dregan@broadcom.com>
> Signed-off-by: William Zhang <william.zhang@broadcom.com>
> Reviewed-by: William Zhang <william.zhang@broadcom.com>

By the way, a Cc: stable tag might be useful as otherwise you may leak
an error code in a status value (which is a bug). And move this patch
first in your series so we're sure it does not conflict with any of the
other patches and can be backported more easily.

Thanks,
Miquèl
  

Hi Miquel,

On 2/20/24 02:02, Miquel Raynal wrote:
> 
> william.zhang@broadcom.com wrote on Wed,  7 Feb 2024 12:22:56 -0800:
> 
>> From: David Regan <dregan@broadcom.com>
>>
>> fix return type for exec_op reset and status detect helper functions
>>
>> Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
>> Closes: http://lists.infradead.org/pipermail/linux-mtd/2023-December/102423.html
>> Fixes: 3c8260ce7663 ("mtd: rawnand: brcmnand: exec_op implementation")
>> Signed-off-by: David Regan <dregan@broadcom.com>
>> Signed-off-by: William Zhang <william.zhang@broadcom.com>
>> Reviewed-by: William Zhang <william.zhang@broadcom.com>
> 
> By the way, a Cc: stable tag might be useful as otherwise you may leak
Sorry I am not very familiar with these term and release procedure.  But 
I don't see exec_op patch is in the current stable release 6.7.5 and LTS 
release.  It is only in the mainline and linux-next.  Do you mean the 
stable tag will make it to the mainline?
> an error code in a status value (which is a bug). And move this patch
> first in your series so we're sure it does not conflict with any of the
> other patches and can be backported more easily.
> 
> Thanks,
> Miquèl
  

On Tue, Feb 20, 2024 at 11:02:40AM +0100, Miquel Raynal wrote:
> 
> william.zhang@broadcom.com wrote on Wed,  7 Feb 2024 12:22:56 -0800:
> 
> > From: David Regan <dregan@broadcom.com>
> > 
> > fix return type for exec_op reset and status detect helper functions
> > 
> > Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
> > Closes: http://lists.infradead.org/pipermail/linux-mtd/2023-December/102423.html
> > Fixes: 3c8260ce7663 ("mtd: rawnand: brcmnand: exec_op implementation")
> > Signed-off-by: David Regan <dregan@broadcom.com>
> > Signed-off-by: William Zhang <william.zhang@broadcom.com>
> > Reviewed-by: William Zhang <william.zhang@broadcom.com>
> 
> By the way, a Cc: stable tag might be useful as otherwise you may leak
> an error code in a status value (which is a bug). And move this patch
> first in your series so we're sure it does not conflict with any of the
> other patches and can be backported more easily.

The original commit 3c8260ce7663 ("mtd: rawnand: brcmnand: exec_op
implementation") is not in stable so we should be okay on this.

regards,
dan carpenter
  

Hi,

dan.carpenter@linaro.org wrote on Wed, 21 Feb 2024 09:16:31 +0300:

> On Tue, Feb 20, 2024 at 11:02:40AM +0100, Miquel Raynal wrote:
> > 
> > william.zhang@broadcom.com wrote on Wed,  7 Feb 2024 12:22:56 -0800:
> >   
> > > From: David Regan <dregan@broadcom.com>
> > > 
> > > fix return type for exec_op reset and status detect helper functions
> > > 
> > > Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
> > > Closes: http://lists.infradead.org/pipermail/linux-mtd/2023-December/102423.html
> > > Fixes: 3c8260ce7663 ("mtd: rawnand: brcmnand: exec_op implementation")
> > > Signed-off-by: David Regan <dregan@broadcom.com>
> > > Signed-off-by: William Zhang <william.zhang@broadcom.com>
> > > Reviewed-by: William Zhang <william.zhang@broadcom.com>  
> > 
> > By the way, a Cc: stable tag might be useful as otherwise you may leak
> > an error code in a status value (which is a bug). And move this patch
> > first in your series so we're sure it does not conflict with any of the
> > other patches and can be backported more easily.  
> 
> The original commit 3c8260ce7663 ("mtd: rawnand: brcmnand: exec_op
> implementation") is not in stable so we should be okay on this.

Right.

However please send again this patch quickly so I can queue it up
before the rest of the series.

Thanks,
Miquèl
  

On 2/21/2024 12:32 AM, 'Miquel Raynal' via BCM-KERNEL-FEEDBACK-LIST,PDL 
wrote:
> Hi,
> 
> dan.carpenter@linaro.org wrote on Wed, 21 Feb 2024 09:16:31 +0300:
> 
>> On Tue, Feb 20, 2024 at 11:02:40AM +0100, Miquel Raynal wrote:
>>>
>>> william.zhang@broadcom.com wrote on Wed,  7 Feb 2024 12:22:56 -0800:
>>>    
>>>> From: David Regan <dregan@broadcom.com>
>>>>
>>>> fix return type for exec_op reset and status detect helper functions
>>>>
>>>> Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
>>>> Closes: http://lists.infradead.org/pipermail/linux-mtd/2023-December/102423.html
>>>> Fixes: 3c8260ce7663 ("mtd: rawnand: brcmnand: exec_op implementation")
>>>> Signed-off-by: David Regan <dregan@broadcom.com>
>>>> Signed-off-by: William Zhang <william.zhang@broadcom.com>
>>>> Reviewed-by: William Zhang <william.zhang@broadcom.com>
>>>
>>> By the way, a Cc: stable tag might be useful as otherwise you may leak
>>> an error code in a status value (which is a bug). And move this patch
>>> first in your series so we're sure it does not conflict with any of the
>>> other patches and can be backported more easily.
>>
>> The original commit 3c8260ce7663 ("mtd: rawnand: brcmnand: exec_op
>> implementation") is not in stable so we should be okay on this.
> 
> Right.
> 
> However please send again this patch quickly so I can queue it up
> before the rest of the series.

Also just to be clear, please do not omit the Fixes: tag, even if the 
commit being fixed is not yet in any released kernel, it is valuable to 
have that information, for say, people who might consider backporting 
features into specific kernel versions. The stable team's tooling should 
be able to figure out that it is not required to pick up that fix since 
the offending commit is not in a released kernel yet anyway.

Thanks!