From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id D8F7524BD0C for ; Thu, 20 Nov 2025 17:58:47 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=170.10.129.124 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1763661529; cv=none; b=pkLFWDRJTDS6QPRJBWR0+xFgjGmkxp9m6CQ/+223oLykBRVn9qFPX3OonKRfCvr/Lwm+TK2yG0TONpKTKyyihEJr5E68FU5XPpttmI/TLJk87HoNyHfCKFp7Hc2iBNyYBridy/AVtSsxH9LAMi2ngj7Y+8HZhRzpTNF6ALsap4E= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1763661529; c=relaxed/simple; bh=VmS5Cf/FqTbc4XNNokRvTgZc5rrdVw8jJtwpW2FLksg=; h=Message-ID:Subject:From:To:Cc:Date:In-Reply-To:References: Content-Type:MIME-Version; b=UuXkCRV+QSeEkP2X53G7qlZU4+uZC/hlPPEOJ4ehuLLSqfPjMMCkvNo/tZ2fjl0K4w/zHSqR7aOfU4+gx2L4hL0qy2nobUcABy1YHoSZKVkHsFiyVYc9tV+P1qRMNhSsD6rWhUDHP5pFmf/v1QhZsg8Jmzf6t/jTn9ikkE7gbAM= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=redhat.com; spf=pass smtp.mailfrom=redhat.com; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b=KLR+CKtG; dkim=pass (2048-bit key) header.d=redhat.com header.i=@redhat.com header.b=CsxWS8/m; arc=none smtp.client-ip=170.10.129.124 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=redhat.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=redhat.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b="KLR+CKtG"; dkim=pass (2048-bit key) header.d=redhat.com header.i=@redhat.com header.b="CsxWS8/m" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1763661526; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=FBAWQgLEjTxMhY3wdWptXKZF7ghZZcFHnbvHOBx3cmI=; b=KLR+CKtGEUuzw+XB7rS/SjlJFTiF39WygJapMxYvEV6SBgDp1VjmtlTPfCY7sAPP2Q8LUA gswnu90TJoAV1inVUIMQye9wExsXyRZeAuL/psFKfqVtipHl63u0HaFSn1tAPYp8uuwBQ3 IP1lBdmKv8+dKWkX4aOLSXsLBmMKGXM= Received: from mail-qk1-f198.google.com (mail-qk1-f198.google.com [209.85.222.198]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-607-QaG0PIA4MACZHVWpeVrCEA-1; Thu, 20 Nov 2025 12:58:45 -0500 X-MC-Unique: QaG0PIA4MACZHVWpeVrCEA-1 X-Mimecast-MFC-AGG-ID: QaG0PIA4MACZHVWpeVrCEA_1763661525 Received: by mail-qk1-f198.google.com with SMTP id af79cd13be357-8b1d8f56e24so328642085a.2 for ; Thu, 20 Nov 2025 09:58:45 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=google; t=1763661525; x=1764266325; darn=vger.kernel.org; h=mime-version:user-agent:content-transfer-encoding:references :in-reply-to:date:cc:to:from:subject:message-id:from:to:cc:subject :date:message-id:reply-to; bh=FBAWQgLEjTxMhY3wdWptXKZF7ghZZcFHnbvHOBx3cmI=; b=CsxWS8/mjGlWAADpUv0Ra49xbmgFFKhBTvKqxuKNbMCcgg+ajcJxTV1lOKuPz9lVMR QkCDoQEfEh0BFY5J2IpuouZoB9yxpmbXf2wd3Q31bNKxicad+lK0/hIIU6hGKEXczlBv jVEkQxaJ6u6xOrm67D50pd2jdgcT4AI7jiO93W/ttN9t0G4zFGDrRUt/nGeZmOiNRZi1 wATLEd0/J8Ds7jTxlRtGhHT7ybVJup74y6Xc7RgpZvSKd0u5i3Jm35zhf2eeomIRE0wa vgefXFLRvxn9vFNDa9O6IAOmAo6fmZxr57QBjkW2nciW9tJLKks5FIewfhu84cTg2WsL LIQA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1763661525; x=1764266325; h=mime-version:user-agent:content-transfer-encoding:references :in-reply-to:date:cc:to:from:subject:message-id:x-gm-gg :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=FBAWQgLEjTxMhY3wdWptXKZF7ghZZcFHnbvHOBx3cmI=; b=DaL72EJbxecW/tudBbCq15vz2thGHUp6mkIsHG9Pqpgy9fF7tudYp1vhBlgSrIRYvH nglXZMXELaLwrURuDXR6ZDaoc6YzfJ+qqYtEnIV0pKQlQtpK7JJdJwX4e04WYzGO49dX M1uWhqfVn5FtvjThWAyUcLCUjBGgHTWNJUkjQn9ysebIo3GhqEf9jY421nCImGYgqSFD oyFj9mRGO0mcCtrCeyM0/CIybD642paAPcQ2cBLv8vAxHFIeGfKMmFuiLSzZ+g344xQ8 VB+rd/uzs/xhu1cky/q4Fer1KbuaMy2K1NnSzFoujWsayNyrx2/rKJajn4qSPlIh+Osf xQfQ== X-Forwarded-Encrypted: i=1; AJvYcCUd5usCzWaBUBbXAp4jvyrYXyvbhM3nVAQSDKtCLtB60cf48mjZfyZQBLYfIDI4EtfXgTqB3kQcuuMsIE0=@vger.kernel.org X-Gm-Message-State: AOJu0YzcqtF75jumPCBH4Q30LVpiltW74ymjGsu2hEOIx5TAAykcB12a 7WjOtMF1AHBtiSQlw+V4Cdb4W/YU6F2zAv4f5WWOz2JZHdCqymKBDW/6KpzJvopZw0jNMNyNd/O F/YPlr4jOkddHgE4k776W93WoMW8BYyGzdwM7DElq+GhoESt6miAdhcAgdflNjMjKGQ== X-Gm-Gg: ASbGnctXcpzVN1i2IT4WsKi/X8NQb4QS/nOXqv4dTHacfyfWHFl4PkJ4aodwpWdvYxT LLz0yojzJnk4N1aoL84vM2xHYqLVO1Q+tGhliR91HqE+W6zjDd+ABuIKGurApjwX7Aei63udxl9 +GoeXSsQnU86b2TUrI4hgQ4xsxP5LEOqT+9KCiGInQyo/tj9YztMhWiTP5pe2OuiDHa+7Si7aB8 yCM3A1H+py0IsolEyQj2gANybB5kOZyYyRL9g8Ac000rZsc3Tq3hjCKDpjN0yenLTOLkpAS3ePl B7umNpsqTewOf6i4D5mxsv8QnoQoCNShMlNzDgSu56BuEw4W0azzJoblmfnRnL9k33E0G/Ku4xO hCmDoe+ZzQ3OtlLDUdXBW82wbIsVQOwXGn6pChGWEgzZgnan2May1Hi1Ji/e+586SmGZxwI64Ur 7b X-Received: by 2002:a05:620a:29d6:b0:883:b565:1acf with SMTP id af79cd13be357-8b32749c608mr633795785a.60.1763661525010; Thu, 20 Nov 2025 09:58:45 -0800 (PST) X-Google-Smtp-Source: AGHT+IHQjuYnyNUCtoiqgSWFXqZwfTOJ3YABSpke0OuFWsE6r3fNUq9NnSWQn2HnLmxFjKwF9Y5Cqw== X-Received: by 2002:a05:620a:29d6:b0:883:b565:1acf with SMTP id af79cd13be357-8b32749c608mr633791585a.60.1763661524583; Thu, 20 Nov 2025 09:58:44 -0800 (PST) Received: from thinkpad-p1.localdomain (pool-174-112-193-187.cpe.net.cable.rogers.com. [174.112.193.187]) by smtp.gmail.com with ESMTPSA id af79cd13be357-8b3293299b3sm200510185a.2.2025.11.20.09.58.43 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 20 Nov 2025 09:58:44 -0800 (PST) Message-ID: <73a0863a70d558efaf29d6b988f3fec6312a22a9.camel@redhat.com> Subject: Re: [PATCH] PCI: host-generic: Move bridge allocation outside of pci_host_common_init() From: Radu Rendec To: Marc Zyngier , linux-pci@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org Cc: Bjorn Helgaas , Manivannan Sadhasivam , Rob Herring , Krzysztof =?UTF-8?Q?Wilczy=C5=84ski?= , Lorenzo Pieralisi Date: Thu, 20 Nov 2025 12:58:42 -0500 In-Reply-To: <20251120113630.2036078-1-maz@kernel.org> References: <20251120113630.2036078-1-maz@kernel.org> Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable User-Agent: Evolution 3.54.3 (3.54.3-2.fc41) Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 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 :) > 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/contr= oller/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_create= (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/contr= oller/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/controller= /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 You forgot to remove the `entry` field from struct apple_pcie. It's no longer needed now that pcie_list is gone. > =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_eca= m_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 { With those two nitpicks addressed, Reviewed-by: Radu Rendec And thanks again for spending time on this and creating the patch. --=20 Radu