[PATCH] mtd: nand: raw: atmel: remove unnecessary return value

[PATCH] mtd: nand: raw: atmel: remove unnecessary return value

[Marcus Folkesson]

The condition 'ret' is always true as it is never set to other than
-EIO.

Remove 'ret' and the condition for copy.

Signed-off-by: Marcus Folkesson <marcus.folkesson@gmail.com>
---

 drivers/mtd/nand/raw/atmel/nand-controller.c | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/drivers/mtd/nand/raw/atmel/nand-controller.c b/drivers/mtd/nand/raw/atmel/nand-controller.c
index ee4ec6da58..00d7e177b9 100644
--- a/drivers/mtd/nand/raw/atmel/nand-controller.c
+++ b/drivers/mtd/nand/raw/atmel/nand-controller.c
@@ -568,12 +568,9 @@ static void atmel_nfc_copy_to_sram(struct nand_chip *chip, const u8 *buf,
 	struct mtd_info *mtd = nand_to_mtd(chip);
 	struct atmel_nand *nand = to_atmel_nand(chip);
 	struct atmel_hsmc_nand_controller *nc;
-	int ret = -EIO;
 
 	nc = to_hsmc_nand_controller(nand->controller);
-
-	if (ret)
-		memcpy_toio(nc->sram.virt, buf, mtd->writesize);
+	memcpy_toio(nc->sram.virt, buf, mtd->writesize);
 
 	if (oob_required)
 		memcpy_toio(nc->sram.virt + mtd->writesize, chip->oob_poi,
@@ -586,12 +583,9 @@ static void atmel_nfc_copy_from_sram(struct nand_chip *chip, u8 *buf,
 	struct mtd_info *mtd = nand_to_mtd(chip);
 	struct atmel_nand *nand = to_atmel_nand(chip);
 	struct atmel_hsmc_nand_controller *nc;
-	int ret = -EIO;
 
 	nc = to_hsmc_nand_controller(nand->controller);
-
-	if (ret)
-		memcpy_fromio(buf, nc->sram.virt, mtd->writesize);
+	memcpy_fromio(buf, nc->sram.virt, mtd->writesize);
 
 	if (oob_required)
 		memcpy_fromio(chip->oob_poi, nc->sram.virt + mtd->writesize,
-- 
2.45.1

Re: [PATCH] mtd: nand: raw: atmel: remove unnecessary return value

[Alexander Dahl]

Hello Marcus,

Am Fri, Aug 09, 2024 at 02:15:43PM +0200 schrieb Marcus Folkesson:
> The condition 'ret' is always true as it is never set to other than
> -EIO.

Technically, you're right.

I quickly compared with the same driver in Linux.  That has some
additional lines for DMA transfers which probably got removed when
porting the driver.

Does the code before your patch throw compiler warnings?  If not, I
would keep it as is.  The compiler will probably optimize it away
anyway, and it would make future ports from Linux easier.

Greets
Alex

> 
> Remove 'ret' and the condition for copy.
> 
> Signed-off-by: Marcus Folkesson <marcus.folkesson@gmail.com>
> ---
> 
>  drivers/mtd/nand/raw/atmel/nand-controller.c | 10 ++--------
>  1 file changed, 2 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/mtd/nand/raw/atmel/nand-controller.c b/drivers/mtd/nand/raw/atmel/nand-controller.c
> index ee4ec6da58..00d7e177b9 100644
> --- a/drivers/mtd/nand/raw/atmel/nand-controller.c
> +++ b/drivers/mtd/nand/raw/atmel/nand-controller.c
> @@ -568,12 +568,9 @@ static void atmel_nfc_copy_to_sram(struct nand_chip *chip, const u8 *buf,
>  	struct mtd_info *mtd = nand_to_mtd(chip);
>  	struct atmel_nand *nand = to_atmel_nand(chip);
>  	struct atmel_hsmc_nand_controller *nc;
> -	int ret = -EIO;
>  
>  	nc = to_hsmc_nand_controller(nand->controller);
> -
> -	if (ret)
> -		memcpy_toio(nc->sram.virt, buf, mtd->writesize);
> +	memcpy_toio(nc->sram.virt, buf, mtd->writesize);
>  
>  	if (oob_required)
>  		memcpy_toio(nc->sram.virt + mtd->writesize, chip->oob_poi,
> @@ -586,12 +583,9 @@ static void atmel_nfc_copy_from_sram(struct nand_chip *chip, u8 *buf,
>  	struct mtd_info *mtd = nand_to_mtd(chip);
>  	struct atmel_nand *nand = to_atmel_nand(chip);
>  	struct atmel_hsmc_nand_controller *nc;
> -	int ret = -EIO;
>  
>  	nc = to_hsmc_nand_controller(nand->controller);
> -
> -	if (ret)
> -		memcpy_fromio(buf, nc->sram.virt, mtd->writesize);
> +	memcpy_fromio(buf, nc->sram.virt, mtd->writesize);
>  
>  	if (oob_required)
>  		memcpy_fromio(chip->oob_poi, nc->sram.virt + mtd->writesize,
> -- 
> 2.45.1
> 

Re: [PATCH] mtd: nand: raw: atmel: remove unnecessary return value

[Michael Nazzareno Trimarchi]

Hi all

On Fri, Aug 9, 2024 at 2:25 PM Alexander Dahl <ada@thorsis.com> wrote:
>
> Hello Marcus,
>
> Am Fri, Aug 09, 2024 at 02:15:43PM +0200 schrieb Marcus Folkesson:
> > The condition 'ret' is always true as it is never set to other than
> > -EIO.
>
> Technically, you're right.
>
> I quickly compared with the same driver in Linux.  That has some
> additional lines for DMA transfers which probably got removed when
> porting the driver.
>
> Does the code before your patch throw compiler warnings?  If not, I
> would keep it as is.  The compiler will probably optimize it away
> anyway, and it would make future ports from Linux easier.
>

I suggest to comment it because it will happen to other people to send a similar
patch

Michael

> Greets
> Alex
>
> >
> > Remove 'ret' and the condition for copy.
> >
> > Signed-off-by: Marcus Folkesson <marcus.folkesson@gmail.com>
> > ---
> >
> >  drivers/mtd/nand/raw/atmel/nand-controller.c | 10 ++--------
> >  1 file changed, 2 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/mtd/nand/raw/atmel/nand-controller.c b/drivers/mtd/nand/raw/atmel/nand-controller.c
> > index ee4ec6da58..00d7e177b9 100644
> > --- a/drivers/mtd/nand/raw/atmel/nand-controller.c
> > +++ b/drivers/mtd/nand/raw/atmel/nand-controller.c
> > @@ -568,12 +568,9 @@ static void atmel_nfc_copy_to_sram(struct nand_chip *chip, const u8 *buf,
> >       struct mtd_info *mtd = nand_to_mtd(chip);
> >       struct atmel_nand *nand = to_atmel_nand(chip);
> >       struct atmel_hsmc_nand_controller *nc;
> > -     int ret = -EIO;
> >
> >       nc = to_hsmc_nand_controller(nand->controller);
> > -
> > -     if (ret)
> > -             memcpy_toio(nc->sram.virt, buf, mtd->writesize);
> > +     memcpy_toio(nc->sram.virt, buf, mtd->writesize);
> >
> >       if (oob_required)
> >               memcpy_toio(nc->sram.virt + mtd->writesize, chip->oob_poi,
> > @@ -586,12 +583,9 @@ static void atmel_nfc_copy_from_sram(struct nand_chip *chip, u8 *buf,
> >       struct mtd_info *mtd = nand_to_mtd(chip);
> >       struct atmel_nand *nand = to_atmel_nand(chip);
> >       struct atmel_hsmc_nand_controller *nc;
> > -     int ret = -EIO;
> >
> >       nc = to_hsmc_nand_controller(nand->controller);
> > -
> > -     if (ret)
> > -             memcpy_fromio(buf, nc->sram.virt, mtd->writesize);
> > +     memcpy_fromio(buf, nc->sram.virt, mtd->writesize);
> >
> >       if (oob_required)
> >               memcpy_fromio(chip->oob_poi, nc->sram.virt + mtd->writesize,
> > --
> > 2.45.1
> >



-- 
Michael Nazzareno Trimarchi
Co-Founder & Chief Executive Officer
M. +39 347 913 2170
michael@amarulasolutions.com
__________________________________

Amarula Solutions BV
Joop Geesinkweg 125, 1114 AB, Amsterdam, NL
T. +31 (0)85 111 9172
info@amarulasolutions.com
www.amarulasolutions.com

Re: [PATCH] mtd: nand: raw: atmel: remove unnecessary return value

[Marcus Folkesson]

[-- Attachment #1: Type: text/plain, Size: 1231 bytes --]

Hello Alexander,

Thanks for fast response!

On Fri, Aug 09, 2024 at 02:25:04PM +0200, Alexander Dahl wrote:
> Hello Marcus,
> 
> Am Fri, Aug 09, 2024 at 02:15:43PM +0200 schrieb Marcus Folkesson:
> > The condition 'ret' is always true as it is never set to other than
> > -EIO.
> 
> Technically, you're right.
> 
> I quickly compared with the same driver in Linux.  That has some
> additional lines for DMA transfers which probably got removed when
> porting the driver.

Yes, I thought is was something like that.
> 
> Does the code before your patch throw compiler warnings?  If not, I

Not the compiler, but vim-ale (lint engine) is yelling loudly at me.

> would keep it as is.  The compiler will probably optimize it away
> anyway, and it would make future ports from Linux easier.

I understand your reasoning but not sure I agree.

I don't think it significantly complicates any porting and the code becomes cleaner.

I also think that the porting become less error-prone because it becomes a 
conscious choice to introduce and use ret if needed.

That is what I think, but I don't really have a very strong opinion about it.

> 
> Greets
> Alex
> 

Best regards,
Marcus Folkesson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] mtd: nand: raw: atmel: remove unnecessary return value

[Marcus Folkesson]

[-- Attachment #1: Type: text/plain, Size: 329 bytes --]

On Fri, Aug 09, 2024 at 02:15:43PM +0200, Marcus Folkesson wrote:
> The condition 'ret' is always true as it is never set to other than
> -EIO.
> 
> Remove 'ret' and the condition for copy.
> 
> Signed-off-by: Marcus Folkesson <marcus.folkesson@gmail.com>
> ---

Any more thoughts on this?

Thanks,
Marcus Folkesson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] mtd: nand: raw: atmel: remove unnecessary return value

[Michael Nazzareno Trimarchi]

Hi Marcus

On Fri, Aug 30, 2024 at 8:59 AM Marcus Folkesson
<marcus.folkesson@gmail.com> wrote:
>
> On Fri, Aug 09, 2024 at 02:15:43PM +0200, Marcus Folkesson wrote:
> > The condition 'ret' is always true as it is never set to other than
> > -EIO.
> >
> > Remove 'ret' and the condition for copy.
> >
> > Signed-off-by: Marcus Folkesson <marcus.folkesson@gmail.com>
> > ---
>
> Any more thoughts on this?

Reviewed-by: Michael Trimarchi <micheal@amarulasolutions.com>

It will be included in the next pull request.

Michael

>
> Thanks,
> Marcus Folkesson



-- 
Michael Nazzareno Trimarchi
Co-Founder & Chief Executive Officer
M. +39 347 913 2170
michael@amarulasolutions.com
__________________________________

Amarula Solutions BV
Joop Geesinkweg 125, 1114 AB, Amsterdam, NL
T. +31 (0)85 111 9172
info@amarulasolutions.com
www.amarulasolutions.com

Re: [PATCH] mtd: nand: raw: atmel: remove unnecessary return value

[Marcus Folkesson]

[-- Attachment #1: Type: text/plain, Size: 749 bytes --]

Hello Michael,

On Fri, Aug 30, 2024 at 09:05:27AM +0200, Michael Nazzareno Trimarchi wrote:
> Hi Marcus
> 
> On Fri, Aug 30, 2024 at 8:59 AM Marcus Folkesson
> <marcus.folkesson@gmail.com> wrote:
> >
> > On Fri, Aug 09, 2024 at 02:15:43PM +0200, Marcus Folkesson wrote:
> > > The condition 'ret' is always true as it is never set to other than
> > > -EIO.
> > >
> > > Remove 'ret' and the condition for copy.
> > >
> > > Signed-off-by: Marcus Folkesson <marcus.folkesson@gmail.com>
> > > ---
> >
> > Any more thoughts on this?
> 
> Reviewed-by: Michael Trimarchi <micheal@amarulasolutions.com>
> 
> It will be included in the next pull request.

Not sure it got included?

> 
> Michael
> 

Thanks,
Marcus Folkesson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] mtd: nand: raw: atmel: remove unnecessary return value

[Michael Nazzareno Trimarchi]

Hi

Il mar 10 dic 2024, 08:08 Marcus Folkesson <marcus.folkesson@gmail.com> ha
scritto:

> Hello Michael,
>
> On Fri, Aug 30, 2024 at 09:05:27AM +0200, Michael Nazzareno Trimarchi
> wrote:
> > Hi Marcus
> >
> > On Fri, Aug 30, 2024 at 8:59 AM Marcus Folkesson
> > <marcus.folkesson@gmail.com> wrote:
> > >
> > > On Fri, Aug 09, 2024 at 02:15:43PM +0200, Marcus Folkesson wrote:
> > > > The condition 'ret' is always true as it is never set to other than
> > > > -EIO.
> > > >
> > > > Remove 'ret' and the condition for copy.
> > > >
> > > > Signed-off-by: Marcus Folkesson <marcus.folkesson@gmail.com>
> > > > ---
> > >
> > > Any more thoughts on this?
> >
> > Reviewed-by: Michael Trimarchi <micheal@amarulasolutions.com>
> >
> > It will be included in the next pull request.
>
> Not sure it got included?
>

I will pick today.

Michael

>
> >
> > Michael
> >
>
> Thanks,
> Marcus Folkesson
>