mbox series

[0/1] drm/msm: request memory region

Message ID 20240405162213.28263-1-kiarash8112hajian@gmail.com
Headers show
Series drm/msm: request memory region | expand

Message

Kiarash Hajian April 5, 2024, 4:22 p.m. UTC
Kiarash Hajian (1):
  drm/msm: request memory region

 drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 2 +-
 drivers/gpu/drm/msm/msm_io_utils.c    | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)


base-commit: 8cb4a9a82b21623dbb4b3051dd30d98356cf95bc

Comments

Dmitry Baryshkov April 5, 2024, 6:18 p.m. UTC | #1
On Fri, 5 Apr 2024 at 19:23, Kiarash Hajian <kiarash8112hajian@gmail.com> wrote:
>
> 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: Kiarash Hajian <kiarash8112hajian@gmail.com>
> ---
>  drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 2 +-
>  drivers/gpu/drm/msm/msm_io_utils.c    | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> index 8bea8ef26f77..e4f7c282799b 100644
> --- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> @@ -1503,7 +1503,7 @@ static void __iomem *a6xx_gmu_get_mmio(struct platform_device *pdev,
>                 return ERR_PTR(-EINVAL);
>         }
>
> -       ret = ioremap(res->start, resource_size(res));
> +       ret = devm_ioremap_resource(&pdev->dev, res);

Now you have erroneous iounmap() calls in the code.

>         if (!ret) {
>                 DRM_DEV_ERROR(&pdev->dev, "Unable to map the %s registers\n", name);
>                 return ERR_PTR(-EINVAL);
> diff --git a/drivers/gpu/drm/msm/msm_io_utils.c b/drivers/gpu/drm/msm/msm_io_utils.c
> index afedd61c3e28..34e598ce869a 100644
> --- a/drivers/gpu/drm/msm/msm_io_utils.c
> +++ b/drivers/gpu/drm/msm/msm_io_utils.c
> @@ -83,7 +83,7 @@ static void __iomem *_msm_ioremap(struct platform_device *pdev, const char *name
>
>         size = resource_size(res);
>
> -       ptr = devm_ioremap(&pdev->dev, res->start, size);
> +       ptr = devm_ioremap_resource(&pdev->dev, res);
>         if (!ptr) {
>                 if (!quiet)
>                         DRM_DEV_ERROR(&pdev->dev, "failed to ioremap: %s\n", name);
> --
> 2.43.0
>
Bjorn Andersson April 7, 2024, 4:10 p.m. UTC | #2
On Fri, Apr 05, 2024 at 12:22:07PM -0400, Kiarash Hajian wrote:
> 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: Kiarash Hajian <kiarash8112hajian@gmail.com>

Happy to see you post your first patches here, Kiarash!

In addition to the issue pointed by Dmitry, you're missing a "v3" in
your subject, indicating that this is the 3rd version of the patch. I'm
also uncertain that you got your recipients list entirely correct.

I'd recommend that you adopt b4, as described at [1], as this helps you
with such details.


Please also update the subject prefix to "drm/msm/a6xx:", to match the
majority of other changes to this file.

[1] https://b4.docs.kernel.org/en/latest/contributor/overview.html

Regards,
Bjorn

> ---
>  drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 2 +-
>  drivers/gpu/drm/msm/msm_io_utils.c    | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> index 8bea8ef26f77..e4f7c282799b 100644
> --- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> @@ -1503,7 +1503,7 @@ static void __iomem *a6xx_gmu_get_mmio(struct platform_device *pdev,
>  		return ERR_PTR(-EINVAL);
>  	}
>  
> -	ret = ioremap(res->start, resource_size(res));
> +	ret = devm_ioremap_resource(&pdev->dev, res);
>  	if (!ret) {
>  		DRM_DEV_ERROR(&pdev->dev, "Unable to map the %s registers\n", name);
>  		return ERR_PTR(-EINVAL);
> diff --git a/drivers/gpu/drm/msm/msm_io_utils.c b/drivers/gpu/drm/msm/msm_io_utils.c
> index afedd61c3e28..34e598ce869a 100644
> --- a/drivers/gpu/drm/msm/msm_io_utils.c
> +++ b/drivers/gpu/drm/msm/msm_io_utils.c
> @@ -83,7 +83,7 @@ static void __iomem *_msm_ioremap(struct platform_device *pdev, const char *name
>  
>  	size = resource_size(res);
>  
> -	ptr = devm_ioremap(&pdev->dev, res->start, size);
> +	ptr = devm_ioremap_resource(&pdev->dev, res);
>  	if (!ptr) {
>  		if (!quiet)
>  			DRM_DEV_ERROR(&pdev->dev, "failed to ioremap: %s\n", name);
> -- 
> 2.43.0
> 
>