2020-09-20 11:40:03

by pierre kuo

[permalink] [raw]
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 <[email protected]>
---
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


2020-09-20 11:40:20

by pierre kuo

[permalink] [raw]
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 <[email protected]>
---
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

2020-09-29 06:23:42

by pierre kuo

[permalink] [raw]
Subject: Re: [PATCH 2/2] driver core: platform: provide devm_platform_iounremap_resource

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 <[email protected]>
> ---
> 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
>

2020-10-02 13:46:57

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH 2/2] driver core: platform: provide devm_platform_iounremap_resource

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

2020-10-04 16:25:46

by pierre kuo

[permalink] [raw]
Subject: Re: [PATCH 2/2] driver core: platform: provide devm_platform_iounremap_resource

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/[email protected]
https://lore.kernel.org/lkml/[email protected]

Appreciate your help,


Attachments:
0001-lib-devres-provide-devm_iounremap_resource.patch (2.23 kB)
0002-driver-core-platform-provide-devm_platform_iounremap.patch (2.40 kB)
Download all attachments

2020-10-04 17:00:45

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH 2/2] driver core: platform: provide devm_platform_iounremap_resource

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/[email protected]
> https://lore.kernel.org/lkml/[email protected]

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

2020-10-05 15:25:16

by pierre kuo

[permalink] [raw]
Subject: Re: [PATCH 2/2] driver core: platform: provide devm_platform_iounremap_resource

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/[email protected]/

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); (**)
........................
}

2020-10-05 15:37:33

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH 2/2] driver core: platform: provide devm_platform_iounremap_resource

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/[email protected]/
>
> 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