* [PATCH v7 0/3] Add Turris Omnia LEDs driver
@ 2020-07-23 12:53 Marek Behún
2020-07-23 12:53 ` [PATCH v7 1/3] dt-bindings: leds: add cznic,turris-omnia-leds binding Marek Behún
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Marek Behún @ 2020-07-23 12:53 UTC (permalink / raw)
To: linux-leds; +Cc: Pavel Machek, jacek.anaszewski, Dan Murphy, Marek Behún
Hi,
this is version 7 of LED driver for Turris Omnia, appliable on top
of Pavel's for-next branch.
Marek
changes since v6:
- comment formatted to 80 columns as per Pavel's request
changes since v5:
- added additionalProperties: false to yaml scheme as requested by Rob
- changed 'multi color' to 'multicolor' in commit message
- changed LEDS_CLASS_MULTI_COLOR to LEDS_CLASS_MULTICOLOR in Kconfig
depend
- added Rob's reviewed-by tag to dt patch and Dan's to driver adding
patch
changes since v4:
- fixed a typo in dt-binding
- change to dt-binding documentation as suggested by Dan Murphy so that
it is less confusing
- cleaning up some things in the code addressed by Dan Murphy
- added sysfs ABI documentation for the brightness file controlling the
brightness of the whole panel
changes since v3:
- added a note in yaml scheme to the description of multi-led property
that no subnodes for the specific channels are needed since this
controller only supports RGB LEDs. A comment into the device-tree
example in the yaml scheme is added saying the same thing.
changes since v2:
- using multicolor LED framework now, major rewrite
- added support for global brightness (Omnia has a button which can
switch between 8 levels of intensity of all the LEDs at once, but
in reality any value between 0% and 100% can be set, and we want to
access this setting via software)
Marek Behún (3):
dt-bindings: leds: add cznic,turris-omnia-leds binding
leds: initial support for Turris Omnia LEDs
Documentation: ABI: leds-turris-omnia: document sysfs attribute
.../sysfs-class-led-driver-turris-omnia | 14 +
.../leds/cznic,turris-omnia-leds.yaml | 90 ++++++
drivers/leds/Kconfig | 11 +
drivers/leds/Makefile | 1 +
drivers/leds/leds-turris-omnia.c | 295 ++++++++++++++++++
5 files changed, 411 insertions(+)
create mode 100644 Documentation/ABI/testing/sysfs-class-led-driver-turris-omnia
create mode 100644 Documentation/devicetree/bindings/leds/cznic,turris-omnia-leds.yaml
create mode 100644 drivers/leds/leds-turris-omnia.c
--
2.26.2
^ permalink raw reply [flat|nested] 8+ messages in thread* [PATCH v7 1/3] dt-bindings: leds: add cznic,turris-omnia-leds binding 2020-07-23 12:53 [PATCH v7 0/3] Add Turris Omnia LEDs driver Marek Behún @ 2020-07-23 12:53 ` Marek Behún 2020-07-28 17:49 ` Rob Herring 2020-07-23 12:53 ` [PATCH v7 2/3] leds: initial support for Turris Omnia LEDs Marek Behún 2020-07-23 12:53 ` [PATCH v7 3/3] Documentation: ABI: leds-turris-omnia: document sysfs attribute Marek Behún 2 siblings, 1 reply; 8+ messages in thread From: Marek Behún @ 2020-07-23 12:53 UTC (permalink / raw) To: linux-leds Cc: Pavel Machek, jacek.anaszewski, Dan Murphy, Marek Behún, Rob Herring, devicetree Add device-tree bindings documentation for Turris Omnia RGB LEDs. Signed-off-by: Marek Behún <marek.behun@nic.cz> Reviewed-by: Rob Herring <robh@kernel.org> Cc: devicetree@vger.kernel.org --- .../leds/cznic,turris-omnia-leds.yaml | 90 +++++++++++++++++++ 1 file changed, 90 insertions(+) create mode 100644 Documentation/devicetree/bindings/leds/cznic,turris-omnia-leds.yaml diff --git a/Documentation/devicetree/bindings/leds/cznic,turris-omnia-leds.yaml b/Documentation/devicetree/bindings/leds/cznic,turris-omnia-leds.yaml new file mode 100644 index 000000000000..24ad1446445e --- /dev/null +++ b/Documentation/devicetree/bindings/leds/cznic,turris-omnia-leds.yaml @@ -0,0 +1,90 @@ +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/leds/cznic,turris-omnia-leds.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: CZ.NIC's Turris Omnia LEDs driver + +maintainers: + - Marek Behún <marek.behun@nic.cz> + +description: + This module adds support for the RGB LEDs found on the front panel of the + Turris Omnia router. There are 12 RGB LEDs that are controlled by a + microcontroller that communicates via the I2C bus. Each LED is described + as a subnode of this I2C device. + +properties: + compatible: + const: cznic,turris-omnia-leds + + reg: + description: I2C slave address of the microcontroller. + maxItems: 1 + + "#address-cells": + const: 1 + + "#size-cells": + const: 0 + +patternProperties: + "^multi-led[0-9a-f]$": + type: object + allOf: + - $ref: leds-class-multicolor.yaml# + description: + This node represents one of the RGB LED devices on Turris Omnia. + No subnodes need to be added for subchannels since this controller only + supports RGB LEDs. + + properties: + reg: + minimum: 0 + maximum: 11 + description: + This property identifies one of the LEDs on the front panel of the + Turris Omnia router. + + required: + - reg + +additionalProperties: false + +examples: + - | + + #include <dt-bindings/leds/common.h> + + i2c0 { + #address-cells = <1>; + #size-cells = <0>; + + led-controller@2b { + compatible = "cznic,turris-omnia-leds"; + reg = <0x2b>; + #address-cells = <1>; + #size-cells = <0>; + + multi-led@0 { + /* + * No subnodes are needed, this controller only supports RGB + * LEDs. + */ + reg = <0>; + color = <LED_COLOR_ID_MULTI>; + function = LED_FUNCTION_POWER; + linux,default-trigger = "heartbeat"; + }; + + multi-led@a { + reg = <0xa>; + color = <LED_COLOR_ID_MULTI>; + function = LED_FUNCTION_INDICATOR; + function-enumerator = <1>; + }; + }; + }; + +... -- 2.26.2 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v7 1/3] dt-bindings: leds: add cznic,turris-omnia-leds binding 2020-07-23 12:53 ` [PATCH v7 1/3] dt-bindings: leds: add cznic,turris-omnia-leds binding Marek Behún @ 2020-07-28 17:49 ` Rob Herring 0 siblings, 0 replies; 8+ messages in thread From: Rob Herring @ 2020-07-28 17:49 UTC (permalink / raw) To: Marek Behún Cc: Linux LED Subsystem, Pavel Machek, Jacek Anaszewski, Dan Murphy, devicetree On Thu, Jul 23, 2020 at 6:53 AM Marek Behún <marek.behun@nic.cz> wrote: > > Add device-tree bindings documentation for Turris Omnia RGB LEDs. > > Signed-off-by: Marek Behún <marek.behun@nic.cz> > Reviewed-by: Rob Herring <robh@kernel.org> > Cc: devicetree@vger.kernel.org > --- > .../leds/cznic,turris-omnia-leds.yaml | 90 +++++++++++++++++++ > 1 file changed, 90 insertions(+) > create mode 100644 Documentation/devicetree/bindings/leds/cznic,turris-omnia-leds.yaml > > diff --git a/Documentation/devicetree/bindings/leds/cznic,turris-omnia-leds.yaml b/Documentation/devicetree/bindings/leds/cznic,turris-omnia-leds.yaml > new file mode 100644 > index 000000000000..24ad1446445e > --- /dev/null > +++ b/Documentation/devicetree/bindings/leds/cznic,turris-omnia-leds.yaml > @@ -0,0 +1,90 @@ > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/leds/cznic,turris-omnia-leds.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: CZ.NIC's Turris Omnia LEDs driver > + > +maintainers: > + - Marek Behún <marek.behun@nic.cz> > + > +description: > + This module adds support for the RGB LEDs found on the front panel of the > + Turris Omnia router. There are 12 RGB LEDs that are controlled by a > + microcontroller that communicates via the I2C bus. Each LED is described > + as a subnode of this I2C device. > + > +properties: > + compatible: > + const: cznic,turris-omnia-leds > + > + reg: > + description: I2C slave address of the microcontroller. > + maxItems: 1 > + > + "#address-cells": > + const: 1 > + > + "#size-cells": > + const: 0 > + > +patternProperties: > + "^multi-led[0-9a-f]$": Doesn't match the example: Documentation/devicetree/bindings/leds/cznic,turris-omnia-leds.example.dt.yaml: led-controller@2b: 'multi-led@0', 'multi-led@a' do not match any of the regexes: '^multi-led[0-9a-f]$', 'pinctrl-[0-9]+' > + type: object > + allOf: > + - $ref: leds-class-multicolor.yaml# > + description: > + This node represents one of the RGB LED devices on Turris Omnia. > + No subnodes need to be added for subchannels since this controller only > + supports RGB LEDs. > + > + properties: > + reg: > + minimum: 0 > + maximum: 11 > + description: > + This property identifies one of the LEDs on the front panel of the > + Turris Omnia router. > + > + required: > + - reg > + > +additionalProperties: false > + > +examples: > + - | > + > + #include <dt-bindings/leds/common.h> > + > + i2c0 { > + #address-cells = <1>; > + #size-cells = <0>; > + > + led-controller@2b { > + compatible = "cznic,turris-omnia-leds"; > + reg = <0x2b>; > + #address-cells = <1>; > + #size-cells = <0>; > + > + multi-led@0 { > + /* > + * No subnodes are needed, this controller only supports RGB > + * LEDs. > + */ > + reg = <0>; > + color = <LED_COLOR_ID_MULTI>; > + function = LED_FUNCTION_POWER; > + linux,default-trigger = "heartbeat"; > + }; > + > + multi-led@a { > + reg = <0xa>; > + color = <LED_COLOR_ID_MULTI>; > + function = LED_FUNCTION_INDICATOR; > + function-enumerator = <1>; > + }; > + }; > + }; > + > +... > -- > 2.26.2 > ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v7 2/3] leds: initial support for Turris Omnia LEDs 2020-07-23 12:53 [PATCH v7 0/3] Add Turris Omnia LEDs driver Marek Behún 2020-07-23 12:53 ` [PATCH v7 1/3] dt-bindings: leds: add cznic,turris-omnia-leds binding Marek Behún @ 2020-07-23 12:53 ` Marek Behún 2020-07-24 10:58 ` Pavel Machek 2020-07-23 12:53 ` [PATCH v7 3/3] Documentation: ABI: leds-turris-omnia: document sysfs attribute Marek Behún 2 siblings, 1 reply; 8+ messages in thread From: Marek Behún @ 2020-07-23 12:53 UTC (permalink / raw) To: linux-leds; +Cc: Pavel Machek, jacek.anaszewski, Dan Murphy, Marek Behún This adds basic support for LEDs on the front side of CZ.NIC's Turris Omnia router. There are 12 RGB LEDs. The controller supports HW triggering mode for the LEDs, but this driver does not support it yet, and sets all the LEDs defined in device-tree into SW mode upon probe. This driver uses the multicolor LED framework. Signed-off-by: Marek Behún <marek.behun@nic.cz> Reviewed-by: Dan Murphy <dmurphy@ti.com> --- drivers/leds/Kconfig | 11 ++ drivers/leds/Makefile | 1 + drivers/leds/leds-turris-omnia.c | 295 +++++++++++++++++++++++++++++++ 3 files changed, 307 insertions(+) create mode 100644 drivers/leds/leds-turris-omnia.c diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig index b9002850b5fa..d7e4be1b7736 100644 --- a/drivers/leds/Kconfig +++ b/drivers/leds/Kconfig @@ -176,6 +176,17 @@ config LEDS_EL15203000 To compile this driver as a module, choose M here: the module will be called leds-el15203000. +config LEDS_TURRIS_OMNIA + tristate "LED support for CZ.NIC's Turris Omnia" + depends on LEDS_CLASS_MULTICOLOR + depends on I2C + depends on MACH_ARMADA_38X || COMPILE_TEST + depends on OF + help + This option enables basic support for the LEDs found on the front + side of CZ.NIC's Turris Omnia router. There are 12 RGB LEDs on the + front panel. + config LEDS_LM3530 tristate "LCD Backlight driver for LM3530" depends on LEDS_CLASS diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile index d684bc76d2b2..c2c7d7ade0d0 100644 --- a/drivers/leds/Makefile +++ b/drivers/leds/Makefile @@ -87,6 +87,7 @@ obj-$(CONFIG_LEDS_TCA6507) += leds-tca6507.o obj-$(CONFIG_LEDS_TI_LMU_COMMON) += leds-ti-lmu-common.o obj-$(CONFIG_LEDS_TLC591XX) += leds-tlc591xx.o obj-$(CONFIG_LEDS_TPS6105X) += leds-tps6105x.o +obj-$(CONFIG_LEDS_TURRIS_OMNIA) += leds-turris-omnia.o obj-$(CONFIG_LEDS_WM831X_STATUS) += leds-wm831x-status.o obj-$(CONFIG_LEDS_WM8350) += leds-wm8350.o obj-$(CONFIG_LEDS_WRAP) += leds-wrap.o diff --git a/drivers/leds/leds-turris-omnia.c b/drivers/leds/leds-turris-omnia.c new file mode 100644 index 000000000000..bb23d8e16614 --- /dev/null +++ b/drivers/leds/leds-turris-omnia.c @@ -0,0 +1,295 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * CZ.NIC's Turris Omnia LEDs driver + * + * 2020 by Marek Behun <marek.behun@nic.cz> + */ + +#include <linux/i2c.h> +#include <linux/led-class-multicolor.h> +#include <linux/module.h> +#include <linux/mutex.h> +#include <linux/of.h> +#include "leds.h" + +#define OMNIA_BOARD_LEDS 12 +#define OMNIA_LED_NUM_CHANNELS 3 + +#define CMD_LED_MODE 3 +#define CMD_LED_MODE_LED(l) ((l) & 0x0f) +#define CMD_LED_MODE_USER 0x10 + +#define CMD_LED_STATE 4 +#define CMD_LED_STATE_LED(l) ((l) & 0x0f) +#define CMD_LED_STATE_ON 0x10 + +#define CMD_LED_COLOR 5 +#define CMD_LED_SET_BRIGHTNESS 7 +#define CMD_LED_GET_BRIGHTNESS 8 + +#define OMNIA_CMD 0 + +#define OMNIA_CMD_LED_COLOR_LED 1 +#define OMNIA_CMD_LED_COLOR_R 2 +#define OMNIA_CMD_LED_COLOR_G 3 +#define OMNIA_CMD_LED_COLOR_B 4 +#define OMNIA_CMD_LED_COLOR_LEN 5 + +struct omnia_led { + struct led_classdev_mc mc_cdev; + struct mc_subled subled_info[OMNIA_LED_NUM_CHANNELS]; + int reg; +}; + +#define to_omnia_led(l) container_of(l, struct omnia_led, mc_cdev) + +struct omnia_leds { + struct i2c_client *client; + struct mutex lock; + struct omnia_led leds[]; +}; + +static int omnia_led_brightness_set_blocking(struct led_classdev *cdev, + enum led_brightness brightness) +{ + struct led_classdev_mc *mc_cdev = lcdev_to_mccdev(cdev); + struct omnia_leds *leds = dev_get_drvdata(cdev->dev->parent); + struct omnia_led *led = to_omnia_led(mc_cdev); + u8 buf[OMNIA_CMD_LED_COLOR_LEN], state; + int ret; + + mutex_lock(&leds->lock); + + led_mc_calc_color_components(&led->mc_cdev, brightness); + + buf[OMNIA_CMD] = CMD_LED_COLOR; + buf[OMNIA_CMD_LED_COLOR_LED] = led->reg; + buf[OMNIA_CMD_LED_COLOR_R] = mc_cdev->subled_info[0].brightness; + buf[OMNIA_CMD_LED_COLOR_G] = mc_cdev->subled_info[1].brightness; + buf[OMNIA_CMD_LED_COLOR_B] = mc_cdev->subled_info[2].brightness; + + state = CMD_LED_STATE_LED(led->reg); + if (buf[OMNIA_CMD_LED_COLOR_R] || buf[OMNIA_CMD_LED_COLOR_G] || buf[OMNIA_CMD_LED_COLOR_B]) + state |= CMD_LED_STATE_ON; + + ret = i2c_smbus_write_byte_data(leds->client, CMD_LED_STATE, state); + if (ret >= 0 && (state & CMD_LED_STATE_ON)) + ret = i2c_master_send(leds->client, buf, 5); + + mutex_unlock(&leds->lock); + + return ret; +} + +static int omnia_led_register(struct i2c_client *client, struct omnia_led *led, + struct device_node *np) +{ + struct led_init_data init_data = {}; + struct device *dev = &client->dev; + struct led_classdev *cdev; + int ret, color; + + ret = of_property_read_u32(np, "reg", &led->reg); + if (ret || led->reg >= OMNIA_BOARD_LEDS) { + dev_warn(dev, + "Node %pOF: must contain 'reg' property with values between 0 and %i\n", + np, OMNIA_BOARD_LEDS - 1); + return 0; + } + + ret = of_property_read_u32(np, "color", &color); + if (ret || color != LED_COLOR_ID_MULTI) { + dev_warn(dev, + "Node %pOF: must contain 'color' property with value LED_COLOR_ID_MULTI\n", + np); + return 0; + } + + led->subled_info[0].color_index = LED_COLOR_ID_RED; + led->subled_info[0].channel = 0; + led->subled_info[1].color_index = LED_COLOR_ID_GREEN; + led->subled_info[1].channel = 1; + led->subled_info[2].color_index = LED_COLOR_ID_BLUE; + led->subled_info[2].channel = 2; + + led->mc_cdev.subled_info = led->subled_info; + led->mc_cdev.num_colors = OMNIA_LED_NUM_CHANNELS; + + init_data.fwnode = &np->fwnode; + + cdev = &led->mc_cdev.led_cdev; + cdev->max_brightness = 255; + cdev->brightness_set_blocking = omnia_led_brightness_set_blocking; + + of_property_read_string(np, "linux,default-trigger", &cdev->default_trigger); + + /* put the LED into software mode */ + ret = i2c_smbus_write_byte_data(client, CMD_LED_MODE, + CMD_LED_MODE_LED(led->reg) | + CMD_LED_MODE_USER); + if (ret < 0) { + dev_err(dev, "Cannot set LED %pOF to software mode: %i\n", np, ret); + return ret; + } + + /* disable the LED */ + ret = i2c_smbus_write_byte_data(client, CMD_LED_STATE, CMD_LED_STATE_LED(led->reg)); + if (ret < 0) { + dev_err(dev, "Cannot set LED %pOF brightness: %i\n", np, ret); + return ret; + } + + ret = devm_led_classdev_multicolor_register_ext(dev, &led->mc_cdev, &init_data); + if (ret < 0) { + dev_err(dev, "Cannot register LED %pOF: %i\n", np, ret); + return ret; + } + + return 1; +} + +/* + * On the front panel of the Turris Omnia router there is also a button which + * can be used to control the intensity of all the LEDs at once, so that if they + * are too bright, user can dim them. + * The microcontroller cycles between 8 levels of this global brightness (from + * 100% to 0%), but this setting can have any integer value between 0 and 100. + * It is therefore convenient to be able to change this setting from software. + * We expose this setting via a sysfs attribute file called "brightness". This + * file lives in the device directory of the LED controller, not an individual + * LED, so it should not confuse users. + */ +static ssize_t brightness_show(struct device *dev, struct device_attribute *a, char *buf) +{ + struct i2c_client *client = to_i2c_client(dev); + struct omnia_leds *leds = i2c_get_clientdata(client); + int ret; + + mutex_lock(&leds->lock); + ret = i2c_smbus_read_byte_data(client, CMD_LED_GET_BRIGHTNESS); + mutex_unlock(&leds->lock); + + if (ret < 0) + return ret; + + return sprintf(buf, "%d\n", ret); +} + +static ssize_t brightness_store(struct device *dev, struct device_attribute *a, const char *buf, + size_t count) +{ + struct i2c_client *client = to_i2c_client(dev); + struct omnia_leds *leds = i2c_get_clientdata(client); + unsigned int brightness; + int ret; + + if (sscanf(buf, "%u", &brightness) != 1) + return -EINVAL; + + if (brightness > 100) + return -EINVAL; + + mutex_lock(&leds->lock); + ret = i2c_smbus_write_byte_data(client, CMD_LED_SET_BRIGHTNESS, (u8) brightness); + mutex_unlock(&leds->lock); + + if (ret < 0) + return ret; + + return count; +} +static DEVICE_ATTR_RW(brightness); + +static struct attribute *omnia_led_controller_attrs[] = { + &dev_attr_brightness.attr, + NULL, +}; +ATTRIBUTE_GROUPS(omnia_led_controller); + +static int omnia_leds_probe(struct i2c_client *client, + const struct i2c_device_id *id) +{ + struct device *dev = &client->dev; + struct device_node *np = dev->of_node, *child; + struct omnia_leds *leds; + struct omnia_led *led; + int ret, count; + + count = of_get_available_child_count(np); + if (!count) { + dev_err(dev, "LEDs are not defined in device tree!\n"); + return -ENODEV; + } else if (count > OMNIA_BOARD_LEDS) { + dev_err(dev, "Too many LEDs defined in device tree!\n"); + return -EINVAL; + } + + leds = devm_kzalloc(dev, struct_size(leds, leds, count), GFP_KERNEL); + if (!leds) + return -ENOMEM; + + leds->client = client; + i2c_set_clientdata(client, leds); + + mutex_init(&leds->lock); + + led = &leds->leds[0]; + for_each_available_child_of_node(np, child) { + ret = omnia_led_register(client, led, child); + if (ret < 0) + return ret; + + led += ret; + } + + if (devm_device_add_groups(dev, omnia_led_controller_groups)) + dev_warn(dev, "Could not add attribute group!\n"); + + return 0; +} + +static int omnia_leds_remove(struct i2c_client *client) +{ + u8 buf[OMNIA_CMD_LED_COLOR_LEN]; + + /* put all LEDs into default (HW triggered) mode */ + i2c_smbus_write_byte_data(client, CMD_LED_MODE, + CMD_LED_MODE_LED(OMNIA_BOARD_LEDS)); + + /* set all LEDs color to [255, 255, 255] */ + buf[OMNIA_CMD] = CMD_LED_COLOR; + buf[OMNIA_CMD_LED_COLOR_LED] = OMNIA_BOARD_LEDS; + buf[OMNIA_CMD_LED_COLOR_R] = 255; + buf[OMNIA_CMD_LED_COLOR_G] = 255; + buf[OMNIA_CMD_LED_COLOR_B] = 255; + + i2c_master_send(client, buf, 5); + + return 0; +} + +static const struct of_device_id of_omnia_leds_match[] = { + { .compatible = "cznic,turris-omnia-leds", }, + {}, +}; + +static const struct i2c_device_id omnia_id[] = { + { "omnia", 0 }, + { } +}; + +static struct i2c_driver omnia_leds_driver = { + .probe = omnia_leds_probe, + .remove = omnia_leds_remove, + .id_table = omnia_id, + .driver = { + .name = "leds-turris-omnia", + .of_match_table = of_omnia_leds_match, + }, +}; + +module_i2c_driver(omnia_leds_driver); + +MODULE_AUTHOR("Marek Behun <marek.behun@nic.cz>"); +MODULE_DESCRIPTION("CZ.NIC's Turris Omnia LEDs"); +MODULE_LICENSE("GPL v2"); -- 2.26.2 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v7 2/3] leds: initial support for Turris Omnia LEDs 2020-07-23 12:53 ` [PATCH v7 2/3] leds: initial support for Turris Omnia LEDs Marek Behún @ 2020-07-24 10:58 ` Pavel Machek 2020-07-24 12:08 ` Marek Behún 0 siblings, 1 reply; 8+ messages in thread From: Pavel Machek @ 2020-07-24 10:58 UTC (permalink / raw) To: Marek Behún; +Cc: linux-leds, jacek.anaszewski, Dan Murphy [-- Attachment #1: Type: text/plain, Size: 2839 bytes --] Hi! > This adds basic support for LEDs on the front side of CZ.NIC's Turris > Omnia router. > > There are 12 RGB LEDs. The controller supports HW triggering mode for > the LEDs, but this driver does not support it yet, and sets all the LEDs > defined in device-tree into SW mode upon probe. > > This driver uses the multicolor LED framework. > > Signed-off-by: Marek Behún <marek.behun@nic.cz> > Reviewed-by: Dan Murphy <dmurphy@ti.com> > --- > drivers/leds/Kconfig | 11 ++ > drivers/leds/Makefile | 1 + > drivers/leds/leds-turris-omnia.c | 295 +++++++++++++++++++++++++++++++ > 3 files changed, 307 insertions(+) > create mode 100644 drivers/leds/leds-turris-omnia.c Whoever told you to use defines here was evil: > +#define OMNIA_CMD_LED_COLOR_LED 1 > +#define OMNIA_CMD_LED_COLOR_R 2 > +#define OMNIA_CMD_LED_COLOR_G 3 > +#define OMNIA_CMD_LED_COLOR_B 4 > +#define OMNIA_CMD_LED_COLOR_LEN 5 > +static int omnia_led_brightness_set_blocking(struct led_classdev *cdev, > + enum led_brightness brightness) > +{ > + struct led_classdev_mc *mc_cdev = lcdev_to_mccdev(cdev); > + struct omnia_leds *leds = dev_get_drvdata(cdev->dev->parent); > + struct omnia_led *led = to_omnia_led(mc_cdev); > + u8 buf[OMNIA_CMD_LED_COLOR_LEN], state; > + int ret; > + > + mutex_lock(&leds->lock); > + > + led_mc_calc_color_components(&led->mc_cdev, brightness); > + > + buf[OMNIA_CMD] = CMD_LED_COLOR; > + buf[OMNIA_CMD_LED_COLOR_LED] = led->reg; > + buf[OMNIA_CMD_LED_COLOR_R] = mc_cdev->subled_info[0].brightness; > + buf[OMNIA_CMD_LED_COLOR_G] = mc_cdev->subled_info[1].brightness; > + buf[OMNIA_CMD_LED_COLOR_B] = mc_cdev->subled_info[2].brightness; Aha, it is LED vs LEN, but please don't obscure code with macros like that. It is important to see that buf[] is fully initialized, macros hide that. > + state = CMD_LED_STATE_LED(led->reg); > + if (buf[OMNIA_CMD_LED_COLOR_R] || buf[OMNIA_CMD_LED_COLOR_G] || buf[OMNIA_CMD_LED_COLOR_B]) > + state |= CMD_LED_STATE_ON; I'd make this conditional on mc_cdev->subled_info[0].brightness[x], not buf, but... > + int ret, count; > + > + count = of_get_available_child_count(np); > + if (!count) { > + dev_err(dev, "LEDs are not defined in device tree!\n"); > + return -ENODEV; > + } else if (count > OMNIA_BOARD_LEDS) { > + dev_err(dev, "Too many LEDs defined in device tree!\n"); > + return -EINVAL; > + } Should count be unsigned? Or better, should we check it is _exactly_ OMNIA_BOARD_LEDS as we only support this known hardware? [I suppose I'll apply it anyway; these can be fixed post-merge]. Best regards, Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 195 bytes --] ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v7 2/3] leds: initial support for Turris Omnia LEDs 2020-07-24 10:58 ` Pavel Machek @ 2020-07-24 12:08 ` Marek Behún 2020-07-24 14:56 ` Dan Murphy 0 siblings, 1 reply; 8+ messages in thread From: Marek Behún @ 2020-07-24 12:08 UTC (permalink / raw) To: Pavel Machek; +Cc: linux-leds, jacek.anaszewski, Dan Murphy On Fri, 24 Jul 2020 12:58:25 +0200 Pavel Machek <pavel@ucw.cz> wrote: > Hi! > > > This adds basic support for LEDs on the front side of CZ.NIC's > > Turris Omnia router. > > > > There are 12 RGB LEDs. The controller supports HW triggering mode > > for the LEDs, but this driver does not support it yet, and sets all > > the LEDs defined in device-tree into SW mode upon probe. > > > > This driver uses the multicolor LED framework. > > > > Signed-off-by: Marek Behún <marek.behun@nic.cz> > > Reviewed-by: Dan Murphy <dmurphy@ti.com> > > --- > > drivers/leds/Kconfig | 11 ++ > > drivers/leds/Makefile | 1 + > > drivers/leds/leds-turris-omnia.c | 295 > > +++++++++++++++++++++++++++++++ 3 files changed, 307 insertions(+) > > create mode 100644 drivers/leds/leds-turris-omnia.c > > Whoever told you to use defines here was evil: I think it was that way when it was first proposed, sometime last year, but Dan complained about magic numbers, so I changed it to macros :D > > +#define OMNIA_CMD_LED_COLOR_LED 1 > > +#define OMNIA_CMD_LED_COLOR_R 2 > > +#define OMNIA_CMD_LED_COLOR_G 3 > > +#define OMNIA_CMD_LED_COLOR_B 4 > > +#define OMNIA_CMD_LED_COLOR_LEN 5 > > > +static int omnia_led_brightness_set_blocking(struct led_classdev > > *cdev, > > + enum led_brightness > > brightness) +{ > > + struct led_classdev_mc *mc_cdev = lcdev_to_mccdev(cdev); > > + struct omnia_leds *leds = > > dev_get_drvdata(cdev->dev->parent); > > + struct omnia_led *led = to_omnia_led(mc_cdev); > > + u8 buf[OMNIA_CMD_LED_COLOR_LEN], state; > > + int ret; > > + > > + mutex_lock(&leds->lock); > > + > > + led_mc_calc_color_components(&led->mc_cdev, brightness); > > + > > + buf[OMNIA_CMD] = CMD_LED_COLOR; > > + buf[OMNIA_CMD_LED_COLOR_LED] = led->reg; > > + buf[OMNIA_CMD_LED_COLOR_R] = > > mc_cdev->subled_info[0].brightness; > > + buf[OMNIA_CMD_LED_COLOR_G] = > > mc_cdev->subled_info[1].brightness; > > + buf[OMNIA_CMD_LED_COLOR_B] = > > mc_cdev->subled_info[2].brightness; > > Aha, it is LED vs LEN, but please don't obscure code with macros like > that. It is important to see that buf[] is fully initialized, macros > hide that. > > > + state = CMD_LED_STATE_LED(led->reg); > > + if (buf[OMNIA_CMD_LED_COLOR_R] || > > buf[OMNIA_CMD_LED_COLOR_G] || buf[OMNIA_CMD_LED_COLOR_B]) > > + state |= CMD_LED_STATE_ON; > > I'd make this conditional on mc_cdev->subled_info[0].brightness[x], > not buf, but... > > > > + int ret, count; > > + > > + count = of_get_available_child_count(np); > > + if (!count) { > > + dev_err(dev, "LEDs are not defined in device > > tree!\n"); > > + return -ENODEV; > > + } else if (count > OMNIA_BOARD_LEDS) { > > + dev_err(dev, "Too many LEDs defined in device > > tree!\n"); > > + return -EINVAL; > > + } > > Should count be unsigned? > > Or better, should we check it is _exactly_ OMNIA_BOARD_LEDS as we only > support this known hardware? I think it shouldn't be an error when fewer than OMNIA_BOARD_LEDS are in device tree... > [I suppose I'll apply it anyway; these can be fixed post-merge]. As you wish. I can sent follow-up patch afterwards for to remove those macros... > Best regards, > Pavel ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v7 2/3] leds: initial support for Turris Omnia LEDs 2020-07-24 12:08 ` Marek Behún @ 2020-07-24 14:56 ` Dan Murphy 0 siblings, 0 replies; 8+ messages in thread From: Dan Murphy @ 2020-07-24 14:56 UTC (permalink / raw) To: Marek Behún, Pavel Machek; +Cc: linux-leds, jacek.anaszewski Marek On 7/24/20 7:08 AM, Marek Behún wrote: > On Fri, 24 Jul 2020 12:58:25 +0200 > Pavel Machek <pavel@ucw.cz> wrote: > >> Hi! >> >>> This adds basic support for LEDs on the front side of CZ.NIC's >>> Turris Omnia router. >>> >>> There are 12 RGB LEDs. The controller supports HW triggering mode >>> for the LEDs, but this driver does not support it yet, and sets all >>> the LEDs defined in device-tree into SW mode upon probe. >>> >>> This driver uses the multicolor LED framework. >>> >>> Signed-off-by: Marek Behún <marek.behun@nic.cz> >>> Reviewed-by: Dan Murphy <dmurphy@ti.com> >>> --- >>> drivers/leds/Kconfig | 11 ++ >>> drivers/leds/Makefile | 1 + >>> drivers/leds/leds-turris-omnia.c | 295 >>> +++++++++++++++++++++++++++++++ 3 files changed, 307 insertions(+) >>> create mode 100644 drivers/leds/leds-turris-omnia.c >> Whoever told you to use defines here was evil: > I think it was that way when it was first proposed, sometime last year, > but Dan complained about magic numbers, so I changed it to macros :D Ah yes I did make a comment about magic numbers but did not explicitly ask for macros, enum would have sufficed and I was more concerned about the buffer length being consistent in the file since it was hard coded twice in two different functions. So I am only partially evil. Dan ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v7 3/3] Documentation: ABI: leds-turris-omnia: document sysfs attribute 2020-07-23 12:53 [PATCH v7 0/3] Add Turris Omnia LEDs driver Marek Behún 2020-07-23 12:53 ` [PATCH v7 1/3] dt-bindings: leds: add cznic,turris-omnia-leds binding Marek Behún 2020-07-23 12:53 ` [PATCH v7 2/3] leds: initial support for Turris Omnia LEDs Marek Behún @ 2020-07-23 12:53 ` Marek Behún 2 siblings, 0 replies; 8+ messages in thread From: Marek Behún @ 2020-07-23 12:53 UTC (permalink / raw) To: linux-leds; +Cc: Pavel Machek, jacek.anaszewski, Dan Murphy, Marek Behún Document the global brightness attribute for the Turris Omnia LED controller. Signed-off-by: Marek Behún <marek.behun@nic.cz> --- .../testing/sysfs-class-led-driver-turris-omnia | 14 ++++++++++++++ 1 file changed, 14 insertions(+) create mode 100644 Documentation/ABI/testing/sysfs-class-led-driver-turris-omnia diff --git a/Documentation/ABI/testing/sysfs-class-led-driver-turris-omnia b/Documentation/ABI/testing/sysfs-class-led-driver-turris-omnia new file mode 100644 index 000000000000..795a5de12fc1 --- /dev/null +++ b/Documentation/ABI/testing/sysfs-class-led-driver-turris-omnia @@ -0,0 +1,14 @@ +What: /sys/class/leds/<led>/device/brightness +Date: July 2020 +KernelVersion: 5.9 +Contact: Marek Behún <marek.behun@nic.cz> +Description: (RW) On the front panel of the Turris Omnia router there is also + a button which can be used to control the intensity of all the + LEDs at once, so that if they are too bright, user can dim them. + + The microcontroller cycles between 8 levels of this global + brightness (from 100% to 0%), but this setting can have any + integer value between 0 and 100. It is therefore convenient to be + able to change this setting from software. + + Format: %i -- 2.26.2 ^ permalink raw reply related [flat|nested] 8+ messages in thread
end of thread, other threads:[~2020-07-28 17:50 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-07-23 12:53 [PATCH v7 0/3] Add Turris Omnia LEDs driver Marek Behún 2020-07-23 12:53 ` [PATCH v7 1/3] dt-bindings: leds: add cznic,turris-omnia-leds binding Marek Behún 2020-07-28 17:49 ` Rob Herring 2020-07-23 12:53 ` [PATCH v7 2/3] leds: initial support for Turris Omnia LEDs Marek Behún 2020-07-24 10:58 ` Pavel Machek 2020-07-24 12:08 ` Marek Behún 2020-07-24 14:56 ` Dan Murphy 2020-07-23 12:53 ` [PATCH v7 3/3] Documentation: ABI: leds-turris-omnia: document sysfs attribute Marek Behún
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).