Fix/refactor LED driver and service

apps/LedService/ledService.adef

 sandboxed: false
 start: auto
-version: 0.1
+version: 0.2
 
 executables:
 {
@@ -16,7 +16,7 @@ processes:
 
     envVars:
     {
-        LE_LOG_LEVEL = DEBUG
+        LE_LOG_LEVEL = INFO
     }
 }
 

apps/LedService/ledServiceComponent/ledService.c

@@ -2,7 +2,6 @@
  *
  * This file provides the implementation of @ref c_led
  *
- *
  * <hr>
  *
  * Copyright (C) Sierra Wireless Inc.
@@ -11,34 +10,41 @@
 #include "legato.h"
 #include "interfaces.h"
 
-//LED file descriptor
-static const char ledFileName[] = "/sys/devices/platform/led.0/led";
+static const char LedFilename[] = "/sys/devices/platform/led.0/led";
 
 //--------------------------------------------------------------------------------------------------
 /**
- * Turn ON the LED
+ * Turn on/off the LED
  */
 //--------------------------------------------------------------------------------------------------
-void ma_led_TurnOn(void)
+static void LedWrite(bool on)
 {
-    FILE *ledFile = NULL;
-
-    ledFile = fopen(ledFileName, "r+");
+    const char *writeData = on ? "1" : "0";
+    FILE *ledFile = fopen(LedFilename, "r+");
     if (ledFile == NULL)
     {
-        LE_ERROR("Open LED device file('%s') failed(%d)", ledFileName, errno);
-        goto cleanup;
+        LE_ERROR("Open LED device file('%s') failed(%d)", LedFilename, errno);
+        return;
     }
 
-    LE_DEBUG("turn on LED");
-    if (fwrite("1", strlen("1") + 1, 1, ledFile) <= 0)
+    LE_DEBUG("Turn %s LED", on ? "on" : "off");
+    if (fwrite(writeData, sizeof(writeData), 1, ledFile) != 1)
     {
-        LE_ERROR("Write LED device file('%s') failed(%d)", ledFileName, errno);
-        goto cleanup;
+        LE_ERROR("Write LED device file('%s') failed", LedFilename);
     }
 
-cleanup:
-    if (ledFile) fclose(ledFile);
+    fclose(ledFile);
+}
+
+//--------------------------------------------------------------------------------------------------
+/**
+ * Turn ON the LED
+ */
+//--------------------------------------------------------------------------------------------------
+void ma_led_TurnOn(void)
+{
+    const bool on = true;
+    LedWrite(on);
 }
 
 //--------------------------------------------------------------------------------------------------
@@ -48,24 +54,8 @@ cleanup:
 //--------------------------------------------------------------------------------------------------
 void ma_led_TurnOff(void)
 {
-    FILE *ledFile = NULL;
-
-    ledFile = fopen(ledFileName, "r+");
-    if (ledFile == NULL)
-    {
-        LE_ERROR("Open LED device file('%s') failed(%d)", ledFileName, errno);
-        goto cleanup;
-    }
-
-    LE_DEBUG("turn off LED");
-    if (fwrite("0", strlen("0") + 1, 1, ledFile) <= 0)
-    {
-        LE_ERROR("Write LED device file('%s') failed(%d)", ledFileName, errno);
-        goto cleanup;
-    }
-
-cleanup:
-    if (ledFile) fclose(ledFile);
+    const bool on = false;
+    LedWrite(on);
 }
 
 //--------------------------------------------------------------------------------------------------
@@ -74,34 +64,42 @@ cleanup:
  *
  * @return
  *	- OFF
- *      - ON
+ *  - ON
  */
 //--------------------------------------------------------------------------------------------------
 ma_led_LedStatus_t ma_led_GetLedStatus(void)
 {
     uint8_t buf[2];
     FILE *ledFile = NULL;
+    ma_led_LedStatus_t res = MA_LED_UNKNOWN;
 
-    ledFile = fopen(ledFileName, "r+");
+    ledFile = fopen(LedFilename, "r");
     if (ledFile == NULL)
     {
-        LE_ERROR("Open LED device file('%s') failed(%d)", ledFileName, errno);
-        goto cleanup;
+        LE_ERROR("Open LED device file('%s') failed(%d)", LedFilename, errno);
+        goto done;
     }
 
-    LE_DEBUG("turn on LED");
-    if (fread(buf, sizeof(buf), 1, ledFile) != sizeof(buf)) 
+    LE_DEBUG("Read LED state");
+    if (fread(buf, sizeof(buf), 1, ledFile) != 1)
+    {
+        LE_ERROR("Read LED device file('%s') failed", LedFilename);
+    }
+    else if (buf[0] == '0')
+    {
+        res = MA_LED_OFF;
+    }
+    else if (buf[0] == '1')
     {
-        LE_ERROR("Read LED device file('%s') failed(%d)", ledFileName, errno);
-        goto cleanup;
+        res = MA_LED_ON;
     }
+    fclose(ledFile);
 
-cleanup:
-    if (ledFile) fclose(ledFile);
-    return (buf[0] == '0') ? MA_LED_OFF:MA_LED_ON;
+done:
+    return res;
 }
 
 COMPONENT_INIT
 {
-    LE_INFO("---------------------- LED Service started");
+    LE_DEBUG("LED Service started");
 }

apps/LedService/ma_led.api

@@ -35,9 +35,9 @@
 //--------------------------------------------------------------------------------------------------
 ENUM LedStatus
 {
-    
-    OFF,   ///< The LED is off
-    ON,    ///< The LED is on
+    OFF,     ///< The LED is off
+    ON,      ///< The LED is on
+    UNKNOWN, ///< The LED status could not be determined
 };
 
 //--------------------------------------------------------------------------------------------------

linux_kernel_modules/led/led.c

@@ -12,14 +12,14 @@ struct led_device {
 };
 
 static ssize_t led_show(struct device *dev, struct device_attribute *attr,
-                        char *buf)
+			char *buf)
 {
 	struct led_device* led = dev_get_drvdata(dev);
 	return sprintf(buf, "%d\n", atomic_read(&led->val));
 }
 
 static int led_store(struct device *dev, struct device_attribute *attr,
-                     const char *buf, size_t count)
+		     const char *buf, size_t count)
 {
 	struct led_device* led = dev_get_drvdata(dev);
 	u8 val;
@@ -42,7 +42,7 @@ static int led_probe(struct platform_device *pdev)
 	int ret = 0;
 	struct mangohredled_platform_data *pdata = dev_get_platdata(&pdev->dev);
 
-	dev_info(&pdev->dev, "%s(): probe\n", __func__);
+	dev_dbg(&pdev->dev, "%s(): probe\n", __func__);
 
 	if (!pdata) {
 		ret = -EINVAL;
@@ -50,7 +50,6 @@ static int led_probe(struct platform_device *pdev)
 		goto  done;
 	}
 
-
 	dev = devm_kzalloc(&pdev->dev, sizeof(*dev), GFP_KERNEL);
 	if (!dev) {
 		ret = -ENOMEM;
@@ -61,15 +60,10 @@ static int led_probe(struct platform_device *pdev)
 	atomic_set(&dev->val, 0);
 	dev->gpio = pdata->gpio;
 
-	ret = devm_gpio_request(&pdev->dev, dev->gpio, dev_name(&pdev->dev));
-	if (ret) {
-		dev_err(&pdev->dev, "failed to request LED gpio\n");
-		goto done;
-	}
-
-	ret = gpio_direction_output(dev->gpio, atomic_read(&dev->val));
+	ret = devm_gpio_request_one(&pdev->dev, dev->gpio, GPIOF_OUT_INIT_LOW,
+				    dev_name(&pdev->dev));
 	if (ret) {
-		dev_err(&pdev->dev, "failed to set LED gpio as output\n");
+		dev_err(&pdev->dev, "failed to setup LED gpio\n");
 		goto done;
 	}
 
@@ -88,7 +82,7 @@ done:
 static int led_remove(struct platform_device *pdev)
 {
 	struct led_device* led = dev_get_drvdata(&pdev->dev);
-	dev_info(&pdev->dev, "%s(): remove\n", __func__);
+	dev_dbg(&pdev->dev, "%s(): remove\n", __func__);
 
 	/* remove sysfs files */
 	device_remove_file(&pdev->dev, &dev_attr_led);
@@ -118,8 +112,7 @@ static struct platform_driver led_driver = {
 
 static int __init led_init(void)
 {
-	platform_driver_register(&led_driver);
-	return 0;
+	return platform_driver_register(&led_driver);
 }
 
 static void __exit led_exit(void)

linux_kernel_modules/led/led.mdef

@@ -5,7 +5,7 @@ sources:
 
 cflags:
 {
-//    -DDEBUG
+    // -DDEBUG
 }
 
 params: