Patch #1 adds a new devres function that I found could be useful for the
driver dcss in drm. Patch #2 makes that driver use the new function.
I compiled this successfully but unfortunately don't have the hardware
to test it for dcss.
So you might want to have a closer look.
Greetings,
P.
Philipp Stanner (2):
platform_device: add devres function region-reqs
drm/dcss: request memory region
drivers/base/platform.c | 37 +++++++++++++++++++++++++++++
drivers/gpu/drm/imx/dcss/dcss-dev.c | 8 +++----
include/linux/platform_device.h | 2 ++
3 files changed, 43 insertions(+), 4 deletions(-)
--
2.43.0
Some drivers want to use (request) a region exclusively but nevertheless
create several mappings within that region.
Currently, there is no managed devres function to request a region
without mapping it.
Add the function devm_platform_get_resource()
Signed-off-by: Philipp Stanner <[email protected]>
---
drivers/base/platform.c | 37 +++++++++++++++++++++++++++++++++
include/linux/platform_device.h | 2 ++
2 files changed, 39 insertions(+)
diff --git a/drivers/base/platform.c b/drivers/base/platform.c
index 10c577963418..243b9ec54d04 100644
--- a/drivers/base/platform.c
+++ b/drivers/base/platform.c
@@ -82,6 +82,43 @@ struct resource *platform_get_mem_or_io(struct platform_device *dev,
}
EXPORT_SYMBOL_GPL(platform_get_mem_or_io);
+/**
+ * devm_platform_get_and_resource - get and request a resource
+ *
+ * @pdev: the platform device to get the resource from
+ * @type: resource type (either IORESOURCE_MEM or IORESOURCE_IO)
+ * @num: resource index
+ * @name: name to be associated with the request
+ *
+ * Return: a pointer to the resource on success, an ERR_PTR on failure.
+ *
+ * Gets a resource and requests it. Use this instead of
+ * devm_platform_ioremap_resource() only if you have to create several single
+ * mappings with devm_ioremap().
+ */
+struct resource *devm_platform_get_resource(struct platform_device *pdev,
+ unsigned int type, unsigned int num, const char *name)
+{
+ struct resource *res;
+
+ res = platform_get_resource(pdev, type, num);
+ if (!res)
+ return ERR_PTR(-EINVAL);
+
+ if (type & IORESOURCE_MEM)
+ res = devm_request_mem_region(&pdev->dev, res->start, res->end, name);
+ else if (type & IORESOURCE_IO)
+ res = devm_request_region(&pdev->dev, res->start, res->end, name);
+ else
+ return ERR_PTR(-EINVAL);
+
+ if (!res)
+ return ERR_PTR(-EBUSY);
+
+ return res;
+}
+EXPORT_SYMBOL_GPL(devm_platform_get_resource);
+
#ifdef CONFIG_HAS_IOMEM
/**
* devm_platform_get_and_ioremap_resource - call devm_ioremap_resource() for a
diff --git a/include/linux/platform_device.h b/include/linux/platform_device.h
index 7a41c72c1959..68e2857521f4 100644
--- a/include/linux/platform_device.h
+++ b/include/linux/platform_device.h
@@ -59,6 +59,8 @@ extern struct resource *platform_get_resource(struct platform_device *,
unsigned int, unsigned int);
extern struct resource *platform_get_mem_or_io(struct platform_device *,
unsigned int);
+extern struct resource *devm_platform_get_resource(struct platform_device *pdev,
+ unsigned int type, unsigned int num, const char *name);
extern struct device *
platform_find_device_by_driver(struct device *start,
--
2.43.0
The driver's memory regions are currently just ioremap()ed, but not
reserved through a request. That's not a bug, but having the request is
a little more robust.
Implement the region-request through the corresponding managed
devres-function.
Signed-off-by: Philipp Stanner <[email protected]>
---
drivers/gpu/drm/imx/dcss/dcss-dev.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/imx/dcss/dcss-dev.c b/drivers/gpu/drm/imx/dcss/dcss-dev.c
index 4f3af0dfb344..efd3a998652d 100644
--- a/drivers/gpu/drm/imx/dcss/dcss-dev.c
+++ b/drivers/gpu/drm/imx/dcss/dcss-dev.c
@@ -177,10 +177,10 @@ struct dcss_dev *dcss_dev_create(struct device *dev, bool hdmi_output)
return ERR_PTR(-ENODEV);
}
- res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- if (!res) {
- dev_err(dev, "cannot get memory resource\n");
- return ERR_PTR(-EINVAL);
+ res = devm_platform_get_resource(pdev, IORESOURCE_MEM, 0, "dcss");
+ if (IS_ERR(res)) {
+ dev_err(dev, "cannot get / request memory resource\n");
+ return res;
}
dcss = kzalloc(sizeof(*dcss), GFP_KERNEL);
--
2.43.0
On Fri, Jan 05, 2024 at 06:22:18PM +0100, Philipp Stanner wrote:
> Some drivers want to use (request) a region exclusively but nevertheless
> create several mappings within that region.
>
> Currently, there is no managed devres function to request a region
> without mapping it.
>
> Add the function devm_platform_get_resource()
>
> Signed-off-by: Philipp Stanner <[email protected]>
> ---
> drivers/base/platform.c | 37 +++++++++++++++++++++++++++++++++
> include/linux/platform_device.h | 2 ++
> 2 files changed, 39 insertions(+)
>
> diff --git a/drivers/base/platform.c b/drivers/base/platform.c
> index 10c577963418..243b9ec54d04 100644
> --- a/drivers/base/platform.c
> +++ b/drivers/base/platform.c
> @@ -82,6 +82,43 @@ struct resource *platform_get_mem_or_io(struct platform_device *dev,
> }
> EXPORT_SYMBOL_GPL(platform_get_mem_or_io);
>
> +/**
> + * devm_platform_get_and_resource - get and request a resource
This function name is wrong.
> + *
> + * @pdev: the platform device to get the resource from
> + * @type: resource type (either IORESOURCE_MEM or IORESOURCE_IO)
> + * @num: resource index
> + * @name: name to be associated with the request
> + *
> + * Return: a pointer to the resource on success, an ERR_PTR on failure.
> + *
> + * Gets a resource and requests it. Use this instead of
> + * devm_platform_ioremap_resource() only if you have to create several single
> + * mappings with devm_ioremap().
> + */
> +struct resource *devm_platform_get_resource(struct platform_device *pdev,
> + unsigned int type, unsigned int num, const char *name)
> +{
> + struct resource *res;
> +
> + res = platform_get_resource(pdev, type, num);
> + if (!res)
> + return ERR_PTR(-EINVAL);
From devm_platform_get_resource I'd expect that it only does
platform_get_resource() + register a cleanup function to undo it.
> + if (type & IORESOURCE_MEM)
> + res = devm_request_mem_region(&pdev->dev, res->start, res->end, name);
> + else if (type & IORESOURCE_IO)
> + res = devm_request_region(&pdev->dev, res->start, res->end, name);
> + else
> + return ERR_PTR(-EINVAL);
So this part is surprising. IMHO your function's name should include
"request".
> + if (!res)
> + return ERR_PTR(-EBUSY);
> +
> + return res;
> +}
> +EXPORT_SYMBOL_GPL(devm_platform_get_resource);
> +
> #ifdef CONFIG_HAS_IOMEM
> /**
> * devm_platform_get_and_ioremap_resource - call devm_ioremap_resource() for a
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | https://www.pengutronix.de/ |
Hi Philipp,
kernel test robot noticed the following build warnings:
[auto build test WARNING on drm-misc/drm-misc-next]
[also build test WARNING on driver-core/driver-core-testing driver-core/driver-core-next driver-core/driver-core-linus linus/master v6.7 next-20240105]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Philipp-Stanner/platform_device-add-devres-function-region-reqs/20240106-012526
base: git://anongit.freedesktop.org/drm/drm-misc drm-misc-next
patch link: https://lore.kernel.org/r/20240105172218.42457-3-pstanner%40redhat.com
patch subject: [PATCH 1/2] platform_device: add devres function region-reqs
config: x86_64-randconfig-101-20240106 (https://download.01.org/0day-ci/archive/20240108/[email protected]/config)
compiler: gcc-7 (Ubuntu 7.5.0-6ubuntu2) 7.5.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240108/[email protected]/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
All warnings (new ones prefixed by >>):
>> drivers/base/platform.c:101: warning: expecting prototype for devm_platform_get_and_resource(). Prototype was for devm_platform_get_resource() instead
vim +101 drivers/base/platform.c
84
85 /**
86 * devm_platform_get_and_resource - get and request a resource
87 *
88 * @pdev: the platform device to get the resource from
89 * @type: resource type (either IORESOURCE_MEM or IORESOURCE_IO)
90 * @num: resource index
91 * @name: name to be associated with the request
92 *
93 * Return: a pointer to the resource on success, an ERR_PTR on failure.
94 *
95 * Gets a resource and requests it. Use this instead of
96 * devm_platform_ioremap_resource() only if you have to create several single
97 * mappings with devm_ioremap().
98 */
99 struct resource *devm_platform_get_resource(struct platform_device *pdev,
100 unsigned int type, unsigned int num, const char *name)
> 101 {
102 struct resource *res;
103
104 res = platform_get_resource(pdev, type, num);
105 if (!res)
106 return ERR_PTR(-EINVAL);
107
108 if (type & IORESOURCE_MEM)
109 res = devm_request_mem_region(&pdev->dev, res->start, res->end, name);
110 else if (type & IORESOURCE_IO)
111 res = devm_request_region(&pdev->dev, res->start, res->end, name);
112 else
113 return ERR_PTR(-EINVAL);
114
115 if (!res)
116 return ERR_PTR(-EBUSY);
117
118 return res;
119 }
120 EXPORT_SYMBOL_GPL(devm_platform_get_resource);
121
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Hi
On Fri, 2024-01-05 at 20:11 +0100, Uwe Kleine-König wrote:
> On Fri, Jan 05, 2024 at 06:22:18PM +0100, Philipp Stanner wrote:
> > Some drivers want to use (request) a region exclusively but
> > nevertheless
> > create several mappings within that region.
> >
> > Currently, there is no managed devres function to request a region
> > without mapping it.
> >
> > Add the function devm_platform_get_resource()
> >
> > Signed-off-by: Philipp Stanner <[email protected]>
> > ---
> > drivers/base/platform.c | 37
> > +++++++++++++++++++++++++++++++++
> > include/linux/platform_device.h | 2 ++
> > 2 files changed, 39 insertions(+)
> >
> > diff --git a/drivers/base/platform.c b/drivers/base/platform.c
> > index 10c577963418..243b9ec54d04 100644
> > --- a/drivers/base/platform.c
> > +++ b/drivers/base/platform.c
> > @@ -82,6 +82,43 @@ struct resource *platform_get_mem_or_io(struct
> > platform_device *dev,
> > }
> > EXPORT_SYMBOL_GPL(platform_get_mem_or_io);
> >
> > +/**
> > + * devm_platform_get_and_resource - get and request a resource
>
> This function name is wrong.
>
> > + *
> > + * @pdev: the platform device to get the resource from
> > + * @type: resource type (either IORESOURCE_MEM or IORESOURCE_IO)
> > + * @num: resource index
> > + * @name: name to be associated with the request
> > + *
> > + * Return: a pointer to the resource on success, an ERR_PTR on
> > failure.
> > + *
> > + * Gets a resource and requests it. Use this instead of
> > + * devm_platform_ioremap_resource() only if you have to create
> > several single
> > + * mappings with devm_ioremap().
> > + */
> > +struct resource *devm_platform_get_resource(struct platform_device
> > *pdev,
> > + unsigned int type, unsigned int num, const char
> > *name)
> > +{
> > + struct resource *res;
> > +
> > + res = platform_get_resource(pdev, type, num);
> > + if (!res)
> > + return ERR_PTR(-EINVAL);
>
> From devm_platform_get_resource I'd expect that it only does
> platform_get_resource() + register a cleanup function to undo it.
>
> > + if (type & IORESOURCE_MEM)
> > + res = devm_request_mem_region(&pdev->dev, res-
> > >start, res->end, name);
> > + else if (type & IORESOURCE_IO)
> > + res = devm_request_region(&pdev->dev, res->start,
> > res->end, name);
> > + else
> > + return ERR_PTR(-EINVAL);
>
> So this part is surprising. IMHO your function's name should include
> "request".
Yes, that sounds very correct to me. I'll address that in v2
Thx for the feedback,
P.
>
> > + if (!res)
> > + return ERR_PTR(-EBUSY);
> > +
> > + return res;
> > +}
> > +EXPORT_SYMBOL_GPL(devm_platform_get_resource);
> > +
> > #ifdef CONFIG_HAS_IOMEM
> > /**
> > * devm_platform_get_and_ioremap_resource - call
> > devm_ioremap_resource() for a
>
> Best regards
> Uwe
>