Implement review comments from Lee as requested in [0] for "leds: Add ChromeOS EC driver". Changes: * Inline DRV_NAME string constant. * Use dev_err() instead of dev_warn() to report errors. * Rename cros_ec_led_probe_led() to cros_ec_led_probe_one(). * Make loop variable "int" where they belong. * Move "Unknown LED" comment to where it belongs. * Register trigger during _probe(). * Use module_platform_driver() and drop all the custom boilerplate. [0] https://lore.kernel.org/lkml/20240614093445.GF3029315@xxxxxxxxxx/T/#m8750abdef6a968ace765645189170814196c9ce9 Signed-off-by: Thomas Weißschuh <linux@xxxxxxxxxxxxxx> --- drivers/leds/leds-cros_ec.c | 50 +++++++++++++-------------------------------- 1 file changed, 14 insertions(+), 36 deletions(-) diff --git a/drivers/leds/leds-cros_ec.c b/drivers/leds/leds-cros_ec.c index 7bb21a587713..275522b81ea5 100644 --- a/drivers/leds/leds-cros_ec.c +++ b/drivers/leds/leds-cros_ec.c @@ -14,8 +14,6 @@ #include <linux/platform_data/cros_ec_commands.h> #include <linux/platform_data/cros_ec_proto.h> -#define DRV_NAME "cros-ec-led" - static const char * const cros_ec_led_functions[] = { [EC_LED_ID_BATTERY_LED] = LED_FUNCTION_CHARGING, [EC_LED_ID_POWER_LED] = LED_FUNCTION_POWER, @@ -152,7 +150,7 @@ static int cros_ec_led_count_subleds(struct device *dev, if (common_range != range) { /* The multicolor LED API expects a uniform max_brightness */ - dev_warn(dev, "Inconsistent LED brightness values\n"); + dev_err(dev, "Inconsistent LED brightness values\n"); return -EINVAL; } } @@ -176,22 +174,21 @@ static const char *cros_ec_led_get_color_name(struct led_classdev_mc *led_mc_cde return led_get_color_name(color); } -static int cros_ec_led_probe_led(struct device *dev, struct cros_ec_device *cros_ec, +static int cros_ec_led_probe_one(struct device *dev, struct cros_ec_device *cros_ec, enum ec_led_id id) { union cros_ec_led_cmd_data arg = {}; struct cros_ec_led_priv *priv; struct led_classdev *led_cdev; struct mc_subled *subleds; - int ret, num_subleds; - size_t i, subled; + int i, ret, num_subleds; + size_t subled; arg.req.led_id = id; arg.req.flags = EC_LED_FLAGS_QUERY; ret = cros_ec_led_send_cmd(cros_ec, &arg); - /* Unknown LED, skip */ if (ret == -EINVAL) - return 0; + return 0; /* Unknown LED, skip */ if (ret == -EOPNOTSUPP) return -ENODEV; if (ret < 0) @@ -247,11 +244,14 @@ static int cros_ec_led_probe(struct platform_device *pdev) struct device *dev = &pdev->dev; struct cros_ec_dev *ec_dev = dev_get_drvdata(dev->parent); struct cros_ec_device *cros_ec = ec_dev->ec_dev; - int ret = 0; - size_t i; + int i, ret = 0; + + ret = devm_led_trigger_register(dev, &cros_ec_led_trigger); + if (ret) + return ret; for (i = 0; i < EC_LED_ID_COUNT; i++) { - ret = cros_ec_led_probe_led(dev, cros_ec, i); + ret = cros_ec_led_probe_one(dev, cros_ec, i); if (ret) break; } @@ -260,38 +260,16 @@ static int cros_ec_led_probe(struct platform_device *pdev) } static const struct platform_device_id cros_ec_led_id[] = { - { DRV_NAME, 0 }, + { "cros-ec-led", 0 }, {} }; static struct platform_driver cros_ec_led_driver = { - .driver.name = DRV_NAME, + .driver.name = "cros-ec-led", .probe = cros_ec_led_probe, .id_table = cros_ec_led_id, }; - -static int __init cros_ec_led_init(void) -{ - int ret; - - ret = led_trigger_register(&cros_ec_led_trigger); - if (ret) - return ret; - - ret = platform_driver_register(&cros_ec_led_driver); - if (ret) - led_trigger_unregister(&cros_ec_led_trigger); - - return ret; -}; -module_init(cros_ec_led_init); - -static void __exit cros_ec_led_exit(void) -{ - platform_driver_unregister(&cros_ec_led_driver); - led_trigger_unregister(&cros_ec_led_trigger); -}; -module_exit(cros_ec_led_exit); +module_platform_driver(cros_ec_led_driver); MODULE_DEVICE_TABLE(platform, cros_ec_led_id); MODULE_DESCRIPTION("ChromeOS EC LED Driver"); --- base-commit: b6774dd948171c478c7aa19318b1d7ae9cf6d7a4 change-id: 20240614-cros_ec-led-review-ec93abc65933 Best regards, -- Thomas Weißschuh <linux@xxxxxxxxxxxxxx>
