* [PATCH v2] staging: sm750fb: Convert sw_i2c_read_sda to return bool
From: Karthikey Kadati @ 2026-01-12 10:38 UTC (permalink / raw)
To: sudipm.mukherjee, teddy.wang, gregkh
Cc: linux-fbdev, linux-staging, linux-kernel, Karthikey Kadati
The sw_i2c_read_sda function currently returns unsigned char (1 or 0).
Standardize it to return bool (true or false) to match kernel standards.
Signed-off-by: Karthikey Kadati <karthikey3608@gmail.com>
---
drivers/staging/sm750fb/ddk750_swi2c.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/sm750fb/ddk750_swi2c.c b/drivers/staging/sm750fb/ddk750_swi2c.c
index 0ef8d4ff2..9d48673d3 100644
--- a/drivers/staging/sm750fb/ddk750_swi2c.c
+++ b/drivers/staging/sm750fb/ddk750_swi2c.c
@@ -180,7 +180,7 @@ static void sw_i2c_sda(unsigned char value)
* Return Value:
* The SDA data bit sent by the Slave
*/
-static unsigned char sw_i2c_read_sda(void)
+static bool sw_i2c_read_sda(void)
{
unsigned long gpio_dir;
unsigned long gpio_data;
@@ -196,9 +196,9 @@ static unsigned char sw_i2c_read_sda(void)
/* Now read the SDA line */
gpio_data = peek32(sw_i2c_data_gpio_data_reg);
if (gpio_data & (1 << sw_i2c_data_gpio))
- return 1;
+ return true;
else
- return 0;
+ return false;
}
/*
--
2.43.0
^ permalink raw reply related
* Re: [PATCH v2] staging: sm750fb: Convert sw_i2c_read_sda to return bool
From: Greg KH @ 2026-01-12 10:49 UTC (permalink / raw)
To: Karthikey Kadati
Cc: sudipm.mukherjee, teddy.wang, linux-fbdev, linux-staging,
linux-kernel
In-Reply-To: <20260112103838.22890-1-karthikey3608@gmail.com>
On Mon, Jan 12, 2026 at 04:08:38PM +0530, Karthikey Kadati wrote:
> The sw_i2c_read_sda function currently returns unsigned char (1 or 0).
>
> Standardize it to return bool (true or false) to match kernel standards.
>
> Signed-off-by: Karthikey Kadati <karthikey3608@gmail.com>
> ---
> drivers/staging/sm750fb/ddk750_swi2c.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/staging/sm750fb/ddk750_swi2c.c b/drivers/staging/sm750fb/ddk750_swi2c.c
> index 0ef8d4ff2..9d48673d3 100644
> --- a/drivers/staging/sm750fb/ddk750_swi2c.c
> +++ b/drivers/staging/sm750fb/ddk750_swi2c.c
> @@ -180,7 +180,7 @@ static void sw_i2c_sda(unsigned char value)
> * Return Value:
> * The SDA data bit sent by the Slave
> */
> -static unsigned char sw_i2c_read_sda(void)
> +static bool sw_i2c_read_sda(void)
> {
> unsigned long gpio_dir;
> unsigned long gpio_data;
> @@ -196,9 +196,9 @@ static unsigned char sw_i2c_read_sda(void)
> /* Now read the SDA line */
> gpio_data = peek32(sw_i2c_data_gpio_data_reg);
> if (gpio_data & (1 << sw_i2c_data_gpio))
> - return 1;
> + return true;
> else
> - return 0;
> + return false;
> }
>
> /*
> --
> 2.43.0
>
>
Hi,
This is the friendly patch-bot of Greg Kroah-Hartman. You have sent him
a patch that has triggered this response. He used to manually respond
to these common problems, but in order to save his sanity (he kept
writing the same thing over and over, yet to different people), I was
created. Hopefully you will not take offence and will fix the problem
in your patch and resubmit it so that it can be accepted into the Linux
kernel tree.
You are receiving this message because of the following common error(s)
as indicated below:
- This looks like a new version of a previously submitted patch, but you
did not list below the --- line any changes from the previous version.
Please read the section entitled "The canonical patch format" in the
kernel file, Documentation/process/submitting-patches.rst for what
needs to be done here to properly describe this.
If you wish to discuss this problem further, or you have questions about
how to resolve this issue, please feel free to respond to this email and
Greg will reply once he has dug out from the pending patches received
from other developers.
thanks,
greg k-h's patch email bot
^ permalink raw reply
* [PATCH] fbdev: vt8500lcdfb: fix missing dma_free_coherent()
From: Thomas Fourier @ 2026-01-12 14:00 UTC (permalink / raw)
Cc: Thomas Fourier, stable, Alexey Charkov, Krzysztof Kozlowski,
Helge Deller, Tony Prisk, linux-arm-kernel, linux-fbdev,
dri-devel, linux-kernel
fbi->fb.screen_buffer is alloced with dma_free_coherent() but is not
freed if the error path is reached.
Fixes: e7b995371fe1 ("video: vt8500: Add devicetree support for vt8500-fb and wm8505-fb")
Cc: <stable@vger.kernel.org>
Signed-off-by: Thomas Fourier <fourier.thomas@gmail.com>
---
drivers/video/fbdev/vt8500lcdfb.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/video/fbdev/vt8500lcdfb.c b/drivers/video/fbdev/vt8500lcdfb.c
index b08a6fdc53fd..85c7a99a7d64 100644
--- a/drivers/video/fbdev/vt8500lcdfb.c
+++ b/drivers/video/fbdev/vt8500lcdfb.c
@@ -369,7 +369,7 @@ static int vt8500lcd_probe(struct platform_device *pdev)
if (fbi->palette_cpu == NULL) {
dev_err(&pdev->dev, "Failed to allocate palette buffer\n");
ret = -ENOMEM;
- goto failed_free_io;
+ goto failed_free_mem_virt;
}
irq = platform_get_irq(pdev, 0);
@@ -432,6 +432,9 @@ static int vt8500lcd_probe(struct platform_device *pdev)
failed_free_palette:
dma_free_coherent(&pdev->dev, fbi->palette_size,
fbi->palette_cpu, fbi->palette_phys);
+failed_free_mem_virt:
+ dma_free_coherent(&pdev->dev, fbi->fb.fix.smem_len,
+ fbi->fb.screen_buffer, fbi->fb.fix.smem_start);
failed_free_io:
iounmap(fbi->regbase);
failed_free_res:
--
2.43.0
^ permalink raw reply related
* Re: [PATCH] fbdev: vt8500lcdfb: fix missing dma_free_coherent()
From: Markus Elfring @ 2026-01-12 14:46 UTC (permalink / raw)
To: Thomas Fourier, linux-fbdev, dri-devel, linux-arm-kernel,
Alexey Charkov, Helge Deller, Krzysztof Kozlowski
Cc: stable, LKML
In-Reply-To: <20260112140031.63594-2-fourier.thomas@gmail.com>
> fbi->fb.screen_buffer is alloced with dma_free_coherent() but is not
allocated with a dma_alloc_coherent() call?
https://elixir.bootlin.com/linux/v6.19-rc4/source/drivers/video/fbdev/vt8500lcdfb.c#L352-L362
> freed if the error path is reached.
See also once more:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.19-rc4#n94
You should probably not only specify message recipients in the header field “Cc”.
Regards,
Markus
^ permalink raw reply
* Re: [PATCH v5] staging: fbtft: use dev_of_fbinfo() instead of info->dev
From: kernel test robot @ 2026-01-12 17:31 UTC (permalink / raw)
To: Chintan Patel, linux-fbdev, linux-staging, linux-omap
Cc: oe-kbuild-all, linux-kernel, dri-devel, tzimmermann, andy, deller,
gregkh, Chintan Patel, kernel test robot
In-Reply-To: <20260112010740.186248-1-chintanlike@gmail.com>
Hi Chintan,
kernel test robot noticed the following build warnings:
[auto build test WARNING on staging/staging-testing]
url: https://github.com/intel-lab-lkp/linux/commits/Chintan-Patel/staging-fbtft-use-dev_of_fbinfo-instead-of-info-dev/20260112-091221
base: staging/staging-testing
patch link: https://lore.kernel.org/r/20260112010740.186248-1-chintanlike%40gmail.com
patch subject: [PATCH v5] staging: fbtft: use dev_of_fbinfo() instead of info->dev
config: nios2-allmodconfig (https://download.01.org/0day-ci/archive/20260113/202601130026.1M3d4LWk-lkp@intel.com/config)
compiler: nios2-linux-gcc (GCC) 11.5.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260113/202601130026.1M3d4LWk-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202601130026.1M3d4LWk-lkp@intel.com/
All warnings (new ones prefixed by >>):
drivers/staging/fbtft/fbtft-core.c: In function 'fbtft_fb_setcolreg':
drivers/staging/fbtft/fbtft-core.c:367:30: error: implicit declaration of function 'dev_of_fbinfo'; did you mean 'dev_info'? [-Werror=implicit-function-declaration]
367 | struct device *dev = dev_of_fbinfo(info);
| ^~~~~~~~~~~~~
| dev_info
>> drivers/staging/fbtft/fbtft-core.c:367:30: warning: initialization of 'struct device *' from 'int' makes pointer from integer without a cast [-Wint-conversion]
drivers/staging/fbtft/fbtft-core.c: In function 'fbtft_fb_blank':
drivers/staging/fbtft/fbtft-core.c:393:30: warning: initialization of 'struct device *' from 'int' makes pointer from integer without a cast [-Wint-conversion]
393 | struct device *dev = dev_of_fbinfo(info);
| ^~~~~~~~~~~~~
drivers/staging/fbtft/fbtft-core.c: In function 'fbtft_register_framebuffer':
drivers/staging/fbtft/fbtft-core.c:744:30: warning: initialization of 'struct device *' from 'int' makes pointer from integer without a cast [-Wint-conversion]
744 | struct device *dev = dev_of_fbinfo(fb_info);
| ^~~~~~~~~~~~~
cc1: some warnings being treated as errors
vim +367 drivers/staging/fbtft/fbtft-core.c
360
361 static int fbtft_fb_setcolreg(unsigned int regno, unsigned int red,
362 unsigned int green, unsigned int blue,
363 unsigned int transp, struct fb_info *info)
364 {
365 unsigned int val;
366 int ret = 1;
> 367 struct device *dev = dev_of_fbinfo(info);
368
369 dev_dbg(dev,
370 "%s(regno=%u, red=0x%X, green=0x%X, blue=0x%X, trans=0x%X)\n",
371 __func__, regno, red, green, blue, transp);
372
373 switch (info->fix.visual) {
374 case FB_VISUAL_TRUECOLOR:
375 if (regno < 16) {
376 u32 *pal = info->pseudo_palette;
377
378 val = chan_to_field(red, &info->var.red);
379 val |= chan_to_field(green, &info->var.green);
380 val |= chan_to_field(blue, &info->var.blue);
381
382 pal[regno] = val;
383 ret = 0;
384 }
385 break;
386 }
387 return ret;
388 }
389
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* Re: [PATCH v5] staging: fbtft: use dev_of_fbinfo() instead of info->dev
From: kernel test robot @ 2026-01-12 18:26 UTC (permalink / raw)
To: Chintan Patel, linux-fbdev, linux-staging, linux-omap
Cc: oe-kbuild-all, linux-kernel, dri-devel, tzimmermann, andy, deller,
gregkh, Chintan Patel, kernel test robot
In-Reply-To: <20260112010740.186248-1-chintanlike@gmail.com>
Hi Chintan,
kernel test robot noticed the following build errors:
[auto build test ERROR on staging/staging-testing]
url: https://github.com/intel-lab-lkp/linux/commits/Chintan-Patel/staging-fbtft-use-dev_of_fbinfo-instead-of-info-dev/20260112-091221
base: staging/staging-testing
patch link: https://lore.kernel.org/r/20260112010740.186248-1-chintanlike%40gmail.com
patch subject: [PATCH v5] staging: fbtft: use dev_of_fbinfo() instead of info->dev
config: sh-allmodconfig (https://download.01.org/0day-ci/archive/20260113/202601130116.rVB5C8Xx-lkp@intel.com/config)
compiler: sh4-linux-gcc (GCC) 15.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260113/202601130116.rVB5C8Xx-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202601130116.rVB5C8Xx-lkp@intel.com/
All errors (new ones prefixed by >>):
drivers/staging/fbtft/fbtft-core.c: In function 'fbtft_fb_setcolreg':
>> drivers/staging/fbtft/fbtft-core.c:367:30: error: implicit declaration of function 'dev_of_fbinfo'; did you mean 'dev_info'? [-Wimplicit-function-declaration]
367 | struct device *dev = dev_of_fbinfo(info);
| ^~~~~~~~~~~~~
| dev_info
>> drivers/staging/fbtft/fbtft-core.c:367:30: error: initialization of 'struct device *' from 'int' makes pointer from integer without a cast [-Wint-conversion]
drivers/staging/fbtft/fbtft-core.c: In function 'fbtft_fb_blank':
drivers/staging/fbtft/fbtft-core.c:393:30: error: initialization of 'struct device *' from 'int' makes pointer from integer without a cast [-Wint-conversion]
393 | struct device *dev = dev_of_fbinfo(info);
| ^~~~~~~~~~~~~
drivers/staging/fbtft/fbtft-core.c: In function 'fbtft_register_framebuffer':
drivers/staging/fbtft/fbtft-core.c:744:30: error: initialization of 'struct device *' from 'int' makes pointer from integer without a cast [-Wint-conversion]
744 | struct device *dev = dev_of_fbinfo(fb_info);
| ^~~~~~~~~~~~~
vim +367 drivers/staging/fbtft/fbtft-core.c
360
361 static int fbtft_fb_setcolreg(unsigned int regno, unsigned int red,
362 unsigned int green, unsigned int blue,
363 unsigned int transp, struct fb_info *info)
364 {
365 unsigned int val;
366 int ret = 1;
> 367 struct device *dev = dev_of_fbinfo(info);
368
369 dev_dbg(dev,
370 "%s(regno=%u, red=0x%X, green=0x%X, blue=0x%X, trans=0x%X)\n",
371 __func__, regno, red, green, blue, transp);
372
373 switch (info->fix.visual) {
374 case FB_VISUAL_TRUECOLOR:
375 if (regno < 16) {
376 u32 *pal = info->pseudo_palette;
377
378 val = chan_to_field(red, &info->var.red);
379 val |= chan_to_field(green, &info->var.green);
380 val |= chan_to_field(blue, &info->var.blue);
381
382 pal[regno] = val;
383 ret = 0;
384 }
385 break;
386 }
387 return ret;
388 }
389
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* Re: [PATCH v1 1/2] dt-bindings: backlight: gpio-backlight: allow multiple GPIOs
From: tessolveupstream @ 2026-01-13 4:45 UTC (permalink / raw)
To: Daniel Thompson
Cc: lee, danielt, jingoohan1, deller, pavel, robh, krzk+dt, conor+dt,
dri-devel, linux-fbdev, linux-leds, devicetree, linux-kernel
In-Reply-To: <aVuKdAyXfWLs-WJI@aspen.lan>
On 05-01-2026 15:25, Daniel Thompson wrote:
> On Mon, Jan 05, 2026 at 02:21:19PM +0530, Sudarshan Shetty wrote:
>> Update the gpio-backlight binding to support configurations that require
>> more than one GPIO for enabling/disabling the backlight.
>>
>> Signed-off-by: Sudarshan Shetty <tessolveupstream@gmail.com>
>> ---
>> .../bindings/leds/backlight/gpio-backlight.yaml | 12 +++++++++++-
>> 1 file changed, 11 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/devicetree/bindings/leds/backlight/gpio-backlight.yaml b/Documentation/devicetree/bindings/leds/backlight/gpio-backlight.yaml
>> index 584030b6b0b9..1483ce4a3480 100644
>> --- a/Documentation/devicetree/bindings/leds/backlight/gpio-backlight.yaml
>> +++ b/Documentation/devicetree/bindings/leds/backlight/gpio-backlight.yaml
>> @@ -17,7 +17,8 @@ properties:
>>
>> gpios:
>> description: The gpio that is used for enabling/disabling the backlight.
>> - maxItems: 1
>> + minItems: 1
>> + maxItems: 2
>
> Why 2?
>
In the current design, the LVDS panel has a single backlight that
is controlled by two GPIOs. Initially, It described as two separate
backlight devices using the same gpio-backlight driver, since the
existing driver supports only one GPIO per instance.
So the maintainer suggested to extend the gpio-backlight driver
and bindings to support multiple GPIOs.
https://lore.kernel.org/all/q63bdon55app4gb2il5e7skyc6z2amcnaiqbqlhen7arkxphtb@3jejbelji2ti/
>
> Daniel.
^ permalink raw reply
* [PATCH v6] staging: fbtft: Use fbdev logging helpers when FB_DEVICE is disabled
From: Chintan Patel @ 2026-01-13 4:59 UTC (permalink / raw)
To: linux-fbdev, linux-staging, linux-omap
Cc: linux-kernel, dri-devel, tzimmermann, andy, deller, gregkh,
Chintan Patel, kernel test robot
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
^ permalink raw reply related
* Re: [PATCH v6] staging: fbtft: Use fbdev logging helpers when FB_DEVICE is disabled
From: Greg KH @ 2026-01-13 6:16 UTC (permalink / raw)
To: Chintan Patel
Cc: linux-fbdev, linux-staging, linux-omap, linux-kernel, dri-devel,
tzimmermann, andy, deller, kernel test robot
In-Reply-To: <20260113045909.336931-1-chintanlike@gmail.com>
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
^ permalink raw reply
* Re: [PATCH v1 2/2] backlight: gpio: add support for multiple GPIOs for backlight control
From: tessolveupstream @ 2026-01-13 7:17 UTC (permalink / raw)
To: Daniel Thompson
Cc: lee, danielt, jingoohan1, deller, pavel, robh, krzk+dt, conor+dt,
dri-devel, linux-fbdev, linux-leds, devicetree, linux-kernel
In-Reply-To: <aVuN7zVUWJ1qsVh8@aspen.lan>
On 05-01-2026 15:39, Daniel Thompson wrote:
> On Mon, Jan 05, 2026 at 02:21:20PM +0530, Sudarshan Shetty wrote:
>> Extend the gpio-backlight driver to handle multiple GPIOs instead of a
>> single one. This allows panels that require driving several enable pins
>> to be controlled by the backlight framework.
>>
>> Signed-off-by: Sudarshan Shetty <tessolveupstream@gmail.com>
>> ---
>> drivers/video/backlight/gpio_backlight.c | 61 +++++++++++++++++-------
>> 1 file changed, 45 insertions(+), 16 deletions(-)
>>
>> diff --git a/drivers/video/backlight/gpio_backlight.c b/drivers/video/backlight/gpio_backlight.c
>> index 728a546904b0..037e1c111e48 100644
>> --- a/drivers/video/backlight/gpio_backlight.c
>> +++ b/drivers/video/backlight/gpio_backlight.c
>> @@ -17,14 +17,18 @@
>>
>> struct gpio_backlight {
>> struct device *dev;
>> - struct gpio_desc *gpiod;
>> + struct gpio_desc **gpiods;
>> + unsigned int num_gpios;
>
> Why not use struct gpio_descs for this?
>
> Once you do that, then most of the gbl->num_gpios loops can be replaced with
> calls to the array based accessors.
>
Based on your feedback, I have updated the implementation to use
struct gpio_descs and array-based accessors, as recommended like
below:
git diff drivers/video/backlight/gpio_backlight.c
diff --git a/drivers/video/backlight/gpio_backlight.c b/drivers/video/backlight/gpio_backlight.c
index 037e1c111e48..e99d7a9dc670 100644
--- a/drivers/video/backlight/gpio_backlight.c
+++ b/drivers/video/backlight/gpio_backlight.c
@@ -14,22 +14,37 @@
#include <linux/platform_device.h>
#include <linux/property.h>
#include <linux/slab.h>
+#include <linux/bitmap.h>
struct gpio_backlight {
struct device *dev;
- struct gpio_desc **gpiods;
+ struct gpio_descs *gpiods;
unsigned int num_gpios;
};
static int gpio_backlight_update_status(struct backlight_device *bl)
{
struct gpio_backlight *gbl = bl_get_data(bl);
- unsigned int i;
+ unsigned int n = gbl->num_gpios;
int br = backlight_get_brightness(bl);
+ unsigned long *value_bitmap;
+ int words = BITS_TO_LONGS(n);
+
+ value_bitmap = kcalloc(words, sizeof(unsigned long), GFP_KERNEL);
+ if (!value_bitmap)
+ return -ENOMEM;
+
+ if (br)
+ bitmap_fill(value_bitmap, n);
+ else
+ bitmap_zero(value_bitmap, n);
- for (i = 0; i < gbl->num_gpios; i++)
- gpiod_set_value_cansleep(gbl->gpiods[i], br);
+ gpiod_set_array_value_cansleep(gbl->gpiods->ndescs,
+ gbl->gpiods->desc,
+ gbl->gpiods->info,
+ value_bitmap);
+ kfree(value_bitmap);
return 0;
}
@@ -67,22 +82,18 @@ static int gpio_backlight_probe(struct platform_device *pdev)
def_value = device_property_read_bool(dev, "default-on");
- gbl->num_gpios = gpiod_count(dev, NULL);
+ gbl->gpiods = devm_gpiod_get_array(dev, NULL, GPIOD_ASIS);
+ if (IS_ERR(gbl->gpiods)) {
+ if (PTR_ERR(gbl->gpiods) == -ENODEV)
+ return dev_err_probe(dev, -EINVAL,
+ "The gpios parameter is missing or invalid\n");
+ return PTR_ERR(gbl->gpiods);
+ }
+
+ gbl->num_gpios = gbl->gpiods->ndescs;
if (gbl->num_gpios == 0)
return dev_err_probe(dev, -EINVAL,
"The gpios parameter is missing or invalid\n");
- gbl->gpiods = devm_kcalloc(dev, gbl->num_gpios, sizeof(*gbl->gpiods),
- GFP_KERNEL);
- if (!gbl->gpiods)
- return -ENOMEM;
-
- for (i = 0; i < gbl->num_gpios; i++) {
- gbl->gpiods[i] =
- devm_gpiod_get_index(dev, NULL, i, GPIOD_ASIS);
- if (IS_ERR(gbl->gpiods[i]))
- return dev_err_probe(dev, PTR_ERR(gbl->gpiods[i]),
- "Failed to get GPIO at index %u\n", i);
- }
memset(&props, 0, sizeof(props));
props.type = BACKLIGHT_RAW;
@@ -101,14 +112,26 @@ static int gpio_backlight_probe(struct platform_device *pdev)
: BACKLIGHT_POWER_OFF;
} else {
bool all_high = true;
-
- for (i = 0; i < gbl->num_gpios; i++) {
- if (gpiod_get_value_cansleep(gbl->gpiods[i]) != 0) {
- all_high = false;
- break;
- }
+ unsigned long *value_bitmap;
+ int words = BITS_TO_LONGS(gbl->num_gpios);
+
+ value_bitmap = kcalloc(words, sizeof(unsigned long), GFP_KERNEL);
+ if (!value_bitmap)
+ return -ENOMEM;
+
+ ret = gpiod_get_array_value_cansleep(gbl->gpiods->ndescs,
+ gbl->gpiods->desc,
+ gbl->gpiods->info,
+ value_bitmap);
+ if (ret) {
+ kfree(value_bitmap);
+ return dev_err_probe(dev, ret,
+ "failed to read initial gpio values\n");
}
+ all_high = bitmap_full(value_bitmap, gbl->num_gpios);
+
+ kfree(value_bitmap);
bl->props.power =
all_high ? BACKLIGHT_POWER_ON : BACKLIGHT_POWER_OFF;
}
@@ -118,7 +141,7 @@ static int gpio_backlight_probe(struct platform_device *pdev)
init_brightness = backlight_get_brightness(bl);
for (i = 0; i < gbl->num_gpios; i++) {
- ret = gpiod_direction_output(gbl->gpiods[i], init_brightness);
+ ret = gpiod_direction_output(gbl->gpiods->desc[i], init_brightness);
if (ret)
return dev_err_probe(dev, ret,
"failed to set gpio %u direction\n",
Could you please share your thoughts on whether this approach
aligns with your expectations?
>
>> };
>>
>> static int gpio_backlight_update_status(struct backlight_device *bl)
>> {
>> struct gpio_backlight *gbl = bl_get_data(bl);
>> + unsigned int i;
>> + int br = backlight_get_brightness(bl);
>>
>> - gpiod_set_value_cansleep(gbl->gpiod, backlight_get_brightness(bl));
>> + for (i = 0; i < gbl->num_gpios; i++)
>> + gpiod_set_value_cansleep(gbl->gpiods[i], br);
>>
>> return 0;
>> }
>> @@ -52,6 +56,7 @@ static int gpio_backlight_probe(struct platform_device *pdev)
>> struct backlight_device *bl;
>> struct gpio_backlight *gbl;
>> int ret, init_brightness, def_value;
>> + unsigned int i;
>>
>> gbl = devm_kzalloc(dev, sizeof(*gbl), GFP_KERNEL);
>> if (gbl == NULL)
>> @@ -62,10 +67,22 @@ static int gpio_backlight_probe(struct platform_device *pdev)
>>
>> def_value = device_property_read_bool(dev, "default-on");
>>
>> - gbl->gpiod = devm_gpiod_get(dev, NULL, GPIOD_ASIS);
>> - if (IS_ERR(gbl->gpiod))
>> - return dev_err_probe(dev, PTR_ERR(gbl->gpiod),
>> - "The gpios parameter is missing or invalid\n");
>> + gbl->num_gpios = gpiod_count(dev, NULL);
>> + if (gbl->num_gpios == 0)
>> + return dev_err_probe(dev, -EINVAL,
>> + "The gpios parameter is missing or invalid\n");
>> + gbl->gpiods = devm_kcalloc(dev, gbl->num_gpios, sizeof(*gbl->gpiods),
>> + GFP_KERNEL);
>> + if (!gbl->gpiods)
>> + return -ENOMEM;
>
> This is definitely easier if you simply use devm_get_array().
>
>
>> +
>> + for (i = 0; i < gbl->num_gpios; i++) {
>> + gbl->gpiods[i] =
>> + devm_gpiod_get_index(dev, NULL, i, GPIOD_ASIS);
>> + if (IS_ERR(gbl->gpiods[i]))
>> + return dev_err_probe(dev, PTR_ERR(gbl->gpiods[i]),
>> + "Failed to get GPIO at index %u\n", i);
>> + }
>>
>> memset(&props, 0, sizeof(props));
>> props.type = BACKLIGHT_RAW;
>> @@ -78,22 +95,34 @@ static int gpio_backlight_probe(struct platform_device *pdev)
>> }
>>
>> /* Set the initial power state */
>> - if (!of_node || !of_node->phandle)
>> + if (!of_node || !of_node->phandle) {
>> /* Not booted with device tree or no phandle link to the node */
>> bl->props.power = def_value ? BACKLIGHT_POWER_ON
>> - : BACKLIGHT_POWER_OFF;
>> - else if (gpiod_get_value_cansleep(gbl->gpiod) == 0)
>> - bl->props.power = BACKLIGHT_POWER_OFF;
>> - else
>> - bl->props.power = BACKLIGHT_POWER_ON;
>> + : BACKLIGHT_POWER_OFF;
>> + } else {
>> + bool all_high = true;
>> +
>> + for (i = 0; i < gbl->num_gpios; i++) {
>> + if (gpiod_get_value_cansleep(gbl->gpiods[i]) != 0) {
>
> Why is there a != here?
>
>
>> + all_high = false;
>> + break;
>> + }
>> + }
>> +
>> + bl->props.power =
>> + all_high ? BACKLIGHT_POWER_ON : BACKLIGHT_POWER_OFF;
>> + }
>>
>> bl->props.brightness = 1;
>>
>> init_brightness = backlight_get_brightness(bl);
>> - ret = gpiod_direction_output(gbl->gpiod, init_brightness);
>> - if (ret) {
>> - dev_err(dev, "failed to set initial brightness\n");
>> - return ret;
>> +
>> + for (i = 0; i < gbl->num_gpios; i++) {
>> + ret = gpiod_direction_output(gbl->gpiods[i], init_brightness);
>> + if (ret)
>> + return dev_err_probe(dev, ret,
>> + "failed to set gpio %u direction\n",
>> + i);
>> }
>>
>> platform_set_drvdata(pdev, bl);
>
>
> Daniel.
^ permalink raw reply related
* [PATCH] staging: fbtft: Change udelay() to usleep_range()
From: Gideon Adjei @ 2026-01-13 17:27 UTC (permalink / raw)
To: Andy Shevchenko, Greg Kroah-Hartman
Cc: Abdun Nihaal, Dan Carpenter, Jianglei Nie, Matthew Wilcox,
dri-devel, linux-fbdev, linux-staging, linux-kernel, Gideon Adjei
Replace udelay() calls >= 100us with usleep_range() to avoid busy-waiting.
This allows the scheduler to run other tasks while waiting.
Signed-off-by: Gideon Adjei <gideonadjei.dev@gmail.com>
---
drivers/staging/fbtft/fb_tinylcd.c | 2 +-
drivers/staging/fbtft/fb_upd161704.c | 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/fbtft/fb_tinylcd.c b/drivers/staging/fbtft/fb_tinylcd.c
index 9469248f2c50..ee8d6b10374a 100644
--- a/drivers/staging/fbtft/fb_tinylcd.c
+++ b/drivers/staging/fbtft/fb_tinylcd.c
@@ -41,7 +41,7 @@ static int init_display(struct fbtft_par *par)
0x00, 0x35, 0x33, 0x00, 0x00, 0x00);
write_reg(par, MIPI_DCS_SET_PIXEL_FORMAT, 0x55);
write_reg(par, MIPI_DCS_EXIT_SLEEP_MODE);
- udelay(250);
+ usleep_range(250, 400);
write_reg(par, MIPI_DCS_SET_DISPLAY_ON);
return 0;
diff --git a/drivers/staging/fbtft/fb_upd161704.c b/drivers/staging/fbtft/fb_upd161704.c
index c680160d6380..aed5cc7cabde 100644
--- a/drivers/staging/fbtft/fb_upd161704.c
+++ b/drivers/staging/fbtft/fb_upd161704.c
@@ -32,7 +32,7 @@ static int init_display(struct fbtft_par *par)
/* oscillator start */
write_reg(par, 0x003A, 0x0001); /*Oscillator 0: stop, 1: operation */
- udelay(100);
+ usleep_range(100, 200);
/* y-setting */
write_reg(par, 0x0024, 0x007B); /* amplitude setting */
@@ -60,7 +60,7 @@ static int init_display(struct fbtft_par *par)
/* Power supply setting */
write_reg(par, 0x0019, 0x0000); /* DC/DC output setting */
- udelay(200);
+ usleep_range(200, 400);
write_reg(par, 0x001A, 0x1000); /* DC/DC frequency setting */
write_reg(par, 0x001B, 0x0023); /* DC/DC rising setting */
write_reg(par, 0x001C, 0x0C01); /* Regulator voltage setting */
--
2.34.1
^ permalink raw reply related
* Re: [PATCH] staging: fbtft: Change udelay() to usleep_range()
From: Matthew Wilcox @ 2026-01-13 17:53 UTC (permalink / raw)
To: Gideon Adjei
Cc: Andy Shevchenko, Greg Kroah-Hartman, Abdun Nihaal, Dan Carpenter,
Jianglei Nie, dri-devel, linux-fbdev, linux-staging, linux-kernel
In-Reply-To: <20260113172756.14744-1-gideonadjei.dev@gmail.com>
On Tue, Jan 13, 2026 at 09:27:56AM -0800, Gideon Adjei wrote:
> Replace udelay() calls >= 100us with usleep_range() to avoid busy-waiting.
> This allows the scheduler to run other tasks while waiting.
Your commit message omits how you know it's safe to sleep in these
situations. Are we holding a spinlock or being called in interrupt
context, for example?
^ permalink raw reply
* [PATCH v2] staging: fbtft: Change udelay() to usleep_range()
From: Gideon Adjei @ 2026-01-13 21:05 UTC (permalink / raw)
To: Andy Shevchenko, Greg Kroah-Hartman
Cc: Abdun Nihaal, Dan Carpenter, Jianglei Nie, Matthew Wilcox,
dri-devel, linux-fbdev, linux-staging, linux-kernel, Gideon Adjei
Replace udelay() calls >= 100us with usleep_range() to avoid busy-waiting.
The delays are used in init_display() callbacks. These callbacks are
invoked by fbtft_probe_common() during the driver's probe path. the
probe path runs in process context which already uses sleeping APIs.
This makes usleep_range() safe to use in these situations.
Signed-off-by: Gideon Adjei <gideonadjei.dev@gmail.com>
---
drivers/staging/fbtft/fb_tinylcd.c | 2 +-
drivers/staging/fbtft/fb_upd161704.c | 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/fbtft/fb_tinylcd.c b/drivers/staging/fbtft/fb_tinylcd.c
index 9469248f2c50..ee8d6b10374a 100644
--- a/drivers/staging/fbtft/fb_tinylcd.c
+++ b/drivers/staging/fbtft/fb_tinylcd.c
@@ -41,7 +41,7 @@ static int init_display(struct fbtft_par *par)
0x00, 0x35, 0x33, 0x00, 0x00, 0x00);
write_reg(par, MIPI_DCS_SET_PIXEL_FORMAT, 0x55);
write_reg(par, MIPI_DCS_EXIT_SLEEP_MODE);
- udelay(250);
+ usleep_range(250, 400);
write_reg(par, MIPI_DCS_SET_DISPLAY_ON);
return 0;
diff --git a/drivers/staging/fbtft/fb_upd161704.c b/drivers/staging/fbtft/fb_upd161704.c
index c680160d6380..aed5cc7cabde 100644
--- a/drivers/staging/fbtft/fb_upd161704.c
+++ b/drivers/staging/fbtft/fb_upd161704.c
@@ -32,7 +32,7 @@ static int init_display(struct fbtft_par *par)
/* oscillator start */
write_reg(par, 0x003A, 0x0001); /*Oscillator 0: stop, 1: operation */
- udelay(100);
+ usleep_range(100, 200);
/* y-setting */
write_reg(par, 0x0024, 0x007B); /* amplitude setting */
@@ -60,7 +60,7 @@ static int init_display(struct fbtft_par *par)
/* Power supply setting */
write_reg(par, 0x0019, 0x0000); /* DC/DC output setting */
- udelay(200);
+ usleep_range(200, 400);
write_reg(par, 0x001A, 0x1000); /* DC/DC frequency setting */
write_reg(par, 0x001B, 0x0023); /* DC/DC rising setting */
write_reg(par, 0x001C, 0x0C01); /* Regulator voltage setting */
--
2.34.1
^ permalink raw reply related
* Re: [PATCH v2] staging: fbtft: Change udelay() to usleep_range()
From: Andy Shevchenko @ 2026-01-13 21:13 UTC (permalink / raw)
To: Gideon Adjei
Cc: Andy Shevchenko, Greg Kroah-Hartman, Abdun Nihaal, Dan Carpenter,
Jianglei Nie, Matthew Wilcox, dri-devel, linux-fbdev,
linux-staging, linux-kernel
In-Reply-To: <20260113210559.3020-1-gideonadjei.dev@gmail.com>
On Tue, Jan 13, 2026 at 11:06 PM Gideon Adjei <gideonadjei.dev@gmail.com> wrote:
>
> Replace udelay() calls >= 100us with usleep_range() to avoid busy-waiting.
>
> The delays are used in init_display() callbacks. These callbacks are
> invoked by fbtft_probe_common() during the driver's probe path. the
> probe path runs in process context which already uses sleeping APIs.
> This makes usleep_range() safe to use in these situations.
Nice, now can we switch to modern API, i.e. fsleep()?
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* [PATCH v3] staging: fbtft: Change udelay() to fsleep()
From: Gideon Adjei @ 2026-01-13 22:17 UTC (permalink / raw)
To: Andy Shevchenko, Greg Kroah-Hartman
Cc: Abdun Nihaal, Dan Carpenter, Jianglei Nie, Matthew Wilcox,
dri-devel, linux-fbdev, linux-staging, linux-kernel, Gideon Adjei
Replace udelay() calls >= 100us with fsleep() to avoid busy-waiting.
The delays are used in init_display() callbacks. These callbacks are
invoked by fbtft_probe_common() during the driver's probe path. The
probe path runs in process context which already uses sleeping APIs.
This makes fsleep() safe to use in these situations.
Signed-off-by: Gideon Adjei <gideonadjei.dev@gmail.com>
---
drivers/staging/fbtft/fb_tinylcd.c | 2 +-
drivers/staging/fbtft/fb_upd161704.c | 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/fbtft/fb_tinylcd.c b/drivers/staging/fbtft/fb_tinylcd.c
index 9469248f2c50..3fb15df31592 100644
--- a/drivers/staging/fbtft/fb_tinylcd.c
+++ b/drivers/staging/fbtft/fb_tinylcd.c
@@ -41,7 +41,7 @@ static int init_display(struct fbtft_par *par)
0x00, 0x35, 0x33, 0x00, 0x00, 0x00);
write_reg(par, MIPI_DCS_SET_PIXEL_FORMAT, 0x55);
write_reg(par, MIPI_DCS_EXIT_SLEEP_MODE);
- udelay(250);
+ fsleep(250);
write_reg(par, MIPI_DCS_SET_DISPLAY_ON);
return 0;
diff --git a/drivers/staging/fbtft/fb_upd161704.c b/drivers/staging/fbtft/fb_upd161704.c
index c680160d6380..7fe2b556e17c 100644
--- a/drivers/staging/fbtft/fb_upd161704.c
+++ b/drivers/staging/fbtft/fb_upd161704.c
@@ -32,7 +32,7 @@ static int init_display(struct fbtft_par *par)
/* oscillator start */
write_reg(par, 0x003A, 0x0001); /*Oscillator 0: stop, 1: operation */
- udelay(100);
+ fsleep(100);
/* y-setting */
write_reg(par, 0x0024, 0x007B); /* amplitude setting */
@@ -60,7 +60,7 @@ static int init_display(struct fbtft_par *par)
/* Power supply setting */
write_reg(par, 0x0019, 0x0000); /* DC/DC output setting */
- udelay(200);
+ fsleep(200);
write_reg(par, 0x001A, 0x1000); /* DC/DC frequency setting */
write_reg(par, 0x001B, 0x0023); /* DC/DC rising setting */
write_reg(par, 0x001C, 0x0C01); /* Regulator voltage setting */
--
2.34.1
^ permalink raw reply related
* Re: [PATCH v3] staging: fbtft: Change udelay() to fsleep()
From: Andy Shevchenko @ 2026-01-13 22:33 UTC (permalink / raw)
To: Gideon Adjei
Cc: Andy Shevchenko, Greg Kroah-Hartman, Abdun Nihaal, Dan Carpenter,
Jianglei Nie, Matthew Wilcox, dri-devel, linux-fbdev,
linux-staging, linux-kernel
In-Reply-To: <20260113221722.5157-1-gideonadjei.dev@gmail.com>
On Wed, Jan 14, 2026 at 12:17 AM Gideon Adjei <gideonadjei.dev@gmail.com> wrote:
>
> Replace udelay() calls >= 100us with fsleep() to avoid busy-waiting.
>
> The delays are used in init_display() callbacks. These callbacks are
> invoked by fbtft_probe_common() during the driver's probe path. The
> probe path runs in process context which already uses sleeping APIs.
> This makes fsleep() safe to use in these situations.
You forgot to add a changelog...
> Signed-off-by: Gideon Adjei <gideonadjei.dev@gmail.com>
> ---
...somewhere here.
No need to send v4 because of this, just reply with the changelogs for
v1->v2 and v2->v3.
Also note, it's assumed that even for such a simple patch the time
between versions is at least 24h.
> drivers/staging/fbtft/fb_tinylcd.c | 2 +-
> drivers/staging/fbtft/fb_upd161704.c | 4 ++--
> 2 files changed, 3 insertions(+), 3 deletions(-)
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH v3] staging: fbtft: Change udelay() to fsleep()
From: Gideon Adjei @ 2026-01-14 0:30 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Greg Kroah-Hartman, Abdun Nihaal, Dan Carpenter, Jianglei Nie,
Matthew Wilcox, dri-devel, linux-fbdev, linux-staging,
linux-kernel
In-Reply-To: <CAHp75Ved+zhACJ9CYLYnkzW6Z52fmZT0VY+0NWKYk6O38HCkdg@mail.gmail.com>
On Wed, Jan 14, 2026 at 12:33:42AM +0200, Andy Shevchenko wrote:
>No need to send v4 because of this, just reply with the changelogs for
>v1->v2 and v2->v3.
>
Changelog:
v1->v2:
- Added explanation to why usleep_range() is safe in this context to
commit message.
v2->v3:
- Switched from usleep_range() to fsleep().
- Corrected a minor grammar error in commit message.
No functional changes to the code itself.
>Also note, it's assumed that even for such a simple patch the time
>between versions is at least 24h.
Thank you for the reminder regarding the expected interval between
revions. I will ensure this observed going forward.
Signed-off-by: Gideon Adjei <gideonadjei.dev@gmail.com>
^ permalink raw reply
* Re: [PATCH v6] staging: fbtft: Use fbdev logging helpers when FB_DEVICE is disabled
From: Chintan Patel @ 2026-01-14 4:47 UTC (permalink / raw)
To: Greg KH
Cc: linux-fbdev, linux-staging, linux-omap, linux-kernel, dri-devel,
tzimmermann, andy, deller, kernel test robot
In-Reply-To: <2026011341-chomp-protegee-6be5@gregkh>
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.
^ permalink raw reply
* Re: [PATCH v3] staging: fbtft: Change udelay() to fsleep()
From: Greg Kroah-Hartman @ 2026-01-14 6:55 UTC (permalink / raw)
To: Gideon Adjei
Cc: Andy Shevchenko, Abdun Nihaal, Dan Carpenter, Jianglei Nie,
Matthew Wilcox, dri-devel, linux-fbdev, linux-staging,
linux-kernel
In-Reply-To: <20260113221722.5157-1-gideonadjei.dev@gmail.com>
On Tue, Jan 13, 2026 at 02:17:22PM -0800, Gideon Adjei wrote:
> Replace udelay() calls >= 100us with fsleep() to avoid busy-waiting.
>
> The delays are used in init_display() callbacks. These callbacks are
> invoked by fbtft_probe_common() during the driver's probe path. The
> probe path runs in process context which already uses sleeping APIs.
> This makes fsleep() safe to use in these situations.
>
> Signed-off-by: Gideon Adjei <gideonadjei.dev@gmail.com>
> ---
> drivers/staging/fbtft/fb_tinylcd.c | 2 +-
> drivers/staging/fbtft/fb_upd161704.c | 4 ++--
> 2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/staging/fbtft/fb_tinylcd.c b/drivers/staging/fbtft/fb_tinylcd.c
> index 9469248f2c50..3fb15df31592 100644
> --- a/drivers/staging/fbtft/fb_tinylcd.c
> +++ b/drivers/staging/fbtft/fb_tinylcd.c
> @@ -41,7 +41,7 @@ static int init_display(struct fbtft_par *par)
> 0x00, 0x35, 0x33, 0x00, 0x00, 0x00);
> write_reg(par, MIPI_DCS_SET_PIXEL_FORMAT, 0x55);
> write_reg(par, MIPI_DCS_EXIT_SLEEP_MODE);
> - udelay(250);
> + fsleep(250);
Without the hardware and specifications to test this with, we can't take
this type of patch. Please see the mailing list archives for the
multitude of times it has been rejected in the past.
sorry, but checkpatch can be ignored here.
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH v6] staging: fbtft: Use fbdev logging helpers when FB_DEVICE is disabled
From: Andy Shevchenko @ 2026-01-14 7:15 UTC (permalink / raw)
To: Chintan Patel
Cc: Greg KH, linux-fbdev, linux-staging, linux-omap, linux-kernel,
dri-devel, tzimmermann, andy, deller, kernel test robot
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
^ permalink raw reply
* Re: [PATCH v6] staging: fbtft: Use fbdev logging helpers when FB_DEVICE is disabled
From: Thomas Zimmermann @ 2026-01-14 11:38 UTC (permalink / raw)
To: Greg KH, Chintan Patel
Cc: linux-fbdev, linux-staging, linux-omap, linux-kernel, dri-devel,
andy, deller, kernel test robot
In-Reply-To: <2026011341-chomp-protegee-6be5@gregkh>
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)
^ permalink raw reply
* printk's threaded legacy console + fbcon => schedule where it should not
From: Sebastian Andrzej Siewior @ 2026-01-14 14:59 UTC (permalink / raw)
To: linux-kernel, linux-serial, linux-fbdev
Cc: Petr Mladek, Steven Rostedt, John Ogness, Sergey Senozhatsky,
Greg Kroah-Hartman, Jiri Slaby, Simona Vetter, Helge Deller
Hi,
legacy_kthread_func() does console_lock() which means
console_may_schedule is 1.
The other path is from vprintk_emit() where we have
if (ft.legacy_direct) {
preempt_disable();
if (console_trylock_spinning())
console_unlock();
preempt_enable();
}
so all printing happens from console_unlock() where
console_may_schedule is 0. This is a small difference. With the legacy
console enabled I get:
| BUG: sleeping function called from invalid context at kernel/printk/printk.c:3377
| in_atomic(): 1, irqs_disabled(): 0, non_block: 0, pid: 15, name: pr/legacy
| preempt_count: 1, expected: 0
| RCU nest depth: 0, expected: 0
| 3 locks held by pr/legacy/15:
| #0: ffffffffa8aebac0 (console_lock){+.+.}-{0:0}, at: legacy_kthread_func+0x6c/0x130
| #1: ffffffffa8aebb18 (console_srcu){....}-{0:0}, at: console_flush_one_record+0x7e/0x4d0
| #2: ffffffffa8c49818 (printing_lock){+.+.}-{3:3}, at: vt_console_print+0x55/0x490
| Preemption disabled at:
| [<0000000000000000>] 0x0
| CPU: 7 UID: 0 PID: 15 Comm: pr/legacy Not tainted 6.19.0-rc5+ #19 PREEMPT(lazy)
| Hardware name: To Be Filled By O.E.M. To Be Filled By O.E.M./Z68 Pro3-M, BIOS P2.30 06/29/2012
| Call Trace:
| <TASK>
| dump_stack_lvl+0x68/0x90
| __might_resched.cold+0xf0/0x12b
| console_conditional_schedule+0x27/0x30
| fbcon_redraw+0xa0/0x240
| fbcon_scroll+0x164/0x1c0
| con_scroll+0xfa/0x200
| lf+0xa5/0xb0
| vt_console_print+0x313/0x490
| console_flush_one_record+0x2a0/0x4d0
| legacy_kthread_func+0x83/0x130
| kthread+0x118/0x250
| ret_from_fork+0x309/0x3b0
| ret_from_fork_asm+0x1a/0x30
| </TASK>
because vt_console_print() acquires a spin_lock for synchronisation
against another caller while console_conditional_schedule() would like
to schedule.
Most callers of console_unlock() do trylock except for few such as
__pr_flush() which are affected by this the same way as the legacy
printing thread. But we don't have much pr_flush() so this is hidden.
Is there a strict need for fbcon_scroll() to schedule in fbcon_redraw()?
From a quick look it looks that intense callers such the printk flush do
cond_resched() on their own and tty does it, too
| fbcon_scroll+0x164/0x1c0
| con_scroll+0xfa/0x200
| lf+0xa5/0xb0
| do_con_write+0xc68/0x2630
| con_write+0xf/0x40
| do_output_char+0x180/0x1e0
| n_tty_write+0x1ba/0x580
| file_tty_write.isra.0+0x17e/0x2c0
the cond_resched() is in file_tty_write()/ iterate_tty_write().
Therefore I would suggest to simply
diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
index 59b4b5e126ba1..53daf7614b1af 100644
--- a/drivers/tty/vt/vt.c
+++ b/drivers/tty/vt/vt.c
@@ -3236,7 +3236,6 @@ static int do_con_write(struct tty_struct *tty, const u8 *buf, int count)
goto rescan_last_byte;
}
con_flush(vc, &draw);
- console_conditional_schedule();
notify_update(vc);
return n;
diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
index 7be9e865325d9..36dd9d4a46ae0 100644
--- a/drivers/video/fbdev/core/fbcon.c
+++ b/drivers/video/fbdev/core/fbcon.c
@@ -1607,12 +1607,10 @@ static void fbcon_redraw_move(struct vc_data *vc, struct fbcon_display *p,
start = s;
}
}
- console_conditional_schedule();
s++;
} while (s < le);
if (s > start)
fbcon_putcs(vc, start, s - start, dy, x);
- console_conditional_schedule();
dy++;
}
}
@@ -1648,14 +1646,12 @@ static void fbcon_redraw_blit(struct vc_data *vc, struct fb_info *info,
}
scr_writew(c, d);
- console_conditional_schedule();
s++;
d++;
} while (s < le);
if (s > start)
par->bitops->bmove(vc, info, line + ycount, x, line, x, 1,
s - start);
- console_conditional_schedule();
if (ycount > 0)
line++;
else {
@@ -1703,13 +1699,11 @@ static void fbcon_redraw(struct vc_data *vc, int line, int count, int offset)
}
}
scr_writew(c, d);
- console_conditional_schedule();
s++;
d++;
} while (s < le);
if (s > start)
fbcon_putcs(vc, start, s - start, line, x);
- console_conditional_schedule();
if (offset > 0)
line++;
else {
diff --git a/include/linux/console.h b/include/linux/console.h
index fc9f5c5c1b04c..ec506d3501965 100644
--- a/include/linux/console.h
+++ b/include/linux/console.h
@@ -697,7 +697,6 @@ extern int unregister_console(struct console *);
extern void console_lock(void);
extern int console_trylock(void);
extern void console_unlock(void);
-extern void console_conditional_schedule(void);
extern void console_unblank(void);
extern void console_flush_on_panic(enum con_flush_mode mode);
extern struct tty_driver *console_device(int *);
diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index 1d765ad242b82..52b1fefdff4e0 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -3362,22 +3362,6 @@ void console_unlock(void)
}
EXPORT_SYMBOL(console_unlock);
-/**
- * console_conditional_schedule - yield the CPU if required
- *
- * If the console code is currently allowed to sleep, and
- * if this CPU should yield the CPU to another task, do
- * so here.
- *
- * Must be called within console_lock();.
- */
-void __sched console_conditional_schedule(void)
-{
- if (console_may_schedule)
- cond_resched();
-}
-EXPORT_SYMBOL(console_conditional_schedule);
-
void console_unblank(void)
{
bool found_unblank = false;
Sebastian
^ permalink raw reply related
* Re: [PATCH v1 1/2] dt-bindings: backlight: gpio-backlight: allow multiple GPIOs
From: Daniel Thompson @ 2026-01-14 15:53 UTC (permalink / raw)
To: tessolveupstream
Cc: lee, danielt, jingoohan1, deller, pavel, robh, krzk+dt, conor+dt,
dri-devel, linux-fbdev, linux-leds, devicetree, linux-kernel
In-Reply-To: <c182df66-8503-49cf-8d1d-7da17214b843@gmail.com>
On Tue, Jan 13, 2026 at 10:15:53AM +0530, tessolveupstream@gmail.com wrote:
>
>
> On 05-01-2026 15:25, Daniel Thompson wrote:
> > On Mon, Jan 05, 2026 at 02:21:19PM +0530, Sudarshan Shetty wrote:
> >> Update the gpio-backlight binding to support configurations that require
> >> more than one GPIO for enabling/disabling the backlight.
> >>
> >> Signed-off-by: Sudarshan Shetty <tessolveupstream@gmail.com>
> >> ---
> >> .../bindings/leds/backlight/gpio-backlight.yaml | 12 +++++++++++-
> >> 1 file changed, 11 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/Documentation/devicetree/bindings/leds/backlight/gpio-backlight.yaml b/Documentation/devicetree/bindings/leds/backlight/gpio-backlight.yaml
> >> index 584030b6b0b9..1483ce4a3480 100644
> >> --- a/Documentation/devicetree/bindings/leds/backlight/gpio-backlight.yaml
> >> +++ b/Documentation/devicetree/bindings/leds/backlight/gpio-backlight.yaml
> >> @@ -17,7 +17,8 @@ properties:
> >>
> >> gpios:
> >> description: The gpio that is used for enabling/disabling the backlight.
> >> - maxItems: 1
> >> + minItems: 1
> >> + maxItems: 2
> >
> > Why 2?
> >
>
> In the current design, the LVDS panel has a single backlight that
> is controlled by two GPIOs. Initially, It described as two separate
> backlight devices using the same gpio-backlight driver, since the
> existing driver supports only one GPIO per instance.
>
> So the maintainer suggested to extend the gpio-backlight driver
> and bindings to support multiple GPIOs.
> https://lore.kernel.org/all/q63bdon55app4gb2il5e7skyc6z2amcnaiqbqlhen7arkxphtb@3jejbelji2ti/
Right. So, once we support multiple GPIOs then why limit it to 2?
Daniel.
^ permalink raw reply
* Re: [PATCH v1 2/2] backlight: gpio: add support for multiple GPIOs for backlight control
From: Daniel Thompson @ 2026-01-14 16:03 UTC (permalink / raw)
To: tessolveupstream
Cc: lee, danielt, jingoohan1, deller, pavel, robh, krzk+dt, conor+dt,
dri-devel, linux-fbdev, linux-leds, devicetree, linux-kernel
In-Reply-To: <1fedb7d7-3a30-4f0f-961f-09613f2a95d0@gmail.com>
On Tue, Jan 13, 2026 at 12:47:26PM +0530, tessolveupstream@gmail.com wrote:
>
>
> On 05-01-2026 15:39, Daniel Thompson wrote:
> > On Mon, Jan 05, 2026 at 02:21:20PM +0530, Sudarshan Shetty wrote:
> >> Extend the gpio-backlight driver to handle multiple GPIOs instead of a
> >> single one. This allows panels that require driving several enable pins
> >> to be controlled by the backlight framework.
> >>
> >> Signed-off-by: Sudarshan Shetty <tessolveupstream@gmail.com>
> >> ---
> >> drivers/video/backlight/gpio_backlight.c | 61 +++++++++++++++++-------
> >> 1 file changed, 45 insertions(+), 16 deletions(-)
> >>
> >> diff --git a/drivers/video/backlight/gpio_backlight.c b/drivers/video/backlight/gpio_backlight.c
> >> index 728a546904b0..037e1c111e48 100644
> >> --- a/drivers/video/backlight/gpio_backlight.c
> >> +++ b/drivers/video/backlight/gpio_backlight.c
> >> @@ -17,14 +17,18 @@
> >>
> >> struct gpio_backlight {
> >> struct device *dev;
> >> - struct gpio_desc *gpiod;
> >> + struct gpio_desc **gpiods;
> >> + unsigned int num_gpios;
> >
> > Why not use struct gpio_descs for this?
> >
> > Once you do that, then most of the gbl->num_gpios loops can be replaced with
> > calls to the array based accessors.
> >
>
> Based on your feedback, I have updated the implementation to use
> struct gpio_descs and array-based accessors, as recommended like
> below:
>
> git diff drivers/video/backlight/gpio_backlight.c
> diff --git a/drivers/video/backlight/gpio_backlight.c b/drivers/video/backlight/gpio_backlight.c
> index 037e1c111e48..e99d7a9dc670 100644
> --- a/drivers/video/backlight/gpio_backlight.c
> +++ b/drivers/video/backlight/gpio_backlight.c
> @@ -14,22 +14,37 @@
> #include <linux/platform_device.h>
> #include <linux/property.h>
> #include <linux/slab.h>
> +#include <linux/bitmap.h>
>
> struct gpio_backlight {
> struct device *dev;
> - struct gpio_desc **gpiods;
> + struct gpio_descs *gpiods;
> unsigned int num_gpios;
> };
>
> static int gpio_backlight_update_status(struct backlight_device *bl)
> {
> struct gpio_backlight *gbl = bl_get_data(bl);
> - unsigned int i;
> + unsigned int n = gbl->num_gpios;
> int br = backlight_get_brightness(bl);
> + unsigned long *value_bitmap;
> + int words = BITS_TO_LONGS(n);
> +
> + value_bitmap = kcalloc(words, sizeof(unsigned long), GFP_KERNEL);
Not sure you need a kcalloc() here. If you want to support more than 32
GPIOs then you can pre-allocate space with a devm_kcalloc() in the probe
method rather than reallocate every time it is used.
To be honest I don't really mind putting a hard limit on the maximum
gpl->num_gpios (so you can just use a local variable) and having no
allocation at all.
> Could you please share your thoughts on whether this approach
> aligns with your expectations?
Looks like it is going in the right direction, yes.
Daniel.
^ permalink raw reply
* Re: [PATCH v6 2/4] backlight: add max25014atg backlight
From: Daniel Thompson @ 2026-01-14 16:29 UTC (permalink / raw)
To: Maud Spierings
Cc: Daniel Thompson, Lee Jones, Jingoo Han, Pavel Machek, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Helge Deller, Shawn Guo,
Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
Liam Girdwood, Mark Brown, dri-devel, linux-leds, devicetree,
linux-kernel, linux-fbdev, imx, linux-arm-kernel
In-Reply-To: <fc5aad54-08fe-453e-a3cf-621414c8a060@gocontroll.com>
On Fri, Jan 09, 2026 at 09:55:18AM +0100, Maud Spierings wrote:
> Do you have any comments about:
>
> > +static void max25014_remove(struct i2c_client *cl)
> > +{
> > + struct max25014 *maxim = i2c_get_clientdata(cl);
> > +
> > + maxim->bl->props.brightness = 0;
> > + max25014_update_status(maxim->bl);
> > + gpiod_set_value_cansleep(maxim->enable, 0);
> > + regulator_disable(maxim->vin);
> > +}
>
> I'm feeling like the setting of the brightness + update status maybe should
> be a call to backlight_device_set_brightness() or maybe it shouldn't really
> be there at all?
Using backlight_device_set_brightness() makes sense (although there is
still a window where userspace could come back in and turn the backlight
on again). And, if both the GPIO and regulator were optional then it is
sensible to set the brightness to zero before removing the driver.
Daniel.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox