Now pci_host_common_init() no longer uses the platform device's drvdata
to store the pointer to the allocated struct pci_host_bridge. Use the
drvdata to store the pointer to the driver's private data, which is the
struct apple_pcie allocated in apple_pcie_probe(). This eliminates the
need to store these pointers in a separate mapping table.
This reverts most of the changes in commit 643c0c9d0496 ("PCI: apple:
Add tracking of probed root ports"). No "Fixes" tag is added because
nothing is broken in that commit. This is just a simplification.
Signed-off-by: Radu Rendec <rrendec@redhat.com>
---
drivers/pci/controller/pcie-apple.c | 53 ++++-------------------------
1 file changed, 7 insertions(+), 46 deletions(-)
diff --git a/drivers/pci/controller/pcie-apple.c b/drivers/pci/controller/pcie-apple.c
index 0380d300adca6..75bb6d51d31c2 100644
--- a/drivers/pci/controller/pcie-apple.c
+++ b/drivers/pci/controller/pcie-apple.c
@@ -187,7 +187,6 @@ struct apple_pcie {
const struct hw_info *hw;
unsigned long *bitmap;
struct list_head ports;
- struct list_head entry;
struct completion event;
struct irq_fwspec fwspec;
u32 nvecs;
@@ -206,9 +205,6 @@ struct apple_pcie_port {
int idx;
};
-static LIST_HEAD(pcie_list);
-static DEFINE_MUTEX(pcie_list_lock);
-
static void rmw_set(u32 set, void __iomem *addr)
{
writel_relaxed(readl_relaxed(addr) | set, addr);
@@ -724,45 +720,13 @@ static int apple_msi_init(struct apple_pcie *pcie)
return 0;
}
-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);
-}
-
-static struct apple_pcie *apple_pcie_lookup(struct device *dev)
-{
- struct apple_pcie *pcie;
-
- guard(mutex)(&pcie_list_lock);
-
- list_for_each_entry(pcie, &pcie_list, entry) {
- if (pcie->dev == dev)
- return pcie;
- }
-
- return NULL;
-}
-
static struct apple_pcie_port *apple_pcie_get_port(struct pci_dev *pdev)
{
struct pci_config_window *cfg = pdev->sysdata;
- struct apple_pcie *pcie;
+ struct apple_pcie *pcie = platform_get_drvdata(to_platform_device(cfg->parent));
struct pci_dev *port_pdev;
struct apple_pcie_port *port;
- pcie = apple_pcie_lookup(cfg->parent);
- if (WARN_ON(!pcie))
- return NULL;
-
/* Find the root port this device is on */
port_pdev = pcie_find_root_port(pdev);
@@ -843,13 +807,9 @@ static void apple_pcie_disable_device(struct pci_host_bridge *bridge, struct pci
static int apple_pcie_init(struct pci_config_window *cfg)
{
struct device *dev = cfg->parent;
- struct apple_pcie *pcie;
+ struct apple_pcie *pcie = platform_get_drvdata(to_platform_device(dev));
int ret;
- pcie = apple_pcie_lookup(dev);
- if (WARN_ON(!pcie))
- return -ENOENT;
-
for_each_available_child_of_node_scoped(dev->of_node, of_port) {
ret = apple_pcie_setup_port(pcie, of_port);
if (ret) {
@@ -876,6 +836,7 @@ static int apple_pcie_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
struct apple_pcie *pcie;
+ struct pci_host_bridge *bridge;
int ret;
pcie = devm_kzalloc(dev, sizeof(*pcie), GFP_KERNEL);
@@ -897,11 +858,11 @@ static int apple_pcie_probe(struct platform_device *pdev)
if (ret)
return ret;
- apple_pcie_register(pcie);
+ bridge = pci_host_common_init(pdev, &apple_pcie_cfg_ecam_ops);
+ if (IS_ERR(bridge))
+ return PTR_ERR(bridge);
- ret = pci_host_common_init(pdev, &apple_pcie_cfg_ecam_ops);
- if (ret)
- apple_pcie_unregister(pcie);
+ platform_set_drvdata(pdev, pcie);
return ret;
}
--
2.51.1
On Tue, 18 Nov 2025 22:12:44 +0000,
Radu Rendec <rrendec@redhat.com> wrote:
>
> Now pci_host_common_init() no longer uses the platform device's drvdata
> to store the pointer to the allocated struct pci_host_bridge. Use the
> drvdata to store the pointer to the driver's private data, which is the
> struct apple_pcie allocated in apple_pcie_probe(). This eliminates the
> need to store these pointers in a separate mapping table.
>
> This reverts most of the changes in commit 643c0c9d0496 ("PCI: apple:
> Add tracking of probed root ports"). No "Fixes" tag is added because
> nothing is broken in that commit. This is just a simplification.
>
> Signed-off-by: Radu Rendec <rrendec@redhat.com>
> ---
> drivers/pci/controller/pcie-apple.c | 53 ++++-------------------------
> 1 file changed, 7 insertions(+), 46 deletions(-)
>
> diff --git a/drivers/pci/controller/pcie-apple.c b/drivers/pci/controller/pcie-apple.c
> index 0380d300adca6..75bb6d51d31c2 100644
> --- a/drivers/pci/controller/pcie-apple.c
> +++ b/drivers/pci/controller/pcie-apple.c
> @@ -187,7 +187,6 @@ struct apple_pcie {
> const struct hw_info *hw;
> unsigned long *bitmap;
> struct list_head ports;
> - struct list_head entry;
> struct completion event;
> struct irq_fwspec fwspec;
> u32 nvecs;
> @@ -206,9 +205,6 @@ struct apple_pcie_port {
> int idx;
> };
>
> -static LIST_HEAD(pcie_list);
> -static DEFINE_MUTEX(pcie_list_lock);
> -
> static void rmw_set(u32 set, void __iomem *addr)
> {
> writel_relaxed(readl_relaxed(addr) | set, addr);
> @@ -724,45 +720,13 @@ static int apple_msi_init(struct apple_pcie *pcie)
> return 0;
> }
>
> -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);
> -}
> -
> -static struct apple_pcie *apple_pcie_lookup(struct device *dev)
> -{
> - struct apple_pcie *pcie;
> -
> - guard(mutex)(&pcie_list_lock);
> -
> - list_for_each_entry(pcie, &pcie_list, entry) {
> - if (pcie->dev == dev)
> - return pcie;
> - }
> -
> - return NULL;
> -}
> -
> static struct apple_pcie_port *apple_pcie_get_port(struct pci_dev *pdev)
> {
> struct pci_config_window *cfg = pdev->sysdata;
> - struct apple_pcie *pcie;
> + struct apple_pcie *pcie = platform_get_drvdata(to_platform_device(cfg->parent));
Isn't that an overly complicated way to write:
struct apple_pcie *pcie = dev_get_drvdata(cfg->parent);
> struct pci_dev *port_pdev;
> struct apple_pcie_port *port;
>
> - pcie = apple_pcie_lookup(cfg->parent);
> - if (WARN_ON(!pcie))
> - return NULL;
> -
> /* Find the root port this device is on */
> port_pdev = pcie_find_root_port(pdev);
>
> @@ -843,13 +807,9 @@ static void apple_pcie_disable_device(struct pci_host_bridge *bridge, struct pci
> static int apple_pcie_init(struct pci_config_window *cfg)
> {
> struct device *dev = cfg->parent;
> - struct apple_pcie *pcie;
> + struct apple_pcie *pcie = platform_get_drvdata(to_platform_device(dev));
> int ret;
>
> - pcie = apple_pcie_lookup(dev);
> - if (WARN_ON(!pcie))
> - return -ENOENT;
> -
> for_each_available_child_of_node_scoped(dev->of_node, of_port) {
> ret = apple_pcie_setup_port(pcie, of_port);
> if (ret) {
> @@ -876,6 +836,7 @@ static int apple_pcie_probe(struct platform_device *pdev)
> {
> struct device *dev = &pdev->dev;
> struct apple_pcie *pcie;
> + struct pci_host_bridge *bridge;
> int ret;
>
> pcie = devm_kzalloc(dev, sizeof(*pcie), GFP_KERNEL);
> @@ -897,11 +858,11 @@ static int apple_pcie_probe(struct platform_device *pdev)
> if (ret)
> return ret;
>
> - apple_pcie_register(pcie);
> + bridge = pci_host_common_init(pdev, &apple_pcie_cfg_ecam_ops);
> + if (IS_ERR(bridge))
> + return PTR_ERR(bridge);
>
> - ret = pci_host_common_init(pdev, &apple_pcie_cfg_ecam_ops);
> - if (ret)
> - apple_pcie_unregister(pcie);
> + platform_set_drvdata(pdev, pcie);
Not quite. You probably want to look at:
- when struct pci_ecam_ops::init() is called...
- ... compared to when you populate the driver data.
Thanks,
M.
--
Without deviation from the norm, progress is not possible.
© 2016 - 2025 Red Hat, Inc.