* [PATCH v2] leds: tests: use a fresh instance for name conflict rejection
@ 2026-06-12 23:06 Lorenzo Egidio
2026-06-17 8:48 ` Lee Jones
0 siblings, 1 reply; 2+ messages in thread
From: Lorenzo Egidio @ 2026-06-12 23:06 UTC (permalink / raw)
To: lee; +Cc: pavel, linux-leds, Lorenzo Egidio
The LED_REJECT_NAME_CONFLICT test currently re-registers an
already registered led_classdev instance.
Use a fresh copy instead so the test exercises the
name-conflict rejection path directly.
Signed-off-by: Lorenzo Egidio <lorenzoegidioms@gmail.com>
---
drivers/leds/led-test.c | 102 ++++++++++++++++++++++++++--------------
1 file changed, 66 insertions(+), 36 deletions(-)
diff --git a/drivers/leds/led-test.c b/drivers/leds/led-test.c
index ddf9aa967..36aef3e13 100644
--- a/drivers/leds/led-test.c
+++ b/drivers/leds/led-test.c
@@ -1,4 +1,5 @@
// SPDX-License-Identifier: GPL-2.0-only
+// leds: tests: clarify name conflict test
/*
* Copyright (C) 2025 Google LLC
*
@@ -10,77 +11,106 @@
#include <linux/device.h>
#include <linux/leds.h>
-#define LED_TEST_POST_REG_BRIGHTNESS 10
+enum {
+ LED_TEST_POST_REG_BRIGHTNESS = 10,
+};
-struct led_test_ddata {
+struct led_test_data {
struct led_classdev cdev;
struct device *dev;
};
-static enum led_brightness led_test_brightness_get(struct led_classdev *cdev)
+static enum led_brightness
+led_test_brightness_get(struct led_classdev *cdev)
{
return LED_TEST_POST_REG_BRIGHTNESS;
}
-static void led_test_class_register(struct kunit *test)
+static void led_test_init_cdev(struct led_classdev *cdev)
{
- struct led_test_ddata *ddata = test->priv;
- struct led_classdev *cdev_clash, *cdev = &ddata->cdev;
- struct device *dev = ddata->dev;
- int ret;
+ memset(cdev, 0, sizeof(*cdev));
- /* Register a LED class device */
cdev->name = "led-test";
- cdev->brightness_get = led_test_brightness_get;
cdev->brightness = 0;
+ cdev->brightness_get = led_test_brightness_get;
+}
+
+static void led_test_class_register(struct kunit *test)
+{
+ struct led_test_data *data = test->priv;
+ struct led_classdev *cdev = &data->cdev;
+ struct led_classdev *cdev_clash;
+ struct led_classdev *cdev_reject;
+ struct device *dev = data->dev;
+ int ret;
+
+ led_test_init_cdev(cdev);
ret = devm_led_classdev_register(dev, cdev);
KUNIT_ASSERT_EQ(test, ret, 0);
+ KUNIT_EXPECT_NOT_NULL(test, cdev->dev);
KUNIT_EXPECT_EQ(test, cdev->max_brightness, LED_FULL);
- KUNIT_EXPECT_EQ(test, cdev->brightness, LED_TEST_POST_REG_BRIGHTNESS);
+ KUNIT_EXPECT_EQ(test, cdev->brightness,
+ LED_TEST_POST_REG_BRIGHTNESS);
KUNIT_EXPECT_STREQ(test, cdev->dev->kobj.name, "led-test");
- /* Register again with the same name - expect it to pass with the LED renamed */
+ /*
+ * Name collision should be resolved automatically by
+ * renaming the device instance while preserving the
+ * logical LED name.
+ */
cdev_clash = devm_kmemdup(dev, cdev, sizeof(*cdev), GFP_KERNEL);
KUNIT_ASSERT_NOT_ERR_OR_NULL(test, cdev_clash);
ret = devm_led_classdev_register(dev, cdev_clash);
KUNIT_ASSERT_EQ(test, ret, 0);
- KUNIT_EXPECT_STREQ(test, cdev_clash->dev->kobj.name, "led-test_1");
- KUNIT_EXPECT_STREQ(test, cdev_clash->name, "led-test");
+ KUNIT_EXPECT_STREQ(test,
+ cdev_clash->dev->kobj.name,
+ "led-test_1");
+ KUNIT_EXPECT_STREQ(test,
+ cdev_clash->name,
+ "led-test");
- /* Enable name conflict rejection and register with the same name again - expect failure */
- cdev_clash->flags |= LED_REJECT_NAME_CONFLICT;
- ret = devm_led_classdev_register(dev, cdev_clash);
+ /*
+ * Verify that explicit conflict rejection fails.
+ */
+ cdev_reject = devm_kmemdup(dev, cdev, sizeof(*cdev), GFP_KERNEL);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, cdev_reject);
+
+ cdev_reject->flags |= LED_REJECT_NAME_CONFLICT;
+
+ ret = devm_led_classdev_register(dev, cdev_reject);
KUNIT_EXPECT_EQ(test, ret, -EEXIST);
}
static void led_test_class_add_lookup_and_get(struct kunit *test)
{
- struct led_test_ddata *ddata = test->priv;
- struct led_classdev *cdev = &ddata->cdev, *cdev_get;
- struct device *dev = ddata->dev;
- struct led_lookup_data lookup;
+ struct led_test_data *data = test->priv;
+ struct led_classdev *cdev = &data->cdev;
+ struct led_classdev *cdev_get;
+ struct device *dev = data->dev;
+ struct led_lookup_data lookup = { };
int ret;
- /* First, register a LED class device */
- cdev->name = "led-test";
+ led_test_init_cdev(cdev);
+
ret = devm_led_classdev_register(dev, cdev);
KUNIT_ASSERT_EQ(test, ret, 0);
- /* Then make the LED available for lookup */
lookup.provider = cdev->name;
lookup.dev_id = dev_name(dev);
lookup.con_id = "led-test-1";
+
led_add_lookup(&lookup);
- /* Finally, attempt to look it up via the API - imagine this was an orthogonal driver */
cdev_get = devm_led_get(dev, "led-test-1");
- KUNIT_ASSERT_FALSE(test, IS_ERR(cdev_get));
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, cdev_get);
- KUNIT_EXPECT_STREQ(test, cdev_get->name, cdev->name);
+ KUNIT_EXPECT_STREQ(test,
+ cdev_get->name,
+ cdev->name);
led_remove_lookup(&lookup);
}
@@ -93,30 +123,29 @@ static struct kunit_case led_test_cases[] = {
static int led_test_init(struct kunit *test)
{
- struct led_test_ddata *ddata;
+ struct led_test_data *data;
struct device *dev;
- ddata = kunit_kzalloc(test, sizeof(*ddata), GFP_KERNEL);
- if (!ddata)
+ data = kunit_kzalloc(test, sizeof(*data), GFP_KERNEL);
+ if (!data)
return -ENOMEM;
- test->priv = ddata;
-
dev = kunit_device_register(test, "led_test");
if (IS_ERR(dev))
return PTR_ERR(dev);
- ddata->dev = get_device(dev);
+ data->dev = get_device(dev);
+ test->priv = data;
return 0;
}
static void led_test_exit(struct kunit *test)
{
- struct led_test_ddata *ddata = test->priv;
+ struct led_test_data *data = test->priv;
- if (ddata && ddata->dev)
- put_device(ddata->dev);
+ if (data && data->dev)
+ put_device(data->dev);
}
static struct kunit_suite led_test_suite = {
@@ -125,6 +154,7 @@ static struct kunit_suite led_test_suite = {
.exit = led_test_exit,
.test_cases = led_test_cases,
};
+
kunit_test_suite(led_test_suite);
MODULE_AUTHOR("Lee Jones <lee@kernel.org>");
--
2.51.0
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH v2] leds: tests: use a fresh instance for name conflict rejection
2026-06-12 23:06 [PATCH v2] leds: tests: use a fresh instance for name conflict rejection Lorenzo Egidio
@ 2026-06-17 8:48 ` Lee Jones
0 siblings, 0 replies; 2+ messages in thread
From: Lee Jones @ 2026-06-17 8:48 UTC (permalink / raw)
To: Lorenzo Egidio; +Cc: pavel, linux-leds
On Fri, 12 Jun 2026, Lorenzo Egidio wrote:
> The LED_REJECT_NAME_CONFLICT test currently re-registers an
> already registered led_classdev instance.
>
> Use a fresh copy instead so the test exercises the
> name-conflict rejection path directly.
>
> Signed-off-by: Lorenzo Egidio <lorenzoegidioms@gmail.com>
How did you author this patch? Honestly.
> ---
> drivers/leds/led-test.c | 102 ++++++++++++++++++++++++++--------------
> 1 file changed, 66 insertions(+), 36 deletions(-)
>
> diff --git a/drivers/leds/led-test.c b/drivers/leds/led-test.c
> index ddf9aa967..36aef3e13 100644
> --- a/drivers/leds/led-test.c
> +++ b/drivers/leds/led-test.c
> @@ -1,4 +1,5 @@
> // SPDX-License-Identifier: GPL-2.0-only
> +// leds: tests: clarify name conflict test
Why did you add this?
> /*
> * Copyright (C) 2025 Google LLC
> *
> @@ -10,77 +11,106 @@
> #include <linux/device.h>
> #include <linux/leds.h>
>
> -#define LED_TEST_POST_REG_BRIGHTNESS 10
> +enum {
> + LED_TEST_POST_REG_BRIGHTNESS = 10,
> +};
Why?
> -struct led_test_ddata {
> +struct led_test_data {
Why rename?
> struct led_classdev cdev;
> struct device *dev;
> };
>
> -static enum led_brightness led_test_brightness_get(struct led_classdev *cdev)
> +static enum led_brightness
> +led_test_brightness_get(struct led_classdev *cdev)
Why?
> {
> return LED_TEST_POST_REG_BRIGHTNESS;
> }
>
> -static void led_test_class_register(struct kunit *test)
> +static void led_test_init_cdev(struct led_classdev *cdev)
> {
> - struct led_test_ddata *ddata = test->priv;
> - struct led_classdev *cdev_clash, *cdev = &ddata->cdev;
> - struct device *dev = ddata->dev;
> - int ret;
> + memset(cdev, 0, sizeof(*cdev));
Why?
>
> - /* Register a LED class device */
> cdev->name = "led-test";
> - cdev->brightness_get = led_test_brightness_get;
> cdev->brightness = 0;
> + cdev->brightness_get = led_test_brightness_get;
> +}
> +
> +static void led_test_class_register(struct kunit *test)
> +{
> + struct led_test_data *data = test->priv;
> + struct led_classdev *cdev = &data->cdev;
> + struct led_classdev *cdev_clash;
> + struct led_classdev *cdev_reject;
> + struct device *dev = data->dev;
> + int ret;
> +
> + led_test_init_cdev(cdev);
>
> ret = devm_led_classdev_register(dev, cdev);
> KUNIT_ASSERT_EQ(test, ret, 0);
>
> + KUNIT_EXPECT_NOT_NULL(test, cdev->dev);
How could this happen?
> KUNIT_EXPECT_EQ(test, cdev->max_brightness, LED_FULL);
> - KUNIT_EXPECT_EQ(test, cdev->brightness, LED_TEST_POST_REG_BRIGHTNESS);
> + KUNIT_EXPECT_EQ(test, cdev->brightness,
> + LED_TEST_POST_REG_BRIGHTNESS);
Why?
> KUNIT_EXPECT_STREQ(test, cdev->dev->kobj.name, "led-test");
>
> - /* Register again with the same name - expect it to pass with the LED renamed */
> + /*
> + * Name collision should be resolved automatically by
> + * renaming the device instance while preserving the
> + * logical LED name.
> + */
> cdev_clash = devm_kmemdup(dev, cdev, sizeof(*cdev), GFP_KERNEL);
> KUNIT_ASSERT_NOT_ERR_OR_NULL(test, cdev_clash);
>
> ret = devm_led_classdev_register(dev, cdev_clash);
> KUNIT_ASSERT_EQ(test, ret, 0);
>
> - KUNIT_EXPECT_STREQ(test, cdev_clash->dev->kobj.name, "led-test_1");
> - KUNIT_EXPECT_STREQ(test, cdev_clash->name, "led-test");
> + KUNIT_EXPECT_STREQ(test,
> + cdev_clash->dev->kobj.name,
> + "led-test_1");
> + KUNIT_EXPECT_STREQ(test,
> + cdev_clash->name,
> + "led-test");
Why?
>
> - /* Enable name conflict rejection and register with the same name again - expect failure */
> - cdev_clash->flags |= LED_REJECT_NAME_CONFLICT;
> - ret = devm_led_classdev_register(dev, cdev_clash);
> + /*
> + * Verify that explicit conflict rejection fails.
> + */
Why did you write a single line comment like this?
> + cdev_reject = devm_kmemdup(dev, cdev, sizeof(*cdev), GFP_KERNEL);
> + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, cdev_reject);
> +
> + cdev_reject->flags |= LED_REJECT_NAME_CONFLICT;
> +
> + ret = devm_led_classdev_register(dev, cdev_reject);
> KUNIT_EXPECT_EQ(test, ret, -EEXIST);
> }
>
> static void led_test_class_add_lookup_and_get(struct kunit *test)
> {
> - struct led_test_ddata *ddata = test->priv;
> - struct led_classdev *cdev = &ddata->cdev, *cdev_get;
> - struct device *dev = ddata->dev;
> - struct led_lookup_data lookup;
> + struct led_test_data *data = test->priv;
> + struct led_classdev *cdev = &data->cdev;
> + struct led_classdev *cdev_get;
> + struct device *dev = data->dev;
> + struct led_lookup_data lookup = { };
> int ret;
>
> - /* First, register a LED class device */
> - cdev->name = "led-test";
> + led_test_init_cdev(cdev);
> +
> ret = devm_led_classdev_register(dev, cdev);
> KUNIT_ASSERT_EQ(test, ret, 0);
>
> - /* Then make the LED available for lookup */
> lookup.provider = cdev->name;
> lookup.dev_id = dev_name(dev);
> lookup.con_id = "led-test-1";
> +
> led_add_lookup(&lookup);
>
> - /* Finally, attempt to look it up via the API - imagine this was an orthogonal driver */
You have removed all commentary - why?
> cdev_get = devm_led_get(dev, "led-test-1");
> - KUNIT_ASSERT_FALSE(test, IS_ERR(cdev_get));
> + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, cdev_get);
>
> - KUNIT_EXPECT_STREQ(test, cdev_get->name, cdev->name);
> + KUNIT_EXPECT_STREQ(test,
> + cdev_get->name,
> + cdev->name);
Why?
>
> led_remove_lookup(&lookup);
> }
> @@ -93,30 +123,29 @@ static struct kunit_case led_test_cases[] = {
>
> static int led_test_init(struct kunit *test)
> {
> - struct led_test_ddata *ddata;
> + struct led_test_data *data;
> struct device *dev;
>
> - ddata = kunit_kzalloc(test, sizeof(*ddata), GFP_KERNEL);
> - if (!ddata)
> + data = kunit_kzalloc(test, sizeof(*data), GFP_KERNEL);
> + if (!data)
> return -ENOMEM;
>
> - test->priv = ddata;
> -
> dev = kunit_device_register(test, "led_test");
> if (IS_ERR(dev))
> return PTR_ERR(dev);
>
> - ddata->dev = get_device(dev);
> + data->dev = get_device(dev);
> + test->priv = data;
>
> return 0;
> }
>
> static void led_test_exit(struct kunit *test)
> {
> - struct led_test_ddata *ddata = test->priv;
> + struct led_test_data *data = test->priv;
>
> - if (ddata && ddata->dev)
> - put_device(ddata->dev);
> + if (data && data->dev)
> + put_device(data->dev);
> }
>
> static struct kunit_suite led_test_suite = {
> @@ -125,6 +154,7 @@ static struct kunit_suite led_test_suite = {
> .exit = led_test_exit,
> .test_cases = led_test_cases,
> };
> +
Why?
> kunit_test_suite(led_test_suite);
>
> MODULE_AUTHOR("Lee Jones <lee@kernel.org>");
> --
> 2.51.0
>
--
Lee Jones
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2026-06-17 8:48 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-12 23:06 [PATCH v2] leds: tests: use a fresh instance for name conflict rejection Lorenzo Egidio
2026-06-17 8:48 ` Lee Jones
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox