Re: [RFC/RFT PATCH] fbdev: q40fb: request memory region

Re: [RFC/RFT PATCH] fbdev: q40fb: request memory region

[Geert Uytterhoeven]

Hi Sukrut,

CC linux-m68k

On Thu, 20 Nov 2025 at 19:03, Sukrut Heroorkar <hsukrut3@gmail.com> wrote:
> The q40fb driver uses a fixed physical address but never reserves
> the corresponding I/O region. Reserve the range  as suggested in
> Documentation/gpu/todo.rst ("Request memory regions in all fbdev drivers").
>
> If the memory cannot be reserved, fail probe with -EBUSY to avoid
> conflicting with another user of the same address.
>
> Signed-off-by: Sukrut Heroorkar <hsukrut3@gmail.com>

Thanks for your patch!

> ---
> Testing: This patch is sent as RFT since Q40 hardware is unavilable and
> QEMU does not emulated a Q40 platform. The change is therefore compile-tested
> only.

I would suggest not to apply this, unless it is tested on real
hardware.  It wouldn't be the first time an innocent-looking change like
this breaks a system. See e.g.
https://lore.kernel.org/all/Y5I2oQexHNdlIbsQ@shell.armlinux.org.uk

> --- a/drivers/video/fbdev/q40fb.c
> +++ b/drivers/video/fbdev/q40fb.c
> @@ -101,6 +101,13 @@ static int q40fb_probe(struct platform_device *dev)
>         info->par = NULL;
>         info->screen_base = (char *) q40fb_fix.smem_start;
>
> +       if (!request_mem_region(q40fb_fix.smem_start, q40fb_fix.smem_len,
> +                               "q40fb")) {
> +               dev_err(&dev->dev, "cannot reserve video memory at 0x%lx\n",
> +                       q40fb_fix.smem_start);
> +               return -EBUSY;
> +       }
> +
>         if (fb_alloc_cmap(&info->cmap, 256, 0) < 0) {
>                 framebuffer_release(info);
>                 return -ENOMEM;

Gr{oetje,eeting}s,

                        Geert


--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [RFC/RFT PATCH] fbdev: q40fb: request memory region

[Helge Deller]

Hi Geert & Sukrut,

On 11/21/25 11:03, Geert Uytterhoeven wrote:
> On Thu, 20 Nov 2025 at 19:03, Sukrut Heroorkar <hsukrut3@gmail.com> wrote:
>> The q40fb driver uses a fixed physical address but never reserves
>> the corresponding I/O region. Reserve the range  as suggested in
>> Documentation/gpu/todo.rst ("Request memory regions in all fbdev drivers").
>>
>> If the memory cannot be reserved, fail probe with -EBUSY to avoid
>> conflicting with another user of the same address.
>>
>> Signed-off-by: Sukrut Heroorkar <hsukrut3@gmail.com>
> 
> Thanks for your patch!
> 
>> ---
>> Testing: This patch is sent as RFT since Q40 hardware is unavilable and
>> QEMU does not emulated a Q40 platform. The change is therefore compile-tested
>> only.
> 
> I would suggest not to apply this, unless it is tested on real
> hardware.  It wouldn't be the first time an innocent-looking change like
> this breaks a system. See e.g.
> https://lore.kernel.org/all/Y5I2oQexHNdlIbsQ@shell.armlinux.org.uk

Geert, that's actually my thought as well, so I'm happy you wrote down
your opinion here!

By any chance, do you (or someone on this list) know of someone who has
that card and might be able to test it?

Helge


>> --- a/drivers/video/fbdev/q40fb.c
>> +++ b/drivers/video/fbdev/q40fb.c
>> @@ -101,6 +101,13 @@ static int q40fb_probe(struct platform_device *dev)
>>          info->par = NULL;
>>          info->screen_base = (char *) q40fb_fix.smem_start;
>>
>> +       if (!request_mem_region(q40fb_fix.smem_start, q40fb_fix.smem_len,
>> +                               "q40fb")) {
>> +               dev_err(&dev->dev, "cannot reserve video memory at 0x%lx\n",
>> +                       q40fb_fix.smem_start);
>> +               return -EBUSY;
>> +       }
>> +
>>          if (fb_alloc_cmap(&info->cmap, 256, 0) < 0) {
>>                  framebuffer_release(info);
>>                  return -ENOMEM;