On 15/03/2023 16:52, Lee Jones wrote:
+ for (;;) {
+ struct led_classdev *led_cdev;
+
+ led_cdev = devm_of_led_get_optional(dev, count);
+ if (IS_ERR(led_cdev))
Doesn't devm_of_led_get_optional() return NULL on failure?
Hi Lee,
Thanks for you review. I'll send an updated version shortly.
devm_of_led_get_optional() return an error-pointer when it cannot get
the LED (like EPROBE_DEFER or ENOMEM). When the LED is not defined, it
returns NULL.
+ return dev_err_probe(dev, PTR_ERR(led_cdev),
+ "Unable to get led #%d", count);
+ /* Reached the end of the list ?*/
Besides the incorrect formatting, that '?' isn't filling me with
confidence.
The comment just meant that NULL indicates the end of the list of the LEDs.
JJ
+ if (!led_cdev)
+ break;
+
+ priv->monochromatics = devm_krealloc_array(dev, priv->monochromatics,
+ count + 1, sizeof(*priv->monochromatics),
+ GFP_KERNEL);
+ if (!priv->monochromatics)
+ return -ENOMEM;
+
+ priv->monochromatics[count] = led_cdev;
+
+ max_brightness = max(max_brightness, led_cdev->max_brightness);
+ count++;
+ }
+
+ subled = devm_kcalloc(dev, count, sizeof(*subled), GFP_KERNEL);
+ if (!subled)
+ return -ENOMEM;
+ priv->mc_cdev.subled_info = subled;
+
+ for (i = 0; i < count; i++) {
+ struct led_classdev *led_cdev = priv->monochromatics[i];
+
+ subled[i].color_index = led_cdev->color;
'\n'
+ /* configure the LED intensity to its maximum */
Use correct grammar in comments please.
Start with an uppercase char.
+ subled[i].intensity = max_brightness;
+ }
+
+ /* init the multicolor's LED class device */
As above and please be fully forthcoming in comments: "Initialise".
+ cdev = &priv->mc_cdev.led_cdev;
+ cdev->flags = LED_CORE_SUSPENDRESUME;
+ cdev->brightness_set_blocking = led_mcg_set;
+ cdev->max_brightness = max_brightness;
+ cdev->color = LED_COLOR_ID_MULTI;
+ priv->mc_cdev.num_colors = count;
+
+ init_data.fwnode = dev_fwnode(dev);
+ ret = devm_led_classdev_multicolor_register_ext(dev, &priv->mc_cdev,
+ &init_data);
Use the full 100-char limit everywhere please.
+ if (ret)
+ return dev_err_probe(dev, ret,
+ "failed to register multicolor led for %s.\n",
"LED"
+ cdev->name);
+
+ ret = led_mcg_set(cdev, cdev->brightness);
+ if (ret)
+ return dev_err_probe(dev, ret,
+ "failed to set led value for %s.",
+ cdev->name);
+
+ for (i = 0; i < count; i++) {
+ struct led_classdev *led_cdev = priv->monochromatics[i];
+
+ /* Make the sysfs of the monochromatic LED read-only */
+ mutex_lock(&led_cdev->led_access);
+ led_sysfs_disable(led_cdev);
+ mutex_unlock(&led_cdev->led_access);
+
+ /* Restore sysfs access when the multicolor LED is released */
+ devm_add_action_or_reset(dev, restore_sysfs_access, led_cdev);
+ }
+
+ return 0;
+}
+
+static const struct of_device_id of_led_mcg_match[] = {
+ { .compatible = "leds-group-multicolor" },
+ {}
+};
+MODULE_DEVICE_TABLE(of, of_led_mcg_match);
+
+static struct platform_driver led_mcg_driver = {
+ .probe = led_mcg_probe,
+ .driver = {
+ .name = "leds_group_multicolor",
+ .of_match_table = of_led_mcg_match,
+ }
+};
+module_platform_driver(led_mcg_driver);
+
+MODULE_AUTHOR("Jean-Jacques Hiblot <jjhiblot@xxxxxxxxxxxxxxx>");
+MODULE_DESCRIPTION("multi-color LED group driver");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:leds-group-multicolor");
--
2.25.1
--
Lee Jones [李琼斯]
