From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-4.0 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 08537C433B4 for ; Tue, 6 Apr 2021 07:17:08 +0000 (UTC) Received: from desiato.infradead.org (desiato.infradead.org [90.155.92.199]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id A822E61247 for ; Tue, 6 Apr 2021 07:17:07 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org A822E61247 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=desiato.20200630; h=Sender:Content-Transfer-Encoding :Content-Type:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:Cc:To:Subject:Message-ID:Date:From:In-Reply-To: References:MIME-Version:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=JAoenzuwElXOnCRLfBFvmneQUiuk71fAGZjl0vlBfSU=; b=m0t8MKQCZb37VEHy+U8iDVjbP WVGdKVZshX0/8kgFtVundHDZJuei3pHiNvZsGkHJboxoQ5oLPd5MIq0O0Yn2iqwDb3gEQk7cbri2l SdTpyrFYKNB8Z3wmMbkPJqXEi5AfAwj8XXDoA2LtD6l8pnnrns7KsavXNgGaK+dZL5OYJMIcxgAp2 sPouqrOllqWkvbW9uPU1vEzEgTlo+nvZS1epgK04ShAMGd/Vo1Un0vgpV/w9Hmr8LVM28cFtRJUw4 /c3Ac57vMxeqRJ7ImGy8zwklc6nnwHeL4vca3fVny7dBKBGIT4OABmvn42EqAAzMfJFglE0sNtw5n NZd1T3gpA==; Received: from localhost ([::1] helo=desiato.infradead.org) by desiato.infradead.org with esmtp (Exim 4.94 #2 (Red Hat Linux)) id 1lTfvk-001jtJ-Pa; Tue, 06 Apr 2021 07:15:22 +0000 Received: from mail.kernel.org ([198.145.29.99]) by desiato.infradead.org with esmtps (Exim 4.94 #2 (Red Hat Linux)) id 1lTfvd-001jsH-RK for linux-arm-kernel@lists.infradead.org; Tue, 06 Apr 2021 07:15:16 +0000 Received: by mail.kernel.org (Postfix) with ESMTPSA id A60A8601FF for ; Tue, 6 Apr 2021 07:15:10 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1617693310; bh=GbXpOGnGfr9ztDbQixGCrSjKEMrp4iZw9IFRsXOcBO8=; h=References:In-Reply-To:From:Date:Subject:To:Cc:From; b=sGUIJ4+ya9r/8732+OAZBHBgNonPDRW/eLgpDis7Yk539bFxTZvggdyGKOA9N5/XE 8ePHCjzOv66g9qTLtvEZfy6mRTIf5Yl4Qp/anQisKALxyv+hU3SOwfUkGudw3F7QDR dyKEva2Hlhb6FMGD5g1Md4A8MGAVi8O/K4y/OxoM6sA3EFKvJEMRTKbrMXD5SBrlXB Ya9oP/V6tIwNDYDUVLrZ83mOS7r0km7RRAXGMM1b3IQZrJTap0b0Im+W5Y0sPr/WHf TnlaZPQKAYl0MQZS2xrDyNTD7XtzMonij6ivt5TpO2MKSaEal1MR1PR8Vk1DvVJuqK 7T27EkTGJtWoA== Received: by mail-oi1-f175.google.com with SMTP id k25so14115397oic.4 for ; Tue, 06 Apr 2021 00:15:10 -0700 (PDT) X-Gm-Message-State: AOAM530+KXQJcjDZvWagAqz3953YhnkaBZ9EVeIftbg+q2HfjxNdum4D 8Bs5zYteQFzkd9yeN4FtPwsimUAihIBEUNzhCwM= X-Google-Smtp-Source: ABdhPJxXQIljWYAgZOKG5+MWuqas6RWrOYkr+agqT1kTdOSeZatMhDuOreXUt9u5Muj8VFP505JJPWyD2H/xNGhQN6c= X-Received: by 2002:aca:5945:: with SMTP id n66mr2221428oib.11.1617693310006; Tue, 06 Apr 2021 00:15:10 -0700 (PDT) MIME-Version: 1.0 References: <20210304213902.83903-1-marcan@marcan.st> <20210304213902.83903-28-marcan@marcan.st> In-Reply-To: From: Arnd Bergmann Date: Tue, 6 Apr 2021 09:14:53 +0200 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [RFT PATCH v3 27/27] arm64: apple: Add initial Apple Mac mini (M1, 2020) devicetree To: Hector Martin Cc: Konrad Dybcio , Linux ARM , Krzysztof Kozlowski , Rob Herring X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210406_081514_261054_0AC5B70B X-CRM114-Status: GOOD ( 28.57 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Mon, Apr 5, 2021 at 7:50 AM Hector Martin wrote: > On 14/03/2021 05.22, Konrad Dybcio wrote: > > [1] > https://lore.kernel.org/linux-arm-kernel/20210216073120.qmlaky43t6uelqc4@kozik-lap/ > > >> +/* > >> + * Apple Mac mini (M1, 2020) > > > > Not sure if it's necessary, you already state it in model="" > > I find it helpful to have identifying info in the top comment, since > it's easier to locate at a glance than visually searching for the model > property. Since I also add identifiers that Apple uses to be able to > cross reference things here, even if it's somewhat duplicative, I think > it makes sense to keep all of it in the top comment. Agreed > >> + > >> +/dts-v1/; > >> + > >> +#include "t8103.dtsi" > >> + > >> +/ { > >> + compatible = "apple,j274", "apple,t8103", "apple,arm-platform"; > >> + model = "Apple Mac mini (M1, 2020)"; > >> + > >> + aliases { > >> + serial0 = &serial0; > > > > Isn't this M1-common? > > It's common to all existing M1 devices, but in principle there is > nothing that says that the stdout path has to be the first UART on any > given device (there is more than one UART, we just aren't declaring the > others yet in this series). This is a common pattern, see e.g. > nvidia/tegra210-smaug.dts. Also, I normally ask for the aliases, chosen and memory nodes to be part of the .dts file since these are settings that may be set by the bootloader and are not specific to the SoCs at all. > >> + compatible = "apple,t8103", "apple,arm-platform"; > > > > Please remove this line, it's getting overwritten anyway. > > I think it's helpful to document the expected compatible subset to be > inherited by board files. The Tegra example I linked above does this too. I'm fine with it either way here, your reasoning makes sense, but I personally don't ask for these to be added or removed. > > Other than this, node order seems to be entirely random. Please sort them > > by address (where applicable) and by name where not, so that this doesn't > > become a huge mess as we go forward. > > Outside of the soc bus I think the order we have makes sense: CPUs > first, timer (which is part of the CPU complex) next, then fixed clocks. > There are only a few things that are going to go in here, so I think an > ad-hoc order like this is okay. > > Inside the bus we only have two nodes so far, and they are indeed in the > wrong order :-). I'll sort them by address. Almost everything else is > going to go in here in the future, so that should keep things sane. Sounds good. Ordering by address is a good default, in particular for the SoC node, but other orders can make sense, too. We used to have some SoCs that sorted everything alphabetically, but that is less common now. The main reason for having a fixed sort order is to help with merge conflicts if we ever get nodes added through different git trees. If both sides add the same node in the same place, that lets git cleanly resolve the merge, while adding the same node in different places duplicates it, and adding different versions in the same place should cause a merge conflict. Arnd _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel