* [PATCH] backlight: aw99706: Fix unused function warnings from suspend/resume ops
From: Nathan Chancellor @ 2025-11-20 20:22 UTC (permalink / raw)
To: Lee Jones, Daniel Thompson, Jingoo Han, Helge Deller, Pengyu Luo,
Junjie Cao
Cc: dri-devel, linux-fbdev, patches, Nathan Chancellor
When building for a platform without CONFIG_PM_SLEEP, such as s390,
there are two unused function warnings:
drivers/video/backlight/aw99706.c:436:12: error: 'aw99706_resume' defined but not used [-Werror=unused-function]
436 | static int aw99706_resume(struct device *dev)
| ^~~~~~~~~~~~~~
drivers/video/backlight/aw99706.c:429:12: error: 'aw99706_suspend' defined but not used [-Werror=unused-function]
429 | static int aw99706_suspend(struct device *dev)
| ^~~~~~~~~~~~~~~
SET_SYSTEM_SLEEP_PM_OPS, used within SIMPLE_DEV_PM_OPS, expands to
nothing when CONFIG_PM_SLEEP is not set, so these functions are
completely unused in this configuration.
SIMPLE_DEV_PM_OPS is deprecated in favor of DEFINE_SIMPLE_DEV_PM_OPS,
which avoids this issue by using pm_sleep_ptr to make these callbacks
NULL when CONFIG_PM_SLEEP is unset while making the callback functions
always appear used to the compiler regardless of configuration. Switch
to DEFINE_SIMPLE_DEV_PM_OPS for aw99706_pm_ops to clear up the warning.
Additionally, wrap the pointer to aw99706_pm_ops in pm_ptr() in
aw99706_i2c_driver to ensure that the structure is completely eliminated
in configurations without CONFIG_PM.
Fixes: 88a8e9b49ee8 ("backlight: aw99706: Add support for Awinic AW99706 backlight")
Signed-off-by: Nathan Chancellor <nathan@kernel.org>
---
drivers/video/backlight/aw99706.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/video/backlight/aw99706.c b/drivers/video/backlight/aw99706.c
index b7c1d24b17ac..df5b23b2f753 100644
--- a/drivers/video/backlight/aw99706.c
+++ b/drivers/video/backlight/aw99706.c
@@ -440,7 +440,7 @@ static int aw99706_resume(struct device *dev)
return aw99706_hw_init(aw);
}
-static SIMPLE_DEV_PM_OPS(aw99706_pm_ops, aw99706_suspend, aw99706_resume);
+static DEFINE_SIMPLE_DEV_PM_OPS(aw99706_pm_ops, aw99706_suspend, aw99706_resume);
static const struct i2c_device_id aw99706_ids[] = {
{ "aw99706" },
@@ -461,7 +461,7 @@ static struct i2c_driver aw99706_i2c_driver = {
.driver = {
.name = "aw99706",
.of_match_table = aw99706_match_table,
- .pm = &aw99706_pm_ops,
+ .pm = pm_ptr(&aw99706_pm_ops),
},
};
---
base-commit: 1704e206cb98c5e43af1483e3b07450055a31008
change-id: 20251120-backlight-aw99706-fix-unused-pm-functions-fe2775c4dec6
Best regards,
--
Nathan Chancellor <nathan@kernel.org>
^ permalink raw reply related
* [RFC/RFT PATCH] fbdev: q40fb: request memory region
From: Sukrut Heroorkar @ 2025-11-20 18:02 UTC (permalink / raw)
To: Helge Deller, Sukrut Heroorkar, open list:FRAMEBUFFER LAYER,
open list:FRAMEBUFFER LAYER, open list
Cc: shuah, david.hunter.linux
The q40fb driver uses a fixed physical address but never reserves
the corresponding I/O region. Reserve the range as suggested in
Documentation/gpu/todo.rst ("Request memory regions in all fbdev drivers").
If the memory cannot be reserved, fail probe with -EBUSY to avoid
conflicting with another user of the same address.
Signed-off-by: Sukrut Heroorkar <hsukrut3@gmail.com>
---
Testing: This patch is sent as RFT since Q40 hardware is unavilable and
QEMU does not emulated a Q40 platform. The change is therefore compile-tested
only.
drivers/video/fbdev/q40fb.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/drivers/video/fbdev/q40fb.c b/drivers/video/fbdev/q40fb.c
index 1ff8fa176124..7b5c31745041 100644
--- a/drivers/video/fbdev/q40fb.c
+++ b/drivers/video/fbdev/q40fb.c
@@ -101,6 +101,13 @@ static int q40fb_probe(struct platform_device *dev)
info->par = NULL;
info->screen_base = (char *) q40fb_fix.smem_start;
+ if (!request_mem_region(q40fb_fix.smem_start, q40fb_fix.smem_len,
+ "q40fb")) {
+ dev_err(&dev->dev, "cannot reserve video memory at 0x%lx\n",
+ q40fb_fix.smem_start);
+ return -EBUSY;
+ }
+
if (fb_alloc_cmap(&info->cmap, 256, 0) < 0) {
framebuffer_release(info);
return -ENOMEM;
--
2.43.0
^ permalink raw reply related
* Re: [PATCH] fbdev: q40fb: request memory region
From: sukrut heroorkar @ 2025-11-20 18:00 UTC (permalink / raw)
To: David Hunter
Cc: Helge Deller, open list:FRAMEBUFFER LAYER,
open list:FRAMEBUFFER LAYER, open list, shuah
In-Reply-To: <4ec784a5-0f67-4fd3-9d51-d89a9fa9a385@gmail.com>
On Wed, Nov 19, 2025 at 7:27 PM David Hunter
<david.hunter.linux@gmail.com> wrote:
>
> On 11/18/25 04:56, Sukrut Heroorkar wrote:
> > The q40fb driver uses a fixed physical address but never reserves
> > the corresponding I/O region. Reserve the range as suggested in
> > Documentation/gpu/todo.rst ("Request memory regions in all fbdev drivers").
> >
> > No functional change beyond claming the resource. This change is compile
> > tested only.
>
> Reserving memory is a significant "functional" change, so you should not
> put "No functional change...". I have noticed that in the mentorship
> program, mentees might say this often times when they have not done
> testing.
>
> Thank you for describing that you did a compile test, but I believe that
> more testing should be done before this patch is accepted.
qemu-system-m68k does not emulate a Q40 machine, thus the change
was compile tested only with W=1 & debugging enabled.
>
> As a result, if you are unable to test this device, I believe that an
> RFT tag should be used. Also, the testing information goes below the
> "---". This puts it in the change log and would make it so that if a
> patch is accepted, everything below the change log is not put in the
> commit message.
Thank you. I will make a note of this for the future patches.
> >
> > Signed-off-by: Sukrut Heroorkar <hsukrut3@gmail.com>
> > ---
> > drivers/video/fbdev/q40fb.c | 7 +++++++
> > 1 file changed, 7 insertions(+)
> >
> > diff --git a/drivers/video/fbdev/q40fb.c b/drivers/video/fbdev/q40fb.c
> > index 1ff8fa176124..935260326c6f 100644
> > --- a/drivers/video/fbdev/q40fb.c
> > +++ b/drivers/video/fbdev/q40fb.c
> > @@ -101,6 +101,12 @@ static int q40fb_probe(struct platform_device *dev)
> > info->par = NULL;
> > info->screen_base = (char *) q40fb_fix.smem_start;
> >
> > + if (!request_mem_region(q40fb_fix.smem_start, q40fb_fix.smem_len,
> > + "q40fb")) {
> > + dev_err(&dev->dev, "cannot reserve video memory at 0x%lx\n",
> > + q40fb_fix.smem_start);
> > + }
> > +
>
> Is this correct? It seems to me that in the case of an error, all you
> are doing is simply logging the error and proceeding. Would this cause
> the device to continue to try to use space that it was not able to
> reserve? I do not have experience with this device or the driver, but
> that does not seem correct to me.
I referred to a patch, which was recently accepted, having a similar
implementation.
However, other fbdev drivers with similar implementation, returns a
-EBUSY when the
If() evaluates true indicating resource already being occupied. I
will make the necessary
changes and resend the patch as RFT.
>
> > if (fb_alloc_cmap(&info->cmap, 256, 0) < 0) {
> > framebuffer_release(info);
> > return -ENOMEM;
> > @@ -144,6 +150,7 @@ static int __init q40fb_init(void)
> > if (ret)
> > platform_driver_unregister(&q40fb_driver);
> > }
> > +
> > return ret;
> > }
> >
>
^ permalink raw reply
* Re: [PATCH v2 1/2] backlight: Add Congatec Board Controller (CGBC) backlight support
From: Krzysztof Kozlowski @ 2025-11-20 14:09 UTC (permalink / raw)
To: petri.karhula, Thomas Richard, Lee Jones, Daniel Thompson,
Jingoo Han, Helge Deller
Cc: linux-kernel, dri-devel, linux-fbdev
In-Reply-To: <20251119-cgbc-backlight-v2-1-4d4edd7ca662@novatron.fi>
On 19/11/2025 09:25, Petri Karhula via B4 Relay wrote:
> +}
> +
> +/**
> + * Remove function for CGBC backlight driver
> + * @pdev: Platform device
> + *
> + * The Linux device-managed resource framework (devres) does the cleanup.
> + * No explicit cleanup is needed here.
> + */
> +static void cgbc_bl_remove(struct platform_device *pdev)
> +{
> + dev_info(&pdev->dev, "CGBC backlight driver removed\n");
> +}
I reviewed v1, but all comments are applicable.
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH] backlight: Add Congatec Board Controller (CGBC) backlight support
From: Krzysztof Kozlowski @ 2025-11-20 14:09 UTC (permalink / raw)
To: petri.karhula, Thomas Richard, Lee Jones, Daniel Thompson,
Jingoo Han, Helge Deller
Cc: linux-kernel, dri-devel, linux-fbdev
In-Reply-To: <20251118-cgbc-backlight-v1-1-cc6ac5301034@novatron.fi>
On 18/11/2025 17:43, Petri Karhula via B4 Relay wrote:
> +
> +/**
> + * Get current backlight brightness
> + * @bl: Backlight device
> + *
> + * Returns the current brightness level by reading from hardware.
> + *
> + * Return: Current brightness level (0-100), or negative error code
> + */
Why are you documenting standard API?
> +static int cgbc_bl_get_brightness(struct backlight_device *bl)
> +{
> + struct cgbc_bl_data *bl_data = bl_get_data(bl);
> + int ret;
> +
> + /* Read current PWM brightness settings */
> + ret = cgbc_bl_read_pwm_settings(bl_data);
> +
> + if (ret < 0) {
> + dev_err(bl_data->dev, "Failed to read PWM settings: %d\n", ret);
> + return ret;
> + }
> +
> + return bl_data->current_brightness;
> +}
> +
> +/* Backlight device operations */
Huh? Can it be a GPIO device operations?
> +static const struct backlight_ops cgbc_bl_ops = {
> + .options = BL_CORE_SUSPENDRESUME,
> + .update_status = cgbc_bl_update_status,
> + .get_brightness = cgbc_bl_get_brightness,
> +};
> +
> +/**
> + * Probe function for CGBC backlight driver
> + * @pdev: Platform device
> + *
> + * Initializes the CGBC backlight driver and registers it with the
> + * Linux backlight subsystem.
> + *
> + * Return: 0 on success, negative error code on failure
Very redundant and useless comment.
> + */
> +static int cgbc_bl_probe(struct platform_device *pdev)
> +{
> + struct cgbc_device_data *cgbc = dev_get_drvdata(pdev->dev.parent);
> + struct cgbc_bl_data *bl_data;
> + struct backlight_properties props;
> + struct backlight_device *bl_dev;
> + int ret;
> +
> + bl_data = devm_kzalloc(&pdev->dev, sizeof(*bl_data), GFP_KERNEL);
> +
Drop blank line. There is never such line between allocation and check.
> + if (!bl_data)
> + return -ENOMEM;
> +
> + bl_data->dev = &pdev->dev;
> + bl_data->cgbc = cgbc;
> +
> + ret = cgbc_bl_read_pwm_settings(bl_data);
> +
> + if (ret) {
> + dev_err(&pdev->dev, "Failed to read initial PWM settings: %d\n",
> + ret);
return dev_err_probe
> + return ret;
> + }
> +
> + memset(&props, 0, sizeof(props));
> + props.type = BACKLIGHT_PLATFORM;
> + props.max_brightness = CGBC_BL_MAX_BRIGHTNESS;
> + props.brightness = bl_data->current_brightness;
> +
> + bl_dev = devm_backlight_device_register(&pdev->dev, "cgbc-backlight",
> + &pdev->dev, bl_data,
> + &cgbc_bl_ops, &props);
> +
> + if (IS_ERR(bl_dev)) {
> + dev_err(&pdev->dev, "Failed to register backlight device\n");
return dev_err_probe
> + return PTR_ERR(bl_dev);
> + }
> +
> + bl_data->bl_dev = bl_dev;
> + platform_set_drvdata(pdev, bl_data);
> +
> + dev_info(&pdev->dev,
> + "CGBC backlight driver registered (brightness=%u)\n",
> + bl_data->current_brightness);
Drop.
This does not look like useful printk message. Drivers should be silent
on success:
https://elixir.bootlin.com/linux/v6.15-rc7/source/Documentation/process/coding-style.rst#L913
https://elixir.bootlin.com/linux/v6.15-rc7/source/Documentation/process/debugging/driver_development_debugging_guide.rst#L79
> +
> + return 0;
> +}
> +
> +/**
> + * Remove function for CGBC backlight driver
> + * @pdev: Platform device
> + *
> + * The Linux device-managed resource framework (devres) does the cleanup.
> + * No explicit cleanup is needed here.
Drop such comments, they are not useful. Please write only useful
comments, not ones stating obvious.
> + */
> +static void cgbc_bl_remove(struct platform_device *pdev)
> +{
> + dev_info(&pdev->dev, "CGBC backlight driver removed\n");
Drop, there is no such code in Linux kernel. Drop it.
> +}
> +
Best regards,
Krzysztof
^ permalink raw reply
* [PATCH] fbdev/tcx.c fix mem_map to correct smem_start offset
From: René Rebe @ 2025-11-20 13:24 UTC (permalink / raw)
To: linux-fbdev; +Cc: Helge Deller
403ae52ac047 ("sparc: fix drivers/video/tcx.c warning") changed the
physbase initializing breaking the user-space mmap, e.g. for Xorg
entirely.
Fix fbdev mmap table so the sbus mmap helper work correctly, and
not try to map vastly (physbase) offset memory.
Fixes: 403ae52ac047 ("sparc: fix drivers/video/tcx.c warning")
Signed-off-by: René Rebe <rene@exactco.de>
---
drivers/video/fbdev/tcx.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/video/fbdev/tcx.c b/drivers/video/fbdev/tcx.c
index f9a0085ad72b..ca9e84e8d860 100644
--- a/drivers/video/fbdev/tcx.c
+++ b/drivers/video/fbdev/tcx.c
@@ -428,7 +428,7 @@ static int tcx_probe(struct platform_device *op)
j = i;
break;
}
- par->mmap_map[i].poff = op->resource[j].start;
+ par->mmap_map[i].poff = op->resource[j].start - info->fix.smem_start;
}
info->fbops = &tcx_ops;
--
2.46.0
--
René Rebe, ExactCODE GmbH, Berlin, Germany
https://exactco.de • https://t2linux.com • https://patreon.com/renerebe
^ permalink raw reply related
* Re: [PATCH] fbdev: core: Fix vmalloc-out-of-bounds in fb_imageblit
From: kernel test robot @ 2025-11-20 9:23 UTC (permalink / raw)
To: ssrane_b23, Zsolt Kajtar, Simona Vetter, Helge Deller
Cc: oe-kbuild-all, Shaurya Rane, linux-fbdev, dri-devel, linux-kernel,
syzbot+5a40432dfe8f86ee657a
In-Reply-To: <20251119133821.89998-1-ssranevjti@gmail.com>
Hi,
kernel test robot noticed the following build warnings:
[auto build test WARNING on drm-misc/drm-misc-next]
[also build test WARNING on drm-tip/drm-tip linus/master v6.18-rc6 next-20251119]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/ssrane_b23-ee-vjti-ac-in/fbdev-core-Fix-vmalloc-out-of-bounds-in-fb_imageblit/20251119-215054
base: git://anongit.freedesktop.org/drm/drm-misc drm-misc-next
patch link: https://lore.kernel.org/r/20251119133821.89998-1-ssranevjti%40gmail.com
patch subject: [PATCH] fbdev: core: Fix vmalloc-out-of-bounds in fb_imageblit
config: nios2-randconfig-r073-20251120 (https://download.01.org/0day-ci/archive/20251120/202511201752.4fVbQwPc-lkp@intel.com/config)
compiler: nios2-linux-gcc (GCC) 10.5.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251120/202511201752.4fVbQwPc-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/202511201752.4fVbQwPc-lkp@intel.com/
All warnings (new ones prefixed by >>):
In file included from drivers/video/fbdev/core/cfbimgblt.c:17:
drivers/video/fbdev/core/fb_imageblit.h: In function 'fb_imageblit':
>> drivers/video/fbdev/core/fb_imageblit.h:490:16: warning: unused variable 'max_offset_bytes' [-Wunused-variable]
490 | unsigned long max_offset_bytes;
| ^~~~~~~~~~~~~~~~
vim +/max_offset_bytes +490 drivers/video/fbdev/core/fb_imageblit.h
480
481 static inline void fb_imageblit(struct fb_info *p, const struct fb_image *image)
482 {
483 int bpp = p->var.bits_per_pixel;
484 unsigned int bits_per_line = BYTES_TO_BITS(p->fix.line_length);
485 struct fb_address dst = fb_address_init(p);
486 struct fb_reverse reverse = fb_reverse_init(p);
487 const u32 *palette = fb_palette(p);
488 struct fb_image clipped_image;
489 u32 max_x, max_y;
> 490 unsigned long max_offset_bytes;
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* Re: [PATCH v2 1/2] backlight: Add Congatec Board Controller (CGBC) backlight support
From: Thomas Richard @ 2025-11-20 9:09 UTC (permalink / raw)
To: petri.karhula, Lee Jones, Daniel Thompson, Jingoo Han,
Helge Deller
Cc: linux-kernel, dri-devel, linux-fbdev
In-Reply-To: <20251119-cgbc-backlight-v2-1-4d4edd7ca662@novatron.fi>
Hello Petri,
Thanks for your patch.
On 11/19/25 9:25 AM, Petri Karhula via B4 Relay wrote:
> From: Petri Karhula <petri.karhula@novatron.fi>
>
> This driver provides backlight brightness control through the Linux
> backlight subsystem. It communicates with the board controller to
> adjust LCD backlight using PWM signals. Communication is done
> through Congatec Board Controller core driver.
>
> Signed-off-by: Petri Karhula <petri.karhula@novatron.fi>
> ---
> drivers/video/backlight/Kconfig | 11 ++
> drivers/video/backlight/Makefile | 1 +
> drivers/video/backlight/cgbc_bl.c | 281 ++++++++++++++++++++++++++++++++++++++
> 3 files changed, 293 insertions(+)
>
> diff --git a/drivers/video/backlight/Kconfig b/drivers/video/backlight/Kconfig
> index d9374d208cee..702f3b8ed036 100644
> --- a/drivers/video/backlight/Kconfig
> +++ b/drivers/video/backlight/Kconfig
> @@ -249,6 +249,17 @@ config BACKLIGHT_PWM
> If you have a LCD backlight adjustable by PWM, say Y to enable
> this driver.
>
> +config BACKLIGHT_CGBC
> + tristate "Congatec Board Controller (CGBC) backlight support"
> + depends on MFD_CGBC && X86
> + help
> + Say Y here to enable support for LCD backlight control on Congatec
> + x86-based boards via the CGBC (Congatec Board Controller).
> +
> + This driver provides backlight brightness control through the Linux
> + backlight subsystem. It communicates with the board controller to
> + adjust LCD backlight using PWM signals.
> +
> config BACKLIGHT_DA903X
> tristate "Backlight Driver for DA9030/DA9034 using WLED"
> depends on PMIC_DA903X
> diff --git a/drivers/video/backlight/Makefile b/drivers/video/backlight/Makefile
> index dfbb169bf6ea..0169fd8873ed 100644
> --- a/drivers/video/backlight/Makefile
> +++ b/drivers/video/backlight/Makefile
> @@ -27,6 +27,7 @@ obj-$(CONFIG_BACKLIGHT_APPLE_DWI) += apple_dwi_bl.o
> obj-$(CONFIG_BACKLIGHT_AS3711) += as3711_bl.o
> obj-$(CONFIG_BACKLIGHT_BD6107) += bd6107.o
> obj-$(CONFIG_BACKLIGHT_CLASS_DEVICE) += backlight.o
> +obj-$(CONFIG_BACKLIGHT_CGBC) += cgbc_bl.o
> obj-$(CONFIG_BACKLIGHT_DA903X) += da903x_bl.o
> obj-$(CONFIG_BACKLIGHT_DA9052) += da9052_bl.o
> obj-$(CONFIG_BACKLIGHT_EP93XX) += ep93xx_bl.o
> diff --git a/drivers/video/backlight/cgbc_bl.c b/drivers/video/backlight/cgbc_bl.c
> new file mode 100644
> index 000000000000..4382321f4cac
> --- /dev/null
> +++ b/drivers/video/backlight/cgbc_bl.c
> @@ -0,0 +1,281 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Congatec Board Controller (CGBC) Backlight Driver
> + *
> + * This driver provides backlight control for LCD displays connected to
> + * Congatec boards via the CGBC (Congatec Board Controller). It integrates
> + * with the Linux backlight subsystem and communicates with hardware through
> + * the cgbc-core module.
> + *
> + * Copyright (C) 2025 Novatron Oy
> + *
> + * Author: Petri Karhula <petri.karhula@novatron.fi>
> + */
> +
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/backlight.h>
> +
> +#include <linux/mfd/cgbc.h>
headers shall be sorted in alphabetical order
> +
> +#define CGBC_BL_DRIVER_VERSION "0.0.1"
not needed
> +
> +#define BLT_PWM_DUTY_MASK 0x7F
> +#define BLT_PWM_INVERTED_MASK 0x80
Use GENMASK
> +
> +/* CGBC command for PWM brightness control*/
> +#define CGBC_CMD_BLT0_PWM 0x75
> +
> +#define CGBC_BL_MAX_BRIGHTNESS 100
> +
> +/**
> + * CGBC backlight driver data
> + * @dev: Pointer to the platform device
> + * @bl_dev: Pointer to the backlight device
> + * @cgbc: Pointer to the parent CGBC device data
> + * @current_brightness: Current brightness level (0-100)
> + */
> +struct cgbc_bl_data {
> + struct device *dev;
> + struct backlight_device *bl_dev;
not used
> + struct cgbc_device_data *cgbc;
> + unsigned int current_brightness;
> +};
> +
> +/**
> + * Read current PWM settings from hardware
> + * @bl_data: Backlight driver data
> + *
> + * Reads the current PWM duty cycle percentage (= brightness level)
> + * from the board controller.
> + *
> + * Return: 0 on success, negative error code on failure
> + */
> +static int cgbc_bl_read_pwm_settings(struct cgbc_bl_data *bl_data)
> +{
> + u8 cmd_buf[4] = { CGBC_CMD_BLT0_PWM, 0, 0, 0 };
> + u8 reply_buf[3];
> + int ret;
> +
> + ret = cgbc_command(bl_data->cgbc, cmd_buf, sizeof(cmd_buf), reply_buf,
> + sizeof(reply_buf), NULL);
> +
> + if (ret < 0) {
> + dev_err(bl_data->dev, "Failed to read PWM settings: %d\n", ret);
> + return ret;
> + }
error message not needed from my point of view.
> +
> + /*
> + * Only return PWM duty factor percentage,
> + * ignore polarity inversion bit (bit 7)
> + */
> + bl_data->current_brightness = reply_buf[0] & BLT_PWM_DUTY_MASK;
I would prefer to use FIELD_GET
> +
> + dev_dbg(bl_data->dev, "Current PWM duty=%u\n", bl_data->current_brightness);
Not needed from my point of view.
> +
> + return 0;
> +}
> +
> +/**
> + * Set backlight brightness
> + * @bl_data: Backlight driver data
> + * @brightness: Brightness level (0-100)
> + *
> + * Sets the backlight brightness by configuring the PWM duty cycle.
> + * Preserves the current polarity and frequency settings.
> + *
> + * Return: 0 on success, negative error code on failure
> + */
> +static int cgbc_bl_set_brightness(struct cgbc_bl_data *bl_data, u8 brightness)
> +{
> + u8 cmd_buf[4] = { CGBC_CMD_BLT0_PWM, 0, 0, 0 };
u8 cmd_buf[4] = { CGBC_CMD_BLT0_PWM };
> + u8 reply_buf[3];
> + int ret;
> +
> + /* Read the current values */
> + ret = cgbc_command(bl_data->cgbc, cmd_buf, sizeof(cmd_buf), reply_buf,
> + sizeof(reply_buf), NULL);
> +
> + if (ret < 0) {
> + dev_err(bl_data->dev, "Failed to read PWM settings: %d\n", ret);
> + return ret;
> + }
error message not needed from my point of view.
> +
> + /*
> + * Prepare command buffer for writing new settings. Only 2nd byte is changed
> + * to set new brightness (PWM duty cycle %). Other balues (polarity, frequency)
values
> + * are preserved from the read values.
> + */
> + cmd_buf[1] = (reply_buf[0] & BLT_PWM_INVERTED_MASK) |
> + (BLT_PWM_DUTY_MASK & brightness);
use FIELD_PREP
> + cmd_buf[2] = reply_buf[1];
> + cmd_buf[3] = reply_buf[2];
> +
> + ret = cgbc_command(bl_data->cgbc, cmd_buf, sizeof(cmd_buf), reply_buf,
> + sizeof(reply_buf), NULL);
> +
> + if (ret < 0) {
> + dev_err(bl_data->dev, "Failed to set brightness: %d\n", ret);
error messages not needed from my point of view.
> + return ret;
> + }
> +
> + bl_data->current_brightness = reply_buf[0] & BLT_PWM_DUTY_MASK;
> +
> + /* Verify the setting was applied correctly */
> + if (bl_data->current_brightness != brightness) {
> + dev_err(bl_data->dev,
> + "Brightness setting verification failed\n");
> + return -EIO;
> + }
Do we really need to check the brightness returned by the board
controller? Have you ever run into a situation where cbgc_command
completed without errors, but the brightness level didn’t match what you
expected? Maybe we could assume that if the cbgc_command returned
successfully the brightness value is correct?
I'm not against checking the backlight value. I looked at Congatec's
implementation and they also check it.
> +
> + dev_dbg(bl_data->dev, "Set brightness to %u\n", brightness);
Not needed, the core already has this message
> +
> + return 0;
> +}
> +
> +/**
> + * Backlight update callback
> + * @bl: Backlight device
> + *
> + * Called by the backlight subsystem when brightness needs to be updated.
> + * Changes the brightness level on the hardware
> + * if requested value differs from the current setting.
> + *
> + * Return: 0 on success, negative error code on failure
> + */
> +static int cgbc_bl_update_status(struct backlight_device *bl)
> +{
> + struct cgbc_bl_data *bl_data = bl_get_data(bl);
> + u8 brightness;
> + int ret;
> +
> + brightness = bl->props.brightness;
use backlight_get_brightness()
> +
> + if (brightness != bl_data->current_brightness) {
> + ret = cgbc_bl_set_brightness(bl_data, brightness);
> +
> + if (ret < 0) {
> + dev_err(bl_data->dev, "Failed to set brightness: %d\n",
> + ret);
> + return ret;
> + }
error message not needed from my point of view.
> + }
> +
> + return 0;
> +}
> +
> +/**
> + * Get current backlight brightness
> + * @bl: Backlight device
> + *
> + * Returns the current brightness level by reading from hardware.
> + *
> + * Return: Current brightness level (0-100), or negative error code
> + */
> +static int cgbc_bl_get_brightness(struct backlight_device *bl)
> +{
> + struct cgbc_bl_data *bl_data = bl_get_data(bl);
> + int ret;
> +
> + /* Read current PWM brightness settings */
> + ret = cgbc_bl_read_pwm_settings(bl_data);
> +
> + if (ret < 0) {
> + dev_err(bl_data->dev, "Failed to read PWM settings: %d\n", ret);
> + return ret;
> + }
error message not needed from my point of view.
If you remove all these error messages, you can also remove the struct
device in the struct cgbc_bl_data.
> +
> + return bl_data->current_brightness;
> +}
Maybe you can remove cgbc_bl_read_pwm_settings() and move all the code
in cgbc_bl_get_brightness(). It makes the code easier to read.
> +
> +/* Backlight device operations */
> +static const struct backlight_ops cgbc_bl_ops = {
> + .options = BL_CORE_SUSPENDRESUME,
> + .update_status = cgbc_bl_update_status,
> + .get_brightness = cgbc_bl_get_brightness,
> +};
> +
> +/**
> + * Probe function for CGBC backlight driver
> + * @pdev: Platform device
> + *
> + * Initializes the CGBC backlight driver and registers it with the
> + * Linux backlight subsystem.
> + *
> + * Return: 0 on success, negative error code on failure
> + */
> +static int cgbc_bl_probe(struct platform_device *pdev)
> +{
> + struct cgbc_device_data *cgbc = dev_get_drvdata(pdev->dev.parent);
> + struct cgbc_bl_data *bl_data;
> + struct backlight_properties props;
> + struct backlight_device *bl_dev;
> + int ret;
nitpick: reverse xmas tree
> +
> + bl_data = devm_kzalloc(&pdev->dev, sizeof(*bl_data), GFP_KERNEL);
> +
nitpick: drop empty line.
> + if (!bl_data)
> + return -ENOMEM;
> +
> + bl_data->dev = &pdev->dev;
> + bl_data->cgbc = cgbc;
> +
> + ret = cgbc_bl_read_pwm_settings(bl_data);
> +
nitpick: drop empty line.
> + if (ret) {
> + dev_err(&pdev->dev, "Failed to read initial PWM settings: %d\n",
> + ret);
> + return ret;
> + }
Use dev_err_probe().
> +
> + memset(&props, 0, sizeof(props));
Use struct backlight_properties props = { };
> + props.type = BACKLIGHT_PLATFORM;
> + props.max_brightness = CGBC_BL_MAX_BRIGHTNESS;
> + props.brightness = bl_data->current_brightness;
> +
> + bl_dev = devm_backlight_device_register(&pdev->dev, "cgbc-backlight",
> + &pdev->dev, bl_data,
> + &cgbc_bl_ops, &props);
> +
> + if (IS_ERR(bl_dev)) {
> + dev_err(&pdev->dev, "Failed to register backlight device\n");
> + return PTR_ERR(bl_dev);
> + }
Use dev_err_probe()
> +
> + bl_data->bl_dev = bl_dev;
> + platform_set_drvdata(pdev, bl_data);
> +
> + dev_info(&pdev->dev,
> + "CGBC backlight driver registered (brightness=%u)\n",
> + bl_data->current_brightness);
No logs if device probes successfully.
> +
> + return 0;
> +}
> +
> +/**
> + * Remove function for CGBC backlight driver
> + * @pdev: Platform device
> + *
> + * The Linux device-managed resource framework (devres) does the cleanup.
> + * No explicit cleanup is needed here.
> + */
> +static void cgbc_bl_remove(struct platform_device *pdev)
> +{
> + dev_info(&pdev->dev, "CGBC backlight driver removed\n");
> +}
Remove operation does nothing so drop it.
> +
> +static struct platform_driver cgbc_bl_driver = {
> + .driver = {
> + .name = "cgbc-backlight",
> + },
> + .probe = cgbc_bl_probe,
> + .remove = cgbc_bl_remove,
> +};
> +
> +module_platform_driver(cgbc_bl_driver);
> +
> +MODULE_AUTHOR("Petri Karhula <petri.karhula@novatron.fi>");
> +MODULE_DESCRIPTION("Congatec Board Controller (CGBC) Backlight Driver");
> +MODULE_LICENSE("GPL");
> +MODULE_VERSION(CGBC_BL_DRIVER_VERSION);
Not needed
> +MODULE_ALIAS("platform:cgbc-backlight");
>
Best Regards,
Thomas
^ permalink raw reply
* Re: [PATCH] fbdev: core: Fix vmalloc-out-of-bounds in fb_imageblit
From: Kajtár Zsolt @ 2025-11-19 21:05 UTC (permalink / raw)
To: ssrane_b23, Simona Vetter, Helge Deller
Cc: linux-fbdev, dri-devel, linux-kernel, syzbot+5a40432dfe8f86ee657a
In-Reply-To: <20251119133821.89998-1-ssranevjti@gmail.com>
[-- Attachment #1.1: Type: text/plain, Size: 1037 bytes --]
Hello!
> This patch replaces the insufficient check with a more precise one. It
> calculates the effective width in bytes of the image (accounting for
> clipping against xres_virtual) and ensures that the last byte of the
> operation falls within the screen buffer. Specifically, it checks if
> '(dy + height - 1) * line_length + effective_width_bytes' exceeds
> screen_size. If it does, the drawing height max_y is reduced to
> prevent the out-of-bounds access.
I know my opinion doesn't count much but would like make a note.
Any bound checks which are applied here or at the entry of the other 2
low level drawing routines are just masking an issue somewhere in the
console code. The text area should be entirely within bounds of the
screen memory. If that's always the case then there shouldn't be any
drawing request outside of the framebuffer either.
Please consider at least to add a warning instead of silent clipping, as
every time such clipping was done it was a result of a bug.
--
-soci-
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]
^ permalink raw reply
* Re: [PATCH] fbdev: q40fb: request memory region
From: David Hunter @ 2025-11-19 13:57 UTC (permalink / raw)
To: Sukrut Heroorkar, Helge Deller, open list:FRAMEBUFFER LAYER,
open list:FRAMEBUFFER LAYER, open list
Cc: shuah, david.hunter.linux
In-Reply-To: <20251118095700.393474-1-hsukrut3@gmail.com>
On 11/18/25 04:56, Sukrut Heroorkar wrote:
> The q40fb driver uses a fixed physical address but never reserves
> the corresponding I/O region. Reserve the range as suggested in
> Documentation/gpu/todo.rst ("Request memory regions in all fbdev drivers").
>
> No functional change beyond claming the resource. This change is compile
> tested only.
Reserving memory is a significant "functional" change, so you should not
put "No functional change...". I have noticed that in the mentorship
program, mentees might say this often times when they have not done
testing.
Thank you for describing that you did a compile test, but I believe that
more testing should be done before this patch is accepted.
As a result, if you are unable to test this device, I believe that an
RFT tag should be used. Also, the testing information goes below the
"---". This puts it in the change log and would make it so that if a
patch is accepted, everything below the change log is not put in the
commit message.
>
> Signed-off-by: Sukrut Heroorkar <hsukrut3@gmail.com>
> ---
> drivers/video/fbdev/q40fb.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/drivers/video/fbdev/q40fb.c b/drivers/video/fbdev/q40fb.c
> index 1ff8fa176124..935260326c6f 100644
> --- a/drivers/video/fbdev/q40fb.c
> +++ b/drivers/video/fbdev/q40fb.c
> @@ -101,6 +101,12 @@ static int q40fb_probe(struct platform_device *dev)
> info->par = NULL;
> info->screen_base = (char *) q40fb_fix.smem_start;
>
> + if (!request_mem_region(q40fb_fix.smem_start, q40fb_fix.smem_len,
> + "q40fb")) {
> + dev_err(&dev->dev, "cannot reserve video memory at 0x%lx\n",
> + q40fb_fix.smem_start);
> + }
> +
Is this correct? It seems to me that in the case of an error, all you
are doing is simply logging the error and proceeding. Would this cause
the device to continue to try to use space that it was not able to
reserve? I do not have experience with this device or the driver, but
that does not seem correct to me.
> if (fb_alloc_cmap(&info->cmap, 256, 0) < 0) {
> framebuffer_release(info);
> return -ENOMEM;
> @@ -144,6 +150,7 @@ static int __init q40fb_init(void)
> if (ret)
> platform_driver_unregister(&q40fb_driver);
> }
> +
> return ret;
> }
>
^ permalink raw reply
* [PATCH] fbdev: core: Fix vmalloc-out-of-bounds in fb_imageblit
From: ssrane_b23 @ 2025-11-19 13:38 UTC (permalink / raw)
To: Zsolt Kajtar, Simona Vetter, Helge Deller
Cc: Shaurya Rane, linux-fbdev, dri-devel, linux-kernel,
syzbot+5a40432dfe8f86ee657a
From: Shaurya Rane <ssrane_b23@ee.vjti.ac.in>
syzbot reported a vmalloc-out-of-bounds write in fb_imageblit. The crash
occurs when drawing an image at the very end of the framebuffer memory.
The current bounds check in fb_imageblit limits the drawing height (max_y)
by dividing the screen size by the line length. However, this calculation
only ensures that the start of the last line fits within the buffer. It
fails to account for the width of the image on that final line. If the
image width (multiplied by bpp) exceeds the remaining space on the last
line, the drawing routine writes past the end of the allocated video
memory.
This patch replaces the insufficient check with a more precise one. It
calculates the effective width in bytes of the image (accounting for
clipping against xres_virtual) and ensures that the last byte of the
operation falls within the screen buffer. Specifically, it checks if
'(dy + height - 1) * line_length + effective_width_bytes' exceeds
screen_size. If it does, the drawing height max_y is reduced to
prevent the out-of-bounds access.
Reported-by: syzbot+5a40432dfe8f86ee657a@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=5a40432dfe8f86ee657a
Signed-off-by: Shaurya Rane <ssrane_b23@ee.vjti.ac.in>
---
drivers/video/fbdev/core/fb_imageblit.h | 66 +++++++++++++++++++++++--
1 file changed, 62 insertions(+), 4 deletions(-)
diff --git a/drivers/video/fbdev/core/fb_imageblit.h b/drivers/video/fbdev/core/fb_imageblit.h
index 3b2bb4946505..0c0d05cff3f8 100644
--- a/drivers/video/fbdev/core/fb_imageblit.h
+++ b/drivers/video/fbdev/core/fb_imageblit.h
@@ -485,11 +485,69 @@ static inline void fb_imageblit(struct fb_info *p, const struct fb_image *image)
struct fb_address dst = fb_address_init(p);
struct fb_reverse reverse = fb_reverse_init(p);
const u32 *palette = fb_palette(p);
+ struct fb_image clipped_image;
+ u32 max_x, max_y;
+ unsigned long max_offset_bytes;
+
+ /* Validate basic parameters */
+ if (!image || !p->screen_buffer || !p->screen_size ||
+ !image->width || !image->height)
+ return;
+
+ /* Calculate maximum addressable coordinates based on virtual resolution and buffer size */
+ max_x = p->var.xres_virtual;
+ max_y = p->var.yres_virtual;
+
+ /* Check against actual buffer size to prevent vmalloc overflow */
+ {
+ unsigned long effective_width_bytes;
+ u32 right_edge = image->dx + image->width;
+
+ if (right_edge < image->dx)
+ right_edge = max_x;
+ else
+ right_edge = min(right_edge, max_x);
+
+ effective_width_bytes = (unsigned long)right_edge * bpp;
+ effective_width_bytes = (effective_width_bytes + 7) / 8;
+
+ if (effective_width_bytes > p->screen_size) {
+ max_y = 0;
+ } else if (p->fix.line_length) {
+ u32 max_lines = (p->screen_size - effective_width_bytes) /
+ p->fix.line_length + 1;
+ if (max_lines < max_y)
+ max_y = max_lines;
+ }
+ }
+
+ /* If image is completely outside bounds, skip it */
+ if (image->dx >= max_x || image->dy >= max_y)
+ return;
+
+ /* Create clipped image - clip to virtual resolution bounds */
+ clipped_image = *image;
+
+ /* Clip width if it extends beyond right edge */
+ if (clipped_image.dx + clipped_image.width > max_x) {
+ if (clipped_image.dx < max_x)
+ clipped_image.width = max_x - clipped_image.dx;
+ else
+ return; /* completely outside */
+ }
+
+ /* Clip height if it extends beyond bottom edge */
+ if (clipped_image.dy + clipped_image.height > max_y) {
+ if (clipped_image.dy < max_y)
+ clipped_image.height = max_y - clipped_image.dy;
+ else
+ return; /* completely outside */
+ }
- fb_address_forward(&dst, image->dy * bits_per_line + image->dx * bpp);
+ fb_address_forward(&dst, clipped_image.dy * bits_per_line + clipped_image.dx * bpp);
- if (image->depth == 1)
- fb_bitmap_imageblit(image, &dst, bits_per_line, palette, bpp, reverse);
+ if (clipped_image.depth == 1)
+ fb_bitmap_imageblit(&clipped_image, &dst, bits_per_line, palette, bpp, reverse);
else
- fb_color_imageblit(image, &dst, bits_per_line, palette, bpp, reverse);
+ fb_color_imageblit(&clipped_image, &dst, bits_per_line, palette, bpp, reverse);
}
--
2.34.1
^ permalink raw reply related
* Re: [syzbot] [fbdev?] KASAN: vmalloc-out-of-bounds Write in imageblit (6)
From: syzbot @ 2025-11-19 10:42 UTC (permalink / raw)
To: linux-fbdev, linux-kernel, ssranevjti, syzkaller-bugs
In-Reply-To: <e69c10c5-ee82-4229-b7b6-e3993442595b@gmail.com>
Hello,
syzbot has tested the proposed patch and the reproducer did not trigger any issue:
Reported-by: syzbot+5a40432dfe8f86ee657a@syzkaller.appspotmail.com
Tested-by: syzbot+5a40432dfe8f86ee657a@syzkaller.appspotmail.com
Tested on:
commit: 8b690556 Merge tag 'for-linus' of git://git.kernel.org..
git tree: upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=10921212580000
kernel config: https://syzkaller.appspot.com/x/.config?x=1cd7f786c0f5182f
dashboard link: https://syzkaller.appspot.com/bug?extid=5a40432dfe8f86ee657a
compiler: gcc (Debian 12.2.0-14+deb12u1) 12.2.0, GNU ld (GNU Binutils for Debian) 2.40
patch: https://syzkaller.appspot.com/x/patch.diff?x=13a21212580000
Note: testing is done by a robot and is best-effort only.
^ permalink raw reply
* Re: [syzbot] [fbdev?] KASAN: vmalloc-out-of-bounds Write in imageblit (6)
From: shaurya @ 2025-11-19 10:18 UTC (permalink / raw)
To: syzbot+5a40432dfe8f86ee657a; +Cc: linux-fbdev, linux-kernel, syzkaller-bugs
In-Reply-To: <691c279e.a70a0220.3124cb.00b5.GAE@google.com>
[-- Attachment #1: Type: text/plain, Size: 82 bytes --]
#syz test:
git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
[-- Attachment #2: 0001-testing-my-fix.patch --]
[-- Type: text/x-patch, Size: 3132 bytes --]
From 188fc7eea4a00500a806b7b122d20289abc2bf00 Mon Sep 17 00:00:00 2001
From: Shaurya Rane <ssrane_b23@ee.vjti.ac.in>
Date: Wed, 19 Nov 2025 15:44:51 +0530
Subject: [PATCH] testing my fix
Signed-off-by: Shaurya Rane <ssrane_b23@ee.vjti.ac.in>
---
drivers/video/fbdev/core/fb_imageblit.h | 66 +++++++++++++++++++++++--
1 file changed, 62 insertions(+), 4 deletions(-)
diff --git a/drivers/video/fbdev/core/fb_imageblit.h b/drivers/video/fbdev/core/fb_imageblit.h
index 3b2bb4946505..aee7f4032164 100644
--- a/drivers/video/fbdev/core/fb_imageblit.h
+++ b/drivers/video/fbdev/core/fb_imageblit.h
@@ -485,11 +485,69 @@ static inline void fb_imageblit(struct fb_info *p, const struct fb_image *image)
struct fb_address dst = fb_address_init(p);
struct fb_reverse reverse = fb_reverse_init(p);
const u32 *palette = fb_palette(p);
+ struct fb_image clipped_image;
+ u32 max_x, max_y;
+ unsigned long max_offset_bytes;
+
+ /* Validate basic parameters */
+ if (!image || !p->screen_buffer || !p->screen_size ||
+ !image->width || !image->height)
+ return;
+
+ /* Calculate maximum addressable coordinates based on virtual resolution and buffer size */
+ max_x = p->var.xres_virtual;
+ max_y = p->var.yres_virtual;
+
+ /* Also check against actual buffer size to prevent vmalloc overflow */
+ {
+ unsigned long effective_width_bytes;
+ u32 right_edge = image->dx + image->width;
+
+ if (right_edge < image->dx)
+ right_edge = max_x;
+ else
+ right_edge = min(right_edge, max_x);
+
+ effective_width_bytes = (unsigned long)right_edge * bpp;
+ effective_width_bytes = (effective_width_bytes + 7) / 8;
+
+ if (effective_width_bytes > p->screen_size) {
+ max_y = 0;
+ } else if (p->fix.line_length) {
+ u32 max_lines = (p->screen_size - effective_width_bytes) /
+ p->fix.line_length + 1;
+ if (max_lines < max_y)
+ max_y = max_lines;
+ }
+ }
+
+ /* If image is completely outside bounds, skip it */
+ if (image->dx >= max_x || image->dy >= max_y)
+ return;
+
+ /* Create clipped image - clip to virtual resolution bounds */
+ clipped_image = *image;
+
+ /* Clip width if it extends beyond right edge */
+ if (clipped_image.dx + clipped_image.width > max_x) {
+ if (clipped_image.dx < max_x)
+ clipped_image.width = max_x - clipped_image.dx;
+ else
+ return; /* completely outside */
+ }
+
+ /* Clip height if it extends beyond bottom edge */
+ if (clipped_image.dy + clipped_image.height > max_y) {
+ if (clipped_image.dy < max_y)
+ clipped_image.height = max_y - clipped_image.dy;
+ else
+ return; /* completely outside */
+ }
- fb_address_forward(&dst, image->dy * bits_per_line + image->dx * bpp);
+ fb_address_forward(&dst, clipped_image.dy * bits_per_line + clipped_image.dx * bpp);
- if (image->depth == 1)
- fb_bitmap_imageblit(image, &dst, bits_per_line, palette, bpp, reverse);
+ if (clipped_image.depth == 1)
+ fb_bitmap_imageblit(&clipped_image, &dst, bits_per_line, palette, bpp, reverse);
else
- fb_color_imageblit(image, &dst, bits_per_line, palette, bpp, reverse);
+ fb_color_imageblit(&clipped_image, &dst, bits_per_line, palette, bpp, reverse);
}
--
2.34.1
^ permalink raw reply related
* [PATCH v2 1/2] backlight: Add Congatec Board Controller (CGBC) backlight support
From: Petri Karhula via B4 Relay @ 2025-11-19 8:25 UTC (permalink / raw)
To: Thomas Richard, Lee Jones, Daniel Thompson, Jingoo Han,
Helge Deller
Cc: linux-kernel, dri-devel, linux-fbdev, Petri Karhula
In-Reply-To: <20251119-cgbc-backlight-v2-0-4d4edd7ca662@novatron.fi>
From: Petri Karhula <petri.karhula@novatron.fi>
This driver provides backlight brightness control through the Linux
backlight subsystem. It communicates with the board controller to
adjust LCD backlight using PWM signals. Communication is done
through Congatec Board Controller core driver.
Signed-off-by: Petri Karhula <petri.karhula@novatron.fi>
---
drivers/video/backlight/Kconfig | 11 ++
drivers/video/backlight/Makefile | 1 +
drivers/video/backlight/cgbc_bl.c | 281 ++++++++++++++++++++++++++++++++++++++
3 files changed, 293 insertions(+)
diff --git a/drivers/video/backlight/Kconfig b/drivers/video/backlight/Kconfig
index d9374d208cee..702f3b8ed036 100644
--- a/drivers/video/backlight/Kconfig
+++ b/drivers/video/backlight/Kconfig
@@ -249,6 +249,17 @@ config BACKLIGHT_PWM
If you have a LCD backlight adjustable by PWM, say Y to enable
this driver.
+config BACKLIGHT_CGBC
+ tristate "Congatec Board Controller (CGBC) backlight support"
+ depends on MFD_CGBC && X86
+ help
+ Say Y here to enable support for LCD backlight control on Congatec
+ x86-based boards via the CGBC (Congatec Board Controller).
+
+ This driver provides backlight brightness control through the Linux
+ backlight subsystem. It communicates with the board controller to
+ adjust LCD backlight using PWM signals.
+
config BACKLIGHT_DA903X
tristate "Backlight Driver for DA9030/DA9034 using WLED"
depends on PMIC_DA903X
diff --git a/drivers/video/backlight/Makefile b/drivers/video/backlight/Makefile
index dfbb169bf6ea..0169fd8873ed 100644
--- a/drivers/video/backlight/Makefile
+++ b/drivers/video/backlight/Makefile
@@ -27,6 +27,7 @@ obj-$(CONFIG_BACKLIGHT_APPLE_DWI) += apple_dwi_bl.o
obj-$(CONFIG_BACKLIGHT_AS3711) += as3711_bl.o
obj-$(CONFIG_BACKLIGHT_BD6107) += bd6107.o
obj-$(CONFIG_BACKLIGHT_CLASS_DEVICE) += backlight.o
+obj-$(CONFIG_BACKLIGHT_CGBC) += cgbc_bl.o
obj-$(CONFIG_BACKLIGHT_DA903X) += da903x_bl.o
obj-$(CONFIG_BACKLIGHT_DA9052) += da9052_bl.o
obj-$(CONFIG_BACKLIGHT_EP93XX) += ep93xx_bl.o
diff --git a/drivers/video/backlight/cgbc_bl.c b/drivers/video/backlight/cgbc_bl.c
new file mode 100644
index 000000000000..4382321f4cac
--- /dev/null
+++ b/drivers/video/backlight/cgbc_bl.c
@@ -0,0 +1,281 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Congatec Board Controller (CGBC) Backlight Driver
+ *
+ * This driver provides backlight control for LCD displays connected to
+ * Congatec boards via the CGBC (Congatec Board Controller). It integrates
+ * with the Linux backlight subsystem and communicates with hardware through
+ * the cgbc-core module.
+ *
+ * Copyright (C) 2025 Novatron Oy
+ *
+ * Author: Petri Karhula <petri.karhula@novatron.fi>
+ */
+
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/backlight.h>
+
+#include <linux/mfd/cgbc.h>
+
+#define CGBC_BL_DRIVER_VERSION "0.0.1"
+
+#define BLT_PWM_DUTY_MASK 0x7F
+#define BLT_PWM_INVERTED_MASK 0x80
+
+/* CGBC command for PWM brightness control*/
+#define CGBC_CMD_BLT0_PWM 0x75
+
+#define CGBC_BL_MAX_BRIGHTNESS 100
+
+/**
+ * CGBC backlight driver data
+ * @dev: Pointer to the platform device
+ * @bl_dev: Pointer to the backlight device
+ * @cgbc: Pointer to the parent CGBC device data
+ * @current_brightness: Current brightness level (0-100)
+ */
+struct cgbc_bl_data {
+ struct device *dev;
+ struct backlight_device *bl_dev;
+ struct cgbc_device_data *cgbc;
+ unsigned int current_brightness;
+};
+
+/**
+ * Read current PWM settings from hardware
+ * @bl_data: Backlight driver data
+ *
+ * Reads the current PWM duty cycle percentage (= brightness level)
+ * from the board controller.
+ *
+ * Return: 0 on success, negative error code on failure
+ */
+static int cgbc_bl_read_pwm_settings(struct cgbc_bl_data *bl_data)
+{
+ u8 cmd_buf[4] = { CGBC_CMD_BLT0_PWM, 0, 0, 0 };
+ u8 reply_buf[3];
+ int ret;
+
+ ret = cgbc_command(bl_data->cgbc, cmd_buf, sizeof(cmd_buf), reply_buf,
+ sizeof(reply_buf), NULL);
+
+ if (ret < 0) {
+ dev_err(bl_data->dev, "Failed to read PWM settings: %d\n", ret);
+ return ret;
+ }
+
+ /*
+ * Only return PWM duty factor percentage,
+ * ignore polarity inversion bit (bit 7)
+ */
+ bl_data->current_brightness = reply_buf[0] & BLT_PWM_DUTY_MASK;
+
+ dev_dbg(bl_data->dev, "Current PWM duty=%u\n", bl_data->current_brightness);
+
+ return 0;
+}
+
+/**
+ * Set backlight brightness
+ * @bl_data: Backlight driver data
+ * @brightness: Brightness level (0-100)
+ *
+ * Sets the backlight brightness by configuring the PWM duty cycle.
+ * Preserves the current polarity and frequency settings.
+ *
+ * Return: 0 on success, negative error code on failure
+ */
+static int cgbc_bl_set_brightness(struct cgbc_bl_data *bl_data, u8 brightness)
+{
+ u8 cmd_buf[4] = { CGBC_CMD_BLT0_PWM, 0, 0, 0 };
+ u8 reply_buf[3];
+ int ret;
+
+ /* Read the current values */
+ ret = cgbc_command(bl_data->cgbc, cmd_buf, sizeof(cmd_buf), reply_buf,
+ sizeof(reply_buf), NULL);
+
+ if (ret < 0) {
+ dev_err(bl_data->dev, "Failed to read PWM settings: %d\n", ret);
+ return ret;
+ }
+
+ /*
+ * Prepare command buffer for writing new settings. Only 2nd byte is changed
+ * to set new brightness (PWM duty cycle %). Other balues (polarity, frequency)
+ * are preserved from the read values.
+ */
+ cmd_buf[1] = (reply_buf[0] & BLT_PWM_INVERTED_MASK) |
+ (BLT_PWM_DUTY_MASK & brightness);
+ cmd_buf[2] = reply_buf[1];
+ cmd_buf[3] = reply_buf[2];
+
+ ret = cgbc_command(bl_data->cgbc, cmd_buf, sizeof(cmd_buf), reply_buf,
+ sizeof(reply_buf), NULL);
+
+ if (ret < 0) {
+ dev_err(bl_data->dev, "Failed to set brightness: %d\n", ret);
+ return ret;
+ }
+
+ bl_data->current_brightness = reply_buf[0] & BLT_PWM_DUTY_MASK;
+
+ /* Verify the setting was applied correctly */
+ if (bl_data->current_brightness != brightness) {
+ dev_err(bl_data->dev,
+ "Brightness setting verification failed\n");
+ return -EIO;
+ }
+
+ dev_dbg(bl_data->dev, "Set brightness to %u\n", brightness);
+
+ return 0;
+}
+
+/**
+ * Backlight update callback
+ * @bl: Backlight device
+ *
+ * Called by the backlight subsystem when brightness needs to be updated.
+ * Changes the brightness level on the hardware
+ * if requested value differs from the current setting.
+ *
+ * Return: 0 on success, negative error code on failure
+ */
+static int cgbc_bl_update_status(struct backlight_device *bl)
+{
+ struct cgbc_bl_data *bl_data = bl_get_data(bl);
+ u8 brightness;
+ int ret;
+
+ brightness = bl->props.brightness;
+
+ if (brightness != bl_data->current_brightness) {
+ ret = cgbc_bl_set_brightness(bl_data, brightness);
+
+ if (ret < 0) {
+ dev_err(bl_data->dev, "Failed to set brightness: %d\n",
+ ret);
+ return ret;
+ }
+ }
+
+ return 0;
+}
+
+/**
+ * Get current backlight brightness
+ * @bl: Backlight device
+ *
+ * Returns the current brightness level by reading from hardware.
+ *
+ * Return: Current brightness level (0-100), or negative error code
+ */
+static int cgbc_bl_get_brightness(struct backlight_device *bl)
+{
+ struct cgbc_bl_data *bl_data = bl_get_data(bl);
+ int ret;
+
+ /* Read current PWM brightness settings */
+ ret = cgbc_bl_read_pwm_settings(bl_data);
+
+ if (ret < 0) {
+ dev_err(bl_data->dev, "Failed to read PWM settings: %d\n", ret);
+ return ret;
+ }
+
+ return bl_data->current_brightness;
+}
+
+/* Backlight device operations */
+static const struct backlight_ops cgbc_bl_ops = {
+ .options = BL_CORE_SUSPENDRESUME,
+ .update_status = cgbc_bl_update_status,
+ .get_brightness = cgbc_bl_get_brightness,
+};
+
+/**
+ * Probe function for CGBC backlight driver
+ * @pdev: Platform device
+ *
+ * Initializes the CGBC backlight driver and registers it with the
+ * Linux backlight subsystem.
+ *
+ * Return: 0 on success, negative error code on failure
+ */
+static int cgbc_bl_probe(struct platform_device *pdev)
+{
+ struct cgbc_device_data *cgbc = dev_get_drvdata(pdev->dev.parent);
+ struct cgbc_bl_data *bl_data;
+ struct backlight_properties props;
+ struct backlight_device *bl_dev;
+ int ret;
+
+ bl_data = devm_kzalloc(&pdev->dev, sizeof(*bl_data), GFP_KERNEL);
+
+ if (!bl_data)
+ return -ENOMEM;
+
+ bl_data->dev = &pdev->dev;
+ bl_data->cgbc = cgbc;
+
+ ret = cgbc_bl_read_pwm_settings(bl_data);
+
+ if (ret) {
+ dev_err(&pdev->dev, "Failed to read initial PWM settings: %d\n",
+ ret);
+ return ret;
+ }
+
+ memset(&props, 0, sizeof(props));
+ props.type = BACKLIGHT_PLATFORM;
+ props.max_brightness = CGBC_BL_MAX_BRIGHTNESS;
+ props.brightness = bl_data->current_brightness;
+
+ bl_dev = devm_backlight_device_register(&pdev->dev, "cgbc-backlight",
+ &pdev->dev, bl_data,
+ &cgbc_bl_ops, &props);
+
+ if (IS_ERR(bl_dev)) {
+ dev_err(&pdev->dev, "Failed to register backlight device\n");
+ return PTR_ERR(bl_dev);
+ }
+
+ bl_data->bl_dev = bl_dev;
+ platform_set_drvdata(pdev, bl_data);
+
+ dev_info(&pdev->dev,
+ "CGBC backlight driver registered (brightness=%u)\n",
+ bl_data->current_brightness);
+
+ return 0;
+}
+
+/**
+ * Remove function for CGBC backlight driver
+ * @pdev: Platform device
+ *
+ * The Linux device-managed resource framework (devres) does the cleanup.
+ * No explicit cleanup is needed here.
+ */
+static void cgbc_bl_remove(struct platform_device *pdev)
+{
+ dev_info(&pdev->dev, "CGBC backlight driver removed\n");
+}
+
+static struct platform_driver cgbc_bl_driver = {
+ .driver = {
+ .name = "cgbc-backlight",
+ },
+ .probe = cgbc_bl_probe,
+ .remove = cgbc_bl_remove,
+};
+
+module_platform_driver(cgbc_bl_driver);
+
+MODULE_AUTHOR("Petri Karhula <petri.karhula@novatron.fi>");
+MODULE_DESCRIPTION("Congatec Board Controller (CGBC) Backlight Driver");
+MODULE_LICENSE("GPL");
+MODULE_VERSION(CGBC_BL_DRIVER_VERSION);
+MODULE_ALIAS("platform:cgbc-backlight");
--
2.34.1
^ permalink raw reply related
* [PATCH v2 2/2] mfd: cgbc: Add support for backlight
From: Petri Karhula via B4 Relay @ 2025-11-19 8:25 UTC (permalink / raw)
To: Thomas Richard, Lee Jones, Daniel Thompson, Jingoo Han,
Helge Deller
Cc: linux-kernel, dri-devel, linux-fbdev, Petri Karhula
In-Reply-To: <20251119-cgbc-backlight-v2-0-4d4edd7ca662@novatron.fi>
From: Petri Karhula <petri.karhula@novatron.fi>
The Board Controller has control for display backlight.
Add backlight cell for the cgbc-backlight driver which
adds support for backlight brightness control.
Signed-off-by: Petri Karhula <petri.karhula@novatron.fi>
---
drivers/mfd/cgbc-core.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/mfd/cgbc-core.c b/drivers/mfd/cgbc-core.c
index 4782ff1114a9..10bb4b414c34 100644
--- a/drivers/mfd/cgbc-core.c
+++ b/drivers/mfd/cgbc-core.c
@@ -237,6 +237,7 @@ static struct mfd_cell cgbc_devs[] = {
{ .name = "cgbc-i2c", .id = 1 },
{ .name = "cgbc-i2c", .id = 2 },
{ .name = "cgbc-hwmon" },
+ { .name = "cgbc-backlight" },
};
static int cgbc_map(struct cgbc_device_data *cgbc)
--
2.34.1
^ permalink raw reply related
* [PATCH v2 0/2] Backlight driver to control backlight behind Congatec Board Controller.
From: Petri Karhula via B4 Relay @ 2025-11-19 8:25 UTC (permalink / raw)
To: Thomas Richard, Lee Jones, Daniel Thompson, Jingoo Han,
Helge Deller
Cc: linux-kernel, dri-devel, linux-fbdev, Petri Karhula
This driver provides backlight brightness control through the Linux
backlight subsystem. It communicates with the board controller to
adjust LCD backlight using PWM signals. Communication is done
through Congatec Board Controller core driver.
Signed-off-by: Petri Karhula <petri.karhula@novatron.fi>
---
Changes in v2:
- Separated Board Controller core driver change into its own patch
- Link to v1: https://lore.kernel.org/r/20251118-cgbc-backlight-v1-1-cc6ac5301034@novatron.fi
---
Petri Karhula (2):
backlight: Add Congatec Board Controller (CGBC) backlight support
mfd: cgbc: Add support for backlight
drivers/mfd/cgbc-core.c | 1 +
drivers/video/backlight/Kconfig | 11 ++
drivers/video/backlight/Makefile | 1 +
drivers/video/backlight/cgbc_bl.c | 281 ++++++++++++++++++++++++++++++++++++++
4 files changed, 294 insertions(+)
---
base-commit: e7c375b181600caf135cfd03eadbc45eb530f2cb
change-id: 20251118-cgbc-backlight-35c1109db0b8
Best regards,
--
Petri Karhula <petri.karhula@novatron.fi>
^ permalink raw reply
* Re: [PATCH] fbdev/tridentfb: replace printk() with dev_*() in probe
From: Javier Garcia @ 2025-11-19 7:16 UTC (permalink / raw)
To: deller; +Cc: linux-fbdev, dri-devel, linux-kernel, shuah
In-Reply-To: <20251115125701.3228804-1-rampxxxx@gmail.com>
Hi Deller,
Any comments on this patch?,
Thanks!
---
Javier Garcia
On Sat, 15 Nov 2025 at 13:57, Javier Garcia <rampxxxx@gmail.com> wrote:
>
> - Replace in `trident_pc_probe()` printk by dev_* fn's
> - Delete the prefix `tridentfb:` from msg strings, not needed now.
>
> Signed-off-by: Javier Garcia <rampxxxx@gmail.com>
> ---
> drivers/video/fbdev/tridentfb.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/video/fbdev/tridentfb.c b/drivers/video/fbdev/tridentfb.c
> index 516cf2a18757..17b7253b8fbe 100644
> --- a/drivers/video/fbdev/tridentfb.c
> +++ b/drivers/video/fbdev/tridentfb.c
> @@ -1631,7 +1631,7 @@ static int trident_pci_probe(struct pci_dev *dev,
> }
>
> if (noaccel) {
> - printk(KERN_DEBUG "disabling acceleration\n");
> + dev_dbg(&dev->dev, "disabling acceleration\n");
> info->flags |= FBINFO_HWACCEL_DISABLED;
> info->pixmap.scan_align = 1;
> }
> @@ -1693,7 +1693,7 @@ static int trident_pci_probe(struct pci_dev *dev,
> info->var.activate |= FB_ACTIVATE_NOW;
> info->device = &dev->dev;
> if (register_framebuffer(info) < 0) {
> - printk(KERN_ERR "tridentfb: could not register framebuffer\n");
> + dev_err(&dev->dev, "could not register framebuffer\n");
> fb_dealloc_cmap(&info->cmap);
> err = -EINVAL;
> goto out_unmap2;
> --
> 2.50.1
>
^ permalink raw reply
* Re: [PATCH] backlight: Add Congatec Board Controller (CGBC) backlight support
From: Lee Jones @ 2025-11-18 18:10 UTC (permalink / raw)
To: petri.karhula
Cc: Thomas Richard, Daniel Thompson, Jingoo Han, Helge Deller,
linux-kernel, dri-devel, linux-fbdev
In-Reply-To: <20251118-cgbc-backlight-v1-1-cc6ac5301034@novatron.fi>
On Tue, 18 Nov 2025, Petri Karhula via B4 Relay wrote:
> From: Petri Karhula <petri.karhula@novatron.fi>
>
> This driver provides backlight brightness control through the Linux
> backlight subsystem. It communicates with the board controller to
> adjust LCD backlight using PWM signals. Communication is done
> through Congatec Board Controller core driver.
>
> Signed-off-by: Petri Karhula <petri.karhula@novatron.fi>
> ---
> Backlight driver to control backlight behind Congatec Board Controller.
> This driver provides backlight brightness control through the Linux
> backlight subsystem. It communicates with the board controller to
> adjust LCD backlight using PWM signals. Communication is done
> through Congatec Board Controller core driver.
>
> ---
> drivers/mfd/cgbc-core.c | 1 +
Please separate this out into its own patch.
> drivers/video/backlight/Kconfig | 11 ++
> drivers/video/backlight/Makefile | 1 +
> drivers/video/backlight/cgbc_bl.c | 281 ++++++++++++++++++++++++++++++++++++++
> 4 files changed, 294 insertions(+)
[...]
--
Lee Jones [李琼斯]
^ permalink raw reply
* [PATCH] backlight: Add Congatec Board Controller (CGBC) backlight support
From: Petri Karhula via B4 Relay @ 2025-11-18 16:43 UTC (permalink / raw)
To: Thomas Richard, Lee Jones, Daniel Thompson, Jingoo Han,
Helge Deller
Cc: linux-kernel, dri-devel, linux-fbdev, Petri Karhula
From: Petri Karhula <petri.karhula@novatron.fi>
This driver provides backlight brightness control through the Linux
backlight subsystem. It communicates with the board controller to
adjust LCD backlight using PWM signals. Communication is done
through Congatec Board Controller core driver.
Signed-off-by: Petri Karhula <petri.karhula@novatron.fi>
---
Backlight driver to control backlight behind Congatec Board Controller.
This driver provides backlight brightness control through the Linux
backlight subsystem. It communicates with the board controller to
adjust LCD backlight using PWM signals. Communication is done
through Congatec Board Controller core driver.
---
drivers/mfd/cgbc-core.c | 1 +
drivers/video/backlight/Kconfig | 11 ++
drivers/video/backlight/Makefile | 1 +
drivers/video/backlight/cgbc_bl.c | 281 ++++++++++++++++++++++++++++++++++++++
4 files changed, 294 insertions(+)
diff --git a/drivers/mfd/cgbc-core.c b/drivers/mfd/cgbc-core.c
index 4782ff1114a9..10bb4b414c34 100644
--- a/drivers/mfd/cgbc-core.c
+++ b/drivers/mfd/cgbc-core.c
@@ -237,6 +237,7 @@ static struct mfd_cell cgbc_devs[] = {
{ .name = "cgbc-i2c", .id = 1 },
{ .name = "cgbc-i2c", .id = 2 },
{ .name = "cgbc-hwmon" },
+ { .name = "cgbc-backlight" },
};
static int cgbc_map(struct cgbc_device_data *cgbc)
diff --git a/drivers/video/backlight/Kconfig b/drivers/video/backlight/Kconfig
index d9374d208cee..702f3b8ed036 100644
--- a/drivers/video/backlight/Kconfig
+++ b/drivers/video/backlight/Kconfig
@@ -249,6 +249,17 @@ config BACKLIGHT_PWM
If you have a LCD backlight adjustable by PWM, say Y to enable
this driver.
+config BACKLIGHT_CGBC
+ tristate "Congatec Board Controller (CGBC) backlight support"
+ depends on MFD_CGBC && X86
+ help
+ Say Y here to enable support for LCD backlight control on Congatec
+ x86-based boards via the CGBC (Congatec Board Controller).
+
+ This driver provides backlight brightness control through the Linux
+ backlight subsystem. It communicates with the board controller to
+ adjust LCD backlight using PWM signals.
+
config BACKLIGHT_DA903X
tristate "Backlight Driver for DA9030/DA9034 using WLED"
depends on PMIC_DA903X
diff --git a/drivers/video/backlight/Makefile b/drivers/video/backlight/Makefile
index dfbb169bf6ea..0169fd8873ed 100644
--- a/drivers/video/backlight/Makefile
+++ b/drivers/video/backlight/Makefile
@@ -27,6 +27,7 @@ obj-$(CONFIG_BACKLIGHT_APPLE_DWI) += apple_dwi_bl.o
obj-$(CONFIG_BACKLIGHT_AS3711) += as3711_bl.o
obj-$(CONFIG_BACKLIGHT_BD6107) += bd6107.o
obj-$(CONFIG_BACKLIGHT_CLASS_DEVICE) += backlight.o
+obj-$(CONFIG_BACKLIGHT_CGBC) += cgbc_bl.o
obj-$(CONFIG_BACKLIGHT_DA903X) += da903x_bl.o
obj-$(CONFIG_BACKLIGHT_DA9052) += da9052_bl.o
obj-$(CONFIG_BACKLIGHT_EP93XX) += ep93xx_bl.o
diff --git a/drivers/video/backlight/cgbc_bl.c b/drivers/video/backlight/cgbc_bl.c
new file mode 100644
index 000000000000..4382321f4cac
--- /dev/null
+++ b/drivers/video/backlight/cgbc_bl.c
@@ -0,0 +1,281 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Congatec Board Controller (CGBC) Backlight Driver
+ *
+ * This driver provides backlight control for LCD displays connected to
+ * Congatec boards via the CGBC (Congatec Board Controller). It integrates
+ * with the Linux backlight subsystem and communicates with hardware through
+ * the cgbc-core module.
+ *
+ * Copyright (C) 2025 Novatron Oy
+ *
+ * Author: Petri Karhula <petri.karhula@novatron.fi>
+ */
+
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/backlight.h>
+
+#include <linux/mfd/cgbc.h>
+
+#define CGBC_BL_DRIVER_VERSION "0.0.1"
+
+#define BLT_PWM_DUTY_MASK 0x7F
+#define BLT_PWM_INVERTED_MASK 0x80
+
+/* CGBC command for PWM brightness control*/
+#define CGBC_CMD_BLT0_PWM 0x75
+
+#define CGBC_BL_MAX_BRIGHTNESS 100
+
+/**
+ * CGBC backlight driver data
+ * @dev: Pointer to the platform device
+ * @bl_dev: Pointer to the backlight device
+ * @cgbc: Pointer to the parent CGBC device data
+ * @current_brightness: Current brightness level (0-100)
+ */
+struct cgbc_bl_data {
+ struct device *dev;
+ struct backlight_device *bl_dev;
+ struct cgbc_device_data *cgbc;
+ unsigned int current_brightness;
+};
+
+/**
+ * Read current PWM settings from hardware
+ * @bl_data: Backlight driver data
+ *
+ * Reads the current PWM duty cycle percentage (= brightness level)
+ * from the board controller.
+ *
+ * Return: 0 on success, negative error code on failure
+ */
+static int cgbc_bl_read_pwm_settings(struct cgbc_bl_data *bl_data)
+{
+ u8 cmd_buf[4] = { CGBC_CMD_BLT0_PWM, 0, 0, 0 };
+ u8 reply_buf[3];
+ int ret;
+
+ ret = cgbc_command(bl_data->cgbc, cmd_buf, sizeof(cmd_buf), reply_buf,
+ sizeof(reply_buf), NULL);
+
+ if (ret < 0) {
+ dev_err(bl_data->dev, "Failed to read PWM settings: %d\n", ret);
+ return ret;
+ }
+
+ /*
+ * Only return PWM duty factor percentage,
+ * ignore polarity inversion bit (bit 7)
+ */
+ bl_data->current_brightness = reply_buf[0] & BLT_PWM_DUTY_MASK;
+
+ dev_dbg(bl_data->dev, "Current PWM duty=%u\n", bl_data->current_brightness);
+
+ return 0;
+}
+
+/**
+ * Set backlight brightness
+ * @bl_data: Backlight driver data
+ * @brightness: Brightness level (0-100)
+ *
+ * Sets the backlight brightness by configuring the PWM duty cycle.
+ * Preserves the current polarity and frequency settings.
+ *
+ * Return: 0 on success, negative error code on failure
+ */
+static int cgbc_bl_set_brightness(struct cgbc_bl_data *bl_data, u8 brightness)
+{
+ u8 cmd_buf[4] = { CGBC_CMD_BLT0_PWM, 0, 0, 0 };
+ u8 reply_buf[3];
+ int ret;
+
+ /* Read the current values */
+ ret = cgbc_command(bl_data->cgbc, cmd_buf, sizeof(cmd_buf), reply_buf,
+ sizeof(reply_buf), NULL);
+
+ if (ret < 0) {
+ dev_err(bl_data->dev, "Failed to read PWM settings: %d\n", ret);
+ return ret;
+ }
+
+ /*
+ * Prepare command buffer for writing new settings. Only 2nd byte is changed
+ * to set new brightness (PWM duty cycle %). Other balues (polarity, frequency)
+ * are preserved from the read values.
+ */
+ cmd_buf[1] = (reply_buf[0] & BLT_PWM_INVERTED_MASK) |
+ (BLT_PWM_DUTY_MASK & brightness);
+ cmd_buf[2] = reply_buf[1];
+ cmd_buf[3] = reply_buf[2];
+
+ ret = cgbc_command(bl_data->cgbc, cmd_buf, sizeof(cmd_buf), reply_buf,
+ sizeof(reply_buf), NULL);
+
+ if (ret < 0) {
+ dev_err(bl_data->dev, "Failed to set brightness: %d\n", ret);
+ return ret;
+ }
+
+ bl_data->current_brightness = reply_buf[0] & BLT_PWM_DUTY_MASK;
+
+ /* Verify the setting was applied correctly */
+ if (bl_data->current_brightness != brightness) {
+ dev_err(bl_data->dev,
+ "Brightness setting verification failed\n");
+ return -EIO;
+ }
+
+ dev_dbg(bl_data->dev, "Set brightness to %u\n", brightness);
+
+ return 0;
+}
+
+/**
+ * Backlight update callback
+ * @bl: Backlight device
+ *
+ * Called by the backlight subsystem when brightness needs to be updated.
+ * Changes the brightness level on the hardware
+ * if requested value differs from the current setting.
+ *
+ * Return: 0 on success, negative error code on failure
+ */
+static int cgbc_bl_update_status(struct backlight_device *bl)
+{
+ struct cgbc_bl_data *bl_data = bl_get_data(bl);
+ u8 brightness;
+ int ret;
+
+ brightness = bl->props.brightness;
+
+ if (brightness != bl_data->current_brightness) {
+ ret = cgbc_bl_set_brightness(bl_data, brightness);
+
+ if (ret < 0) {
+ dev_err(bl_data->dev, "Failed to set brightness: %d\n",
+ ret);
+ return ret;
+ }
+ }
+
+ return 0;
+}
+
+/**
+ * Get current backlight brightness
+ * @bl: Backlight device
+ *
+ * Returns the current brightness level by reading from hardware.
+ *
+ * Return: Current brightness level (0-100), or negative error code
+ */
+static int cgbc_bl_get_brightness(struct backlight_device *bl)
+{
+ struct cgbc_bl_data *bl_data = bl_get_data(bl);
+ int ret;
+
+ /* Read current PWM brightness settings */
+ ret = cgbc_bl_read_pwm_settings(bl_data);
+
+ if (ret < 0) {
+ dev_err(bl_data->dev, "Failed to read PWM settings: %d\n", ret);
+ return ret;
+ }
+
+ return bl_data->current_brightness;
+}
+
+/* Backlight device operations */
+static const struct backlight_ops cgbc_bl_ops = {
+ .options = BL_CORE_SUSPENDRESUME,
+ .update_status = cgbc_bl_update_status,
+ .get_brightness = cgbc_bl_get_brightness,
+};
+
+/**
+ * Probe function for CGBC backlight driver
+ * @pdev: Platform device
+ *
+ * Initializes the CGBC backlight driver and registers it with the
+ * Linux backlight subsystem.
+ *
+ * Return: 0 on success, negative error code on failure
+ */
+static int cgbc_bl_probe(struct platform_device *pdev)
+{
+ struct cgbc_device_data *cgbc = dev_get_drvdata(pdev->dev.parent);
+ struct cgbc_bl_data *bl_data;
+ struct backlight_properties props;
+ struct backlight_device *bl_dev;
+ int ret;
+
+ bl_data = devm_kzalloc(&pdev->dev, sizeof(*bl_data), GFP_KERNEL);
+
+ if (!bl_data)
+ return -ENOMEM;
+
+ bl_data->dev = &pdev->dev;
+ bl_data->cgbc = cgbc;
+
+ ret = cgbc_bl_read_pwm_settings(bl_data);
+
+ if (ret) {
+ dev_err(&pdev->dev, "Failed to read initial PWM settings: %d\n",
+ ret);
+ return ret;
+ }
+
+ memset(&props, 0, sizeof(props));
+ props.type = BACKLIGHT_PLATFORM;
+ props.max_brightness = CGBC_BL_MAX_BRIGHTNESS;
+ props.brightness = bl_data->current_brightness;
+
+ bl_dev = devm_backlight_device_register(&pdev->dev, "cgbc-backlight",
+ &pdev->dev, bl_data,
+ &cgbc_bl_ops, &props);
+
+ if (IS_ERR(bl_dev)) {
+ dev_err(&pdev->dev, "Failed to register backlight device\n");
+ return PTR_ERR(bl_dev);
+ }
+
+ bl_data->bl_dev = bl_dev;
+ platform_set_drvdata(pdev, bl_data);
+
+ dev_info(&pdev->dev,
+ "CGBC backlight driver registered (brightness=%u)\n",
+ bl_data->current_brightness);
+
+ return 0;
+}
+
+/**
+ * Remove function for CGBC backlight driver
+ * @pdev: Platform device
+ *
+ * The Linux device-managed resource framework (devres) does the cleanup.
+ * No explicit cleanup is needed here.
+ */
+static void cgbc_bl_remove(struct platform_device *pdev)
+{
+ dev_info(&pdev->dev, "CGBC backlight driver removed\n");
+}
+
+static struct platform_driver cgbc_bl_driver = {
+ .driver = {
+ .name = "cgbc-backlight",
+ },
+ .probe = cgbc_bl_probe,
+ .remove = cgbc_bl_remove,
+};
+
+module_platform_driver(cgbc_bl_driver);
+
+MODULE_AUTHOR("Petri Karhula <petri.karhula@novatron.fi>");
+MODULE_DESCRIPTION("Congatec Board Controller (CGBC) Backlight Driver");
+MODULE_LICENSE("GPL");
+MODULE_VERSION(CGBC_BL_DRIVER_VERSION);
+MODULE_ALIAS("platform:cgbc-backlight");
---
base-commit: e7c375b181600caf135cfd03eadbc45eb530f2cb
change-id: 20251118-cgbc-backlight-35c1109db0b8
Best regards,
--
Petri Karhula <petri.karhula@novatron.fi>
^ permalink raw reply related
* Re: [PATCH] backlight: pwm_bl: apply the initial backlight state with sane defaults
From: Daniel Thompson @ 2025-11-18 12:52 UTC (permalink / raw)
To: Mark Brown
Cc: Michael Grzeschik, Uwe Kleine-König, Lee Jones, Jingoo Han,
Helge Deller, Pengutronix, linux-pwm, dri-devel, linux-fbdev,
linux-kernel
In-Reply-To: <f492d4d3-751c-40a3-bb93-0e1bb192cde7@sirena.org.uk>
On Fri, Nov 14, 2025 at 02:09:56PM +0000, Mark Brown wrote:
> On Thu, Jul 31, 2025 at 10:47:18AM +0200, Michael Grzeschik wrote:
> > Currently when calling pwm_apply_might_sleep in the probe routine
> > the pwm will be configured with an not fully defined state.
> >
> > The duty_cycle is not yet set in that moment. There is a final
> > backlight_update_status call that will have a properly setup state.
> > However this change in the backlight can create a short flicker if the
> > backlight was already preinitialised.
>
> I'm seeing the libre.computer Renegade Elite producing warnings during
> boot in -next which bisect to this patch. The warnings are:
>
> [ 24.175095] input: adc-keys as /devices/platform/adc-keys/input/input1
> [ 24.176612] ------------[ cut here ]------------
> [ 24.177048] WARNING: CPU: 0 PID: 0 at kernel/context_tracking.c:127 ct_kernel_exit.constprop.0+0x98/0xa0
>
> ...
>
> [ 24.190106] Call trace:
> [ 24.190325] ct_kernel_exit.constprop.0+0x98/0xa0 (P)
> [ 24.190775] ct_idle_enter+0x10/0x20
> [ 24.191096] cpuidle_enter_state+0x1fc/0x320
> [ 24.191476] cpuidle_enter+0x38/0x50
> [ 24.191802] do_idle+0x1e4/0x260
> [ 24.192094] cpu_startup_entry+0x34/0x3c
> [ 24.192444] rest_init+0xdc/0xe0
> [ 24.192734] console_on_rootfs+0x0/0x6c
> [ 24.193082] __primary_switched+0x88/0x90
> [ 24.193445] ---[ end trace 0000000000000000 ]---
>
> which seems a little surprising but there is some console stuff there
> that looks relevant.
>
> Full log:
>
> https://lava.sirena.org.uk/scheduler/job/2086528#L897
Michael, reading these logs it looks to me like the underlying oops
is this backtrace (which makes a lot more sense given the code you
altered):
[ 24.133631] Call trace:
[ 24.133853] pwm_backlight_probe+0x830/0x868 [pwm_bl] (P)
[ 24.134341] platform_probe+0x5c/0xa4
[ 24.134679] really_probe+0xbc/0x2c0
[ 24.135001] __driver_probe_device+0x78/0x120
[ 24.135391] driver_probe_device+0x3c/0x154
[ 24.135765] __driver_attach+0x90/0x1a0
[ 24.136111] bus_for_each_dev+0x7c/0xdc
[ 24.136462] driver_attach+0x24/0x38
[ 24.136785] bus_add_driver+0xe4/0x208
[ 24.137124] driver_register+0x68/0x130
[ 24.137468] __platform_driver_register+0x24/0x30
[ 24.137888] pwm_backlight_driver_init+0x20/0x1000 [pwm_bl]
[ 24.138389] do_one_initcall+0x60/0x1d4
[ 24.138735] do_init_module+0x54/0x23c
[ 24.139073] load_module+0x1760/0x1cf0
[ 24.139407] init_module_from_file+0x88/0xcc
[ 24.139787] __arm64_sys_finit_module+0x1bc/0x338
[ 24.140207] invoke_syscall+0x48/0x104
[ 24.140549] el0_svc_common.constprop.0+0x40/0xe0
[ 24.140970] do_el0_svc+0x1c/0x28
[ 24.141268] el0_svc+0x34/0xec
[ 24.141548] el0t_64_sync_handler+0xa0/0xf0
[ 24.141920] el0t_64_sync+0x198/0x19c
Should we back out the patch for now?
Daniel.
^ permalink raw reply
* [PATCH] fbdev: q40fb: request memory region
From: Sukrut Heroorkar @ 2025-11-18 9:56 UTC (permalink / raw)
To: Helge Deller, Sukrut Heroorkar, open list:FRAMEBUFFER LAYER,
open list:FRAMEBUFFER LAYER, open list
Cc: shuah, david.hunter.linux
The q40fb driver uses a fixed physical address but never reserves
the corresponding I/O region. Reserve the range as suggested in
Documentation/gpu/todo.rst ("Request memory regions in all fbdev drivers").
No functional change beyond claming the resource. This change is compile
tested only.
Signed-off-by: Sukrut Heroorkar <hsukrut3@gmail.com>
---
drivers/video/fbdev/q40fb.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/drivers/video/fbdev/q40fb.c b/drivers/video/fbdev/q40fb.c
index 1ff8fa176124..935260326c6f 100644
--- a/drivers/video/fbdev/q40fb.c
+++ b/drivers/video/fbdev/q40fb.c
@@ -101,6 +101,12 @@ static int q40fb_probe(struct platform_device *dev)
info->par = NULL;
info->screen_base = (char *) q40fb_fix.smem_start;
+ if (!request_mem_region(q40fb_fix.smem_start, q40fb_fix.smem_len,
+ "q40fb")) {
+ dev_err(&dev->dev, "cannot reserve video memory at 0x%lx\n",
+ q40fb_fix.smem_start);
+ }
+
if (fb_alloc_cmap(&info->cmap, 256, 0) < 0) {
framebuffer_release(info);
return -ENOMEM;
@@ -144,6 +150,7 @@ static int __init q40fb_init(void)
if (ret)
platform_driver_unregister(&q40fb_driver);
}
+
return ret;
}
--
2.43.0
^ permalink raw reply related
* Re: [syzbot] [fbdev?] KASAN: vmalloc-out-of-bounds Write in imageblit (6)
From: syzbot @ 2025-11-18 8:00 UTC (permalink / raw)
To: deller, dri-devel, linux-fbdev, linux-kernel, simona, soci,
syzkaller-bugs
In-Reply-To: <6907c8c8.a70a0220.37351b.0012.GAE@google.com>
syzbot has found a reproducer for the following issue on:
HEAD commit: e7c375b18160 Merge tag 'vfs-6.18-rc7.fixes' of gitolite.ke..
git tree: upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=15476692580000
kernel config: https://syzkaller.appspot.com/x/.config?x=1cd7f786c0f5182f
dashboard link: https://syzkaller.appspot.com/bug?extid=5a40432dfe8f86ee657a
compiler: gcc (Debian 12.2.0-14+deb12u1) 12.2.0, GNU ld (GNU Binutils for Debian) 2.40
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=10a70332580000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=173228b4580000
Downloadable assets:
disk image: https://storage.googleapis.com/syzbot-assets/1fcb660703f1/disk-e7c375b1.raw.xz
vmlinux: https://storage.googleapis.com/syzbot-assets/4bf314965321/vmlinux-e7c375b1.xz
kernel image: https://storage.googleapis.com/syzbot-assets/456b373fea36/bzImage-e7c375b1.xz
IMPORTANT: if you fix the issue, please add the following tag to the commit:
Reported-by: syzbot+5a40432dfe8f86ee657a@syzkaller.appspotmail.com
RDX: 0000000000000007 RSI: 0000000000000000 RDI: 0000000000000003
RBP: 00007fff34de03f0 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000001
R13: 00007fd0417e5fa0 R14: 00007fd0417e5fa0 R15: 0000000000000003
</TASK>
==================================================================
BUG: KASAN: vmalloc-out-of-bounds in fb_write_offset drivers/video/fbdev/core/sysmem.h:30 [inline]
BUG: KASAN: vmalloc-out-of-bounds in fb_bitmap_2ppw drivers/video/fbdev/core/fb_imageblit.h:364 [inline]
BUG: KASAN: vmalloc-out-of-bounds in fb_bitmap_imageblit drivers/video/fbdev/core/fb_imageblit.h:462 [inline]
BUG: KASAN: vmalloc-out-of-bounds in fb_imageblit drivers/video/fbdev/core/fb_imageblit.h:492 [inline]
BUG: KASAN: vmalloc-out-of-bounds in sys_imageblit+0x1a6f/0x1e60 drivers/video/fbdev/core/sysimgblt.c:24
Write of size 8 at addr ffffc90003749fc0 by task syz.0.17/6037
CPU: 0 UID: 0 PID: 6037 Comm: syz.0.17 Not tainted syzkaller #0 PREEMPT(full)
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 10/25/2025
Call Trace:
<TASK>
__dump_stack lib/dump_stack.c:94 [inline]
dump_stack_lvl+0x116/0x1f0 lib/dump_stack.c:120
print_address_description mm/kasan/report.c:378 [inline]
print_report+0xcd/0x630 mm/kasan/report.c:482
kasan_report+0xe0/0x110 mm/kasan/report.c:595
fb_write_offset drivers/video/fbdev/core/sysmem.h:30 [inline]
fb_bitmap_2ppw drivers/video/fbdev/core/fb_imageblit.h:364 [inline]
fb_bitmap_imageblit drivers/video/fbdev/core/fb_imageblit.h:462 [inline]
fb_imageblit drivers/video/fbdev/core/fb_imageblit.h:492 [inline]
sys_imageblit+0x1a6f/0x1e60 drivers/video/fbdev/core/sysimgblt.c:24
drm_fbdev_shmem_defio_imageblit+0x20/0x130 drivers/gpu/drm/drm_fbdev_shmem.c:38
cw_putcs_aligned drivers/video/fbdev/core/fbcon_cw.c:110 [inline]
cw_putcs+0x917/0xbb0 drivers/video/fbdev/core/fbcon_cw.c:158
fbcon_putcs+0x387/0x450 drivers/video/fbdev/core/fbcon.c:1320
do_update_region+0x2e9/0x3f0 drivers/tty/vt/vt.c:628
redraw_screen+0x63f/0x760 drivers/tty/vt/vt.c:980
fbcon_modechanged+0x456/0x6b0 drivers/video/fbdev/core/fbcon.c:2710
fbcon_rotate drivers/video/fbdev/core/fbcon.c:228 [inline]
rotate_store+0x258/0x2f0 drivers/video/fbdev/core/fbcon.c:3215
dev_attr_store+0x58/0x80 drivers/base/core.c:2437
sysfs_kf_write+0xf2/0x150 fs/sysfs/file.c:142
kernfs_fop_write_iter+0x3af/0x570 fs/kernfs/file.c:352
new_sync_write fs/read_write.c:593 [inline]
vfs_write+0x7d3/0x11d0 fs/read_write.c:686
ksys_write+0x12a/0x250 fs/read_write.c:738
do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline]
do_syscall_64+0xcd/0xfa0 arch/x86/entry/syscall_64.c:94
entry_SYSCALL_64_after_hwframe+0x77/0x7f
RIP: 0033:0x7fd04158f6c9
Code: ff ff c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 40 00 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 c7 c1 a8 ff ff ff f7 d8 64 89 01 48
RSP: 002b:00007fff34de0398 EFLAGS: 00000246 ORIG_RAX: 0000000000000001
RAX: ffffffffffffffda RBX: 00007fd0417e5fa0 RCX: 00007fd04158f6c9
RDX: 0000000000000007 RSI: 0000000000000000 RDI: 0000000000000003
RBP: 00007fff34de03f0 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000001
R13: 00007fd0417e5fa0 R14: 00007fd0417e5fa0 R15: 0000000000000003
</TASK>
The buggy address belongs to a vmalloc virtual mapping
Memory state around the buggy address:
ffffc90003749e80: f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8
ffffc90003749f00: f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8
>ffffc90003749f80: f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8
^
ffffc9000374a000: f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8
ffffc9000374a080: f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8
==================================================================
---
If you want syzbot to run the reproducer, reply with:
#syz test: git://repo/address.git branch-or-commit-hash
If you attach or paste a git patch, syzbot will apply it before testing.
^ permalink raw reply
* Re: [PATCH] drm, fbcon, vga_switcheroo: Avoid race condition in fbcon setup
From: Javier Martinez Canillas @ 2025-11-17 15:14 UTC (permalink / raw)
To: Thomas Zimmermann, maarten.lankhorst, mripard, airlied, simona,
deller, lukas, ville.syrjala, sam
Cc: dri-devel, linux-fbdev
In-Reply-To: <9306d41f-6afc-4277-9198-a23e51cbd9f6@suse.de>
Thomas Zimmermann <tzimmermann@suse.de> writes:
> Hi
>
> Am 17.11.25 um 11:32 schrieb Javier Martinez Canillas:
>> Thomas Zimmermann <tzimmermann@suse.de> writes:
>>
>> Hello Thomas,
>>
>>> Protect vga_switcheroo_client_fb_set() with console lock. Avoids OOB
>>> access in fbcon_remap_all(). Without holding the console lock the call
>>> races with switching outputs.
>>>
>>> VGA switcheroo calls fbcon_remap_all() when switching clients. The fbcon
>>> function uses struct fb_info.node, which is set by register_framebuffer().
>>> As the fb-helper code currently sets up VGA switcheroo before registering
>>> the framebuffer, the value of node is -1 and therefore not a legal value.
>>> For example, fbcon uses the value within set_con2fb_map() [1] as an index
>>> into an array.
>>>
>>> Moving vga_switcheroo_client_fb_set() after register_framebuffer() can
>>> result in VGA switching that does not switch fbcon correctly.
>>>
>>> Therefore move vga_switcheroo_client_fb_set() under fbcon_fb_registered(),
>>> which already holds the console lock. Fbdev calls fbcon_fb_registered()
>>> from within register_framebuffer(). Serializes the helper with VGA
>>> switcheroo's call to fbcon_remap_all().
>>>
>>> Although vga_switcheroo_client_fb_set() takes an instance of struct fb_info
>>> as parameter, it really only needs the contained fbcon state. Moving the
>>> call to fbcon initialization is therefore cleaner than before. Only amdgpu,
>>> i915, nouveau and radeon support vga_switcheroo. For all other drivers,
>>> this change does nothing.
>>>
>>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>>> Link: https://elixir.bootlin.com/linux/v6.17/source/drivers/video/fbdev/core/fbcon.c#L2942 # [1]
>>> ---
>> I'm not that familiar with fbcon and vga_switcheroo to properly review
>> your patch but after reading the explanation in the commit message and
>> reading the diff, the change does make sense to me.
>>
>> Acked-by: Javier Martinez Canillas <javierm@redhat.com>
>>
>> But I think that would be good if you get some testing for the drivers
>> that make use of vga_switcheroo. Also, do you need a Fixes tag ?
>
> I've ran the testing on amdgpu and i915 so that nothing breaks. The bug
> is hard to reproduce though. I've discovered it by reading the code.
>
Thanks.
I usually put that kind of information between the --- separator and the
start of the diff. Since that info can be useful for reviewers and doesn't
end in the commited patch, due tools like `git am` omitting that section.
> About Fixes, the problem has been in the code forever. So IDK what Fixes
> would make sense. Just in case:
>
I see. Then I agree that having the tag is less useful.
--
Best regards,
Javier Martinez Canillas
Core Platforms
Red Hat
^ permalink raw reply
* Re: [PATCH] drm, fbcon, vga_switcheroo: Avoid race condition in fbcon setup
From: Thomas Zimmermann @ 2025-11-17 10:59 UTC (permalink / raw)
To: Javier Martinez Canillas, maarten.lankhorst, mripard, airlied,
simona, deller, lukas, ville.syrjala, sam
Cc: dri-devel, linux-fbdev
In-Reply-To: <87fradkkzp.fsf@ocarina.mail-host-address-is-not-set>
Hi
Am 17.11.25 um 11:32 schrieb Javier Martinez Canillas:
> Thomas Zimmermann <tzimmermann@suse.de> writes:
>
> Hello Thomas,
>
>> Protect vga_switcheroo_client_fb_set() with console lock. Avoids OOB
>> access in fbcon_remap_all(). Without holding the console lock the call
>> races with switching outputs.
>>
>> VGA switcheroo calls fbcon_remap_all() when switching clients. The fbcon
>> function uses struct fb_info.node, which is set by register_framebuffer().
>> As the fb-helper code currently sets up VGA switcheroo before registering
>> the framebuffer, the value of node is -1 and therefore not a legal value.
>> For example, fbcon uses the value within set_con2fb_map() [1] as an index
>> into an array.
>>
>> Moving vga_switcheroo_client_fb_set() after register_framebuffer() can
>> result in VGA switching that does not switch fbcon correctly.
>>
>> Therefore move vga_switcheroo_client_fb_set() under fbcon_fb_registered(),
>> which already holds the console lock. Fbdev calls fbcon_fb_registered()
>> from within register_framebuffer(). Serializes the helper with VGA
>> switcheroo's call to fbcon_remap_all().
>>
>> Although vga_switcheroo_client_fb_set() takes an instance of struct fb_info
>> as parameter, it really only needs the contained fbcon state. Moving the
>> call to fbcon initialization is therefore cleaner than before. Only amdgpu,
>> i915, nouveau and radeon support vga_switcheroo. For all other drivers,
>> this change does nothing.
>>
>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>> Link: https://elixir.bootlin.com/linux/v6.17/source/drivers/video/fbdev/core/fbcon.c#L2942 # [1]
>> ---
> I'm not that familiar with fbcon and vga_switcheroo to properly review
> your patch but after reading the explanation in the commit message and
> reading the diff, the change does make sense to me.
>
> Acked-by: Javier Martinez Canillas <javierm@redhat.com>
>
> But I think that would be good if you get some testing for the drivers
> that make use of vga_switcheroo. Also, do you need a Fixes tag ?
I've ran the testing on amdgpu and i915 so that nothing breaks. The bug
is hard to reproduce though. I've discovered it by reading the code.
About Fixes, the problem has been in the code forever. So IDK what Fixes
would make sense. Just in case:
Fixes: 6a9ee8af344e ("vga_switcheroo: initial implementation (v15)")
Cc: Dave Airlie <airlied@redhat.com>
Cc: Jani Nikula <jani.nikula@linux.intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Tvrtko Ursulin <tursulin@ursulin.net>
Cc: Lyude Paul <lyude@redhat.com>
Cc: Danilo Krummrich <dakr@kernel.org>
Cc: Alex Deucher <alexander.deucher@amd.com>
Cc: "Christian König" <christian.koenig@amd.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Maxime Ripard <mripard@kernel.org>
Cc: Thomas Zimmermann <tzimmermann@suse.de>
Cc: Lukas Wunner <lukas@wunner.de>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Helge Deller <deller@gmx.de>
Cc: David Airlie <airlied@gmail.com>
Cc: Simona Vetter <simona@ffwll.ch>
Cc: intel-gfx@lists.freedesktop.org
Cc: dri-devel@lists.freedesktop.org
Cc: nouveau@lists.freedesktop.org
Cc: amd-gfx@lists.freedesktop.org
Cc: linux-fbdev@vger.kernel.org
Cc: <stable@vger.kernel.org> # v2.6.34+
Best regards
Thomas
>
--
--
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
* Re: [PATCH] drm, fbcon, vga_switcheroo: Avoid race condition in fbcon setup
From: Javier Martinez Canillas @ 2025-11-17 10:32 UTC (permalink / raw)
To: Thomas Zimmermann, maarten.lankhorst, mripard, airlied, simona,
deller, lukas, ville.syrjala, sam
Cc: dri-devel, linux-fbdev, Thomas Zimmermann
In-Reply-To: <20251105161549.98836-1-tzimmermann@suse.de>
Thomas Zimmermann <tzimmermann@suse.de> writes:
Hello Thomas,
> Protect vga_switcheroo_client_fb_set() with console lock. Avoids OOB
> access in fbcon_remap_all(). Without holding the console lock the call
> races with switching outputs.
>
> VGA switcheroo calls fbcon_remap_all() when switching clients. The fbcon
> function uses struct fb_info.node, which is set by register_framebuffer().
> As the fb-helper code currently sets up VGA switcheroo before registering
> the framebuffer, the value of node is -1 and therefore not a legal value.
> For example, fbcon uses the value within set_con2fb_map() [1] as an index
> into an array.
>
> Moving vga_switcheroo_client_fb_set() after register_framebuffer() can
> result in VGA switching that does not switch fbcon correctly.
>
> Therefore move vga_switcheroo_client_fb_set() under fbcon_fb_registered(),
> which already holds the console lock. Fbdev calls fbcon_fb_registered()
> from within register_framebuffer(). Serializes the helper with VGA
> switcheroo's call to fbcon_remap_all().
>
> Although vga_switcheroo_client_fb_set() takes an instance of struct fb_info
> as parameter, it really only needs the contained fbcon state. Moving the
> call to fbcon initialization is therefore cleaner than before. Only amdgpu,
> i915, nouveau and radeon support vga_switcheroo. For all other drivers,
> this change does nothing.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> Link: https://elixir.bootlin.com/linux/v6.17/source/drivers/video/fbdev/core/fbcon.c#L2942 # [1]
> ---
I'm not that familiar with fbcon and vga_switcheroo to properly review
your patch but after reading the explanation in the commit message and
reading the diff, the change does make sense to me.
Acked-by: Javier Martinez Canillas <javierm@redhat.com>
But I think that would be good if you get some testing for the drivers
that make use of vga_switcheroo. Also, do you need a Fixes tag ?
--
Best regards,
Javier Martinez Canillas
Core Platforms
Red Hat
^ 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