All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andy Shevchenko <andriy.shevchenko@intel.com>
To: Chintan Patel <chintanlike@gmail.com>
Cc: Greg KH <gregkh@linuxfoundation.org>,
	linux-fbdev@vger.kernel.org, linux-staging@lists.linux.dev,
	linux-omap@vger.kernel.org, linux-kernel@vger.kernel.org,
	dri-devel@lists.freedesktop.org, tzimmermann@suse.de,
	andy@kernel.org, deller@gmx.de, kernel test robot <lkp@intel.com>
Subject: Re: [PATCH v6] staging: fbtft: Use fbdev logging helpers when FB_DEVICE is disabled
Date: Wed, 14 Jan 2026 09:15:27 +0200	[thread overview]
Message-ID: <aWdCj9FPfOaLNf8L@smile.fi.intel.com> (raw)
In-Reply-To: <0a90bd0a-cb74-43a3-a50b-4c83bc086556@gmail.com>

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



  reply	other threads:[~2026-01-14  7:15 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-01-13  4:59 [PATCH v6] staging: fbtft: Use fbdev logging helpers when FB_DEVICE is disabled Chintan Patel
2026-01-13  6:16 ` Greg KH
2026-01-14  4:47   ` Chintan Patel
2026-01-14  7:15     ` Andy Shevchenko [this message]
2026-01-14 11:38   ` Thomas Zimmermann
2026-01-15  4:05     ` Chintan Patel
2026-01-15  7:55 ` Thomas Zimmermann
2026-01-16  2:59   ` Chintan Patel

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aWdCj9FPfOaLNf8L@smile.fi.intel.com \
    --to=andriy.shevchenko@intel.com \
    --cc=andy@kernel.org \
    --cc=chintanlike@gmail.com \
    --cc=deller@gmx.de \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-fbdev@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-omap@vger.kernel.org \
    --cc=linux-staging@lists.linux.dev \
    --cc=lkp@intel.com \
    --cc=tzimmermann@suse.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.