* [PATCH v1 04/13] ASoC: uda1380: use devm_kmemdup_array()
From: Raag Jadav @ 2025-02-21 16:53 UTC (permalink / raw)
To: perex, tiwai, broonie, lgirdwood, deller, andriy.shevchenko, sre,
sakari.ailus, mchehab, hverkuil-cisco, jdmason, fancer.lancer
Cc: linux-sound, linux-fbdev, linux-pm, linux-media, ntb,
linux-kernel, Raag Jadav
In-Reply-To: <20250221165333.2780888-1-raag.jadav@intel.com>
Convert to use devm_kmemdup_array() and while at it, make the size robust
against type changes.
Signed-off-by: Raag Jadav <raag.jadav@intel.com>
---
sound/soc/codecs/uda1380.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/sound/soc/codecs/uda1380.c b/sound/soc/codecs/uda1380.c
index 4f8fdd574585..c179d865b938 100644
--- a/sound/soc/codecs/uda1380.c
+++ b/sound/soc/codecs/uda1380.c
@@ -766,10 +766,8 @@ static int uda1380_i2c_probe(struct i2c_client *i2c)
return ret;
}
- uda1380->reg_cache = devm_kmemdup(&i2c->dev,
- uda1380_reg,
- ARRAY_SIZE(uda1380_reg) * sizeof(u16),
- GFP_KERNEL);
+ uda1380->reg_cache = devm_kmemdup_array(&i2c->dev, uda1380_reg, ARRAY_SIZE(uda1380_reg),
+ sizeof(uda1380_reg[0]), GFP_KERNEL);
if (!uda1380->reg_cache)
return -ENOMEM;
--
2.34.1
^ permalink raw reply related
* [PATCH v1 03/13] ASoC: tlv320dac33: use devm_kmemdup_array()
From: Raag Jadav @ 2025-02-21 16:53 UTC (permalink / raw)
To: perex, tiwai, broonie, lgirdwood, deller, andriy.shevchenko, sre,
sakari.ailus, mchehab, hverkuil-cisco, jdmason, fancer.lancer
Cc: linux-sound, linux-fbdev, linux-pm, linux-media, ntb,
linux-kernel, Raag Jadav
In-Reply-To: <20250221165333.2780888-1-raag.jadav@intel.com>
Convert to use devm_kmemdup_array() and while at it, make the size robust
against type changes.
Signed-off-by: Raag Jadav <raag.jadav@intel.com>
---
sound/soc/codecs/tlv320dac33.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/sound/soc/codecs/tlv320dac33.c b/sound/soc/codecs/tlv320dac33.c
index fa46f51d4341..423b9264a205 100644
--- a/sound/soc/codecs/tlv320dac33.c
+++ b/sound/soc/codecs/tlv320dac33.c
@@ -1477,10 +1477,8 @@ static int dac33_i2c_probe(struct i2c_client *client)
if (dac33 == NULL)
return -ENOMEM;
- dac33->reg_cache = devm_kmemdup(&client->dev,
- dac33_reg,
- ARRAY_SIZE(dac33_reg) * sizeof(u8),
- GFP_KERNEL);
+ dac33->reg_cache = devm_kmemdup_array(&client->dev, dac33_reg, ARRAY_SIZE(dac33_reg),
+ sizeof(dac33_reg[0]), GFP_KERNEL);
if (!dac33->reg_cache)
return -ENOMEM;
--
2.34.1
^ permalink raw reply related
* [PATCH v1 02/13] ASoC: hdac_hdmi: use devm_kmemdup_array()
From: Raag Jadav @ 2025-02-21 16:53 UTC (permalink / raw)
To: perex, tiwai, broonie, lgirdwood, deller, andriy.shevchenko, sre,
sakari.ailus, mchehab, hverkuil-cisco, jdmason, fancer.lancer
Cc: linux-sound, linux-fbdev, linux-pm, linux-media, ntb,
linux-kernel, Raag Jadav
In-Reply-To: <20250221165333.2780888-1-raag.jadav@intel.com>
Convert to use devm_kmemdup_array() and while at it, make the size robust
against type changes.
Signed-off-by: Raag Jadav <raag.jadav@intel.com>
---
sound/soc/codecs/hdac_hdmi.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/sound/soc/codecs/hdac_hdmi.c b/sound/soc/codecs/hdac_hdmi.c
index e1a7f0b0c0f3..3bea5d09219a 100644
--- a/sound/soc/codecs/hdac_hdmi.c
+++ b/sound/soc/codecs/hdac_hdmi.c
@@ -1017,8 +1017,7 @@ static int hdac_hdmi_create_pin_port_muxs(struct hdac_device *hdev,
return -ENOMEM;
}
- se->texts = devm_kmemdup(&hdev->dev, items,
- (num_items * sizeof(char *)), GFP_KERNEL);
+ se->texts = devm_kmemdup_array(&hdev->dev, items, num_items, sizeof(items[0]), GFP_KERNEL);
if (!se->texts)
return -ENOMEM;
--
2.34.1
^ permalink raw reply related
* [PATCH v1 01/13] ASoC: Intel: avs: use devm_kmemdup_array()
From: Raag Jadav @ 2025-02-21 16:53 UTC (permalink / raw)
To: perex, tiwai, broonie, lgirdwood, deller, andriy.shevchenko, sre,
sakari.ailus, mchehab, hverkuil-cisco, jdmason, fancer.lancer
Cc: linux-sound, linux-fbdev, linux-pm, linux-media, ntb,
linux-kernel, Raag Jadav, Amadeusz Sławiński,
Linus Walleij
In-Reply-To: <20250221165333.2780888-1-raag.jadav@intel.com>
Convert to use devm_kmemdup_array() and while at it, use source size
instead of destination.
Signed-off-by: Raag Jadav <raag.jadav@intel.com>
Acked-by: Mark Brown <broonie@kernel.org>
Reviewed-by: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
---
sound/soc/intel/avs/boards/da7219.c | 3 ++-
sound/soc/intel/avs/boards/es8336.c | 3 ++-
sound/soc/intel/avs/boards/nau8825.c | 3 ++-
sound/soc/intel/avs/boards/rt274.c | 3 ++-
sound/soc/intel/avs/boards/rt286.c | 3 ++-
sound/soc/intel/avs/boards/rt298.c | 3 ++-
sound/soc/intel/avs/boards/rt5663.c | 3 ++-
sound/soc/intel/avs/boards/rt5682.c | 3 ++-
8 files changed, 16 insertions(+), 8 deletions(-)
diff --git a/sound/soc/intel/avs/boards/da7219.c b/sound/soc/intel/avs/boards/da7219.c
index 76078a7005b0..9507a96f26ac 100644
--- a/sound/soc/intel/avs/boards/da7219.c
+++ b/sound/soc/intel/avs/boards/da7219.c
@@ -113,7 +113,8 @@ static int avs_da7219_codec_init(struct snd_soc_pcm_runtime *runtime)
}
num_pins = ARRAY_SIZE(card_headset_pins);
- pins = devm_kmemdup(card->dev, card_headset_pins, sizeof(*pins) * num_pins, GFP_KERNEL);
+ pins = devm_kmemdup_array(card->dev, card_headset_pins, num_pins,
+ sizeof(card_headset_pins[0]), GFP_KERNEL);
if (!pins)
return -ENOMEM;
diff --git a/sound/soc/intel/avs/boards/es8336.c b/sound/soc/intel/avs/boards/es8336.c
index 426ce37105ae..6f3c4f6c9302 100644
--- a/sound/soc/intel/avs/boards/es8336.c
+++ b/sound/soc/intel/avs/boards/es8336.c
@@ -109,7 +109,8 @@ static int avs_es8336_codec_init(struct snd_soc_pcm_runtime *runtime)
data = snd_soc_card_get_drvdata(card);
num_pins = ARRAY_SIZE(card_headset_pins);
- pins = devm_kmemdup(card->dev, card_headset_pins, sizeof(*pins) * num_pins, GFP_KERNEL);
+ pins = devm_kmemdup_array(card->dev, card_headset_pins, num_pins,
+ sizeof(card_headset_pins[0]), GFP_KERNEL);
if (!pins)
return -ENOMEM;
diff --git a/sound/soc/intel/avs/boards/nau8825.c b/sound/soc/intel/avs/boards/nau8825.c
index bf902540744c..6833eebd82d6 100644
--- a/sound/soc/intel/avs/boards/nau8825.c
+++ b/sound/soc/intel/avs/boards/nau8825.c
@@ -88,7 +88,8 @@ static int avs_nau8825_codec_init(struct snd_soc_pcm_runtime *runtime)
jack = snd_soc_card_get_drvdata(card);
num_pins = ARRAY_SIZE(card_headset_pins);
- pins = devm_kmemdup(card->dev, card_headset_pins, sizeof(*pins) * num_pins, GFP_KERNEL);
+ pins = devm_kmemdup_array(card->dev, card_headset_pins, num_pins,
+ sizeof(card_headset_pins[0]), GFP_KERNEL);
if (!pins)
return -ENOMEM;
diff --git a/sound/soc/intel/avs/boards/rt274.c b/sound/soc/intel/avs/boards/rt274.c
index 4b6c02a40204..f5caafc21861 100644
--- a/sound/soc/intel/avs/boards/rt274.c
+++ b/sound/soc/intel/avs/boards/rt274.c
@@ -98,7 +98,8 @@ static int avs_rt274_codec_init(struct snd_soc_pcm_runtime *runtime)
jack = snd_soc_card_get_drvdata(card);
num_pins = ARRAY_SIZE(card_headset_pins);
- pins = devm_kmemdup(card->dev, card_headset_pins, sizeof(*pins) * num_pins, GFP_KERNEL);
+ pins = devm_kmemdup_array(card->dev, card_headset_pins, num_pins,
+ sizeof(card_headset_pins[0]), GFP_KERNEL);
if (!pins)
return -ENOMEM;
diff --git a/sound/soc/intel/avs/boards/rt286.c b/sound/soc/intel/avs/boards/rt286.c
index e40563ca99fd..1eb0399c0fae 100644
--- a/sound/soc/intel/avs/boards/rt286.c
+++ b/sound/soc/intel/avs/boards/rt286.c
@@ -59,7 +59,8 @@ static int avs_rt286_codec_init(struct snd_soc_pcm_runtime *runtime)
jack = snd_soc_card_get_drvdata(card);
num_pins = ARRAY_SIZE(card_headset_pins);
- pins = devm_kmemdup(card->dev, card_headset_pins, sizeof(*pins) * num_pins, GFP_KERNEL);
+ pins = devm_kmemdup_array(card->dev, card_headset_pins, num_pins,
+ sizeof(card_headset_pins[0]), GFP_KERNEL);
if (!pins)
return -ENOMEM;
diff --git a/sound/soc/intel/avs/boards/rt298.c b/sound/soc/intel/avs/boards/rt298.c
index 94fce07c83f9..85269a3be981 100644
--- a/sound/soc/intel/avs/boards/rt298.c
+++ b/sound/soc/intel/avs/boards/rt298.c
@@ -70,7 +70,8 @@ static int avs_rt298_codec_init(struct snd_soc_pcm_runtime *runtime)
jack = snd_soc_card_get_drvdata(card);
num_pins = ARRAY_SIZE(card_headset_pins);
- pins = devm_kmemdup(card->dev, card_headset_pins, sizeof(*pins) * num_pins, GFP_KERNEL);
+ pins = devm_kmemdup_array(card->dev, card_headset_pins, num_pins,
+ sizeof(card_headset_pins[0]), GFP_KERNEL);
if (!pins)
return -ENOMEM;
diff --git a/sound/soc/intel/avs/boards/rt5663.c b/sound/soc/intel/avs/boards/rt5663.c
index b456b9d14665..e3310b3268ba 100644
--- a/sound/soc/intel/avs/boards/rt5663.c
+++ b/sound/soc/intel/avs/boards/rt5663.c
@@ -65,7 +65,8 @@ static int avs_rt5663_codec_init(struct snd_soc_pcm_runtime *runtime)
jack = &priv->jack;
num_pins = ARRAY_SIZE(card_headset_pins);
- pins = devm_kmemdup(card->dev, card_headset_pins, sizeof(*pins) * num_pins, GFP_KERNEL);
+ pins = devm_kmemdup_array(card->dev, card_headset_pins, num_pins,
+ sizeof(card_headset_pins[0]), GFP_KERNEL);
if (!pins)
return -ENOMEM;
diff --git a/sound/soc/intel/avs/boards/rt5682.c b/sound/soc/intel/avs/boards/rt5682.c
index 335960cfd7ba..339df0b944c1 100644
--- a/sound/soc/intel/avs/boards/rt5682.c
+++ b/sound/soc/intel/avs/boards/rt5682.c
@@ -102,7 +102,8 @@ static int avs_rt5682_codec_init(struct snd_soc_pcm_runtime *runtime)
jack = snd_soc_card_get_drvdata(card);
num_pins = ARRAY_SIZE(card_jack_pins);
- pins = devm_kmemdup(card->dev, card_jack_pins, sizeof(*pins) * num_pins, GFP_KERNEL);
+ pins = devm_kmemdup_array(card->dev, card_jack_pins, num_pins,
+ sizeof(card_jack_pins[0]), GFP_KERNEL);
if (!pins)
return -ENOMEM;
--
2.34.1
^ permalink raw reply related
* [PATCH v1 00/13] Convert to use devm_kmemdup_array()
From: Raag Jadav @ 2025-02-21 16:53 UTC (permalink / raw)
To: perex, tiwai, broonie, lgirdwood, deller, andriy.shevchenko, sre,
sakari.ailus, mchehab, hverkuil-cisco, jdmason, fancer.lancer
Cc: linux-sound, linux-fbdev, linux-pm, linux-media, ntb,
linux-kernel, Raag Jadav
This series is the second wave of patches to add users of newly introduced
devm_kmemdup_array() helper. Original series on [1].
This depends on changes available on immutable tag[2]. Feel free to pick
your subsystem patches with it, or share your preferred way to route them.
[1] https://lore.kernel.org/r/20250212062513.2254767-1-raag.jadav@intel.com
[2] https://lore.kernel.org/r/Z7cqCaME4LxTTBn6@black.fi.intel.com
Raag Jadav (13):
ASoC: Intel: avs: use devm_kmemdup_array()
ASoC: hdac_hdmi: use devm_kmemdup_array()
ASoC: tlv320dac33: use devm_kmemdup_array()
ASoC: uda1380: use devm_kmemdup_array()
ASoC: meson: axg-tdm-interface: use devm_kmemdup_array()
ASoC: uniphier: use devm_kmemdup_array()
fbdev: pxafb: use devm_kmemdup_array()
power: supply: sc27xx: use devm_kmemdup_array()
regulator: devres: use devm_kmemdup_array()
regulator: cros-ec: use devm_kmemdup_array()
media: atmel-isi: use devm_kmemdup_array()
media: stm32-dcmi: use devm_kmemdup_array()
ntb: idt: use devm_kmemdup_array()
drivers/media/platform/atmel/atmel-isi.c | 8 ++------
drivers/media/platform/st/stm32/stm32-dcmi.c | 8 ++------
drivers/ntb/hw/idt/ntb_hw_idt.c | 11 +++-------
drivers/power/supply/sc27xx_fuel_gauge.c | 5 ++---
drivers/regulator/cros-ec-regulator.c | 4 ++--
drivers/regulator/devres.c | 5 ++---
drivers/video/fbdev/pxafb.c | 21 ++++++++------------
sound/soc/codecs/hdac_hdmi.c | 3 +--
sound/soc/codecs/tlv320dac33.c | 6 ++----
sound/soc/codecs/uda1380.c | 6 ++----
sound/soc/intel/avs/boards/da7219.c | 3 ++-
sound/soc/intel/avs/boards/es8336.c | 3 ++-
sound/soc/intel/avs/boards/nau8825.c | 3 ++-
sound/soc/intel/avs/boards/rt274.c | 3 ++-
sound/soc/intel/avs/boards/rt286.c | 3 ++-
sound/soc/intel/avs/boards/rt298.c | 3 ++-
sound/soc/intel/avs/boards/rt5663.c | 3 ++-
sound/soc/intel/avs/boards/rt5682.c | 3 ++-
sound/soc/meson/axg-tdm-interface.c | 9 ++-------
sound/soc/uniphier/aio-cpu.c | 8 ++------
20 files changed, 46 insertions(+), 72 deletions(-)
base-commit: b16e9f8547a328b19af59afc213ce323124d11e9
--
2.34.1
^ permalink raw reply
* Re: [PATCH v6 0/3] Apple DWI backlight driver
From: Lee Jones @ 2025-02-20 15:14 UTC (permalink / raw)
To: Janne Grunau, Sven Peter, Alyssa Rosenzweig, Lee Jones,
Daniel Thompson, Jingoo Han, Pavel Machek, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Helge Deller, asahi,
linux-arm-kernel, dri-devel, linux-leds, devicetree, linux-kernel,
linux-fbdev, Nick Chan
In-Reply-To: <20250214040306.16312-1-towinchenmi@gmail.com>
On Fri, 14 Feb 2025 12:02:11 +0800, Nick Chan wrote:
> Apple SoCs come with a 2-wire interface named DWI. On some iPhones, iPads
> and iPod touches the backlight controller is connected via this interface.
> This series adds a backlight driver for backlight controllers connected
> this way.
>
> Changes since v5:
> - Remove default y from drivers/video/backlight/Kconfig
>
> [...]
Applied, thanks!
[1/3] dt-bindings: leds: backlight: apple,dwi-bl: Add Apple DWI backlight
commit: 0508d17506fffb6d38df4c2dc737fb4f343a0840
[2/3] backlight: apple_dwi_bl: Add Apple DWI backlight driver
commit: ea45d216dd4e5b389af984f8c9effa1312e3cd74
[3/3] MAINTAINERS: Add entries for Apple DWI backlight controller
commit: d1ebaf003a065d5d337b8fa3d69f9b90d7bb759d
--
Lee Jones [李琼斯]
^ permalink raw reply
* Re: [PATCH 12/13] leds: backlight trigger: Replace fb events with a dedicated function call
From: Lee Jones @ 2025-02-20 15:00 UTC (permalink / raw)
To: Thomas Zimmermann
Cc: pavel, danielt, jingoohan1, deller, simona, linux-leds, dri-devel,
linux-fbdev
In-Reply-To: <f4652dbd-7544-4a6d-98d0-f4b64d60a435@suse.de>
On Thu, 13 Feb 2025, Thomas Zimmermann wrote:
> Hi
>
> Am 11.02.25 um 14:57 schrieb Lee Jones:
> > On Thu, 06 Feb 2025, Thomas Zimmermann wrote:
> >
> > > Remove support for fb events from the led backlight trigger. Provide the
> > > helper ledtrig_backlight_blank() instead. Call it from fbdev to inform
> > > the trigger of changes to a display's blank state.
> > >
> > > Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> > > ---
> > > drivers/leds/trigger/ledtrig-backlight.c | 31 +++++-------------------
> > > drivers/video/fbdev/core/fbmem.c | 21 +++++++++-------
> > > include/linux/leds.h | 6 +++++
> > > 3 files changed, 24 insertions(+), 34 deletions(-)
> > >
> > > diff --git a/drivers/leds/trigger/ledtrig-backlight.c b/drivers/leds/trigger/ledtrig-backlight.c
> > > index f9317f93483b..e3ae4adc29e3 100644
> > > --- a/drivers/leds/trigger/ledtrig-backlight.c
> > > +++ b/drivers/leds/trigger/ledtrig-backlight.c
> > > @@ -10,7 +10,6 @@
> > > #include <linux/kernel.h>
> > > #include <linux/slab.h>
> > > #include <linux/init.h>
> > > -#include <linux/fb.h>
> > > #include <linux/leds.h>
> > > #include "../leds.h"
> > > @@ -21,7 +20,6 @@ struct bl_trig_notifier {
> > > struct led_classdev *led;
> > > int brightness;
> > > int old_status;
> > > - struct notifier_block notifier;
> > > unsigned invert;
> > > struct list_head entry;
> > > @@ -30,7 +28,7 @@ struct bl_trig_notifier {
> > > static struct list_head ledtrig_backlight_list;
> > > static struct mutex ledtrig_backlight_list_mutex;
> > > -static void ledtrig_backlight_blank(struct bl_trig_notifier *n, bool on)
> > > +static void __ledtrig_backlight_blank(struct bl_trig_notifier *n, bool on)
> > I'm confused. Didn't you just introduce this?
>
> It's being renamed here; probably something to avoid.
>
>
> >
> > > {
> > > struct led_classdev *led = n->led;
> > > int new_status = !on ? BLANK : UNBLANK;
> > > @@ -48,23 +46,14 @@ static void ledtrig_backlight_blank(struct bl_trig_notifier *n, bool on)
> > > n->old_status = new_status;
> > > }
> > > -static int fb_notifier_callback(struct notifier_block *p,
> > > - unsigned long event, void *data)
> > > +void ledtrig_backlight_blank(bool on)
> > > {
> > > - struct bl_trig_notifier *n = container_of(p,
> > > - struct bl_trig_notifier, notifier);
> > > - struct fb_event *fb_event = data;
> > > - int *blank;
> > > -
> > > - /* If we aren't interested in this event, skip it immediately ... */
> > > - if (event != FB_EVENT_BLANK)
> > > - return 0;
> > > -
> > > - blank = fb_event->data;
> > > + struct bl_trig_notifier *n;
> > > - ledtrig_backlight_blank(n, !blank[0]);
> > > + guard(mutex)(&ledtrig_backlight_list_mutex);
> > > - return 0;
> > > + list_for_each_entry(n, &ledtrig_backlight_list, entry)
> > > + __ledtrig_backlight_blank(n, on);
> > > }
> > > static ssize_t bl_trig_invert_show(struct device *dev,
> > > @@ -110,8 +99,6 @@ ATTRIBUTE_GROUPS(bl_trig);
> > > static int bl_trig_activate(struct led_classdev *led)
> > > {
> > > - int ret;
> > > -
> > > struct bl_trig_notifier *n;
> > > n = kzalloc(sizeof(struct bl_trig_notifier), GFP_KERNEL);
> > > @@ -122,11 +109,6 @@ static int bl_trig_activate(struct led_classdev *led)
> > > n->led = led;
> > > n->brightness = led->brightness;
> > > n->old_status = UNBLANK;
> > > - n->notifier.notifier_call = fb_notifier_callback;
> > > -
> > > - ret = fb_register_client(&n->notifier);
> > > - if (ret)
> > > - dev_err(led->dev, "unable to register backlight trigger\n");
> > > mutex_lock(&ledtrig_backlight_list_mutex);
> > > list_add(&n->entry, &ledtrig_backlight_list);
> > > @@ -143,7 +125,6 @@ static void bl_trig_deactivate(struct led_classdev *led)
> > > list_del(&n->entry);
> > > mutex_unlock(&ledtrig_backlight_list_mutex);
> > > - fb_unregister_client(&n->notifier);
> > > kfree(n);
> > > }
> > > diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c
> > > index fb7ca143a996..92c3fe2a7aff 100644
> > > --- a/drivers/video/fbdev/core/fbmem.c
> > > +++ b/drivers/video/fbdev/core/fbmem.c
> > > @@ -16,6 +16,7 @@
> > > #include <linux/fb.h>
> > > #include <linux/fbcon.h>
> > > #include <linux/lcd.h>
> > > +#include <linux/leds.h>
> > > #include <video/nomodeset.h>
> > > @@ -373,11 +374,19 @@ static void fb_lcd_notify_blank(struct fb_info *info)
> > > #endif
> > > }
> > > +static void fb_ledtrig_backlight_notify_blank(struct fb_info *info)
> > > +{
> > > +#if IS_REACHABLE(CONFIG_LEDS_TRIGGER_BACKLIGHT)
> > #iferry is generally discouraged in C files.
> >
> > Does is_defined() work for you?
>
> I don't think so. This ifdef covers the case that fbdev is built in, but led
> is a module (i.e., fbdev=y && led=m).
>
> > /
> > > + if (info->blank == FB_BLANK_UNBLANK)
> > > + ledtrig_backlight_blank(true);
> > If !CONFIG_LEDS_TRIGGER_BACKLIGHT(), then ledtrig_backlight_blank() is a
> > noop anyway, right? So why encompass it in the #if at all?
>
> Because of (fbdev=y && led=m) again. ledtrig_backlight_blank() would be
> defined then, but not linkable from fbdev. Preferably, I'd rather leave out
> the ifdef in the led header file.
#ifdefs are not generally welcome in C-files. Please rework it.
--
Lee Jones [李琼斯]
^ permalink raw reply
* [PATCH] fbdev: Register sysfs groups through device_add_group
From: oushixiong1025 @ 2025-02-20 9:59 UTC (permalink / raw)
To: Simona Vetter
Cc: Helge Deller, linux-fbdev, dri-devel, linux-kernel, oushixiong
From: oushixiong <oushixiong@kylinos.cn>
Use device_add_group() to simplify creation.
Signed-off-by: Shixiong Ou <oushixiong@kylinos.cn>
---
drivers/video/fbdev/core/fbsysfs.c | 69 +++++++++++++++++-------------
1 file changed, 39 insertions(+), 30 deletions(-)
diff --git a/drivers/video/fbdev/core/fbsysfs.c b/drivers/video/fbdev/core/fbsysfs.c
index 1b3c9958ef5c..06d75c767579 100644
--- a/drivers/video/fbdev/core/fbsysfs.c
+++ b/drivers/video/fbdev/core/fbsysfs.c
@@ -416,55 +416,64 @@ static ssize_t show_bl_curve(struct device *device,
/* When cmap is added back in it should be a binary attribute
* not a text one. Consideration should also be given to converting
* fbdev to use configfs instead of sysfs */
-static struct device_attribute device_attrs[] = {
- __ATTR(bits_per_pixel, S_IRUGO|S_IWUSR, show_bpp, store_bpp),
- __ATTR(blank, S_IRUGO|S_IWUSR, show_blank, store_blank),
- __ATTR(console, S_IRUGO|S_IWUSR, show_console, store_console),
- __ATTR(cursor, S_IRUGO|S_IWUSR, show_cursor, store_cursor),
- __ATTR(mode, S_IRUGO|S_IWUSR, show_mode, store_mode),
- __ATTR(modes, S_IRUGO|S_IWUSR, show_modes, store_modes),
- __ATTR(pan, S_IRUGO|S_IWUSR, show_pan, store_pan),
- __ATTR(virtual_size, S_IRUGO|S_IWUSR, show_virtual, store_virtual),
- __ATTR(name, S_IRUGO, show_name, NULL),
- __ATTR(stride, S_IRUGO, show_stride, NULL),
- __ATTR(rotate, S_IRUGO|S_IWUSR, show_rotate, store_rotate),
- __ATTR(state, S_IRUGO|S_IWUSR, show_fbstate, store_fbstate),
+static DEVICE_ATTR(bits_per_pixel, 0644, show_bpp, store_bpp);
+static DEVICE_ATTR(blank, 0644, show_blank, store_blank);
+static DEVICE_ATTR(console, 0644, show_console, store_console);
+static DEVICE_ATTR(cursor, 0644, show_cursor, store_cursor);
+static DEVICE_ATTR(mode, 0644, show_mode, store_mode);
+static DEVICE_ATTR(modes, 0644, show_modes, store_modes);
+static DEVICE_ATTR(pan, 0644, show_pan, store_pan);
+static DEVICE_ATTR(virtual_size, 0644, show_virtual, store_virtual);
+static DEVICE_ATTR(name, 0444, show_name, NULL);
+static DEVICE_ATTR(stride, 0444, show_stride, NULL);
+static DEVICE_ATTR(rotate, 0644, show_rotate, store_rotate);
+static DEVICE_ATTR(state, 0644, show_fbstate, store_fbstate);
#if IS_ENABLED(CONFIG_FB_BACKLIGHT)
- __ATTR(bl_curve, S_IRUGO|S_IWUSR, show_bl_curve, store_bl_curve),
+static DEVICE_ATTR(bl_curve, 0644, show_bl_curve, store_bl_curve);
#endif
+
+static struct attribute *fb_device_attrs[] = {
+ &dev_attr_bits_per_pixel.attr,
+ &dev_attr_blank.attr,
+ &dev_attr_console.attr,
+ &dev_attr_cursor.attr,
+ &dev_attr_mode.attr,
+ &dev_attr_modes.attr,
+ &dev_attr_pan.attr,
+ &dev_attr_virtual_size.attr,
+ &dev_attr_name.attr,
+ &dev_attr_stride.attr,
+ &dev_attr_rotate.attr,
+ &dev_attr_state.attr,
+#if IS_ENABLED(CONFIG_FB_BACKLIGHT)
+ &dev_attr_bl_curve.attr,
+#endif
+ NULL,
+};
+
+static const struct attribute_group fb_device_attr_group = {
+ .attrs = fb_device_attrs,
};
static int fb_init_device(struct fb_info *fb_info)
{
- int i, error = 0;
+ int ret;
dev_set_drvdata(fb_info->dev, fb_info);
fb_info->class_flag |= FB_SYSFS_FLAG_ATTR;
- for (i = 0; i < ARRAY_SIZE(device_attrs); i++) {
- error = device_create_file(fb_info->dev, &device_attrs[i]);
-
- if (error)
- break;
- }
-
- if (error) {
- while (--i >= 0)
- device_remove_file(fb_info->dev, &device_attrs[i]);
+ ret = device_add_group(fb_info->dev, &fb_device_attr_group);
+ if (ret)
fb_info->class_flag &= ~FB_SYSFS_FLAG_ATTR;
- }
return 0;
}
static void fb_cleanup_device(struct fb_info *fb_info)
{
- unsigned int i;
-
if (fb_info->class_flag & FB_SYSFS_FLAG_ATTR) {
- for (i = 0; i < ARRAY_SIZE(device_attrs); i++)
- device_remove_file(fb_info->dev, &device_attrs[i]);
+ device_remove_group(fb_info->dev, &fb_device_attr_group);
fb_info->class_flag &= ~FB_SYSFS_FLAG_ATTR;
}
--
2.17.1
^ permalink raw reply related
* Re: [PATCH v2 2/2] mfd: lm3533: convert to use OF
From: kernel test robot @ 2025-02-19 23:51 UTC (permalink / raw)
To: Svyatoslav Ryhel, Lee Jones, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Jonathan Cameron, Lars-Peter Clausen, Pavel Machek,
Daniel Thompson, Jingoo Han, Helge Deller, Andy Shevchenko,
Uwe Kleine-König
Cc: llvm, oe-kbuild-all, devicetree, linux-kernel, linux-iio,
linux-leds, dri-devel, linux-fbdev
In-Reply-To: <20250218132702.114669-3-clamor95@gmail.com>
Hi Svyatoslav,
kernel test robot noticed the following build errors:
[auto build test ERROR on lee-mfd/for-mfd-next]
[also build test ERROR on lee-leds/for-leds-next robh/for-next linus/master v6.14-rc3 next-20250219]
[cannot apply to lee-mfd/for-mfd-fixes]
[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/Svyatoslav-Ryhel/dt-bindings-mfd-Document-TI-LM3533-MFD/20250218-212857
base: https://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd.git for-mfd-next
patch link: https://lore.kernel.org/r/20250218132702.114669-3-clamor95%40gmail.com
patch subject: [PATCH v2 2/2] mfd: lm3533: convert to use OF
config: arm-randconfig-002-20250219 (https://download.01.org/0day-ci/archive/20250220/202502200718.H8t6Uv7b-lkp@intel.com/config)
compiler: clang version 17.0.6 (https://github.com/llvm/llvm-project 6009708b4367171ccdbf4b5905cb6a803753fe18)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250220/202502200718.H8t6Uv7b-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/202502200718.H8t6Uv7b-lkp@intel.com/
All errors (new ones prefixed by >>):
>> drivers/video/backlight/lm3533_bl.c:417:25: error: use of undeclared identifier 'lm3533_match_table'; did you mean 'lm3533_bl_match_table'?
417 | MODULE_DEVICE_TABLE(of, lm3533_match_table);
| ^~~~~~~~~~~~~~~~~~
| lm3533_bl_match_table
include/linux/module.h:250:15: note: expanded from macro 'MODULE_DEVICE_TABLE'
250 | extern typeof(name) __mod_device_table__##type##__##name \
| ^
drivers/video/backlight/lm3533_bl.c:413:34: note: 'lm3533_bl_match_table' declared here
413 | static const struct of_device_id lm3533_bl_match_table[] = {
| ^
1 error generated.
vim +417 drivers/video/backlight/lm3533_bl.c
412
413 static const struct of_device_id lm3533_bl_match_table[] = {
414 { .compatible = "ti,lm3533-backlight" },
415 { },
416 };
> 417 MODULE_DEVICE_TABLE(of, lm3533_match_table);
418
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* Re: [PATCH v2 2/2] mfd: lm3533: convert to use OF
From: Andy Shevchenko @ 2025-02-19 20:20 UTC (permalink / raw)
To: kernel test robot
Cc: Svyatoslav Ryhel, Lee Jones, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Jonathan Cameron, Lars-Peter Clausen, Pavel Machek,
Daniel Thompson, Jingoo Han, Helge Deller, Uwe Kleine-König,
oe-kbuild-all, devicetree, linux-kernel, linux-iio, linux-leds,
dri-devel, linux-fbdev
In-Reply-To: <202502192343.twEQ3SSs-lkp@intel.com>
On Thu, Feb 20, 2025 at 12:02:51AM +0800, kernel test robot wrote:
> Hi Svyatoslav,
>
> kernel test robot noticed the following build errors:
It a second time you send not even compiled code.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH v2 2/2] mfd: lm3533: convert to use OF
From: Andy Shevchenko @ 2025-02-19 20:18 UTC (permalink / raw)
To: Svyatoslav Ryhel
Cc: Lee Jones, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Jonathan Cameron, Lars-Peter Clausen, Pavel Machek,
Daniel Thompson, Jingoo Han, Helge Deller, Uwe Kleine-König,
devicetree, linux-kernel, linux-iio, linux-leds, dri-devel,
linux-fbdev
In-Reply-To: <CAPVz0n19D1mS_prvMo-HD3m7U9+iknm49JSj5ydNUHoqjK7gZw@mail.gmail.com>
On Wed, Feb 19, 2025 at 07:39:29PM +0200, Svyatoslav Ryhel wrote:
> ср, 19 лют. 2025 р. о 17:56 Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> пише:
> > On Wed, Feb 19, 2025 at 05:13:15PM +0200, Svyatoslav Ryhel wrote:
> > > ср, 19 лют. 2025 р. о 17:07 Andy Shevchenko
> > > <andriy.shevchenko@linux.intel.com> пише:
> > > > On Wed, Feb 19, 2025 at 04:36:38PM +0200, Svyatoslav Ryhel wrote:
> > > > > ср, 19 лют. 2025 р. о 16:27 Andy Shevchenko
> > > > > <andriy.shevchenko@linux.intel.com> пише:
...
> > > > > MFD part is removed since MFD cells binding is unconditional, while
> > > > > the device supports any amount of children grater then one. For
> > > > > example, my device uses only backlight at bank A, while all other
> > > > > subdevices are not present and used. This patch switches to dynamic
> > > > > bind of children.
> > > >
> > > > MFD does the same. Please, take your time and get familiar with how MFD works.
> > >
> > > It does not. I have tried. If mfd cell binding is missing, driver will
> > > complain and fail.
> >
> > I really don't know what exactly is going on here, you can check it later, but
> > I'm 100% sure that MFD works for only driver that are present. What you are
> > describing is how component framework is designed.
> >
> > To proove my words you can check drivers/mfd/intel_soc_pmic_*.c and find listed
> > cells that never had a driver in the Linux kernel. They are just placeholders.
>
> This example is not valid since those drivers do not use device tree.
You didn't get the point. I'm telling that it should not matter if there is a
device driver present or absent. The rest will be enumerated as usual.
I even checked the code that handles of_compatible member from struct mfd_cell and
at a brief glance I do not see that it can do otherwise.
Care to elaborate what and where is the error happened exactly?
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH v2 2/2] mfd: lm3533: convert to use OF
From: kernel test robot @ 2025-02-19 16:02 UTC (permalink / raw)
To: Svyatoslav Ryhel, Lee Jones, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Jonathan Cameron, Lars-Peter Clausen, Pavel Machek,
Daniel Thompson, Jingoo Han, Helge Deller, Andy Shevchenko,
Uwe Kleine-König
Cc: oe-kbuild-all, devicetree, linux-kernel, linux-iio, linux-leds,
dri-devel, linux-fbdev
In-Reply-To: <20250218132702.114669-3-clamor95@gmail.com>
Hi Svyatoslav,
kernel test robot noticed the following build errors:
[auto build test ERROR on lee-mfd/for-mfd-next]
[also build test ERROR on lee-leds/for-leds-next robh/for-next linus/master v6.14-rc3 next-20250219]
[cannot apply to lee-mfd/for-mfd-fixes]
[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/Svyatoslav-Ryhel/dt-bindings-mfd-Document-TI-LM3533-MFD/20250218-212857
base: https://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd.git for-mfd-next
patch link: https://lore.kernel.org/r/20250218132702.114669-3-clamor95%40gmail.com
patch subject: [PATCH v2 2/2] mfd: lm3533: convert to use OF
config: i386-buildonly-randconfig-003-20250219 (https://download.01.org/0day-ci/archive/20250219/202502192343.twEQ3SSs-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250219/202502192343.twEQ3SSs-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/202502192343.twEQ3SSs-lkp@intel.com/
All errors (new ones prefixed by >>):
In file included from include/linux/device/driver.h:21,
from include/linux/device.h:32,
from include/linux/iio/iio.h:10,
from drivers/iio/light/lm3533-als.c:15:
>> drivers/iio/light/lm3533-als.c:921:25: error: 'lm3533_match_table' undeclared here (not in a function); did you mean 'lm3533_als_match_table'?
921 | MODULE_DEVICE_TABLE(of, lm3533_match_table);
| ^~~~~~~~~~~~~~~~~~
include/linux/module.h:250:15: note: in definition of macro 'MODULE_DEVICE_TABLE'
250 | extern typeof(name) __mod_device_table__##type##__##name \
| ^~~~
include/linux/module.h:250:21: error: '__mod_device_table__of__lm3533_match_table' aliased to undefined symbol 'lm3533_match_table'
250 | extern typeof(name) __mod_device_table__##type##__##name \
| ^~~~~~~~~~~~~~~~~~~~
drivers/iio/light/lm3533-als.c:921:1: note: in expansion of macro 'MODULE_DEVICE_TABLE'
921 | MODULE_DEVICE_TABLE(of, lm3533_match_table);
| ^~~~~~~~~~~~~~~~~~~
vim +921 drivers/iio/light/lm3533-als.c
916
917 static const struct of_device_id lm3533_als_match_table[] = {
918 { .compatible = "ti,lm3533-als" },
919 { },
920 };
> 921 MODULE_DEVICE_TABLE(of, lm3533_match_table);
922
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* Re: [PATCH v2 2/2] mfd: lm3533: convert to use OF
From: Svyatoslav Ryhel @ 2025-02-19 17:39 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Lee Jones, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Jonathan Cameron, Lars-Peter Clausen, Pavel Machek,
Daniel Thompson, Jingoo Han, Helge Deller, Uwe Kleine-König,
devicetree, linux-kernel, linux-iio, linux-leds, dri-devel,
linux-fbdev
In-Reply-To: <Z7X3ZvQbSe75U-AR@smile.fi.intel.com>
ср, 19 лют. 2025 р. о 17:56 Andy Shevchenko
<andriy.shevchenko@linux.intel.com> пише:
>
> On Wed, Feb 19, 2025 at 05:13:15PM +0200, Svyatoslav Ryhel wrote:
> > ср, 19 лют. 2025 р. о 17:07 Andy Shevchenko
> > <andriy.shevchenko@linux.intel.com> пише:
> > > On Wed, Feb 19, 2025 at 04:36:38PM +0200, Svyatoslav Ryhel wrote:
> > > > ср, 19 лют. 2025 р. о 16:27 Andy Shevchenko
> > > > <andriy.shevchenko@linux.intel.com> пише:
>
> ...
>
> > > > MFD part is removed since MFD cells binding is unconditional, while
> > > > the device supports any amount of children grater then one. For
> > > > example, my device uses only backlight at bank A, while all other
> > > > subdevices are not present and used. This patch switches to dynamic
> > > > bind of children.
> > >
> > > MFD does the same. Please, take your time and get familiar with how MFD works.
> >
> > It does not. I have tried. If mfd cell binding is missing, driver will
> > complain and fail.
>
> I really don't know what exactly is going on here, you can check it later, but
> I'm 100% sure that MFD works for only driver that are present. What you are
> describing is how component framework is designed.
>
> To proove my words you can check drivers/mfd/intel_soc_pmic_*.c and find listed
> cells that never had a driver in the Linux kernel. They are just placeholders.
>
This example is not valid since those drivers do not use device tree.
> ...
>
> > > --
> > > With Best Regards,
> > > Andy Shevchenko
>
> Please, when answering, remove unrelated context from the replies.
>
> ...
>
> > Let this driver rot for now, I might return to it. At some point
>
> Up to you. But thanks for trying!
>
Thank you for suggestions. Once I complete more urgent tasks, I might
do some tinkering with this driver.
> --
> With Best Regards,
> Andy Shevchenko
>
>
^ permalink raw reply
* Re: [PATCH v2 2/2] mfd: lm3533: convert to use OF
From: Andy Shevchenko @ 2025-02-19 15:23 UTC (permalink / raw)
To: Svyatoslav Ryhel
Cc: Lee Jones, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Jonathan Cameron, Lars-Peter Clausen, Pavel Machek,
Daniel Thompson, Jingoo Han, Helge Deller, Uwe Kleine-König,
devicetree, linux-kernel, linux-iio, linux-leds, dri-devel,
linux-fbdev
In-Reply-To: <CAPVz0n2WwAOb1UU7J7aDTdhXXCaAZkCpYjW_nc_CBRgkGWdEOw@mail.gmail.com>
On Wed, Feb 19, 2025 at 05:13:15PM +0200, Svyatoslav Ryhel wrote:
> ср, 19 лют. 2025 р. о 17:07 Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> пише:
> > On Wed, Feb 19, 2025 at 04:36:38PM +0200, Svyatoslav Ryhel wrote:
> > > ср, 19 лют. 2025 р. о 16:27 Andy Shevchenko
> > > <andriy.shevchenko@linux.intel.com> пише:
...
> > > MFD part is removed since MFD cells binding is unconditional, while
> > > the device supports any amount of children grater then one. For
> > > example, my device uses only backlight at bank A, while all other
> > > subdevices are not present and used. This patch switches to dynamic
> > > bind of children.
> >
> > MFD does the same. Please, take your time and get familiar with how MFD works.
>
> It does not. I have tried. If mfd cell binding is missing, driver will
> complain and fail.
I really don't know what exactly is going on here, you can check it later, but
I'm 100% sure that MFD works for only driver that are present. What you are
describing is how component framework is designed.
To proove my words you can check drivers/mfd/intel_soc_pmic_*.c and find listed
cells that never had a driver in the Linux kernel. They are just placeholders.
...
> > --
> > With Best Regards,
> > Andy Shevchenko
Please, when answering, remove unrelated context from the replies.
...
> Let this driver rot for now, I might return to it. At some point
Up to you. But thanks for trying!
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH v2 2/2] mfd: lm3533: convert to use OF
From: Svyatoslav Ryhel @ 2025-02-19 15:13 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Lee Jones, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Jonathan Cameron, Lars-Peter Clausen, Pavel Machek,
Daniel Thompson, Jingoo Han, Helge Deller, Uwe Kleine-König,
devicetree, linux-kernel, linux-iio, linux-leds, dri-devel,
linux-fbdev
In-Reply-To: <Z7XzgfHcjyK_UZKv@smile.fi.intel.com>
ср, 19 лют. 2025 р. о 17:07 Andy Shevchenko
<andriy.shevchenko@linux.intel.com> пише:
>
> On Wed, Feb 19, 2025 at 04:36:38PM +0200, Svyatoslav Ryhel wrote:
> > ср, 19 лют. 2025 р. о 16:27 Andy Shevchenko
> > <andriy.shevchenko@linux.intel.com> пише:
> > > On Tue, Feb 18, 2025 at 03:27:00PM +0200, Svyatoslav Ryhel wrote:
> > > > Remove platform data and fully relay on OF and device tree
> > > > parsing and binding devices.
> > >
> > > Thanks for following the advice, but the problem with this change as it does
> > > too much at once. It should be split to a few simpler ones.
> > > On top of that, this removes MFD participation from the driver but leaves it
> > > under MFD realm. Moreover, looking briefly at the code it looks like it open
> > > codes the parts of MFD. The latter needs a very goo justification which commit
> > > message is missing.
>
> ...
>
> > Splitting this into a set of commits would be nearly impossible,
>
> I don't buy this.
> One patch can introduce device property support.
> Another one removes the old platform data interface.
>
> So, at bare minimum there will be two patches. (Besides the advice to have
> two more.)
>
> > original driver does not relay on OF, it relays on platform data.
>
> And?..
>
> > Ripping out platform data will leave behind a broken useless driver.
>
> Hmm... This cna be the case if and only if we have the user in kernel.
> Is this the case?
>
> > So it has to be done simultaneously.
>
> Nope.
>
> > MFD part is removed since MFD cells binding is unconditional, while
> > the device supports any amount of children grater then one. For
> > example, my device uses only backlight at bank A, while all other
> > subdevices are not present and used. This patch switches to dynamic
> > bind of children.
>
> MFD does the same. Please, take your time and get familiar with how MFD works.
>
It does not. I have tried. If mfd cell binding is missing, driver will
complain and fail.
> ...
>
> > > > + device_property_read_string(&pdev->dev, "linux,default-trigger",
> > > > + &led->cdev.default_trigger);
> > >
> > > One prerequisite patch you probably want is an introduction of
> > >
> > > struct device *dev = &pdev->dev;
> > >
> > > in the respective ->probe() implementations. This, in particular, makes the
> > > above lines shorter and fit one line.
> >
> > This is not a scope of this patchset. Original driver uses &pdev->dev
>
> Indirectly it is. The change you are proposing tries to continue using this
> construction with making needlessly longer.
>
> ...
>
> > > > + if (!strcmp(comatible, "ti,lm3533-als"))
> > > > + lm3533->have_als = 1;
> > >
> > > If you end up having this, it's not the best what we can do. OF ID tables have
> > > a driver_data field exactly for the cases like this.
> >
> > This is required by core driver to handle some attributes and is here
> > solely not to touch those in this patch.
>
> What does this mean?
>
> --
> With Best Regards,
> Andy Shevchenko
>
>
Let this driver rot for now, I might return to it. At some point
^ permalink raw reply
* Re: [PATCH v2 2/2] mfd: lm3533: convert to use OF
From: Andy Shevchenko @ 2025-02-19 15:06 UTC (permalink / raw)
To: Svyatoslav Ryhel
Cc: Lee Jones, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Jonathan Cameron, Lars-Peter Clausen, Pavel Machek,
Daniel Thompson, Jingoo Han, Helge Deller, Uwe Kleine-König,
devicetree, linux-kernel, linux-iio, linux-leds, dri-devel,
linux-fbdev
In-Reply-To: <CAPVz0n1_WQyOHhtEVAh53uhEUhZvqqZSEJh6XALtSrVfkMSLYw@mail.gmail.com>
On Wed, Feb 19, 2025 at 04:36:38PM +0200, Svyatoslav Ryhel wrote:
> ср, 19 лют. 2025 р. о 16:27 Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> пише:
> > On Tue, Feb 18, 2025 at 03:27:00PM +0200, Svyatoslav Ryhel wrote:
> > > Remove platform data and fully relay on OF and device tree
> > > parsing and binding devices.
> >
> > Thanks for following the advice, but the problem with this change as it does
> > too much at once. It should be split to a few simpler ones.
> > On top of that, this removes MFD participation from the driver but leaves it
> > under MFD realm. Moreover, looking briefly at the code it looks like it open
> > codes the parts of MFD. The latter needs a very goo justification which commit
> > message is missing.
...
> Splitting this into a set of commits would be nearly impossible,
I don't buy this.
One patch can introduce device property support.
Another one removes the old platform data interface.
So, at bare minimum there will be two patches. (Besides the advice to have
two more.)
> original driver does not relay on OF, it relays on platform data.
And?..
> Ripping out platform data will leave behind a broken useless driver.
Hmm... This cna be the case if and only if we have the user in kernel.
Is this the case?
> So it has to be done simultaneously.
Nope.
> MFD part is removed since MFD cells binding is unconditional, while
> the device supports any amount of children grater then one. For
> example, my device uses only backlight at bank A, while all other
> subdevices are not present and used. This patch switches to dynamic
> bind of children.
MFD does the same. Please, take your time and get familiar with how MFD works.
...
> > > + device_property_read_string(&pdev->dev, "linux,default-trigger",
> > > + &led->cdev.default_trigger);
> >
> > One prerequisite patch you probably want is an introduction of
> >
> > struct device *dev = &pdev->dev;
> >
> > in the respective ->probe() implementations. This, in particular, makes the
> > above lines shorter and fit one line.
>
> This is not a scope of this patchset. Original driver uses &pdev->dev
Indirectly it is. The change you are proposing tries to continue using this
construction with making needlessly longer.
...
> > > + if (!strcmp(comatible, "ti,lm3533-als"))
> > > + lm3533->have_als = 1;
> >
> > If you end up having this, it's not the best what we can do. OF ID tables have
> > a driver_data field exactly for the cases like this.
>
> This is required by core driver to handle some attributes and is here
> solely not to touch those in this patch.
What does this mean?
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH v2 2/2] mfd: lm3533: convert to use OF
From: Svyatoslav Ryhel @ 2025-02-19 14:36 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Lee Jones, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Jonathan Cameron, Lars-Peter Clausen, Pavel Machek,
Daniel Thompson, Jingoo Han, Helge Deller, Uwe Kleine-König,
devicetree, linux-kernel, linux-iio, linux-leds, dri-devel,
linux-fbdev
In-Reply-To: <Z7XqKcOUt5niXzpv@smile.fi.intel.com>
ср, 19 лют. 2025 р. о 16:27 Andy Shevchenko
<andriy.shevchenko@linux.intel.com> пише:
>
> On Tue, Feb 18, 2025 at 03:27:00PM +0200, Svyatoslav Ryhel wrote:
> > Remove platform data and fully relay on OF and device tree
> > parsing and binding devices.
>
> Thanks for following the advice, but the problem with this change as it does
> too much at once. It should be split to a few simpler ones.
> On top of that, this removes MFD participation from the driver but leaves it
> under MFD realm. Moreover, looking briefly at the code it looks like it open
> codes the parts of MFD. The latter needs a very goo justification which commit
> message is missing.
>
Splitting this into a set of commits would be nearly impossible,
original driver does not relay on OF, it relays on platform data.
Ripping out platform data will leave behind a broken useless driver.
So it has to be done simultaneously.
MFD part is removed since MFD cells binding is unconditional, while
the device supports any amount of children grater then one. For
example, my device uses only backlight at bank A, while all other
subdevices are not present and used. This patch switches to dynamic
bind of children.
> ...
>
> > +static const struct of_device_id lm3533_als_match_table[] = {
> > + { .compatible = "ti,lm3533-als" },
> > + { },
>
> No comma for the terminator entry. I think I already pointed that out earlier.
>
> > +};
>
> ...
>
> > + device_property_read_string(&pdev->dev, "linux,default-trigger",
> > + &led->cdev.default_trigger);
>
> One prerequisite patch you probably want is an introduction of
>
> struct device *dev = &pdev->dev;
>
> in the respective ->probe() implementations. This, in particular, makes the
> above lines shorter and fit one line.
>
This is not a scope of this patchset. Original driver uses &pdev->dev
> ...
>
> > +static const struct of_device_id lm3533_led_match_table[] = {
> > + { .compatible = "ti,lm3533-leds" },
> > + { },
>
> As per above.
>
> > +};
>
> ...
>
> > + if (!strcmp(comatible, "ti,lm3533-als"))
> > + lm3533->have_als = 1;
>
> If you end up having this, it's not the best what we can do. OF ID tables have
> a driver_data field exactly for the cases like this.
>
This is required by core driver to handle some attributes and is here
solely not to touch those in this patch.
> ...
>
> > + if (!strcmp(comatible, "ti,lm3533-backlight"))
> > + lm3533->have_backlights = 1;
>
> Ditto.
>
> ...
>
> > + if (!strcmp(comatible, "ti,lm3533-leds"))
> > + lm3533->have_leds = 1;
>
> Ditto.
>
> ...
>
> > + ret = lm3533_update(bl->lm3533, LM3533_REG_CTRLBANK_AB_BCONF,
> > + 1 << (2 * id + 1), 1 << (2 * id + 1));
>
> BIT() and better to use a temporary variable for this calculation.
>
> > + if (ret)
> > + return ret;
>
> ...
>
> > + ret = lm3533_update(bl->lm3533, LM3533_REG_OUTPUT_CONF1,
> > + id | id << 1, BIT(0) | BIT(1));
>
> mask = GENMASK();
> ..., id ? mask : 0, mask);
>
> > + if (ret)
> > + return ret;
> > + }
>
> ...
>
> > + bd = devm_backlight_device_register(&pdev->dev, pdev->name, pdev->dev.parent,
> > + bl, &lm3533_bl_ops, &props);
>
>
> With the advice from above:
>
> bd = devm_backlight_device_register(dev, pdev->name, dev->parent, bl, &lm3533_bl_ops,
> &props);
>
>
> > if (IS_ERR(bd)) {
> > dev_err(&pdev->dev, "failed to register backlight device\n");
> > return PTR_ERR(bd);
>
> Consider another prerequisite patch (which should come before the firstly
> proposed one):
>
> struct device *dev = &pdev->dev; // yes, this can go in this change
> ...
>
> if (IS_ERR(bd))
> return dev_err_probe(dev, PTR_ERR(bd), "failed to register backlight device\n");
>
> ...
>
> > +static const struct of_device_id lm3533_bl_match_table[] = {
> > + { .compatible = "ti,lm3533-backlight" },
> > + { },
>
> As per above.
>
> > +};
>
> --
> With Best Regards,
> Andy Shevchenko
>
>
^ permalink raw reply
* Re: [PATCH v2 2/2] mfd: lm3533: convert to use OF
From: Andy Shevchenko @ 2025-02-19 14:26 UTC (permalink / raw)
To: Svyatoslav Ryhel
Cc: Lee Jones, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Jonathan Cameron, Lars-Peter Clausen, Pavel Machek,
Daniel Thompson, Jingoo Han, Helge Deller, Uwe Kleine-König,
devicetree, linux-kernel, linux-iio, linux-leds, dri-devel,
linux-fbdev
In-Reply-To: <20250218132702.114669-3-clamor95@gmail.com>
On Tue, Feb 18, 2025 at 03:27:00PM +0200, Svyatoslav Ryhel wrote:
> Remove platform data and fully relay on OF and device tree
> parsing and binding devices.
Thanks for following the advice, but the problem with this change as it does
too much at once. It should be split to a few simpler ones.
On top of that, this removes MFD participation from the driver but leaves it
under MFD realm. Moreover, looking briefly at the code it looks like it open
codes the parts of MFD. The latter needs a very goo justification which commit
message is missing.
...
> +static const struct of_device_id lm3533_als_match_table[] = {
> + { .compatible = "ti,lm3533-als" },
> + { },
No comma for the terminator entry. I think I already pointed that out earlier.
> +};
...
> + device_property_read_string(&pdev->dev, "linux,default-trigger",
> + &led->cdev.default_trigger);
One prerequisite patch you probably want is an introduction of
struct device *dev = &pdev->dev;
in the respective ->probe() implementations. This, in particular, makes the
above lines shorter and fit one line.
...
> +static const struct of_device_id lm3533_led_match_table[] = {
> + { .compatible = "ti,lm3533-leds" },
> + { },
As per above.
> +};
...
> + if (!strcmp(comatible, "ti,lm3533-als"))
> + lm3533->have_als = 1;
If you end up having this, it's not the best what we can do. OF ID tables have
a driver_data field exactly for the cases like this.
...
> + if (!strcmp(comatible, "ti,lm3533-backlight"))
> + lm3533->have_backlights = 1;
Ditto.
...
> + if (!strcmp(comatible, "ti,lm3533-leds"))
> + lm3533->have_leds = 1;
Ditto.
...
> + ret = lm3533_update(bl->lm3533, LM3533_REG_CTRLBANK_AB_BCONF,
> + 1 << (2 * id + 1), 1 << (2 * id + 1));
BIT() and better to use a temporary variable for this calculation.
> + if (ret)
> + return ret;
...
> + ret = lm3533_update(bl->lm3533, LM3533_REG_OUTPUT_CONF1,
> + id | id << 1, BIT(0) | BIT(1));
mask = GENMASK();
..., id ? mask : 0, mask);
> + if (ret)
> + return ret;
> + }
...
> + bd = devm_backlight_device_register(&pdev->dev, pdev->name, pdev->dev.parent,
> + bl, &lm3533_bl_ops, &props);
With the advice from above:
bd = devm_backlight_device_register(dev, pdev->name, dev->parent, bl, &lm3533_bl_ops,
&props);
> if (IS_ERR(bd)) {
> dev_err(&pdev->dev, "failed to register backlight device\n");
> return PTR_ERR(bd);
Consider another prerequisite patch (which should come before the firstly
proposed one):
struct device *dev = &pdev->dev; // yes, this can go in this change
...
if (IS_ERR(bd))
return dev_err_probe(dev, PTR_ERR(bd), "failed to register backlight device\n");
...
> +static const struct of_device_id lm3533_bl_match_table[] = {
> + { .compatible = "ti,lm3533-backlight" },
> + { },
As per above.
> +};
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* [PATCH] drivers: video: backlight: Fix NULL Pointer Dereference in backlight_device_register()
From: Haoyu Li @ 2025-02-19 12:29 UTC (permalink / raw)
To: danielt
Cc: chenyuan0y, deller, dri-devel, jani.nikula, jingoohan1, lee,
lihaoyu499, linux-fbdev, linux-kernel, robh, stable,
zichenxie0106
In-Reply-To: <Z65fFRKgqk-33HXI@aspen.lan>
As per Jani and Daniel's feedback, I have updated the patch so that
the `wled->name` null check now occurs in the `wled_configure`
function, right after the `devm_kasprintf` callsite. This should
resolve the issue.
The updated patch is as follows:
In the function "wled_probe", the "wled->name" is dynamically allocated
(wled_probe -> wled_configure -> devm_kasprintf), and it is possible
for it to be NULL.
To avoid dereferencing a NULL pointer (wled_probe ->
devm_backlight_device_register -> backlight_device_register),
we add a null-check after the allocation rather than in
backlight_device_register.
Fixes: f86b77583d88 ("backlight: pm8941: Convert to using %pOFn instead of device_node.name")
Signed-off-by: Haoyu Li <lihaoyu499@gmail.com>
Cc: stable@vger.kernel.org
---
drivers/video/backlight/qcom-wled.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/video/backlight/qcom-wled.c b/drivers/video/backlight/qcom-wled.c
index 9afe701b2a1b..3dacfef821ca 100644
--- a/drivers/video/backlight/qcom-wled.c
+++ b/drivers/video/backlight/qcom-wled.c
@@ -1409,6 +1409,11 @@ static int wled_configure(struct wled *wled)
if (rc)
wled->name = devm_kasprintf(dev, GFP_KERNEL, "%pOFn", dev->of_node);
+ if (!wled->name) {
+ dev_err(wled->dev, "Fail to initialize wled name\n");
+ return -EINVAL;
+ }
+
switch (wled->version) {
case 3:
u32_opts = wled3_opts;
--
2.34.1
^ permalink raw reply related
* [PATCH 6.1 341/578] m68k: vga: Fix I/O defines
From: Greg Kroah-Hartman @ 2025-02-19 8:25 UTC (permalink / raw)
To: stable
Cc: Greg Kroah-Hartman, patches, Thomas Zimmermann, kernel test robot,
Geert Uytterhoeven, linux-fbdev, dri-devel, Helge Deller
In-Reply-To: <20250219082652.891560343@linuxfoundation.org>
6.1-stable review patch. If anyone has any objections, please let me know.
------------------
From: Thomas Zimmermann <tzimmermann@suse.de>
commit 53036937a101b5faeaf98e7438555fa854a1a844 upstream.
Including m68k's <asm/raw_io.h> in vga.h on nommu platforms results
in conflicting defines with io_no.h for various I/O macros from the
__raw_read and __raw_write families. An example error is
In file included from arch/m68k/include/asm/vga.h:12,
from include/video/vga.h:22,
from include/linux/vgaarb.h:34,
from drivers/video/aperture.c:12:
>> arch/m68k/include/asm/raw_io.h:39: warning: "__raw_readb" redefined
39 | #define __raw_readb in_8
|
In file included from arch/m68k/include/asm/io.h:6,
from include/linux/io.h:13,
from include/linux/irq.h:20,
from include/asm-generic/hardirq.h:17,
from ./arch/m68k/include/generated/asm/hardirq.h:1,
from include/linux/hardirq.h:11,
from include/linux/interrupt.h:11,
from include/linux/trace_recursion.h:5,
from include/linux/ftrace.h:10,
from include/linux/kprobes.h:28,
from include/linux/kgdb.h:19,
from include/linux/fb.h:6,
from drivers/video/aperture.c:5:
arch/m68k/include/asm/io_no.h:16: note: this is the location of the previous definition
16 | #define __raw_readb(addr) \
|
Include <asm/io.h>, which avoids raw_io.h on nommu platforms.
Also change the defined values of some of the read/write symbols in
vga.h to __raw_read/__raw_write as the raw_in/raw_out symbols are not
generally available.
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202501071629.DNEswlm8-lkp@intel.com/
Fixes: 5c3f968712ce ("m68k/video: Create <asm/vga.h>")
Cc: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: linux-fbdev@vger.kernel.org
Cc: dri-devel@lists.freedesktop.org
Cc: Helge Deller <deller@gmx.de>
Cc: stable@vger.kernel.org # v3.5+
Reviewed-by: Geert Uytterhoeven <geert@linux-m68k.org>
Link: https://lore.kernel.org/20250107095912.130530-1-tzimmermann@suse.de
Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
arch/m68k/include/asm/vga.h | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
--- a/arch/m68k/include/asm/vga.h
+++ b/arch/m68k/include/asm/vga.h
@@ -9,7 +9,7 @@
*/
#ifndef CONFIG_PCI
-#include <asm/raw_io.h>
+#include <asm/io.h>
#include <asm/kmap.h>
/*
@@ -29,9 +29,9 @@
#define inw_p(port) 0
#define outb_p(port, val) do { } while (0)
#define outw(port, val) do { } while (0)
-#define readb raw_inb
-#define writeb raw_outb
-#define writew raw_outw
+#define readb __raw_readb
+#define writeb __raw_writeb
+#define writew __raw_writew
#endif /* CONFIG_PCI */
#endif /* _ASM_M68K_VGA_H */
^ permalink raw reply
* [PATCH 6.1 185/578] efi: sysfb_efi: fix W=1 warnings when EFI is not set
From: Greg Kroah-Hartman @ 2025-02-19 8:23 UTC (permalink / raw)
To: stable
Cc: Greg Kroah-Hartman, patches, Randy Dunlap, kernel test robot,
David Rheinsberg, Hans de Goede, Javier Martinez Canillas,
Peter Jones, Simona Vetter, linux-fbdev, Ard Biesheuvel,
linux-efi, Thomas Zimmermann, Sasha Levin
In-Reply-To: <20250219082652.891560343@linuxfoundation.org>
6.1-stable review patch. If anyone has any objections, please let me know.
------------------
From: Randy Dunlap <rdunlap@infradead.org>
[ Upstream commit 19fdc68aa7b90b1d3d600e873a3e050a39e7663d ]
A build with W=1 fails because there are code and data that are not
needed or used when CONFIG_EFI is not set. Move the "#ifdef CONFIG_EFI"
block to earlier in the source file so that the unused code/data are
not built.
drivers/firmware/efi/sysfb_efi.c:345:39: warning: ‘efifb_fwnode_ops’ defined but not used [-Wunused-const-variable=]
345 | static const struct fwnode_operations efifb_fwnode_ops = {
| ^~~~~~~~~~~~~~~~
drivers/firmware/efi/sysfb_efi.c:238:35: warning: ‘efifb_dmi_swap_width_height’ defined but not used [-Wunused-const-variable=]
238 | static const struct dmi_system_id efifb_dmi_swap_width_height[] __initconst = {
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/firmware/efi/sysfb_efi.c:188:35: warning: ‘efifb_dmi_system_table’ defined but not used [-Wunused-const-variable=]
188 | static const struct dmi_system_id efifb_dmi_system_table[] __initconst = {
| ^~~~~~~~~~~~~~~~~~~~~~
Fixes: 15d27b15de96 ("efi: sysfb_efi: fix build when EFI is not set")
Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202501071933.20nlmJJt-lkp@intel.com/
Cc: David Rheinsberg <david@readahead.eu>
Cc: Hans de Goede <hdegoede@redhat.com>
Cc: Javier Martinez Canillas <javierm@redhat.com>
Cc: Peter Jones <pjones@redhat.com>
Cc: Simona Vetter <simona@ffwll.ch>
Cc: linux-fbdev@vger.kernel.org
Cc: Ard Biesheuvel <ardb@kernel.org>
Cc: linux-efi@vger.kernel.org
Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/firmware/efi/sysfb_efi.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/firmware/efi/sysfb_efi.c b/drivers/firmware/efi/sysfb_efi.c
index 456d0e5eaf78b..f479680299838 100644
--- a/drivers/firmware/efi/sysfb_efi.c
+++ b/drivers/firmware/efi/sysfb_efi.c
@@ -91,6 +91,7 @@ void efifb_setup_from_dmi(struct screen_info *si, const char *opt)
_ret_; \
})
+#ifdef CONFIG_EFI
static int __init efifb_set_system(const struct dmi_system_id *id)
{
struct efifb_dmi_info *info = id->driver_data;
@@ -346,7 +347,6 @@ static const struct fwnode_operations efifb_fwnode_ops = {
.add_links = efifb_add_links,
};
-#ifdef CONFIG_EFI
static struct fwnode_handle efifb_fwnode;
__init void sysfb_apply_efi_quirks(void)
--
2.39.5
^ permalink raw reply related
* [PATCH] fbdev: lcdcfb: Register sysfs groups through driver core
From: oushixiong1025 @ 2025-02-19 8:44 UTC (permalink / raw)
To: Helge Deller
Cc: Thomas Zimmermann, Laurent Pinchart, Lee Jones,
Uwe Kleine-König, Arnd Bergmann, linux-fbdev, dri-devel,
linux-kernel, Shixiong Ou
From: Shixiong Ou <oushixiong@kylinos.cn>
[WHY]
1. The driver forgot to call device_remove_file()
in sh_mobile_lcdc_overlay_fb_unregister(), and there was
no error handling when calling device_create_file() failed.
2. This should probably use device_add_group() instead of
individual files to simplify both creation and removal. [Arnd]
3. The driver core can register and cleanup sysfs groups already.
as commit 95cdd538e0e5 ("fbdev: efifb: Register sysfs groups
through driver core").
[HOW]
Register sysfs groups through driver core.
Signed-off-by: Shixiong Ou <oushixiong@kylinos.cn>
---
drivers/video/fbdev/sh_mobile_lcdcfb.c | 29 ++++++++++++--------------
1 file changed, 13 insertions(+), 16 deletions(-)
diff --git a/drivers/video/fbdev/sh_mobile_lcdcfb.c b/drivers/video/fbdev/sh_mobile_lcdcfb.c
index 4715dcb59811..dd950e4ab5ce 100644
--- a/drivers/video/fbdev/sh_mobile_lcdcfb.c
+++ b/drivers/video/fbdev/sh_mobile_lcdcfb.c
@@ -1338,16 +1338,19 @@ overlay_rop3_store(struct device *dev, struct device_attribute *attr,
return count;
}
-static const struct device_attribute overlay_sysfs_attrs[] = {
- __ATTR(ovl_alpha, S_IRUGO|S_IWUSR,
- overlay_alpha_show, overlay_alpha_store),
- __ATTR(ovl_mode, S_IRUGO|S_IWUSR,
- overlay_mode_show, overlay_mode_store),
- __ATTR(ovl_position, S_IRUGO|S_IWUSR,
- overlay_position_show, overlay_position_store),
- __ATTR(ovl_rop3, S_IRUGO|S_IWUSR,
- overlay_rop3_show, overlay_rop3_store),
+static DEVICE_ATTR_RW(overlay_alpha);
+static DEVICE_ATTR_RW(overlay_mode);
+static DEVICE_ATTR_RW(overlay_position);
+static DEVICE_ATTR_RW(overlay_rop3);
+
+static struct attribute *overlay_sysfs_attrs[] = {
+ &dev_attr_overlay_alpha.attr,
+ &dev_attr_overlay_mode.attr,
+ &dev_attr_overlay_position.attr,
+ &dev_attr_overlay_rop3.attr,
+ NULL,
};
+ATTRIBUTE_GROUPS(overlay_sysfs);
static const struct fb_fix_screeninfo sh_mobile_lcdc_overlay_fix = {
.id = "SH Mobile LCDC",
@@ -1516,7 +1519,6 @@ sh_mobile_lcdc_overlay_fb_register(struct sh_mobile_lcdc_overlay *ovl)
{
struct sh_mobile_lcdc_priv *lcdc = ovl->channel->lcdc;
struct fb_info *info = ovl->info;
- unsigned int i;
int ret;
if (info == NULL)
@@ -1530,12 +1532,6 @@ sh_mobile_lcdc_overlay_fb_register(struct sh_mobile_lcdc_overlay *ovl)
dev_name(lcdc->dev), ovl->index, info->var.xres,
info->var.yres, info->var.bits_per_pixel);
- for (i = 0; i < ARRAY_SIZE(overlay_sysfs_attrs); ++i) {
- ret = device_create_file(info->dev, &overlay_sysfs_attrs[i]);
- if (ret < 0)
- return ret;
- }
-
return 0;
}
@@ -2641,6 +2637,7 @@ static int sh_mobile_lcdc_probe(struct platform_device *pdev)
static struct platform_driver sh_mobile_lcdc_driver = {
.driver = {
.name = "sh_mobile_lcdc_fb",
+ .dev_groups = overlay_sysfs_groups,
.pm = &sh_mobile_lcdc_dev_pm_ops,
},
.probe = sh_mobile_lcdc_probe,
--
2.25.1
^ permalink raw reply related
* Re: [PATCH v2] fbdev: lcdcfb: add missing device_remove_file()
From: Shixiong Ou @ 2025-02-19 7:40 UTC (permalink / raw)
To: Arnd Bergmann, Helge Deller
Cc: Thomas Zimmermann, Laurent Pinchart, Lee Jones,
Uwe Kleine-König, linux-fbdev, dri-devel, linux-kernel,
oushixiong
In-Reply-To: <4f2ae439-1bdc-4593-9151-e15981509344@app.fastmail.com>
在 2025/2/19 14:47, Arnd Bergmann 写道:
> On Sat, Feb 8, 2025, at 10:29, oushixiong1025@163.com wrote:
>> From: Shixiong Ou <oushixiong@kylinos.cn>
>>
>> 1. The device_remove_file() need to be called when driver is removing.
>> 2. The device_remove_file() need to be called if the call to
>> device_create_file() fails.
> This should probably use device_add_group() instead of
> individual files to simplify both creation and removal.
> It would also avoid the bug you introduced that gcc warns
> about.
>
> Arnd
Thank you for your suggestion. I will incorporate your advice and resend a patch.
Thanks and Regards,
Shixiong Ou.
^ permalink raw reply
* [PATCH v3] fbdev: lcdcfb: add missing device_remove_file()
From: oushixiong1025 @ 2025-02-19 6:48 UTC (permalink / raw)
To: Helge Deller
Cc: Thomas Zimmermann, Laurent Pinchart, Lee Jones,
Uwe Kleine-König, Arnd Bergmann, linux-fbdev, dri-devel,
linux-kernel, Shixiong Ou
From: Shixiong Ou <oushixiong@kylinos.cn>
1. The device_remove_file() need to be called when driver is removing.
2. The device_remove_file() need to be called if the call to
device_create_file() fails.
Signed-off-by: Shixiong Ou <oushixiong@kylinos.cn>
---
v1->v2:
add missing 'return error'.
call device_remove_file() in sh_mobile_lcdc_overlay_fb_unregister().
v2->v3:
change the type of 'i' to int.
add overlay_sysfs_attrs_enabled flag.
drivers/video/fbdev/sh_mobile_lcdcfb.c | 25 +++++++++++++++++++++----
1 file changed, 21 insertions(+), 4 deletions(-)
diff --git a/drivers/video/fbdev/sh_mobile_lcdcfb.c b/drivers/video/fbdev/sh_mobile_lcdcfb.c
index 4715dcb59811..eaf782133542 100644
--- a/drivers/video/fbdev/sh_mobile_lcdcfb.c
+++ b/drivers/video/fbdev/sh_mobile_lcdcfb.c
@@ -200,6 +200,8 @@ struct sh_mobile_lcdc_overlay {
unsigned int pitch;
int pos_x;
int pos_y;
+
+ bool overlay_sysfs_attrs_enabled;
};
struct sh_mobile_lcdc_priv {
@@ -1504,10 +1506,16 @@ static void
sh_mobile_lcdc_overlay_fb_unregister(struct sh_mobile_lcdc_overlay *ovl)
{
struct fb_info *info = ovl->info;
+ unsigned int i;
if (info == NULL || info->dev == NULL)
return;
+ if (ovl->overlay_sysfs_attrs_enabled) {
+ for (i = 0; i < ARRAY_SIZE(overlay_sysfs_attrs); ++i)
+ device_remove_file(info->dev, &overlay_sysfs_attrs[i]);
+ ovl->overlay_sysfs_attrs_enabled = false;
+ }
unregister_framebuffer(ovl->info);
}
@@ -1516,7 +1524,7 @@ sh_mobile_lcdc_overlay_fb_register(struct sh_mobile_lcdc_overlay *ovl)
{
struct sh_mobile_lcdc_priv *lcdc = ovl->channel->lcdc;
struct fb_info *info = ovl->info;
- unsigned int i;
+ int i, error = 0;
int ret;
if (info == NULL)
@@ -1530,10 +1538,19 @@ sh_mobile_lcdc_overlay_fb_register(struct sh_mobile_lcdc_overlay *ovl)
dev_name(lcdc->dev), ovl->index, info->var.xres,
info->var.yres, info->var.bits_per_pixel);
+ ovl->overlay_sysfs_attrs_enabled = true;
+
for (i = 0; i < ARRAY_SIZE(overlay_sysfs_attrs); ++i) {
- ret = device_create_file(info->dev, &overlay_sysfs_attrs[i]);
- if (ret < 0)
- return ret;
+ error = device_create_file(info->dev, &overlay_sysfs_attrs[i]);
+ if (error)
+ break;
+ }
+
+ if (error) {
+ while (--i >= 0)
+ device_remove_file(info->dev, &overlay_sysfs_attrs[i]);
+ ovl->overlay_sysfs_attrs_enabled = false;
+ return error;
}
return 0;
--
2.25.1
^ permalink raw reply related
* Re: [PATCH v2] fbdev: lcdcfb: add missing device_remove_file()
From: Arnd Bergmann @ 2025-02-19 6:47 UTC (permalink / raw)
To: oushixiong, Helge Deller
Cc: Thomas Zimmermann, Laurent Pinchart, Lee Jones,
Uwe Kleine-König, linux-fbdev, dri-devel, linux-kernel,
oushixiong
In-Reply-To: <20250208092918.251733-1-oushixiong1025@163.com>
On Sat, Feb 8, 2025, at 10:29, oushixiong1025@163.com wrote:
> From: Shixiong Ou <oushixiong@kylinos.cn>
>
> 1. The device_remove_file() need to be called when driver is removing.
> 2. The device_remove_file() need to be called if the call to
> device_create_file() fails.
This should probably use device_add_group() instead of
individual files to simplify both creation and removal.
It would also avoid the bug you introduced that gcc warns
about.
Arnd
^ 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