[PATCH] mtd: nand: raw: atmel: remove unnecessary return value
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,
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
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
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
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
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
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
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
participants (3)
-
Alexander Dahl -
Marcus Folkesson -
Michael Nazzareno Trimarchi