[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <aWdCj9FPfOaLNf8L@smile.fi.intel.com>
Date: Wed, 14 Jan 2026 09:15:27 +0200
From: Andy Shevchenko <andriy.shevchenko@...el.com>
To: Chintan Patel <chintanlike@...il.com>
Cc: Greg KH <gregkh@...uxfoundation.org>, linux-fbdev@...r.kernel.org,
linux-staging@...ts.linux.dev, linux-omap@...r.kernel.org,
linux-kernel@...r.kernel.org, dri-devel@...ts.freedesktop.org,
tzimmermann@...e.de, andy@...nel.org, deller@....de,
kernel test robot <lkp@...el.com>
Subject: Re: [PATCH v6] staging: fbtft: Use fbdev logging helpers when
FB_DEVICE is disabled
On Tue, Jan 13, 2026 at 08:47:54PM -0800, Chintan Patel wrote:
> On 1/12/26 22:16, Greg KH wrote:
> > On Mon, Jan 12, 2026 at 08:59:09PM -0800, Chintan Patel wrote:
> > > Replace direct accesses to info->dev with fb_dbg() and fb_info()
> > > helpers to avoid build failures when CONFIG_FB_DEVICE=n.
> >
> > Why is there a fb_* specific logging helper? dev_info() and dev_dbg()
> > should be used instead.
>
> You’re correct that dev_dbg()/dev_info() are the standard logging APIs.
>
> The reason I switched to fb_dbg()/fb_info() is not stylistic: direct
> dereferences of info->dev / fb_info->dev are invalid when
> CONFIG_FB_DEVICE=n, which causes compile-time errors.
>
> fb_dbg() and fb_info() are framebuffer-specific helpers that handle
> this case correctly, allowing logging without touching info->dev.
>
> > > Fixes: a06d03f9f238 ("staging: fbtft: Make FB_DEVICE dependency optional")
> >
> > Is this really a bug?
>
> The build failure occurs when CONFIG_FB_DEVICE=n, where direct
> dereferences of info->dev / fb_info->dev are not valid. This was reported by
> the kernel test robot.
>
> That said, I’m fine dropping the Fixes tag if you don’t consider this a
> regression.
I believe the point Greg made is that: If it's a bug, state it more clearly in
the commit message. The summary of the above sounds to me like a good enough
justification to leave Fixes tag as is.
...
> Same reason: dereferencing info->dev is invalid when CONFIG_FB_DEVICE=n.
> fb_dbg() handles this correctly without needing info->dev.
Similar comment here, make it more clearly, e.g. by adding more details in the
commit message, like explaining that there is no such a field to access when it
goes under some circumstances.
...
> > > + fb_info(fb_info,
> > > + "%s frame buffer, %dx%d, %d KiB video memory%s, fps=%lu%s\n",
> > > + fb_info->fix.id, fb_info->var.xres, fb_info->var.yres,
> > > + fb_info->fix.smem_len >> 10, text1,
> > > + HZ / fb_info->fbdefio->delay, text2);
> >
> > When drivers work properly, they are quiet. Why is this needed at all
> > except as a debug message?
>
> Agreed. The informational message during framebuffer registration is not
> necessary. I will either remove it entirely or convert it to a debug-only
> message.
>
> I’ll rework the patch accordingly and resend.
If you go this direction, I would do it in two stages (first is a direct
fix for a compilation issue and second one is switching to dbg level, each
with the respective commit message), but I leave it up to you and Greg.
--
With Best Regards,
Andy Shevchenko
Powered by blists - more mailing lists