lkml.org 
[lkml]   [2018]   [Oct]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH v2 2/3] leds: upboard: Add LED support
From
Date
Hi Javier,

Thank you for the patch.

I have few trivial issues below, please take a look.

On 10/19/2018 07:15 PM, Dan O'Donovan wrote:
> From: Javier Arteaga <javier@emutex.com>
>
> Allow userspace to use the on-board LEDs as "upboard:<color>:".
>
> Acked-by: Pavel Machek <pavel@ucw.cz>
> Signed-off-by: Javier Arteaga <javier@emutex.com>
> Signed-off-by: Dan O'Donovan <dan@emutex.com>
> ---
> drivers/leds/Kconfig | 10 +++++
> drivers/leds/Makefile | 1 +
> drivers/leds/leds-upboard.c | 104 ++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 115 insertions(+)
> create mode 100644 drivers/leds/leds-upboard.c
>
> diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
> index 44097a3..0ed8857 100644
> --- a/drivers/leds/Kconfig
> +++ b/drivers/leds/Kconfig
> @@ -756,6 +756,16 @@ config LEDS_NIC78BX
> To compile this driver as a module, choose M here: the module
> will be called leds-nic78bx.
>
> +config LEDS_UPBOARD
> + tristate "LED support for the UP Squared"
> + depends on LEDS_CLASS
> + depends on MFD_UPBOARD
> + help
> + This option enables support for the LEDs on the UP Squared board.
> +
> + This driver can also be built as a module. If so, the module
> + will be called leds-upboard.
> +
> comment "LED Triggers"
> source "drivers/leds/trigger/Kconfig"
>
> diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
> index 420b5d2..c85f18f 100644
> --- a/drivers/leds/Makefile
> +++ b/drivers/leds/Makefile
> @@ -78,6 +78,7 @@ obj-$(CONFIG_LEDS_MT6323) += leds-mt6323.o
> obj-$(CONFIG_LEDS_LM3692X) += leds-lm3692x.o
> obj-$(CONFIG_LEDS_SC27XX_BLTC) += leds-sc27xx-bltc.o
> obj-$(CONFIG_LEDS_LM3601X) += leds-lm3601x.o
> +obj-$(CONFIG_LEDS_UPBOARD) += leds-upboard.o
>
> # LED SPI Drivers
> obj-$(CONFIG_LEDS_CR0014114) += leds-cr0014114.o
> diff --git a/drivers/leds/leds-upboard.c b/drivers/leds/leds-upboard.c
> new file mode 100644
> index 0000000..34a6973
> --- /dev/null
> +++ b/drivers/leds/leds-upboard.c
> @@ -0,0 +1,104 @@
> +// SPDX-License-Identifier: GPL-2.0
> +//
> +// UP Board LED driver
> +//
> +// Copyright (c) 2018, Emutex Ltd.
> +//
> +// Author: Javier Arteaga <javier@emutex.com>
> +//
> +
> +#include <linux/kernel.h>
> +#include <linux/leds.h>
> +#include <linux/mfd/upboard.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +#include <linux/acpi.h>

The last include should go first to keep alphabetical order.

> +
> +#define to_upboard_led(cdev) container_of(cdev, struct upboard_led, cdev)
> +
> +static const char * const upboard_led_names[] = {
> + "upboard:blue:",
> + "upboard:yellow:",
> + "upboard:green:",
> + "upboard:red:",
> +};
> +
> +struct upboard_led {
> + struct regmap_field *field;
> + struct led_classdev cdev;
> +};
> +
> +static enum led_brightness upboard_led_brightness_get(struct led_classdev *cdev)
> +{
> + struct upboard_led *led = to_upboard_led(cdev);
> + int brightness = 0;
> +
> + regmap_field_read(led->field, &brightness);

Please check the return value.

> + return brightness;
> +}
> +
> +static void upboard_led_brightness_set(struct led_classdev *cdev,
> + enum led_brightness brightness)
> +{
> + struct upboard_led *led = to_upboard_led(cdev);
> +
> + regmap_field_write(led->field, brightness);

Ditto.

> +}
> +
> +static int upboard_led_probe(struct platform_device *pdev)
> +{
> + unsigned int led_index = pdev->id;
> + struct device *dev = &pdev->dev;
> + struct acpi_device *adev;
> + struct upboard_led *led;
> + struct regmap *regmap;
> + struct reg_field conf = {
> + .reg = UPBOARD_REG_FUNC_EN0,
> + .lsb = led_index,
> + .msb = led_index,
> + };
> +
> + adev = ACPI_COMPANION(dev);
> + if (!adev || strcmp(acpi_device_hid(adev), "AANT0F01"))
> + return -ENODEV;
> +
> + if (led_index >= ARRAY_SIZE(upboard_led_names))
> + return -EINVAL;
> +
> + if (!dev->parent)
> + return -EINVAL;
> +
> + regmap = dev_get_regmap(dev->parent, NULL);
> + if (!regmap)
> + return -EINVAL;
> +
> + led = devm_kzalloc(dev, sizeof(*led), GFP_KERNEL);
> + if (!led)
> + return -ENOMEM;
> +
> + led->field = devm_regmap_field_alloc(dev, regmap, conf);
> + if (IS_ERR(led->field))
> + return PTR_ERR(led->field);
> +
> + led->cdev.max_brightness = 1;

s/1/LED_ON/

> + led->cdev.brightness_get = upboard_led_brightness_get;
> + led->cdev.brightness_set = upboard_led_brightness_set;
> + led->cdev.name = upboard_led_names[led_index];
> +
> + return devm_led_classdev_register(dev, &led->cdev);
> +}
> +
> +static struct platform_driver upboard_led_driver = {
> + .driver = {
> + .name = "upboard-led",
> + },
> +};
> +
> +module_platform_driver_probe(upboard_led_driver, upboard_led_probe);
> +
> +MODULE_ALIAS("platform:upboard-led");
> +MODULE_AUTHOR("Javier Arteaga <javier@emutex.com>");
> +MODULE_DESCRIPTION("UP Board LED driver");
> +MODULE_LICENSE("GPL v2");
>

--
Best regards,
Jacek Anaszewski

\
 
 \ /
  Last update: 2018-10-23 20:51    [W:0.338 / U:12.792 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site