[PATCH] leds: tests: clarify name conflict test It is a good practice to use a fresh led_classdev instance when doing the testing. LED_REJECT_NAME_CONFLICT rather than re-registering the already registered LED class device. Besides that, do a zero initialization of led_lookup_data.

[PATCH] leds: tests: clarify name conflict test It is a good practice to use a fresh led_classdev instance when doing the testing. LED_REJECT_NAME_CONFLICT rather than re-registering the already registered LED class device. Besides that, do a zero initialization of led_lookup_data.

[Lorenzo Egidio]

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.50.1

Re: [PATCH] leds: tests: clarify name conflict test It is a good practice to use a fresh led_classdev instance when doing the testing. LED_REJECT_NAME_CONFLICT rather than re-registering the already registered LED class device. Besides that, do a zero initialization of led_lookup_data.

[Pavel Machek]

> Signed-off-by: Lorenzo Egidio <lorenzoegidioms@gmail.com>
> ---
>  drivers/leds/led-test.c | 102 ++++++++++++++++++++++++++--------------
>  1 file changed, 66 insertions(+), 36 deletions(-)

NAK.
							Pavel