linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/2] lib: devres: provide devm_iounremap_resource()
@ 2020-09-20 11:38 pierre Kuo
  2020-09-20 11:38 ` [PATCH 2/2] driver core: platform: provide devm_platform_iounremap_resource pierre Kuo
  0 siblings, 1 reply; 8+ messages in thread
From: pierre Kuo @ 2020-09-20 11:38 UTC (permalink / raw)
  To: gregkh, rafael; +Cc: linux-kernel, pierre Kuo

Driver doesn't have a single helper function to release memroy
allocated by devm_ioremap_resource(). That mean it needs respectively
to call devm_release_mem_region() and devm_iounmap() for memory release.

This patch creates a helper, devm_iounremap_resource(), to combine above
operations.

Signed-off-by: pierre Kuo <vichy.kuo@gmail.com>
---
 include/linux/device.h |  2 ++
 lib/devres.c           | 25 +++++++++++++++++++++++++
 2 files changed, 27 insertions(+)

diff --git a/include/linux/device.h b/include/linux/device.h
index 9e6ea8931a52..33ec7e54c1a9 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -240,6 +240,8 @@ void devm_free_pages(struct device *dev, unsigned long addr);
 
 void __iomem *devm_ioremap_resource(struct device *dev,
 				    const struct resource *res);
+void devm_iounremap_resource(struct device *dev,
+			     const struct resource *res, void __iomem *addr);
 void __iomem *devm_ioremap_resource_wc(struct device *dev,
 				       const struct resource *res);
 
diff --git a/lib/devres.c b/lib/devres.c
index ebb1573d9ae3..cdda0cd0a263 100644
--- a/lib/devres.c
+++ b/lib/devres.c
@@ -113,6 +113,31 @@ void devm_iounmap(struct device *dev, void __iomem *addr)
 }
 EXPORT_SYMBOL(devm_iounmap);
 
+/**
+ * devm_iounremap_resource() - release mem region, and unremap address
+ * @dev: generic device to handle the resource for
+ * @res: resource of mem region to be release
+ * @addr: address to unmap
+ *
+ * Release memory region and unmap address.
+ */
+void devm_iounremap_resource(struct device *dev,
+			     const struct resource *res, void __iomem *addr)
+{
+	resource_size_t size;
+
+	BUG_ON(!dev);
+	if (!res || resource_type(res) != IORESOURCE_MEM) {
+		dev_err(dev, "invalid resource\n");
+		return;
+	}
+
+	size = resource_size(res);
+	devm_release_mem_region(dev, res->start, size);
+	devm_iounmap(dev, addr);
+}
+EXPORT_SYMBOL(devm_iounremap_resource);
+
 static void __iomem *
 __devm_ioremap_resource(struct device *dev, const struct resource *res,
 			enum devm_ioremap_type type)
-- 
2.17.1


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

* [PATCH 2/2] driver core: platform: provide devm_platform_iounremap_resource
  2020-09-20 11:38 [PATCH 1/2] lib: devres: provide devm_iounremap_resource() pierre Kuo
@ 2020-09-20 11:38 ` pierre Kuo
  2020-09-29  6:21   ` pierre kuo
  0 siblings, 1 reply; 8+ messages in thread
From: pierre Kuo @ 2020-09-20 11:38 UTC (permalink / raw)
  To: gregkh, rafael; +Cc: linux-kernel, pierre Kuo

Combine platform_get_resource() and devm_iounremap_resource() to release
the iomem allocated by devm_platform_get_and_ioremap_resource().

Signed-off-by: pierre Kuo <vichy.kuo@gmail.com>
---
 drivers/base/platform.c         | 24 ++++++++++++++++++++++++
 include/linux/platform_device.h |  4 ++++
 2 files changed, 28 insertions(+)

diff --git a/drivers/base/platform.c b/drivers/base/platform.c
index e5d8a0503b4f..e2655c00873f 100644
--- a/drivers/base/platform.c
+++ b/drivers/base/platform.c
@@ -84,6 +84,30 @@ devm_platform_get_and_ioremap_resource(struct platform_device *pdev,
 }
 EXPORT_SYMBOL_GPL(devm_platform_get_and_ioremap_resource);
 
+/**
+ * devm_platform_iounremap_resource - call devm_iounremap_resource() for a
+ *				      platform device with memory that addr points to.
+ *
+ * @pdev: platform device to use both for memory resource lookup as well as
+ *        resource management
+ * @index: resource index
+ * @addr: address to be unmap.
+ */
+void
+devm_platform_iounremap_resource(struct platform_device *pdev,
+				 unsigned int index, void __iomem *addr)
+{
+	struct resource *r;
+
+	r = platform_get_resource(pdev, IORESOURCE_MEM, index);
+	if (!r)
+		dev_err(&pdev->dev,
+			"MEM resource index %d not found\n", index);
+	else
+		devm_iounremap_resource(&pdev->dev, r, addr);
+}
+EXPORT_SYMBOL_GPL(devm_platform_iounremap_resource);
+
 /**
  * devm_platform_ioremap_resource - call devm_ioremap_resource() for a platform
  *				    device
diff --git a/include/linux/platform_device.h b/include/linux/platform_device.h
index 77a2aada106d..75da15937679 100644
--- a/include/linux/platform_device.h
+++ b/include/linux/platform_device.h
@@ -67,6 +67,10 @@ devm_platform_ioremap_resource_wc(struct platform_device *pdev,
 extern void __iomem *
 devm_platform_ioremap_resource_byname(struct platform_device *pdev,
 				      const char *name);
+extern void
+devm_platform_iounremap_resource(struct platform_device *pdev,
+				 unsigned int index,
+				 void __iomem *addr);
 extern int platform_get_irq(struct platform_device *, unsigned int);
 extern int platform_get_irq_optional(struct platform_device *, unsigned int);
 extern int platform_irq_count(struct platform_device *);
-- 
2.17.1


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

* Re: [PATCH 2/2] driver core: platform: provide devm_platform_iounremap_resource
  2020-09-20 11:38 ` [PATCH 2/2] driver core: platform: provide devm_platform_iounremap_resource pierre Kuo
@ 2020-09-29  6:21   ` pierre kuo
  2020-10-02 13:45     ` Greg KH
  0 siblings, 1 reply; 8+ messages in thread
From: pierre kuo @ 2020-09-29  6:21 UTC (permalink / raw)
  To: Greg KH, rafael; +Cc: Linux Kernel Mailing List

Hi Greg and Rafael:
Would you please help to review these 2 patches?

https://lkml.org/lkml/2020/9/20/112
https://lkml.org/lkml/2020/9/20/113

Appreciate ur help in advance.

>
> Combine platform_get_resource() and devm_iounremap_resource() to release
> the iomem allocated by devm_platform_get_and_ioremap_resource().
>
> Signed-off-by: pierre Kuo <vichy.kuo@gmail.com>
> ---
>  drivers/base/platform.c         | 24 ++++++++++++++++++++++++
>  include/linux/platform_device.h |  4 ++++
>  2 files changed, 28 insertions(+)
>
> diff --git a/drivers/base/platform.c b/drivers/base/platform.c
> index e5d8a0503b4f..e2655c00873f 100644
> --- a/drivers/base/platform.c
> +++ b/drivers/base/platform.c
> @@ -84,6 +84,30 @@ devm_platform_get_and_ioremap_resource(struct platform_device *pdev,
>  }
>  EXPORT_SYMBOL_GPL(devm_platform_get_and_ioremap_resource);
>
> +/**
> + * devm_platform_iounremap_resource - call devm_iounremap_resource() for a
> + *                                   platform device with memory that addr points to.
> + *
> + * @pdev: platform device to use both for memory resource lookup as well as
> + *        resource management
> + * @index: resource index
> + * @addr: address to be unmap.
> + */
> +void
> +devm_platform_iounremap_resource(struct platform_device *pdev,
> +                                unsigned int index, void __iomem *addr)
> +{
> +       struct resource *r;
> +
> +       r = platform_get_resource(pdev, IORESOURCE_MEM, index);
> +       if (!r)
> +               dev_err(&pdev->dev,
> +                       "MEM resource index %d not found\n", index);
> +       else
> +               devm_iounremap_resource(&pdev->dev, r, addr);
> +}
> +EXPORT_SYMBOL_GPL(devm_platform_iounremap_resource);
> +
>  /**
>   * devm_platform_ioremap_resource - call devm_ioremap_resource() for a platform
>   *                                 device
> diff --git a/include/linux/platform_device.h b/include/linux/platform_device.h
> index 77a2aada106d..75da15937679 100644
> --- a/include/linux/platform_device.h
> +++ b/include/linux/platform_device.h
> @@ -67,6 +67,10 @@ devm_platform_ioremap_resource_wc(struct platform_device *pdev,
>  extern void __iomem *
>  devm_platform_ioremap_resource_byname(struct platform_device *pdev,
>                                       const char *name);
> +extern void
> +devm_platform_iounremap_resource(struct platform_device *pdev,
> +                                unsigned int index,
> +                                void __iomem *addr);
>  extern int platform_get_irq(struct platform_device *, unsigned int);
>  extern int platform_get_irq_optional(struct platform_device *, unsigned int);
>  extern int platform_irq_count(struct platform_device *);
> --
> 2.17.1
>

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

* Re: [PATCH 2/2] driver core: platform: provide devm_platform_iounremap_resource
  2020-09-29  6:21   ` pierre kuo
@ 2020-10-02 13:45     ` Greg KH
  2020-10-04 16:21       ` pierre kuo
  0 siblings, 1 reply; 8+ messages in thread
From: Greg KH @ 2020-10-02 13:45 UTC (permalink / raw)
  To: pierre kuo; +Cc: rafael, Linux Kernel Mailing List

On Tue, Sep 29, 2020 at 02:21:37PM +0800, pierre kuo wrote:
> Hi Greg and Rafael:
> Would you please help to review these 2 patches?
> 
> https://lkml.org/lkml/2020/9/20/112
> https://lkml.org/lkml/2020/9/20/113

Please resend, I can't take patches off of a random web site.

Now lore.kernel.org I could take them from :)

thanks,

greg k-h

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

* Re: [PATCH 2/2] driver core: platform: provide devm_platform_iounremap_resource
  2020-10-02 13:45     ` Greg KH
@ 2020-10-04 16:21       ` pierre kuo
  2020-10-04 16:47         ` Greg KH
  0 siblings, 1 reply; 8+ messages in thread
From: pierre kuo @ 2020-10-04 16:21 UTC (permalink / raw)
  To: Greg KH; +Cc: rafael, Linux Kernel Mailing List

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

hi Greg:
> Please resend, I can't take patches off of a random web site.
> Now lore.kernel.org I could take them from :)

Please refer to the attachments and links on lore.kernel.org.

https://lore.kernel.org/lkml/20200920113808.22223-1-vichy.kuo@gmail.com
https://lore.kernel.org/lkml/20200920113808.22223-2-vichy.kuo@gmail.com

Appreciate your help,

[-- Attachment #2: 0001-lib-devres-provide-devm_iounremap_resource.patch --]
[-- Type: text/x-patch, Size: 2280 bytes --]

From b141d537904b71b802770d9c0fc3787b98c5cf71 Mon Sep 17 00:00:00 2001
From: pierre Kuo <vichy.kuo@gmail.com>
Date: Tue, 18 Aug 2020 23:05:00 +0800
Subject: [PATCH 1/2] lib: devres: provide devm_iounremap_resource()

Driver doesn't have a single helper function to release memroy
allocated by devm_ioremap_resource(). That mean it needs respectively
to call devm_release_mem_region() and devm_iounmap() for memory release.

This patch creates a helper, devm_iounremap_resource(), to combine above
operations.

Signed-off-by: pierre Kuo <vichy.kuo@gmail.com>
---
 include/linux/device.h |  2 ++
 lib/devres.c           | 25 +++++++++++++++++++++++++
 2 files changed, 27 insertions(+)

diff --git a/include/linux/device.h b/include/linux/device.h
index 9e6ea8931a52..33ec7e54c1a9 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -240,6 +240,8 @@ void devm_free_pages(struct device *dev, unsigned long addr);
 
 void __iomem *devm_ioremap_resource(struct device *dev,
 				    const struct resource *res);
+void devm_iounremap_resource(struct device *dev,
+			     const struct resource *res, void __iomem *addr);
 void __iomem *devm_ioremap_resource_wc(struct device *dev,
 				       const struct resource *res);
 
diff --git a/lib/devres.c b/lib/devres.c
index ebb1573d9ae3..cdda0cd0a263 100644
--- a/lib/devres.c
+++ b/lib/devres.c
@@ -113,6 +113,31 @@ void devm_iounmap(struct device *dev, void __iomem *addr)
 }
 EXPORT_SYMBOL(devm_iounmap);
 
+/**
+ * devm_iounremap_resource() - release mem region, and unremap address
+ * @dev: generic device to handle the resource for
+ * @res: resource of mem region to be release
+ * @addr: address to unmap
+ *
+ * Release memory region and unmap address.
+ */
+void devm_iounremap_resource(struct device *dev,
+			     const struct resource *res, void __iomem *addr)
+{
+	resource_size_t size;
+
+	BUG_ON(!dev);
+	if (!res || resource_type(res) != IORESOURCE_MEM) {
+		dev_err(dev, "invalid resource\n");
+		return;
+	}
+
+	size = resource_size(res);
+	devm_release_mem_region(dev, res->start, size);
+	devm_iounmap(dev, addr);
+}
+EXPORT_SYMBOL(devm_iounremap_resource);
+
 static void __iomem *
 __devm_ioremap_resource(struct device *dev, const struct resource *res,
 			enum devm_ioremap_type type)
-- 
2.17.1


[-- Attachment #3: 0002-driver-core-platform-provide-devm_platform_iounremap.patch --]
[-- Type: text/x-patch, Size: 2454 bytes --]

From 33afa315c3c941b303e9b3152552010ad266ebbf Mon Sep 17 00:00:00 2001
From: pierre Kuo <vichy.kuo@gmail.com>
Date: Wed, 19 Aug 2020 15:57:05 +0800
Subject: [PATCH 2/2] driver core: platform: provide
 devm_platform_iounremap_resource

Combine platform_get_resource() and devm_iounremap_resource() to release
the iomem allocated by devm_platform_get_and_ioremap_resource().

Signed-off-by: pierre Kuo <vichy.kuo@gmail.com>
---
 drivers/base/platform.c         | 24 ++++++++++++++++++++++++
 include/linux/platform_device.h |  4 ++++
 2 files changed, 28 insertions(+)

diff --git a/drivers/base/platform.c b/drivers/base/platform.c
index e5d8a0503b4f..e2655c00873f 100644
--- a/drivers/base/platform.c
+++ b/drivers/base/platform.c
@@ -84,6 +84,30 @@ devm_platform_get_and_ioremap_resource(struct platform_device *pdev,
 }
 EXPORT_SYMBOL_GPL(devm_platform_get_and_ioremap_resource);
 
+/**
+ * devm_platform_iounremap_resource - call devm_iounremap_resource() for a
+ *				      platform device with memory that addr points to.
+ *
+ * @pdev: platform device to use both for memory resource lookup as well as
+ *        resource management
+ * @index: resource index
+ * @addr: address to be unmap.
+ */
+void
+devm_platform_iounremap_resource(struct platform_device *pdev,
+				 unsigned int index, void __iomem *addr)
+{
+	struct resource *r;
+
+	r = platform_get_resource(pdev, IORESOURCE_MEM, index);
+	if (!r)
+		dev_err(&pdev->dev,
+			"MEM resource index %d not found\n", index);
+	else
+		devm_iounremap_resource(&pdev->dev, r, addr);
+}
+EXPORT_SYMBOL_GPL(devm_platform_iounremap_resource);
+
 /**
  * devm_platform_ioremap_resource - call devm_ioremap_resource() for a platform
  *				    device
diff --git a/include/linux/platform_device.h b/include/linux/platform_device.h
index 77a2aada106d..75da15937679 100644
--- a/include/linux/platform_device.h
+++ b/include/linux/platform_device.h
@@ -67,6 +67,10 @@ devm_platform_ioremap_resource_wc(struct platform_device *pdev,
 extern void __iomem *
 devm_platform_ioremap_resource_byname(struct platform_device *pdev,
 				      const char *name);
+extern void
+devm_platform_iounremap_resource(struct platform_device *pdev,
+				 unsigned int index,
+				 void __iomem *addr);
 extern int platform_get_irq(struct platform_device *, unsigned int);
 extern int platform_get_irq_optional(struct platform_device *, unsigned int);
 extern int platform_irq_count(struct platform_device *);
-- 
2.17.1


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

* Re: [PATCH 2/2] driver core: platform: provide devm_platform_iounremap_resource
  2020-10-04 16:21       ` pierre kuo
@ 2020-10-04 16:47         ` Greg KH
  2020-10-05 15:23           ` pierre kuo
  0 siblings, 1 reply; 8+ messages in thread
From: Greg KH @ 2020-10-04 16:47 UTC (permalink / raw)
  To: pierre kuo; +Cc: rafael, Linux Kernel Mailing List

On Mon, Oct 05, 2020 at 12:21:12AM +0800, pierre kuo wrote:
> hi Greg:
> > Please resend, I can't take patches off of a random web site.
> > Now lore.kernel.org I could take them from :)
> 
> Please refer to the attachments and links on lore.kernel.org.
> 
> https://lore.kernel.org/lkml/20200920113808.22223-1-vichy.kuo@gmail.com
> https://lore.kernel.org/lkml/20200920113808.22223-2-vichy.kuo@gmail.com

Why are you adding new functions but not actually calling them anywhere?
We don't like adding infrastructure that no one uses, that's just
wasteful.

Please redo the series and include some conversions as well, so that we
can see if these new functions are even needed or not.

thanks,

greg k-h

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

* Re: [PATCH 2/2] driver core: platform: provide devm_platform_iounremap_resource
  2020-10-04 16:47         ` Greg KH
@ 2020-10-05 15:23           ` pierre kuo
  2020-10-05 15:31             ` Greg KH
  0 siblings, 1 reply; 8+ messages in thread
From: pierre kuo @ 2020-10-05 15:23 UTC (permalink / raw)
  To: Greg KH; +Cc: rafael, Linux Kernel Mailing List

hi Greg:
> Why are you adding new functions but not actually calling them anywhere?

Below patch introduce a single helper, devm_platform_ioremap_resource,
which combines
platform_get_resource() and devm_ioremap_resource(). But there is no
single helper to release
those resources in driver removing stage.

https://lore.kernel.org/lkml/20190215152507.31066-2-brgl@bgdev.pl/

That means driver owner still need to call below (*) and (**) for
releasing resource.
Therefore, this patch adds a single release helper that can be paired with
devm_platform_ioremap_resource.

Appreciate ur kind help,

foo_probe(pdev)
{
    iomem = devm_platform_ioremap_resource(pdev, 0);
    ....
}


foo_remove(pdev)
{
    devm_iounmap(iomem);   (*)
    devm_release_mem_region(dev, res->start, size); (**)
   ........................
}

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

* Re: [PATCH 2/2] driver core: platform: provide devm_platform_iounremap_resource
  2020-10-05 15:23           ` pierre kuo
@ 2020-10-05 15:31             ` Greg KH
  0 siblings, 0 replies; 8+ messages in thread
From: Greg KH @ 2020-10-05 15:31 UTC (permalink / raw)
  To: pierre kuo; +Cc: rafael, Linux Kernel Mailing List

On Mon, Oct 05, 2020 at 11:23:18PM +0800, pierre kuo wrote:
> hi Greg:
> > Why are you adding new functions but not actually calling them anywhere?
> 
> Below patch introduce a single helper, devm_platform_ioremap_resource,
> which combines
> platform_get_resource() and devm_ioremap_resource(). But there is no
> single helper to release
> those resources in driver removing stage.
> 
> https://lore.kernel.org/lkml/20190215152507.31066-2-brgl@bgdev.pl/
> 
> That means driver owner still need to call below (*) and (**) for
> releasing resource.
> Therefore, this patch adds a single release helper that can be paired with
> devm_platform_ioremap_resource.
> 
> Appreciate ur kind help,
> 
> foo_probe(pdev)
> {
>     iomem = devm_platform_ioremap_resource(pdev, 0);
>     ....
> }
> 
> 
> foo_remove(pdev)
> {
>     devm_iounmap(iomem);   (*)
>     devm_release_mem_region(dev, res->start, size); (**)
>    ........................
> }

I don't understand this at all, sorry.  Please submit your patch series,
with some drivers converted to use the new functions.  Otherwise we can
not properly review it.

thanks,

greg k-h

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

end of thread, other threads:[~2020-10-05 15:35 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-20 11:38 [PATCH 1/2] lib: devres: provide devm_iounremap_resource() pierre Kuo
2020-09-20 11:38 ` [PATCH 2/2] driver core: platform: provide devm_platform_iounremap_resource pierre Kuo
2020-09-29  6:21   ` pierre kuo
2020-10-02 13:45     ` Greg KH
2020-10-04 16:21       ` pierre kuo
2020-10-04 16:47         ` Greg KH
2020-10-05 15:23           ` pierre kuo
2020-10-05 15:31             ` Greg KH

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).