linux-pci.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 1/2] resource: Add device-managed request/release_resource()
@ 2014-08-01 12:15 Thierry Reding
  2014-08-01 12:15 ` [PATCH v2 2/2] PCI: tegra: Implement a proper resource hierarchy Thierry Reding
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Thierry Reding @ 2014-08-01 12:15 UTC (permalink / raw)
  To: Bjorn Helgaas, Tejun Heo
  Cc: Stephen Warren, linux-pci, linux-tegra, linux-kernel

From: Thierry Reding <treding@nvidia.com>

Provide device-managed implementations of the request_resource() and
release_resource() functions. Upon failure to request a resource, the
new devm_request_resource() function will output an error message for
consistent error reporting.

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
Changes in v2:
- use more natural flow for error handling
- don't needlessly check for NULL pointers

 Documentation/driver-model/devres.txt |  2 +
 include/linux/ioport.h                |  5 +++
 kernel/resource.c                     | 70 +++++++++++++++++++++++++++++++++++
 3 files changed, 77 insertions(+)

diff --git a/Documentation/driver-model/devres.txt b/Documentation/driver-model/devres.txt
index d14710b04439..befc3fe12ba6 100644
--- a/Documentation/driver-model/devres.txt
+++ b/Documentation/driver-model/devres.txt
@@ -264,8 +264,10 @@ IIO
 IO region
   devm_release_mem_region()
   devm_release_region()
+  devm_release_resource()
   devm_request_mem_region()
   devm_request_region()
+  devm_request_resource()
 
 IOMAP
   devm_ioport_map()
diff --git a/include/linux/ioport.h b/include/linux/ioport.h
index 142ec544167c..2c5250222278 100644
--- a/include/linux/ioport.h
+++ b/include/linux/ioport.h
@@ -215,6 +215,11 @@ static inline int __deprecated check_region(resource_size_t s,
 
 /* Wrappers for managed devices */
 struct device;
+
+extern int devm_request_resource(struct device *dev, struct resource *root,
+				 struct resource *new);
+extern void devm_release_resource(struct device *dev, struct resource *new);
+
 #define devm_request_region(dev,start,n,name) \
 	__devm_request_region(dev, &ioport_resource, (start), (n), (name))
 #define devm_request_mem_region(dev,start,n,name) \
diff --git a/kernel/resource.c b/kernel/resource.c
index da14b8d09296..ca24f19f9d18 100644
--- a/kernel/resource.c
+++ b/kernel/resource.c
@@ -1248,6 +1248,76 @@ int release_mem_region_adjustable(struct resource *parent,
 /*
  * Managed region resource
  */
+static void devm_resource_release(struct device *dev, void *ptr)
+{
+	struct resource **r = ptr;
+
+	release_resource(*r);
+}
+
+/**
+ * devm_request_resource() - request and reserve an I/O or memory resource
+ * @dev: device for which to request the resource
+ * @root: root of the resource tree from which to request the resource
+ * @new: descriptor of the resource to request
+ *
+ * This is a device-managed version of request_resource(). There is usually
+ * no need to release resources requested by this function explicitly since
+ * that will be taken care of when the device is unbound from its driver.
+ * If for some reason the resource needs to be released explicitly, because
+ * of ordering issues for example, drivers must call devm_release_resource()
+ * rather than the regular release_resource().
+ *
+ * When a conflict is detected between any existing resources and the newly
+ * requested resource, an error message will be printed.
+ *
+ * Returns 0 on success or a negative error code on failure.
+ */
+int devm_request_resource(struct device *dev, struct resource *root,
+			  struct resource *new)
+{
+	struct resource *conflict, **ptr;
+
+	ptr = devres_alloc(devm_resource_release, sizeof(*ptr), GFP_KERNEL);
+	if (!ptr)
+		return -ENOMEM;
+
+	*ptr = new;
+
+	conflict = request_resource_conflict(root, new);
+	if (conflict) {
+		dev_err(dev, "resource collision: %pR conflicts with %s %pR\n",
+			new, conflict->name, conflict);
+		devres_free(ptr);
+		return -EBUSY;
+	}
+
+	devres_add(dev, ptr);
+	return 0;
+}
+EXPORT_SYMBOL(devm_request_resource);
+
+static int devm_resource_match(struct device *dev, void *res, void *data)
+{
+	struct resource **ptr = res;
+
+	return *ptr == data;
+}
+
+/**
+ * devm_release_resource() - release a previously requested resource
+ * @dev: device for which to release the resource
+ * @new: descriptor of the resource to release
+ *
+ * Releases a resource previously requested using devm_request_resource().
+ */
+void devm_release_resource(struct device *dev, struct resource *new)
+{
+	WARN_ON(devres_release(dev, devm_resource_release, devm_resource_match,
+			       new));
+}
+EXPORT_SYMBOL(devm_release_resource);
+
 struct region_devres {
 	struct resource *parent;
 	resource_size_t start;
-- 
2.0.3


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v2 2/2] PCI: tegra: Implement a proper resource hierarchy
  2014-08-01 12:15 [PATCH v2 1/2] resource: Add device-managed request/release_resource() Thierry Reding
@ 2014-08-01 12:15 ` Thierry Reding
  2014-08-01 12:55 ` [PATCH v2 1/2] resource: Add device-managed request/release_resource() Tejun Heo
  2014-09-04 20:53 ` Bjorn Helgaas
  2 siblings, 0 replies; 6+ messages in thread
From: Thierry Reding @ 2014-08-01 12:15 UTC (permalink / raw)
  To: Bjorn Helgaas, Tejun Heo
  Cc: Stephen Warren, linux-pci, linux-tegra, linux-kernel

From: Thierry Reding <treding@nvidia.com>

Currently the resource hierarchy generated from the PCIe host bridge is
completely flat:

	$ cat /proc/iomem
	00000000-00000fff : /pcie-controller@00003000/pci@1,0
	00003000-000037ff : pads
	00003800-000039ff : afi
	10000000-1fffffff : cs
	28000000-28003fff : r8169
	28004000-28004fff : r8169
	...

The host bridge driver doesn't request all the resources that are used.
Windows allocated to each of the root ports aren't tracked, so there is
no way for resources allocated to individual devices to be matched up
with the correct parent resource by the PCI core.

This patch addresses this in two steps. It first takes the union of all
regions associated with the PCIe host bridge (control registers, root
port registers, configuration space, I/O and prefetchable as well as
non-prefetchable memory regions) and uses it as the new root of the
resource hierarchy.

Subsequently, regions are allocated from within this new root resource
so that the resource tree looks much more like what's expected:

	# cat /proc/iomem
	00000000-3fffffff : /pcie-controller@00003000
	  00000000-00000fff : /pcie-controller@00003000/pci@1,0
	  00003000-000037ff : pads
	  00003800-000039ff : afi
	  10000000-1fffffff : cs
	  20000000-27ffffff : non-prefetchable
	  28000000-3fffffff : prefetchable
	    28000000-280fffff : PCI Bus 0000:01
	      28000000-28003fff : 0000:01:00.0
	        28000000-28003fff : r8169
	      28004000-28004fff : 0000:01:00.0
	        28004000-28004fff : r8169
	...

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 drivers/pci/host/pci-tegra.c | 32 +++++++++++++++++++++++++++++---
 1 file changed, 29 insertions(+), 3 deletions(-)

diff --git a/drivers/pci/host/pci-tegra.c b/drivers/pci/host/pci-tegra.c
index 0fb0fdb223d5..83fa33e16650 100644
--- a/drivers/pci/host/pci-tegra.c
+++ b/drivers/pci/host/pci-tegra.c
@@ -253,6 +253,7 @@ struct tegra_pcie {
 	struct list_head buses;
 	struct resource *cs;
 
+	struct resource all;
 	struct resource io;
 	struct resource mem;
 	struct resource prefetch;
@@ -626,6 +627,15 @@ 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)
 {
 	struct tegra_pcie *pcie = sys_to_pcie(sys);
+	int err;
+
+	err = devm_request_resource(pcie->dev, &pcie->all, &pcie->mem);
+	if (err < 0)
+		return err;
+
+	err = devm_request_resource(pcie->dev, &pcie->all, &pcie->prefetch);
+	if (err)
+		return err;
 
 	pci_add_resource_offset(&sys->resources, &pcie->mem, sys->mem_offset);
 	pci_add_resource_offset(&sys->resources, &pcie->prefetch,
@@ -1514,6 +1524,12 @@ static int tegra_pcie_parse_dt(struct tegra_pcie *pcie)
 	struct resource res;
 	int err;
 
+	memset(&pcie->all, 0, sizeof(pcie->all));
+	pcie->all.flags = IORESOURCE_MEM;
+	pcie->all.name = np->full_name;
+	pcie->all.start = ~0;
+	pcie->all.end = 0;
+
 	if (of_pci_range_parser_init(&parser, np)) {
 		dev_err(pcie->dev, "missing \"ranges\" property\n");
 		return -EINVAL;
@@ -1525,21 +1541,31 @@ static int tegra_pcie_parse_dt(struct tegra_pcie *pcie)
 		switch (res.flags & IORESOURCE_TYPE_BITS) {
 		case IORESOURCE_IO:
 			memcpy(&pcie->io, &res, sizeof(res));
-			pcie->io.name = "I/O";
+			pcie->io.name = np->full_name;
 			break;
 
 		case IORESOURCE_MEM:
 			if (res.flags & IORESOURCE_PREFETCH) {
 				memcpy(&pcie->prefetch, &res, sizeof(res));
-				pcie->prefetch.name = "PREFETCH";
+				pcie->prefetch.name = "prefetchable";
 			} else {
 				memcpy(&pcie->mem, &res, sizeof(res));
-				pcie->mem.name = "MEM";
+				pcie->mem.name = "non-prefetchable";
 			}
 			break;
 		}
+
+		if (res.start <= pcie->all.start)
+			pcie->all.start = res.start;
+
+		if (res.end >= pcie->all.end)
+			pcie->all.end = res.end;
 	}
 
+	err = devm_request_resource(pcie->dev, &iomem_resource, &pcie->all);
+	if (err < 0)
+		return err;
+
 	err = of_pci_parse_bus_range(np, &pcie->busn);
 	if (err < 0) {
 		dev_err(pcie->dev, "failed to parse ranges property: %d\n",
-- 
2.0.3


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 1/2] resource: Add device-managed request/release_resource()
  2014-08-01 12:15 [PATCH v2 1/2] resource: Add device-managed request/release_resource() Thierry Reding
  2014-08-01 12:15 ` [PATCH v2 2/2] PCI: tegra: Implement a proper resource hierarchy Thierry Reding
@ 2014-08-01 12:55 ` Tejun Heo
  2014-08-01 13:55   ` Thierry Reding
  2014-09-04 20:53 ` Bjorn Helgaas
  2 siblings, 1 reply; 6+ messages in thread
From: Tejun Heo @ 2014-08-01 12:55 UTC (permalink / raw)
  To: Thierry Reding
  Cc: Bjorn Helgaas, Stephen Warren, linux-pci, linux-tegra, linux-kernel

On Fri, Aug 01, 2014 at 02:15:10PM +0200, Thierry Reding wrote:
> From: Thierry Reding <treding@nvidia.com>
> 
> Provide device-managed implementations of the request_resource() and
> release_resource() functions. Upon failure to request a resource, the
> new devm_request_resource() function will output an error message for
> consistent error reporting.
> 
> Signed-off-by: Thierry Reding <treding@nvidia.com>

Acked-by: Tejun Heo <tj@kernel.org>

But please also update Documentation/driver-model/devres.txt.

Thanks.

-- 
tejun

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 1/2] resource: Add device-managed request/release_resource()
  2014-08-01 12:55 ` [PATCH v2 1/2] resource: Add device-managed request/release_resource() Tejun Heo
@ 2014-08-01 13:55   ` Thierry Reding
  2014-08-01 13:58     ` Tejun Heo
  0 siblings, 1 reply; 6+ messages in thread
From: Thierry Reding @ 2014-08-01 13:55 UTC (permalink / raw)
  To: Tejun Heo
  Cc: Bjorn Helgaas, Stephen Warren, linux-pci, linux-tegra, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 1186 bytes --]

On Fri, Aug 01, 2014 at 08:55:22AM -0400, Tejun Heo wrote:
> On Fri, Aug 01, 2014 at 02:15:10PM +0200, Thierry Reding wrote:
> > From: Thierry Reding <treding@nvidia.com>
> > 
> > Provide device-managed implementations of the request_resource() and
> > release_resource() functions. Upon failure to request a resource, the
> > new devm_request_resource() function will output an error message for
> > consistent error reporting.
> > 
> > Signed-off-by: Thierry Reding <treding@nvidia.com>
> 
> Acked-by: Tejun Heo <tj@kernel.org>
> 
> But please also update Documentation/driver-model/devres.txt.

The patch already contains the below hunk. Do you expect anything more
to be added to that file?

Thierry

diff --git a/Documentation/driver-model/devres.txt b/Documentation/driver-model/devres.txt
index d14710b04439..befc3fe12ba6 100644
--- a/Documentation/driver-model/devres.txt
+++ b/Documentation/driver-model/devres.txt
@@ -264,8 +264,10 @@ IIO
 IO region
   devm_release_mem_region()
   devm_release_region()
+  devm_release_resource()
   devm_request_mem_region()
   devm_request_region()
+  devm_request_resource()

 IOMAP
   devm_ioport_map()

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 1/2] resource: Add device-managed request/release_resource()
  2014-08-01 13:55   ` Thierry Reding
@ 2014-08-01 13:58     ` Tejun Heo
  0 siblings, 0 replies; 6+ messages in thread
From: Tejun Heo @ 2014-08-01 13:58 UTC (permalink / raw)
  To: Thierry Reding
  Cc: Bjorn Helgaas, Stephen Warren, linux-pci, linux-tegra, linux-kernel

On Fri, Aug 01, 2014 at 03:55:42PM +0200, Thierry Reding wrote:
> On Fri, Aug 01, 2014 at 08:55:22AM -0400, Tejun Heo wrote:
> > On Fri, Aug 01, 2014 at 02:15:10PM +0200, Thierry Reding wrote:
> > > From: Thierry Reding <treding@nvidia.com>
> > > 
> > > Provide device-managed implementations of the request_resource() and
> > > release_resource() functions. Upon failure to request a resource, the
> > > new devm_request_resource() function will output an error message for
> > > consistent error reporting.
> > > 
> > > Signed-off-by: Thierry Reding <treding@nvidia.com>
> > 
> > Acked-by: Tejun Heo <tj@kernel.org>
> > 
> > But please also update Documentation/driver-model/devres.txt.
> 
> The patch already contains the below hunk. Do you expect anything more
> to be added to that file?

Oops, I missed that.  All look fine to me.

Thanks.

-- 
tejun

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 1/2] resource: Add device-managed request/release_resource()
  2014-08-01 12:15 [PATCH v2 1/2] resource: Add device-managed request/release_resource() Thierry Reding
  2014-08-01 12:15 ` [PATCH v2 2/2] PCI: tegra: Implement a proper resource hierarchy Thierry Reding
  2014-08-01 12:55 ` [PATCH v2 1/2] resource: Add device-managed request/release_resource() Tejun Heo
@ 2014-09-04 20:53 ` Bjorn Helgaas
  2 siblings, 0 replies; 6+ messages in thread
From: Bjorn Helgaas @ 2014-09-04 20:53 UTC (permalink / raw)
  To: Thierry Reding
  Cc: Tejun Heo, Stephen Warren, linux-pci, linux-tegra, linux-kernel

On Fri, Aug 01, 2014 at 02:15:10PM +0200, Thierry Reding wrote:
> From: Thierry Reding <treding@nvidia.com>
> 
> Provide device-managed implementations of the request_resource() and
> release_resource() functions. Upon failure to request a resource, the
> new devm_request_resource() function will output an error message for
> consistent error reporting.
> 
> Signed-off-by: Thierry Reding <treding@nvidia.com>

Applied both, with Tejun's ack on the first, for v3.18, thanks!

> ---
> Changes in v2:
> - use more natural flow for error handling
> - don't needlessly check for NULL pointers
> 
>  Documentation/driver-model/devres.txt |  2 +
>  include/linux/ioport.h                |  5 +++
>  kernel/resource.c                     | 70 +++++++++++++++++++++++++++++++++++
>  3 files changed, 77 insertions(+)
> 
> diff --git a/Documentation/driver-model/devres.txt b/Documentation/driver-model/devres.txt
> index d14710b04439..befc3fe12ba6 100644
> --- a/Documentation/driver-model/devres.txt
> +++ b/Documentation/driver-model/devres.txt
> @@ -264,8 +264,10 @@ IIO
>  IO region
>    devm_release_mem_region()
>    devm_release_region()
> +  devm_release_resource()
>    devm_request_mem_region()
>    devm_request_region()
> +  devm_request_resource()
>  
>  IOMAP
>    devm_ioport_map()
> diff --git a/include/linux/ioport.h b/include/linux/ioport.h
> index 142ec544167c..2c5250222278 100644
> --- a/include/linux/ioport.h
> +++ b/include/linux/ioport.h
> @@ -215,6 +215,11 @@ static inline int __deprecated check_region(resource_size_t s,
>  
>  /* Wrappers for managed devices */
>  struct device;
> +
> +extern int devm_request_resource(struct device *dev, struct resource *root,
> +				 struct resource *new);
> +extern void devm_release_resource(struct device *dev, struct resource *new);
> +
>  #define devm_request_region(dev,start,n,name) \
>  	__devm_request_region(dev, &ioport_resource, (start), (n), (name))
>  #define devm_request_mem_region(dev,start,n,name) \
> diff --git a/kernel/resource.c b/kernel/resource.c
> index da14b8d09296..ca24f19f9d18 100644
> --- a/kernel/resource.c
> +++ b/kernel/resource.c
> @@ -1248,6 +1248,76 @@ int release_mem_region_adjustable(struct resource *parent,
>  /*
>   * Managed region resource
>   */
> +static void devm_resource_release(struct device *dev, void *ptr)
> +{
> +	struct resource **r = ptr;
> +
> +	release_resource(*r);
> +}
> +
> +/**
> + * devm_request_resource() - request and reserve an I/O or memory resource
> + * @dev: device for which to request the resource
> + * @root: root of the resource tree from which to request the resource
> + * @new: descriptor of the resource to request
> + *
> + * This is a device-managed version of request_resource(). There is usually
> + * no need to release resources requested by this function explicitly since
> + * that will be taken care of when the device is unbound from its driver.
> + * If for some reason the resource needs to be released explicitly, because
> + * of ordering issues for example, drivers must call devm_release_resource()
> + * rather than the regular release_resource().
> + *
> + * When a conflict is detected between any existing resources and the newly
> + * requested resource, an error message will be printed.
> + *
> + * Returns 0 on success or a negative error code on failure.
> + */
> +int devm_request_resource(struct device *dev, struct resource *root,
> +			  struct resource *new)
> +{
> +	struct resource *conflict, **ptr;
> +
> +	ptr = devres_alloc(devm_resource_release, sizeof(*ptr), GFP_KERNEL);
> +	if (!ptr)
> +		return -ENOMEM;
> +
> +	*ptr = new;
> +
> +	conflict = request_resource_conflict(root, new);
> +	if (conflict) {
> +		dev_err(dev, "resource collision: %pR conflicts with %s %pR\n",
> +			new, conflict->name, conflict);
> +		devres_free(ptr);
> +		return -EBUSY;
> +	}
> +
> +	devres_add(dev, ptr);
> +	return 0;
> +}
> +EXPORT_SYMBOL(devm_request_resource);
> +
> +static int devm_resource_match(struct device *dev, void *res, void *data)
> +{
> +	struct resource **ptr = res;
> +
> +	return *ptr == data;
> +}
> +
> +/**
> + * devm_release_resource() - release a previously requested resource
> + * @dev: device for which to release the resource
> + * @new: descriptor of the resource to release
> + *
> + * Releases a resource previously requested using devm_request_resource().
> + */
> +void devm_release_resource(struct device *dev, struct resource *new)
> +{
> +	WARN_ON(devres_release(dev, devm_resource_release, devm_resource_match,
> +			       new));
> +}
> +EXPORT_SYMBOL(devm_release_resource);
> +
>  struct region_devres {
>  	struct resource *parent;
>  	resource_size_t start;
> -- 
> 2.0.3
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2014-09-04 20:53 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-08-01 12:15 [PATCH v2 1/2] resource: Add device-managed request/release_resource() Thierry Reding
2014-08-01 12:15 ` [PATCH v2 2/2] PCI: tegra: Implement a proper resource hierarchy Thierry Reding
2014-08-01 12:55 ` [PATCH v2 1/2] resource: Add device-managed request/release_resource() Tejun Heo
2014-08-01 13:55   ` Thierry Reding
2014-08-01 13:58     ` Tejun Heo
2014-09-04 20:53 ` Bjorn Helgaas

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).