From patchwork Wed Nov 13 19:00:15 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Rob Herring X-Patchwork-Id: 3179341 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.19.201]) by patchwork1.web.kernel.org (Postfix) with ESMTP id 46DB89F432 for ; Wed, 13 Nov 2013 19:01:20 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id CD5792075A for ; Wed, 13 Nov 2013 19:01:18 +0000 (UTC) Received: from casper.infradead.org (casper.infradead.org [85.118.1.10]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id F064B2074C for ; Wed, 13 Nov 2013 19:01:13 +0000 (UTC) Received: from merlin.infradead.org ([2001:4978:20e::2]) by casper.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1VgfgR-0004YN-D4; Wed, 13 Nov 2013 19:00:59 +0000 Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1VgfgH-0001ei-0Q; Wed, 13 Nov 2013 19:00:49 +0000 Received: from mail-ob0-f172.google.com ([209.85.214.172]) by merlin.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1VgfgC-0001TT-VM for linux-arm-kernel@lists.infradead.org; Wed, 13 Nov 2013 19:00:46 +0000 Received: by mail-ob0-f172.google.com with SMTP id wm4so962410obc.3 for ; Wed, 13 Nov 2013 11:00:19 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=message-id:date:from:user-agent:mime-version:to:cc:subject :references:in-reply-to:content-type:content-transfer-encoding; bh=WyfRcDQP1ogdZAqQLtuSUClYLQdoPL2Neyx/zdGMmEY=; b=T7lkEN40vuc3Wttc0JVx2V/8GfA7+bc/Xs0l3xPR6jfqLuhPJFoBk0mcrEZvtq4OZJ LKMSBkGzH6fYqCPqX9ptY4oPuF53jQmrO+5g2yKJF07J5fZBZXpgjsWqxJujZtWBIqIE dVk+yzn9nbZAq17JiLR/SDXemKw1j2TCSqz3BC+OzrLDNf8tnnKbadDFBNb/yBdcY+yj HjWyEqEdAPU24PMG25l37AOFjnFEnFtAZFsBEEkPVM+dQr93ZUI9SEHmDv1OQT840LX+ LRGSUvPqDQAwcN1TIM6t9mF4c6AkWodwXF/Qv6o5lYVPA+ny4jr+N8uFkReS9PB16NyY Za+A== X-Received: by 10.60.56.42 with SMTP id x10mr2459437oep.95.1384369216421; Wed, 13 Nov 2013 11:00:16 -0800 (PST) Received: from [10.10.10.90] ([173.226.190.126]) by mx.google.com with ESMTPSA id rr6sm365578oeb.0.2013.11.13.11.00.15 for (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Wed, 13 Nov 2013 11:00:15 -0800 (PST) Message-ID: <5283CC3F.7030306@gmail.com> Date: Wed, 13 Nov 2013 13:00:15 -0600 From: Rob Herring User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.1.0 MIME-Version: 1.0 To: Olof Johansson , Grant Likely Subject: Re: [PATCH v2] ARM: remove name from machine_desc for DT platforms References: <1383511801-31682-1-git-send-email-robherring2@gmail.com> <20131111173826.GA8809@quad.lixom.net> In-Reply-To: <20131111173826.GA8809@quad.lixom.net> X-Enigmail-Version: 1.6 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20131113_140045_152067_00CFF7D5 X-CRM114-Status: GOOD ( 31.42 ) X-Spam-Score: -2.5 (--) Cc: "arm@kernel.org" , "linux-arm-kernel@lists.infradead.org" , Rob Herring X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , 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.1 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 11/11/2013 11:38 AM, Olof Johansson wrote: > On Tue, Nov 05, 2013 at 08:34:56AM -0600, Rob Herring wrote: >> On Mon, Nov 4, 2013 at 5:36 AM, Grant Likely wrote: >>> On Sun, Nov 3, 2013 at 8:50 PM, Rob Herring wrote: >>>> From: Rob Herring >>>> >>>> As part of the effort to ultimately remove struct machine_desc, the >>>> machine name is not really needed. It is only used for /proc/cpuinfo >>>> "Hardware" field. Get this information from the DT instead and remove >>>> the name string in the machine descriptor for DT machine descriptors. The >>>> model or machine compatible property from the DT is used instead. >>> >>> I've obviously missed some discussions because I wasn't aware of the >>> plan to remove machine_desc. That's neither here not there, but for as >>> long as the structure still exists it is really useful to discover >>> which machine_desc the kernel chooses at boot time. If you're going to >>> remove this, then at the very least I would embed the filename or >>> something similar into the machine desc so it is available for >>> debugging. It can go away when the structure is removed. >> >> It's a very long term goal to remove it completely, but it is now >> optional for "simple" platforms with the default machine_desc and >> default init ptrs. You might want to look at the patches and the WIP >> branch I posted[1] which removes the machine_desc for highbank. >> There's some core boilerplate I'd like your opinion on. Also, I'm sure >> you are aware of the resistance to add a machine_desc to arm64, so I'm >> looking at this from that perspective as well. I'm sure there will be >> platforms which look very similar across arm and arm64. >> >> On boot, the model or compatible string from the DTB is printed out >> (that change is actually in my DT arch clean-up series, not this >> patch). This has made what is printed out on boot consistent across >> arches using DT. There is no good reason for this to be arch specific. >> Presumably the compatible string is unique and just as easily grepped >> for in the kernel source as the machine_desc name is. Adding a >> filename string here seems like useless bloat to me. > > I think I agree a bit with Grant's concern here. > > The compatible string is unique, but the machine descriptors might not > be -- a given device tree might match two of them (one more generic, > one more specific) -- it would be useful to know which one was used. Or, > if you missed to enable the platform support for your board so it booted > with the generic descriptor, but you still needed your own. I would push back on anyone having multiple machine descriptors for that purpose. > Matching it back to which devicetree the kernel was booted with shouldn't be > hard, I agree -- but knowing which machine entry was picked at boot time > is still useful information. We need to separate what we print on boot vs. what is in /proc/cpuinfo. One issue I'm trying to solve here is customers complain that Midway systems print "Calxeda Highbank" in /proc/cpuinfo. I don't think adding a machine desc only for adding a string for Midway would be right. Also, I'm close to removing the highbank machine descriptor altogether and printing out "Generic DT" from the default machine desc is not particularly useful. Do we at least have agreement that the "right" thing to print in cpuinfo is the model or compatible from DT? What is printed on boot is already changed to be based on DT, but that seems to be what you and Grant care about. Does something like this work for you? This will print either "Machine model: model string (matched compat)" or "Machine model: 1st compat str (matched compat)" Rob @@ -652,6 +653,7 @@ const void * __init of_flat_dt_match_machine(const void *default_match, while ((data = get_next_compat(&compat))) { score = of_flat_dt_match(dt_root, compat); if (score > 0 && score < best_score) { + best_compat = compat[score - 1]; best_data = data; best_score = score; } @@ -674,7 +676,7 @@ const void * __init of_flat_dt_match_machine(const void *default_match, return NULL; } - pr_info("Machine model: %s\n", of_flat_dt_get_machine_name()); + pr_info("Machine model: %s (matched %s)\n", of_flat_dt_get_machine_name(), best_compat); return best_data; } diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c index 5c47910..466b360 100644 --- a/drivers/of/fdt.c +++ b/drivers/of/fdt.c @@ -645,6 +645,7 @@ const void * __init of_flat_dt_match_machine(const void *default_match, const void *data = NULL; const void *best_data = default_match; const char *const *compat; + const char *best_compat = NULL; unsigned long dt_root; unsigned int best_score = ~1, score = 0;