[PATCH 0/2] platform_device: add new devres function

[PATCH 0/2] platform_device: add new devres function

[Philipp Stanner]

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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 1/2] platform_device: add devres function region-reqs

[Philipp Stanner]

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
+ *
+ * @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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 2/2] drm/dcss: request memory region

[Philipp Stanner]

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 <pstanner@redhat.com>
---
 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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/2] platform_device: add devres function region-reqs

[Uwe Kleine-König]

[-- Attachment #1.1: Type: text/plain, Size: 2673 bytes --]

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

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/2] platform_device: add devres function region-reqs

[kernel test robot]

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	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/2] platform_device: add devres function region-reqs

[Philipp Stanner]

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
> 


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel