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 Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 6FEBCD3B98D for ; Tue, 9 Dec 2025 17:56:39 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:List-Subscribe:List-Help :List-Post:List-Archive:List-Unsubscribe:List-Id:In-Reply-To: Content-Transfer-Encoding:Content-Type:MIME-Version:References:Message-ID: Subject:Cc:To:Date:From:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=VS+RzLACBnaDYUv6qVFgvyvNQaXuhOHqAm0J3pZTOy4=; b=MVRerfZHIdwUeGdPNd4QI2nEfY SEaxDIKRwwqqBhsyyuKeYEQtkOOYPYuTLI/xubjVwjDeyXt+NW3wJyBpDrdnjxyK/34EkQZCFvGAK mUVE49odnJcbIjiAQD7zPmTOyziko1Zpm1XdyRZj4EzUMGaDVDkl25NlxlBN2bAVXiyHq0B+FbAXR c7fNGhd2lTRjmiMchzwBOfANglKmX0s3mjVAXxNAjYAHW1Na3lMhzQ0Zr7tyVdeOQF0SbyJF5PzCr S3CxF+oW1dqJ8JRMMeMHoV+5yOujF3HfECR63b7/57+2JIHIrrUTRf8xmuDN/o3QQ/m7WCi5qbzvV RQH1LCVg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.98.2 #2 (Red Hat Linux)) id 1vT1wv-0000000Ecfa-48hq; Tue, 09 Dec 2025 17:56:33 +0000 Received: from mail-ej1-x629.google.com ([2a00:1450:4864:20::629]) by bombadil.infradead.org with esmtps (Exim 4.98.2 #2 (Red Hat Linux)) id 1vT1ws-0000000EceV-3mB4 for linux-arm-kernel@lists.infradead.org; Tue, 09 Dec 2025 17:56:32 +0000 Received: by mail-ej1-x629.google.com with SMTP id a640c23a62f3a-b736ffc531fso230658966b.1 for ; Tue, 09 Dec 2025 09:56:30 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.com; s=google; t=1765302989; x=1765907789; darn=lists.infradead.org; h=in-reply-to:content-transfer-encoding:content-disposition :mime-version:references:message-id:subject:cc:to:date:from:from:to :cc:subject:date:message-id:reply-to; bh=VS+RzLACBnaDYUv6qVFgvyvNQaXuhOHqAm0J3pZTOy4=; b=URujgwlbhFqsA+D6mTIyoO7kRpu/MVqOgxlp+TyBQZ3Fqzyr3aPGJYj2dgcuA6IoD5 d4xf2+Hy92Eshm3s9woVKORJz/nm++5Z/j9ADKPt3V1jU5x3CDLLCa/GAiwUey6qW7g0 fdB8C9QgBwEXm0qqknObSB0MEBVRtRkHVD6gl/oPfPz3IlgFZnAwDCUT0XhY4kweekME ubXPZrF+SdhNYmItECDRMvbbkKIkj0epiwlGm8i5QuMt8lY1cKIFWADOyTqxviISCbrl DkybR5DNdh9IlwOjeprKu4aGL98UdYGGhvm0kCgKNkfoLAtU5lHpscFfe4BAbnbrK3Pn RnZw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1765302989; x=1765907789; h=in-reply-to:content-transfer-encoding:content-disposition :mime-version:references:message-id:subject:cc:to:date:from:x-gm-gg :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=VS+RzLACBnaDYUv6qVFgvyvNQaXuhOHqAm0J3pZTOy4=; b=kz/mInqoe5HXpVfsV07eSxp0c7oNLDaoxw0DkNz6Zy2qnzn97f6Q2S+ZPj1OAsxz3E s+UlmxoOADMqWsixNRHvz+poTqY0OJOqlBURjoFxQyTx88fXp2/bDjpg/ZiakT5BsTmJ HDl4Qac+smjaActUkwCbXnGGBCLBkRfAQnntqGoerwHCkOHMx08a/DBv+ggmfj+gri04 JPi7ig9XScyOMXOwWcd2CTSTcxdv+i3I9WrsG7H89+cRNlrNmH+xV7mK6v7AppZnNytB /IxOROSfE4HlUsnD6HX1PedZVcPWPyiiiZEXvJHbJNIMYcXTBIEZcNUkuSydE1O44Kr4 klqA== X-Forwarded-Encrypted: i=1; AJvYcCVRm4mzBWfvY74dSvCoV8zvrldNd/I7EceIB37nv4+CQ5+sgBRLgjISQ4KnL38Lwjs5H+KQWNmvQLGq744oBnFY@lists.infradead.org X-Gm-Message-State: AOJu0YyB9rrKSSn2TwzoRX+CtTcKPkJnJ6Wq0MmdDFqGkK4nc5a67z0s a+L44oaL/pgs2z9V5Z9xiMNbwj+UTefxzL3w5zknU4SO+1dDpfjTh8lWz3m5E5fJDLk= X-Gm-Gg: ASbGncvwm6OwgfTMFb9Vo9d9s0JIGlDUZN2JDqcXyvt5tlo/PXDakeUhQhTHjZZVm+a b2FgAyiLyARXvQcL0okg9I72zqOUa9FyPMCuiAdMciTi2AyKC8MIDKvlFkaLk3IMbt+hEldUvi0 NJcaMulv82/+tzAeli2ObHdbICEuBjj8OJQB/+JmyoExA1+Tl+pdr5HJa00k+8JbU3GocA6MuGw 9WOX3WI8vrUiZtVkHhb+XpuvMF3QFjPwA2Y/gZhO2Eo9shzz1C7YaJkP9gsw5MpoGfHJIg1aB2Q GRUrb+yBdb9qnGuGDI1ic1zFXwYqoMqe8jTTuCs31UdjO3DMCMtLz0WWa5ckQPxtxeeQnTDS//8 QIg3dK+SlB2mM7IfAugeYbEYMhMnHE2ES4rcttgjHlPttCTEB9psh/p8Z/jDWZpMlpUGy0Ka8TZ C7CpCjy6obnI9ZrIL/rZSTrSGyj5tsPhm+yiGHyegSIjYo8IBr/nx6wEslns/KM+VK X-Google-Smtp-Source: AGHT+IGvw1bEvqj04ezow2gxdTDu27du8/6Xud1clcGv0XhFoxUHy3g9uZN+OOy+YeCHeDUPqPTLnA== X-Received: by 2002:a17:907:3c90:b0:b76:e89f:98a9 with SMTP id a640c23a62f3a-b7a2484b7c2mr1285132866b.61.1765302988299; Tue, 09 Dec 2025 09:56:28 -0800 (PST) Received: from localhost (host-87-6-211-245.retail.telecomitalia.it. [87.6.211.245]) by smtp.gmail.com with ESMTPSA id a640c23a62f3a-b79f49c94fbsm1430165766b.53.2025.12.09.09.56.27 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 09 Dec 2025 09:56:27 -0800 (PST) From: Andrea della Porta X-Google-Original-From: Andrea della Porta Date: Tue, 9 Dec 2025 18:58:58 +0100 To: Rob Herring Cc: Andrea della Porta , Krzysztof Kozlowski , Jim Quinlan , Florian Fainelli , Broadcom internal kernel review list , Bjorn Helgaas , Lorenzo Pieralisi , kwilczynski@kernel.org, Manivannan Sadhasivam , Krzysztof Kozlowski , Conor Dooley , linux-pci@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-rpi-kernel@lists.infradead.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, iivanov@suse.de, svarbanov@suse.de, mbrugger@suse.com, Jonathan Bell , Phil Elwell , kernel test robot Subject: Re: [PATCH] dt-bindings: pci: brcmstb: Add rp1-nexus node to fix DTC warning Message-ID: References: <20250812085037.13517-1-andrea.porta@suse.com> <4fee3870-f9d5-48e3-a5be-6df581d3e296@kernel.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20251209_095630_987682_6D78FD2B X-CRM114-Status: GOOD ( 65.06 ) 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: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org Hi Rob, On 11:09 Thu 04 Dec , Rob Herring wrote: > On Mon, Sep 1, 2025 at 3:48 AM Andrea della Porta wrote: > > > > Hi Krzysztof, > > > > On 08:50 Fri 22 Aug , Krzysztof Kozlowski wrote: > > > On 21/08/2025 17:22, Andrea della Porta wrote: > > > > Hi Krzysztof, > > > > > > > > On 10:55 Tue 12 Aug , Krzysztof Kozlowski wrote: > > > >> On 12/08/2025 10:50, Andrea della Porta wrote: > > > >>> The devicetree compiler is complaining as follows: > > > >>> > > > >>> arch/arm64/boot/dts/broadcom/rp1-nexus.dtsi:3.11-14.3: Warning (unit_address_vs_reg): /axi/pcie@1000120000/rp1_nexus: node has a reg or ranges property, but no unit name > > > >>> /home/andrea/linux-torvalds/arch/arm64/boot/dts/broadcom/bcm2712-rpi-5-b.dtb: pcie@1000120000: Unevaluated properties are not allowed ('rp1_nexus' was unexpected) > > > >> > > > >> Please trim the paths. > > > > > > > > Ack. > > > > > > > >> > > > >>> > > > >>> Add the optional node that fix this to the DT binding. > > > >>> > > > >>> Reported-by: kernel test robot > > > >>> Closes: https://lore.kernel.org/oe-kbuild-all/202506041952.baJDYBT4-lkp@intel.com/ > > > >>> Signed-off-by: Andrea della Porta > > > >>> --- > > > >>> Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml | 9 +++++++++ > > > >>> 1 file changed, 9 insertions(+) > > > >>> > > > >>> diff --git a/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml b/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml > > > >>> index 812ef5957cfc..7d8ba920b652 100644 > > > >>> --- a/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml > > > >>> +++ b/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml > > > >>> @@ -126,6 +126,15 @@ required: > > > >>> allOf: > > > >>> - $ref: /schemas/pci/pci-host-bridge.yaml# > > > >>> - $ref: /schemas/interrupt-controller/msi-controller.yaml# > > > >>> + - if: > > > >>> + properties: > > > >>> + compatible: > > > >>> + contains: > > > >>> + const: brcm,bcm2712-pcie > > > >>> + then: > > > >>> + properties: > > > >>> + rp1_nexus: > > > >> > > > >> No, you cannot document post-factum... This does not follow DTS coding > > > >> style. > > > > > > > > I think I didn't catch what you mean here: would that mean that > > > > we cannot resolve that warning since we cannot add anything to the > > > > binding? > > > > > > I meant, you cannot use a warning from the code you recently introduced > > > as a reason to use incorrect style. > > > > > > Fixing warning is of course fine and correct, but for the code recently > > > introduced and which bypassed ABI review it is basically like new review > > > of new ABI. > > > > > > This needs standard review practice, so you need to document WHY you > > > need such node. Warning is not the reason here why you are doing. If > > > this was part of original patchset, like it should have been, you would > > > not use some imaginary warning as reason, right? > > > > > > So provide reason why you need here this dedicated child, what is that > > > child representing. > > > > Ack. > > > > > > > > Otherwise I can suggest: drop the child and DTSO, this also solves the > > > warning... > > > > This would not fix the issue: it's the non overlay that needs the specific > > node. But I got the point, and we have a solution for that (see below). > > > > > > > > > > > > > Regarding rp1_nexus, you're right I guess it should be > > > > rp1-nexus as per DTS coding style. > > > > > > > >> > > > >> Also: > > > >> > > > >> Node names should be generic. See also an explanation and list of > > > >> examples (not exhaustive) in DT specification: > > > >> https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation > > > > > > > > In this case it could be difficult: we need to search for a DT node > > > > > > Search like in driver? That's wrong, you should be searching by compatible. > > > > Thanks for the hint. Searching by compatble is the solution. > > No, it is not. This is partly true, indeed. On one side there's the need to avoid a specific node name ('rp1_nexus'), so the only other unique identifier would be the compatible string ('pci1de4,1' in this case, which identifies that specific device). Unfortunately, the same compatible string is also assigned to the pci endpoint node filled automatically by enabling CONFIG_PCI_DYNAMIC_OF_NODES. We would end up with two nodes with the same compatible, which is not unique anymore. This applies only when using 'full' dtb (bcm2712-rpi-5-b.dtb) *and* you enable CONFIG_PCI_DYNAMIC_OF_NODES, the latter being not necessary since the overlay dtb (...-ovl-rp1.dtb) is not in use here. To overcome this problem, the solutions I can think of are the following: 1- Just disable CONFIG_PCI_DYNAMIC_OF_NODES should work, but only when using the full dtb version. However, if the user enable that option for debug or to use the overlay dtb version, he better be sure not to use teh full dtb or it won't work. This solution seems really weak. 2- Add another compatible string other than 'pci1de4,1', so it will be really unique. > > > > > > > > starting from the DT root and using generic names like pci@0,0 or > > > > dev@0,0 could possibly led to conflicts with other peripherals. > > > > That's why I chose a specific name. > > > > > > Dunno, depends what can be there, but you do not get a specific > > > (non-generic) device node name for a generic PCI device or endpoint. > > > > I would use 'port' instead of rp1-nexus. Would it work for you? > > Do you still plan to fix this? This is broken far worse than just the node name. Yes, if we want to get rid of that nasty warning and comply with DT guidelines, I think I really need to fix that. > > The 'rp1_nexus' node is applied to the PCI host bridge. That's wrong > unless this is PCI rather than PCIe. There's the root port device in > between. > > The clue that things are wrong are start in the driver here: > > rp1_node = of_find_node_by_name(NULL, "rp1_nexus"); > if (!rp1_node) { > rp1_node = dev_of_node(dev); > skip_ovl = false; > } > > You should not need to do this nor care what the node name is. The PCI > core should have populated pdev->dev.of_node for you. If not, your DT > is wrong. Turn on CONFIG_PCI_DYNAMIC_OF_NODES and look at the > resulting PCI nodes. They should also match what the hierarchy looks > like with lspci. I don't recommend you rely on > CONFIG_PCI_DYNAMIC_OF_NODES, but statically populate the nodes in the > DT. First, CONFIG_PCI_DYNAMIC_OF_NODES is an under development thing > and I hope to get rid of the config option. Second, your case is > static (i.e. not a PCIe card in a slot) so there is no issue > hardcoding the DT. The 'full' dtb (bcm2712-rpi-5-b.dtb) is indeed statically populated. CONFIG_PCI_DYNAMIC_OF_NODES is needed only if you use the overlay approach (bcm2712-rpi-5-b-ovl-rp1.dtb) and in that case the node will be added to the correct device node at runtime, and there won't be any node labeled as rp1_nexus. That conditional just check for teh presence of the rp1_nexus node to choose if the overlay should be loaded at runtime (if rp1_nexus is present, then we are in the static case so no overlay need to be loaded). > > As far as the node name, I don't care so much as long as the driver > doesn't care and you don't use '_' or 'pcie?' (that's for PCI > bridges). > > And why do we have drivers/misc/rp1/rp1-pci.dtso and a .dtso in > arch/arm64? There should not be both. drivers/misc/rp1/rp1-pci.dtso is just a wrapper over rp1-common.dtsi, which is to be compiled in as binary blob in the driver and loaded at runtime. rp1.dtbo is optional, it's there just for completeness in case anyone want to load teh overlay from the fw, as explained here: https://lore.kernel.org/all/ab9ab3536baf5fdf6016f2a01044f00034189291.1742418429.git.andrea.porta@suse.com/ If it causes confusion, we can probably get rid of it with no penalty. Many thanks, Andrea > > Rob