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 143B3C282EC for ; Mon, 10 Mar 2025 15:14:54 +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=x6bOGmoklXRtJQxdeWHpYOOfYuKnU0zJof9I6zGwu64=; b=gSPR+na1A/3ZObIlFmKJARy8cf m4DAArWNR1q624+xEwK9A0wIKw5d1n0awCgoKKL48aBWqdyjKJ06DRSLFMJxP7d9d1uKsMxU7hS5H X1gtOaaq8kHMzsd3R6oYpuEOnw6J7HSupbc+5Rrkmhn/ClDHAAww2C3yuP+jnaH3kHiQ0FJt1Jqr1 wS8AoiRgxgaxZRMWOLiCAB9GGhi1sFRKex6XRddae/RGWhSIhAKEoVH0kMAi+DCBLaLIAzDLdDLmI bkJzg21dXRjRiVoBZuo5nDpEN4jCqlyS8Yr3XYkGOGvpeevBHLi/hfslAaWfaJdOoIlpn+MmmdWQM jZ0J3hKQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.98 #2 (Red Hat Linux)) id 1treq2-0000000384e-16PH; Mon, 10 Mar 2025 15:14:42 +0000 Received: from mail-wr1-x42b.google.com ([2a00:1450:4864:20::42b]) by bombadil.infradead.org with esmtps (Exim 4.98 #2 (Red Hat Linux)) id 1trdeL-00000002rgd-421N for linux-arm-kernel@lists.infradead.org; Mon, 10 Mar 2025 13:58:35 +0000 Received: by mail-wr1-x42b.google.com with SMTP id ffacd0b85a97d-390cf7458f5so4140441f8f.2 for ; Mon, 10 Mar 2025 06:58:32 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.com; s=google; t=1741615112; x=1742219912; 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=x6bOGmoklXRtJQxdeWHpYOOfYuKnU0zJof9I6zGwu64=; b=fSQ8+Ib35/6ouGQlQ4FFKa/PZrVhnWcmj4NJBPsJb+Gq5rVsLhsUVY4VzOs+S5Bdln dfd9brdwZjaUmKbcB4VjamQeqkFEPQg3f+L6Sk1KKtTMscygh1LtAwf/O/+/0bA6QdYb SWuXQ4D7anMFw0urTCp1gpTTS9wk5xz+K62chLooU2Ur1s1fj6Tkn2WflWq4BbbuPn7R uj1qjMCj/37KrCYUqUFK1jLte7rpEvN88uucee8bhBNYt0wWMXv6PNNW8Pe6WDOytEBx B3aOUcjHZkSDqEqa9IL0oWH1N/lhhKYaxLbDyWedOh1YJ4L7SpsoIinzCDTzSsWYx2/D Be5g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1741615112; x=1742219912; h=in-reply-to:content-transfer-encoding:content-disposition :mime-version:references:message-id:subject:cc:to:date:from :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=x6bOGmoklXRtJQxdeWHpYOOfYuKnU0zJof9I6zGwu64=; b=Ie7T5FyNv4NWqZJdlh71jJv1VpsWbsBSYP11vBSe5qSok5K6lmPUuq2MhIFSqAFrVv WfIazSgwvbeoextX2yA5Uu8sVY+yPKvjUieaLnexjhmqDZ0j6dTBpYYvIhDYKHRvOuir D0WcPcJDho8MquUHlnKhujq1HX0EmtffD+IPDbgHUXsSs7xQe43J51ZtTt+h4UjS7u8N iU0oQ9TPrjpg4nla+e2rVEaQtKSXFEMxBYcMQoZFArDRPc9zWKojWuXBKG3wEiphst83 GBk9OoFNNrCZ2pgmw8yEtbNI4Eq7TnkFhN3ptakh+3VidA9JydcMMmTED/k7ytkioULJ C39Q== X-Forwarded-Encrypted: i=1; AJvYcCUKROOTSOhf6/C7Ymf9iIRPJYl6r9vIwt58W/zouuprfQqe8a6qVp7Wlv0A9BXK5/z5kUugfGSHOn2S9oQsufst@lists.infradead.org X-Gm-Message-State: AOJu0YzUqwKLKKRYngGh0uHGirwe8CfDIaroaJmPUf1BW6F0CcTfyPSs XTqHlfslMysyH7Yc2WcunAD1jjM+II6NFfonE+VSBWenv3wBts29fetEJ5N9jR8= X-Gm-Gg: ASbGncsokD9c/JV11gRgKwBC5LuGMeIj8RJ0ISK5XblPo1cvCg9IUpmRcqEsfqnGNOp cg2jw3haYDRaztgevE60w2cMZRwXN/4+6fKAi2kNkhSggqblNT8qFaLc236HQnrp61YBAA8tP+r Tqx6Nd2QU6zve5fcfs+3Jbh1YDNg1pF4HxAsO6zrrLyfvJ88h3qR9SccxVSXMx+TKqePhC0h2z4 sYptKyDtQ1oChzm6Zw1Tei8OuzYHQAS/xuYFx2+UJjg476TcfCh24OE4xHaDLJK9g+v/PF/eT9v L5gjfkXYBRaRJ6lhvrcsEwHJxntxpWHh+mn6ptB0wYFhSUjNH8tBcIaVwcE9RVIcp1nN1GRhh7r dQsLlv7YvQ62ZSYKl8P+mFU4= X-Google-Smtp-Source: AGHT+IGyhk5+VAMcITm5rrq0aJ90sKdh4cQmUoqGgN6Meob+XW4ZpicfvmV1X3pFEScV3weEws3UqA== X-Received: by 2002:a05:6000:1fa4:b0:38f:577f:2f6d with SMTP id ffacd0b85a97d-39132d093demr10173742f8f.2.1741615111629; Mon, 10 Mar 2025 06:58:31 -0700 (PDT) Received: from localhost (host-87-14-236-98.retail.telecomitalia.it. [87.14.236.98]) by smtp.gmail.com with ESMTPSA id a640c23a62f3a-ac299a025c7sm236754666b.51.2025.03.10.06.58.30 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 10 Mar 2025 06:58:31 -0700 (PDT) From: Andrea della Porta X-Google-Original-From: Andrea della Porta Date: Mon, 10 Mar 2025 14:59:41 +0100 To: Phil Elwell Cc: Herve Codina , Andrea della Porta , andrew@lunn.ch, Arnd Bergmann , "maintainer:BROADCOM BCM7XXX ARM ARCHITECTURE" , bhelgaas@google.com, brgl@bgdev.pl, Catalin Marinas , Conor Dooley , derek.kiernan@amd.com, devicetree@vger.kernel.org, dragan.cvetic@amd.com, Florian Fainelli , Greg Kroah-Hartman , krzk+dt@kernel.org, kw@linux.com, Linus Walleij , linux-arm-kernel , linux-clk@vger.kernel.org, linux-gpio@vger.kernel.org, LKML , "open list:PCI NATIVE HOST BRIDGE AND ENDPOINT DRIVERS" , "moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE" , lpieralisi@kernel.org, luca.ceresoli@bootlin.com, manivannan.sadhasivam@linaro.org, masahiroy@kernel.org, Michael Turquette , Rob Herring , saravanak@google.com, Stephen Boyd , thomas.petazzoni@bootlin.com, Stefan Wahren , Will Deacon , Dave Stevenson Subject: Re: [PATCH v6 00/10] Add support for RaspberryPi RP1 PCI device using a DT overlay Message-ID: References: <20250213171435.1c2ce376@bootlin.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 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-20250310_065833_995611_1FF86B63 X-CRM114-Status: GOOD ( 47.42 ) 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, On 16:27 Thu 13 Feb , Phil Elwell wrote: > Hi Hervé, > > On Thu, 13 Feb 2025 at 16:14, Herve Codina wrote: > > > > Hi Phil, > > > > On Thu, 13 Feb 2025 15:18:45 +0000 > > Phil Elwell wrote: > > > > > Hi Andrea, > > > > > > The problem with this approach (loading an overlay from the RP1 PCIe > > > driver), and it's one that I have raised with you offline, is that > > > (unless anyone can prove otherwise) it becomes impossible to create a > > > Pi 5 DTS file which makes use of the RP1's resources. How do you > > > declare something as simple as a button wired to an RP1 GPIO, or fan > > > connected to a PWM output? > > > > The driver could be improved in a second step. > > For instance, it could load the dtbo from user-space using request_firmare() > > instead of loading the embedded dtbo. > > > > > > > > If this is the preferred route to upstream adoption, I would prefer it > > > if rp1.dtso could be split in two - an rp1.dtsi similar to what we > > > have downstream, and an rp1.dtso that #includes it. In this way we can > > > keep the patching and duplication to a minimum. > > > > Indeed, having a rp1.dtsi avoid duplication but how the rp1.dtso in > > the the kernel sources could include user customization (button, fan, ...) > > without being modified ? > > At least we have to '#include '. > > > > Requesting the dtbo from user-space allows to let the user to create > > its own dtso without the need to modify the one in kernel sources. > > > > Does it make sense ? > > I think I understand what you are saying, but at this point the RP1 > overlay would no longer be an RP1 overlay - it would be an > RP1-and-everything-connected-to-it overlay, which is inherently > board-specific. Which user-space process do you think would be > responsible for loading this alternative overlay, choosing carefully > based on the platform it is running on? Doesn't that place quite a > burden on all the OS maintainers who up to now have just needed a > kernel and a bunch of dtb files? > > If it is considered essential that the upstream Pi 5 dts file does not > include RP1 and its children, then Raspberry Pi are going to have to > walk a different path until we've seen how that can work. By splitting > rp1.dtso as I suggested, and perhaps providing an alternative helper > function that only applies the built-in overlay if the device node > doesn't already exist, we get to stay as close to upstream as > possible. > > Phil So, the problem is twofold: the first is due to the fact that downstream expects the dtb to be fully declared at fw load time (I'll call that *monolithic* dtb from now on), the second is about how to represent dependencies between board dtb and rp1 overlay which arises only when using overlays instead of a monolithic dtb. The former issue must be solved first in order for the latter to even exists (if we don't use overlay, the dependencies are fully exposed in the dtb since the beginning), so I'll concentrate on the former for now. There are 3 possible scenarios to be reconciled: 1 - MONOLITHIC DTB This is the downstream case, where it's advisable to have only one dtb blob containing everything (rp1 included) loaded by the fw. In this case the resulting devicetree would looks like: axi { pcie@120000 { rp1_nexus { pci-ep-bus@1 { ... } } } } 2 - RP1 LOADED FROM OVERLAY BY THE FW In this case the rp1 dt node is loaded from overlay directly by the fw and the resulting devicetree is exactly equal to the monolithic dtb scenario. In order for that overlay to be loaded by fw, just add 'dtoverlay=rp1' in 'config.txt'. 3 - RP1 LOADED FROM OVERLAY AT RUNTIME Here it's the rp1 driver that loads the overlay at runtime, which is the case that this patchset originally proposed. The devicetree ends up like this: axi { pcie@120000 { pci@0,0 { dev@0,0 { pci-ep-bus@1 { ... } } } } } and this is exepcially useful to cope with the case in which there's no DT natively used, e.g. on ACPI systems. In order for all those 3 mentioned scenatios to work, I propose the following inclusion scheme for for the dts files (the arrow points to the includer): rp1-pci.dtso rp1.dtso ^ ^ | | rp1-common.dtsi ----> rp1-nexus.dtsi ----> bcm2712-rpi-5-b-MONOLITHIC.dts where those dts are defined as follows (omitting the internal properties for clarity sake): - rp1-common.dtsi ------- // definition of core rp1 and its peripherals, common // for all cases pci_ep_bus: pci-ep-bus@1 { rp1_clocks { }; rp1_gpio { }; rp1_eth { }; }; - rp1-pci.dtso ---------- // ovl linked in the rp1 driver code to be loaded at // runtime from rp1 driver. Only for case 3 /plugin/; fragment@0 { target-path=""; __overlay__ { #include "rp1-common.dtsi" }; } - rp1-nexus.dtsi ------- // adapter to decouple rp1 ranges for non runtime-loaded // overlay case (i.e. only for case 1 and 2) rp1_nexus { ranges = ... #include "rp1-common.dtsi" }; - rp1.dtso ------------ // overlay to be loaded by fw (case 2) /plugin/; &pcie2 { #include "rp1-nexus.dtsi" }; - bcm2712-rpi-5-b-MONOLITHIC.dts --- // monolithic dtb to avoid any overlay use // (case 1) / { ... all rpi5 board dts ... &pcie2 { #include "rp1-nexus.dtsi" }; }; with only minimal changes to the rp1 driver code, I can confirm that all those scenarios can coexits and are working fine. Before processding with a new patchset I'd like to have some thoughts about that, do you think this is a viable approach? Many thanks, Andrea