Received: by 2002:a05:6358:16cd:b0:dc:6189:e246 with SMTP id r13csp549283rwl; Fri, 4 Nov 2022 03:40:59 -0700 (PDT) X-Google-Smtp-Source: AMsMyM54M7pfLUtakZcKbFqKh7Zc5spDgNJE3vwAX0Fk3kE81B/cK+/ojlXjdNi7TkOCxad4wYQu X-Received: by 2002:a05:6a00:e1b:b0:537:7c74:c405 with SMTP id bq27-20020a056a000e1b00b005377c74c405mr34887846pfb.43.1667558458972; Fri, 04 Nov 2022 03:40:58 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1667558458; cv=none; d=google.com; s=arc-20160816; b=JLqXdT6rwXJKIHJk/KqZG377sfr88YLlo/WZA2vUVGu8W5K97gJzn++LJq1NS6P6vf ka12Pl2Vz4DCvDun5fSDhOTFLLxeQnmlv8SrO9/q4yhvPTawrMdDNYeXC59FbBY3KAxb HS0T2+ACfoKEzKozchRvhAuOkI1EqhHf3S7l6uFkwxuKOPWHxMo0f8kI04NErCRotgR/ /OIchd86xoLeGudgP9GBB6CQ5JcGU8UL0ehAKBNaQdAgiWz2iB0bYj9wppl2vPtw3Tag BJR2Krs/gkwgwx5Bfq8l46uKLDA2N41dT0egeDnVM6sGkkmsvIikuonkNkzM2QFin4ze 2XIA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:in-reply-to:from :references:cc:to:subject:user-agent:mime-version:date:message-id; bh=GekycuMLPmr1+tcOJR821BL4QGLym3CgoYMZFVibqv8=; b=dr1DQnNDGs5q5+YSzy2gqIFkOQ1nrvUEJtaMlbWtXm1Z3HT8McMekJG91QuFLHTuir o1qCv4A4NWo55iyUM7CirNj/onr/0uYDx5t8nfrb5O0/UDtCEPbqU42AsvN3VNjPJyrX FBAfEIiszsZptOZdfDDjqgSh2jzrs8jw+m8LRC6DR1GYBXAOzBypDwQjreqMjcxY6RHg cfRh4f0zK1lOcVeq4ZoCUHphk5OCtyIuqoPM3dx7UYKErDwiCiggJNqfMXlKD5frMNtO QqtVIHmI3QZoJijiNDb4AKqrLeATTLvtNpEz2KBSsMo3Y+Cz1Kn5GVnIA8bFWmqObKhq jYrA== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Return-Path: Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id q12-20020a170902eb8c00b0018694b85992si3714765plg.389.2022.11.04.03.40.46; Fri, 04 Nov 2022 03:40:58 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231359AbiKDJc2 (ORCPT + 96 others); Fri, 4 Nov 2022 05:32:28 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:50206 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231320AbiKDJc0 (ORCPT ); Fri, 4 Nov 2022 05:32:26 -0400 Received: from relay08.th.seeweb.it (relay08.th.seeweb.it [5.144.164.169]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 8760627933 for ; Fri, 4 Nov 2022 02:32:24 -0700 (PDT) Received: from [192.168.31.208] (unknown [194.29.137.22]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by m-r2.th.seeweb.it (Postfix) with ESMTPSA id 139F9401A0; Fri, 4 Nov 2022 10:32:21 +0100 (CET) Message-ID: <23a0dc6b-b704-c094-96dc-cf2c083ef55e@somainline.org> Date: Fri, 4 Nov 2022 10:32:19 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:102.0) Gecko/20100101 Thunderbird/102.4.1 Subject: Re: [PATCH v3 3/4] arm64: dts: qcom: Add base QDU1000/QRU1000 DTSIs To: Trilok Soni , Melody Olvera , Krzysztof Kozlowski , Andy Gross , Bjorn Andersson , Rob Herring , Krzysztof Kozlowski Cc: linux-arm-msm@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org References: <20221026200429.162212-1-quic_molvera@quicinc.com> <20221026200429.162212-4-quic_molvera@quicinc.com> <9eaaf256-8de2-ddc9-ac95-aed9b0670f5e@linaro.org> <4832b716-6caf-cf72-1c7e-f21a0670cbaa@quicinc.com> <5109d728-ebea-21ca-3ee1-15710dfd6f1b@quicinc.com> From: Konrad Dybcio In-Reply-To: <5109d728-ebea-21ca-3ee1-15710dfd6f1b@quicinc.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-1.9 required=5.0 tests=BAYES_00,NICE_REPLY_A, RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 04/11/2022 05:05, Trilok Soni wrote: > + Adding Konrad, Bjorn is already there in this email > > On 11/3/2022 2:13 PM, Melody Olvera wrote: >> >> >> On 11/2/2022 9:24 AM, Krzysztof Kozlowski wrote: >>> On 31/10/2022 17:49, Melody Olvera wrote: >>>> >>>> On 10/27/2022 8:21 AM, Krzysztof Kozlowski wrote: >>>>> On 26/10/2022 16:04, Melody Olvera wrote: >>>>>> Add the base DTSI files for QDU1000 and QRU1000 SoCs, including base >>>>>> descriptions of CPUs, GCC, RPMHCC, QUP, TLMM, and >>>>>> interrupt-controller >>>>>> to boot to shell with console on these SoCs. >>>>>> >>>>>> Signed-off-by: Melody Olvera >>>>>> --- >>>>>>   arch/arm64/boot/dts/qcom/qdu1000.dtsi | 1406 >>>>>> +++++++++++++++++++++++++ >>>>> Please use scripts/get_maintainers.pl to get a list of necessary >>>>> people >>>>> and lists to CC.  It might happen, that command when run on an older >>>>> kernel, gives you outdated entries.  Therefore please be sure you >>>>> base >>>>> your patches on recent Linux kernel. >>>> Sure thing; we talked about this on a different patch. >>>>>> arch/arm64/boot/dts/qcom/qru1000.dtsi |   27 + >>>>>>   2 files changed, 1433 insertions(+) >>>>>>   create mode 100644 arch/arm64/boot/dts/qcom/qdu1000.dtsi >>>>>>   create mode 100644 arch/arm64/boot/dts/qcom/qru1000.dtsi >>>>>> >>>>>> diff --git a/arch/arm64/boot/dts/qcom/qdu1000.dtsi >>>>>> b/arch/arm64/boot/dts/qcom/qdu1000.dtsi >>>>>> new file mode 100644 >>>>>> index 000000000000..76474106e931 >>>>>> --- /dev/null >>>>>> +++ b/arch/arm64/boot/dts/qcom/qdu1000.dtsi >>>>>> @@ -0,0 +1,1406 @@ >>>>>> +// SPDX-License-Identifier: BSD-3-Clause >>>>>> +/* >>>>>> + * Copyright (c) 2022 Qualcomm Innovation Center, Inc. All >>>>>> rights reserved. >>>>>> + */ >>>>> (...) >>>>> >>>>>> + >>>>>> +    soc: soc@0 { >>>>>> +        #address-cells = <2>; >>>>>> +        #size-cells = <2>; >>>>>> +        ranges = <0 0 0 0 0x10 0>; >>>>>> +        dma-ranges = <0 0 0 0 0x10 0>; >>>>>> +        compatible = "simple-bus"; >>>>>> + >>>>>> +        gcc: clock-controller@80000 { >>>>>> +            compatible = "qcom,gcc-qdu1000", "syscon"; >>>>>> +            reg = <0x0 0x80000 0x0 0x1f4200>; >>>>>> +            #clock-cells = <1>; >>>>>> +            #reset-cells = <1>; >>>>>> +            #power-domain-cells = <1>; >>>>>> +            clocks = <&rpmhcc RPMH_CXO_CLK>, <&sleep_clk>; >>>>>> +            clock-names = "bi_tcxo", "sleep_clk"; >>>>>> +        }; >>>>>> + >>>>>> +        gpi_dma0: dma-controller@900000  { >>>>>> +            compatible = "qcom,sm6350-gpi-dma"; >>>>> You should add here a specific compatible as well. Same in other >>>>> places. >>>>> All places. I had impression we talked about this few times, so I >>>>> don't >>>>> know what is missing on your side. >>>>> >>>>> This must be: >>>>> "qcom,qdu1000-gpi-dma", "qcom,sm6350-gpi-dma" >>>> Got it. I talked to Stephan and he said either your suggestion or >>>> just using >>>> preexisting compatibles would be ok. I thought it might be cleaner >>>> to not >>>> have the qdu compats, but I'm fine either way. We use specific compats so that if it turns out this specific SoC (or rather the bundled firmware) has some peculiar bugs, we can retroactively only apply them to that specific SoC by adding the compatible somewhere in .c code if need be. >>>>>> +            #dma-cells = <3>; >>>>>> +            reg = <0x0 0x900000 0x0 0x60000>; >>>>>> +            interrupts = , >>>>>> +                     , >>>>>> +                     , >>>>>> +                     , >>>>>> +                     , >>>>>> +                     , >>>>>> +                     , >>>>>> +                     , >>>>>> +                     , >>>>>> +                     , >>>>>> +                     , >>>>>> +                     ; >>>>>> +            dma-channels = <12>; >>>>>> +            dma-channel-mask = <0x3f>; >>>>>> +            iommus = <&apps_smmu 0xf6 0x0>; >>>>>> +        }; >>>>>> + >>>>> (...) >>>>> >>>>> >>>>>> + >>>>>> +        tlmm: pinctrl@f000000 { >>>>>> +            compatible = "qcom,qdu1000-tlmm"; >>>>>> +            reg = <0x0 0xf000000 0x0 0x1000000>; >>>>>> +            interrupts = ; >>>>>> +            gpio-controller; >>>>>> +            #gpio-cells = <2>; >>>>>> +            interrupt-controller; >>>>>> +            #interrupt-cells = <2>; >>>>>> +            gpio-ranges = <&tlmm 0 0 151>; >>>>>> +            wakeup-parent = <&pdc>; >>>>>> + >>>>>> +            qup_uart0_default: qup-uart0-default-state { >>>>>> +                pins = "gpio6", "gpio7", "gpio8", "gpio9"; >>>>>> +                function = "qup00"; >>>>>> +            }; >>>>>> + >>>>>> +            qup_i2c1_data_clk: qup-i2c1-data-clk-state { >>>>>> +                pins = "gpio10", "gpio11"; >>>>>> +                function = "qup01"; >>>>>> +                drive-strength = <2>; >>>>> Can we have some generic agreement where to put drive-strengths >>>>> and bias? >>>>> >>>>> See also: >>>>> https://lore.kernel.org/linux-devicetree/20221026200357.391635-2-krzysztof.kozlowski@linaro.org/ >>>>> >>>>> >>>>> https://lore.kernel.org/lkml/CAD=FV=VUL4GmjaibAMhKNdpEso_Hg_R=XeMaqah1LSj_9-Ce4Q@mail.gmail.com/ >>>>> I agree with Doug on having 'generic' drive-strength. Moreover, maybe even adding some property like bias-type = would be cool to make it more flexible so that we could trim off A LOT of repeated lines (remember, most boards are more or less copies of the reference design for a given platform like QRD or CRD) going forward. >>>> Not sure how much two-sense I have for the conversation at large, >>>> but generally I agree with Doug's >>>> point in the first paragraph. Pulls for this soc are consistent >>>> across boards so I don't think it makes >>>> sense to move them to the board files here. I vote that these stay >>>> here. >>> I would be great if Konrad and Bjorn shared their opinion on this... >>> but >>> wait, you did not Cc all maintainers... Eh. >> I'm not sure why this is being brought up again; we've already >> discussed this here >> https://lore.kernel.org/all/9707bf67-1b22-8a77-7193-fc909b4f49de@quicinc.com/ >> >> Would you like to discuss this issue here, on the next version, or >> not at all? >> >> On a side note, I'm uncomfortable with how our continued interactions >> are going >> and do not believe this to be conductive to continued collaboration. >> I would ask that >> we keep our correspondence polite and professional moving forward. > > I have added Konrad and Bjorn is already there on the thread. Our > understanding is that CCing maintainers comment is for next patch > series after this one. BTW: you can feed git send-email with --cc-cmd='./scripts/get_maintainer.pl --norolestats' and it'll pick the right people for you (most of the time, anyway). > > Bjorn, please check and comment on above? If requires we should start > writing the guidelines for MSM boards since lot of comments are based > on the experience or knowledge in the community Vs caught by tools - > so it is easy to be missed by developers submitting new boards. Thoughts? Big yes! Some of the points should probably even be raised wrt the DT spec itself, such as property order. Konrad > > ---Trilok Soni > >