From patchwork Mon Nov 16 04:12:44 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Brian Norris X-Patchwork-Id: 7620741 Return-Path: X-Original-To: patchwork-linux-arm@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork1.web.kernel.org (Postfix) with ESMTP id AD6719F2E2 for ; Mon, 16 Nov 2015 04:15:21 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id AA17820588 for ; Mon, 16 Nov 2015 04:15:20 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.9]) (using TLSv1.2 with cipher AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id A76662057E for ; Mon, 16 Nov 2015 04:15:19 +0000 (UTC) Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1ZyBAJ-0002o9-HO; Mon, 16 Nov 2015 04:13:15 +0000 Received: from mail-pa0-x22f.google.com ([2607:f8b0:400e:c03::22f]) by bombadil.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1ZyBAD-0002m8-D9; Mon, 16 Nov 2015 04:13:10 +0000 Received: by pacdm15 with SMTP id dm15so161161189pac.3; Sun, 15 Nov 2015 20:12:48 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=date:from:to:cc:subject:message-id:references:mime-version :content-type:content-disposition:in-reply-to:user-agent; bh=gnFHOi91Q0nf3nBYTql+g/OL0lhFtD7p4oB5ZrzIJCU=; b=hjKlBnuP/f6WKOFZOA0qW4Gw+dKHtUtRVU/IjaSvfaAk/iHnmFAIg9g5EXtj/5jyBA GcFoH759d50uX1UOyWrMmtdKrhh5Ehmi+LC9Mq8mlWnK5QPh8i89tBXBQN0KpFDriGxV ms10ELkPx361ivWA64naCzVN6AKJhFZ9UdxZ5yFWpzRIgasoGYEBz4cvN713dGiZ0oLH fkqtzvinjlGfGwoGTKqg9AMUDmfVzqzrlc+IGmbixlBSgq4F5WoQSm3GPLnUxS1SSfBJ /GOb4jhR1v3BZb/LRGThqZuYIrRDqm2U8qMuzDH5AtYAEPBZvzxmHeL5krCHjwtIpTcZ w5Vg== X-Received: by 10.68.178.229 with SMTP id db5mr51984307pbc.79.1447647168234; Sun, 15 Nov 2015 20:12:48 -0800 (PST) Received: from brian-ubuntu (c-71-198-3-2.hsd1.ca.comcast.net. [71.198.3.2]) by smtp.gmail.com with ESMTPSA id ij3sm33118846pbb.62.2015.11.15.20.12.46 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Sun, 15 Nov 2015 20:12:47 -0800 (PST) Date: Sun, 15 Nov 2015 20:12:44 -0800 From: Brian Norris To: Linus Walleij Subject: Re: [PATCH 1/3] mtd: create a partition type device tree binding Message-ID: <20151116041244.GA4021@brian-ubuntu> References: <1446123152-22666-1-git-send-email-linus.walleij@linaro.org> <20151029162908.GA13275@localhost> <20151030175145.GF13239@google.com> <20151113220039.GA74382@google.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.23 (2014-03-12) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20151115_201309_487810_E8B4FFF7 X-CRM114-Status: GOOD ( 27.44 ) X-Spam-Score: -2.7 (--) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Michal Suchanek , "devicetree@vger.kernel.org" , "devicetree-spec@vger.kernel.org" , Hauke Mehrtens , =?utf-8?B?UmFmYcWCIE1pxYJlY2tp?= , Liviu Dudau , Jason Gunthorpe , "linux-mtd@lists.infradead.org" , David Woodhouse , "linux-arm-kernel@lists.infradead.org" Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org X-Spam-Status: No, score=-4.7 required=5.0 tests=BAYES_00, DKIM_ADSP_CUSTOM_MED, DKIM_SIGNED, FREEMAIL_FROM, RCVD_IN_DNSWL_MED, RP_MATCHES_RCVD, T_DKIM_INVALID, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Sat, Nov 14, 2015 at 11:46:59AM +0100, Linus Walleij wrote: > On Fri, Nov 13, 2015 at 11:00 PM, Brian Norris > wrote: > > (...) > > (2) we should define a comptible property for the hard-coded > > partitioning case (e.g., compatible = "partitions") > (...) > > If we went with option (2), then ofpart.c could just check only for > > 'compatible = "partitions"' (or similar), and if not found bail out. > > So this "hard-coded partitioning case" the case is where all partitions > are defined in the device tree as described in > Documentation/devicetree/bindings/mtd/partition.txt ? Right. > Or is it a way to indicate that this array > static const char * const part_probe_types_def[] = { > "cmdlinepart", "RedBoot", "ofpart", "ofoldpart", NULL }; > in physmap_of.c should be used? No. At this point, I would consider that to be a legacy method. We still have to support this for many cases (including the non-DT case; but hopefully we can do better than that soon), and that would be an option. > Sorry if I don't fully follow, I'm a bit stupid when it comes to the MTD > helicopter view of the situation.... :( Yeah...sorry if I wasn't too clear. And I definitely don't blame you for not understanding the mess that MTD often is :( > > I think option (2) makes more sense. But it would require an update to > > the binding and code for 4.4, since [1] was only introduced during this > > release cycle. > > Does that mean all old device trees that specify no compatible > string all of a sudden stop working? We can't break the DT ABI, so I > guess not. No, that's not what I was intending. The binding before commmit fe2585e9c29a ("doc: dt: mtd: support partitions in a special 'partitions' subnode") should still stay working as-is. That is, we don't mess with the way things worked for anything that doesn't have a 'partitions' subnode. But now that we have a 'partitions' subnode (in 4.4-rc1), I'm just suggesting that we enforce this always have a compatible property, so we can be more clear on the difference between: partitions { // do I have a // compatible = "partitons"; // here? partition@0 { label = "foo-partition"; reg = <0 0x100000>; }; }; and partitions { compatible = "arm,arm-flash-structure"; subnode { // what if we need something here eventually? }; }; This would require some modifications to partitions.txt and to drivers/mtd/ofpart.c. > A bit confused here, I can't really see what I should do with the patch... Hopefully that cleared up a bit? The code changes for my suggestion would just be something like this, I think. (Not tested in any way.) I was just bringing this up for discussion, since it's related to your/Rob's new proposal. I'll send a proper (and tested) patch, along with a doc update, if that looks reasonable. Brian diff --git a/drivers/mtd/ofpart.c b/drivers/mtd/ofpart.c index 669c3452f278..6811bc5440a4 100644 --- a/drivers/mtd/ofpart.c +++ b/drivers/mtd/ofpart.c @@ -50,6 +50,10 @@ static int parse_ofpart_partitions(struct mtd_info *master, master->name, mtd_node->full_name); ofpart_node = mtd_node; dedicated = false; + } else { + /* The "partitions" subnode may belong to some other parser */ + if (!of_device_is_compatible(ofpart_node, "partitions")) + return 0; } /* First count the subnodes */