| Message ID | 20240809121556.3215132-1-marcus.folkesson@gmail.com |
|---|---|
| State | New |
| Delegated to: | Dario Binacchi |
| Headers | show |
| Series | mtd: nand: raw: atmel: remove unnecessary return value | expand |
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
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,
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(-)