All of lore.kernel.org
 help / color / mirror / Atom feed
From: Thierry Reding <thierry.reding@gmail.com>
To: Bjorn Helgaas <bhelgaas@google.com>
Cc: Arnd Bergmann <arnd@arndb.de>, Tomasz Nowicki <tn@semihalf.com>,
	Liviu Dudau <liviu.dudau@arm.com>,
	linux-pci@vger.kernel.org, linux-tegra@vger.kernel.org
Subject: [PATCH v3 4/4] PCI: tegra: Use new pci_register_host_bridge() interface
Date: Mon, 15 Aug 2016 17:36:47 +0200	[thread overview]
Message-ID: <20160815153647.1075-4-thierry.reding@gmail.com> (raw)
In-Reply-To: <20160815153647.1075-1-thierry.reding@gmail.com>

From: Arnd Bergmann <arnd@arndb.de>

Tegra is one of the remaining platforms that still use the traditional
pci_common_init_dev() interface for probing PCI host bridges.

This demonstrates how to convert it to the pci_register_host interface
I just added in a previous patch. This leads to a more linear probe
sequence that can handle errors better because we avoid callbacks into
the driver, and it makes the driver architecture independent.

As a side note, I should mention that I noticed this driver does not
register any IORESOURCE_IO resource with the bus, but instead registers
the I/O port window as a memory resource, which is surely a bug.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 drivers/pci/host/pci-tegra.c | 99 +++++++++++++++++++++++---------------------
 1 file changed, 52 insertions(+), 47 deletions(-)

