From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751423AbdJDU0x (ORCPT ); Wed, 4 Oct 2017 16:26:53 -0400 Received: from mail-wm0-f68.google.com ([74.125.82.68]:35933 "EHLO mail-wm0-f68.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751148AbdJDU0u (ORCPT ); Wed, 4 Oct 2017 16:26:50 -0400 X-Google-Smtp-Source: AOwi7QBr7fhkksWLpf4aQsFlU9N0OzcGOABBYeJ9STyKQidOpP2J0ZhJqEbe1ANKjnjzr+EAw7beMQ== Subject: Re: [PATCH 5/5] cris: nand: Split a condition check in crisv32_nand_flash_probe() To: SF Markus Elfring , linux-cris-kernel@axis.com, Jesper Nilsson , Mikael Starvik Cc: LKML References: <290ad3d7-eb40-5c60-068f-3835063babc2@users.sourceforge.net> From: Alexander Sverdlin Message-ID: <4aa6e2f1-2324-6740-021a-24ee8cd63104@gmail.com> Date: Wed, 4 Oct 2017 22:26:45 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.1 MIME-Version: 1.0 In-Reply-To: <290ad3d7-eb40-5c60-068f-3835063babc2@users.sourceforge.net> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hello Markus, On 04/10/17 20:50, SF Markus Elfring wrote: > * Split a condition check for failed calls of the function "ioremap" > so that the return value in the variable "write_cs" will also be > immediately checked. > > * Adjust jump targets according to the Linux coding style convention. > > Signed-off-by: Markus Elfring > --- > arch/cris/arch-v32/drivers/mach-fs/nandflash.c | 17 +++++++++++------ > 1 file changed, 11 insertions(+), 6 deletions(-) > > diff --git a/arch/cris/arch-v32/drivers/mach-fs/nandflash.c b/arch/cris/arch-v32/drivers/mach-fs/nandflash.c > index 2b53f0c615ea..564218a12213 100644 > --- a/arch/cris/arch-v32/drivers/mach-fs/nandflash.c > +++ b/arch/cris/arch-v32/drivers/mach-fs/nandflash.c > @@ -113,13 +113,17 @@ struct mtd_info *__init crisv32_nand_flash_probe(void) > return NULL; > > read_cs = ioremap(MEM_CSP0_START | MEM_NON_CACHEABLE, 8192); > - write_cs = ioremap(MEM_CSP1_START | MEM_NON_CACHEABLE, 8192); > - > - if (!read_cs || !write_cs) { > + if (!read_cs) { > printk(KERN_ERR "CRISv32 NAND ioremap failed\n"); > goto out_mtd; > } > > + write_cs = ioremap(MEM_CSP1_START | MEM_NON_CACHEABLE, 8192); > + if (!write_cs) { > + printk(KERN_ERR "CRISv32 NAND ioremap failed\n"); > + goto unmap_read; > + } > + Instead of duplicating the error message I'd rather put an "if" in the iounmap() path. > /* Get pointer to private data */ > this = &wrapper->chip; > crisv32_mtd = nand_to_mtd(this); > @@ -149,13 +153,14 @@ struct mtd_info *__init crisv32_nand_flash_probe(void) > > /* Scan to find existence of the device */ > if (nand_scan(crisv32_mtd, 1)) > - goto out_ior; > + goto unmap_io; > > return crisv32_mtd; > > -out_ior: > - iounmap((void *)read_cs); > +unmap_io: > iounmap((void *)write_cs); > +unmap_read: > + iounmap((void *)read_cs); I believe, (void *) casting is wrong, it does nothing than removing __iomem qualifier. I'd rather write if (read_cs) iounmap(read_cs); if (write_cs) iounmap(write_cs); > out_mtd: > kfree(wrapper);