Received-SPF: pass (google.com: domain of linux-wireless+bounces-1662-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Date: Wed, 10 Jan 2024 14:28:53 +0100
From: Lukas Wunner <lukas@wunner.de>
To: Bartosz Golaszewski <brgl@bgdev.pl>
Cc: Kalle Valo <kvalo@kernel.org>, "David S . Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	Rob Herring <robh+dt@kernel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
	Conor Dooley <conor+dt@kernel.org>,
	Bjorn Andersson <andersson@kernel.org>,
	Konrad Dybcio <konrad.dybcio@linaro.org>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Will Deacon <will@kernel.org>, Bjorn Helgaas <bhelgaas@google.com>,
	Heiko Stuebner <heiko@sntech.de>,
	Jernej Skrabec <jernej.skrabec@gmail.com>,
	Chris Morgan <macromorgan@hotmail.com>,
	Linus Walleij <linus.walleij@linaro.org>,
	Geert Uytterhoeven <geert+renesas@glider.be>,
	Arnd Bergmann <arnd@arndb.de>,
	Neil Armstrong <neil.armstrong@linaro.org>,
	=?iso-8859-1?Q?N=EDcolas_F_=2E_R_=2E_A_=2E?= Prado <nfraprado@collabora.com>,
	Marek Szyprowski <m.szyprowski@samsung.com>,
	Peng Fan <peng.fan@nxp.com>, Robert Richter <rrichter@amd.com>,
	Dan Williams <dan.j.williams@intel.com>,
	Jonathan Cameron <Jonathan.Cameron@huawei.com>,
	Terry Bowman <terry.bowman@amd.com>,
	Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>,
	Ilpo =?iso-8859-1?Q?J=E4rvinen?= <ilpo.jarvinen@linux.intel.com>,
	Huacai Chen <chenhuacai@kernel.org>, Alex Elder <elder@linaro.org>,
	Srini Kandagatla <srinivas.kandagatla@linaro.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	linux-wireless@vger.kernel.org, netdev@vger.kernel.org,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-arm-msm@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	linux-pci@vger.kernel.org,
	Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Subject: Re: [RFC 3/9] PCI/portdrv: create platform devices for child OF nodes
Message-ID: <20240110132853.GA6860@wunner.de>
References: <20240104130123.37115-1-brgl@bgdev.pl>
 <20240104130123.37115-4-brgl@bgdev.pl>
 <20240109144327.GA10780@wunner.de>
 <CAMRc=MdXO6c6asvRSn_Z8-oFS48hroT+dazGKB6WWY1_Zu7f1Q@mail.gmail.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <CAMRc=MdXO6c6asvRSn_Z8-oFS48hroT+dazGKB6WWY1_Zu7f1Q@mail.gmail.com>
User-Agent: Mutt/1.10.1 (2018-07-13)

On Wed, Jan 10, 2024 at 01:55:18PM +0100, Bartosz Golaszewski wrote:
> On Tue, Jan 9, 2024 at 3:43???PM Lukas Wunner <lukas@wunner.de> wrote:
> > On Thu, Jan 04, 2024 at 02:01:17PM +0100, Bartosz Golaszewski wrote:
> > > In order to introduce PCIe power-sequencing, we need to create platform
> > > devices for child nodes of the port driver node. They will get matched
> > > against the pwrseq drivers (if one exists) and then the actuak PCIe
> > > device will reuse the node once it's detected on the bus.
> > [...]
> > > --- a/drivers/pci/pcie/portdrv.c
> > > +++ b/drivers/pci/pcie/portdrv.c
> > > @@ -715,7 +716,7 @@ static int pcie_portdrv_probe(struct pci_dev *dev,
> > >               pm_runtime_allow(&dev->dev);
> > >       }
> > >
> > > -     return 0;
> > > +     return devm_of_platform_populate(&dev->dev);
> > >  }
> >
> > I think this belongs in of_pci_make_dev_node(), portdrv seems totally
> > the wrong place.  Note that you're currently calling this for RCECs
> > (Root Complex Event Collectors) as well, which is likely not what
> > you want.
> >
> 
> of_pci_make_dev_node() is only called when the relevant PCI device is
> instantiated which doesn't happen until it's powered-up and scanned -
> precisely the problem I'm trying to address.

No, of_pci_make_dev_node() is called *before* device_attach(),
i.e. before portdrv has even probed.  So it seems this should
work perfectly well for your use case.


> > devm functions can't be used in the PCI core, so symmetrically call
> > of_platform_unpopulate() from of_pci_remove_node().
> 
> I don't doubt what you're saying is true (I've seen worse things) but
> this is the probe() callback of a driver using the driver model. Why
> wouldn't devres work?

The long term plan is to move the functionality in portdrv to
the PCI core.  Because devm functions can't be used in the PCI
core, adding new ones to portdrv will *add* a new roadblock to
migrating portdrv to the PCI core.  In other words, it makes
future maintenance more difficult.

Generally, only PCIe port services which share the same interrupt
(hotplug, PME, bandwith notification, flit error counter, ...)
need to live in portdrv.  Arbitrary other stuff should not be
shoehorned into portdrv.

Thanks,

Lukas