From 1605577538c790e1d99df3b1595917160f1fe799 Mon Sep 17 00:00:00 2001 From: Erik Karlsson Date: Fri, 21 Feb 2025 14:37:38 +0100 Subject: [PATCH] gryphon-led-module: fix error handling The dev_get_drvdata/platform_get_drvdata functions do not return an error pointer. Use devm_gpiod_get_index to manage GPIO resources. Do not support obsolete Linux versions. Check the return value of gpiod_direction_output --- gryphon-led-module/src/main.c | 86 +++++------------------------------ 1 file changed, 12 insertions(+), 74 deletions(-) diff --git a/gryphon-led-module/src/main.c b/gryphon-led-module/src/main.c index c1c5e6775..6cd066863 100644 --- a/gryphon-led-module/src/main.c +++ b/gryphon-led-module/src/main.c @@ -47,11 +47,6 @@ static ssize_t get_led_color(struct device *dev, int len; struct sk9822_leds *sk9822 = dev_get_drvdata(dev); - if (IS_ERR(sk9822)) { - printk(KERN_ERR "Platform get drvdata returned NULL\n"); - return -EIO; - } - len = scnprintf(buf, PAGE_SIZE, "%02x%02x%02x\n", sk9822->led_colors[0].r, sk9822->led_colors[0].g, sk9822->led_colors[0].b); if (len <= 0) { dev_err(dev, "sk9822: Invalid sprintf len: %d\n", len); @@ -74,11 +69,6 @@ static ssize_t set_led_color(struct device *dev, size_t buflen = count; struct sk9822_leds *sk9822 = dev_get_drvdata(dev); - if (IS_ERR(sk9822)) { - printk(KERN_ERR "Platform get drvdata returned NULL\n"); - return -EIO; - } - /* strip newline */ if ((count > 0) && (buf[count-1] == '\n')) { buflen--; @@ -112,11 +102,6 @@ static ssize_t get_led_brightness(struct device *dev, int len; struct sk9822_leds *sk9822 = dev_get_drvdata(dev); - if (IS_ERR(sk9822)) { - printk(KERN_ERR "Platform get drvdata returned NULL\n"); - return -EIO; - } - len = scnprintf(buf, PAGE_SIZE, "%x\n", sk9822->led_brightness); if (len <= 0) { dev_err(dev, "sk9822: Invalid sprintf len: %d\n", len); @@ -139,11 +124,6 @@ static ssize_t set_led_brightness(struct device *dev, struct sk9822_leds *sk9822 = dev_get_drvdata(dev); unsigned long val = SK9822_DEFAULT_BRIGHTNESS; - if (IS_ERR(sk9822)) { - printk(KERN_ERR "Platform get drvdata returned NULL\n"); - return -EIO; - } - if (kstrtoul(buf, 16, &val)) { return -EINVAL; } @@ -210,49 +190,33 @@ static int canyon_led_probe(struct platform_device *pdev) platform_set_drvdata(pdev, leds); -#if LINUX_VERSION_CODE < KERNEL_VERSION(4, 3, 0) - leds->clock_gpio = gpiod_get_index(&pdev->dev, "led", 0); -#elif LINUX_VERSION_CODE >= KERNEL_VERSION(4, 3, 0) - leds->clock_gpio = gpiod_get_index(&pdev->dev, "led", 0, GPIOD_OUT_HIGH); -#else - dev_warn(&pdev->dev, "Kernel version Not supported\n"); - exit(1); -#endif + leds->clock_gpio = devm_gpiod_get_index(&pdev->dev, "led", 0, GPIOD_OUT_HIGH); if (IS_ERR(leds->clock_gpio)) { dev_err(&pdev->dev, "Failed to acquire clock GPIO %ld\n", PTR_ERR(leds->clock_gpio)); return PTR_ERR(leds->clock_gpio); } - gpiod_direction_output(leds->clock_gpio, 1); - if (IS_ERR(leds->clock_gpio)) { - dev_err(&pdev->dev, "Failed to acquire clock GPIO %ld\n", - PTR_ERR(leds->clock_gpio)); - return PTR_ERR(leds->clock_gpio); + ret = gpiod_direction_output(leds->clock_gpio, 1); + if (ret) { + dev_err(&pdev->dev, "Failed to set clock GPIO output %d\n", ret); + return ret; } else { printk(KERN_INFO "Got clock gpio\n"); gpiod_set_value(leds->clock_gpio, 0); } -#if LINUX_VERSION_CODE < KERNEL_VERSION(4, 3, 0) - leds->data_gpio = gpiod_get_index(&pdev->dev, "led", 1); -#elif LINUX_VERSION_CODE >= KERNEL_VERSION(4, 3, 0) - leds->data_gpio = gpiod_get_index(&pdev->dev, "led", 1, GPIOD_OUT_HIGH); -#else - dev_warn(&pdev->dev, "Kernel version Not supported\n"); - exit(1); -#endif + leds->data_gpio = devm_gpiod_get_index(&pdev->dev, "led", 1, GPIOD_OUT_HIGH); if (IS_ERR(leds->data_gpio)) { dev_err(&pdev->dev, "Failed to acquire data GPIO %ld\n", PTR_ERR(leds->data_gpio)); return PTR_ERR(leds->data_gpio); } - gpiod_direction_output(leds->data_gpio, 1); - if (IS_ERR(leds->data_gpio)) { - dev_err(&pdev->dev, "Failed to acquire data GPIO %ld\n", - PTR_ERR(leds->data_gpio)); - return PTR_ERR(leds->data_gpio); + ret = gpiod_direction_output(leds->data_gpio, 1); + if (ret) { + dev_err(&pdev->dev, "Failed to set data GPIO output %d\n", ret); + return ret; } else { printk(KERN_INFO "Got data gpio\n"); gpiod_set_value(leds->data_gpio, 0); @@ -288,41 +252,15 @@ static void canyon_led_off(struct sk9822_leds *leds) static int canyon_led_remove(struct platform_device *pdev) { - struct sk9822_leds *leds; - sysfs_remove_group(&pdev->dev.kobj, &sk9822_dev_attr_group); - - leds = platform_get_drvdata(pdev); - if (IS_ERR(leds)) { - printk(KERN_ERR "Platform get drvdata returned NULL\n"); - return -1; - } - - canyon_led_off(leds); - - if (leds->clock_gpio) { - gpiod_put(leds->clock_gpio); - } - - if (leds->data_gpio) { - gpiod_put(leds->data_gpio); - } - - printk(KERN_NOTICE "Bye, bye\n"); + canyon_led_off(platform_get_drvdata(pdev)); return 0; } static void canyon_led_shutdown(struct platform_device *pdev) { - struct sk9822_leds *leds = platform_get_drvdata(pdev); - - if (IS_ERR(leds)) { - printk(KERN_ERR "Platform get drvdata returned NULL\n"); - return; - } - - canyon_led_off(leds); + canyon_led_off(platform_get_drvdata(pdev)); } /**