diff mbox series

[v5,4/6] drivers: leds: Add the iEi WT61P803 PUZZLE LED driver

Message ID 20201017211035.257110-5-luka.kovacic@sartura.hr
State Superseded
Headers show
Series Add support for the iEi WT61P803 PUZZLE MCU | expand

Commit Message

Luka Kovacic Oct. 17, 2020, 9:10 p.m. UTC 
Add support for the iEi WT61P803 PUZZLE LED driver.
Currently only the front panel power LED is supported.

This driver depends on the iEi WT61P803 PUZZLE MFD driver.

Signed-off-by: Luka Kovacic <luka.kovacic@sartura.hr>
Cc: Luka Perkov <luka.perkov@sartura.hr>
Cc: Robert Marko <robert.marko@sartura.hr>
---
 drivers/leds/Kconfig                    |   8 ++
 drivers/leds/Makefile                   |   1 +
 drivers/leds/leds-iei-wt61p803-puzzle.c | 147 ++++++++++++++++++++++++
 3 files changed, 156 insertions(+)
 create mode 100644 drivers/leds/leds-iei-wt61p803-puzzle.c

Comments

Andy Shevchenko Oct. 18, 2020, 6:50 p.m. UTC | #1 
On Sun, Oct 18, 2020 at 12:18 AM Luka Kovacic <luka.kovacic@sartura.hr> wrote:
>
> Add support for the iEi WT61P803 PUZZLE LED driver.
> Currently only the front panel power LED is supported.
>
> This driver depends on the iEi WT61P803 PUZZLE MFD driver.

...

> +/**
> + * struct iei_wt61p803_puzzle_led - MCU LED Driver
> + *
> + * @mcu:               MCU struct pointer
> + * @response_buffer    Global MCU response buffer allocation
> + * @lock:              General mutex lock to protect simultaneous R/W access to led_power_state
> + * @led_power_state:   State of the front panel power LED
> + * @cdev:              LED classdev
> + */
> +struct iei_wt61p803_puzzle_led {
> +       struct iei_wt61p803_puzzle *mcu;
> +       unsigned char *response_buffer;
> +       struct mutex lock;
> +       int led_power_state;

> +       struct led_classdev cdev;

If you are using container_of() and move this member to be first, you
will effectively make the container_of() a no-op.

> +};
> +
> +static inline struct iei_wt61p803_puzzle_led *cdev_to_iei_wt61p803_puzzle_led
> +       (struct led_classdev *led_cdev)
> +{
> +       return dev_get_drvdata(led_cdev->dev->parent);

Why not simply call container_of()

> +}

...

> +       ret = fwnode_property_read_u32(child, "reg", &reg);
> +       if (ret || reg > 1) {
> +               dev_err(dev, "Could not register 'reg' (%lu)\n", (unsigned long)reg);

When you cast explicitly during printf() you are doing something wrong
in 99.9% cases.
What's wrong with %u in this case?

> +               ret = -EINVAL;
> +               goto err_child_node;
> +       }
Luka Kovacic Oct. 19, 2020, 9:35 p.m. UTC | #2 
Hello Andy,

On Sun, Oct 18, 2020 at 8:50 PM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
>

> On Sun, Oct 18, 2020 at 12:18 AM Luka Kovacic <luka.kovacic@sartura.hr> wrote:

> >

> > Add support for the iEi WT61P803 PUZZLE LED driver.

> > Currently only the front panel power LED is supported.

> >

> > This driver depends on the iEi WT61P803 PUZZLE MFD driver.

>

> ...


I'll expand the description a bit.

>

> > +/**

> > + * struct iei_wt61p803_puzzle_led - MCU LED Driver

> > + *

> > + * @mcu:               MCU struct pointer

> > + * @response_buffer    Global MCU response buffer allocation

> > + * @lock:              General mutex lock to protect simultaneous R/W access to led_power_state

> > + * @led_power_state:   State of the front panel power LED

> > + * @cdev:              LED classdev

> > + */

> > +struct iei_wt61p803_puzzle_led {

> > +       struct iei_wt61p803_puzzle *mcu;

> > +       unsigned char *response_buffer;

> > +       struct mutex lock;

> > +       int led_power_state;

>

> > +       struct led_classdev cdev;

>

> If you are using container_of() and move this member to be first, you

> will effectively make the container_of() a no-op.

>

> > +};

> > +

