* [PATCH v4 0/2] leds: Add a driver for the BD2606MVV @ 2023-04-14 5:53 Andreas Kemnade 2023-04-14 5:53 ` [PATCH v4 1/2] dt-bindings: leds: Add ROHM BD2606MVV LED driver Andreas Kemnade 2023-04-14 5:53 ` [PATCH v4 2/2] leds: bd2606mvv: Driver for the Rohm 6 Channel i2c " Andreas Kemnade 0 siblings, 2 replies; 12+ messages in thread From: Andreas Kemnade @ 2023-04-14 5:53 UTC (permalink / raw) To: pavel, lee, robh+dt, krzysztof.kozlowski+dt, andreas, linux-leds, devicetree, linux-kernel, Matti Vaittinen Add the binding description and the corresponding driver for the BD2606. Changes in V4: - minor tuning of description in bindings Changes in V3: - binding cleanup - move active variable from long-living struct onto stack Changes in V2: - Add Datasheet link - use fwnode api - remove childnode count check, that will bail out anyways later. - add enable-gpios to binding but not to driver due to lack of testing ability Andreas Kemnade (2): dt-bindings: leds: Add ROHM BD2606MVV LED driver leds: bd2606mvv: Driver for the Rohm 6 Channel i2c LED driver .../bindings/leds/rohm,bd2606mvv.yaml | 81 ++++++++++ drivers/leds/Kconfig | 11 ++ drivers/leds/Makefile | 1 + drivers/leds/leds-bd2606mvv.c | 143 ++++++++++++++++++ 4 files changed, 236 insertions(+) create mode 100644 Documentation/devicetree/bindings/leds/rohm,bd2606mvv.yaml create mode 100644 drivers/leds/leds-bd2606mvv.c -- 2.39.2 ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v4 1/2] dt-bindings: leds: Add ROHM BD2606MVV LED driver 2023-04-14 5:53 [PATCH v4 0/2] leds: Add a driver for the BD2606MVV Andreas Kemnade @ 2023-04-14 5:53 ` Andreas Kemnade 2023-04-14 15:28 ` Krzysztof Kozlowski 2023-04-14 5:53 ` [PATCH v4 2/2] leds: bd2606mvv: Driver for the Rohm 6 Channel i2c " Andreas Kemnade 1 sibling, 1 reply; 12+ messages in thread From: Andreas Kemnade @ 2023-04-14 5:53 UTC (permalink / raw) To: pavel, lee, robh+dt, krzysztof.kozlowski+dt, andreas, linux-leds, devicetree, linux-kernel, Matti Vaittinen Document ROHM BD2606MVV LED driver devicetree bindings. Signed-off-by: Andreas Kemnade <andreas@kemnade.info> Reviewed-by: Matti Vaittinen <mazziesaccount@gmail.com> --- .../bindings/leds/rohm,bd2606mvv.yaml | 81 +++++++++++++++++++ 1 file changed, 81 insertions(+) create mode 100644 Documentation/devicetree/bindings/leds/rohm,bd2606mvv.yaml diff --git a/Documentation/devicetree/bindings/leds/rohm,bd2606mvv.yaml b/Documentation/devicetree/bindings/leds/rohm,bd2606mvv.yaml new file mode 100644 index 0000000000000..14700a2e5feaa --- /dev/null +++ b/Documentation/devicetree/bindings/leds/rohm,bd2606mvv.yaml @@ -0,0 +1,81 @@ +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/leds/rohm,bd2606mvv.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: ROHM BD2606MVV LED controller + +maintainers: + - Andreas Kemnade <andreas@kemnade.info> + +description: + The BD2606 MVV is a programmable LED controller connected via I2C that can + drive 6 separate lines. Each of them can be individually switched on and off, + but the brightness setting is shared between pairs of them. + + Datasheet is available at + https://fscdn.rohm.com/en/products/databook/datasheet/ic/power/led_driver/bd2606mvv_1-e.pdf + +properties: + compatible: + const: rohm,bd2606mvv + + reg: + maxItems: 1 + + "#address-cells": + const: 1 + + "#size-cells": + const: 0 + + enable-gpios: + maxItems: 1 + description: GPIO pin to enable/disable the device. + +patternProperties: + "^led@[0-6]$": + type: object + $ref: common.yaml# + unevaluatedProperties: false + + properties: + reg: + minimum: 0 + maximum: 6 + + required: + - reg + +additionalProperties: false + +examples: + - | + #include <dt-bindings/leds/common.h> + + i2c { + #address-cells = <1>; + #size-cells = <0>; + + led-controller@66 { + compatible = "rohm,bd2606mvv"; + reg = <0x66>; + #address-cells = <1>; + #size-cells = <0>; + + led@0 { + reg = <0x0>; + color = <LED_COLOR_ID_RED>; + function = LED_FUNCTION_POWER; + }; + + led@2 { + reg = <0x2>; + color = <LED_COLOR_ID_WHITE>; + function = LED_FUNCTION_STATUS; + }; + }; + }; + +... -- 2.39.2 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v4 1/2] dt-bindings: leds: Add ROHM BD2606MVV LED driver 2023-04-14 5:53 ` [PATCH v4 1/2] dt-bindings: leds: Add ROHM BD2606MVV LED driver Andreas Kemnade @ 2023-04-14 15:28 ` Krzysztof Kozlowski 2023-04-14 15:54 ` Andreas Kemnade 0 siblings, 1 reply; 12+ messages in thread From: Krzysztof Kozlowski @ 2023-04-14 15:28 UTC (permalink / raw) To: Andreas Kemnade, pavel, lee, robh+dt, krzysztof.kozlowski+dt, linux-leds, devicetree, linux-kernel, Matti Vaittinen On 14/04/2023 07:53, Andreas Kemnade wrote: > Document ROHM BD2606MVV LED driver devicetree bindings. Subject: no improvements and no comments from your side. Why? > > Signed-off-by: Andreas Kemnade <andreas@kemnade.info> > Reviewed-by: Matti Vaittinen <mazziesaccount@gmail.com> > --- > .../bindings/leds/rohm,bd2606mvv.yaml | 81 +++++++++++++++++++ > 1 file changed, 81 insertions(+) > create mode 100644 Documentation/devicetree/bindings/leds/rohm,bd2606mvv.yaml > > diff --git a/Documentation/devicetree/bindings/leds/rohm,bd2606mvv.yaml b/Documentation/devicetree/bindings/leds/rohm,bd2606mvv.yaml > new file mode 100644 > index 0000000000000..14700a2e5feaa > --- /dev/null > +++ b/Documentation/devicetree/bindings/leds/rohm,bd2606mvv.yaml > @@ -0,0 +1,81 @@ > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/leds/rohm,bd2606mvv.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: ROHM BD2606MVV LED controller > + > +maintainers: > + - Andreas Kemnade <andreas@kemnade.info> > + > +description: > + The BD2606 MVV is a programmable LED controller connected via I2C that can > + drive 6 separate lines. Each of them can be individually switched on and off, > + but the brightness setting is shared between pairs of them. > + > + Datasheet is available at > + https://fscdn.rohm.com/en/products/databook/datasheet/ic/power/led_driver/bd2606mvv_1-e.pdf > + > +properties: > + compatible: > + const: rohm,bd2606mvv > + > + reg: > + maxItems: 1 > + > + "#address-cells": > + const: 1 > + > + "#size-cells": > + const: 0 > + > + enable-gpios: > + maxItems: 1 Drop maxItems. It cannot be different. > + description: GPIO pin to enable/disable the device. > + Best regards, Krzysztof ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v4 1/2] dt-bindings: leds: Add ROHM BD2606MVV LED driver 2023-04-14 15:28 ` Krzysztof Kozlowski @ 2023-04-14 15:54 ` Andreas Kemnade 2023-04-14 15:56 ` Andreas Kemnade 2023-04-14 21:17 ` Krzysztof Kozlowski 0 siblings, 2 replies; 12+ messages in thread From: Andreas Kemnade @ 2023-04-14 15:54 UTC (permalink / raw) To: Krzysztof Kozlowski Cc: pavel, lee, robh+dt, krzysztof.kozlowski+dt, linux-leds, devicetree, linux-kernel, Matti Vaittinen On Fri, 14 Apr 2023 17:28:49 +0200 Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > On 14/04/2023 07:53, Andreas Kemnade wrote: > > Document ROHM BD2606MVV LED driver devicetree bindings. > > Subject: no improvements and no comments from your side. Why? > old subject (v2): dt-bindings: leds: ROHM BD2606MVV LED driver Your comment: Subject: maybe drop "driver" (suggests it is for Linux drivers, although maybe it matches the actual hardware here?) and add missing verb, e.g. "Add ROHM ..." New Subject (v3/4): dt-bindings: leds: Add ROHM BD2606MVV LED driver What is still missing? Regards, Andreas ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v4 1/2] dt-bindings: leds: Add ROHM BD2606MVV LED driver 2023-04-14 15:54 ` Andreas Kemnade @ 2023-04-14 15:56 ` Andreas Kemnade 2023-04-14 21:17 ` Krzysztof Kozlowski 1 sibling, 0 replies; 12+ messages in thread From: Andreas Kemnade @ 2023-04-14 15:56 UTC (permalink / raw) To: Krzysztof Kozlowski Cc: pavel, lee, robh+dt, krzysztof.kozlowski+dt, linux-leds, devicetree, linux-kernel, Matti Vaittinen On Fri, 14 Apr 2023 17:54:12 +0200 Andreas Kemnade <andreas@kemnade.info> wrote: > On Fri, 14 Apr 2023 17:28:49 +0200 > Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > > > On 14/04/2023 07:53, Andreas Kemnade wrote: > > > Document ROHM BD2606MVV LED driver devicetree bindings. > > > > Subject: no improvements and no comments from your side. Why? > > > old subject (v2): > > dt-bindings: leds: ROHM BD2606MVV LED driver > > Your comment: > Subject: maybe drop "driver" (suggests it is for Linux drivers, although > maybe it matches the actual hardware here?) and add missing verb, e.g. > "Add ROHM ..." > > New Subject (v3/4): > dt-bindings: leds: Add ROHM BD2606MVV LED driver > > What is still missing? > ok, answering myself: missing verb was added ("Add") but s/driver// is missing Sorry. Regards, Andreas ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v4 1/2] dt-bindings: leds: Add ROHM BD2606MVV LED driver 2023-04-14 15:54 ` Andreas Kemnade 2023-04-14 15:56 ` Andreas Kemnade @ 2023-04-14 21:17 ` Krzysztof Kozlowski 2023-04-16 21:25 ` Andreas Kemnade 1 sibling, 1 reply; 12+ messages in thread From: Krzysztof Kozlowski @ 2023-04-14 21:17 UTC (permalink / raw) To: Andreas Kemnade Cc: pavel, lee, robh+dt, krzysztof.kozlowski+dt, linux-leds, devicetree, linux-kernel, Matti Vaittinen On 14/04/2023 17:54, Andreas Kemnade wrote: > On Fri, 14 Apr 2023 17:28:49 +0200 > Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > >> On 14/04/2023 07:53, Andreas Kemnade wrote: >>> Document ROHM BD2606MVV LED driver devicetree bindings. >> >> Subject: no improvements and no comments from your side. Why? >> > old subject (v2): > > dt-bindings: leds: ROHM BD2606MVV LED driver > > Your comment: > Subject: maybe drop "driver" (suggests it is for Linux drivers, although > maybe it matches the actual hardware here?) and add missing verb, e.g. > "Add ROHM ..." > > New Subject (v3/4): > dt-bindings: leds: Add ROHM BD2606MVV LED driver > > What is still missing? There is still "driver". Comment was: drop "driver". Where is it dropped? If you do not agree, sure, just respond with something. Best regards, Krzysztof ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v4 1/2] dt-bindings: leds: Add ROHM BD2606MVV LED driver 2023-04-14 21:17 ` Krzysztof Kozlowski @ 2023-04-16 21:25 ` Andreas Kemnade 0 siblings, 0 replies; 12+ messages in thread From: Andreas Kemnade @ 2023-04-16 21:25 UTC (permalink / raw) To: Krzysztof Kozlowski Cc: pavel, lee, robh+dt, krzysztof.kozlowski+dt, linux-leds, devicetree, linux-kernel, Matti Vaittinen Hi, On Fri, 14 Apr 2023 23:17:56 +0200 Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > On 14/04/2023 17:54, Andreas Kemnade wrote: > > On Fri, 14 Apr 2023 17:28:49 +0200 > > Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > > > >> On 14/04/2023 07:53, Andreas Kemnade wrote: > >>> Document ROHM BD2606MVV LED driver devicetree bindings. > >> > >> Subject: no improvements and no comments from your side. Why? > >> > > old subject (v2): > > > > dt-bindings: leds: ROHM BD2606MVV LED driver > > > > Your comment: > > Subject: maybe drop "driver" (suggests it is for Linux drivers, although > > maybe it matches the actual hardware here?) and add missing verb, e.g. > > "Add ROHM ..." > > > > New Subject (v3/4): > > dt-bindings: leds: Add ROHM BD2606MVV LED driver > > > > What is still missing? > > There is still "driver". Comment was: drop "driver". Where is it dropped? > > If you do not agree, sure, just respond with something. > I am fine with both. On one hand BD2606MVV is not a LED by itself but LEDs can be connected to it. so the chip itself can be called LED driver. But on the other hand I think that holds true for everything in drivers/leds and binding/leds and we do not call the subsystem leddriver. So there are reasons for and against "driver" in the subject line. Regards, Andreas ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v4 2/2] leds: bd2606mvv: Driver for the Rohm 6 Channel i2c LED driver 2023-04-14 5:53 [PATCH v4 0/2] leds: Add a driver for the BD2606MVV Andreas Kemnade 2023-04-14 5:53 ` [PATCH v4 1/2] dt-bindings: leds: Add ROHM BD2606MVV LED driver Andreas Kemnade @ 2023-04-14 5:53 ` Andreas Kemnade 2023-04-14 12:18 ` Pavel Machek 1 sibling, 1 reply; 12+ messages in thread From: Andreas Kemnade @ 2023-04-14 5:53 UTC (permalink / raw) To: pavel, lee, robh+dt, krzysztof.kozlowski+dt, andreas, linux-leds, devicetree, linux-kernel, Matti Vaittinen The device provides 6 channels which can be individually turned off and on but groups of two channels share a common brightness register. Limitation: The GPIO to enable the device is not used yet. Signed-off-by: Andreas Kemnade <andreas@kemnade.info> Reviewed-by: Matti Vaittinen <mazziesaccount@gmail.com> --- drivers/leds/Kconfig | 11 +++ drivers/leds/Makefile | 1 + drivers/leds/leds-bd2606mvv.c | 143 ++++++++++++++++++++++++++++++++++ 3 files changed, 155 insertions(+) create mode 100644 drivers/leds/leds-bd2606mvv.c diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig index 9dbce09eabacf..cc4eadbb2542e 100644 --- a/drivers/leds/Kconfig +++ b/drivers/leds/Kconfig @@ -551,6 +551,17 @@ config LEDS_REGULATOR help This option enables support for regulator driven LEDs. +config LEDS_BD2606MVV + tristate "LED driver for BD2606MVV" + depends on LEDS_CLASS + depends on I2C + select REGMAP_I2C + help + This option enables support for BD2606MVV LED driver chips + accessed via the I2C bus. It supports setting brightness, with + the limitiation that there are groups of two channels sharing + a brightness setting, but not the on/off setting. + config LEDS_BD2802 tristate "LED driver for BD2802 RGB LED" depends on LEDS_CLASS diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile index d30395d11fd84..c07d1512c745a 100644 --- a/drivers/leds/Makefile +++ b/drivers/leds/Makefile @@ -17,6 +17,7 @@ obj-$(CONFIG_LEDS_ARIEL) += leds-ariel.o obj-$(CONFIG_LEDS_AW2013) += leds-aw2013.o obj-$(CONFIG_LEDS_BCM6328) += leds-bcm6328.o obj-$(CONFIG_LEDS_BCM6358) += leds-bcm6358.o +obj-$(CONFIG_LEDS_BD2606MVV) += leds-bd2606mvv.o obj-$(CONFIG_LEDS_BD2802) += leds-bd2802.o obj-$(CONFIG_LEDS_BLINKM) += leds-blinkm.o obj-$(CONFIG_LEDS_CLEVO_MAIL) += leds-clevo-mail.o diff --git a/drivers/leds/leds-bd2606mvv.c b/drivers/leds/leds-bd2606mvv.c new file mode 100644 index 0000000000000..81ceeaa434494 --- /dev/null +++ b/drivers/leds/leds-bd2606mvv.c @@ -0,0 +1,143 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * Copyright (C) 2023 Andreas Kemnade + * + * Datasheet: + * https://fscdn.rohm.com/en/products/databook/datasheet/ic/power/led_driver/bd2606mvv_1-e.pdf + */ + +#include <linux/i2c.h> +#include <linux/leds.h> +#include <linux/module.h> +#include <linux/mod_devicetable.h> +#include <linux/property.h> +#include <linux/regmap.h> +#include <linux/slab.h> + +#define BD2606_MAX_LEDS 6 +#define BD2606_MAX_BRIGHTNESS 63 +#define BD2606_REG_PWRCNT 3 +#define ldev_to_led(c) container_of(c, struct bd2606mvv_led, ldev) + +struct bd2606mvv_led { + unsigned int led_no; + struct led_classdev ldev; + struct bd2606mvv_priv *priv; +}; + +struct bd2606mvv_priv { + struct bd2606mvv_led leds[BD2606_MAX_LEDS]; + struct regmap *regmap; +}; + +static int +bd2606mvv_brightness_set(struct led_classdev *led_cdev, + enum led_brightness brightness) +{ + struct bd2606mvv_led *led = ldev_to_led(led_cdev); + struct bd2606mvv_priv *priv = led->priv; + int err; + + if (brightness == 0) + return regmap_update_bits(priv->regmap, + BD2606_REG_PWRCNT, + 1 << led->led_no, + 0); + + /* shared brightness register */ + err = regmap_write(priv->regmap, led->led_no / 2, + brightness); + if (err) + return err; + + return regmap_update_bits(priv->regmap, + BD2606_REG_PWRCNT, + 1 << led->led_no, + 1 << led->led_no); +} + +static const struct regmap_config bd2606mvv_regmap = { + .reg_bits = 8, + .val_bits = 8, + .max_register = 0x3, +}; + +static int bd2606mvv_probe(struct i2c_client *client) +{ + struct fwnode_handle *np, *child; + struct device *dev = &client->dev; + struct bd2606mvv_priv *priv; + bool active[BD2606_MAX_LEDS] = { 0 }; + int err, reg; + + np = dev_fwnode(dev); + if (!np) + return -ENODEV; + + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL); + if (!priv) + return -ENOMEM; + + priv->regmap = devm_regmap_init_i2c(client, &bd2606mvv_regmap); + if (IS_ERR(priv->regmap)) { + err = PTR_ERR(priv->regmap); + dev_err(dev, "Failed to allocate register map: %d\n", err); + return err; + } + + i2c_set_clientdata(client, priv); + + fwnode_for_each_available_child_node(np, child) { + struct bd2606mvv_led *led; + struct led_init_data init_data = {}; + + init_data.fwnode = child; + + err = fwnode_property_read_u32(child, "reg", ®); + if (err) { + fwnode_handle_put(child); + return err; + } + if (reg < 0 || reg >= BD2606_MAX_LEDS || + active[reg]) { + fwnode_handle_put(child); + return -EINVAL; + } + led = &priv->leds[reg]; + + active[reg] = true; + led->priv = priv; + led->led_no = reg; + led->ldev.brightness_set_blocking = bd2606mvv_brightness_set; + led->ldev.max_brightness = BD2606_MAX_BRIGHTNESS; + err = devm_led_classdev_register_ext(dev, &led->ldev, + &init_data); + if (err < 0) { + fwnode_handle_put(child); + return dev_err_probe(dev, err, + "couldn't register LED %s\n", + led->ldev.name); + } + } + return 0; +} + +static const struct of_device_id __maybe_unused of_bd2606mvv_leds_match[] = { + { .compatible = "rohm,bd2606mvv", }, + {}, +}; +MODULE_DEVICE_TABLE(of, of_bd2606mvv_leds_match); + +static struct i2c_driver bd2606mvv_driver = { + .driver = { + .name = "leds-bd2606mvv", + .of_match_table = of_match_ptr(of_bd2606mvv_leds_match), + }, + .probe_new = bd2606mvv_probe, +}; + +module_i2c_driver(bd2606mvv_driver); + +MODULE_AUTHOR("Andreas Kemnade <andreas@kemnade.info>"); +MODULE_DESCRIPTION("BD2606 LED driver"); +MODULE_LICENSE("GPL"); -- 2.39.2 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v4 2/2] leds: bd2606mvv: Driver for the Rohm 6 Channel i2c LED driver 2023-04-14 5:53 ` [PATCH v4 2/2] leds: bd2606mvv: Driver for the Rohm 6 Channel i2c " Andreas Kemnade @ 2023-04-14 12:18 ` Pavel Machek 2023-04-14 22:05 ` Andreas Kemnade 0 siblings, 1 reply; 12+ messages in thread From: Pavel Machek @ 2023-04-14 12:18 UTC (permalink / raw) To: Andreas Kemnade Cc: lee, robh+dt, krzysztof.kozlowski+dt, linux-leds, devicetree, linux-kernel, Matti Vaittinen [-- Attachment #1: Type: text/plain, Size: 1559 bytes --] Hi! > The device provides 6 channels which can be individually > turned off and on but groups of two channels share a common brightness > register. Yeah, well.. Turn it into 3-channel controller with brightness or 6-channel on/off one... You can't really share brightness. > +++ b/drivers/leds/Kconfig > @@ -551,6 +551,17 @@ config LEDS_REGULATOR > help > This option enables support for regulator driven LEDs. > > +config LEDS_BD2606MVV > + tristate "LED driver for BD2606MVV" > + depends on LEDS_CLASS > + depends on I2C > + select REGMAP_I2C > + help > + This option enables support for BD2606MVV LED driver chips > + accessed via the I2C bus. It supports setting brightness, with > + the limitiation that there are groups of two channels sharing > + a brightness setting, but not the on/off setting. > + This driver can be used as a module... > +static int > +bd2606mvv_brightness_set(struct led_classdev *led_cdev, > + enum led_brightness brightness) > +{ > + struct bd2606mvv_led *led = ldev_to_led(led_cdev); > + struct bd2606mvv_priv *priv = led->priv; > + int err; > + > + if (brightness == 0) > + return regmap_update_bits(priv->regmap, > + BD2606_REG_PWRCNT, > + 1 << led->led_no, > + 0); > + > + /* shared brightness register */ > + err = regmap_write(priv->regmap, led->led_no / 2, > + brightness); > + if (err) > + return err; Yeah. No. Best regards, Pavel -- People of Russia, stop Putin before his war on Ukraine escalates. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 195 bytes --] ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v4 2/2] leds: bd2606mvv: Driver for the Rohm 6 Channel i2c LED driver 2023-04-14 12:18 ` Pavel Machek @ 2023-04-14 22:05 ` Andreas Kemnade 2023-04-15 8:01 ` Pavel Machek 2023-04-17 12:32 ` Pavel Machek 0 siblings, 2 replies; 12+ messages in thread From: Andreas Kemnade @ 2023-04-14 22:05 UTC (permalink / raw) To: Pavel Machek Cc: lee, robh+dt, krzysztof.kozlowski+dt, linux-leds, devicetree, linux-kernel, Matti Vaittinen Hi Pavel, On Fri, 14 Apr 2023 14:18:56 +0200 Pavel Machek <pavel@ucw.cz> wrote: > Hi! > > > The device provides 6 channels which can be individually > > turned off and on but groups of two channels share a common brightness > > register. > > Yeah, well.. Turn it into 3-channel controller with brightness or > 6-channel on/off one... You can't really share brightness. > No, I cannot change the hardware, so it is a 6-channel with limitations. And the devicetree has to describe the hardware and not the driver. What is discussable is just how the driver should deal with that: I see 5 possibilities. a) ignore the shared brightness problem (status quo) b) never set a brightness other than full on/off c) ignore one led of each pair (not register it at all{ d) couple also the on/off of the pairs, so present to userspace only max. 3 leds. e) allow full brightness control where independently possible, if LEDs are defined where that leads to conflicts, register them with max_brightness=1 and use them in on/off mode. My preference were a) or e), the most possible usages. e) has a cleaner interface to the userspace. I would not upstream b) Regards, Andreas > > +++ b/drivers/leds/Kconfig > > @@ -551,6 +551,17 @@ config LEDS_REGULATOR > > help > > This option enables support for regulator driven LEDs. > > > > +config LEDS_BD2606MVV > > + tristate "LED driver for BD2606MVV" > > + depends on LEDS_CLASS > > + depends on I2C > > + select REGMAP_I2C > > + help > > + This option enables support for BD2606MVV LED driver chips > > + accessed via the I2C bus. It supports setting brightness, with > > + the limitiation that there are groups of two channels sharing > > + a brightness setting, but not the on/off setting. > > + > > This driver can be used as a module... > > > > +static int > > +bd2606mvv_brightness_set(struct led_classdev *led_cdev, > > + enum led_brightness brightness) > > +{ > > + struct bd2606mvv_led *led = ldev_to_led(led_cdev); > > + struct bd2606mvv_priv *priv = led->priv; > > + int err; > > + > > + if (brightness == 0) > > + return regmap_update_bits(priv->regmap, > > + BD2606_REG_PWRCNT, > > + 1 << led->led_no, > > + 0); > > + > > + /* shared brightness register */ > > + err = regmap_write(priv->regmap, led->led_no / 2, > > + brightness); > > + if (err) > > + return err; > > Yeah. No. > > Best regards, > Pavel ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v4 2/2] leds: bd2606mvv: Driver for the Rohm 6 Channel i2c LED driver 2023-04-14 22:05 ` Andreas Kemnade @ 2023-04-15 8:01 ` Pavel Machek 2023-04-17 12:32 ` Pavel Machek 1 sibling, 0 replies; 12+ messages in thread From: Pavel Machek @ 2023-04-15 8:01 UTC (permalink / raw) To: Andreas Kemnade Cc: lee, robh+dt, krzysztof.kozlowski+dt, linux-leds, devicetree, linux-kernel, Matti Vaittinen [-- Attachment #1: Type: text/plain, Size: 1335 bytes --] Hi! > > > The device provides 6 channels which can be individually > > > turned off and on but groups of two channels share a common brightness > > > register. > > > > Yeah, well.. Turn it into 3-channel controller with brightness or > > 6-channel on/off one... You can't really share brightness. > > > No, I cannot change the hardware, so it is a 6-channel with limitations. > And the devicetree has to describe the hardware and not the driver. Device tree is okay, I commented on the driver. > What is discussable is just how the driver should deal with that: > > I see 5 possibilities. > a) ignore the shared brightness problem (status quo) > b) never set a brightness other than full on/off > c) ignore one led of each pair (not register it at all{ > d) couple also the on/off of the pairs, so present to > userspace only max. 3 leds. > e) allow full brightness control where independently possible, > if LEDs are defined where that leads to conflicts, > register them with max_brightness=1 and use them > in on/off mode. > > My preference were a) or e), the most possible usages. > e) has a cleaner interface to the userspace. b) c) e) are acceptable to me. So I guess e) is preffered. BR, Pavel -- People of Russia, stop Putin before his war on Ukraine escalates. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 195 bytes --] ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v4 2/2] leds: bd2606mvv: Driver for the Rohm 6 Channel i2c LED driver 2023-04-14 22:05 ` Andreas Kemnade 2023-04-15 8:01 ` Pavel Machek @ 2023-04-17 12:32 ` Pavel Machek 1 sibling, 0 replies; 12+ messages in thread From: Pavel Machek @ 2023-04-17 12:32 UTC (permalink / raw) To: Andreas Kemnade Cc: lee, robh+dt, krzysztof.kozlowski+dt, linux-leds, devicetree, linux-kernel, Matti Vaittinen [-- Attachment #1: Type: text/plain, Size: 653 bytes --] Hi! > > > +config LEDS_BD2606MVV > > > + tristate "LED driver for BD2606MVV" > > > + depends on LEDS_CLASS > > > + depends on I2C > > > + select REGMAP_I2C > > > + help > > > + This option enables support for BD2606MVV LED driver chips > > > + accessed via the I2C bus. It supports setting brightness, with > > > + the limitiation that there are groups of two channels sharing > > > + a brightness setting, but not the on/off setting. > > > + > > > > This driver can be used as a module... And I was pointing this out before... BR, Pavel -- People of Russia, stop Putin before his war on Ukraine escalates. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 195 bytes --] ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2023-04-17 12:32 UTC | newest] Thread overview: 12+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-04-14 5:53 [PATCH v4 0/2] leds: Add a driver for the BD2606MVV Andreas Kemnade 2023-04-14 5:53 ` [PATCH v4 1/2] dt-bindings: leds: Add ROHM BD2606MVV LED driver Andreas Kemnade 2023-04-14 15:28 ` Krzysztof Kozlowski 2023-04-14 15:54 ` Andreas Kemnade 2023-04-14 15:56 ` Andreas Kemnade 2023-04-14 21:17 ` Krzysztof Kozlowski 2023-04-16 21:25 ` Andreas Kemnade 2023-04-14 5:53 ` [PATCH v4 2/2] leds: bd2606mvv: Driver for the Rohm 6 Channel i2c " Andreas Kemnade 2023-04-14 12:18 ` Pavel Machek 2023-04-14 22:05 ` Andreas Kemnade 2023-04-15 8:01 ` Pavel Machek 2023-04-17 12:32 ` Pavel Machek
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).