staging: fbtft: Use fbdev logging helpers when FB_DEVICE is disabled

[PATCH v6] staging: fbtft: Use fbdev logging helpers when FB_DEVICE is disabled

Posted by Chintan Patel 3 weeks, 4 days ago

Replace direct accesses to info->dev with fb_dbg() and fb_info()
helpers to avoid build failures when CONFIG_FB_DEVICE=n.

Fixes: a06d03f9f238 ("staging: fbtft: Make FB_DEVICE dependency optional")
Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202601110740.Y9XK5HtN-lkp@intel.com
Signed-off-by: Chintan Patel <chintanlike@gmail.com>

Changes in v6:
- Switch debug/info logging to fb_dbg() and fb_info()(suggested by Thomas Zimmermann)
- Drop dev_of_fbinfo() usage in favor of framebuffer helpers that implicitly
  handle the debug/info context.
- Drop __func__ usage per review feedback(suggested by greg k-h)
- Add Fixes tag for a06d03f9f238 ("staging: fbtft: Make FB_DEVICE dependency optional")
  (suggested by Andy Shevchenko)

Changes in v5:
- Initial attempt to replace info->dev accesses using
  dev_of_fbinfo() helper
---
 drivers/staging/fbtft/fbtft-core.c | 19 +++++++++----------
 1 file changed, 9 insertions(+), 10 deletions(-)

