Re: [PATCH 3/5] drm/simpledrm: Request memory region in driver

[Jocelyn Falempe <jfalempe@redhat.com>]

On 24/01/2022 13:36, Thomas Zimmermann wrote:
> Requesting the framebuffer memory in simpledrm marks the memory
> range as busy. This used to be done by the firmware sysfb code,
> but the driver is the correct place.
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---
>   drivers/gpu/drm/tiny/simpledrm.c | 20 +++++++++++++++-----
>   1 file changed, 15 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/tiny/simpledrm.c b/drivers/gpu/drm/tiny/simpledrm.c
> index 04146da2d1d8..f72b71511a65 100644
> --- a/drivers/gpu/drm/tiny/simpledrm.c
> +++ b/drivers/gpu/drm/tiny/simpledrm.c
> @@ -526,21 +526,31 @@ static int simpledrm_device_init_mm(struct simpledrm_device *sdev)
>   {
>   	struct drm_device *dev = &sdev->dev;
>   	struct platform_device *pdev = sdev->pdev;
> -	struct resource *mem;
> +	struct resource *res, *mem;
>   	void __iomem *screen_base;
>   	int ret;
>   
> -	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> -	if (!mem)
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	if (!res)
>   		return -EINVAL;
>   
> -	ret = devm_aperture_acquire_from_firmware(dev, mem->start, resource_size(mem));
> +	ret = devm_aperture_acquire_from_firmware(dev, res->start, resource_size(res));
>   	if (ret) {
>   		drm_err(dev, "could not acquire memory range %pr: error %d\n",
> -			mem, ret);
> +			res, ret);
>   		return ret;
>   	}
>   
> +	mem = devm_request_mem_region(&pdev->dev, res->start, resource_size(res),
> +				      sdev->dev.driver->name);
> +	if (!mem) {
> +		/*
> +		 * We cannot make this fatal. Sometimes this comes from magic
> +		 * spaces our resource handlers simply don't know about
> +		 */
> +		drm_warn(dev, "could not acquire memory region %pr\n", res);
> +	}
> +
>   	screen_base = devm_ioremap_wc(&pdev->dev, mem->start,
>   				      resource_size(mem));

if mem is NULL, accessing mem->start will segfault after the warning.
I think you renamed "mem" to "res" so probably it should be renamed here 
too ?

>   	if (!screen_base)