> > +static inline struct iei_wt61p803_puzzle_led *cdev_to_iei_wt61p803_puzzle_led

> > +       (struct led_classdev *led_cdev)

> > +{

> > +       return dev_get_drvdata(led_cdev->dev->parent);

>

> Why not simply call container_of()


I agree, container_of() is much cleaner, when used here.

>

> > +}

>

> ...

>

> > +       ret = fwnode_property_read_u32(child, "reg", &reg);

> > +       if (ret || reg > 1) {

> > +               dev_err(dev, "Could not register 'reg' (%lu)\n", (unsigned long)reg);

>

> When you cast explicitly during printf() you are doing something wrong

> in 99.9% cases.

> What's wrong with %u in this case?


I'll remove the cast, %u should be okay.

>

> > +               ret = -EINVAL;

> > +               goto err_child_node;

> > +       }

>

> --

> With Best Regards,

> Andy Shevchenko


Kind regards,
Luka
diff mbox series

Patch

diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index 1c181df24eae..8a25fb753dec 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -332,6 +332,14 @@  config LEDS_IPAQ_MICRO
 	  Choose this option if you want to use the notification LED on
 	  Compaq/HP iPAQ h3100 and h3600.
 
+config LEDS_IEI_WT61P803_PUZZLE
+	tristate "LED Support for the iEi WT61P803 PUZZLE MCU"
+	depends on LEDS_CLASS
+	depends on MFD_IEI_WT61P803_PUZZLE
+	help
+	  This option enables support for LEDs controlled by the iEi WT61P803
+	  M801 MCU.
+
 config LEDS_HP6XX
 	tristate "LED Support for the HP Jornada 6xx"
 	depends on LEDS_CLASS
diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
index c2c7d7ade0d0..cd362437fefd 100644
--- a/drivers/leds/Makefile
+++ b/drivers/leds/Makefile
@@ -34,6 +34,7 @@  obj-$(CONFIG_LEDS_HP6XX)		+= leds-hp6xx.o
 obj-$(CONFIG_LEDS_INTEL_SS4200)		+= leds-ss4200.o
 obj-$(CONFIG_LEDS_IP30)			+= leds-ip30.o
 obj-$(CONFIG_LEDS_IPAQ_MICRO)		+= leds-ipaq-micro.o
+obj-$(CONFIG_LEDS_IEI_WT61P803_PUZZLE)	+= leds-iei-wt61p803-puzzle.o
 obj-$(CONFIG_LEDS_IS31FL319X)		+= leds-is31fl319x.o
 obj-$(CONFIG_LEDS_IS31FL32XX)		+= leds-is31fl32xx.o
 obj-$(CONFIG_LEDS_KTD2692)		+= leds-ktd2692.o
