Message ID | 20230413191541.1073027-4-ahalaney@redhat.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:b0ea:0:b0:3b6:4342:cba0 with SMTP id b10csp1269772vqo; Thu, 13 Apr 2023 12:26:21 -0700 (PDT) X-Google-Smtp-Source: AKy350aBA6yL3mtI5eV3uSCN3aMrS57zb8xtVLVpECEvNI3s7heFDAqbaTr+IaoWy3Mf4ZVm8n43 X-Received: by 2002:a05:6a00:2313:b0:639:c88b:c3e0 with SMTP id h19-20020a056a00231300b00639c88bc3e0mr4685714pfh.22.1681413980819; Thu, 13 Apr 2023 12:26:20 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1681413980; cv=none; d=google.com; s=arc-20160816; b=GnhccsOJmrZS0hUp6XvJN+hzvdjhU3hAUnTMtYz0wG97oWXihRRdY6De3K/jhIutVR ShMHA4y1aJ1L5zYPfnpScsGXbWGXMklzgYNN4kJ57wjBe2UI26g1DVTcH53YKXQr3WBY Av96HbRE7TyUzctjqwTPTAdVjsoSq1lV8t6EX67DIN0+a81JsjQZ3Jcq/nk3MSRA972p p2uDiP/c4+byxe+xGkzyDmbCRcPeVcwhvBj8lTY+qRd+lCqLsj2xZCP6OM1zo2jX9Xne QSVWCcgVslgwgVPAWc5ij0HzV2jxrOegMNPJNacHyYWa+BKkQxbn5YEkktvliNkWyKK+ E23A== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :references:in-reply-to:message-id:date:subject:cc:to:from :dkim-signature; bh=xTgGczCO0mNwpgaEE0ax8YC4P9ZUQjVz+mrfnTw0NtY=; b=TNwjxB6GxO6Fx/bcb2i6epfsmWs3JfcbtxP1tSKJEwhL1WjTGEmS4ZrBNGvcIzHLX0 DiS2BnBlqsRh/EFqSFiM/S9+1uosoNzdJf6yBXnnMwhfpndjdRLfBhtwBL4rwPQ3gawC fALHMOX7B6y2Cvdb2wfc+QJ9q6K1YfdmDqC7fr3LuyOMzRfkqs2vhaCM4hmK6svPD4oO Zq3drjcwDpxXJ9FVnjjwUeu0Z3/fo7IcV5ZcWFFxhhhfOjfz/2Cqn6Ioo6//j2hbf+9+ 6vlfIZ7LaWS8cVzfciIxzIpBx3d6ugLVVRaPH2Q2tKcQ5+Bo1PeJ522rkN9F+cKeIbTQ WrTg== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=MEwbCuGN; 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; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=redhat.com Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id h10-20020a63df4a000000b0050b3083b64fsi2631352pgj.429.2023.04.13.12.26.05; Thu, 13 Apr 2023 12:26:20 -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; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=MEwbCuGN; 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; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=redhat.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229804AbjDMTRb (ORCPT <rfc822;peter110.wang@gmail.com> + 99 others); Thu, 13 Apr 2023 15:17:31 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:55726 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229869AbjDMTRV (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Thu, 13 Apr 2023 15:17:21 -0400 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 445E28697 for <linux-kernel@vger.kernel.org>; Thu, 13 Apr 2023 12:15:58 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1681413357; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=xTgGczCO0mNwpgaEE0ax8YC4P9ZUQjVz+mrfnTw0NtY=; b=MEwbCuGNArhj0FNwILgZwzUlzH+bo+ARNyjyFV5nShFo6/ZgQLHoC37OBFjgCoIEgISc+a xZ3+Q4KPM8ReMqbN6g7KSowKX6Dgh+VRjlU9oUTHYaOOGlXeFLyAeAGVf+1/peuMI8Vmsq s4u0VyHnMb/wwwOtnOyly+sy1RK7C5U= Received: from mail-yw1-f199.google.com (mail-yw1-f199.google.com [209.85.128.199]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-182-oFwq-ayTNeOnlutu9_IdBA-1; Thu, 13 Apr 2023 15:15:56 -0400 X-MC-Unique: oFwq-ayTNeOnlutu9_IdBA-1 Received: by mail-yw1-f199.google.com with SMTP id 00721157ae682-54fde069e4aso11362367b3.3 for <linux-kernel@vger.kernel.org>; Thu, 13 Apr 2023 12:15:56 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1681413355; x=1684005355; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=xTgGczCO0mNwpgaEE0ax8YC4P9ZUQjVz+mrfnTw0NtY=; b=dJTDhwpD7lz0S2EJ7ds0ltET9Avgjw+hsi4sej9Ql3pu11HlMlH1xUhzdqxralhzYh 0P/ZScicXNYjpEXnY7xtY4RiURLrhaGaW7MkV2zKnIb/lidIKfBZ2qRMJ8S0faujs6H/ rmDnIFsIavBYFnWVkQMPGcfu8qjEIqb+LvTDS7bU4HmCe8OqSjYkyydkBbUdjeVgcnGz 35S3iE03uKVeSptTajghJi6KMctA4rOnb1Dlfn46bLqJebc7I2gi/e4IUxzb9C707+af wBXZsGqBHA61iKnrndKPjZ7kqiCTxyrZTIjZohne07QSNtHbOymZuDSSJo/ZXun+atKR A9iQ== X-Gm-Message-State: AAQBX9e02aOhzWy9e9GuC/HCuZrmqjVhj1+shtMn8xU/OKSu2MArpTUQ JsLbVK+dJ20D4Wwgc3q6CKpX46vHhTd/1Am9XJETUeRIuXmkiZejmtS0P81AWnJYjRPzRvTDdJY vBZBM0sg+Gumh3Sy5iVL9NlNrebHTQh06c3V4JcQ69g60vNnxW11T+LTmVWjf1ImJyT8a95cIob J+h+ltCslM X-Received: by 2002:a81:4810:0:b0:536:3451:b2ab with SMTP id v16-20020a814810000000b005363451b2abmr2997382ywa.51.1681413355476; Thu, 13 Apr 2023 12:15:55 -0700 (PDT) X-Received: by 2002:a81:4810:0:b0:536:3451:b2ab with SMTP id v16-20020a814810000000b005363451b2abmr2997349ywa.51.1681413355120; Thu, 13 Apr 2023 12:15:55 -0700 (PDT) Received: from halaney-x13s.redhat.com (104-53-165-62.lightspeed.stlsmo.sbcglobal.net. [104.53.165.62]) by smtp.gmail.com with ESMTPSA id t11-20020a81780b000000b00545a4ec318dsm673203ywc.13.2023.04.13.12.15.54 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 13 Apr 2023 12:15:54 -0700 (PDT) From: Andrew Halaney <ahalaney@redhat.com> To: linux-kernel@vger.kernel.org Cc: agross@kernel.org, andersson@kernel.org, konrad.dybcio@linaro.org, robh+dt@kernel.org, krzysztof.kozlowski+dt@linaro.org, mturquette@baylibre.com, sboyd@kernel.org, richardcochran@gmail.com, linux-arm-msm@vger.kernel.org, devicetree@vger.kernel.org, linux-clk@vger.kernel.org, netdev@vger.kernel.org, bmasney@redhat.com, echanude@redhat.com, ncai@quicinc.com, jsuraj@qti.qualcomm.com, hisunil@quicinc.com, Andrew Halaney <ahalaney@redhat.com> Subject: [PATCH v5 3/3] arm64: dts: qcom: sa8540p-ride: Add ethernet nodes Date: Thu, 13 Apr 2023 14:15:41 -0500 Message-Id: <20230413191541.1073027-4-ahalaney@redhat.com> X-Mailer: git-send-email 2.39.2 In-Reply-To: <20230413191541.1073027-1-ahalaney@redhat.com> References: <20230413191541.1073027-1-ahalaney@redhat.com> MIME-Version: 1.0 Content-type: text/plain Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_NONE 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: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1763090346431546426?= X-GMAIL-MSGID: =?utf-8?q?1763090346431546426?= |
Series |
Add EMAC3 support for sa8540p-ride (devicetree/clk bits)
|
|
Commit Message
Andrew Halaney
April 13, 2023, 7:15 p.m. UTC
Enable both the MACs found on the board. ethernet0 and ethernet1 both ultimately go to a series of on board switches which aren't managed by this processor. ethernet0 is connected to a Marvell 88EA1512 phy via RGMII. That goes to the series of switches via SGMII on the "media" side of the phy. RGMII_SGMII mode is enabled via devicetree register descriptions. The switch on the "media" side has auto-negotiation disabled, so configuration from userspace similar to: ethtool -s eth0 autoneg off speed 1000 duplex full is necessary to get traffic flowing on that interface. ethernet1 is in a mac2mac/fixed-link configuration going to the same series of switches directly via RGMII. Tested-by: Brian Masney <bmasney@redhat.com> Signed-off-by: Andrew Halaney <ahalaney@redhat.com> --- This patch is why there's a v5, I couldn't ignore the needless interrupts-parent Konrad pointed out in the sa8155-adp.dts over at: https://lore.kernel.org/linux-arm-msm/88d41729-86be-95cb-2fda-1b809f07ed6b@linaro.org/ Changes since v4: * Remove needless interrupt-parent (Konrad) * Add Tested-by (Brian) Changes since v3: * Compatible goes first in node (Krzysztof) Changes since v1 and v2: * None arch/arm64/boot/dts/qcom/sa8540p-ride.dts | 179 ++++++++++++++++++++++ 1 file changed, 179 insertions(+)
Comments
Quoting Andrew Halaney (2023-04-13 12:15:41) > arch/arm64/boot/dts/qcom/sa8540p-ride.dts | 179 ++++++++++++++++++++++ > 1 file changed, 179 insertions(+) > > diff --git a/arch/arm64/boot/dts/qcom/sa8540p-ride.dts b/arch/arm64/boot/dts/qcom/sa8540p-ride.dts > index 40db5aa0803c..650cd54f418e 100644 > --- a/arch/arm64/boot/dts/qcom/sa8540p-ride.dts > +++ b/arch/arm64/boot/dts/qcom/sa8540p-ride.dts > @@ -28,6 +28,65 @@ aliases { > chosen { > stdout-path = "serial0:115200n8"; > }; > + > + mtl_rx_setup: rx-queues-config { Is there a reason why this isn't a child of an ethernet node? > + snps,rx-queues-to-use = <1>; > + snps,rx-sched-sp; > +
On Thu, Apr 13, 2023 at 01:47:19PM -0700, Stephen Boyd wrote: > Quoting Andrew Halaney (2023-04-13 12:15:41) > > arch/arm64/boot/dts/qcom/sa8540p-ride.dts | 179 ++++++++++++++++++++++ > > 1 file changed, 179 insertions(+) > > > > diff --git a/arch/arm64/boot/dts/qcom/sa8540p-ride.dts b/arch/arm64/boot/dts/qcom/sa8540p-ride.dts > > index 40db5aa0803c..650cd54f418e 100644 > > --- a/arch/arm64/boot/dts/qcom/sa8540p-ride.dts > > +++ b/arch/arm64/boot/dts/qcom/sa8540p-ride.dts > > @@ -28,6 +28,65 @@ aliases { > > chosen { > > stdout-path = "serial0:115200n8"; > > }; > > + > > + mtl_rx_setup: rx-queues-config { > > Is there a reason why this isn't a child of an ethernet node? > > I debated if it was more appropriate to: 1. make a duplicate in each ethernet node (ethernet0/1) 2. Put it in one and reference from both 3. have it floating around independent like this, similar to what is done in sa8155p-adp.dts[0] I chose 3 as it seemed cleanest, but if there's a good argument for a different approach I'm all ears! [0] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/qcom/sa8155p-adp.dts?id=de4664485abbc0529b1eec44d0061bbfe58a28fb#n50 Thanks, Andrew
Quoting Andrew Halaney (2023-04-13 14:01:27) > On Thu, Apr 13, 2023 at 01:47:19PM -0700, Stephen Boyd wrote: > > Quoting Andrew Halaney (2023-04-13 12:15:41) > > > arch/arm64/boot/dts/qcom/sa8540p-ride.dts | 179 ++++++++++++++++++++++ > > > 1 file changed, 179 insertions(+) > > > > > > diff --git a/arch/arm64/boot/dts/qcom/sa8540p-ride.dts b/arch/arm64/boot/dts/qcom/sa8540p-ride.dts > > > index 40db5aa0803c..650cd54f418e 100644 > > > --- a/arch/arm64/boot/dts/qcom/sa8540p-ride.dts > > > +++ b/arch/arm64/boot/dts/qcom/sa8540p-ride.dts > > > @@ -28,6 +28,65 @@ aliases { > > > chosen { > > > stdout-path = "serial0:115200n8"; > > > }; > > > + > > > + mtl_rx_setup: rx-queues-config { > > > > Is there a reason why this isn't a child of an ethernet node? > > > > > > I debated if it was more appropriate to: > > 1. make a duplicate in each ethernet node (ethernet0/1) > 2. Put it in one and reference from both > 3. have it floating around independent like this, similar to what is > done in sa8155p-adp.dts[0] > > I chose 3 as it seemed cleanest, but if there's a good argument for a > different approach I'm all ears! I wonder if it allows the binding checker to catch bad properties by having it under the ethernet node? That's the only thing I can think of that may be improved, but I'll let binding reviewers comment here.
On Thu, Apr 13, 2023 at 03:05:26PM -0700, Stephen Boyd wrote: > Quoting Andrew Halaney (2023-04-13 14:01:27) > > On Thu, Apr 13, 2023 at 01:47:19PM -0700, Stephen Boyd wrote: > > > Quoting Andrew Halaney (2023-04-13 12:15:41) > > > > arch/arm64/boot/dts/qcom/sa8540p-ride.dts | 179 ++++++++++++++++++++++ > > > > 1 file changed, 179 insertions(+) > > > > > > > > diff --git a/arch/arm64/boot/dts/qcom/sa8540p-ride.dts b/arch/arm64/boot/dts/qcom/sa8540p-ride.dts > > > > index 40db5aa0803c..650cd54f418e 100644 > > > > --- a/arch/arm64/boot/dts/qcom/sa8540p-ride.dts > > > > +++ b/arch/arm64/boot/dts/qcom/sa8540p-ride.dts > > > > @@ -28,6 +28,65 @@ aliases { > > > > chosen { > > > > stdout-path = "serial0:115200n8"; > > > > }; > > > > + > > > > + mtl_rx_setup: rx-queues-config { > > > > > > Is there a reason why this isn't a child of an ethernet node? > > > > > > > > > > I debated if it was more appropriate to: > > > > 1. make a duplicate in each ethernet node (ethernet0/1) > > 2. Put it in one and reference from both > > 3. have it floating around independent like this, similar to what is > > done in sa8155p-adp.dts[0] > > > > I chose 3 as it seemed cleanest, but if there's a good argument for a > > different approach I'm all ears! > > I wonder if it allows the binding checker to catch bad properties by > having it under the ethernet node? That's the only thing I can think of > that may be improved, but I'll let binding reviewers comment here. > Thanks, I was curious so I played around to answer the question via testing, and you're right... rx-queues-config/tx-queues-config aren't evaluated unless they sit under the node with the compatible (i.e. it doesn't just follow the phandle and evaluate). That makes sense to me I suppose. So, I guess, would maintainers prefer to see option (1) or (2) above? I want that thing evaluated. Option 1., above, has duplicated configuration, but is probably a more accurate representation of the hardware description. Option 2., above, doesn't duplicate rx-queues-config/tx-queues-config, but is a weirder representation of hardware description, and only complains once (which is fine since it's shared) when the binding checker runs (i.e. only the etherent parent containing rx-queues-config yells). In the below example you can see what I mean by the "only complains once" comment as well as illustration that the patchset as is doesn't allow rx-queues-config/tx-queues-config to be validated by dt-binding checks: (dtschema) ahalaney@halaney-x13s ~/git/redhat/stmmac (git)-[stmmac] % # Purposely introduce a dt-binding error on top of the current patchset :( (dtschema) ahalaney@halaney-x13s ~/git/redhat/stmmac (git)-[stmmac] % git diff :( diff --git a/arch/arm64/boot/dts/qcom/sa8540p-ride.dts b/arch/arm64/boot/dts/qcom/sa8540p-ride.dts index 650cd54f418e..ecb0000db4e2 100644 --- a/arch/arm64/boot/dts/qcom/sa8540p-ride.dts +++ b/arch/arm64/boot/dts/qcom/sa8540p-ride.dts @@ -54,7 +54,7 @@ queue2 { queue3 { snps,avb-algorithm; - snps,map-to-dma-channel = <0x3>; + snps,map-to-dma-channel = "not-correct"; snps,priority = <0xc>; }; }; (dtschema) ahalaney@halaney-x13s ~/git/redhat/stmmac (git)-[stmmac] % make CHECK_DTBS=y DT_SCHEMA_FILES=/net/qcom,ethqos.yaml qcom/sa8540p-ride.dtb DTC_CHK arch/arm64/boot/dts/qcom/sa8540p-ride.dtb (dtschema) ahalaney@halaney-x13s ~/git/redhat/stmmac (git)-[stmmac] % # That should have failed (dtschema) ahalaney@halaney-x13s ~/git/redhat/stmmac (git)-[stmmac] % # Move the whole node under ethernet0, have ethernet1 reference via phandle only still (dtschema) ahalaney@halaney-x13s ~/git/redhat/stmmac (git)-[stmmac] % git diff | cat diff --git a/arch/arm64/boot/dts/qcom/sa8540p-ride.dts b/arch/arm64/boot/dts/qcom/sa8540p-ride.dts index 650cd54f418e..451246936731 100644 --- a/arch/arm64/boot/dts/qcom/sa8540p-ride.dts +++ b/arch/arm64/boot/dts/qcom/sa8540p-ride.dts @@ -29,35 +29,6 @@ chosen { stdout-path = "serial0:115200n8"; }; - mtl_rx_setup: rx-queues-config { - snps,rx-queues-to-use = <1>; - snps,rx-sched-sp; - - queue0 { - snps,dcb-algorithm; - snps,map-to-dma-channel = <0x0>; - snps,route-up; - snps,priority = <0x1>; - }; - - queue1 { - snps,dcb-algorithm; - snps,map-to-dma-channel = <0x1>; - snps,route-ptp; - }; - - queue2 { - snps,avb-algorithm; - snps,map-to-dma-channel = <0x2>; - snps,route-avcp; - }; - - queue3 { - snps,avb-algorithm; - snps,map-to-dma-channel = <0x3>; - snps,priority = <0xc>; - }; - }; mtl_tx_setup: tx-queues-config { snps,tx-queues-to-use = <1>; @@ -223,6 +194,36 @@ ðernet0 { status = "okay"; + mtl_rx_setup: rx-queues-config { + snps,rx-queues-to-use = <1>; + snps,rx-sched-sp; + + queue0 { + snps,dcb-algorithm; + snps,map-to-dma-channel = <0x0>; + snps,route-up; + snps,priority = <0x1>; + }; + + queue1 { + snps,dcb-algorithm; + snps,map-to-dma-channel = <0x1>; + snps,route-ptp; + }; + + queue2 { + snps,avb-algorithm; + snps,map-to-dma-channel = <0x2>; + snps,route-avcp; + }; + + queue3 { + snps,avb-algorithm; + snps,map-to-dma-channel = "not-correct"; + snps,priority = <0xc>; + }; + }; + mdio { compatible = "snps,dwmac-mdio"; #address-cells = <1>; (dtschema) ahalaney@halaney-x13s ~/git/redhat/stmmac (git)-[stmmac] % make CHECK_DTBS=y DT_SCHEMA_FILES=/net/qcom,ethqos.yaml qcom/sa8540p-ride.dtb DTC_CHK arch/arm64/boot/dts/qcom/sa8540p-ride.dtb /home/ahalaney/git/redhat/stmmac/arch/arm64/boot/dts/qcom/sa8540p-ride.dtb: ethernet@20000: rx-queues-config:queue3:snps,map-to-dma-channel:0: [1852797997, 1668248178, 1701016576] is too long From schema: /home/ahalaney/git/redhat/stmmac/Documentation/devicetree/bindings/net/qcom,ethqos.yaml /home/ahalaney/git/redhat/stmmac/arch/arm64/boot/dts/qcom/sa8540p-ride.dtb: ethernet@20000: Unevaluated properties are not allowed ('max-speed', 'mdio', 'phy-handle', 'phy-mode', 'power-domains', 'rx-fifo-depth', 'rx-queues-config', 'snps,mtl-rx-config', 'snps,mtl-tx-config', 'snps,pbl', 'snps,tso', 'tx-fifo-depth' were unexpected) From schema: /home/ahalaney/git/redhat/stmmac/Documentation/devicetree/bindings/net/qcom,ethqos.yaml (dtschema) ahalaney@halaney-x13s ~/git/redhat/stmmac (git)-[stmmac] % # That warned as expected, since snps,dwmac.yaml failed on rx-queues-config it warns, \ and since part of the schema failed its not inherited (hence the Unevaluated properties warning following) \ also note how only ethernet0 (@20000) is evaluating rx-queues-config since that's where the rx-queues-config node lives (dtschema) ahalaney@halaney-x13s ~/git/redhat/stmmac (git)-[stmmac] % Thanks for the review! - Andrew
Hey Krzysztof, Some advice below would be appreciated. I discussed offline with Bjorn between the two approaches below but it sounded like ultimately we'd defer to your preference here! On Fri, Apr 14, 2023 at 09:58:44AM -0500, Andrew Halaney wrote: > On Thu, Apr 13, 2023 at 03:05:26PM -0700, Stephen Boyd wrote: > > Quoting Andrew Halaney (2023-04-13 14:01:27) > > > On Thu, Apr 13, 2023 at 01:47:19PM -0700, Stephen Boyd wrote: > > > > Quoting Andrew Halaney (2023-04-13 12:15:41) > > > > > arch/arm64/boot/dts/qcom/sa8540p-ride.dts | 179 ++++++++++++++++++++++ > > > > > 1 file changed, 179 insertions(+) > > > > > > > > > > diff --git a/arch/arm64/boot/dts/qcom/sa8540p-ride.dts b/arch/arm64/boot/dts/qcom/sa8540p-ride.dts > > > > > index 40db5aa0803c..650cd54f418e 100644 > > > > > --- a/arch/arm64/boot/dts/qcom/sa8540p-ride.dts > > > > > +++ b/arch/arm64/boot/dts/qcom/sa8540p-ride.dts > > > > > @@ -28,6 +28,65 @@ aliases { > > > > > chosen { > > > > > stdout-path = "serial0:115200n8"; > > > > > }; > > > > > + > > > > > + mtl_rx_setup: rx-queues-config { > > > > > > > > Is there a reason why this isn't a child of an ethernet node? > > > > > > > > > > > > > > I debated if it was more appropriate to: > > > > > > 1. make a duplicate in each ethernet node (ethernet0/1) > > > 2. Put it in one and reference from both > > > 3. have it floating around independent like this, similar to what is > > > done in sa8155p-adp.dts[0] > > > > > > I chose 3 as it seemed cleanest, but if there's a good argument for a > > > different approach I'm all ears! > > > > I wonder if it allows the binding checker to catch bad properties by > > having it under the ethernet node? That's the only thing I can think of > > that may be improved, but I'll let binding reviewers comment here. > > > > Thanks, I was curious so I played around to answer the question via > testing, and you're right... rx-queues-config/tx-queues-config aren't > evaluated unless they sit under the node with the compatible (i.e. it > doesn't just follow the phandle and evaluate). That makes sense to me I > suppose. > > So, I guess, would maintainers prefer to see option (1) or (2) above? I > want that thing evaluated. > > Option 1., above, has duplicated configuration, but is probably a more accurate > representation of the hardware description. > > Option 2., above, doesn't duplicate rx-queues-config/tx-queues-config, > but is a weirder representation of hardware description, and only > complains once (which is fine since it's shared) when the binding checker > runs (i.e. only the etherent parent containing rx-queues-config yells). > For what it is worth, I prefer option 1 above :) > In the below example you can see what I mean by the "only complains > once" comment as well as illustration that the patchset as is doesn't > allow rx-queues-config/tx-queues-config to be validated by dt-binding > checks: > > (dtschema) ahalaney@halaney-x13s ~/git/redhat/stmmac (git)-[stmmac] % # Purposely introduce a dt-binding error on top of the current patchset :( > (dtschema) ahalaney@halaney-x13s ~/git/redhat/stmmac (git)-[stmmac] % git diff :( > diff --git a/arch/arm64/boot/dts/qcom/sa8540p-ride.dts b/arch/arm64/boot/dts/qcom/sa8540p-ride.dts > index 650cd54f418e..ecb0000db4e2 100644 > --- a/arch/arm64/boot/dts/qcom/sa8540p-ride.dts > +++ b/arch/arm64/boot/dts/qcom/sa8540p-ride.dts > @@ -54,7 +54,7 @@ queue2 { > > queue3 { > snps,avb-algorithm; > - snps,map-to-dma-channel = <0x3>; > + snps,map-to-dma-channel = "not-correct"; > snps,priority = <0xc>; > }; > }; > (dtschema) ahalaney@halaney-x13s ~/git/redhat/stmmac (git)-[stmmac] % make CHECK_DTBS=y DT_SCHEMA_FILES=/net/qcom,ethqos.yaml qcom/sa8540p-ride.dtb > DTC_CHK arch/arm64/boot/dts/qcom/sa8540p-ride.dtb > (dtschema) ahalaney@halaney-x13s ~/git/redhat/stmmac (git)-[stmmac] % # That should have failed > (dtschema) ahalaney@halaney-x13s ~/git/redhat/stmmac (git)-[stmmac] % # Move the whole node under ethernet0, have ethernet1 reference via phandle only still > (dtschema) ahalaney@halaney-x13s ~/git/redhat/stmmac (git)-[stmmac] % git diff | cat > diff --git a/arch/arm64/boot/dts/qcom/sa8540p-ride.dts b/arch/arm64/boot/dts/qcom/sa8540p-ride.dts > index 650cd54f418e..451246936731 100644 > --- a/arch/arm64/boot/dts/qcom/sa8540p-ride.dts > +++ b/arch/arm64/boot/dts/qcom/sa8540p-ride.dts > @@ -29,35 +29,6 @@ chosen { > stdout-path = "serial0:115200n8"; > }; > > - mtl_rx_setup: rx-queues-config { > - snps,rx-queues-to-use = <1>; > - snps,rx-sched-sp; > - > - queue0 { > - snps,dcb-algorithm; > - snps,map-to-dma-channel = <0x0>; > - snps,route-up; > - snps,priority = <0x1>; > - }; > - > - queue1 { > - snps,dcb-algorithm; > - snps,map-to-dma-channel = <0x1>; > - snps,route-ptp; > - }; > - > - queue2 { > - snps,avb-algorithm; > - snps,map-to-dma-channel = <0x2>; > - snps,route-avcp; > - }; > - > - queue3 { > - snps,avb-algorithm; > - snps,map-to-dma-channel = <0x3>; > - snps,priority = <0xc>; > - }; > - }; > > mtl_tx_setup: tx-queues-config { > snps,tx-queues-to-use = <1>; > @@ -223,6 +194,36 @@ ðernet0 { > > status = "okay"; > > + mtl_rx_setup: rx-queues-config { > + snps,rx-queues-to-use = <1>; > + snps,rx-sched-sp; > + > + queue0 { > + snps,dcb-algorithm; > + snps,map-to-dma-channel = <0x0>; > + snps,route-up; > + snps,priority = <0x1>; > + }; > + > + queue1 { > + snps,dcb-algorithm; > + snps,map-to-dma-channel = <0x1>; > + snps,route-ptp; > + }; > + > + queue2 { > + snps,avb-algorithm; > + snps,map-to-dma-channel = <0x2>; > + snps,route-avcp; > + }; > + > + queue3 { > + snps,avb-algorithm; > + snps,map-to-dma-channel = "not-correct"; > + snps,priority = <0xc>; > + }; > + }; > + > mdio { > compatible = "snps,dwmac-mdio"; > #address-cells = <1>; > (dtschema) ahalaney@halaney-x13s ~/git/redhat/stmmac (git)-[stmmac] % make CHECK_DTBS=y DT_SCHEMA_FILES=/net/qcom,ethqos.yaml qcom/sa8540p-ride.dtb > DTC_CHK arch/arm64/boot/dts/qcom/sa8540p-ride.dtb > /home/ahalaney/git/redhat/stmmac/arch/arm64/boot/dts/qcom/sa8540p-ride.dtb: ethernet@20000: rx-queues-config:queue3:snps,map-to-dma-channel:0: [1852797997, 1668248178, 1701016576] is too long > From schema: /home/ahalaney/git/redhat/stmmac/Documentation/devicetree/bindings/net/qcom,ethqos.yaml > /home/ahalaney/git/redhat/stmmac/arch/arm64/boot/dts/qcom/sa8540p-ride.dtb: ethernet@20000: Unevaluated properties are not allowed ('max-speed', 'mdio', 'phy-handle', 'phy-mode', 'power-domains', 'rx-fifo-depth', 'rx-queues-config', 'snps,mtl-rx-config', 'snps,mtl-tx-config', 'snps,pbl', 'snps,tso', 'tx-fifo-depth' were unexpected) > From schema: /home/ahalaney/git/redhat/stmmac/Documentation/devicetree/bindings/net/qcom,ethqos.yaml > (dtschema) ahalaney@halaney-x13s ~/git/redhat/stmmac (git)-[stmmac] % # That warned as expected, since snps,dwmac.yaml failed on rx-queues-config it warns, \ > and since part of the schema failed its not inherited (hence the Unevaluated properties warning following) \ > also note how only ethernet0 (@20000) is evaluating rx-queues-config since that's where the rx-queues-config node lives > (dtschema) ahalaney@halaney-x13s ~/git/redhat/stmmac (git)-[stmmac] % > > Thanks for the review! > - Andrew
diff --git a/arch/arm64/boot/dts/qcom/sa8540p-ride.dts b/arch/arm64/boot/dts/qcom/sa8540p-ride.dts index 40db5aa0803c..650cd54f418e 100644 --- a/arch/arm64/boot/dts/qcom/sa8540p-ride.dts +++ b/arch/arm64/boot/dts/qcom/sa8540p-ride.dts @@ -28,6 +28,65 @@ aliases { chosen { stdout-path = "serial0:115200n8"; }; + + mtl_rx_setup: rx-queues-config { + snps,rx-queues-to-use = <1>; + snps,rx-sched-sp; + + queue0 { + snps,dcb-algorithm; + snps,map-to-dma-channel = <0x0>; + snps,route-up; + snps,priority = <0x1>; + }; + + queue1 { + snps,dcb-algorithm; + snps,map-to-dma-channel = <0x1>; + snps,route-ptp; + }; + + queue2 { + snps,avb-algorithm; + snps,map-to-dma-channel = <0x2>; + snps,route-avcp; + }; + + queue3 { + snps,avb-algorithm; + snps,map-to-dma-channel = <0x3>; + snps,priority = <0xc>; + }; + }; + + mtl_tx_setup: tx-queues-config { + snps,tx-queues-to-use = <1>; + snps,tx-sched-sp; + + queue0 { + snps,dcb-algorithm; + }; + + queue1 { + snps,dcb-algorithm; + }; + + queue2 { + snps,avb-algorithm; + snps,send_slope = <0x1000>; + snps,idle_slope = <0x1000>; + snps,high_credit = <0x3e800>; + snps,low_credit = <0xffc18000>; + }; + + queue3 { + snps,avb-algorithm; + snps,send_slope = <0x1000>; + snps,idle_slope = <0x1000>; + snps,high_credit = <0x3e800>; + snps,low_credit = <0xffc18000>; + }; + }; }; &apps_rsc { @@ -151,6 +210,66 @@ vreg_l8g: ldo8 { }; }; +ðernet0 { + snps,mtl-rx-config = <&mtl_rx_setup>; + snps,mtl-tx-config = <&mtl_tx_setup>; + + max-speed = <1000>; + phy-handle = <&rgmii_phy>; + phy-mode = "rgmii-txid"; + + pinctrl-names = "default"; + pinctrl-0 = <ðernet0_default>; + + status = "okay"; + + mdio { + compatible = "snps,dwmac-mdio"; + #address-cells = <1>; + #size-cells = <0>; + + /* Marvell 88EA1512 */ + rgmii_phy: phy@8 { + reg = <0x8>; + + interrupts-extended = <&tlmm 127 IRQ_TYPE_EDGE_FALLING>; + + reset-gpios = <&pmm8540c_gpios 1 GPIO_ACTIVE_LOW>; + reset-assert-us = <11000>; + reset-deassert-us = <70000>; + + device_type = "ethernet-phy"; + + /* Set to RGMII_SGMII mode and soft reset. Turn off auto-negotiation + * from userspace to talk to the switch on the SGMII side of things + */ + marvell,reg-init = + /* Set MODE[2:0] to RGMII_SGMII */ + <0x12 0x14 0xfff8 0x4>, + /* Soft reset required after changing MODE[2:0] */ + <0x12 0x14 0x7fff 0x8000>; + }; + }; +}; + +ðernet1 { + snps,mtl-rx-config = <&mtl_rx_setup>; + snps,mtl-tx-config = <&mtl_tx_setup>; + + max-speed = <1000>; + phy-mode = "rgmii-txid"; + + pinctrl-names = "default"; + pinctrl-0 = <ðernet1_default>; + + status = "okay"; + + fixed-link { + speed = <1000>; + full-duplex; + }; +}; + &i2c0 { pinctrl-names = "default"; pinctrl-0 = <&i2c0_default>; @@ -316,6 +435,66 @@ &xo_board_clk { /* PINCTRL */ &tlmm { + ethernet0_default: ethernet0-default-state { + mdc-pins { + pins = "gpio175"; + function = "rgmii_0"; + drive-strength = <16>; + bias-pull-up; + }; + + mdio-pins { + pins = "gpio176"; + function = "rgmii_0"; + drive-strength = <16>; + bias-pull-up; + }; + + rgmii-tx-pins { + pins = "gpio183", "gpio184", "gpio185", "gpio186", "gpio187", "gpio188"; + function = "rgmii_0"; + drive-strength = <16>; + bias-pull-up; + }; + + rgmii-rx-pins { + pins = "gpio177", "gpio178", "gpio179", "gpio180", "gpio181", "gpio182"; + function = "rgmii_0"; + drive-strength = <16>; + bias-disable; + }; + }; + + ethernet1_default: ethernet1-default-state { + mdc-pins { + pins = "gpio97"; + function = "rgmii_1"; + drive-strength = <16>; + bias-pull-up; + }; + + mdio-pins { + pins = "gpio98"; + function = "rgmii_1"; + drive-strength = <16>; + bias-pull-up; + }; + + rgmii-tx-pins { + pins = "gpio105", "gpio106", "gpio107", "gpio108", "gpio109", "gpio110"; + function = "rgmii_1"; + drive-strength = <16>; + bias-pull-up; + }; + + rgmii-rx-pins { + pins = "gpio99", "gpio100", "gpio101", "gpio102", "gpio103", "gpio104"; + function = "rgmii_1"; + drive-strength = <16>; + bias-disable; + }; + }; + i2c0_default: i2c0-default-state { /* To USB7002T-I/KDXVA0 USB hub (SIP1 only) */ pins = "gpio135", "gpio136";