From 67bce62ac1959d8b7d731fc201694942f50a01b7 Mon Sep 17 00:00:00 2001 From: Randy Dunlap Date: Thu, 14 Jul 2022 18:59:48 -0700 Subject: [PATCH 01/23] leds: clevo-mail: fix Kconfig "its" grammar Use the possessive "its" instead of the contraction "it's" where appropriate. Signed-off-by: Randy Dunlap Cc: Pavel Machek Cc: Lee Jones Cc: linux-leds@vger.kernel.org Signed-off-by: Pavel Machek --- drivers/leds/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig index a49979f41eee7..acd4f754c9192 100644 --- a/drivers/leds/Kconfig +++ b/drivers/leds/Kconfig @@ -456,7 +456,7 @@ config LEDS_CLEVO_MAIL The driver supports two kinds of interface: using ledtrig-timer or through /sys/class/leds/clevo::mail/brightness. As this LED - cannot change it's brightness it blinks instead. The brightness + cannot change its brightness it blinks instead. The brightness value 0 means off, 1..127 means blink at 0.5Hz and 128..255 means blink at 1Hz. From 399e7aa82105ea46d8998fa535b047541c48030f Mon Sep 17 00:00:00 2001 From: Emil Renner Berthing Date: Tue, 5 Jul 2022 23:01:40 +0200 Subject: [PATCH 02/23] leds: pwm-multicolor: Don't show -EPROBE_DEFER as errors When requesting a PWM it might return -EPROBE_DEFER if it hasn't probed yet. This is not an error, so just propagate the -EPROBE_DEFER without logging anything. There is already dev_err_probe for exactly this situation. Fixes: 9fa2762110dd ("leds: Add PWM multicolor driver") Signed-off-by: Emil Renner Berthing Reviewed-by: Geert Uytterhoeven Signed-off-by: Pavel Machek --- drivers/leds/rgb/leds-pwm-multicolor.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/leds/rgb/leds-pwm-multicolor.c b/drivers/leds/rgb/leds-pwm-multicolor.c index 45e38708ecb17..eb67b89d28e92 100644 --- a/drivers/leds/rgb/leds-pwm-multicolor.c +++ b/drivers/leds/rgb/leds-pwm-multicolor.c @@ -72,8 +72,7 @@ static int iterate_subleds(struct device *dev, struct pwm_mc_led *priv, pwmled = &priv->leds[priv->mc_cdev.num_colors]; pwmled->pwm = devm_fwnode_pwm_get(dev, fwnode, NULL); if (IS_ERR(pwmled->pwm)) { - ret = PTR_ERR(pwmled->pwm); - dev_err(dev, "unable to request PWM: %d\n", ret); + ret = dev_err_probe(dev, PTR_ERR(pwmled->pwm), "unable to request PWM\n"); goto release_fwnode; } pwm_init_state(pwmled->pwm, &pwmled->state); From 83543c6e54b9dd18950533fff730e68c19cc053a Mon Sep 17 00:00:00 2001 From: Emil Renner Berthing Date: Tue, 5 Jul 2022 23:01:41 +0200 Subject: [PATCH 03/23] dt-bindings: leds: pwm-multicolor: Add active-low property Add the active-low property to LEDs that are part of a multicolor LED just like the regular PWM LEDs have. Signed-off-by: Emil Renner Berthing Reviewed-by: Geert Uytterhoeven Signed-off-by: Pavel Machek --- .../devicetree/bindings/leds/leds-pwm-multicolor.yaml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Documentation/devicetree/bindings/leds/leds-pwm-multicolor.yaml b/Documentation/devicetree/bindings/leds/leds-pwm-multicolor.yaml index 6625a528f7275..8d614eaf5c5ed 100644 --- a/Documentation/devicetree/bindings/leds/leds-pwm-multicolor.yaml +++ b/Documentation/devicetree/bindings/leds/leds-pwm-multicolor.yaml @@ -33,6 +33,10 @@ properties: pwm-names: true + active-low: + description: For PWMs where the LED is wired to supply rather than ground. + type: boolean + color: true required: From 3dd7ed589f2eda3904ab4b9fc49bbbc6d450d7f3 Mon Sep 17 00:00:00 2001 From: Emil Renner Berthing Date: Tue, 5 Jul 2022 23:01:42 +0200 Subject: [PATCH 04/23] leds: pwm-multicolor: Support active-low LEDs Add support for LEDs wired up to light when the PWM output is low, just like the regular PWM LEDs. Signed-off-by: Emil Renner Berthing Reviewed-by: Geert Uytterhoeven Signed-off-by: Pavel Machek --- drivers/leds/rgb/leds-pwm-multicolor.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/drivers/leds/rgb/leds-pwm-multicolor.c b/drivers/leds/rgb/leds-pwm-multicolor.c index eb67b89d28e92..da9d2218ae184 100644 --- a/drivers/leds/rgb/leds-pwm-multicolor.c +++ b/drivers/leds/rgb/leds-pwm-multicolor.c @@ -19,6 +19,7 @@ struct pwm_led { struct pwm_device *pwm; struct pwm_state state; + bool active_low; }; struct pwm_mc_led { @@ -45,6 +46,9 @@ static int led_pwm_mc_set(struct led_classdev *cdev, duty *= mc_cdev->subled_info[i].brightness; do_div(duty, cdev->max_brightness); + if (priv->leds[i].active_low) + duty = priv->leds[i].state.period - duty; + priv->leds[i].state.duty_cycle = duty; priv->leds[i].state.enabled = duty > 0; ret = pwm_apply_state(priv->leds[i].pwm, @@ -76,6 +80,7 @@ static int iterate_subleds(struct device *dev, struct pwm_mc_led *priv, goto release_fwnode; } pwm_init_state(pwmled->pwm, &pwmled->state); + pwmled->active_low = fwnode_property_read_bool(fwnode, "active-low"); ret = fwnode_property_read_u32(fwnode, "color", &color); if (ret) { From 9b60b675194cf59215dc83d52522513da23e84f2 Mon Sep 17 00:00:00 2001 From: Pavel Machek Date: Sun, 17 Jul 2022 12:59:48 +0200 Subject: [PATCH 05/23] leds: clevo-mail: Mark as broken pending interface fix Setting blink rate using brightness is unusual and should be fixed. Signed-off-by: Pavel Machek --- drivers/leds/Kconfig | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig index acd4f754c9192..eaba0a8347faf 100644 --- a/drivers/leds/Kconfig +++ b/drivers/leds/Kconfig @@ -447,12 +447,12 @@ config LEDS_LP8860 config LEDS_CLEVO_MAIL tristate "Mail LED on Clevo notebook" - depends on LEDS_CLASS + depends on LEDS_CLASS && BROKEN depends on X86 && SERIO_I8042 && DMI help This driver makes the mail LED accessible from userspace - programs through the leds subsystem. This LED have three - known mode: off, blink at 0.5Hz and blink at 1Hz. + programs through the LEDs subsystem. This LED has three + known modes: off, blink at 0.5Hz and blink at 1Hz. The driver supports two kinds of interface: using ledtrig-timer or through /sys/class/leds/clevo::mail/brightness. As this LED From 13344f8ce8a0d98aa7f5d69ce3b47393c73a343b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafa=C5=82=20Mi=C5=82ecki?= Date: Mon, 27 Dec 2021 15:59:04 +0100 Subject: [PATCH 06/23] dt-bindings: leds: add Broadcom's BCM63138 controller MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Broadcom used 2 LEDs hardware blocks for their BCM63xx SoCs: 1. Older one (BCM6318, BCM6328, BCM6362, BCM63268, BCM6838) 2. Newer one (BCM6848, BCM6858, BCM63138, BCM63148, BCM63381, BCM68360) The newer one was also later also used on BCM4908 SoC. Old block is already documented in the leds-bcm6328.yaml. This binding documents the new one which uses different registers & programming. It's first used in BCM63138 thus the binding name. Signed-off-by: Rafał Miłecki Reviewed-by: Rob Herring Reviewed-by: Florian Fainelli Signed-off-by: Pavel Machek --- .../bindings/leds/leds-bcm63138.yaml | 95 +++++++++++++++++++ 1 file changed, 95 insertions(+) create mode 100644 Documentation/devicetree/bindings/leds/leds-bcm63138.yaml diff --git a/Documentation/devicetree/bindings/leds/leds-bcm63138.yaml b/Documentation/devicetree/bindings/leds/leds-bcm63138.yaml new file mode 100644 index 0000000000000..99cd4ba9b0ca2 --- /dev/null +++ b/Documentation/devicetree/bindings/leds/leds-bcm63138.yaml @@ -0,0 +1,95 @@ +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/leds/leds-bcm63138.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Broadcom's BCM63138 LEDs controller + +maintainers: + - Rafał Miłecki + +description: | + This LEDs controller was first used on BCM63138 and later reused on BCM4908, + BCM6848, BCM6858, BCM63138, BCM63148, BCM63381 and BCM68360 SoCs. + + It supports up to 32 LEDs that can be connected parallelly or serially. It + also includes limited support for hardware blinking. + + Binding serially connected LEDs isn't documented yet. + +properties: + compatible: + oneOf: + - items: + - enum: + - brcm,bcm4908-leds + - brcm,bcm6848-leds + - brcm,bcm6858-leds + - brcm,bcm63148-leds + - brcm,bcm63381-leds + - brcm,bcm68360-leds + - const: brcm,bcm63138-leds + - const: brcm,bcm63138-leds + + reg: + maxItems: 1 + + "#address-cells": + const: 1 + + "#size-cells": + const: 0 + +patternProperties: + "^led@[a-f0-9]+$": + type: object + + $ref: common.yaml# + + properties: + reg: + maxItems: 1 + description: LED pin number + + active-low: + type: boolean + description: Makes LED active low. + + required: + - reg + + unevaluatedProperties: false + +required: + - reg + - "#address-cells" + - "#size-cells" + +additionalProperties: false + +examples: + - | + #include + + leds@ff800800 { + compatible = "brcm,bcm4908-leds", "brcm,bcm63138-leds"; + reg = <0xff800800 0xdc>; + + #address-cells = <1>; + #size-cells = <0>; + + led@0 { + reg = <0x0>; + function = LED_FUNCTION_POWER; + color = ; + default-state = "on"; + }; + + led@3 { + reg = <0x3>; + function = LED_FUNCTION_STATUS; + color = ; + active-low; + }; + }; From a0ba692072d89075d0a75c7ad9df31f2c1ee9a1c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafa=C5=82=20Mi=C5=82ecki?= Date: Mon, 27 Dec 2021 15:59:05 +0100 Subject: [PATCH 07/23] leds: bcm63138: add support for BCM63138 controller MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It's a new controller first introduced in BCM63138 SoC. Later it was also used in BCM4908, some BCM68xx and some BCM63xxx SoCs. Signed-off-by: Rafał Miłecki Reviewed-by: Florian Fainelli Signed-off-by: Pavel Machek --- drivers/leds/blink/Kconfig | 12 ++ drivers/leds/blink/Makefile | 1 + drivers/leds/blink/leds-bcm63138.c | 308 +++++++++++++++++++++++++++++ 3 files changed, 321 insertions(+) create mode 100644 drivers/leds/blink/leds-bcm63138.c diff --git a/drivers/leds/blink/Kconfig b/drivers/leds/blink/Kconfig index 59ba81e40e853..c680af01b5a90 100644 --- a/drivers/leds/blink/Kconfig +++ b/drivers/leds/blink/Kconfig @@ -1,3 +1,15 @@ +config LEDS_BCM63138 + tristate "LED Support for Broadcom BCM63138 SoC" + depends on LEDS_CLASS + depends on ARCH_BCM4908 || ARCH_BCM_5301X || BCM63XX || COMPILE_TEST + depends on HAS_IOMEM + depends on OF + default ARCH_BCM4908 + help + This option enables support for LED controller that is part of + BCM63138 SoC. The same hardware block is known to be also used + in BCM4908, BCM6848, BCM6858, BCM63148, BCM63381 and BCM68360. + config LEDS_LGM tristate "LED support for LGM SoC series" depends on X86 || COMPILE_TEST diff --git a/drivers/leds/blink/Makefile b/drivers/leds/blink/Makefile index fa5d04dccf139..447029f4153ae 100644 --- a/drivers/leds/blink/Makefile +++ b/drivers/leds/blink/Makefile @@ -1,2 +1,3 @@ # SPDX-License-Identifier: GPL-2.0 +obj-$(CONFIG_LEDS_BCM63138) += leds-bcm63138.o obj-$(CONFIG_LEDS_LGM) += leds-lgm-sso.o diff --git a/drivers/leds/blink/leds-bcm63138.c b/drivers/leds/blink/leds-bcm63138.c new file mode 100644 index 0000000000000..c083cb1332cbf --- /dev/null +++ b/drivers/leds/blink/leds-bcm63138.c @@ -0,0 +1,308 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * Copyright (C) 2021 Rafał Miłecki + */ +#include +#include +#include +#include +#include +#include +#include +#include + +#define BCM63138_MAX_LEDS 32 +#define BCM63138_MAX_BRIGHTNESS 9 + +#define BCM63138_LED_BITS 4 /* how many bits control a single LED */ +#define BCM63138_LED_MASK ((1 << BCM63138_LED_BITS) - 1) /* 0xf */ +#define BCM63138_LEDS_PER_REG (32 / BCM63138_LED_BITS) /* 8 */ + +#define BCM63138_GLB_CTRL 0x00 +#define BCM63138_GLB_CTRL_SERIAL_LED_DATA_PPOL 0x00000002 +#define BCM63138_GLB_CTRL_SERIAL_LED_EN_POL 0x00000008 +#define BCM63138_MASK 0x04 +#define BCM63138_HW_LED_EN 0x08 +#define BCM63138_SERIAL_LED_SHIFT_SEL 0x0c +#define BCM63138_FLASH_RATE_CTRL1 0x10 +#define BCM63138_FLASH_RATE_CTRL2 0x14 +#define BCM63138_FLASH_RATE_CTRL3 0x18 +#define BCM63138_FLASH_RATE_CTRL4 0x1c +#define BCM63138_BRIGHT_CTRL1 0x20 +#define BCM63138_BRIGHT_CTRL2 0x24 +#define BCM63138_BRIGHT_CTRL3 0x28 +#define BCM63138_BRIGHT_CTRL4 0x2c +#define BCM63138_POWER_LED_CFG 0x30 +#define BCM63138_HW_POLARITY 0xb4 +#define BCM63138_SW_DATA 0xb8 +#define BCM63138_SW_POLARITY 0xbc +#define BCM63138_PARALLEL_LED_POLARITY 0xc0 +#define BCM63138_SERIAL_LED_POLARITY 0xc4 +#define BCM63138_HW_LED_STATUS 0xc8 +#define BCM63138_FLASH_CTRL_STATUS 0xcc +#define BCM63138_FLASH_BRT_CTRL 0xd0 +#define BCM63138_FLASH_P_LED_OUT_STATUS 0xd4 +#define BCM63138_FLASH_S_LED_OUT_STATUS 0xd8 + +struct bcm63138_leds { + struct device *dev; + void __iomem *base; + spinlock_t lock; +}; + +struct bcm63138_led { + struct bcm63138_leds *leds; + struct led_classdev cdev; + u32 pin; + bool active_low; +}; + +/* + * I/O access + */ + +static void bcm63138_leds_write(struct bcm63138_leds *leds, unsigned int reg, + u32 data) +{ + writel(data, leds->base + reg); +} + +static unsigned long bcm63138_leds_read(struct bcm63138_leds *leds, + unsigned int reg) +{ + return readl(leds->base + reg); +} + +static void bcm63138_leds_update_bits(struct bcm63138_leds *leds, + unsigned int reg, u32 mask, u32 val) +{ + WARN_ON(val & ~mask); + + bcm63138_leds_write(leds, reg, (bcm63138_leds_read(leds, reg) & ~mask) | (val & mask)); +} + +/* + * Helpers + */ + +static void bcm63138_leds_set_flash_rate(struct bcm63138_leds *leds, + struct bcm63138_led *led, + u8 value) +{ + int reg_offset = (led->pin >> fls((BCM63138_LEDS_PER_REG - 1))) * 4; + int shift = (led->pin & (BCM63138_LEDS_PER_REG - 1)) * BCM63138_LED_BITS; + + bcm63138_leds_update_bits(leds, BCM63138_FLASH_RATE_CTRL1 + reg_offset, + BCM63138_LED_MASK << shift, value << shift); +} + +static void bcm63138_leds_set_bright(struct bcm63138_leds *leds, + struct bcm63138_led *led, + u8 value) +{ + int reg_offset = (led->pin >> fls((BCM63138_LEDS_PER_REG - 1))) * 4; + int shift = (led->pin & (BCM63138_LEDS_PER_REG - 1)) * BCM63138_LED_BITS; + + bcm63138_leds_update_bits(leds, BCM63138_BRIGHT_CTRL1 + reg_offset, + BCM63138_LED_MASK << shift, value << shift); +} + +static void bcm63138_leds_enable_led(struct bcm63138_leds *leds, + struct bcm63138_led *led, + enum led_brightness value) +{ + u32 bit = BIT(led->pin); + + bcm63138_leds_update_bits(leds, BCM63138_SW_DATA, bit, + value == LED_OFF ? 0 : bit); +} + +/* + * API callbacks + */ + +static void bcm63138_leds_brightness_set(struct led_classdev *led_cdev, + enum led_brightness value) +{ + struct bcm63138_led *led = container_of(led_cdev, struct bcm63138_led, cdev); + struct bcm63138_leds *leds = led->leds; + unsigned long flags; + + spin_lock_irqsave(&leds->lock, flags); + + bcm63138_leds_enable_led(leds, led, value); + if (!value) + bcm63138_leds_set_flash_rate(leds, led, 0); + else + bcm63138_leds_set_bright(leds, led, value); + + spin_unlock_irqrestore(&leds->lock, flags); +} + +static int bcm63138_leds_blink_set(struct led_classdev *led_cdev, + unsigned long *delay_on, + unsigned long *delay_off) +{ + struct bcm63138_led *led = container_of(led_cdev, struct bcm63138_led, cdev); + struct bcm63138_leds *leds = led->leds; + unsigned long flags; + u8 value; + + if (!*delay_on && !*delay_off) { + *delay_on = 640; + *delay_off = 640; + } + + if (*delay_on != *delay_off) { + dev_dbg(led_cdev->dev, "Blinking at unequal delays is not supported\n"); + return -EINVAL; + } + + switch (*delay_on) { + case 1152 ... 1408: /* 1280 ms ± 10% */ + value = 0x7; + break; + case 576 ... 704: /* 640 ms ± 10% */ + value = 0x6; + break; + case 288 ... 352: /* 320 ms ± 10% */ + value = 0x5; + break; + case 126 ... 154: /* 140 ms ± 10% */ + value = 0x4; + break; + case 59 ... 72: /* 65 ms ± 10% */ + value = 0x3; + break; + default: + dev_dbg(led_cdev->dev, "Blinking delay value %lu is unsupported\n", + *delay_on); + return -EINVAL; + } + + spin_lock_irqsave(&leds->lock, flags); + + bcm63138_leds_enable_led(leds, led, BCM63138_MAX_BRIGHTNESS); + bcm63138_leds_set_flash_rate(leds, led, value); + + spin_unlock_irqrestore(&leds->lock, flags); + + return 0; +} + +/* + * LED driver + */ + +static void bcm63138_leds_create_led(struct bcm63138_leds *leds, + struct device_node *np) +{ + struct led_init_data init_data = { + .fwnode = of_fwnode_handle(np), + }; + struct device *dev = leds->dev; + struct bcm63138_led *led; + struct pinctrl *pinctrl; + u32 bit; + int err; + + led = devm_kzalloc(dev, sizeof(*led), GFP_KERNEL); + if (!led) { + dev_err(dev, "Failed to alloc LED\n"); + return; + } + + led->leds = leds; + + if (of_property_read_u32(np, "reg", &led->pin)) { + dev_err(dev, "Missing \"reg\" property in %pOF\n", np); + goto err_free; + } + + if (led->pin >= BCM63138_MAX_LEDS) { + dev_err(dev, "Invalid \"reg\" value %d\n", led->pin); + goto err_free; + } + + led->active_low = of_property_read_bool(np, "active-low"); + + led->cdev.max_brightness = BCM63138_MAX_BRIGHTNESS; + led->cdev.brightness_set = bcm63138_leds_brightness_set; + led->cdev.blink_set = bcm63138_leds_blink_set; + + err = devm_led_classdev_register_ext(dev, &led->cdev, &init_data); + if (err) { + dev_err(dev, "Failed to register LED %pOF: %d\n", np, err); + goto err_free; + } + + pinctrl = devm_pinctrl_get_select_default(led->cdev.dev); + if (IS_ERR(pinctrl) && PTR_ERR(pinctrl) != -ENODEV) { + dev_warn(led->cdev.dev, "Failed to select %pOF pinctrl: %ld\n", + np, PTR_ERR(pinctrl)); + } + + bit = BIT(led->pin); + bcm63138_leds_update_bits(leds, BCM63138_PARALLEL_LED_POLARITY, bit, + led->active_low ? 0 : bit); + bcm63138_leds_update_bits(leds, BCM63138_HW_LED_EN, bit, 0); + bcm63138_leds_set_flash_rate(leds, led, 0); + bcm63138_leds_enable_led(leds, led, led->cdev.brightness); + + return; + +err_free: + devm_kfree(dev, led); +} + +static int bcm63138_leds_probe(struct platform_device *pdev) +{ + struct device_node *np = dev_of_node(&pdev->dev); + struct device *dev = &pdev->dev; + struct bcm63138_leds *leds; + struct device_node *child; + + leds = devm_kzalloc(dev, sizeof(*leds), GFP_KERNEL); + if (!leds) + return -ENOMEM; + + leds->dev = dev; + + leds->base = devm_platform_ioremap_resource(pdev, 0); + if (IS_ERR(leds->base)) + return PTR_ERR(leds->base); + + spin_lock_init(&leds->lock); + + bcm63138_leds_write(leds, BCM63138_GLB_CTRL, + BCM63138_GLB_CTRL_SERIAL_LED_DATA_PPOL | + BCM63138_GLB_CTRL_SERIAL_LED_EN_POL); + bcm63138_leds_write(leds, BCM63138_HW_LED_EN, 0); + bcm63138_leds_write(leds, BCM63138_SERIAL_LED_POLARITY, 0); + bcm63138_leds_write(leds, BCM63138_PARALLEL_LED_POLARITY, 0); + + for_each_available_child_of_node(np, child) { + bcm63138_leds_create_led(leds, child); + } + + return 0; +} + +static const struct of_device_id bcm63138_leds_of_match_table[] = { + { .compatible = "brcm,bcm63138-leds", }, + { }, +}; + +static struct platform_driver bcm63138_leds_driver = { + .probe = bcm63138_leds_probe, + .driver = { + .name = "leds-bcm63xxx", + .of_match_table = bcm63138_leds_of_match_table, + }, +}; + +module_platform_driver(bcm63138_leds_driver); + +MODULE_AUTHOR("Rafał Miłecki"); +MODULE_LICENSE("GPL"); +MODULE_DEVICE_TABLE(of, bcm63138_leds_of_match_table); From de40c8496ead3e25d1e989999eed0c3ecee8fc96 Mon Sep 17 00:00:00 2001 From: Krzysztof Kozlowski Date: Fri, 24 Jun 2022 13:21:06 +0200 Subject: [PATCH 08/23] dt-bindings: leds: class-multicolor: reference class directly in multi-led node The leds/common.yaml is referenced directly in each LED node, which leads to people doing the same with leds/leds-class-multicolor.yaml. This is not correct because leds-class-multicolor.yaml defined multi-led property and its children. Some schemas implemented this incorrect. Rework this to match same behavior common.yaml, so expect the multi-led node to reference the leds-class-multicolor.yaml. Fixing allows to add unevaluatedProperties:false. Signed-off-by: Krzysztof Kozlowski Reviewed-by: Rob Herring Signed-off-by: Pavel Machek --- .../leds/cznic,turris-omnia-leds.yaml | 2 ++ .../bindings/leds/leds-class-multicolor.yaml | 32 +++++++++---------- .../devicetree/bindings/leds/leds-lp50xx.yaml | 2 ++ .../bindings/leds/leds-pwm-multicolor.yaml | 5 ++- .../bindings/leds/leds-qcom-lpg.yaml | 2 ++ 5 files changed, 24 insertions(+), 19 deletions(-) diff --git a/Documentation/devicetree/bindings/leds/cznic,turris-omnia-leds.yaml b/Documentation/devicetree/bindings/leds/cznic,turris-omnia-leds.yaml index 9362b1ef9e88a..14bebe1ad8f8a 100644 --- a/Documentation/devicetree/bindings/leds/cznic,turris-omnia-leds.yaml +++ b/Documentation/devicetree/bindings/leds/cznic,turris-omnia-leds.yaml @@ -33,6 +33,8 @@ patternProperties: "^multi-led@[0-9a-b]$": type: object $ref: leds-class-multicolor.yaml# + unevaluatedProperties: false + 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 diff --git a/Documentation/devicetree/bindings/leds/leds-class-multicolor.yaml b/Documentation/devicetree/bindings/leds/leds-class-multicolor.yaml index f41d021ed6774..12693483231f7 100644 --- a/Documentation/devicetree/bindings/leds/leds-class-multicolor.yaml +++ b/Documentation/devicetree/bindings/leds/leds-class-multicolor.yaml @@ -19,22 +19,22 @@ description: | LED class. Common LED nodes and properties are inherited from the common.yaml within this documentation directory. -patternProperties: - "^multi-led(@[0-9a-f])?$": - type: object - description: Represents the LEDs that are to be grouped. - properties: - color: - description: | - For multicolor LED support this property should be defined as either - LED_COLOR_ID_RGB or LED_COLOR_ID_MULTI which can be found in - include/linux/leds/common.h. - enum: [ 8, 9 ] - - $ref: "common.yaml#" - - required: - - color +properties: + $nodename: + pattern: "^multi-led(@[0-9a-f])?$" + + color: + description: | + For multicolor LED support this property should be defined as either + LED_COLOR_ID_RGB or LED_COLOR_ID_MULTI which can be found in + include/linux/leds/common.h. + enum: [ 8, 9 ] + +required: + - color + +allOf: + - $ref: "common.yaml#" additionalProperties: true diff --git a/Documentation/devicetree/bindings/leds/leds-lp50xx.yaml b/Documentation/devicetree/bindings/leds/leds-lp50xx.yaml index f12fe5b53f30d..64d07fd20eab3 100644 --- a/Documentation/devicetree/bindings/leds/leds-lp50xx.yaml +++ b/Documentation/devicetree/bindings/leds/leds-lp50xx.yaml @@ -56,6 +56,8 @@ patternProperties: '^multi-led@[0-9a-f]$': type: object $ref: leds-class-multicolor.yaml# + unevaluatedProperties: false + properties: reg: minItems: 1 diff --git a/Documentation/devicetree/bindings/leds/leds-pwm-multicolor.yaml b/Documentation/devicetree/bindings/leds/leds-pwm-multicolor.yaml index 8d614eaf5c5ed..70b4bad00a632 100644 --- a/Documentation/devicetree/bindings/leds/leds-pwm-multicolor.yaml +++ b/Documentation/devicetree/bindings/leds/leds-pwm-multicolor.yaml @@ -19,6 +19,8 @@ properties: multi-led: type: object + $ref: leds-class-multicolor.yaml# + unevaluatedProperties: false patternProperties: "^led-[0-9a-z]+$": @@ -46,9 +48,6 @@ properties: required: - compatible -allOf: - - $ref: leds-class-multicolor.yaml# - additionalProperties: false examples: diff --git a/Documentation/devicetree/bindings/leds/leds-qcom-lpg.yaml b/Documentation/devicetree/bindings/leds/leds-qcom-lpg.yaml index 409a4c7298e12..fe336fa16518f 100644 --- a/Documentation/devicetree/bindings/leds/leds-qcom-lpg.yaml +++ b/Documentation/devicetree/bindings/leds/leds-qcom-lpg.yaml @@ -58,6 +58,8 @@ properties: multi-led: type: object $ref: leds-class-multicolor.yaml# + unevaluatedProperties: false + properties: "#address-cells": const: 1 From 38a3b05ac078c918827bc838b0be186b68795344 Mon Sep 17 00:00:00 2001 From: Krzysztof Kozlowski Date: Fri, 24 Jun 2022 13:13:25 +0200 Subject: [PATCH 09/23] dt-bindings: leds: lp50xx: fix LED children names The lp50xx LEDs expects to have single-color LED children with unit addresses. This is required by the driver and provided by existing DTSes. Fix the binding to match actual usage. Signed-off-by: Krzysztof Kozlowski Reviewed-by: Rob Herring [This depends on previous schema-fixing patch, so I deleted Fixes; we don't want to deal with this in stable] Signed-off-by: Pavel Machek --- Documentation/devicetree/bindings/leds/leds-lp50xx.yaml | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/Documentation/devicetree/bindings/leds/leds-lp50xx.yaml b/Documentation/devicetree/bindings/leds/leds-lp50xx.yaml index 64d07fd20eab3..b272a20552466 100644 --- a/Documentation/devicetree/bindings/leds/leds-lp50xx.yaml +++ b/Documentation/devicetree/bindings/leds/leds-lp50xx.yaml @@ -67,8 +67,14 @@ patternProperties: for the child node. The LED modules can either be used stand alone or grouped into a module bank. + '#address-cells': + const: 1 + + '#size-cells': + const: 0 + patternProperties: - "(^led-[0-9a-f]$|led)": + "^led@[0-9a-f]+$": type: object $ref: common.yaml# From 13b64a0c19059b38150c79d65d350ae44034c5df Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafa=C5=82=20Mi=C5=82ecki?= Date: Sun, 17 Jul 2022 14:42:46 +0200 Subject: [PATCH 10/23] dt-bindings: leds: leds-bcm63138: unify full stops in descriptions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Description of "reg" doesn't have full stop at the end. It makes sense as it's a one-sentence only. Use the same style for "active-low". Reported-by: Pavel Machek Signed-off-by: Rafał Miłecki Signed-off-by: Pavel Machek --- Documentation/devicetree/bindings/leds/leds-bcm63138.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/devicetree/bindings/leds/leds-bcm63138.yaml b/Documentation/devicetree/bindings/leds/leds-bcm63138.yaml index 99cd4ba9b0ca2..52252fb6bb321 100644 --- a/Documentation/devicetree/bindings/leds/leds-bcm63138.yaml +++ b/Documentation/devicetree/bindings/leds/leds-bcm63138.yaml @@ -54,7 +54,7 @@ patternProperties: active-low: type: boolean - description: Makes LED active low. + description: Makes LED active low required: - reg From bcc607cdbb1f931111196699426f0cb83bfb296a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafa=C5=82=20Mi=C5=82ecki?= Date: Sun, 17 Jul 2022 14:42:47 +0200 Subject: [PATCH 11/23] leds: add help info about BCM63138 module name MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It's what we do for all other LEDs drivers. Reported-by: Pavel Machek Signed-off-by: Rafał Miłecki Signed-off-by: Pavel Machek --- drivers/leds/blink/Kconfig | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/leds/blink/Kconfig b/drivers/leds/blink/Kconfig index c680af01b5a90..945c84286a4ed 100644 --- a/drivers/leds/blink/Kconfig +++ b/drivers/leds/blink/Kconfig @@ -10,6 +10,8 @@ config LEDS_BCM63138 BCM63138 SoC. The same hardware block is known to be also used in BCM4908, BCM6848, BCM6858, BCM63148, BCM63381 and BCM68360. + If compiled as module it will be called leds-bcm63138. + config LEDS_LGM tristate "LED support for LGM SoC series" depends on X86 || COMPILE_TEST From 92cfc71ee2ddfb499ed53e21b28bdf8739bc70bc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafa=C5=82=20Mi=C5=82ecki?= Date: Sun, 17 Jul 2022 14:42:48 +0200 Subject: [PATCH 12/23] leds: leds-bcm63138: get rid of LED_OFF MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The whole "enum led_brightness" is marked as obsolete. Replace it with a (non-)zero check. Reported-by: Pavel Machek Signed-off-by: Rafał Miłecki Signed-off-by: Pavel Machek --- drivers/leds/blink/leds-bcm63138.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/leds/blink/leds-bcm63138.c b/drivers/leds/blink/leds-bcm63138.c index c083cb1332cbf..2cf2761e49145 100644 --- a/drivers/leds/blink/leds-bcm63138.c +++ b/drivers/leds/blink/leds-bcm63138.c @@ -113,8 +113,7 @@ static void bcm63138_leds_enable_led(struct bcm63138_leds *leds, { u32 bit = BIT(led->pin); - bcm63138_leds_update_bits(leds, BCM63138_SW_DATA, bit, - value == LED_OFF ? 0 : bit); + bcm63138_leds_update_bits(leds, BCM63138_SW_DATA, bit, value ? bit : 0); } /* From a01633cd867b8ddf2d8321fe575dc3c54e037b09 Mon Sep 17 00:00:00 2001 From: Greg Kroah-Hartman Date: Fri, 29 Jul 2022 16:03:46 +0200 Subject: [PATCH 13/23] leds: turris-omnia: convert to use dev_groups MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The driver core supports the ability to handle the creation and removal of device-specific sysfs files in a race-free manner. Take advantage of that by converting this driver to use this by moving the sysfs attributes into a group and assigning the dev_groups pointer to it. Cc: Pavel Machek Cc: linux-leds@vger.kernel.org Cc: linux-kernel@vger.kernel.org Reviewed-by: Marek Behún Signed-off-by: Greg Kroah-Hartman Signed-off-by: Pavel Machek --- drivers/leds/leds-turris-omnia.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/leds/leds-turris-omnia.c b/drivers/leds/leds-turris-omnia.c index 1adfed1c0619b..eac6f4a573b2f 100644 --- a/drivers/leds/leds-turris-omnia.c +++ b/drivers/leds/leds-turris-omnia.c @@ -239,9 +239,6 @@ static int omnia_leds_probe(struct i2c_client *client, led += ret; } - if (devm_device_add_groups(dev, omnia_led_controller_groups)) - dev_warn(dev, "Could not add attribute group!\n"); - return 0; } @@ -283,6 +280,7 @@ static struct i2c_driver omnia_leds_driver = { .driver = { .name = "leds-turris-omnia", .of_match_table = of_omnia_leds_match, + .dev_groups = omnia_led_controller_groups, }, }; From e625e2238a270df743cbc623b3abad8d0010abc9 Mon Sep 17 00:00:00 2001 From: Krzysztof Kozlowski Date: Tue, 19 Jul 2022 09:45:42 +0200 Subject: [PATCH 14/23] dt-bindings: leds: pwm-multicolor: document max-brigthness The Multicolor PWM LED uses max-brigthness property (in the example and in the driver), so document it to fixi dt_binding_check warning like: leds/leds-pwm-multicolor.example.dtb: led-controller: multi-led: Unevaluated properties are not allowed ('max-brightness' was unexpected) Reported-by: Rob Herring Reviewed-by: Rob Herring Signed-off-by: Krzysztof Kozlowski Signed-off-by: Pavel Machek --- .../devicetree/bindings/leds/leds-pwm-multicolor.yaml | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/Documentation/devicetree/bindings/leds/leds-pwm-multicolor.yaml b/Documentation/devicetree/bindings/leds/leds-pwm-multicolor.yaml index 70b4bad00a632..43f1cd6a53b2d 100644 --- a/Documentation/devicetree/bindings/leds/leds-pwm-multicolor.yaml +++ b/Documentation/devicetree/bindings/leds/leds-pwm-multicolor.yaml @@ -22,6 +22,12 @@ properties: $ref: leds-class-multicolor.yaml# unevaluatedProperties: false + properties: + max-brightness: + description: + Maximum brightness possible for the LED + $ref: /schemas/types.yaml#/definitions/uint32 + patternProperties: "^led-[0-9a-z]+$": type: object From dc6d28f4859d1fcbd2f5505862a768f07c7a9770 Mon Sep 17 00:00:00 2001 From: Vincent Knecht Date: Tue, 12 Jul 2022 12:08:29 +0200 Subject: [PATCH 15/23] leds: is31fl319x: Add missing si-en compatibles Add si-en compatibles for all chip variants. Signed-off-by: Vincent Knecht Signed-off-by: Pavel Machek --- drivers/leds/leds-is31fl319x.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/drivers/leds/leds-is31fl319x.c b/drivers/leds/leds-is31fl319x.c index 4161b9dd7e488..0db5d49881317 100644 --- a/drivers/leds/leds-is31fl319x.c +++ b/drivers/leds/leds-is31fl319x.c @@ -102,6 +102,10 @@ static const struct of_device_id of_is31fl319x_match[] = { { .compatible = "issi,is31fl3193", .data = &is31fl3193_cdef, }, { .compatible = "issi,is31fl3196", .data = &is31fl3196_cdef, }, { .compatible = "issi,is31fl3199", .data = &is31fl3199_cdef, }, + { .compatible = "si-en,sn3190", .data = &is31fl3190_cdef, }, + { .compatible = "si-en,sn3191", .data = &is31fl3190_cdef, }, + { .compatible = "si-en,sn3193", .data = &is31fl3193_cdef, }, + { .compatible = "si-en,sn3196", .data = &is31fl3196_cdef, }, { .compatible = "si-en,sn3199", .data = &is31fl3199_cdef, }, { } }; @@ -432,6 +436,10 @@ static const struct i2c_device_id is31fl319x_id[] = { { "is31fl3193" }, { "is31fl3196" }, { "is31fl3199" }, + { "sn3190" }, + { "sn3191" }, + { "sn3193" }, + { "sn3196" }, { "sn3199" }, {}, }; From 8e6dde1b4495348a90e8a49147e8145d957cda52 Mon Sep 17 00:00:00 2001 From: Vincent Knecht Date: Tue, 12 Jul 2022 12:08:30 +0200 Subject: [PATCH 16/23] leds: is31fl319x: Use non-wildcard names for vars, structs and defines In order to add real support for is31fl3190, is31fl3191 and is31fl3193, rename variant-dependent elements to not use 319X where needed. 3190 suffix is used for is31fl3190, is31fl3191 and is31fl3193 circuits. 3196 suffix is used for is31fl3196 and is31fl3199. Those two groups have different register maps, current settings and even a different interpretation of the software shutdown bit: https://lumissil.com/assets/pdf/core/IS31FL3190_DS.pdf https://lumissil.com/assets/pdf/core/IS31FL3191_DS.pdf https://lumissil.com/assets/pdf/core/IS31FL3193_DS.pdf https://lumissil.com/assets/pdf/core/IS31FL3196_DS.pdf https://lumissil.com/assets/pdf/core/IS31FL3199_DS.pdf Rename variables, structures and defines in preparation of the splitting. No functional nor behaviour change. Signed-off-by: Vincent Knecht Signed-off-by: Pavel Machek --- drivers/leds/leds-is31fl319x.c | 142 ++++++++++++++++----------------- 1 file changed, 71 insertions(+), 71 deletions(-) diff --git a/drivers/leds/leds-is31fl319x.c b/drivers/leds/leds-is31fl319x.c index 0db5d49881317..672b0e9e1d85b 100644 --- a/drivers/leds/leds-is31fl319x.c +++ b/drivers/leds/leds-is31fl319x.c @@ -21,39 +21,39 @@ /* register numbers */ #define IS31FL319X_SHUTDOWN 0x00 -#define IS31FL319X_CTRL1 0x01 -#define IS31FL319X_CTRL2 0x02 -#define IS31FL319X_CONFIG1 0x03 -#define IS31FL319X_CONFIG2 0x04 -#define IS31FL319X_RAMP_MODE 0x05 -#define IS31FL319X_BREATH_MASK 0x06 -#define IS31FL319X_PWM(channel) (0x07 + channel) -#define IS31FL319X_DATA_UPDATE 0x10 -#define IS31FL319X_T0(channel) (0x11 + channel) -#define IS31FL319X_T123_1 0x1a -#define IS31FL319X_T123_2 0x1b -#define IS31FL319X_T123_3 0x1c -#define IS31FL319X_T4(channel) (0x1d + channel) -#define IS31FL319X_TIME_UPDATE 0x26 -#define IS31FL319X_RESET 0xff - -#define IS31FL319X_REG_CNT (IS31FL319X_RESET + 1) +#define IS31FL3196_CTRL1 0x01 +#define IS31FL3196_CTRL2 0x02 +#define IS31FL3196_CONFIG1 0x03 +#define IS31FL3196_CONFIG2 0x04 +#define IS31FL3196_RAMP_MODE 0x05 +#define IS31FL3196_BREATH_MARK 0x06 +#define IS31FL3196_PWM(channel) (0x07 + channel) +#define IS31FL3196_DATA_UPDATE 0x10 +#define IS31FL3196_T0(channel) (0x11 + channel) +#define IS31FL3196_T123_1 0x1a +#define IS31FL3196_T123_2 0x1b +#define IS31FL3196_T123_3 0x1c +#define IS31FL3196_T4(channel) (0x1d + channel) +#define IS31FL3196_TIME_UPDATE 0x26 +#define IS31FL3196_RESET 0xff + +#define IS31FL3196_REG_CNT (IS31FL3196_RESET + 1) #define IS31FL319X_MAX_LEDS 9 /* CS (Current Setting) in CONFIG2 register */ -#define IS31FL319X_CONFIG2_CS_SHIFT 4 -#define IS31FL319X_CONFIG2_CS_MASK 0x7 -#define IS31FL319X_CONFIG2_CS_STEP_REF 12 +#define IS31FL3196_CONFIG2_CS_SHIFT 4 +#define IS31FL3196_CONFIG2_CS_MASK GENMASK(2, 0) +#define IS31FL3196_CONFIG2_CS_STEP_REF 12 -#define IS31FL319X_CURRENT_MIN ((u32)5000) -#define IS31FL319X_CURRENT_MAX ((u32)40000) -#define IS31FL319X_CURRENT_STEP ((u32)5000) -#define IS31FL319X_CURRENT_DEFAULT ((u32)20000) +#define IS31FL3196_CURRENT_uA_MIN 5000 +#define IS31FL3196_CURRENT_uA_MAX 40000 +#define IS31FL3196_CURRENT_uA_STEP 5000 +#define IS31FL3196_CURRENT_uA_DEFAULT 20000 /* Audio gain in CONFIG2 register */ -#define IS31FL319X_AUDIO_GAIN_DB_MAX ((u32)21) -#define IS31FL319X_AUDIO_GAIN_DB_STEP ((u32)3) +#define IS31FL3196_AUDIO_GAIN_DB_MAX ((u32)21) +#define IS31FL3196_AUDIO_GAIN_DB_STEP 3 /* * regmap is used as a cache of chip's register space, @@ -111,7 +111,7 @@ static const struct of_device_id of_is31fl319x_match[] = { }; MODULE_DEVICE_TABLE(of, of_is31fl319x_match); -static int is31fl319x_brightness_set(struct led_classdev *cdev, +static int is31fl3196_brightness_set(struct led_classdev *cdev, enum led_brightness brightness) { struct is31fl319x_led *led = container_of(cdev, struct is31fl319x_led, @@ -127,7 +127,7 @@ static int is31fl319x_brightness_set(struct led_classdev *cdev, mutex_lock(&is31->lock); /* update PWM register */ - ret = regmap_write(is31->regmap, IS31FL319X_PWM(chan), brightness); + ret = regmap_write(is31->regmap, IS31FL3196_PWM(chan), brightness); if (ret < 0) goto out; @@ -141,7 +141,7 @@ static int is31fl319x_brightness_set(struct led_classdev *cdev, * the current setting, we read from the regmap cache */ - ret = regmap_read(is31->regmap, IS31FL319X_PWM(i), &pwm_value); + ret = regmap_read(is31->regmap, IS31FL3196_PWM(i), &pwm_value); dev_dbg(&is31->client->dev, "%s read %d: ret=%d: %d\n", __func__, i, ret, pwm_value); on = ret >= 0 && pwm_value > LED_OFF; @@ -157,10 +157,10 @@ static int is31fl319x_brightness_set(struct led_classdev *cdev, if (ctrl1 > 0 || ctrl2 > 0) { dev_dbg(&is31->client->dev, "power up %02x %02x\n", ctrl1, ctrl2); - regmap_write(is31->regmap, IS31FL319X_CTRL1, ctrl1); - regmap_write(is31->regmap, IS31FL319X_CTRL2, ctrl2); + regmap_write(is31->regmap, IS31FL3196_CTRL1, ctrl1); + regmap_write(is31->regmap, IS31FL3196_CTRL2, ctrl2); /* update PWMs */ - regmap_write(is31->regmap, IS31FL319X_DATA_UPDATE, 0x00); + regmap_write(is31->regmap, IS31FL3196_DATA_UPDATE, 0x00); /* enable chip from shut down */ ret = regmap_write(is31->regmap, IS31FL319X_SHUTDOWN, 0x01); } else { @@ -190,14 +190,14 @@ static int is31fl319x_parse_child_dt(const struct device *dev, if (ret < 0 && ret != -EINVAL) /* is optional */ return ret; - led->max_microamp = IS31FL319X_CURRENT_DEFAULT; + led->max_microamp = IS31FL3196_CURRENT_uA_DEFAULT; ret = of_property_read_u32(child, "led-max-microamp", &led->max_microamp); if (!ret) { - if (led->max_microamp < IS31FL319X_CURRENT_MIN) + if (led->max_microamp < IS31FL3196_CURRENT_uA_MIN) return -EINVAL; /* not supported */ led->max_microamp = min(led->max_microamp, - IS31FL319X_CURRENT_MAX); + IS31FL3196_CURRENT_uA_MAX); } return 0; @@ -271,7 +271,7 @@ static int is31fl319x_parse_dt(struct device *dev, ret = of_property_read_u32(np, "audio-gain-db", &is31->audio_gain_db); if (!ret) is31->audio_gain_db = min(is31->audio_gain_db, - IS31FL319X_AUDIO_GAIN_DB_MAX); + IS31FL3196_AUDIO_GAIN_DB_MAX); return 0; @@ -285,55 +285,55 @@ static bool is31fl319x_readable_reg(struct device *dev, unsigned int reg) return false; } -static bool is31fl319x_volatile_reg(struct device *dev, unsigned int reg) +static bool is31fl3196_volatile_reg(struct device *dev, unsigned int reg) { /* volatile registers are not cached */ switch (reg) { - case IS31FL319X_DATA_UPDATE: - case IS31FL319X_TIME_UPDATE: - case IS31FL319X_RESET: + case IS31FL3196_DATA_UPDATE: + case IS31FL3196_TIME_UPDATE: + case IS31FL3196_RESET: return true; /* always write-through */ default: return false; } } -static const struct reg_default is31fl319x_reg_defaults[] = { - { IS31FL319X_CONFIG1, 0x00}, - { IS31FL319X_CONFIG2, 0x00}, - { IS31FL319X_PWM(0), 0x00}, - { IS31FL319X_PWM(1), 0x00}, - { IS31FL319X_PWM(2), 0x00}, - { IS31FL319X_PWM(3), 0x00}, - { IS31FL319X_PWM(4), 0x00}, - { IS31FL319X_PWM(5), 0x00}, - { IS31FL319X_PWM(6), 0x00}, - { IS31FL319X_PWM(7), 0x00}, - { IS31FL319X_PWM(8), 0x00}, +static const struct reg_default is31fl3196_reg_defaults[] = { + { IS31FL3196_CONFIG1, 0x00 }, + { IS31FL3196_CONFIG2, 0x00 }, + { IS31FL3196_PWM(0), 0x00 }, + { IS31FL3196_PWM(1), 0x00 }, + { IS31FL3196_PWM(2), 0x00 }, + { IS31FL3196_PWM(3), 0x00 }, + { IS31FL3196_PWM(4), 0x00 }, + { IS31FL3196_PWM(5), 0x00 }, + { IS31FL3196_PWM(6), 0x00 }, + { IS31FL3196_PWM(7), 0x00 }, + { IS31FL3196_PWM(8), 0x00 }, }; -static struct regmap_config regmap_config = { +static struct regmap_config is31fl3196_regmap_config = { .reg_bits = 8, .val_bits = 8, - .max_register = IS31FL319X_REG_CNT, + .max_register = IS31FL3196_REG_CNT, .cache_type = REGCACHE_FLAT, .readable_reg = is31fl319x_readable_reg, - .volatile_reg = is31fl319x_volatile_reg, - .reg_defaults = is31fl319x_reg_defaults, - .num_reg_defaults = ARRAY_SIZE(is31fl319x_reg_defaults), + .volatile_reg = is31fl3196_volatile_reg, + .reg_defaults = is31fl3196_reg_defaults, + .num_reg_defaults = ARRAY_SIZE(is31fl3196_reg_defaults), }; -static inline int is31fl319x_microamp_to_cs(struct device *dev, u32 microamp) +static inline int is31fl3196_microamp_to_cs(struct device *dev, u32 microamp) { /* round down to nearest supported value (range check done by caller) */ - u32 step = microamp / IS31FL319X_CURRENT_STEP; + u32 step = microamp / IS31FL3196_CURRENT_uA_STEP; - return ((IS31FL319X_CONFIG2_CS_STEP_REF - step) & - IS31FL319X_CONFIG2_CS_MASK) << - IS31FL319X_CONFIG2_CS_SHIFT; /* CS encoding */ + return ((IS31FL3196_CONFIG2_CS_STEP_REF - step) & + IS31FL3196_CONFIG2_CS_MASK) << + IS31FL3196_CONFIG2_CS_SHIFT; /* CS encoding */ } -static inline int is31fl319x_db_to_gain(u32 dezibel) +static inline int is31fl3196_db_to_gain(u32 dezibel) { /* round down to nearest supported value (range check done by caller) */ - return dezibel / IS31FL319X_AUDIO_GAIN_DB_STEP; + return dezibel / IS31FL3196_AUDIO_GAIN_DB_STEP; } static int is31fl319x_probe(struct i2c_client *client, @@ -343,7 +343,7 @@ static int is31fl319x_probe(struct i2c_client *client, struct device *dev = &client->dev; int err; int i = 0; - u32 aggregated_led_microamp = IS31FL319X_CURRENT_MAX; + u32 aggregated_led_microamp = IS31FL3196_CURRENT_uA_MAX; if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) return -EIO; @@ -365,7 +365,7 @@ static int is31fl319x_probe(struct i2c_client *client, } is31->client = client; - is31->regmap = devm_regmap_init_i2c(client, ®map_config); + is31->regmap = devm_regmap_init_i2c(client, &is31fl3196_regmap_config); if (IS_ERR(is31->regmap)) { dev_err(&client->dev, "failed to allocate register map\n"); err = PTR_ERR(is31->regmap); @@ -375,7 +375,7 @@ static int is31fl319x_probe(struct i2c_client *client, i2c_set_clientdata(client, is31); /* check for write-reply from chip (we can't read any registers) */ - err = regmap_write(is31->regmap, IS31FL319X_RESET, 0x00); + err = regmap_write(is31->regmap, IS31FL3196_RESET, 0x00); if (err < 0) { dev_err(&client->dev, "no response from chip write: err = %d\n", err); @@ -393,9 +393,9 @@ static int is31fl319x_probe(struct i2c_client *client, is31->leds[i].max_microamp < aggregated_led_microamp) aggregated_led_microamp = is31->leds[i].max_microamp; - regmap_write(is31->regmap, IS31FL319X_CONFIG2, - is31fl319x_microamp_to_cs(dev, aggregated_led_microamp) | - is31fl319x_db_to_gain(is31->audio_gain_db)); + regmap_write(is31->regmap, IS31FL3196_CONFIG2, + is31fl3196_microamp_to_cs(dev, aggregated_led_microamp) | + is31fl3196_db_to_gain(is31->audio_gain_db)); for (i = 0; i < is31->cdef->num_leds; i++) { struct is31fl319x_led *led = &is31->leds[i]; @@ -404,7 +404,7 @@ static int is31fl319x_probe(struct i2c_client *client, continue; led->chip = is31; - led->cdev.brightness_set_blocking = is31fl319x_brightness_set; + led->cdev.brightness_set_blocking = is31fl3196_brightness_set; err = devm_led_classdev_register(&client->dev, &led->cdev); if (err < 0) From bd34266fdec2826b852dd099b17af5df2b8c92b1 Mon Sep 17 00:00:00 2001 From: Vincent Knecht Date: Tue, 12 Jul 2022 12:08:31 +0200 Subject: [PATCH 17/23] leds: is31fl319x: Move chipset-specific values in chipdef struct Allow setting chips' specifics in chipdef struct by adding fields for: - the reset register address - a pointer to a regmap_config struct - a pointer to a brightness_set function - current default, min and max values - a boolean to distinguish 319{0,1,3} and 319{6,9} chips and use those fields in places where distinction has to be made. The fields for 319{0,1,3} still point to 319{6,9} values. No functional change. Signed-off-by: Vincent Knecht Signed-off-by: Pavel Machek --- drivers/leds/leds-is31fl319x.c | 208 ++++++++++++++++++++------------- 1 file changed, 125 insertions(+), 83 deletions(-) diff --git a/drivers/leds/leds-is31fl319x.c b/drivers/leds/leds-is31fl319x.c index 672b0e9e1d85b..fe0cb68b583b1 100644 --- a/drivers/leds/leds-is31fl319x.c +++ b/drivers/leds/leds-is31fl319x.c @@ -78,38 +78,58 @@ struct is31fl319x_chip { struct is31fl319x_chipdef { int num_leds; + u8 reset_reg; + const struct regmap_config *is31fl319x_regmap_config; + int (*brightness_set)(struct led_classdev *cdev, enum led_brightness brightness); + u32 current_default; + u32 current_min; + u32 current_max; + bool is_3196or3199; }; -static const struct is31fl319x_chipdef is31fl3190_cdef = { - .num_leds = 1, -}; - -static const struct is31fl319x_chipdef is31fl3193_cdef = { - .num_leds = 3, -}; +static bool is31fl319x_readable_reg(struct device *dev, unsigned int reg) +{ + /* we have no readable registers */ + return false; +} -static const struct is31fl319x_chipdef is31fl3196_cdef = { - .num_leds = 6, -}; +static bool is31fl3196_volatile_reg(struct device *dev, unsigned int reg) +{ + /* volatile registers are not cached */ + switch (reg) { + case IS31FL3196_DATA_UPDATE: + case IS31FL3196_TIME_UPDATE: + case IS31FL3196_RESET: + return true; /* always write-through */ + default: + return false; + } +} -static const struct is31fl319x_chipdef is31fl3199_cdef = { - .num_leds = 9, +static const struct reg_default is31fl3196_reg_defaults[] = { + { IS31FL3196_CONFIG1, 0x00 }, + { IS31FL3196_CONFIG2, 0x00 }, + { IS31FL3196_PWM(0), 0x00 }, + { IS31FL3196_PWM(1), 0x00 }, + { IS31FL3196_PWM(2), 0x00 }, + { IS31FL3196_PWM(3), 0x00 }, + { IS31FL3196_PWM(4), 0x00 }, + { IS31FL3196_PWM(5), 0x00 }, + { IS31FL3196_PWM(6), 0x00 }, + { IS31FL3196_PWM(7), 0x00 }, + { IS31FL3196_PWM(8), 0x00 }, }; -static const struct of_device_id of_is31fl319x_match[] = { - { .compatible = "issi,is31fl3190", .data = &is31fl3190_cdef, }, - { .compatible = "issi,is31fl3191", .data = &is31fl3190_cdef, }, - { .compatible = "issi,is31fl3193", .data = &is31fl3193_cdef, }, - { .compatible = "issi,is31fl3196", .data = &is31fl3196_cdef, }, - { .compatible = "issi,is31fl3199", .data = &is31fl3199_cdef, }, - { .compatible = "si-en,sn3190", .data = &is31fl3190_cdef, }, - { .compatible = "si-en,sn3191", .data = &is31fl3190_cdef, }, - { .compatible = "si-en,sn3193", .data = &is31fl3193_cdef, }, - { .compatible = "si-en,sn3196", .data = &is31fl3196_cdef, }, - { .compatible = "si-en,sn3199", .data = &is31fl3199_cdef, }, - { } +static struct regmap_config is31fl3196_regmap_config = { + .reg_bits = 8, + .val_bits = 8, + .max_register = IS31FL3196_REG_CNT, + .cache_type = REGCACHE_FLAT, + .readable_reg = is31fl319x_readable_reg, + .volatile_reg = is31fl3196_volatile_reg, + .reg_defaults = is31fl3196_reg_defaults, + .num_reg_defaults = ARRAY_SIZE(is31fl3196_reg_defaults), }; -MODULE_DEVICE_TABLE(of, of_is31fl319x_match); static int is31fl3196_brightness_set(struct led_classdev *cdev, enum led_brightness brightness) @@ -175,9 +195,69 @@ static int is31fl3196_brightness_set(struct led_classdev *cdev, return ret; } +static const struct is31fl319x_chipdef is31fl3190_cdef = { + .num_leds = 1, + .reset_reg = IS31FL3196_RESET, + .is31fl319x_regmap_config = &is31fl3196_regmap_config, + .brightness_set = is31fl3196_brightness_set, + .current_default = IS31FL3196_CURRENT_uA_DEFAULT, + .current_min = IS31FL3196_CURRENT_uA_MIN, + .current_max = IS31FL3196_CURRENT_uA_MAX, + .is_3196or3199 = true, +}; + +static const struct is31fl319x_chipdef is31fl3193_cdef = { + .num_leds = 3, + .reset_reg = IS31FL3196_RESET, + .is31fl319x_regmap_config = &is31fl3196_regmap_config, + .brightness_set = is31fl3196_brightness_set, + .current_default = IS31FL3196_CURRENT_uA_DEFAULT, + .current_min = IS31FL3196_CURRENT_uA_MIN, + .current_max = IS31FL3196_CURRENT_uA_MAX, + .is_3196or3199 = true, +}; + +static const struct is31fl319x_chipdef is31fl3196_cdef = { + .num_leds = 6, + .reset_reg = IS31FL3196_RESET, + .is31fl319x_regmap_config = &is31fl3196_regmap_config, + .brightness_set = is31fl3196_brightness_set, + .current_default = IS31FL3196_CURRENT_uA_DEFAULT, + .current_min = IS31FL3196_CURRENT_uA_MIN, + .current_max = IS31FL3196_CURRENT_uA_MAX, + .is_3196or3199 = true, +}; + +static const struct is31fl319x_chipdef is31fl3199_cdef = { + .num_leds = 9, + .reset_reg = IS31FL3196_RESET, + .is31fl319x_regmap_config = &is31fl3196_regmap_config, + .brightness_set = is31fl3196_brightness_set, + .current_default = IS31FL3196_CURRENT_uA_DEFAULT, + .current_min = IS31FL3196_CURRENT_uA_MIN, + .current_max = IS31FL3196_CURRENT_uA_MAX, + .is_3196or3199 = true, +}; + +static const struct of_device_id of_is31fl319x_match[] = { + { .compatible = "issi,is31fl3190", .data = &is31fl3190_cdef, }, + { .compatible = "issi,is31fl3191", .data = &is31fl3190_cdef, }, + { .compatible = "issi,is31fl3193", .data = &is31fl3193_cdef, }, + { .compatible = "issi,is31fl3196", .data = &is31fl3196_cdef, }, + { .compatible = "issi,is31fl3199", .data = &is31fl3199_cdef, }, + { .compatible = "si-en,sn3190", .data = &is31fl3190_cdef, }, + { .compatible = "si-en,sn3191", .data = &is31fl3190_cdef, }, + { .compatible = "si-en,sn3193", .data = &is31fl3193_cdef, }, + { .compatible = "si-en,sn3196", .data = &is31fl3196_cdef, }, + { .compatible = "si-en,sn3199", .data = &is31fl3199_cdef, }, + { } +}; +MODULE_DEVICE_TABLE(of, of_is31fl319x_match); + static int is31fl319x_parse_child_dt(const struct device *dev, const struct device_node *child, - struct is31fl319x_led *led) + struct is31fl319x_led *led, + struct is31fl319x_chip *is31) { struct led_classdev *cdev = &led->cdev; int ret; @@ -190,14 +270,14 @@ static int is31fl319x_parse_child_dt(const struct device *dev, if (ret < 0 && ret != -EINVAL) /* is optional */ return ret; - led->max_microamp = IS31FL3196_CURRENT_uA_DEFAULT; + led->max_microamp = is31->cdef->current_default; ret = of_property_read_u32(child, "led-max-microamp", &led->max_microamp); if (!ret) { - if (led->max_microamp < IS31FL3196_CURRENT_uA_MIN) + if (led->max_microamp < is31->cdef->current_min) return -EINVAL; /* not supported */ led->max_microamp = min(led->max_microamp, - IS31FL3196_CURRENT_uA_MAX); + is31->cdef->current_max); } return 0; @@ -258,7 +338,7 @@ static int is31fl319x_parse_dt(struct device *dev, goto put_child_node; } - ret = is31fl319x_parse_child_dt(dev, child, led); + ret = is31fl319x_parse_child_dt(dev, child, led, is31); if (ret) { dev_err(dev, "led %u DT parsing failed\n", reg); goto put_child_node; @@ -268,10 +348,12 @@ static int is31fl319x_parse_dt(struct device *dev, } is31->audio_gain_db = 0; - ret = of_property_read_u32(np, "audio-gain-db", &is31->audio_gain_db); - if (!ret) - is31->audio_gain_db = min(is31->audio_gain_db, - IS31FL3196_AUDIO_GAIN_DB_MAX); + if (is31->cdef->is_3196or3199) { + ret = of_property_read_u32(np, "audio-gain-db", &is31->audio_gain_db); + if (!ret) + is31->audio_gain_db = min(is31->audio_gain_db, + IS31FL3196_AUDIO_GAIN_DB_MAX); + } return 0; @@ -280,48 +362,6 @@ static int is31fl319x_parse_dt(struct device *dev, return ret; } -static bool is31fl319x_readable_reg(struct device *dev, unsigned int reg) -{ /* we have no readable registers */ - return false; -} - -static bool is31fl3196_volatile_reg(struct device *dev, unsigned int reg) -{ /* volatile registers are not cached */ - switch (reg) { - case IS31FL3196_DATA_UPDATE: - case IS31FL3196_TIME_UPDATE: - case IS31FL3196_RESET: - return true; /* always write-through */ - default: - return false; - } -} - -static const struct reg_default is31fl3196_reg_defaults[] = { - { IS31FL3196_CONFIG1, 0x00 }, - { IS31FL3196_CONFIG2, 0x00 }, - { IS31FL3196_PWM(0), 0x00 }, - { IS31FL3196_PWM(1), 0x00 }, - { IS31FL3196_PWM(2), 0x00 }, - { IS31FL3196_PWM(3), 0x00 }, - { IS31FL3196_PWM(4), 0x00 }, - { IS31FL3196_PWM(5), 0x00 }, - { IS31FL3196_PWM(6), 0x00 }, - { IS31FL3196_PWM(7), 0x00 }, - { IS31FL3196_PWM(8), 0x00 }, -}; - -static struct regmap_config is31fl3196_regmap_config = { - .reg_bits = 8, - .val_bits = 8, - .max_register = IS31FL3196_REG_CNT, - .cache_type = REGCACHE_FLAT, - .readable_reg = is31fl319x_readable_reg, - .volatile_reg = is31fl3196_volatile_reg, - .reg_defaults = is31fl3196_reg_defaults, - .num_reg_defaults = ARRAY_SIZE(is31fl3196_reg_defaults), -}; - static inline int is31fl3196_microamp_to_cs(struct device *dev, u32 microamp) { /* round down to nearest supported value (range check done by caller) */ u32 step = microamp / IS31FL3196_CURRENT_uA_STEP; @@ -343,7 +383,7 @@ static int is31fl319x_probe(struct i2c_client *client, struct device *dev = &client->dev; int err; int i = 0; - u32 aggregated_led_microamp = IS31FL3196_CURRENT_uA_MAX; + u32 aggregated_led_microamp; if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) return -EIO; @@ -365,7 +405,7 @@ static int is31fl319x_probe(struct i2c_client *client, } is31->client = client; - is31->regmap = devm_regmap_init_i2c(client, &is31fl3196_regmap_config); + is31->regmap = devm_regmap_init_i2c(client, is31->cdef->is31fl319x_regmap_config); if (IS_ERR(is31->regmap)) { dev_err(&client->dev, "failed to allocate register map\n"); err = PTR_ERR(is31->regmap); @@ -375,7 +415,7 @@ static int is31fl319x_probe(struct i2c_client *client, i2c_set_clientdata(client, is31); /* check for write-reply from chip (we can't read any registers) */ - err = regmap_write(is31->regmap, IS31FL3196_RESET, 0x00); + err = regmap_write(is31->regmap, is31->cdef->reset_reg, 0x00); if (err < 0) { dev_err(&client->dev, "no response from chip write: err = %d\n", err); @@ -388,14 +428,16 @@ static int is31fl319x_probe(struct i2c_client *client, * But the chip does not allow to limit individual LEDs. * So we take minimum from all subnodes for safety of hardware. */ + aggregated_led_microamp = is31->cdef->current_max; for (i = 0; i < is31->cdef->num_leds; i++) if (is31->leds[i].configured && is31->leds[i].max_microamp < aggregated_led_microamp) aggregated_led_microamp = is31->leds[i].max_microamp; - regmap_write(is31->regmap, IS31FL3196_CONFIG2, - is31fl3196_microamp_to_cs(dev, aggregated_led_microamp) | - is31fl3196_db_to_gain(is31->audio_gain_db)); + if (is31->cdef->is_3196or3199) + regmap_write(is31->regmap, IS31FL3196_CONFIG2, + is31fl3196_microamp_to_cs(dev, aggregated_led_microamp) | + is31fl3196_db_to_gain(is31->audio_gain_db)); for (i = 0; i < is31->cdef->num_leds; i++) { struct is31fl319x_led *led = &is31->leds[i]; @@ -404,7 +446,7 @@ static int is31fl319x_probe(struct i2c_client *client, continue; led->chip = is31; - led->cdev.brightness_set_blocking = is31fl3196_brightness_set; + led->cdev.brightness_set_blocking = is31->cdef->brightness_set; err = devm_led_classdev_register(&client->dev, &led->cdev); if (err < 0) From fa877cf1abb9ef297cfc413b78e998e4fc1394d3 Mon Sep 17 00:00:00 2001 From: Vincent Knecht Date: Tue, 12 Jul 2022 12:08:32 +0200 Subject: [PATCH 18/23] leds: is31fl319x: Add support for is31fl319{0,1,3} chips Set specific chipset structs values for is31fl319{0,1,3} so that those chips can actually work. Datasheets: https://lumissil.com/assets/pdf/core/IS31FL3190_DS.pdf https://lumissil.com/assets/pdf/core/IS31FL3191_DS.pdf https://lumissil.com/assets/pdf/core/IS31FL3193_DS.pdf https://lumissil.com/assets/pdf/core/IS31FL3196_DS.pdf https://lumissil.com/assets/pdf/core/IS31FL3199_DS.pdf Signed-off-by: Vincent Knecht Signed-off-by: Pavel Machek --- drivers/leds/leds-is31fl319x.c | 161 ++++++++++++++++++++++++++++++--- 1 file changed, 147 insertions(+), 14 deletions(-) diff --git a/drivers/leds/leds-is31fl319x.c b/drivers/leds/leds-is31fl319x.c index fe0cb68b583b1..2314922bc0099 100644 --- a/drivers/leds/leds-is31fl319x.c +++ b/drivers/leds/leds-is31fl319x.c @@ -21,6 +21,31 @@ /* register numbers */ #define IS31FL319X_SHUTDOWN 0x00 + +/* registers for 3190, 3191 and 3193 */ +#define IS31FL3190_BREATHING 0x01 +#define IS31FL3190_LEDMODE 0x02 +#define IS31FL3190_CURRENT 0x03 +#define IS31FL3190_PWM(channel) (0x04 + channel) +#define IS31FL3190_DATA_UPDATE 0x07 +#define IS31FL3190_T0(channel) (0x0a + channel) +#define IS31FL3190_T1T2(channel) (0x10 + channel) +#define IS31FL3190_T3T4(channel) (0x16 + channel) +#define IS31FL3190_TIME_UPDATE 0x1c +#define IS31FL3190_LEDCONTROL 0x1d +#define IS31FL3190_RESET 0x2f + +#define IS31FL3190_CURRENT_uA_MIN 5000 +#define IS31FL3190_CURRENT_uA_DEFAULT 42000 +#define IS31FL3190_CURRENT_uA_MAX 42000 +#define IS31FL3190_CURRENT_MASK GENMASK(4, 2) +#define IS31FL3190_CURRENT_5_mA 0x02 +#define IS31FL3190_CURRENT_10_mA 0x01 +#define IS31FL3190_CURRENT_17dot5_mA 0x04 +#define IS31FL3190_CURRENT_30_mA 0x03 +#define IS31FL3190_CURRENT_42_mA 0x00 + +/* registers for 3196 and 3199 */ #define IS31FL3196_CTRL1 0x01 #define IS31FL3196_CTRL2 0x02 #define IS31FL3196_CONFIG1 0x03 @@ -93,6 +118,38 @@ static bool is31fl319x_readable_reg(struct device *dev, unsigned int reg) return false; } +static bool is31fl3190_volatile_reg(struct device *dev, unsigned int reg) +{ + /* volatile registers are not cached */ + switch (reg) { + case IS31FL3190_DATA_UPDATE: + case IS31FL3190_TIME_UPDATE: + case IS31FL3190_RESET: + return true; /* always write-through */ + default: + return false; + } +} + +static const struct reg_default is31fl3190_reg_defaults[] = { + { IS31FL3190_LEDMODE, 0x00 }, + { IS31FL3190_CURRENT, 0x00 }, + { IS31FL3190_PWM(0), 0x00 }, + { IS31FL3190_PWM(1), 0x00 }, + { IS31FL3190_PWM(2), 0x00 }, +}; + +static struct regmap_config is31fl3190_regmap_config = { + .reg_bits = 8, + .val_bits = 8, + .max_register = IS31FL3190_RESET, + .cache_type = REGCACHE_FLAT, + .readable_reg = is31fl319x_readable_reg, + .volatile_reg = is31fl3190_volatile_reg, + .reg_defaults = is31fl3190_reg_defaults, + .num_reg_defaults = ARRAY_SIZE(is31fl3190_reg_defaults), +}; + static bool is31fl3196_volatile_reg(struct device *dev, unsigned int reg) { /* volatile registers are not cached */ @@ -131,6 +188,60 @@ static struct regmap_config is31fl3196_regmap_config = { .num_reg_defaults = ARRAY_SIZE(is31fl3196_reg_defaults), }; +static int is31fl3190_brightness_set(struct led_classdev *cdev, + enum led_brightness brightness) +{ + struct is31fl319x_led *led = container_of(cdev, struct is31fl319x_led, cdev); + struct is31fl319x_chip *is31 = led->chip; + int chan = led - is31->leds; + int ret; + int i; + u8 ctrl = 0; + + dev_dbg(&is31->client->dev, "%s %d: %d\n", __func__, chan, brightness); + + mutex_lock(&is31->lock); + + /* update PWM register */ + ret = regmap_write(is31->regmap, IS31FL3190_PWM(chan), brightness); + if (ret < 0) + goto out; + + /* read current brightness of all PWM channels */ + for (i = 0; i < is31->cdef->num_leds; i++) { + unsigned int pwm_value; + bool on; + + /* + * since neither cdev nor the chip can provide + * the current setting, we read from the regmap cache + */ + + ret = regmap_read(is31->regmap, IS31FL3190_PWM(i), &pwm_value); + on = ret >= 0 && pwm_value > LED_OFF; + + ctrl |= on << i; + } + + if (ctrl > 0) { + dev_dbg(&is31->client->dev, "power up %02x\n", ctrl); + regmap_write(is31->regmap, IS31FL3190_LEDCONTROL, ctrl); + /* update PWMs */ + regmap_write(is31->regmap, IS31FL3190_DATA_UPDATE, 0x00); + /* enable chip from shut down and enable all channels */ + ret = regmap_write(is31->regmap, IS31FL319X_SHUTDOWN, 0x20); + } else { + dev_dbg(&is31->client->dev, "power down\n"); + /* shut down (no need to clear LEDCONTROL) */ + ret = regmap_write(is31->regmap, IS31FL319X_SHUTDOWN, 0x01); + } + +out: + mutex_unlock(&is31->lock); + + return ret; +} + static int is31fl3196_brightness_set(struct led_classdev *cdev, enum led_brightness brightness) { @@ -197,24 +308,24 @@ static int is31fl3196_brightness_set(struct led_classdev *cdev, static const struct is31fl319x_chipdef is31fl3190_cdef = { .num_leds = 1, - .reset_reg = IS31FL3196_RESET, - .is31fl319x_regmap_config = &is31fl3196_regmap_config, - .brightness_set = is31fl3196_brightness_set, - .current_default = IS31FL3196_CURRENT_uA_DEFAULT, - .current_min = IS31FL3196_CURRENT_uA_MIN, - .current_max = IS31FL3196_CURRENT_uA_MAX, - .is_3196or3199 = true, + .reset_reg = IS31FL3190_RESET, + .is31fl319x_regmap_config = &is31fl3190_regmap_config, + .brightness_set = is31fl3190_brightness_set, + .current_default = IS31FL3190_CURRENT_uA_DEFAULT, + .current_min = IS31FL3190_CURRENT_uA_MIN, + .current_max = IS31FL3190_CURRENT_uA_MAX, + .is_3196or3199 = false, }; static const struct is31fl319x_chipdef is31fl3193_cdef = { .num_leds = 3, - .reset_reg = IS31FL3196_RESET, - .is31fl319x_regmap_config = &is31fl3196_regmap_config, - .brightness_set = is31fl3196_brightness_set, - .current_default = IS31FL3196_CURRENT_uA_DEFAULT, - .current_min = IS31FL3196_CURRENT_uA_MIN, - .current_max = IS31FL3196_CURRENT_uA_MAX, - .is_3196or3199 = true, + .reset_reg = IS31FL3190_RESET, + .is31fl319x_regmap_config = &is31fl3190_regmap_config, + .brightness_set = is31fl3190_brightness_set, + .current_default = IS31FL3190_CURRENT_uA_DEFAULT, + .current_min = IS31FL3190_CURRENT_uA_MIN, + .current_max = IS31FL3190_CURRENT_uA_MAX, + .is_3196or3199 = false, }; static const struct is31fl319x_chipdef is31fl3196_cdef = { @@ -362,6 +473,25 @@ static int is31fl319x_parse_dt(struct device *dev, return ret; } +static inline int is31fl3190_microamp_to_cs(struct device *dev, u32 microamp) +{ + switch (microamp) { + case 5000: + return IS31FL3190_CURRENT_5_mA; + case 10000: + return IS31FL3190_CURRENT_10_mA; + case 17500: + return IS31FL3190_CURRENT_17dot5_mA; + case 30000: + return IS31FL3190_CURRENT_30_mA; + case 42000: + return IS31FL3190_CURRENT_42_mA; + default: + dev_warn(dev, "Unsupported current value: %d, using 5000 µA!\n", microamp); + return IS31FL3190_CURRENT_5_mA; + } +} + static inline int is31fl3196_microamp_to_cs(struct device *dev, u32 microamp) { /* round down to nearest supported value (range check done by caller) */ u32 step = microamp / IS31FL3196_CURRENT_uA_STEP; @@ -438,6 +568,9 @@ static int is31fl319x_probe(struct i2c_client *client, regmap_write(is31->regmap, IS31FL3196_CONFIG2, is31fl3196_microamp_to_cs(dev, aggregated_led_microamp) | is31fl3196_db_to_gain(is31->audio_gain_db)); + else + regmap_update_bits(is31->regmap, IS31FL3190_CURRENT, IS31FL3190_CURRENT_MASK, + is31fl3190_microamp_to_cs(dev, aggregated_led_microamp)); for (i = 0; i < is31->cdef->num_leds; i++) { struct is31fl319x_led *led = &is31->leds[i]; From 774268347938bb8e541eb9f6831e24045c353811 Mon Sep 17 00:00:00 2001 From: Vincent Knecht Date: Tue, 12 Jul 2022 12:08:33 +0200 Subject: [PATCH 19/23] leds: is31fl319x: Cleanup formatting and dev_dbg calls Fix remaining non-standard place for comments. Remove remaining dev_dbg after regmap_read in is31fl3196_brightness_set. Remove __func__ in dev_dbg calls, instead adding "channel" string to describe the first displayed value. Change remaining container_of() call to be on one line, as well as a few others. Signed-off-by: Vincent Knecht Signed-off-by: Pavel Machek --- drivers/leds/leds-is31fl319x.c | 22 +++++++++------------- 1 file changed, 9 insertions(+), 13 deletions(-) diff --git a/drivers/leds/leds-is31fl319x.c b/drivers/leds/leds-is31fl319x.c index 2314922bc0099..056cf3bcffdc7 100644 --- a/drivers/leds/leds-is31fl319x.c +++ b/drivers/leds/leds-is31fl319x.c @@ -198,7 +198,7 @@ static int is31fl3190_brightness_set(struct led_classdev *cdev, int i; u8 ctrl = 0; - dev_dbg(&is31->client->dev, "%s %d: %d\n", __func__, chan, brightness); + dev_dbg(&is31->client->dev, "channel %d: %d\n", chan, brightness); mutex_lock(&is31->lock); @@ -245,15 +245,14 @@ static int is31fl3190_brightness_set(struct led_classdev *cdev, static int is31fl3196_brightness_set(struct led_classdev *cdev, enum led_brightness brightness) { - struct is31fl319x_led *led = container_of(cdev, struct is31fl319x_led, - cdev); + struct is31fl319x_led *led = container_of(cdev, struct is31fl319x_led, cdev); struct is31fl319x_chip *is31 = led->chip; int chan = led - is31->leds; int ret; int i; u8 ctrl1 = 0, ctrl2 = 0; - dev_dbg(&is31->client->dev, "%s %d: %d\n", __func__, chan, brightness); + dev_dbg(&is31->client->dev, "channel %d: %d\n", chan, brightness); mutex_lock(&is31->lock); @@ -273,8 +272,6 @@ static int is31fl3196_brightness_set(struct led_classdev *cdev, */ ret = regmap_read(is31->regmap, IS31FL3196_PWM(i), &pwm_value); - dev_dbg(&is31->client->dev, "%s read %d: ret=%d: %d\n", - __func__, i, ret, pwm_value); on = ret >= 0 && pwm_value > LED_OFF; if (i < 3) @@ -404,9 +401,7 @@ static int is31fl319x_parse_dt(struct device *dev, if (!np) return -ENODEV; - is31->shutdown_gpio = devm_gpiod_get_optional(dev, - "shutdown", - GPIOD_OUT_HIGH); + is31->shutdown_gpio = devm_gpiod_get_optional(dev, "shutdown", GPIOD_OUT_HIGH); if (IS_ERR(is31->shutdown_gpio)) { ret = PTR_ERR(is31->shutdown_gpio); dev_err(dev, "Failed to get shutdown gpio: %d\n", ret); @@ -493,7 +488,8 @@ static inline int is31fl3190_microamp_to_cs(struct device *dev, u32 microamp) } static inline int is31fl3196_microamp_to_cs(struct device *dev, u32 microamp) -{ /* round down to nearest supported value (range check done by caller) */ +{ + /* round down to nearest supported value (range check done by caller) */ u32 step = microamp / IS31FL3196_CURRENT_uA_STEP; return ((IS31FL3196_CONFIG2_CS_STEP_REF - step) & @@ -502,7 +498,8 @@ static inline int is31fl3196_microamp_to_cs(struct device *dev, u32 microamp) } static inline int is31fl3196_db_to_gain(u32 dezibel) -{ /* round down to nearest supported value (range check done by caller) */ +{ + /* round down to nearest supported value (range check done by caller) */ return dezibel / IS31FL3196_AUDIO_GAIN_DB_STEP; } @@ -547,8 +544,7 @@ static int is31fl319x_probe(struct i2c_client *client, /* check for write-reply from chip (we can't read any registers) */ err = regmap_write(is31->regmap, is31->cdef->reset_reg, 0x00); if (err < 0) { - dev_err(&client->dev, "no response from chip write: err = %d\n", - err); + dev_err(&client->dev, "no response from chip write: err = %d\n", err); err = -EIO; /* does not answer */ goto free_mutex; } From 69a9b172adbde580a645767a161bb4cc9dc63d9b Mon Sep 17 00:00:00 2001 From: Andy Shevchenko Date: Tue, 12 Jul 2022 12:08:34 +0200 Subject: [PATCH 20/23] leds: is31fl319x: Make use of device properties Convert the module to be property provider agnostic and allow it to be used on non-OF platforms. Add mod_devicetable.h include. Signed-off-by: Andy Shevchenko Signed-off-by: Vincent Knecht Signed-off-by: Pavel Machek --- drivers/leds/Kconfig | 2 +- drivers/leds/leds-is31fl319x.c | 44 ++++++++++++++++------------------ 2 files changed, 21 insertions(+), 25 deletions(-) diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig index eaba0a8347faf..499d0f215a8bf 100644 --- a/drivers/leds/Kconfig +++ b/drivers/leds/Kconfig @@ -697,7 +697,7 @@ config LEDS_MENF21BMC config LEDS_IS31FL319X tristate "LED Support for ISSI IS31FL319x I2C LED controller family" - depends on LEDS_CLASS && I2C && OF + depends on LEDS_CLASS && I2C select REGMAP_I2C help This option enables support for LEDs connected to ISSI IS31FL319x diff --git a/drivers/leds/leds-is31fl319x.c b/drivers/leds/leds-is31fl319x.c index 056cf3bcffdc7..b1a0ca340e245 100644 --- a/drivers/leds/leds-is31fl319x.c +++ b/drivers/leds/leds-is31fl319x.c @@ -11,9 +11,9 @@ #include #include #include +#include #include -#include -#include +#include #include #include #include @@ -362,25 +362,23 @@ static const struct of_device_id of_is31fl319x_match[] = { }; MODULE_DEVICE_TABLE(of, of_is31fl319x_match); -static int is31fl319x_parse_child_dt(const struct device *dev, - const struct device_node *child, +static int is31fl319x_parse_child_fw(const struct device *dev, + const struct fwnode_handle *child, struct is31fl319x_led *led, struct is31fl319x_chip *is31) { struct led_classdev *cdev = &led->cdev; int ret; - if (of_property_read_string(child, "label", &cdev->name)) - cdev->name = child->name; + if (fwnode_property_read_string(child, "label", &cdev->name)) + cdev->name = fwnode_get_name(child); - ret = of_property_read_string(child, "linux,default-trigger", - &cdev->default_trigger); + ret = fwnode_property_read_string(child, "linux,default-trigger", &cdev->default_trigger); if (ret < 0 && ret != -EINVAL) /* is optional */ return ret; led->max_microamp = is31->cdef->current_default; - ret = of_property_read_u32(child, "led-max-microamp", - &led->max_microamp); + ret = fwnode_property_read_u32(child, "led-max-microamp", &led->max_microamp); if (!ret) { if (led->max_microamp < is31->cdef->current_min) return -EINVAL; /* not supported */ @@ -391,16 +389,12 @@ static int is31fl319x_parse_child_dt(const struct device *dev, return 0; } -static int is31fl319x_parse_dt(struct device *dev, - struct is31fl319x_chip *is31) +static int is31fl319x_parse_fw(struct device *dev, struct is31fl319x_chip *is31) { - struct device_node *np = dev_of_node(dev), *child; + struct fwnode_handle *fwnode = dev_fwnode(dev), *child; int count; int ret; - if (!np) - return -ENODEV; - is31->shutdown_gpio = devm_gpiod_get_optional(dev, "shutdown", GPIOD_OUT_HIGH); if (IS_ERR(is31->shutdown_gpio)) { ret = PTR_ERR(is31->shutdown_gpio); @@ -410,7 +404,9 @@ static int is31fl319x_parse_dt(struct device *dev, is31->cdef = device_get_match_data(dev); - count = of_get_available_child_count(np); + count = 0; + fwnode_for_each_available_child_node(fwnode, child) + count++; dev_dbg(dev, "probing with %d leds defined in DT\n", count); @@ -420,11 +416,11 @@ static int is31fl319x_parse_dt(struct device *dev, return -ENODEV; } - for_each_available_child_of_node(np, child) { + fwnode_for_each_available_child_node(fwnode, child) { struct is31fl319x_led *led; u32 reg; - ret = of_property_read_u32(child, "reg", ®); + ret = fwnode_property_read_u32(child, "reg", ®); if (ret) { dev_err(dev, "Failed to read led 'reg' property\n"); goto put_child_node; @@ -444,7 +440,7 @@ static int is31fl319x_parse_dt(struct device *dev, goto put_child_node; } - ret = is31fl319x_parse_child_dt(dev, child, led, is31); + ret = is31fl319x_parse_child_fw(dev, child, led, is31); if (ret) { dev_err(dev, "led %u DT parsing failed\n", reg); goto put_child_node; @@ -455,7 +451,7 @@ static int is31fl319x_parse_dt(struct device *dev, is31->audio_gain_db = 0; if (is31->cdef->is_3196or3199) { - ret = of_property_read_u32(np, "audio-gain-db", &is31->audio_gain_db); + ret = fwnode_property_read_u32(fwnode, "audio-gain-db", &is31->audio_gain_db); if (!ret) is31->audio_gain_db = min(is31->audio_gain_db, IS31FL3196_AUDIO_GAIN_DB_MAX); @@ -464,7 +460,7 @@ static int is31fl319x_parse_dt(struct device *dev, return 0; put_child_node: - of_node_put(child); + fwnode_handle_put(child); return ret; } @@ -521,7 +517,7 @@ static int is31fl319x_probe(struct i2c_client *client, mutex_init(&is31->lock); - err = is31fl319x_parse_dt(&client->dev, is31); + err = is31fl319x_parse_fw(&client->dev, is31); if (err) goto free_mutex; @@ -619,7 +615,7 @@ MODULE_DEVICE_TABLE(i2c, is31fl319x_id); static struct i2c_driver is31fl319x_driver = { .driver = { .name = "leds-is31fl319x", - .of_match_table = of_match_ptr(of_is31fl319x_match), + .of_match_table = of_is31fl319x_match, }, .probe = is31fl319x_probe, .remove = is31fl319x_remove, From 0d77252bc4d28b6b80b4c9b56927cbf74a4d0eb1 Mon Sep 17 00:00:00 2001 From: Andy Shevchenko Date: Tue, 12 Jul 2022 12:08:35 +0200 Subject: [PATCH 21/23] leds: is31fl319x: Make use of dev_err_probe() Simplify the error handling in probe function by switching from dev_err() to dev_err_probe(). Signed-off-by: Andy Shevchenko Signed-off-by: Vincent Knecht Signed-off-by: Pavel Machek --- drivers/leds/leds-is31fl319x.c | 33 +++++++++++++-------------------- 1 file changed, 13 insertions(+), 20 deletions(-) diff --git a/drivers/leds/leds-is31fl319x.c b/drivers/leds/leds-is31fl319x.c index b1a0ca340e245..d9cf70505c43d 100644 --- a/drivers/leds/leds-is31fl319x.c +++ b/drivers/leds/leds-is31fl319x.c @@ -396,11 +396,9 @@ static int is31fl319x_parse_fw(struct device *dev, struct is31fl319x_chip *is31) int ret; is31->shutdown_gpio = devm_gpiod_get_optional(dev, "shutdown", GPIOD_OUT_HIGH); - if (IS_ERR(is31->shutdown_gpio)) { - ret = PTR_ERR(is31->shutdown_gpio); - dev_err(dev, "Failed to get shutdown gpio: %d\n", ret); - return ret; - } + if (IS_ERR(is31->shutdown_gpio)) + return dev_err_probe(dev, PTR_ERR(is31->shutdown_gpio), + "Failed to get shutdown gpio\n"); is31->cdef = device_get_match_data(dev); @@ -410,11 +408,10 @@ static int is31fl319x_parse_fw(struct device *dev, struct is31fl319x_chip *is31) dev_dbg(dev, "probing with %d leds defined in DT\n", count); - if (!count || count > is31->cdef->num_leds) { - dev_err(dev, "Number of leds defined must be between 1 and %u\n", - is31->cdef->num_leds); - return -ENODEV; - } + if (!count || count > is31->cdef->num_leds) + return dev_err_probe(dev, -ENODEV, + "Number of leds defined must be between 1 and %u\n", + is31->cdef->num_leds); fwnode_for_each_available_child_node(fwnode, child) { struct is31fl319x_led *led; @@ -422,27 +419,25 @@ static int is31fl319x_parse_fw(struct device *dev, struct is31fl319x_chip *is31) ret = fwnode_property_read_u32(child, "reg", ®); if (ret) { - dev_err(dev, "Failed to read led 'reg' property\n"); + ret = dev_err_probe(dev, ret, "Failed to read led 'reg' property\n"); goto put_child_node; } if (reg < 1 || reg > is31->cdef->num_leds) { - dev_err(dev, "invalid led reg %u\n", reg); - ret = -EINVAL; + ret = dev_err_probe(dev, -EINVAL, "invalid led reg %u\n", reg); goto put_child_node; } led = &is31->leds[reg - 1]; if (led->configured) { - dev_err(dev, "led %u is already configured\n", reg); - ret = -EINVAL; + ret = dev_err_probe(dev, -EINVAL, "led %u is already configured\n", reg); goto put_child_node; } ret = is31fl319x_parse_child_fw(dev, child, led, is31); if (ret) { - dev_err(dev, "led %u DT parsing failed\n", reg); + ret = dev_err_probe(dev, ret, "led %u DT parsing failed\n", reg); goto put_child_node; } @@ -530,8 +525,7 @@ static int is31fl319x_probe(struct i2c_client *client, is31->client = client; is31->regmap = devm_regmap_init_i2c(client, is31->cdef->is31fl319x_regmap_config); if (IS_ERR(is31->regmap)) { - dev_err(&client->dev, "failed to allocate register map\n"); - err = PTR_ERR(is31->regmap); + err = dev_err_probe(dev, PTR_ERR(is31->regmap), "failed to allocate register map\n"); goto free_mutex; } @@ -540,8 +534,7 @@ static int is31fl319x_probe(struct i2c_client *client, /* check for write-reply from chip (we can't read any registers) */ err = regmap_write(is31->regmap, is31->cdef->reset_reg, 0x00); if (err < 0) { - dev_err(&client->dev, "no response from chip write: err = %d\n", err); - err = -EIO; /* does not answer */ + err = dev_err_probe(dev, err, "no response from chip write\n"); goto free_mutex; } From e1af5c8155867e6773474649c76e5c68940008d5 Mon Sep 17 00:00:00 2001 From: Andy Shevchenko Date: Tue, 12 Jul 2022 12:08:36 +0200 Subject: [PATCH 22/23] leds: is31fl319x: Fix devm vs. non-devm ordering When non-devm resources are allocated they mustn't be followed by devm allocations, otherwise it will break the tear down ordering and might lead to crashes or other bugs during ->remove() stage. Fix this by wrapping mutex_destroy() call with devm_add_action_or_reset(). Signed-off-by: Andy Shevchenko Signed-off-by: Vincent Knecht Signed-off-by: Pavel Machek --- drivers/leds/leds-is31fl319x.c | 32 +++++++++----------------------- 1 file changed, 9 insertions(+), 23 deletions(-) diff --git a/drivers/leds/leds-is31fl319x.c b/drivers/leds/leds-is31fl319x.c index d9cf70505c43d..233ed67830a37 100644 --- a/drivers/leds/leds-is31fl319x.c +++ b/drivers/leds/leds-is31fl319x.c @@ -511,10 +511,13 @@ static int is31fl319x_probe(struct i2c_client *client, return -ENOMEM; mutex_init(&is31->lock); + err = devm_add_action(dev, (void (*)(void *))mutex_destroy, &is31->lock); + if (err) + return err; err = is31fl319x_parse_fw(&client->dev, is31); if (err) - goto free_mutex; + return err; if (is31->shutdown_gpio) { gpiod_direction_output(is31->shutdown_gpio, 0); @@ -524,19 +527,15 @@ static int is31fl319x_probe(struct i2c_client *client, is31->client = client; is31->regmap = devm_regmap_init_i2c(client, is31->cdef->is31fl319x_regmap_config); - if (IS_ERR(is31->regmap)) { - err = dev_err_probe(dev, PTR_ERR(is31->regmap), "failed to allocate register map\n"); - goto free_mutex; - } + if (IS_ERR(is31->regmap)) + return dev_err_probe(dev, PTR_ERR(is31->regmap), "failed to allocate register map\n"); i2c_set_clientdata(client, is31); /* check for write-reply from chip (we can't read any registers) */ err = regmap_write(is31->regmap, is31->cdef->reset_reg, 0x00); - if (err < 0) { - err = dev_err_probe(dev, err, "no response from chip write\n"); - goto free_mutex; - } + if (err < 0) + return dev_err_probe(dev, err, "no response from chip write\n"); /* * Kernel conventions require per-LED led-max-microamp property. @@ -568,22 +567,10 @@ static int is31fl319x_probe(struct i2c_client *client, err = devm_led_classdev_register(&client->dev, &led->cdev); if (err < 0) - goto free_mutex; + return err; } return 0; - -free_mutex: - mutex_destroy(&is31->lock); - return err; -} - -static int is31fl319x_remove(struct i2c_client *client) -{ - struct is31fl319x_chip *is31 = i2c_get_clientdata(client); - - mutex_destroy(&is31->lock); - return 0; } /* @@ -611,7 +598,6 @@ static struct i2c_driver is31fl319x_driver = { .of_match_table = of_is31fl319x_match, }, .probe = is31fl319x_probe, - .remove = is31fl319x_remove, .id_table = is31fl319x_id, }; From 38ba0bb291aacd92d8eaa4a1aa8b63ce4286e797 Mon Sep 17 00:00:00 2001 From: Andy Shevchenko Date: Tue, 12 Jul 2022 12:08:37 +0200 Subject: [PATCH 23/23] leds: is31fl319x: use simple i2c probe function The i2c probe functions here don't use the id information provided in their second argument, so the single-parameter i2c probe function ("probe_new") can be used instead. Signed-off-by: Andy Shevchenko Signed-off-by: Vincent Knecht Signed-off-by: Pavel Machek --- drivers/leds/leds-is31fl319x.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/drivers/leds/leds-is31fl319x.c b/drivers/leds/leds-is31fl319x.c index 233ed67830a37..52b59b62f437c 100644 --- a/drivers/leds/leds-is31fl319x.c +++ b/drivers/leds/leds-is31fl319x.c @@ -494,8 +494,7 @@ static inline int is31fl3196_db_to_gain(u32 dezibel) return dezibel / IS31FL3196_AUDIO_GAIN_DB_STEP; } -static int is31fl319x_probe(struct i2c_client *client, - const struct i2c_device_id *id) +static int is31fl319x_probe(struct i2c_client *client) { struct is31fl319x_chip *is31; struct device *dev = &client->dev; @@ -597,7 +596,7 @@ static struct i2c_driver is31fl319x_driver = { .name = "leds-is31fl319x", .of_match_table = of_is31fl319x_match, }, - .probe = is31fl319x_probe, + .probe_new = is31fl319x_probe, .id_table = is31fl319x_id, };