diff --git a/drivers/leds/leds-iei-wt61p803-puzzle.c b/drivers/leds/leds-iei-wt61p803-puzzle.c
new file mode 100644
index 000000000000..0b2c665a5bbb
--- /dev/null
+++ b/drivers/leds/leds-iei-wt61p803-puzzle.c
@@ -0,0 +1,147 @@ 
+// SPDX-License-Identifier: GPL-2.0-only
+/* iEi WT61P803 PUZZLE MCU LED Driver
+ *
+ * Copyright (C) 2020 Sartura Ltd.
+ * Author: Luka Kovacic <luka.kovacic@sartura.hr>
+ */
+
+#include <linux/leds.h>
+#include <linux/mfd/iei-wt61p803-puzzle.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/property.h>
+#include <linux/slab.h>
+
+enum iei_wt61p803_puzzle_led_state {
+	IEI_LED_OFF = 0x30,
+	IEI_LED_ON = 0x31,
+	IEI_LED_BLINK_5HZ = 0x32,
+	IEI_LED_BLINK_1HZ = 0x33,
+};
+
+/**
+ * struct iei_wt61p803_puzzle_led - MCU LED Driver
+ *
+ * @mcu:		MCU struct pointer
+ * @response_buffer	Global MCU response buffer allocation
+ * @lock:		General mutex lock to protect simultaneous R/W access to led_power_state
+ * @led_power_state:	State of the front panel power LED
+ * @cdev:		LED classdev
+ */
+struct iei_wt61p803_puzzle_led {
+	struct iei_wt61p803_puzzle *mcu;
+	unsigned char *response_buffer;
+	struct mutex lock;
+	int led_power_state;
+	struct led_classdev cdev;
+};
+
+static inline struct iei_wt61p803_puzzle_led *cdev_to_iei_wt61p803_puzzle_led
+	(struct led_classdev *led_cdev)
+{
+	return dev_get_drvdata(led_cdev->dev->parent);
+}
+
+static int iei_wt61p803_puzzle_led_brightness_set_blocking(struct led_classdev *cdev,
+							   enum led_brightness brightness)
+{
+	struct iei_wt61p803_puzzle_led *priv = cdev_to_iei_wt61p803_puzzle_led(cdev);
+	unsigned char *resp_buf = priv->response_buffer;
+	unsigned char led_power_cmd[5] = {};
+	size_t reply_size;
+
+	led_power_cmd[0] = IEI_WT61P803_PUZZLE_CMD_HEADER_START;
+	led_power_cmd[1] = IEI_WT61P803_PUZZLE_CMD_LED;
+	led_power_cmd[2] = IEI_WT61P803_PUZZLE_CMD_LED_POWER;
+	led_power_cmd[3] = brightness == LED_OFF ? IEI_LED_OFF : IEI_LED_ON;
+
+	mutex_lock(&priv->lock);
+	priv->led_power_state = brightness;
+	mutex_unlock(&priv->lock);
+
+	return iei_wt61p803_puzzle_write_command(priv->mcu, led_power_cmd,
+			sizeof(led_power_cmd), resp_buf, &reply_size);
+}
+
+static enum led_brightness iei_wt61p803_puzzle_led_brightness_get(struct led_classdev *cdev)
+{
+	struct iei_wt61p803_puzzle_led *priv = cdev_to_iei_wt61p803_puzzle_led(cdev);
+	int led_state;
+
+	mutex_lock(&priv->lock);
+	led_state = priv->led_power_state;
+	mutex_unlock(&priv->lock);
+
+	return led_state;
+}
+
+static int iei_wt61p803_puzzle_led_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct iei_wt61p803_puzzle *mcu = dev_get_drvdata(dev->parent);
+	struct iei_wt61p803_puzzle_led *priv;
+	struct led_init_data init_data = {};
+	struct fwnode_handle *child;
+	int ret;
+	u32 reg;
+
+	if (device_get_child_node_count(dev) != 1)
+		return -EINVAL;
+
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	priv->response_buffer = devm_kzalloc(dev, IEI_WT61P803_PUZZLE_BUF_SIZE, GFP_KERNEL);
+	if (!priv->response_buffer)
+		return -ENOMEM;
+
+	priv->mcu = mcu;
+	priv->led_power_state = 1;
+	mutex_init(&priv->lock);
+	dev_set_drvdata(dev, priv);
+
+	child = device_get_next_child_node(dev, NULL);
+
+	ret = fwnode_property_read_u32(child, "reg", &reg);
+	if (ret || reg > 1) {
+		dev_err(dev, "Could not register 'reg' (%lu)\n", (unsigned long)reg);
+		ret = -EINVAL;
+		goto err_child_node;
+	}
+
+	priv->cdev.brightness_set_blocking = iei_wt61p803_puzzle_led_brightness_set_blocking;
+	priv->cdev.brightness_get = iei_wt61p803_puzzle_led_brightness_get;
+	priv->cdev.max_brightness = 1;
+	init_data.fwnode = child;
+
+	ret = devm_led_classdev_register_ext(dev, &priv->cdev, &init_data);
+	if (ret) {
+		dev_err(dev, "Could not register LED\n");
+		goto err_child_node;
+	}
+err_child_node:
+	fwnode_handle_put(child);
+	return ret;
+}
+
+static const struct of_device_id iei_wt61p803_puzzle_led_of_match[] = {
+	{ .compatible = "iei,wt61p803-puzzle-leds" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, iei_wt61p803_puzzle_led_of_match);
+
+static struct platform_driver iei_wt61p803_puzzle_led_driver = {
+	.driver = {
+		.name = "iei-wt61p803-puzzle-led",
+		.of_match_table = iei_wt61p803_puzzle_led_of_match,
+	},
+	.probe = iei_wt61p803_puzzle_led_probe,
+};
+module_platform_driver(iei_wt61p803_puzzle_led_driver);
+
+MODULE_DESCRIPTION("iEi WT61P803 PUZZLE front panel LED driver");
+MODULE_AUTHOR("Luka Kovacic <luka.kovacic@sartura.hr>");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:leds-iei-wt61p803-puzzle");