From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-pj1-f50.google.com (mail-pj1-f50.google.com [209.85.216.50]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id B71E415AD1 for ; Thu, 11 Jan 2024 12:40:25 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linaro.org Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=linaro.org Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=linaro.org header.i=@linaro.org header.b="I7L9pOnj" Received: by mail-pj1-f50.google.com with SMTP id 98e67ed59e1d1-28bc7155755so2786006a91.2 for ; Thu, 11 Jan 2024 04:40:25 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1704976825; x=1705581625; darn=vger.kernel.org; h=in-reply-to:content-transfer-encoding:content-disposition :mime-version:references:message-id:subject:cc:to:from:date:from:to :cc:subject:date:message-id:reply-to; bh=lUjrrZ3Jg1GUMAie5ebnK4hlRzI9jSk+yPsyIMgP7Cg=; b=I7L9pOnj2gmHPXX3UUbnrWYfZH2x5047cV/fypzF3Y4ux3Y0oF/TgcPKblFJhFqyfN uRt+i6HwRmtVLCXSAecx2Ihe27aeeuvT5TpzeIlMY+pOieEKi5vEYiIqh8Yh10B/i7CD RswIwp5eHwjY6N1qjV8nIrBBXxP6J14XSk3vW2Bn7p6zOzOIE/PEiBwUrY0QGlV1B3uq 5jloVDT1ujj9h5SuzxJPTboxkF1HAHNqZEefAsPr+DxEp2kTY7JW+n/TN6Iti8VvdcC1 B9JbnzOo84Tyhq4z4XFf3s4P+tBtYNJme3NJUYiuB1DgqPvdhTHQvmnigGmTOd/jg4fy 8JSg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1704976825; x=1705581625; h=in-reply-to:content-transfer-encoding:content-disposition :mime-version:references:message-id:subject:cc:to:from:date :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=lUjrrZ3Jg1GUMAie5ebnK4hlRzI9jSk+yPsyIMgP7Cg=; b=lJiiMcSFwn1KlMtXRAMu2X8MoXf7iZvYPpdJULbnzdvGTdd3LXyaSsc7gykgNDRnKR +teGH0DGor/BCDTFKsSpUIs4XPc4qhllXWBwJBoxNndU5hsct4jbBouRDOkQB6IRhEDk CSW33VYNEebCJL3uUdB3ViAQGVyMVQzDRTnZTyGl6Ffhf1o6/QZiGnfm2fK0PZqdOUq7 wC+qgYd+L1SQDrNolqTHtg0LOszcAZiOlBRhhD3HI+0OVqol5dpIf+DBFVwQVe5BQpSk pBfRahtR4oHJoPAeuRcnb37xI4bGSBryNaQci1XH3bZRV7qY1aK22DCpfxKidu7y+d4C Ugcg== X-Gm-Message-State: AOJu0Yx8HB7dzxiNoG1Oc8ZnB18Cd7fxXQTtZeSMmo5VA9Bc8wCf8gYZ NbBWkJBz50qP06ncIO9LD7UET3LX6vLi X-Google-Smtp-Source: AGHT+IFlzCZDk+7khh2nGY76iLSM3Y0+E3O+l032jjoiVmXYGJDOGVhuEwa5Sr5OwMDDcz8NiN7tAg== X-Received: by 2002:a17:90a:4984:b0:28c:f1f3:4dcb with SMTP id d4-20020a17090a498400b0028cf1f34dcbmr914767pjh.69.1704976825101; Thu, 11 Jan 2024 04:40:25 -0800 (PST) Received: from thinkpad ([202.131.159.18]) by smtp.gmail.com with ESMTPSA id mf15-20020a17090b184f00b0028cef2025ddsm1418530pjb.15.2024.01.11.04.40.13 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 11 Jan 2024 04:40:24 -0800 (PST) Date: Thu, 11 Jan 2024 18:10:09 +0530 From: Manivannan Sadhasivam To: Dan Williams Cc: Lukas Wunner , Bartosz Golaszewski , Kalle Valo , "David S . Miller" , Eric Dumazet , Jakub Kicinski , Paolo Abeni , Rob Herring , Krzysztof Kozlowski , Conor Dooley , Bjorn Andersson , Konrad Dybcio , Catalin Marinas , Will Deacon , Bjorn Helgaas , Heiko Stuebner , Jernej Skrabec , Chris Morgan , Linus Walleij , Geert Uytterhoeven , Arnd Bergmann , Neil Armstrong , =?iso-8859-1?Q?N=EDcolas_F_=2E_R_=2E_A_=2E?= Prado , Marek Szyprowski , Peng Fan , Robert Richter , Jonathan Cameron , Terry Bowman , Kuppuswamy Sathyanarayanan , Ilpo =?iso-8859-1?Q?J=E4rvinen?= , Huacai Chen , Alex Elder , Srini Kandagatla , Greg Kroah-Hartman , 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 Subject: Re: [RFC 3/9] PCI/portdrv: create platform devices for child OF nodes Message-ID: <20240111124009.GA3003@thinkpad> References: <20240104130123.37115-1-brgl@bgdev.pl> <20240104130123.37115-4-brgl@bgdev.pl> <20240109144327.GA10780@wunner.de> <20240110132853.GA6860@wunner.de> <659f00ed271b3_5cee2942@dwillia2-xfh.jf.intel.com.notmuch> Precedence: bulk X-Mailing-List: devicetree@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <659f00ed271b3_5cee2942@dwillia2-xfh.jf.intel.com.notmuch> On Wed, Jan 10, 2024 at 12:41:17PM -0800, Dan Williams wrote: > [ add Terry ] > > > Lukas Wunner wrote: > > On Wed, Jan 10, 2024 at 01:55:18PM +0100, Bartosz Golaszewski wrote: > > > On Tue, Jan 9, 2024 at 3:43???PM Lukas Wunner 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. > > I came here to say the same thing. It is already the case that portdrv > is not a good model to build new functionality upon [1], and PCI core > enlightenment should be considered first. > The primary reason for plugging the power sequencing into portdrv is due to portdrv binding with all the bridge devices and acting as management driver for the bridges. This is where exactly the power sequencing part needs to be plugged in IMO. But if the idea of the portdrv is just to expose services based on interrupts, then please suggest a better place to plug this power sequencing part. - Mani > The portdrv model is impeding Terry's CXL Port error handling effort, so > I am on the lookout for portdrv growing new entanglements to unwind > later. > > [1]: http://lore.kernel.org/r/20221025232535.GA579167@bhelgaas > -- மணிவண்ணன் சதாசிவம்