diff --git a/drivers/staging/fbtft/fbtft-core.c b/drivers/staging/fbtft/fbtft-core.c
index 8a5ccc8ae0a1..1b3b62950205 100644
--- a/drivers/staging/fbtft/fbtft-core.c
+++ b/drivers/staging/fbtft/fbtft-core.c
@@ -365,9 +365,9 @@ static int fbtft_fb_setcolreg(unsigned int regno, unsigned int red,
 	unsigned int val;
 	int ret = 1;
 
-	dev_dbg(info->dev,
-		"%s(regno=%u, red=0x%X, green=0x%X, blue=0x%X, trans=0x%X)\n",
-		__func__, regno, red, green, blue, transp);
+	fb_dbg(info,
+	       "regno=%u, red=0x%X, green=0x%X, blue=0x%X, trans=0x%X\n",
+	       regno, red, green, blue, transp);
 
 	switch (info->fix.visual) {
 	case FB_VISUAL_TRUECOLOR:
@@ -391,8 +391,7 @@ static int fbtft_fb_blank(int blank, struct fb_info *info)
 	struct fbtft_par *par = info->par;
 	int ret = -EINVAL;
 
-	dev_dbg(info->dev, "%s(blank=%d)\n",
-		__func__, blank);
+	fb_dbg(info, "blank=%d\n", blank);
 
 	if (!par->fbtftops.blank)
 		return ret;
@@ -793,11 +792,11 @@ int fbtft_register_framebuffer(struct fb_info *fb_info)
 	if (spi)
 		sprintf(text2, ", spi%d.%d at %d MHz", spi->controller->bus_num,
 			spi_get_chipselect(spi, 0), spi->max_speed_hz / 1000000);
-	dev_info(fb_info->dev,
-		 "%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);
+	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);
 
 	/* Turn on backlight if available */
 	if (fb_info->bl_dev) {
-- 
2.43.0

Re: [PATCH v6] staging: fbtft: Use fbdev logging helpers when FB_DEVICE is disabled

Posted by Thomas Zimmermann 3 weeks, 2 days ago

Hi

Am 13.01.26 um 05:59 schrieb Chintan Patel:
> Replace direct accesses to info->dev with fb_dbg() and fb_info()
> helpers to avoid build failures when CONFIG_FB_DEVICE=n.
>
> Fixes: a06d03f9f238 ("staging: fbtft: Make FB_DEVICE dependency optional")
> Reported-by: kernel test robot <lkp@intel.com>
> Closes: https://lore.kernel.org/oe-kbuild-all/202601110740.Y9XK5HtN-lkp@intel.com
> Signed-off-by: Chintan Patel <chintanlike@gmail.com>
>
> Changes in v6:
> - Switch debug/info logging to fb_dbg() and fb_info()(suggested by Thomas Zimmermann)
> - Drop dev_of_fbinfo() usage in favor of framebuffer helpers that implicitly
>    handle the debug/info context.
> - Drop __func__ usage per review feedback(suggested by greg k-h)
> - Add Fixes tag for a06d03f9f238 ("staging: fbtft: Make FB_DEVICE dependency optional")
>    (suggested by Andy Shevchenko)
>
> Changes in v5:
> - Initial attempt to replace info->dev accesses using
>    dev_of_fbinfo() helper
> ---
>   drivers/staging/fbtft/fbtft-core.c | 19 +++++++++----------
>   1 file changed, 9 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/staging/fbtft/fbtft-core.c b/drivers/staging/fbtft/fbtft-core.c
> index 8a5ccc8ae0a1..1b3b62950205 100644
> --- a/drivers/staging/fbtft/fbtft-core.c
> +++ b/drivers/staging/fbtft/fbtft-core.c
> @@ -365,9 +365,9 @@ static int fbtft_fb_setcolreg(unsigned int regno, unsigned int red,
>   	unsigned int val;
>   	int ret = 1;
>   
> -	dev_dbg(info->dev,
> -		"%s(regno=%u, red=0x%X, green=0x%X, blue=0x%X, trans=0x%X)\n",
> -		__func__, regno, red, green, blue, transp);
> +	fb_dbg(info,
> +	       "regno=%u, red=0x%X, green=0x%X, blue=0x%X, trans=0x%X\n",
> +	       regno, red, green, blue, transp);
>   
>   	switch (info->fix.visual) {
>   	case FB_VISUAL_TRUECOLOR:
> @@ -391,8 +391,7 @@ static int fbtft_fb_blank(int blank, struct fb_info *info)
>   	struct fbtft_par *par = info->par;
>   	int ret = -EINVAL;
>   
> -	dev_dbg(info->dev, "%s(blank=%d)\n",
> -		__func__, blank);
> +	fb_dbg(info, "blank=%d\n", blank);
>   
>   	if (!par->fbtftops.blank)
>   		return ret;
> @@ -793,11 +792,11 @@ int fbtft_register_framebuffer(struct fb_info *fb_info)
>   	if (spi)
>   		sprintf(text2, ", spi%d.%d at %d MHz", spi->controller->bus_num,
>   			spi_get_chipselect(spi, 0), spi->max_speed_hz / 1000000);
> -	dev_info(fb_info->dev,
> -		 "%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);
> +	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);

As discussed before, this should become fb_dbg().  Drivers should not 
print status reports unless they do not work as expected.

Best regards
Thomas

>   
>   	/* Turn on backlight if available */
>   	if (fb_info->bl_dev) {

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Jochen Jaser, Andrew McDonald, Werner Knoblich, (HRB 36809, AG Nürnberg)

Re: [PATCH v6] staging: fbtft: Use fbdev logging helpers when FB_DEVICE is disabled

Posted by Chintan Patel 3 weeks, 1 day ago


On 1/14/26 23:55, Thomas Zimmermann wrote:
> Hi
> 
> Am 13.01.26 um 05:59 schrieb Chintan Patel:
>> Replace direct accesses to info->dev with fb_dbg() and fb_info()
>> helpers to avoid build failures when CONFIG_FB_DEVICE=n.
>>
>> Fixes: a06d03f9f238 ("staging: fbtft: Make FB_DEVICE dependency 
>> optional")
>> Reported-by: kernel test robot <lkp@intel.com>
>> Closes: https://lore.kernel.org/oe-kbuild-all/202601110740.Y9XK5HtN- 
>> lkp@intel.com
>> Signed-off-by: Chintan Patel <chintanlike@gmail.com>
>>
>> Changes in v6:
>> - Switch debug/info logging to fb_dbg() and fb_info()(suggested by 
>> Thomas Zimmermann)
>> - Drop dev_of_fbinfo() usage in favor of framebuffer helpers that 
>> implicitly
>>    handle the debug/info context.
>> - Drop __func__ usage per review feedback(suggested by greg k-h)
>> - Add Fixes tag for a06d03f9f238 ("staging: fbtft: Make FB_DEVICE 
>> dependency optional")
>>    (suggested by Andy Shevchenko)
>>
>> Changes in v5:
>> - Initial attempt to replace info->dev accesses using
>>    dev_of_fbinfo() helper
>> ---
>>   drivers/staging/fbtft/fbtft-core.c | 19 +++++++++----------
>>   1 file changed, 9 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/staging/fbtft/fbtft-core.c b/drivers/staging/ 
>> fbtft/fbtft-core.c
>> index 8a5ccc8ae0a1..1b3b62950205 100644
>> --- a/drivers/staging/fbtft/fbtft-core.c
>> +++ b/drivers/staging/fbtft/fbtft-core.c
>> @@ -365,9 +365,9 @@ static int fbtft_fb_setcolreg(unsigned int regno, 
>> unsigned int red,
>>       unsigned int val;
>>       int ret = 1;
>> -    dev_dbg(info->dev,
>> -        "%s(regno=%u, red=0x%X, green=0x%X, blue=0x%X, trans=0x%X)\n",
>> -        __func__, regno, red, green, blue, transp);
>> +    fb_dbg(info,
>> +           "regno=%u, red=0x%X, green=0x%X, blue=0x%X, trans=0x%X\n",
>> +           regno, red, green, blue, transp);
>>       switch (info->fix.visual) {
>>       case FB_VISUAL_TRUECOLOR:
>> @@ -391,8 +391,7 @@ static int fbtft_fb_blank(int blank, struct 
>> fb_info *info)
>>       struct fbtft_par *par = info->par;
>>       int ret = -EINVAL;
>> -    dev_dbg(info->dev, "%s(blank=%d)\n",
>> -        __func__, blank);
>> +    fb_dbg(info, "blank=%d\n", blank);
>>       if (!par->fbtftops.blank)
>>           return ret;
>> @@ -793,11 +792,11 @@ int fbtft_register_framebuffer(struct fb_info 
>> *fb_info)
>>       if (spi)
>>           sprintf(text2, ", spi%d.%d at %d MHz", spi->controller- 
>> >bus_num,
>>               spi_get_chipselect(spi, 0), spi->max_speed_hz / 1000000);
>> -    dev_info(fb_info->dev,
>> -         "%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);
>> +    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);
> 
> As discussed before, this should become fb_dbg().  Drivers should not 
> print status reports unless they do not work as expected.

Agree - I will send 2 patches(series) as per feedback 1) a patch focused 
purely on fixing the compilation issue by avoiding info->dev 
dereferences (using fb_dbg() where logging remains), and
2) a follow-up cleanup that downgrades the framebuffer
     registration message to debug level.

> Best regards
> Thomas
> 
>>       /* Turn on backlight if available */
>>       if (fb_info->bl_dev) {
>

Re: [PATCH v6] staging: fbtft: Use fbdev logging helpers when FB_DEVICE is disabled

Posted by Greg KH 3 weeks, 4 days ago

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.

> Fixes: a06d03f9f238 ("staging: fbtft: Make FB_DEVICE dependency optional")

Is this really a bug?

> Reported-by: kernel test robot <lkp@intel.com>
> Closes: https://lore.kernel.org/oe-kbuild-all/202601110740.Y9XK5HtN-lkp@intel.com
> Signed-off-by: Chintan Patel <chintanlike@gmail.com>
> 
> Changes in v6:
> - Switch debug/info logging to fb_dbg() and fb_info()(suggested by Thomas Zimmermann)
> - Drop dev_of_fbinfo() usage in favor of framebuffer helpers that implicitly
>   handle the debug/info context.
> - Drop __func__ usage per review feedback(suggested by greg k-h)
> - Add Fixes tag for a06d03f9f238 ("staging: fbtft: Make FB_DEVICE dependency optional")
>   (suggested by Andy Shevchenko)
> 
> Changes in v5:
> - Initial attempt to replace info->dev accesses using
>   dev_of_fbinfo() helper
> ---

The changelog stuff goes below the --- line.

>  drivers/staging/fbtft/fbtft-core.c | 19 +++++++++----------
>  1 file changed, 9 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/staging/fbtft/fbtft-core.c b/drivers/staging/fbtft/fbtft-core.c
> index 8a5ccc8ae0a1..1b3b62950205 100644
> --- a/drivers/staging/fbtft/fbtft-core.c
> +++ b/drivers/staging/fbtft/fbtft-core.c
> @@ -365,9 +365,9 @@ static int fbtft_fb_setcolreg(unsigned int regno, unsigned int red,
>  	unsigned int val;
>  	int ret = 1;
>  
> -	dev_dbg(info->dev,
> -		"%s(regno=%u, red=0x%X, green=0x%X, blue=0x%X, trans=0x%X)\n",
> -		__func__, regno, red, green, blue, transp);
> +	fb_dbg(info,
> +	       "regno=%u, red=0x%X, green=0x%X, blue=0x%X, trans=0x%X\n",
> +	       regno, red, green, blue, transp);

I dont understand what is wrong with the existing dev_dbg() line (with
the exception that __func__ should not be in it.

>  
>  	switch (info->fix.visual) {
>  	case FB_VISUAL_TRUECOLOR:
> @@ -391,8 +391,7 @@ static int fbtft_fb_blank(int blank, struct fb_info *info)
>  	struct fbtft_par *par = info->par;
>  	int ret = -EINVAL;
>  
> -	dev_dbg(info->dev, "%s(blank=%d)\n",
> -		__func__, blank);
> +	fb_dbg(info, "blank=%d\n", blank);

Same here, what's wrong with dev_dbg()?


>  
>  	if (!par->fbtftops.blank)
>  		return ret;
> @@ -793,11 +792,11 @@ int fbtft_register_framebuffer(struct fb_info *fb_info)
>  	if (spi)
>  		sprintf(text2, ", spi%d.%d at %d MHz", spi->controller->bus_num,
>  			spi_get_chipselect(spi, 0), spi->max_speed_hz / 1000000);
> -	dev_info(fb_info->dev,
> -		 "%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);
> +	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?

thanks,

greg k-h

Re: [PATCH v6] staging: fbtft: Use fbdev logging helpers when FB_DEVICE is disabled

Posted by Thomas Zimmermann 3 weeks, 3 days ago

Hi

Am 13.01.26 um 07:16 schrieb Greg KH:
> 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.

Fbdev is entirely inconsistent about its logging. There's dev_*(), 
there's pr_*(), and even printk(). The problem with dev_*() logging is 
that devices are not always available. The HW device can be NULL and 
might not be all that useful in practice. The Fbdev software device is 
often not even compiled in nowadays. (This patch is about that problem.) 
Hence the next best option is to make fb_*() logging helpers that 
address these problems. They are based on pr_*() and print the 
framebuffer index, which should always be available after 
register_framebuffer().

>
>> Fixes: a06d03f9f238 ("staging: fbtft: Make FB_DEVICE dependency optional")
> Is this really a bug?
>
>> Reported-by: kernel test robot <lkp@intel.com>
>> Closes: https://lore.kernel.org/oe-kbuild-all/202601110740.Y9XK5HtN-lkp@intel.com
>> Signed-off-by: Chintan Patel <chintanlike@gmail.com>
>>
>> Changes in v6:
>> - Switch debug/info logging to fb_dbg() and fb_info()(suggested by Thomas Zimmermann)
>> - Drop dev_of_fbinfo() usage in favor of framebuffer helpers that implicitly
>>    handle the debug/info context.
>> - Drop __func__ usage per review feedback(suggested by greg k-h)
>> - Add Fixes tag for a06d03f9f238 ("staging: fbtft: Make FB_DEVICE dependency optional")
>>    (suggested by Andy Shevchenko)
>>
>> Changes in v5:
>> - Initial attempt to replace info->dev accesses using
>>    dev_of_fbinfo() helper
>> ---
> The changelog stuff goes below the --- line.
>
>>   drivers/staging/fbtft/fbtft-core.c | 19 +++++++++----------
>>   1 file changed, 9 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/staging/fbtft/fbtft-core.c b/drivers/staging/fbtft/fbtft-core.c
>> index 8a5ccc8ae0a1..1b3b62950205 100644
>> --- a/drivers/staging/fbtft/fbtft-core.c
>> +++ b/drivers/staging/fbtft/fbtft-core.c
>> @@ -365,9 +365,9 @@ static int fbtft_fb_setcolreg(unsigned int regno, unsigned int red,
>>   	unsigned int val;
>>   	int ret = 1;
>>   
>> -	dev_dbg(info->dev,
>> -		"%s(regno=%u, red=0x%X, green=0x%X, blue=0x%X, trans=0x%X)\n",
>> -		__func__, regno, red, green, blue, transp);
>> +	fb_dbg(info,
>> +	       "regno=%u, red=0x%X, green=0x%X, blue=0x%X, trans=0x%X\n",
>> +	       regno, red, green, blue, transp);
> I dont understand what is wrong with the existing dev_dbg() line (with
> the exception that __func__ should not be in it.
>
>>   
>>   	switch (info->fix.visual) {
>>   	case FB_VISUAL_TRUECOLOR:
>> @@ -391,8 +391,7 @@ static int fbtft_fb_blank(int blank, struct fb_info *info)
>>   	struct fbtft_par *par = info->par;
>>   	int ret = -EINVAL;
>>   
>> -	dev_dbg(info->dev, "%s(blank=%d)\n",
>> -		__func__, blank);
>> +	fb_dbg(info, "blank=%d\n", blank);
> Same here, what's wrong with dev_dbg()?
>
>
>>   
>>   	if (!par->fbtftops.blank)
>>   		return ret;
>> @@ -793,11 +792,11 @@ int fbtft_register_framebuffer(struct fb_info *fb_info)
>>   	if (spi)
>>   		sprintf(text2, ", spi%d.%d at %d MHz", spi->controller->bus_num,
>>   			spi_get_chipselect(spi, 0), spi->max_speed_hz / 1000000);
>> -	dev_info(fb_info->dev,
>> -		 "%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);
>> +	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. If there's anything useful in this output, it should be printed 
with _dbg(), but not _info().

Best regards
Thomas

>
> thanks,
>
> greg k-h

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Jochen Jaser, Andrew McDonald, Werner Knoblich, (HRB 36809, AG Nürnberg)

Re: [PATCH v6] staging: fbtft: Use fbdev logging helpers when FB_DEVICE is disabled

Posted by Chintan Patel 3 weeks, 2 days ago


On 1/14/26 03:38, Thomas Zimmermann wrote:
> Hi
> 
> Am 13.01.26 um 07:16 schrieb Greg KH:
>> 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.
> 
> Fbdev is entirely inconsistent about its logging. There's dev_*(), 
> there's pr_*(), and even printk(). The problem with dev_*() logging is 
> that devices are not always available. The HW device can be NULL and 
> might not be all that useful in practice. The Fbdev software device is 
> often not even compiled in nowadays. (This patch is about that problem.) 
> Hence the next best option is to make fb_*() logging helpers that 
> address these problems. They are based on pr_*() and print the 
> framebuffer index, which should always be available after 
> register_framebuffer().
> 
>>

Thanks Andy and Thomas.

I’ll update the commit message to clearly describe the underlying issue.

I’ll also split the changes as suggested in 2 patches and send v7:
1) a patch focused purely on fixing the compilation issue by avoiding
    info->dev dereferences (using fb_dbg() where logging remains), and
2) a follow-up cleanup that removes or downgrades the framebuffer
    registration message to debug level.

I’ll rework the series accordingly and resend.

Thanks for the guidance.

Re: [PATCH v6] staging: fbtft: Use fbdev logging helpers when FB_DEVICE is disabled

Posted by Chintan Patel 3 weeks, 3 days ago


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.

>> Reported-by: kernel test robot <lkp@intel.com>
>> Closes: https://lore.kernel.org/oe-kbuild-all/202601110740.Y9XK5HtN-lkp@intel.com
>> Signed-off-by: Chintan Patel <chintanlike@gmail.com>
>>
>> Changes in v6:
>> - Switch debug/info logging to fb_dbg() and fb_info()(suggested by Thomas Zimmermann)
>> - Drop dev_of_fbinfo() usage in favor of framebuffer helpers that implicitly
>>    handle the debug/info context.
>> - Drop __func__ usage per review feedback(suggested by greg k-h)
>> - Add Fixes tag for a06d03f9f238 ("staging: fbtft: Make FB_DEVICE dependency optional")
>>    (suggested by Andy Shevchenko)
>>
>> Changes in v5:
>> - Initial attempt to replace info->dev accesses using
>>    dev_of_fbinfo() helper
>> ---
> 
> The changelog stuff goes below the --- line.

Ack.

> 
>>   drivers/staging/fbtft/fbtft-core.c | 19 +++++++++----------
>>   1 file changed, 9 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/staging/fbtft/fbtft-core.c b/drivers/staging/fbtft/fbtft-core.c
>> index 8a5ccc8ae0a1..1b3b62950205 100644
>> --- a/drivers/staging/fbtft/fbtft-core.c
>> +++ b/drivers/staging/fbtft/fbtft-core.c
>> @@ -365,9 +365,9 @@ static int fbtft_fb_setcolreg(unsigned int regno, unsigned int red,
>>   	unsigned int val;
>>   	int ret = 1;
>>   
>> -	dev_dbg(info->dev,
>> -		"%s(regno=%u, red=0x%X, green=0x%X, blue=0x%X, trans=0x%X)\n",
>> -		__func__, regno, red, green, blue, transp);
>> +	fb_dbg(info,
>> +	       "regno=%u, red=0x%X, green=0x%X, blue=0x%X, trans=0x%X\n",
>> +	       regno, red, green, blue, transp);
> 
> I dont understand what is wrong with the existing dev_dbg() line (with
> the exception that __func__ should not be in it.
> 
>>   
>>   	switch (info->fix.visual) {
>>   	case FB_VISUAL_TRUECOLOR:
>> @@ -391,8 +391,7 @@ static int fbtft_fb_blank(int blank, struct fb_info *info)
>>   	struct fbtft_par *par = info->par;
>>   	int ret = -EINVAL;
>>   
>> -	dev_dbg(info->dev, "%s(blank=%d)\n",
>> -		__func__, blank);
>> +	fb_dbg(info, "blank=%d\n", blank);
> 
> Same here, what's wrong with dev_dbg()?
> 

Same reason: dereferencing info->dev is invalid when CONFIG_FB_DEVICE=n. 
fb_dbg() handles this correctly without needing info->dev.

>>   
>>   	if (!par->fbtftops.blank)
>>   		return ret;
>> @@ -793,11 +792,11 @@ int fbtft_register_framebuffer(struct fb_info *fb_info)
>>   	if (spi)
>>   		sprintf(text2, ", spi%d.%d at %d MHz", spi->controller->bus_num,
>>   			spi_get_chipselect(spi, 0), spi->max_speed_hz / 1000000);
>> -	dev_info(fb_info->dev,
>> -		 "%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);
>> +	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.

Thanks again for the guidance.

Re: [PATCH v6] staging: fbtft: Use fbdev logging helpers when FB_DEVICE is disabled

Posted by Andy Shevchenko 3 weeks, 3 days ago

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