diff --git a/drivers/pci/host/pci-tegra.c b/drivers/pci/host/pci-tegra.c
index 2d520755b1d7..6737d1be9798 100644
--- a/drivers/pci/host/pci-tegra.c
+++ b/drivers/pci/host/pci-tegra.c
@@ -260,6 +260,7 @@ static inline struct tegra_msi *to_tegra_msi(struct msi_controller *chip)
 }
 
 struct tegra_pcie {
+	struct pci_host_bridge *bridge;
 	struct device *dev;
 
 	void __iomem *pads;
@@ -322,11 +323,6 @@ struct tegra_pcie_bus {
 	unsigned int nr;
 };
 
-static inline struct tegra_pcie *sys_to_pcie(struct pci_sys_data *sys)
-{
-	return sys->private_data;
-}
-
 static inline void afi_writel(struct tegra_pcie *pcie, u32 value,
 			      unsigned long offset)
 {
@@ -430,7 +426,8 @@ free:
 
 static int tegra_pcie_add_bus(struct pci_bus *bus)
 {
-	struct tegra_pcie *pcie = sys_to_pcie(bus->sysdata);
+	struct pci_host_bridge *host = pci_find_host_bridge(bus);
+	struct tegra_pcie *pcie = host->private;
 	struct tegra_pcie_bus *b;
 
 	b = tegra_pcie_bus_alloc(pcie, bus->number);
@@ -444,7 +441,8 @@ static int tegra_pcie_add_bus(struct pci_bus *bus)
 
 static void tegra_pcie_remove_bus(struct pci_bus *child)
 {
-	struct tegra_pcie *pcie = sys_to_pcie(child->sysdata);
+	struct pci_host_bridge *host = pci_find_host_bridge(child);
+	struct tegra_pcie *pcie = host->private;
 	struct tegra_pcie_bus *bus, *tmp;
 
 	list_for_each_entry_safe(bus, tmp, &pcie->buses, list) {
@@ -461,7 +459,8 @@ static void __iomem *tegra_pcie_map_bus(struct pci_bus *bus,
 					unsigned int devfn,
 					int where)
 {
-	struct tegra_pcie *pcie = sys_to_pcie(bus->sysdata);
+	struct pci_host_bridge *host = pci_find_host_bridge(bus);
+	struct tegra_pcie *pcie = host->private;
 	void __iomem *addr = NULL;
 
 	if (bus->number == 0) {
@@ -609,35 +608,29 @@ static void tegra_pcie_relax_enable(struct pci_dev *dev)
 }
 DECLARE_PCI_FIXUP_FINAL(PCI_ANY_ID, PCI_ANY_ID, tegra_pcie_relax_enable);
 
-static int tegra_pcie_setup(int nr, struct pci_sys_data *sys)
+static int tegra_pcie_request_resources(struct tegra_pcie *pcie)
 {
-	struct tegra_pcie *pcie = sys_to_pcie(sys);
+	struct list_head *windows = &pcie->bridge->windows;
 	int err;
 
-	sys->mem_offset = pcie->offset.mem;
-	sys->io_offset = pcie->offset.io;
-
-	err = devm_request_resource(pcie->dev, &iomem_resource, &pcie->io);
-	if (err < 0)
-		return err;
-
-	pci_add_resource_offset(&sys->resources, &pcie->pio, sys->io_offset);
-	pci_add_resource_offset(&sys->resources, &pcie->mem, sys->mem_offset);
-	pci_add_resource_offset(&sys->resources, &pcie->prefetch,
-				sys->mem_offset);
-	pci_add_resource(&sys->resources, &pcie->busn);
+	pci_add_resource_offset(windows, &pcie->pio, pcie->offset.io);
+	pci_add_resource_offset(windows, &pcie->mem, pcie->offset.mem);
+	pci_add_resource_offset(windows, &pcie->prefetch, pcie->offset.mem);
+	pci_add_resource(windows, &pcie->busn);
 
-	err = devm_request_pci_bus_resources(pcie->dev, &sys->resources);
+	err = devm_request_pci_bus_resources(pcie->dev, windows);
 	if (err < 0)
 		return err;
 
 	pci_remap_iospace(&pcie->pio, pcie->io.start);
-	return 1;
+
+	return 0;
 }
 
 static int tegra_pcie_map_irq(const struct pci_dev *pdev, u8 slot, u8 pin)
 {
-	struct tegra_pcie *pcie = sys_to_pcie(pdev->bus->sysdata);
+	struct pci_host_bridge *host = pci_find_host_bridge(pdev->bus);
+	struct tegra_pcie *pcie = host->private;
 	int irq;
 
 	tegra_cpuidle_pcie_irqs_in_use();
@@ -1544,6 +1537,8 @@ static int tegra_pcie_enable_msi(struct tegra_pcie *pcie)
 	reg |= AFI_INTR_MASK_MSI_MASK;
 	afi_writel(pcie, reg, AFI_INTR_MASK);
 
+	pcie->bridge->msi = &msi->chip;
+
 	return 0;
 
 err:
@@ -2006,10 +2001,9 @@ retry:
 	return false;
 }
 
-static int tegra_pcie_enable(struct tegra_pcie *pcie)
+static void tegra_pcie_enable_ports(struct tegra_pcie *pcie)
 {
 	struct tegra_pcie_port *port, *tmp;
-	struct hw_pci hw;
 
 	list_for_each_entry_safe(port, tmp, &pcie->ports, list) {
 		dev_info(pcie->dev, "probing port %u, using %u lanes\n",
@@ -2025,22 +2019,6 @@ static int tegra_pcie_enable(struct tegra_pcie *pcie)
 		tegra_pcie_port_disable(port);
 		tegra_pcie_port_free(port);
 	}
-
-	memset(&hw, 0, sizeof(hw));
-
-#ifdef CONFIG_PCI_MSI
-	hw.msi_ctrl = &pcie->msi.chip;
-#endif
-
-	hw.nr_controllers = 1;
-	hw.private_data = (void **)&pcie;
-	hw.setup = tegra_pcie_setup;
-	hw.map_irq = tegra_pcie_map_irq;
-	hw.ops = &tegra_pcie_ops;
-
-	pci_common_init_dev(pcie->dev, &hw);
-
-	return 0;
 }
 
 static const struct tegra_pcie_soc tegra20_pcie = {
@@ -2201,13 +2179,18 @@ remove:
 
 static int tegra_pcie_probe(struct platform_device *pdev)
 {
+	struct pci_host_bridge *bridge;
 	struct tegra_pcie *pcie;
+	struct pci_bus *child;
 	int err;
 
-	pcie = devm_kzalloc(&pdev->dev, sizeof(*pcie), GFP_KERNEL);
-	if (!pcie)
+	bridge = pci_alloc_host_bridge(sizeof(*pcie));
+	if (!bridge)
 		return -ENOMEM;
 
+	pcie = bridge->private;
+	pcie->bridge = bridge;
+
 	pcie->soc = of_device_get_match_data(&pdev->dev);
 	INIT_LIST_HEAD(&pcie->buses);
 	INIT_LIST_HEAD(&pcie->ports);
@@ -2227,6 +2210,10 @@ static int tegra_pcie_probe(struct platform_device *pdev)
 	if (err)
 		goto put_resources;
 
+	err = tegra_pcie_request_resources(pcie);
+	if (err)
+		goto put_resources;
+
 	/* setup the AFI address translations */
 	tegra_pcie_setup_translations(pcie);
 
@@ -2240,12 +2227,30 @@ static int tegra_pcie_probe(struct platform_device *pdev)
 		}
 	}
 
-	err = tegra_pcie_enable(pcie);
+	tegra_pcie_enable_ports(pcie);
+
+	pci_add_flags(PCI_REASSIGN_ALL_RSRC | PCI_REASSIGN_ALL_BUS);
+	bridge->busnr = pcie->busn.start;
+	bridge->dev.parent = &pdev->dev;
+	bridge->ops = &tegra_pcie_ops;
+
+	err = pci_register_host_bridge(bridge);
 	if (err < 0) {
-		dev_err(&pdev->dev, "failed to enable PCIe ports: %d\n", err);
+		dev_err(&pdev->dev, "failed to register host: %d\n", err);
 		goto disable_msi;
 	}
 
+	pci_scan_child_bus(bridge->bus);
+
+	pci_fixup_irqs(pci_common_swizzle, tegra_pcie_map_irq);
+	pci_bus_size_bridges(bridge->bus);
+	pci_bus_assign_resources(bridge->bus);
+
+	list_for_each_entry(child, &bridge->bus->children, node)
+		pcie_bus_configure_settings(child);
+
+	pci_bus_add_devices(bridge->bus);
+
 	if (IS_ENABLED(CONFIG_DEBUG_FS)) {
 		err = tegra_pcie_debugfs_init(pcie);
 		if (err < 0)
-- 
2.9.0


WARNING: multiple messages have this Message-ID (diff)
From: Thierry Reding <thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
To: Bjorn Helgaas <bhelgaas-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
Cc: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>,
	Tomasz Nowicki <tn-nYOzD4b6Jr9Wk0Htik3J/w@public.gmane.org>,
	Liviu Dudau <liviu.dudau-5wv7dgnIgG8@public.gmane.org>,
	linux-pci-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Subject: [PATCH v3 4/4] PCI: tegra: Use new pci_register_host_bridge() interface
Date: Mon, 15 Aug 2016 17:36:47 +0200	[thread overview]
Message-ID: <20160815153647.1075-4-thierry.reding@gmail.com> (raw)
In-Reply-To: <20160815153647.1075-1-thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

From: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>

Tegra is one of the remaining platforms that still use the traditional
pci_common_init_dev() interface for probing PCI host bridges.

This demonstrates how to convert it to the pci_register_host interface
I just added in a previous patch. This leads to a more linear probe
sequence that can handle errors better because we avoid callbacks into
the driver, and it makes the driver architecture independent.

As a side note, I should mention that I noticed this driver does not
register any IORESOURCE_IO resource with the bus, but instead registers
the I/O port window as a memory resource, which is surely a bug.

Signed-off-by: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>
Signed-off-by: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
 drivers/pci/host/pci-tegra.c | 99 +++++++++++++++++++++++---------------------
 1 file changed, 52 insertions(+), 47 deletions(-)

diff --git a/drivers/pci/host/pci-tegra.c b/drivers/pci/host/pci-tegra.c
index 2d520755b1d7..6737d1be9798 100644
--- a/drivers/pci/host/pci-tegra.c
+++ b/drivers/pci/host/pci-tegra.c
@@ -260,6 +260,7 @@ static inline struct tegra_msi *to_tegra_msi(struct msi_controller *chip)
 }
 
 struct tegra_pcie {
+	struct pci_host_bridge *bridge;
 	struct device *dev;
 
 	void __iomem *pads;
@@ -322,11 +323,6 @@ struct tegra_pcie_bus {
 	unsigned int nr;
 };
 
-static inline struct tegra_pcie *sys_to_pcie(struct pci_sys_data *sys)
-{
-	return sys->private_data;
-}
-
 static inline void afi_writel(struct tegra_pcie *pcie, u32 value,
 			      unsigned long offset)
 {
@@ -430,7 +426,8 @@ free:
 
 static int tegra_pcie_add_bus(struct pci_bus *bus)
 {
-	struct tegra_pcie *pcie = sys_to_pcie(bus->sysdata);
+	struct pci_host_bridge *host = pci_find_host_bridge(bus);
+	struct tegra_pcie *pcie = host->private;
 	struct tegra_pcie_bus *b;
 
 	b = tegra_pcie_bus_alloc(pcie, bus->number);
@@ -444,7 +441,8 @@ static int tegra_pcie_add_bus(struct pci_bus *bus)
 
 static void tegra_pcie_remove_bus(struct pci_bus *child)
 {
-	struct tegra_pcie *pcie = sys_to_pcie(child->sysdata);
+	struct pci_host_bridge *host = pci_find_host_bridge(child);
+	struct tegra_pcie *pcie = host->private;
 	struct tegra_pcie_bus *bus, *tmp;
 
 	list_for_each_entry_safe(bus, tmp, &pcie->buses, list) {
@@ -461,7 +459,8 @@ static void __iomem *tegra_pcie_map_bus(struct pci_bus *bus,
 					unsigned int devfn,
 					int where)
 {
-	struct tegra_pcie *pcie = sys_to_pcie(bus->sysdata);
+	struct pci_host_bridge *host = pci_find_host_bridge(bus);
+	struct tegra_pcie *pcie = host->private;
 	void __iomem *addr = NULL;
 
 	if (bus->number == 0) {
@@ -609,35 +608,29 @@ static void tegra_pcie_relax_enable(struct pci_dev *dev)
 }
 DECLARE_PCI_FIXUP_FINAL(PCI_ANY_ID, PCI_ANY_ID, tegra_pcie_relax_enable);
 
-static int tegra_pcie_setup(int nr, struct pci_sys_data *sys)
+static int tegra_pcie_request_resources(struct tegra_pcie *pcie)
 {
-	struct tegra_pcie *pcie = sys_to_pcie(sys);
+	struct list_head *windows = &pcie->bridge->windows;
 	int err;
 
-	sys->mem_offset = pcie->offset.mem;
-	sys->io_offset = pcie->offset.io;
-
-	err = devm_request_resource(pcie->dev, &iomem_resource, &pcie->io);
-	if (err < 0)
-		return err;
-
-	pci_add_resource_offset(&sys->resources, &pcie->pio, sys->io_offset);
-	pci_add_resource_offset(&sys->resources, &pcie->mem, sys->mem_offset);
-	pci_add_resource_offset(&sys->resources, &pcie->prefetch,
-				sys->mem_offset);
-	pci_add_resource(&sys->resources, &pcie->busn);
+	pci_add_resource_offset(windows, &pcie->pio, pcie->offset.io);
+	pci_add_resource_offset(windows, &pcie->mem, pcie->offset.mem);
+	pci_add_resource_offset(windows, &pcie->prefetch, pcie->offset.mem);
+	pci_add_resource(windows, &pcie->busn);
 
-	err = devm_request_pci_bus_resources(pcie->dev, &sys->resources);
+	err = devm_request_pci_bus_resources(pcie->dev, windows);
 	if (err < 0)
 		return err;
 
 	pci_remap_iospace(&pcie->pio, pcie->io.start);
-	return 1;
+
+	return 0;
 }
 
 static int tegra_pcie_map_irq(const struct pci_dev *pdev, u8 slot, u8 pin)
 {
-	struct tegra_pcie *pcie = sys_to_pcie(pdev->bus->sysdata);
+	struct pci_host_bridge *host = pci_find_host_bridge(pdev->bus);
+	struct tegra_pcie *pcie = host->private;
 	int irq;
 
 	tegra_cpuidle_pcie_irqs_in_use();
@@ -1544,6 +1537,8 @@ static int tegra_pcie_enable_msi(struct tegra_pcie *pcie)
 	reg |= AFI_INTR_MASK_MSI_MASK;
 	afi_writel(pcie, reg, AFI_INTR_MASK);
 
+	pcie->bridge->msi = &msi->chip;
+
 	return 0;
 
 err:
@@ -2006,10 +2001,9 @@ retry:
 	return false;
 }
 
-static int tegra_pcie_enable(struct tegra_pcie *pcie)
+static void tegra_pcie_enable_ports(struct tegra_pcie *pcie)
 {
 	struct tegra_pcie_port *port, *tmp;
-	struct hw_pci hw;
 
 	list_for_each_entry_safe(port, tmp, &pcie->ports, list) {
 		dev_info(pcie->dev, "probing port %u, using %u lanes\n",
@@ -2025,22 +2019,6 @@ static int tegra_pcie_enable(struct tegra_pcie *pcie)
 		tegra_pcie_port_disable(port);
 		tegra_pcie_port_free(port);
 	}
-
-	memset(&hw, 0, sizeof(hw));
-
-#ifdef CONFIG_PCI_MSI
-	hw.msi_ctrl = &pcie->msi.chip;
-#endif
-
-	hw.nr_controllers = 1;
-	hw.private_data = (void **)&pcie;
-	hw.setup = tegra_pcie_setup;
-	hw.map_irq = tegra_pcie_map_irq;
-	hw.ops = &tegra_pcie_ops;
-
-	pci_common_init_dev(pcie->dev, &hw);
-
-	return 0;
 }
 
 static const struct tegra_pcie_soc tegra20_pcie = {
@@ -2201,13 +2179,18 @@ remove:
 
 static int tegra_pcie_probe(struct platform_device *pdev)
 {
+	struct pci_host_bridge *bridge;
 	struct tegra_pcie *pcie;
+	struct pci_bus *child;
 	int err;
 
-	pcie = devm_kzalloc(&pdev->dev, sizeof(*pcie), GFP_KERNEL);
-	if (!pcie)
+	bridge = pci_alloc_host_bridge(sizeof(*pcie));
+	if (!bridge)
 		return -ENOMEM;
 
+	pcie = bridge->private;
+	pcie->bridge = bridge;
+
 	pcie->soc = of_device_get_match_data(&pdev->dev);
 	INIT_LIST_HEAD(&pcie->buses);
 	INIT_LIST_HEAD(&pcie->ports);
@@ -2227,6 +2210,10 @@ static int tegra_pcie_probe(struct platform_device *pdev)
 	if (err)
 		goto put_resources;
 
+	err = tegra_pcie_request_resources(pcie);
+	if (err)
+		goto put_resources;
+
 	/* setup the AFI address translations */
 	tegra_pcie_setup_translations(pcie);
 
@@ -2240,12 +2227,30 @@ static int tegra_pcie_probe(struct platform_device *pdev)
 		}
 	}
 
-	err = tegra_pcie_enable(pcie);
+	tegra_pcie_enable_ports(pcie);
+
+	pci_add_flags(PCI_REASSIGN_ALL_RSRC | PCI_REASSIGN_ALL_BUS);
+	bridge->busnr = pcie->busn.start;
+	bridge->dev.parent = &pdev->dev;
+	bridge->ops = &tegra_pcie_ops;
+
+	err = pci_register_host_bridge(bridge);
 	if (err < 0) {
-		dev_err(&pdev->dev, "failed to enable PCIe ports: %d\n", err);
+		dev_err(&pdev->dev, "failed to register host: %d\n", err);
 		goto disable_msi;
 	}
 
+	pci_scan_child_bus(bridge->bus);
+
+	pci_fixup_irqs(pci_common_swizzle, tegra_pcie_map_irq);
+	pci_bus_size_bridges(bridge->bus);
+	pci_bus_assign_resources(bridge->bus);
+
+	list_for_each_entry(child, &bridge->bus->children, node)
+		pcie_bus_configure_settings(child);
+
+	pci_bus_add_devices(bridge->bus);
+
 	if (IS_ENABLED(CONFIG_DEBUG_FS)) {
 		err = tegra_pcie_debugfs_init(pcie);
 		if (err < 0)
-- 
2.9.0

  parent reply	other threads:[~2016-08-15 15:37 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-08-15 15:36 [PATCH v3 1/4] PCI: Add new method for registering PCI hosts Thierry Reding
2016-08-15 15:36 ` Thierry Reding
2016-08-15 15:36 ` [PATCH v3 2/4] PCI: Allow driver-specific data in host bridge Thierry Reding
2016-08-15 15:36   ` Thierry Reding
2016-08-19 16:55   ` Lorenzo Pieralisi
2016-08-19 16:55     ` Lorenzo Pieralisi
2016-08-22 14:00     ` Arnd Bergmann
2016-08-22 14:00       ` Arnd Bergmann
2016-08-23 13:59       ` Lorenzo Pieralisi
2016-08-23 13:59         ` Lorenzo Pieralisi
2016-11-25  7:26       ` Thierry Reding
2016-11-25  7:26         ` Thierry Reding
2016-11-25  9:53         ` Arnd Bergmann
2016-11-25  9:53           ` Arnd Bergmann
2016-08-15 15:36 ` [PATCH v3 3/4] PCI: Make host bridge interface publicly available Thierry Reding
2016-08-15 15:36   ` Thierry Reding
2016-08-15 15:36 ` Thierry Reding [this message]
2016-08-15 15:36   ` [PATCH v3 4/4] PCI: tegra: Use new pci_register_host_bridge() interface Thierry Reding
2016-08-19 17:13   ` Lorenzo Pieralisi
2016-08-19 17:13     ` Lorenzo Pieralisi
2016-08-22 13:50     ` Arnd Bergmann
2016-08-22 13:50       ` Arnd Bergmann
2016-08-23 14:01       ` Lorenzo Pieralisi
2016-08-23 14:01         ` Lorenzo Pieralisi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160815153647.1075-4-thierry.reding@gmail.com \
    --to=thierry.reding@gmail.com \
    --cc=arnd@arndb.de \
    --cc=bhelgaas@google.com \
    --cc=linux-pci@vger.kernel.org \
    --cc=linux-tegra@vger.kernel.org \
    --cc=liviu.dudau@arm.com \
    --cc=tn@semihalf.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.