On 1/24/22 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@xxxxxxx>
> ---
> drivers/video/fbdev/simplefb.c | 59 ++++++++++++++++++++++++----------
> 1 file changed, 42 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/video/fbdev/simplefb.c b/drivers/video/fbdev/simplefb.c
> index 57541887188b..84452028ecc9 100644
> --- a/drivers/video/fbdev/simplefb.c
> +++ b/drivers/video/fbdev/simplefb.c
> @@ -66,16 +66,36 @@ static int simplefb_setcolreg(u_int regno, u_int red, u_int green, u_int blue,
> return 0;
> }
>
> -struct simplefb_par;
> +struct simplefb_par {
> + u32 palette[PSEUDO_PALETTE_SIZE];
> +#if defined CONFIG_OF && defined CONFIG_COMMON_CLK
> + bool clks_enabled;
> + unsigned int clk_count;
> + struct clk **clks;
> +#endif
> +#if defined CONFIG_OF && defined CONFIG_REGULATOR
> + bool regulators_enabled;
> + u32 regulator_count;
> + struct regulator **regulators;
> +#endif
> + bool release_mem_region;
Maybe instead you could have:
struct resource *mem;
that would be more consistent with the fields for the other
resources like clocks, regulators, etc.
> +};
> +
> static void simplefb_clocks_destroy(struct simplefb_par *par);
> static void simplefb_regulators_destroy(struct simplefb_par *par);
>
> static void simplefb_destroy(struct fb_info *info)
> {
> + struct simplefb_par *par = info->par;
> +
> simplefb_regulators_destroy(info->par);
> simplefb_clocks_destroy(info->par);
> if (info->screen_base)
> iounmap(info->screen_base);
> +
> + if (par->release_mem_region)
> + release_mem_region(info->apertures->ranges[0].base,
> + info->apertures->ranges[0].size);
and here you could instead use the pointer to the resource:
if (par->mem)
release_mem_region(par->mem->start, resource_size(par->mem));
[snip]
> static int simplefb_probe(struct platform_device *pdev)
> {
> + bool request_mem_succeeded = false;
> int ret;
> struct simplefb_params params;
> struct fb_info *info;
> @@ -436,9 +443,22 @@ static int simplefb_probe(struct platform_device *pdev)
> return -EINVAL;
> }
>
> + if (request_mem_region(mem->start, resource_size(mem), "simplefb")) {
> + request_mem_succeeded = true;
and if you do the request_mem_region() after the struct fb_info allocation then
this could just be:
par->mem = mem;
Best regards,
--
Javier Martinez Canillas
Linux Engineering
Red Hat
