From patchwork Wed Jun 24 12:59:16 2009 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Catalin Marinas X-Patchwork-Id: 29125 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from bombadil.infradead.org (bombadil.infradead.org [18.85.46.34]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client did not present a certificate) by bilbo.ozlabs.org (Postfix) with ESMTPS id 41C32B7090 for ; Wed, 24 Jun 2009 23:04:09 +1000 (EST) Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.69 #1 (Red Hat Linux)) id 1MJS5D-0001xr-Oy; Wed, 24 Jun 2009 13:00:21 +0000 Received: from cam-admin0.cambridge.arm.com ([193.131.176.58]) by bombadil.infradead.org with esmtp (Exim 4.69 #1 (Red Hat Linux)) id 1MJS55-0001U9-Fa for linux-mtd@lists.infradead.org; Wed, 24 Jun 2009 13:00:07 +0000 Received: from cam-owa1.Emea.Arm.com (cam-owa1.emea.arm.com [10.1.255.62]) by cam-admin0.cambridge.arm.com (8.12.6/8.12.6) with ESMTP id n5OCuYZm016493; Wed, 24 Jun 2009 13:56:34 +0100 (BST) Received: from [10.1.68.81] ([10.1.255.212]) by cam-owa1.Emea.Arm.com with Microsoft SMTPSVC(6.0.3790.0); Wed, 24 Jun 2009 13:59:17 +0100 Subject: Re: BUS_ID_SIZE is going away From: Catalin Marinas To: David Woodhouse In-Reply-To: <1245842095.25547.5380.camel@macbook.infradead.org> References: <1245527124.30962.14.camel@yio.site> <1245675383.25547.50.camel@macbook.infradead.org> <1245832038.2483.3.camel@yio.site> <1245842095.25547.5380.camel@macbook.infradead.org> Organization: ARM Ltd Date: Wed, 24 Jun 2009 13:59:16 +0100 Message-Id: <1245848356.32629.14.camel@pc1117.cambridge.arm.com> Mime-Version: 1.0 X-Mailer: Evolution 2.22.3.1 X-OriginalArrivalTime: 24 Jun 2009 12:59:17.0928 (UTC) FILETIME=[956D5A80:01C9F4CB] X-Spam-Score: -4.0 (----) X-Spam-Report: SpamAssassin version 3.2.5 on bombadil.infradead.org summary: Content analysis details: (-4.0 points) pts rule name description ---- ---------------------- -------------------------------------------------- -4.0 RCVD_IN_DNSWL_MED RBL: Sender listed at http://www.dnswl.org/, medium trust [193.131.176.58 listed in list.dnswl.org] Cc: Russell King , Takashi Iwai , Greg KH , Kay Sievers , linux-kernel , James Bottomley , linux-mtd@lists.infradead.org, "David S. Miller" X-BeenThere: linux-mtd@lists.infradead.org X-Mailman-Version: 2.1.11 Precedence: list List-Id: Linux MTD discussion mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: linux-mtd-bounces@lists.infradead.org Errors-To: linux-mtd-bounces+incoming=patchwork.ozlabs.org@lists.infradead.org On Wed, 2009-06-24 at 12:14 +0100, David Woodhouse wrote: > On Wed, 2009-06-24 at 10:27 +0200, Kay Sievers wrote: > > On Mon, 2009-06-22 at 13:56 +0100, David Woodhouse wrote: > > > I have this queued for 2.6.31 but have been on jury duty for the last 2 > > > weeks so I'm hoping to get the pull request to Linus today now that I'm > > > free. > > > > The old one is gone, but seems you merged a new one with BUS_ID_SIZE :) > > > > ./drivers/mtd/maps/integrator-flash.c:#define SUBDEV_NAME_SIZE (BUS_ID_SIZE + 2) > > Hm, true :) > > Catalin, can you test the following? Yes, it still works (with some remarks below). > Btw, I'm unconvinced by the existing error handling -- if > armflash_subdev_probe() fails, it doesn't look like _that_ subdev > actually gets cleaned up in the loop at 'subdev_err:'. So we don't > release the memory region (and neither do we free the newly-kmalloced > name). If armflash_subdev_probe() fails, it cleans up after itself so there is no need to handle it in the subdev_err loop. With your patch, however, this should be done explicitly if the subdev probing fails. Something like below: diff --git a/drivers/mtd/maps/integrator-flash.c b/drivers/mtd/maps/integrator-flash.c index b9fac5b..2aac41b 100644 --- a/drivers/mtd/maps/integrator-flash.c +++ b/drivers/mtd/maps/integrator-flash.c @@ -188,8 +188,11 @@ static int armflash_probe(struct platform_device *dev) subdev->plat = plat; err = armflash_subdev_probe(subdev, res); - if (err) + if (err) { + kfree(subdev->name); + subdev->name = NULL; break; + } } info->nr_subdev = i; > In fact, do we still need a separate platform device and driver for ARM > systems? What does it provide that the physmap driver does (and can) > not? (I cc'ed Russell as well since he wrote the integrator-flash driver and may have comments) I gave physmap a quick try on RealView boards and it seems to work fine with the patch below. The only difference is the AFS partition parsing probes string, though this is no longer used on ARM Ltd platforms (some old ones still use it). I'll test it a bit more and, if there are no other comments, I'll push a patch to convert the ARM Ltd platforms to physmap (but that's for the next merging window as I don't think I have time to test it well enough now). RealView: Convert the platform code to use the physmap flash driver From: Catalin Marinas This platform was still using the integrator-flash driver but this pretty much duplicates the physmap one. Signed-off-by: Catalin Marinas --- arch/arm/mach-realview/core.c | 31 ++++--------------------------- 1 files changed, 4 insertions(+), 27 deletions(-) diff --git a/arch/arm/mach-realview/core.c b/arch/arm/mach-realview/core.c index 0cc6f42..ef33872 100644 --- a/arch/arm/mach-realview/core.c +++ b/arch/arm/mach-realview/core.c @@ -30,6 +30,7 @@ #include #include #include +#include #include #include @@ -41,7 +42,6 @@ #include #include -#include #include #include #include @@ -76,27 +76,7 @@ unsigned long long sched_clock(void) #define REALVIEW_FLASHCTRL (__io_address(REALVIEW_SYS_BASE) + REALVIEW_SYS_FLASH_OFFSET) -static int realview_flash_init(void) -{ - u32 val; - - val = __raw_readl(REALVIEW_FLASHCTRL); - val &= ~REALVIEW_FLASHPROG_FLVPPEN; - __raw_writel(val, REALVIEW_FLASHCTRL); - - return 0; -} - -static void realview_flash_exit(void) -{ - u32 val; - - val = __raw_readl(REALVIEW_FLASHCTRL); - val &= ~REALVIEW_FLASHPROG_FLVPPEN; - __raw_writel(val, REALVIEW_FLASHCTRL); -} - -static void realview_flash_set_vpp(int on) +static void realview_flash_set_vpp(struct map_info *map, int on) { u32 val; @@ -108,16 +88,13 @@ static void realview_flash_set_vpp(int on) __raw_writel(val, REALVIEW_FLASHCTRL); } -static struct flash_platform_data realview_flash_data = { - .map_name = "cfi_probe", +static struct physmap_flash_data realview_flash_data = { .width = 4, - .init = realview_flash_init, - .exit = realview_flash_exit, .set_vpp = realview_flash_set_vpp, }; struct platform_device realview_flash_device = { - .name = "armflash", + .name = "physmap-flash", .id = 0, .dev = { .platform_data = &realview_flash_data,