| Message ID | 20240105172218.42457-3-pstanner@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | platform_device: add new devres function | expand |
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 <pstanner@redhat.com> > --- > 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
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/202401081547.VBj4FKaw-lkp@intel.com/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/202401081547.VBj4FKaw-lkp@intel.com/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 <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202401081547.VBj4FKaw-lkp@intel.com/ 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
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 <pstanner@redhat.com> > > --- > > 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 >
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,
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 <pstanner@redhat.com> --- drivers/base/platform.c | 37 +++++++++++++++++++++++++++++++++ include/linux/platform_device.h | 2 ++ 2 files changed, 39 insertions(+)