Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932925AbbBBJUY (ORCPT ); Mon, 2 Feb 2015 04:20:24 -0500 Received: from mail-pa0-f45.google.com ([209.85.220.45]:50216 "EHLO mail-pa0-f45.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752114AbbBBJUS (ORCPT ); Mon, 2 Feb 2015 04:20:18 -0500 Date: Mon, 2 Feb 2015 01:20:13 -0800 From: Brian Norris To: Ricard Wanderlof Cc: Niklas Cassel , "linux-mtd@lists.infradead.org" , "dwmw2@infradead.org" , "linux-kernel@vger.kernel.org" Subject: Re: [PATCH] mtd: avoid registering reboot notifier twice Message-ID: <20150202092013.GG3523@norris-Latitude-E6410> References: <1422752930-696-1-git-send-email-nks@flawful.org> <20150201230723.GA3523@norris-Latitude-E6410> <54CF2261.1040707@flawful.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2261 Lines: 52 On Mon, Feb 02, 2015 at 10:01:51AM +0100, Ricard Wanderlof wrote: > > > On 02/02/2015 12:07 AM, Brian Norris wrote: > > > > > > No driver should be calling mtd_device_parse_register() multiple times > > > on the same mtd_info. Under what context do you see this? What driver? > > arch/cris/arch-v32/drivers/axisflashmap.c > > Looking at it another way, why should it not be allowed, if the only > thing stopping it from working is Niklas' fix below? It severely breaks the way that normal MTD partition parsers work, for one. cmdlineparts could never be sanely applied here, for instance. > If there are multiple > ways a driver can acquire partitioning information, it is handy to be able > to do mtd_device_parse_register() several times, rather than collecting > all the partitioning information in one place and doing the call there, > which I suppose would be the alternative. It may be handy, but it breaks the abstraction that is established in most every other case, that partitions are determined by exactly one source: a single list from platform data, device tree, the kernel command line, or an on-flash parser (e.g., RedBoot, bcm47xxpart, etc.). > I've noted that mtd/nand/diskonchip.c does it too: > > mtd_device_register(mtd, NULL, 0); > if (!no_autopart) > mtd_device_register(mtd, parts, numparts); > > but this driver might be essentially deprecated, I don't know. Yeah, I just noticed that one too. TBH, I'm not sure the reboot notifier patch places that code in the best place anyway. There really is no central initialization code for the 'master' mtd_info struct, as this initialization is handled ad-hoc in every single MTD driver. It just happens that the partition parsing/registration function fit nicely as the one place that (almost) all MTD master devices got registered. But this just exposed yet another instance where my assumptions are challenged. There's so much legacy crap in MTD that sometimes I wonder why I even bother... Brian -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/