* Re: [PATCH v1 2/2] backlight: gpio: add support for multiple GPIOs for backlight control
From: Daniel Thompson @ 2026-01-05 10:09 UTC (permalink / raw)
To: Sudarshan Shetty
Cc: lee, danielt, jingoohan1, deller, pavel, robh, krzk+dt, conor+dt,
dri-devel, linux-fbdev, linux-leds, devicetree, linux-kernel
In-Reply-To: <20260105085120.230862-3-tessolveupstream@gmail.com>
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.
> };
>
> 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
* [PATCH 5.10.y] fbcon: Fix the issue of uninitialized charcount in the remaining consoles
From: Luo Gengkun @ 2026-01-05 10:28 UTC (permalink / raw)
To: b.zolnierkie; +Cc: dri-devel, linux-fbdev, linux-kernel
After commit 0998a6cb2326 ("fbdev: bitblit: bound-check glyph index in
bit_putcs*") was merged, using alt+ctrl+f1 to switch the tty from tty0 to
tty1 results in garbled display.
The reason is the vc->vc_font.charcount is 0, it is clearly an
uninitialized value. The mainline is fine because commit a1ac250a82a5
("fbcon: Avoid using FNTCHARCNT() and hard-coded built-in font charcount")
assigns the fvc->vc_font.charcount to vc->vc_font.charcount.
Cc: stable@vger.kernel.org
Signed-off-by: Luo Gengkun <luogengkun@huaweicloud.com>
---
drivers/video/fbdev/core/fbcon.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
index 3dd03e02bf97..900c1ccef98b 100644
--- a/drivers/video/fbdev/core/fbcon.c
+++ b/drivers/video/fbdev/core/fbcon.c
@@ -1070,6 +1070,7 @@ static void fbcon_init(struct vc_data *vc, int init)
fvc->vc_font.data);
vc->vc_font.width = fvc->vc_font.width;
vc->vc_font.height = fvc->vc_font.height;
+ vc->vc_font.charcount = fvc->vc_font.charcount;
p->userfont = t->userfont;
if (p->userfont)
--
2.34.1
^ permalink raw reply related
* Re: [PATCH v1 1/2] dt-bindings: backlight: gpio-backlight: allow multiple GPIOs
From: Daniel Thompson @ 2026-01-05 9:55 UTC (permalink / raw)
To: Sudarshan Shetty
Cc: lee, danielt, jingoohan1, deller, pavel, robh, krzk+dt, conor+dt,
dri-devel, linux-fbdev, linux-leds, devicetree, linux-kernel
In-Reply-To: <20260105085120.230862-2-tessolveupstream@gmail.com>
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?
Daniel.
^ permalink raw reply
* Re: [PATCH v2] staging: sm750fb: fix unused tmp in sw_i2c_wait
From: Greg KH @ 2026-01-05 9:12 UTC (permalink / raw)
To: sun jian; +Cc: linux-staging, sudipm.mukherjee, teddy.wang, linux-fbdev
In-Reply-To: <CABFUUZHRz919=C=fmMqH1sQcURbm+qiQB795xWPCd9Rax_M4ZQ@mail.gmail.com>
On Mon, Jan 05, 2026 at 04:57:48PM +0800, sun jian wrote:
> Hi Greg,
>
> Sorry about that — I mistakenly replied only to you.
>
> As you pointed out, sw_i2c_wait() sits on the bit-banged I2C GPIO
> transitions, so changing the delay semantics without hardware validation
> is risky. I don't have access to the hardware to validate timing/behavior,
> and I can't justify that udelay(1) is equivalent to the existing loop.
>
> Please ignore v2 (and v1). I won't resend a warning-only workaround.
>
> If someone with the hardware can help validate a proper fix (e.g. a
> well-justified time-based delay, or reworking this to use a proper I2C
> bit-banging helper), I'm happy to revisit.
A time-based one is going to be the correct solution as every cpu will
run that "let's count some numbers" loop at a different speed :(
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH v2] staging: sm750fb: fix unused tmp in sw_i2c_wait
From: sun jian @ 2026-01-05 8:57 UTC (permalink / raw)
To: Greg KH; +Cc: linux-staging, sudipm.mukherjee, teddy.wang, linux-fbdev
In-Reply-To: <2026010505-surging-resurface-a7d3@gregkh>
Hi Greg,
Sorry about that — I mistakenly replied only to you.
As you pointed out, sw_i2c_wait() sits on the bit-banged I2C GPIO
transitions, so changing the delay semantics without hardware validation
is risky. I don't have access to the hardware to validate timing/behavior,
and I can't justify that udelay(1) is equivalent to the existing loop.
Please ignore v2 (and v1). I won't resend a warning-only workaround.
If someone with the hardware can help validate a proper fix (e.g. a
well-justified time-based delay, or reworking this to use a proper I2C
bit-banging helper), I'm happy to revisit.
Thanks,
Sun
On Mon, Jan 5, 2026 at 4:05 PM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Mon, Jan 05, 2026 at 03:49:17PM +0800, Sun Jian wrote:
> > clang W=1 warns that 'tmp' is set but not used in sw_i2c_wait().
> >
> > sw_i2c_wait() provides the delay between bit-banged I2C GPIO transitions.
> > Replace the loop-count delay with a time-based udelay(1) to avoid CPU-
> > dependent timing and fix the warning.
>
> Why is udelay(1) the same here?
>
> > Compile-tested with clang W=1; no hardware available to validate timing.
>
> That's going to prevent us from being able to take this, sorry :(
>
>
> >
> > Signed-off-by: Sun Jian <sun.jian.kdev@gmail.com>
> > ---
> > v2:
> > - Replace cpu_relax() delay loop with time-based udelay(1) to avoid
> > CPU-dependent timing (per Greg's feedback).
> >
> > v1:
> > - Used cpu_relax() in the loop to silence -Wunused-but-set-variable.
> > ---
> > drivers/staging/sm750fb/ddk750_swi2c.c | 8 ++------
> > 1 file changed, 2 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/staging/sm750fb/ddk750_swi2c.c b/drivers/staging/sm750fb/ddk750_swi2c.c
> > index 0ef8d4ff2ef9..d5843fa69bfa 100644
> > --- a/drivers/staging/sm750fb/ddk750_swi2c.c
> > +++ b/drivers/staging/sm750fb/ddk750_swi2c.c
> > @@ -11,6 +11,7 @@
> > #include "ddk750_reg.h"
> > #include "ddk750_swi2c.h"
> > #include "ddk750_power.h"
> > +#include <linux/delay.h>
>
> Shouldn't this be at the top of the include lines?
>
> >
> > /*
> > * I2C Software Master Driver:
> > @@ -92,12 +93,7 @@ static void sw_i2c_wait(void)
> > * it's more reliable than counter loop ..
> > * write 0x61 to 0x3ce and read from 0x3cf
> > */
> > - int i, tmp;
> > -
> > - for (i = 0; i < 600; i++) {
> > - tmp = i;
> > - tmp += i;
> > - }
> > + udelay(1);
>
> You are ignoring the comments in this function.
>
> Also, if you reduce this to a single call, shouldn't this whole function
> be removed?
>
> thanks,
>
> greg k-h
^ permalink raw reply
* [PATCH v1 2/2] backlight: gpio: add support for multiple GPIOs for backlight control
From: Sudarshan Shetty @ 2026-01-05 8:51 UTC (permalink / raw)
To: lee, danielt, jingoohan1
Cc: deller, pavel, robh, krzk+dt, conor+dt, dri-devel, linux-fbdev,
linux-leds, devicetree, linux-kernel, Sudarshan Shetty
In-Reply-To: <20260105085120.230862-1-tessolveupstream@gmail.com>
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;
};
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;
+
+ 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) {
+ 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);
--
2.34.1
^ permalink raw reply related
* [PATCH v1 1/2] dt-bindings: backlight: gpio-backlight: allow multiple GPIOs
From: Sudarshan Shetty @ 2026-01-05 8:51 UTC (permalink / raw)
To: lee, danielt, jingoohan1
Cc: deller, pavel, robh, krzk+dt, conor+dt, dri-devel, linux-fbdev,
linux-leds, devicetree, linux-kernel, Sudarshan Shetty
In-Reply-To: <20260105085120.230862-1-tessolveupstream@gmail.com>
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
default-on:
description: enable the backlight at boot.
@@ -38,4 +39,13 @@ examples:
default-on;
};
+ - |
+ #include <dt-bindings/gpio/gpio.h>
+ backlight {
+ compatible = "gpio-backlight";
+ gpios = <&gpio3 4 GPIO_ACTIVE_HIGH>,
+ <&gpio3 5 GPIO_ACTIVE_HIGH>;
+ default-on;
+ };
+
...
--
2.34.1
^ permalink raw reply related
* [PATCH v1 0/2] backlight: gpio-backlight: Add support for multiple GPIOs
From: Sudarshan Shetty @ 2026-01-05 8:51 UTC (permalink / raw)
To: lee, danielt, jingoohan1
Cc: deller, pavel, robh, krzk+dt, conor+dt, dri-devel, linux-fbdev,
linux-leds, devicetree, linux-kernel, Sudarshan Shetty
Hi all,
This patch extends the gpio-backlight driver and its Device Tree
bindings to support multiple GPIOs for controlling a single
backlight device.
Some panels require more than one GPIO to enable or disable the
backlight, and previously the driver only supported a single GPIO.
With this change:
- The driver now handles an array of GPIOs and updates all of them
based on brightness state.
- The Device Tree binding has been updated to allow specifying 1 or 2
GPIOs for a backlight node.
This approach avoids describing multiple backlight devices in DT for a
single panel.
Thanks,
Anusha
Sudarshan Shetty (2):
dt-bindings: backlight: gpio-backlight: allow multiple GPIOs
backlight: gpio: add support for multiple GPIOs for backlight control
.../leds/backlight/gpio-backlight.yaml | 12 +++-
drivers/video/backlight/gpio_backlight.c | 61 ++++++++++++++-----
2 files changed, 56 insertions(+), 17 deletions(-)
--
2.34.1
^ permalink raw reply
* Re: [PATCH v2] staging: sm750fb: fix unused tmp in sw_i2c_wait
From: Greg KH @ 2026-01-05 8:05 UTC (permalink / raw)
To: Sun Jian; +Cc: linux-staging, sudipm.mukherjee, teddy.wang, linux-fbdev
In-Reply-To: <20260105074917.607201-1-sun.jian.kdev@gmail.com>
On Mon, Jan 05, 2026 at 03:49:17PM +0800, Sun Jian wrote:
> clang W=1 warns that 'tmp' is set but not used in sw_i2c_wait().
>
> sw_i2c_wait() provides the delay between bit-banged I2C GPIO transitions.
> Replace the loop-count delay with a time-based udelay(1) to avoid CPU-
> dependent timing and fix the warning.
Why is udelay(1) the same here?
> Compile-tested with clang W=1; no hardware available to validate timing.
That's going to prevent us from being able to take this, sorry :(
>
> Signed-off-by: Sun Jian <sun.jian.kdev@gmail.com>
> ---
> v2:
> - Replace cpu_relax() delay loop with time-based udelay(1) to avoid
> CPU-dependent timing (per Greg's feedback).
>
> v1:
> - Used cpu_relax() in the loop to silence -Wunused-but-set-variable.
> ---
> drivers/staging/sm750fb/ddk750_swi2c.c | 8 ++------
> 1 file changed, 2 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/staging/sm750fb/ddk750_swi2c.c b/drivers/staging/sm750fb/ddk750_swi2c.c
> index 0ef8d4ff2ef9..d5843fa69bfa 100644
> --- a/drivers/staging/sm750fb/ddk750_swi2c.c
> +++ b/drivers/staging/sm750fb/ddk750_swi2c.c
> @@ -11,6 +11,7 @@
> #include "ddk750_reg.h"
> #include "ddk750_swi2c.h"
> #include "ddk750_power.h"
> +#include <linux/delay.h>
Shouldn't this be at the top of the include lines?
>
> /*
> * I2C Software Master Driver:
> @@ -92,12 +93,7 @@ static void sw_i2c_wait(void)
> * it's more reliable than counter loop ..
> * write 0x61 to 0x3ce and read from 0x3cf
> */
> - int i, tmp;
> -
> - for (i = 0; i < 600; i++) {
> - tmp = i;
> - tmp += i;
> - }
> + udelay(1);
You are ignoring the comments in this function.
Also, if you reduce this to a single call, shouldn't this whole function
be removed?
thanks,
greg k-h
^ permalink raw reply
* [PATCH v2] staging: sm750fb: fix unused tmp in sw_i2c_wait
From: Sun Jian @ 2026-01-05 7:49 UTC (permalink / raw)
To: linux-staging; +Cc: sudipm.mukherjee, teddy.wang, gregkh, linux-fbdev, Sun Jian
In-Reply-To: <20260105021026.556244-1-sun.jian.kdev@gmail.com>
clang W=1 warns that 'tmp' is set but not used in sw_i2c_wait().
sw_i2c_wait() provides the delay between bit-banged I2C GPIO transitions.
Replace the loop-count delay with a time-based udelay(1) to avoid CPU-
dependent timing and fix the warning.
Compile-tested with clang W=1; no hardware available to validate timing.
Signed-off-by: Sun Jian <sun.jian.kdev@gmail.com>
---
v2:
- Replace cpu_relax() delay loop with time-based udelay(1) to avoid
CPU-dependent timing (per Greg's feedback).
v1:
- Used cpu_relax() in the loop to silence -Wunused-but-set-variable.
---
drivers/staging/sm750fb/ddk750_swi2c.c | 8 ++------
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/drivers/staging/sm750fb/ddk750_swi2c.c b/drivers/staging/sm750fb/ddk750_swi2c.c
index 0ef8d4ff2ef9..d5843fa69bfa 100644
--- a/drivers/staging/sm750fb/ddk750_swi2c.c
+++ b/drivers/staging/sm750fb/ddk750_swi2c.c
@@ -11,6 +11,7 @@
#include "ddk750_reg.h"
#include "ddk750_swi2c.h"
#include "ddk750_power.h"
+#include <linux/delay.h>
/*
* I2C Software Master Driver:
@@ -92,12 +93,7 @@ static void sw_i2c_wait(void)
* it's more reliable than counter loop ..
* write 0x61 to 0x3ce and read from 0x3cf
*/
- int i, tmp;
-
- for (i = 0; i < 600; i++) {
- tmp = i;
- tmp += i;
- }
+ udelay(1);
}
/*
--
2.43.0
^ permalink raw reply related
* Re: [PATCH] staging: sm750fb: fix unused tmp in sw_i2c_wait
From: Greg KH @ 2026-01-05 6:28 UTC (permalink / raw)
To: Sun Jian; +Cc: linux-staging, sudipm.mukherjee, teddy.wang, linux-fbdev
In-Reply-To: <20260105021026.556244-1-sun.jian.kdev@gmail.com>
On Mon, Jan 05, 2026 at 10:10:26AM +0800, Sun Jian wrote:
> clang W=1 warns that 'tmp' is set but not used in sw_i2c_wait().
>
> Replace the dummy loop with cpu_relax() to keep the delay loop without
> triggering -Wunused-but-set-variable.
>
> No functional change intended.
>
> Signed-off-by: Sun Jian <sun.jian.kdev@gmail.com>
> ---
> drivers/staging/sm750fb/ddk750_swi2c.c | 9 ++++-----
> 1 file changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/staging/sm750fb/ddk750_swi2c.c b/drivers/staging/sm750fb/ddk750_swi2c.c
> index 0ef8d4ff2ef9..279a1a10f132 100644
> --- a/drivers/staging/sm750fb/ddk750_swi2c.c
> +++ b/drivers/staging/sm750fb/ddk750_swi2c.c
> @@ -92,12 +92,11 @@ static void sw_i2c_wait(void)
> * it's more reliable than counter loop ..
> * write 0x61 to 0x3ce and read from 0x3cf
> */
> - int i, tmp;
> + int i;
> +
> + for (i = 0; i < 600; i++)
> + cpu_relax();
>
> - for (i = 0; i < 600; i++) {
> - tmp = i;
> - tmp += i;
> - }
You just slowed down this loop a lot, are you sure it still works? And
really, either way, this isn't the proper way to "sleep", as it will
be different speeds on different cpus/systems, and not work properly at
all. Can you fix this correctly?
thanks,
greg k-h
^ permalink raw reply
* [PATCH] staging: sm750fb: fix unused tmp in sw_i2c_wait
From: Sun Jian @ 2026-01-05 2:10 UTC (permalink / raw)
To: linux-staging; +Cc: sudipm.mukherjee, teddy.wang, gregkh, linux-fbdev, Sun Jian
clang W=1 warns that 'tmp' is set but not used in sw_i2c_wait().
Replace the dummy loop with cpu_relax() to keep the delay loop without
triggering -Wunused-but-set-variable.
No functional change intended.
Signed-off-by: Sun Jian <sun.jian.kdev@gmail.com>
---
drivers/staging/sm750fb/ddk750_swi2c.c | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/drivers/staging/sm750fb/ddk750_swi2c.c b/drivers/staging/sm750fb/ddk750_swi2c.c
index 0ef8d4ff2ef9..279a1a10f132 100644
--- a/drivers/staging/sm750fb/ddk750_swi2c.c
+++ b/drivers/staging/sm750fb/ddk750_swi2c.c
@@ -92,12 +92,11 @@ static void sw_i2c_wait(void)
* it's more reliable than counter loop ..
* write 0x61 to 0x3ce and read from 0x3cf
*/
- int i, tmp;
+ int i;
+
+ for (i = 0; i < 600; i++)
+ cpu_relax();
- for (i = 0; i < 600; i++) {
- tmp = i;
- tmp += i;
- }
}
/*
--
2.43.0
^ permalink raw reply related
* [PATCH v1 4/4] staging: fbtft: hx8353d: send LUT via buffer to reduce stack usage
From: Sun Jian @ 2026-01-04 11:06 UTC (permalink / raw)
To: Andy Shevchenko, Greg Kroah-Hartman
Cc: linux-staging, linux-fbdev, dri-devel, Sun Jian
In-Reply-To: <20260104110638.532615-1-sun.jian.kdev@gmail.com>
Clang reports a large stack frame in init_display()
(-Wframe-larger-than=1024) due to the very large
write_reg(MIPI_DCS_WRITE_LUT, ...) call.
Send MIPI_DCS_WRITE_LUT followed by the LUT payload using
fbtft_write_buf_dc() to avoid the varargs/NUMARGS stack blow-up.
No functional change intended.
Signed-off-by: Sun Jian <sun.jian.kdev@gmail.com>
---
drivers/staging/fbtft/fb_hx8353d.c | 38 ++++++++++++++++++++++--------
1 file changed, 28 insertions(+), 10 deletions(-)
diff --git a/drivers/staging/fbtft/fb_hx8353d.c b/drivers/staging/fbtft/fb_hx8353d.c
index 3e73b69b6a27..7e667d60792f 100644
--- a/drivers/staging/fbtft/fb_hx8353d.c
+++ b/drivers/staging/fbtft/fb_hx8353d.c
@@ -17,6 +17,21 @@
#define DRVNAME "fb_hx8353d"
#define DEFAULT_GAMMA "50 77 40 08 BF 00 03 0F 00 01 73 00 72 03 B0 0F 08 00 0F"
+static const u8 lut[] = {
+ 0, 2, 4, 6, 8, 10, 12, 14, 16, 18, 20, 22, 24, 26, 28, 30,
+ 32, 34, 36, 38, 40, 42, 44, 46, 48, 50, 52, 54, 56, 58, 60, 62,
+ 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15,
+ 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31,
+ 32, 33, 34, 35, 36, 37, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47,
+ 48, 49, 50, 51, 52, 53, 54, 55, 56, 57, 58, 59, 60, 61, 62, 63,
+ 0, 2, 4, 6, 8, 10, 12, 14, 16, 18, 20, 22, 24, 26, 28, 30,
+ 32, 34, 36, 38, 40, 42, 44, 46, 48, 50, 52, 54, 56, 58, 60, 62,
+ 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15,
+ 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31,
+ 32, 33, 34, 35, 36, 37, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47,
+ 48, 49, 50, 51, 52, 53, 54, 55, 56, 57, 58, 59, 60, 61, 62, 63,
+ };
+
static int init_display(struct fbtft_par *par)
{
par->fbtftops.reset(par);
@@ -48,18 +63,21 @@ static int init_display(struct fbtft_par *par)
write_reg(par, MIPI_DCS_SET_DISPLAY_ON);
/* RGBSET */
- write_reg(par, MIPI_DCS_WRITE_LUT,
- 0, 2, 4, 6, 8, 10, 12, 14, 16, 18, 20, 22, 24, 26, 28, 30,
- 32, 34, 36, 38, 40, 42, 44, 46, 48, 50, 52, 54, 56, 58, 60, 62,
- 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15,
- 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31,
- 32, 33, 34, 35, 36, 37, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47,
- 48, 49, 50, 51, 52, 53, 54, 55, 56, 57, 58, 59, 60, 61, 62, 63,
- 0, 2, 4, 6, 8, 10, 12, 14, 16, 18, 20, 22, 24, 26, 28, 30,
- 32, 34, 36, 38, 40, 42, 44, 46, 48, 50, 52, 54, 56, 58, 60, 62);
+ {
+ u8 cmd = MIPI_DCS_WRITE_LUT;
+ int ret;
+
+ ret = fbtft_write_buf_dc(par, &cmd, 1, 0);
+ if (ret < 0)
+ return ret;
+
+ ret = fbtft_write_buf_dc(par, (void *)lut, sizeof(lut), 1);
+ if (ret < 0)
+ return ret;
+ }
return 0;
-};
+}
static void set_addr_win(struct fbtft_par *par, int xs, int ys, int xe, int ye)
{
--
2.43.0
^ permalink raw reply related
* [PATCH v1 3/4] staging: fbtft: ssd1331: send gamma table via fbtft_write_buf_dc()
From: Sun Jian @ 2026-01-04 11:06 UTC (permalink / raw)
To: Andy Shevchenko, Greg Kroah-Hartman
Cc: linux-staging, linux-fbdev, dri-devel, Sun Jian
In-Reply-To: <20260104110638.532615-1-sun.jian.kdev@gmail.com>
Clang reports a large stack frame in set_gamma() (-Wframe-larger-than=1024)
because write_reg() expands into a large varargs call and triggers
NUMARGS((int[]){...}) stack allocation.
Use fbtft_write_buf_dc() to send the command byte (0xB8) and the gamma
payload as a buffer.
No functional change intended.
Signed-off-by: Sun Jian <sun.jian.kdev@gmail.com>
---
drivers/staging/fbtft/fb_ssd1331.c | 29 +++++++++++++++--------------
1 file changed, 15 insertions(+), 14 deletions(-)
diff --git a/drivers/staging/fbtft/fb_ssd1331.c b/drivers/staging/fbtft/fb_ssd1331.c
index 06b7056d6c71..cbe10f191f5b 100644
--- a/drivers/staging/fbtft/fb_ssd1331.c
+++ b/drivers/staging/fbtft/fb_ssd1331.c
@@ -130,37 +130,38 @@ static void write_reg8_bus8(struct fbtft_par *par, int len, ...)
*/
static int set_gamma(struct fbtft_par *par, u32 *curves)
{
- unsigned long tmp[GAMMA_NUM * GAMMA_LEN];
+ u8 data[GAMMA_LEN];
+ u8 cmd = 0xB8;
int i, acc = 0;
+ int ret;
- for (i = 0; i < 63; i++) {
+ for (i = 0; i < GAMMA_LEN; i++) {
if (i > 0 && curves[i] < 2) {
dev_err(par->info->device,
"Illegal value in Grayscale Lookup Table at index %d. Must be greater than 1\n",
i);
return -EINVAL;
}
+
acc += curves[i];
- tmp[i] = acc;
+
if (acc > 180) {
dev_err(par->info->device,
"Illegal value(s) in Grayscale Lookup Table. At index=%d, the accumulated value has exceeded 180\n",
i);
return -EINVAL;
}
+
+ data[i] = acc;
}
- write_reg(par, 0xB8,
- tmp[0], tmp[1], tmp[2], tmp[3], tmp[4], tmp[5], tmp[6],
- tmp[7], tmp[8], tmp[9], tmp[10], tmp[11], tmp[12], tmp[13],
- tmp[14], tmp[15], tmp[16], tmp[17], tmp[18], tmp[19], tmp[20],
- tmp[21], tmp[22], tmp[23], tmp[24], tmp[25], tmp[26], tmp[27],
- tmp[28], tmp[29], tmp[30], tmp[31], tmp[32], tmp[33], tmp[34],
- tmp[35], tmp[36], tmp[37], tmp[38], tmp[39], tmp[40], tmp[41],
- tmp[42], tmp[43], tmp[44], tmp[45], tmp[46], tmp[47], tmp[48],
- tmp[49], tmp[50], tmp[51], tmp[52], tmp[53], tmp[54], tmp[55],
- tmp[56], tmp[57], tmp[58], tmp[59], tmp[60], tmp[61],
- tmp[62]);
+ ret = fbtft_write_buf_dc(par, &cmd, 1, 0);
+ if (ret < 0)
+ return ret;
+
+ ret = fbtft_write_buf_dc(par, data, sizeof(data), 1);
+ if (ret < 0)
+ return ret;
return 0;
}
--
2.43.0
^ permalink raw reply related
* [PATCH v1 2/4] staging: fbtft: ssd1351: send gamma table via fbtft_write_buf_dc()
From: Sun Jian @ 2026-01-04 11:06 UTC (permalink / raw)
To: Andy Shevchenko, Greg Kroah-Hartman
Cc: linux-staging, linux-fbdev, dri-devel, Sun Jian
In-Reply-To: <20260104110638.532615-1-sun.jian.kdev@gmail.com>
Clang reports a large stack frame in set_gamma() (-Wframe-larger-than=1024)
due to the large write_reg() call emitting 63 gamma bytes via varargs.
Send the command byte (0xB8) and the gamma payload using
fbtft_write_buf_dc() to avoid the varargs/NUMARGS stack blow-up.
No functional change intended.
Signed-off-by: Sun Jian <sun.jian.kdev@gmail.com>
---
drivers/staging/fbtft/fb_ssd1351.c | 35 +++++++++++++-----------------
1 file changed, 15 insertions(+), 20 deletions(-)
diff --git a/drivers/staging/fbtft/fb_ssd1351.c b/drivers/staging/fbtft/fb_ssd1351.c
index 6736b09b2f45..b4ab2c81e528 100644
--- a/drivers/staging/fbtft/fb_ssd1351.c
+++ b/drivers/staging/fbtft/fb_ssd1351.c
@@ -119,43 +119,38 @@ static int set_var(struct fbtft_par *par)
*/
static int set_gamma(struct fbtft_par *par, u32 *curves)
{
- unsigned long tmp[GAMMA_NUM * GAMMA_LEN];
+ u8 data[GAMMA_LEN];
+ u8 cmd = 0xB8;
int i, acc = 0;
+ int ret;
- for (i = 0; i < 63; i++) {
+ for (i = 0; i < GAMMA_LEN; i++) {
if (i > 0 && curves[i] < 2) {
dev_err(par->info->device,
"Illegal value in Grayscale Lookup Table at index %d : %d. Must be greater than 1\n",
i, curves[i]);
return -EINVAL;
}
+
acc += curves[i];
- tmp[i] = acc;
+
if (acc > 180) {
dev_err(par->info->device,
"Illegal value(s) in Grayscale Lookup Table. At index=%d : %d, the accumulated value has exceeded 180\n",
i, acc);
return -EINVAL;
}
+
+ data[i] = acc;
}
- write_reg(par, 0xB8,
- tmp[0], tmp[1], tmp[2], tmp[3],
- tmp[4], tmp[5], tmp[6], tmp[7],
- tmp[8], tmp[9], tmp[10], tmp[11],
- tmp[12], tmp[13], tmp[14], tmp[15],
- tmp[16], tmp[17], tmp[18], tmp[19],
- tmp[20], tmp[21], tmp[22], tmp[23],
- tmp[24], tmp[25], tmp[26], tmp[27],
- tmp[28], tmp[29], tmp[30], tmp[31],
- tmp[32], tmp[33], tmp[34], tmp[35],
- tmp[36], tmp[37], tmp[38], tmp[39],
- tmp[40], tmp[41], tmp[42], tmp[43],
- tmp[44], tmp[45], tmp[46], tmp[47],
- tmp[48], tmp[49], tmp[50], tmp[51],
- tmp[52], tmp[53], tmp[54], tmp[55],
- tmp[56], tmp[57], tmp[58], tmp[59],
- tmp[60], tmp[61], tmp[62]);
+ ret = fbtft_write_buf_dc(par, &cmd, 1, 0);
+ if (ret < 0)
+ return ret;
+
+ ret = fbtft_write_buf_dc(par, data, sizeof(data), 1);
+ if (ret < 0)
+ return ret;
return 0;
}
--
2.43.0
^ permalink raw reply related
* [PATCH v1 1/4] staging: fbtft: core: avoid large stack usage in DT init parsing
From: Sun Jian @ 2026-01-04 11:06 UTC (permalink / raw)
To: Andy Shevchenko, Greg Kroah-Hartman
Cc: linux-staging, linux-fbdev, dri-devel, Sun Jian
In-Reply-To: <20260104110638.532615-1-sun.jian.kdev@gmail.com>
Clang reports a large stack frame for fbtft_init_display_from_property()
(-Wframe-larger-than=1024) when the init sequence is emitted through a
fixed 64-argument write_register() call.
write_reg()/write_register() relies on NUMARGS((int[]){...}) and large
varargs which inflates stack usage. Switch the DT "init" path to send the
command byte and the payload via fbtft_write_buf_dc() instead.
No functional change intended: the same register values are sent in the
same order, only the transport is changed.
Signed-off-by: Sun Jian <sun.jian.kdev@gmail.com>
---
drivers/staging/fbtft/fbtft-core.c | 32 ++++++++++++------------------
1 file changed, 13 insertions(+), 19 deletions(-)
diff --git a/drivers/staging/fbtft/fbtft-core.c b/drivers/staging/fbtft/fbtft-core.c
index 8a5ccc8ae0a1..127d0de87e03 100644
--- a/drivers/staging/fbtft/fbtft-core.c
+++ b/drivers/staging/fbtft/fbtft-core.c
@@ -846,7 +846,8 @@ EXPORT_SYMBOL(fbtft_unregister_framebuffer);
static int fbtft_init_display_from_property(struct fbtft_par *par)
{
struct device *dev = par->info->device;
- int buf[64], count, index, i, j, ret;
+ u8 buf[64];
+ int count, index, i, j, ret;
u32 *values;
u32 val;
@@ -881,7 +882,7 @@ static int fbtft_init_display_from_property(struct fbtft_par *par)
ret = -EINVAL;
goto out_free;
}
- buf[i++] = val;
+ buf[i++] = val & 0xFF;
val = values[++index];
}
/* make debug message */
@@ -891,23 +892,16 @@ static int fbtft_init_display_from_property(struct fbtft_par *par)
fbtft_par_dbg(DEBUG_INIT_DISPLAY, par,
"buf[%d] = %02X\n", j, buf[j]);
- par->fbtftops.write_register(par, i,
- buf[0], buf[1], buf[2], buf[3],
- buf[4], buf[5], buf[6], buf[7],
- buf[8], buf[9], buf[10], buf[11],
- buf[12], buf[13], buf[14], buf[15],
- buf[16], buf[17], buf[18], buf[19],
- buf[20], buf[21], buf[22], buf[23],
- buf[24], buf[25], buf[26], buf[27],
- buf[28], buf[29], buf[30], buf[31],
- buf[32], buf[33], buf[34], buf[35],
- buf[36], buf[37], buf[38], buf[39],
- buf[40], buf[41], buf[42], buf[43],
- buf[44], buf[45], buf[46], buf[47],
- buf[48], buf[49], buf[50], buf[51],
- buf[52], buf[53], buf[54], buf[55],
- buf[56], buf[57], buf[58], buf[59],
- buf[60], buf[61], buf[62], buf[63]);
+ /* buf[0] is command, buf[1..i-1] is data */
+ ret = fbtft_write_buf_dc(par, &buf[0], 1, 0);
+ if (ret < 0)
+ goto out_free;
+
+ if (i > 1) {
+ ret = fbtft_write_buf_dc(par, &buf[1], i - 1, 1);
+ if (ret < 0)
+ goto out_free;
+ }
} else if (val & FBTFT_OF_INIT_DELAY) {
fbtft_par_dbg(DEBUG_INIT_DISPLAY, par,
"init: msleep(%u)\n", val & 0xFFFF);
--
2.43.0
^ permalink raw reply related
* [PATCH v1 0/4] staging: fbtft: reduce stack usage by avoiding large write_reg() varargs
From: Sun Jian @ 2026-01-04 11:06 UTC (permalink / raw)
To: Andy Shevchenko, Greg Kroah-Hartman
Cc: linux-staging, linux-fbdev, dri-devel, Sun Jian
Hi,
This series fixes clang `-Wframe-larger-than=1024` warnings in the fbtft
staging drivers.
The warnings are triggered by very large `write_reg()`/`write_register()`
varargs calls, which result in excessive stack usage.
Switch the affected paths to send a u8 command byte followed by the u8
payload using `fbtft_write_buf_dc()`. The register values and ordering are
kept unchanged; only the transfer method is updated.
Patches:
1/4 staging: fbtft: core: avoid large stack usage in DT init parsing
2/4 staging: fbtft: ssd1351: send gamma table via fbtft_write_buf_dc()
3/4 staging: fbtft: ssd1331: send gamma table via fbtft_write_buf_dc()
4/4 staging: fbtft: hx8353d: send LUT via buffer to reduce stack usage
Thanks,
Sun Jian
--
Sun Jian (4):
staging: fbtft: core: avoid large stack usage in DT init parsing
staging: fbtft: ssd1351: send gamma table via fbtft_write_buf_dc()
staging: fbtft: ssd1331: send gamma table via fbtft_write_buf_dc()
staging: fbtft: hx8353d: send LUT via buffer to reduce stack usage
drivers/staging/fbtft/fb_hx8353d.c | 38 ++++++++++++++++++++++--------
drivers/staging/fbtft/fb_ssd1331.c | 29 ++++++++++++-----------
drivers/staging/fbtft/fb_ssd1351.c | 35 ++++++++++++---------------
drivers/staging/fbtft/fbtft-core.c | 32 ++++++++++---------------
4 files changed, 71 insertions(+), 63 deletions(-)
--
2.43.0
^ permalink raw reply
* Re: [PATCH v3 4/4] fbdev: sh_mobile_lcdc: Make FB_DEVICE dependency optional
From: Andy Shevchenko @ 2026-01-03 13:15 UTC (permalink / raw)
To: Helge Deller
Cc: Chintan Patel, Helge Deller, andy, linux-fbdev, linux-omap,
linux-kernel, dri-devel
In-Reply-To: <73007a0a-dd96-43eb-be2e-ccbf8b19cd79@gmx.de>
On Sat, Jan 03, 2026 at 10:59:44AM +0100, Helge Deller wrote:
> On 12/30/25 19:25, Chintan Patel wrote:
> > On 12/30/25 00:13, Helge Deller wrote:
...
> > > > -ATTRIBUTE_GROUPS(overlay_sysfs);
> > >
> > > Instead of replacing the ^ ATTRIBUTE_GROUPS() by the code below,
> > > isn't it possible to just mark the overlay_sysfs_attrs[] array
> > > _maybe_unused, and just do:
> > > + #ifdef CONFIG_FB_DEVICE
> > > + ATTRIBUTE_GROUPS(overlay_sysfs);
> > > + #endif
> > >
> > > ?
> >
> > Yes, the __maybe_unused + #ifdef ATTRIBUTE_GROUPS() approach would work.
> >
> > I went with the PTR_IF(IS_ENABLED()) pattern because Andy suggested
> > using PTR_IF() to conditionally include overlay_sysfs_group in
> > overlay_sysfs_groups, and to keep .dev_groups always populated while
> > letting the device core skip NULL groups. This avoids conditional
> > wiring via #ifdef and keeps the code type-checked without
> > CONFIG_FB_DEVICE.
> > If you still prefer the simpler #ifdef ATTRIBUTE_GROUPS() approach
> > for this driver, I can switch to that, but I wanted to follow Andy’s
> > guidance here.
>
> I assume Andy will agree to my suggested approach, as it's cleaner
> and avoids code bloat/duplication. Maybe you send out a v4 with my
> suggested approach, then it's easier to judge... ?
I'm also fine with original code. But a suggested approach would work as well
(at least like it sounds from the above description). Ideally would be nice to
get rid of ifdeffery completely (that's why we have PTR_IF() for), although
it might be not so readable. TL;DR: the most readable solution is the winner.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* [PATCH] staging: sm750fb: fix typo 'resetted' -> 'reset'
From: Akiyoshi Kurita @ 2026-01-03 12:08 UTC (permalink / raw)
To: sudipm.mukherjee
Cc: teddy.wang, gregkh, linux-fbdev, linux-staging, linux-kernel,
Akiyoshi Kurita
Fix a typo in a comment.
Signed-off-by: Akiyoshi Kurita <weibu@redadmin.org>
---
drivers/staging/sm750fb/ddk750_chip.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/sm750fb/ddk750_chip.c b/drivers/staging/sm750fb/ddk750_chip.c
index 025dae3756aa..136692b00804 100644
--- a/drivers/staging/sm750fb/ddk750_chip.c
+++ b/drivers/staging/sm750fb/ddk750_chip.c
@@ -249,7 +249,7 @@ int ddk750_init_hw(struct initchip_param *p_init_param)
* Reset the memory controller.
* If the memory controller is not reset in SM750,
* the system might hang when sw accesses the memory.
- * The memory should be resetted after changing the MXCLK.
+ * The memory should be reset after changing the MXCLK.
*/
if (p_init_param->reset_memory == 1) {
reg = peek32(MISC_CTRL);
--
2.47.3
^ permalink raw reply related
* Re: [PATCH v3 4/4] fbdev: sh_mobile_lcdc: Make FB_DEVICE dependency optional
From: Helge Deller @ 2026-01-03 9:59 UTC (permalink / raw)
To: Chintan Patel, Helge Deller, andy
Cc: linux-fbdev, linux-omap, linux-kernel, dri-devel
In-Reply-To: <aa2b08f8-e6c3-4768-bce9-e36ad9d1e74b@gmail.com>
On 12/30/25 19:25, Chintan Patel wrote:
>
>
> On 12/30/25 00:13, Helge Deller wrote:
>> * Chintan Patel <chintanlike@gmail.com>:
>>> The sh_mobile_lcdc driver exposes overlay configuration via sysfs, but the
>>> core driver does not require CONFIG_FB_DEVICE.
>>>
>>> Make sysfs support optional by defining overlay_sysfs_groups conditionally
>>> using PTR_IF(). The driver always sets .dev_groups, and the kernel
>>> naturally skips NULL attribute groups while the code remains buildable
>>> and type-checked.
>>>
>>> Suggested-by: Helge Deller <deller@gmx.de>
>>> Signed-off-by: Chintan Patel <chintanlike@gmail.com>
>>> ---
>>> drivers/video/fbdev/sh_mobile_lcdcfb.c | 12 +++++++++++-
>>> 1 file changed, 11 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/video/fbdev/sh_mobile_lcdcfb.c b/drivers/video/fbdev/sh_mobile_lcdcfb.c
>>> index dd950e4ab5ce..cb7ed1ff9165 100644
>>> --- a/drivers/video/fbdev/sh_mobile_lcdcfb.c
>>> +++ b/drivers/video/fbdev/sh_mobile_lcdcfb.c
>>> @@ -1350,7 +1350,17 @@ static struct attribute *overlay_sysfs_attrs[] = {
>>> &dev_attr_overlay_rop3.attr,
>>> NULL,
>>> };
>>> -ATTRIBUTE_GROUPS(overlay_sysfs);
>>
>> Instead of replacing the ^ ATTRIBUTE_GROUPS() by the code below,
>> isn't it possible to just mark the overlay_sysfs_attrs[] array
>> _maybe_unused, and just do:
>> + #ifdef CONFIG_FB_DEVICE
>> + ATTRIBUTE_GROUPS(overlay_sysfs);
>> + #endif
>>
>> ?
> Hi Helge,
>
> Yes, the __maybe_unused + #ifdef ATTRIBUTE_GROUPS() approach would work.
>
> I went with the PTR_IF(IS_ENABLED()) pattern because Andy suggested
> using PTR_IF() to conditionally include overlay_sysfs_group in
> overlay_sysfs_groups, and to keep .dev_groups always populated while
> letting the device core skip NULL groups. This avoids conditional
> wiring via #ifdef and keeps the code type-checked without
> CONFIG_FB_DEVICE.
> If you still prefer the simpler #ifdef ATTRIBUTE_GROUPS() approach
> for this driver, I can switch to that, but I wanted to follow Andy’s
> guidance here.
I assume Andy will agree to my suggested approach, as it's cleaner
and avoids code bloat/duplication. Maybe you send out a v4 with my
suggested approach, then it's easier to judge... ?
Helge
^ permalink raw reply
* Re: [PATCH 4/6] newport_con: depend on LOGO_LINUX_CLUT224 instead of LOGO_SGI_CLUT224
From: Vincent Mailhol @ 2026-01-02 21:54 UTC (permalink / raw)
To: Helge Deller
Cc: linux-fbdev, dri-devel, linux-kernel, linux-sh,
Greg Kroah-Hartman, Yoshinori Sato, Rich Felker,
John Paul Adrian Glaubitz
In-Reply-To: <20251230-custom-logo-v1-4-4736374569ee@kernel.org>
On 30/12/2025 at 23:20, Vincent Mailhol wrote:
> newport_show_logo() is only activated if CONFIG_LOGO_LINUX_CLUT224 is
^^^^^^^^^^^^^^^^^^^^^^^^^
This should have been CONFIG_LOGO_SGI_CLUT224. But that's only a typo in
the patch description.
@Helger, do you want me to send a v3 or can you directly apply the typo
fix in your linux-fbdev/for-next branch?
> set (otherwise it is a NOP). This configuration value will be removed
> in an upcoming change so instead, make it depend on LOGO_LINUX_CLUT224.
>
> Signed-off-by: Vincent Mailhol <mailhol@kernel.org>
Yours sincerely,
Vincent Mailhol
^ permalink raw reply
* Re: [PATCH 1/6] video/logo: remove orphan .pgm Makefile rule
From: David Heidelberg @ 2026-01-02 20:26 UTC (permalink / raw)
To: Helge Deller, Vincent Mailhol
Cc: linux-fbdev, dri-devel, linux-kernel, linux-sh,
Greg Kroah-Hartman, Rich Felker, John Paul Adrian Glaubitz
In-Reply-To: <90526885-9597-40eb-903b-7f741f87a7fc@gmx.de>
On 02/01/2026 20:59, Helge Deller wrote:
> On 1/2/26 18:25, Vincent Mailhol wrote:
>> On 02/01/2026 at 18:18, David Heidelberg wrote:
>>> On 02/01/2026 18:13, Vincent Mailhol wrote:
>>>> On 02/01/2026 at 17:34, David Heidelberg wrote:
>>>>> On 30/12/2025 23:20, Vincent Mailhol wrote:
>>>>>> The kernel has no actual grey-scale logos. And looking at the git
>>>>>> history, it seems that there never was one (or maybe there was in the
>>>>>> pre-git history? I did not check that far…)
>>>>>>
>>>>>> Remove the Makefile rule for the .pgm grey scale images.
>>>>>
>>>>> Great to see this series.
>>>>
>>>> Thanks!
>>>>
>>>>> I think the Fixes: tag should still go here, even if it is not very
>>>>> specific.
>>>>
>>>> But then, what do I put in the fixes tag? This:
>>>>
>>>> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
>>>>
>>>> ?
>>>
>>> Yes
>>>
>>>>
>>>> I am not sure it is worth bothering the stable team for something that
>>>> isn't causing any real harm.
>>>
>>> That was my original thinking as well, but the Fixes tag is not only
>>> about stable backports. It is also used for tracking, tooling, and
>>> documentation, so stable picking up such patches is just one of its
>>> purposes.
>>
>> OK. Then why not. I added the tag in my local tree, but I will wait a
>> couple days for the other review comments before sending. I will not
>> spam everyone with a v3 just for that.
>
> I like your patch!
> So, I've added the v2 series to the fbdev git tree.
>
> I don't think a "Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")" tag is
> appropriate, since it doesn't fixes anything.
If there were ever grayscale logos, that would fix the pre-Git commit
removal, which left unused code behind.
It seems each maintainer has a different view on tag usage.
I’ll try to familiarize myself more with subsystem practices before
attempting another review. I need to get used to this diversity.
David
^ permalink raw reply
* Re: [PATCH 1/6] video/logo: remove orphan .pgm Makefile rule
From: Vincent Mailhol @ 2026-01-02 20:20 UTC (permalink / raw)
To: Helge Deller, David Heidelberg
Cc: linux-fbdev, dri-devel, linux-kernel, linux-sh,
Greg Kroah-Hartman, Rich Felker, John Paul Adrian Glaubitz
In-Reply-To: <90526885-9597-40eb-903b-7f741f87a7fc@gmx.de>
On 02/01/2026 at 20:59, Helge Deller wrote:
> On 1/2/26 18:25, Vincent Mailhol wrote:
>> On 02/01/2026 at 18:18, David Heidelberg wrote:
>>> On 02/01/2026 18:13, Vincent Mailhol wrote:
>>>> On 02/01/2026 at 17:34, David Heidelberg wrote:
>>>>> On 30/12/2025 23:20, Vincent Mailhol wrote:
>>>>>> The kernel has no actual grey-scale logos. And looking at the git
>>>>>> history, it seems that there never was one (or maybe there was in the
>>>>>> pre-git history? I did not check that far…)
>>>>>>
>>>>>> Remove the Makefile rule for the .pgm grey scale images.
>>>>>
>>>>> Great to see this series.
>>>>
>>>> Thanks!
>>>>
>>>>> I think the Fixes: tag should still go here, even if it is not very
>>>>> specific.
>>>>
>>>> But then, what do I put in the fixes tag? This:
>>>>
>>>> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
>>>>
>>>> ?
>>>
>>> Yes
>>>
>>>>
>>>> I am not sure it is worth bothering the stable team for something that
>>>> isn't causing any real harm.
>>>
>>> That was my original thinking as well, but the Fixes tag is not only
>>> about stable backports. It is also used for tracking, tooling, and
>>> documentation, so stable picking up such patches is just one of its
>>> purposes.
>>
>> OK. Then why not. I added the tag in my local tree, but I will wait a
>> couple days for the other review comments before sending. I will not
>> spam everyone with a v3 just for that.
>
> I like your patch!
> So, I've added the v2 series to the fbdev git tree.
Thanks!
> I don't think a "Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")" tag is
> appropriate, since it doesn't fixes anything.
Ack.
> Let's see if issues arise due to the wider testing...
Yes. It is always hard for this kind of changes to take care of all the
intricacies (like the one for the Playstation 3 extra logo…)
Well, I spend a fair amount of effort, grepping the different logos
throughout the source code and didn't find any other corner cases.
Fingers crossed!
Yours sincerely,
Vincent Mailhol
^ permalink raw reply
* Re: [PATCH 1/6] video/logo: remove orphan .pgm Makefile rule
From: Helge Deller @ 2026-01-02 19:59 UTC (permalink / raw)
To: Vincent Mailhol, David Heidelberg
Cc: linux-fbdev, dri-devel, linux-kernel, linux-sh,
Greg Kroah-Hartman, Rich Felker, John Paul Adrian Glaubitz
In-Reply-To: <d20c6baa-bd1f-4de7-afa7-0abb99e3bcb5@kernel.org>
On 1/2/26 18:25, Vincent Mailhol wrote:
> On 02/01/2026 at 18:18, David Heidelberg wrote:
>> On 02/01/2026 18:13, Vincent Mailhol wrote:
>>> On 02/01/2026 at 17:34, David Heidelberg wrote:
>>>> On 30/12/2025 23:20, Vincent Mailhol wrote:
>>>>> The kernel has no actual grey-scale logos. And looking at the git
>>>>> history, it seems that there never was one (or maybe there was in the
>>>>> pre-git history? I did not check that far…)
>>>>>
>>>>> Remove the Makefile rule for the .pgm grey scale images.
>>>>
>>>> Great to see this series.
>>>
>>> Thanks!
>>>
>>>> I think the Fixes: tag should still go here, even if it is not very
>>>> specific.
>>>
>>> But then, what do I put in the fixes tag? This:
>>>
>>> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
>>>
>>> ?
>>
>> Yes
>>
>>>
>>> I am not sure it is worth bothering the stable team for something that
>>> isn't causing any real harm.
>>
>> That was my original thinking as well, but the Fixes tag is not only
>> about stable backports. It is also used for tracking, tooling, and
>> documentation, so stable picking up such patches is just one of its
>> purposes.
>
> OK. Then why not. I added the tag in my local tree, but I will wait a
> couple days for the other review comments before sending. I will not
> spam everyone with a v3 just for that.
I like your patch!
So, I've added the v2 series to the fbdev git tree.
I don't think a "Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")" tag is
appropriate, since it doesn't fixes anything.
Let's see if issues arise due to the wider testing...
Helge
^ permalink raw reply
* Re: [PATCH 0/6] video/logo: allow custom boot logo and simplify logic
From: Vincent Mailhol @ 2026-01-02 19:55 UTC (permalink / raw)
To: Markus Reichelt
Cc: Helge Deller, Greg Kroah-Hartman, Yoshinori Sato, Rich Felker,
John Paul Adrian Glaubitz, linux-fbdev, dri-devel, linux-kernel,
linux-sh
In-Reply-To: <71446537-a2a0-4051-8e4d-20256bf17824@kernel.org>
On 02/01/2026 at 18:18, Vincent Mailhol wrote:
> On 02/01/2026 at 17:30, Markus Reichelt wrote:
(...)
>> Haven't tested your patch series cos stuff just works for me.
>> Looking forward to feedback from all those logo nerds out there.
>
> Looking forward for your Tested-by tag!
D'oh, I misread your message. When my brain parsed your text, I read it as:
Haven't tested your patch series cos work stuff…
As if you were too busy with your work to test it now.
Yours sincerely,
Vincent Mailhol
^ 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