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 2BE14CFA754 for ; Fri, 21 Nov 2025 08:48:25 +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:Content-Transfer-Encoding: Content-Type:MIME-Version:References:In-Reply-To:Subject:Cc:To:From: Message-ID:Date:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=dYV7kYk0gVZSLvesijw8GqzdIX8qi3XKfGlYXeHpxBc=; b=HZytKRwViW9ddNd2lEGZer0dvu T+ZaEwvkJT79P4Z9B9MUlAcTaGMT0eSlt/MhCb1MenJ+FzSKAYG4iBKwkfMzfcMTC1WgF+hAVylKU UX/f2tTMv8S/g5dvr8oSkhOZ1j5v3mHD0h3L09lvqxBJ8Fg+pJjilqBlstwsDiT3vdFfOiDKOKpfg EFu8+q9uN1WyVki7ba9DHrY+VNrQ86jhXlEXdHwzDjjETtQ1RrmWMd+T0tXQuUOIDbvbIAVblOph0 3co7fUKd/nyeVJ1dmU1RY/S4q1CUzYinPpBcmDtGKAebm1QFxQ2CPFJpJq18gpOo1oKgYfQ8tIwKQ lgfsGVDw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.98.2 #2 (Red Hat Linux)) id 1vMMoV-000000085st-1Ov4; Fri, 21 Nov 2025 08:48:19 +0000 Received: from tor.source.kernel.org ([2600:3c04:e001:324:0:1991:8:25]) by bombadil.infradead.org with esmtps (Exim 4.98.2 #2 (Red Hat Linux)) id 1vMMoU-000000085sh-14xv for linux-arm-kernel@lists.infradead.org; Fri, 21 Nov 2025 08:48:18 +0000 Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by tor.source.kernel.org (Postfix) with ESMTP id 12D96601EF; Fri, 21 Nov 2025 08:48:17 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 96255C4CEF1; Fri, 21 Nov 2025 08:48:16 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1763714896; bh=OUDGpBZDkEF3ySxw/DubSWv8lCLf/8bprs+E92p9msE=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=nym8QA/4pUtTT7KbzxjHpCROPkgxhY9urUzAdVQAW4mpzhEzAe/cqwUjM3tsNl0sQ Q1UY5xAqHAeOon/ygQvG56xZtzLRVqzRX7aLhNHDRw0m413K6JOPxKRawIDw9a7V0P CGoD5MXgj1z32uWsaITdlo+7eZqoErgliNIUBzUhmbYcUfSJKfH4PhZj9DgZme/B/5 ljtJqSl7l5PQFB6I3xGG6BUFXMZkz/DHLmR5UyqwRwzkPiabHyhWkP1HXZOOE2sbYu zDi3ks2K82mRl2GXZtCbUB9Dvmd0684Dl8M4cTDdzcsDysAYK/ka07AQxXv15zSjbu rvesMlrkY9Kfg== Received: from sofa.misterjones.org ([185.219.108.64] helo=goblin-girl.misterjones.org) by disco-boy.misterjones.org with esmtpsa (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.98.2) (envelope-from ) id 1vMMoP-00000007A9E-3sXc; Fri, 21 Nov 2025 08:48:14 +0000 Date: Fri, 21 Nov 2025 08:48:13 +0000 Message-ID: <863467srea.wl-maz@kernel.org> From: Marc Zyngier To: Radu Rendec Cc: linux-pci@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, Bjorn Helgaas , Manivannan Sadhasivam , Rob Herring , Krzysztof =?UTF-8?B?V2lsY3p5xYRza2k=?= , Lorenzo Pieralisi Subject: Re: [PATCH] PCI: host-generic: Move bridge allocation outside of pci_host_common_init() In-Reply-To: <73a0863a70d558efaf29d6b988f3fec6312a22a9.camel@redhat.com> References: <20251120113630.2036078-1-maz@kernel.org> <73a0863a70d558efaf29d6b988f3fec6312a22a9.camel@redhat.com> User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI-EPG/1.14.7 (Harue) FLIM-LB/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL-LB/10.8 EasyPG/1.0.0 Emacs/30.1 (aarch64-unknown-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable X-SA-Exim-Connect-IP: 185.219.108.64 X-SA-Exim-Rcpt-To: rrendec@redhat.com, linux-pci@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, bhelgaas@google.com, mani@kernel.org, robh@kernel.org, kwilczynski@kernel.org, lpieralisi@kernel.org X-SA-Exim-Mail-From: maz@kernel.org X-SA-Exim-Scanned: No (on disco-boy.misterjones.org); SAEximRunCond expanded to false 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 On Thu, 20 Nov 2025 17:58:42 +0000, Radu Rendec wrote: >=20 > On Thu, 2025-11-20 at 11:36 +0000, Marc Zyngier wrote: > > Having the host bridge allocation inside pci_host_common_init() results > > in a lot of complexity in the pcie-apple driver (the only direct user > > of this function outside of code PCI code). > ^^^^ > nit: s/code/core :) >=20 > > It forces the allocation of driver-specific tracking structures outside > > of the bridge allocation, which in turns requires it to use inneficient > > data structures to match the bridge and the private structre as needed. > >=20 > > Instead, let the bridge structure be passed to pci_host_common_init(), > > allowing the driver to allocate it together with the private data, > > as it is usually intended. The driver can then retrieve the bridge > > via the owning device attached to the PCI config window structure. > > This allows the pcie-apple driver to be significantly simplified. > >=20 > > Both core and driver code are changed in one go to avoid going via > > a transitional interface. > >=20 > > Link: https://lore.kernel.org/r/86jyzms036.wl-maz@kernel.org > > Signed-off-by: Marc Zyngier > > Cc: Radu Rendec > > Cc: Bjorn Helgaas > > Cc: Manivannan Sadhasivam > > Cc: Rob Herring > > Cc: Krzysztof Wilczy=C5=84ski > > Cc: Lorenzo Pieralisi > > --- > > =C2=A0drivers/pci/controller/pci-host-common.c | 13 ++++---- > > =C2=A0drivers/pci/controller/pci-host-common.h |=C2=A0 1 + > > =C2=A0drivers/pci/controller/pcie-apple.c=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= | 42 ++++-------------------- > > =C2=A03 files changed, 14 insertions(+), 42 deletions(-) > >=20 > > diff --git a/drivers/pci/controller/pci-host-common.c b/drivers/pci/con= troller/pci-host-common.c > > index 810d1c8de24e9..c473e7c03baca 100644 > > --- a/drivers/pci/controller/pci-host-common.c > > +++ b/drivers/pci/controller/pci-host-common.c > > @@ -53,16 +53,12 @@ struct pci_config_window *pci_host_common_ecam_crea= te(struct device *dev, > > =C2=A0EXPORT_SYMBOL_GPL(pci_host_common_ecam_create); > > =C2=A0 > > =C2=A0int pci_host_common_init(struct platform_device *pdev, > > + struct pci_host_bridge *bridge, > > =C2=A0 const struct pci_ecam_ops *ops) > > =C2=A0{ > > =C2=A0 struct device *dev =3D &pdev->dev; > > - struct pci_host_bridge *bridge; > > =C2=A0 struct pci_config_window *cfg; > > =C2=A0 > > - bridge =3D devm_pci_alloc_host_bridge(dev, 0); > > - if (!bridge) > > - return -ENOMEM; > > - > > =C2=A0 of_pci_check_probe_only(); > > =C2=A0 > > =C2=A0 platform_set_drvdata(pdev, bridge); > > @@ -85,12 +81,17 @@ EXPORT_SYMBOL_GPL(pci_host_common_init); > > =C2=A0int pci_host_common_probe(struct platform_device *pdev) > > =C2=A0{ > > =C2=A0 const struct pci_ecam_ops *ops; > > + struct pci_host_bridge *bridge; > > =C2=A0 > > =C2=A0 ops =3D of_device_get_match_data(&pdev->dev); > > =C2=A0 if (!ops) > > =C2=A0 return -ENODEV; > > =C2=A0 > > - return pci_host_common_init(pdev, ops); > > + bridge =3D devm_pci_alloc_host_bridge(&pdev->dev, 0); > > + if (!bridge) > > + return -ENOMEM; > > + > > + return pci_host_common_init(pdev, bridge, ops); > > =C2=A0} > > =C2=A0EXPORT_SYMBOL_GPL(pci_host_common_probe); > > =C2=A0 > > diff --git a/drivers/pci/controller/pci-host-common.h b/drivers/pci/con= troller/pci-host-common.h > > index 51c35ec0cf37d..b5075d4bd7eb3 100644 > > --- a/drivers/pci/controller/pci-host-common.h > > +++ b/drivers/pci/controller/pci-host-common.h > > @@ -14,6 +14,7 @@ struct pci_ecam_ops; > > =C2=A0 > > =C2=A0int pci_host_common_probe(struct platform_device *pdev); > > =C2=A0int pci_host_common_init(struct platform_device *pdev, > > + struct pci_host_bridge *bridge, > > =C2=A0 const struct pci_ecam_ops *ops); > > =C2=A0void pci_host_common_remove(struct platform_device *pdev); > > =C2=A0 > > diff --git a/drivers/pci/controller/pcie-apple.c b/drivers/pci/controll= er/pcie-apple.c > > index 0380d300adca6..a902aa81a497e 100644 > > --- a/drivers/pci/controller/pcie-apple.c > > +++ b/drivers/pci/controller/pcie-apple.c > > @@ -206,9 +206,6 @@ struct apple_pcie_port { > > =C2=A0 int idx; > > =C2=A0}; > > =C2=A0 > > -static LIST_HEAD(pcie_list); > > -static DEFINE_MUTEX(pcie_list_lock); > > - > > =C2=A0static void rmw_set(u32 set, void __iomem *addr) > > =C2=A0{ > > =C2=A0 writel_relaxed(readl_relaxed(addr) | set, addr); > > @@ -724,32 +721,9 @@ static int apple_msi_init(struct apple_pcie *pcie) > > =C2=A0 return 0; > > =C2=A0} > > =C2=A0 > > -static void apple_pcie_register(struct apple_pcie *pcie) > > -{ > > - guard(mutex)(&pcie_list_lock); > > - > > - list_add_tail(&pcie->entry, &pcie_list); > > -} > > - > > -static void apple_pcie_unregister(struct apple_pcie *pcie) > > -{ > > - guard(mutex)(&pcie_list_lock); > > - > > - list_del(&pcie->entry); > > -} > > - > > =C2=A0static struct apple_pcie *apple_pcie_lookup(struct device *dev) > > =C2=A0{ > > - struct apple_pcie *pcie; > > - > > - guard(mutex)(&pcie_list_lock); > > - > > - list_for_each_entry(pcie, &pcie_list, entry) { > > - if (pcie->dev =3D=3D dev) > > - return pcie; > > - } > > - > > - return NULL; > > + return pci_host_bridge_priv(dev_get_drvdata(dev)); > > =C2=A0} > >=20 > >=20 >=20 > You forgot to remove the `entry` field from struct apple_pcie. It's no > longer needed now that pcie_list is gone. Ah, good point. Fixed. >=20 > > =C2=A0 > > =C2=A0static struct apple_pcie_port *apple_pcie_get_port(struct pci_dev= *pdev) > > @@ -875,13 +849,15 @@ static const struct pci_ecam_ops apple_pcie_cfg_e= cam_ops =3D { > > =C2=A0static int apple_pcie_probe(struct platform_device *pdev) > > =C2=A0{ > > =C2=A0 struct device *dev =3D &pdev->dev; > > + struct pci_host_bridge *bridge; > > =C2=A0 struct apple_pcie *pcie; > > =C2=A0 int ret; > > =C2=A0 > > - pcie =3D devm_kzalloc(dev, sizeof(*pcie), GFP_KERNEL); > > - if (!pcie) > > + bridge =3D devm_pci_alloc_host_bridge(dev, sizeof(*pcie)); > > + if (!bridge) > > =C2=A0 return -ENOMEM; > > =C2=A0 > > + pcie =3D pci_host_bridge_priv(bridge); > > =C2=A0 pcie->dev =3D dev; > > =C2=A0 pcie->hw =3D of_device_get_match_data(dev); > > =C2=A0 if (!pcie->hw) > > @@ -897,13 +873,7 @@ static int apple_pcie_probe(struct platform_device= *pdev) > > =C2=A0 if (ret) > > =C2=A0 return ret; > > =C2=A0 > > - apple_pcie_register(pcie); > > - > > - ret =3D pci_host_common_init(pdev, &apple_pcie_cfg_ecam_ops); > > - if (ret) > > - apple_pcie_unregister(pcie); > > - > > - return ret; > > + return pci_host_common_init(pdev, bridge, &apple_pcie_cfg_ecam_ops); > > =C2=A0} > > =C2=A0 > > =C2=A0static const struct of_device_id apple_pcie_of_match[] =3D { >=20 > With those two nitpicks addressed, >=20 > Reviewed-by: Radu Rendec Thanks. I'll repost this patch some time next week, as this is only an optimisation, not really a fix. >=20 > And thanks again for spending time on this and creating the patch. Nerd-sniping is a sport, it seems... ;-) Cheers, M. --=20 Without deviation from the norm, progress is not possible.