* [PATCH v3 0/2] leds: add aw20xx driver @ 2023-03-14 12:02 Martin Kurbanov 2023-03-14 12:02 ` [PATCH v3 1/2] dt-bindings: leds: add binding for aw200xx Martin Kurbanov 2023-03-14 12:02 ` [PATCH v3 2/2] leds: add aw20xx driver Martin Kurbanov 0 siblings, 2 replies; 12+ messages in thread From: Martin Kurbanov @ 2023-03-14 12:02 UTC (permalink / raw) To: Pavel Machek, Lee Jones, Rob Herring, Krzysztof Kozlowski, Andy Shevchenko Cc: linux-leds, linux-kernel, kernel, devicetree, Martin Kurbanov This patch series adds support for AWINIC AW20036/AW20054/AW20072 LED driver programmed via an I2C interface. This driver supports following AW200XX features: - Individual 64-level DIM currents Datasheet: aw20036 - https://www.awinic.com/en/productDetail/AW20036QNR#tech-docs aw20054 - https://www.awinic.com/en/productDetail/AW20054QNR#tech-docs aw20072 - https://www.awinic.com/en/productDetail/AW20072QNR#tech-docs Add YAML dt-binding schema for AW200XX. Changelog: v2 -> v3: - Update datasheet links - Make cosmetic changes as Andy suggested at [1] v1 -> v2: - Remove the hardware pattern support (I will send a separate patch) - Support the 'led-max-microamp' property [1] https://lore.kernel.org/all/20230228211046.109693-1-mmkurbanov@sberdevices.ru/ Martin Kurbanov (2): dt-bindings: leds: add binding for aw200xx leds: add aw20xx driver .../testing/sysfs-class-led-driver-aw200xx | 5 + .../bindings/leds/awinic,aw200xx.yaml | 126 ++++ drivers/leds/Kconfig | 14 + drivers/leds/Makefile | 1 + drivers/leds/leds-aw200xx.c | 638 ++++++++++++++++++ 5 files changed, 784 insertions(+) create mode 100644 Documentation/ABI/testing/sysfs-class-led-driver-aw200xx create mode 100644 Documentation/devicetree/bindings/leds/awinic,aw200xx.yaml create mode 100644 drivers/leds/leds-aw200xx.c -- 2.37.2 ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v3 1/2] dt-bindings: leds: add binding for aw200xx 2023-03-14 12:02 [PATCH v3 0/2] leds: add aw20xx driver Martin Kurbanov @ 2023-03-14 12:02 ` Martin Kurbanov 2023-03-16 7:11 ` Krzysztof Kozlowski 2023-03-16 16:40 ` Lee Jones 2023-03-14 12:02 ` [PATCH v3 2/2] leds: add aw20xx driver Martin Kurbanov 1 sibling, 2 replies; 12+ messages in thread From: Martin Kurbanov @ 2023-03-14 12:02 UTC (permalink / raw) To: Pavel Machek, Lee Jones, Rob Herring, Krzysztof Kozlowski, Andy Shevchenko Cc: linux-leds, linux-kernel, kernel, devicetree, Martin Kurbanov Add YAML devicetree binding for AWINIC AW20036/AW20052/AW20074 led driver. Signed-off-by: Martin Kurbanov <mmkurbanov@sberdevices.ru> --- .../bindings/leds/awinic,aw200xx.yaml | 126 ++++++++++++++++++ 1 file changed, 126 insertions(+) create mode 100644 Documentation/devicetree/bindings/leds/awinic,aw200xx.yaml diff --git a/Documentation/devicetree/bindings/leds/awinic,aw200xx.yaml b/Documentation/devicetree/bindings/leds/awinic,aw200xx.yaml new file mode 100644 index 000000000000..feb5febaf361 --- /dev/null +++ b/Documentation/devicetree/bindings/leds/awinic,aw200xx.yaml @@ -0,0 +1,126 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/leds/awinic,aw200xx.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: AWINIC AW200XX LED + +maintainers: + - Martin Kurbanov <mmkurbanov@sberdevices.ru> + +description: | + This controller is present on AW20036/AW20054/AW20072. + It is a 3x12/6x9/6x12 matrix LED programmed via + an I2C interface, up to 36/54/72 LEDs or 12/18/24 RGBs, + 3 pattern controllers for auto breathing or group dimming control. + + For more product information please see the link below: + aw20036 - https://www.awinic.com/en/productDetail/AW20036QNR#tech-docs + aw20054 - https://www.awinic.com/en/productDetail/AW20054QNR#tech-docs + aw20072 - https://www.awinic.com/en/productDetail/AW20072QNR#tech-docs + +properties: + compatible: + enum: + - awinic,aw20036 + - awinic,aw20054 + - awinic,aw20072 + + reg: + maxItems: 1 + + "#address-cells": + const: 1 + + "#size-cells": + const: 0 + + awinic,display-rows: + $ref: /schemas/types.yaml#/definitions/uint32 + description: + Leds matrix size + +patternProperties: + "^led@[0-9a-f]$": + type: object + $ref: common.yaml# + unevaluatedProperties: false + + properties: + reg: + description: + LED number + maxItems: 1 + + led-max-microamp: + default: 9780 + description: | + Note that a driver will take the minimum of all LED limits + since the chip has a single global setting. + The maximum output current of each LED is calculated by the + following formula: + IMAXled = 160000 * (592 / 600.5) * (1 / display-rows) + And the minimum output current formula: + IMINled = 3300 * (592 / 600.5) * (1 / display-rows) + +required: + - compatible + - reg + - "#address-cells" + - "#size-cells" + - awinic,display-rows + +allOf: + - if: + properties: + compatible: + contains: + const: awinic,aw20036 + then: + properties: + awinic,display-rows: + enum: [1, 2, 3] + else: + properties: + awinic,display-rows: + enum: [1, 2, 3, 4, 5, 6, 7] + +additionalProperties: false + +examples: + - | + #include <dt-bindings/leds/common.h> + + i2c { + #address-cells = <1>; + #size-cells = <0>; + + led-controller@3a { + compatible = "awinic,aw20036"; + reg = <0x3a>; + #address-cells = <1>; + #size-cells = <0>; + awinic,display-rows = <3>; + + led@0 { + reg = <0x0>; + color = <LED_COLOR_ID_RED>; + led-max-microamp = <9780>; + }; + + led@1 { + reg = <0x1>; + color = <LED_COLOR_ID_GREEN>; + led-max-microamp = <9780>; + }; + + led@2 { + reg = <0x2>; + color = <LED_COLOR_ID_BLUE>; + led-max-microamp = <9780>; + }; + }; + }; + +... -- 2.37.2 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v3 1/2] dt-bindings: leds: add binding for aw200xx 2023-03-14 12:02 ` [PATCH v3 1/2] dt-bindings: leds: add binding for aw200xx Martin Kurbanov @ 2023-03-16 7:11 ` Krzysztof Kozlowski 2023-03-16 16:40 ` Lee Jones 1 sibling, 0 replies; 12+ messages in thread From: Krzysztof Kozlowski @ 2023-03-16 7:11 UTC (permalink / raw) To: Martin Kurbanov, Pavel Machek, Lee Jones, Rob Herring, Krzysztof Kozlowski, Andy Shevchenko Cc: linux-leds, linux-kernel, kernel, devicetree On 14/03/2023 13:02, Martin Kurbanov wrote: > Add YAML devicetree binding for AWINIC AW20036/AW20052/AW20074 > led driver. > > Signed-off-by: Martin Kurbanov <mmkurbanov@sberdevices.ru> > --- Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> Best regards, Krzysztof ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v3 1/2] dt-bindings: leds: add binding for aw200xx 2023-03-14 12:02 ` [PATCH v3 1/2] dt-bindings: leds: add binding for aw200xx Martin Kurbanov 2023-03-16 7:11 ` Krzysztof Kozlowski @ 2023-03-16 16:40 ` Lee Jones 2023-03-16 19:54 ` Martin Kurbanov 1 sibling, 1 reply; 12+ messages in thread From: Lee Jones @ 2023-03-16 16:40 UTC (permalink / raw) To: Martin Kurbanov Cc: Pavel Machek, Rob Herring, Krzysztof Kozlowski, Andy Shevchenko, linux-leds, linux-kernel, kernel, devicetree On Tue, 14 Mar 2023, Martin Kurbanov wrote: > Add YAML devicetree binding for AWINIC AW20036/AW20052/AW20074 > led driver. > > Signed-off-by: Martin Kurbanov <mmkurbanov@sberdevices.ru> > --- > .../bindings/leds/awinic,aw200xx.yaml | 126 ++++++++++++++++++ > 1 file changed, 126 insertions(+) > create mode 100644 Documentation/devicetree/bindings/leds/awinic,aw200xx.yaml Applied, thanks -- Lee Jones [李琼斯] ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v3 1/2] dt-bindings: leds: add binding for aw200xx 2023-03-16 16:40 ` Lee Jones @ 2023-03-16 19:54 ` Martin Kurbanov 2023-03-17 8:09 ` Lee Jones 0 siblings, 1 reply; 12+ messages in thread From: Martin Kurbanov @ 2023-03-16 19:54 UTC (permalink / raw) To: Lee Jones Cc: Pavel Machek, Rob Herring, Krzysztof Kozlowski, Andy Shevchenko, linux-leds, linux-kernel, kernel, devicetree On 2023-03-16 19:40, Lee Jones wrote: > On Tue, 14 Mar 2023, Martin Kurbanov wrote: > >> Add YAML devicetree binding for AWINIC AW20036/AW20052/AW20074 >> led driver. >> >> Signed-off-by: Martin Kurbanov <mmkurbanov@sberdevices.ru> >> --- >> .../bindings/leds/awinic,aw200xx.yaml | 126 ++++++++++++++++++ >> 1 file changed, 126 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/leds/awinic,aw200xx.yaml > > Applied, thanks > > -- > Lee Jones [李琼斯] Hello Lee, Thank you for quick feedback! Sorry, I don't understand one thing. Driver implementation from the patch series must be improved, so currently it's not applied. Does dt bindings make sense without it? I don't think so. Please fix me if I'm wrong. -- Best Regards, Kurbanov Martin ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v3 1/2] dt-bindings: leds: add binding for aw200xx 2023-03-16 19:54 ` Martin Kurbanov @ 2023-03-17 8:09 ` Lee Jones 0 siblings, 0 replies; 12+ messages in thread From: Lee Jones @ 2023-03-17 8:09 UTC (permalink / raw) To: Martin Kurbanov Cc: Pavel Machek, Rob Herring, Krzysztof Kozlowski, Andy Shevchenko, linux-leds, linux-kernel, kernel, devicetree On Thu, 16 Mar 2023, Martin Kurbanov wrote: > On 2023-03-16 19:40, Lee Jones wrote: > > On Tue, 14 Mar 2023, Martin Kurbanov wrote: > > > >> Add YAML devicetree binding for AWINIC AW20036/AW20052/AW20074 > >> led driver. > >> > >> Signed-off-by: Martin Kurbanov <mmkurbanov@sberdevices.ru> > >> --- > >> .../bindings/leds/awinic,aw200xx.yaml | 126 ++++++++++++++++++ > >> 1 file changed, 126 insertions(+) > >> create mode 100644 Documentation/devicetree/bindings/leds/awinic,aw200xx.yaml > > > > Applied, thanks > > Hello Lee, > Thank you for quick feedback! Sorry, I don't understand one thing. > Driver implementation from the patch series must be improved, so > currently it's not applied. Does dt bindings make sense without it? > I don't think so. Please fix me if I'm wrong. No problem. Un-applied, thanks. -- Lee Jones [李琼斯] ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v3 2/2] leds: add aw20xx driver 2023-03-14 12:02 [PATCH v3 0/2] leds: add aw20xx driver Martin Kurbanov 2023-03-14 12:02 ` [PATCH v3 1/2] dt-bindings: leds: add binding for aw200xx Martin Kurbanov @ 2023-03-14 12:02 ` Martin Kurbanov 2023-03-14 12:12 ` Martin Kurbanov ` (2 more replies) 1 sibling, 3 replies; 12+ messages in thread From: Martin Kurbanov @ 2023-03-14 12:02 UTC (permalink / raw) To: Pavel Machek, Lee Jones, Rob Herring, Krzysztof Kozlowski, Andy Shevchenko Cc: linux-leds, linux-kernel, kernel, devicetree, Martin Kurbanov This commit adds support for AWINIC AW20036/AW20054/AW20072 LED driver. This driver supports following AW200XX features: - Individual 64-level DIM currents Signed-off-by: Martin Kurbanov <mmkurbanov@sberdevices.ru> --- .../testing/sysfs-class-led-driver-aw200xx | 5 + drivers/leds/Kconfig | 14 + drivers/leds/Makefile | 1 + drivers/leds/leds-aw200xx.c | 638 ++++++++++++++++++ 4 files changed, 658 insertions(+) create mode 100644 Documentation/ABI/testing/sysfs-class-led-driver-aw200xx create mode 100644 drivers/leds/leds-aw200xx.c diff --git a/Documentation/ABI/testing/sysfs-class-led-driver-aw200xx b/Documentation/ABI/testing/sysfs-class-led-driver-aw200xx new file mode 100644 index 000000000000..2122e3ee3d95 --- /dev/null +++ b/Documentation/ABI/testing/sysfs-class-led-driver-aw200xx @@ -0,0 +1,5 @@ +What: /sys/class/leds/<led>/dim +Date: March 2023 +Description: 64-level DIM current. If you write a negative value or + "auto", the dim will be calculated according to the + brightness. diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig index 499d0f215a8b..546e5aeccc43 100644 --- a/drivers/leds/Kconfig +++ b/drivers/leds/Kconfig @@ -94,6 +94,20 @@ config LEDS_ARIEL Say Y to if your machine is a Dell Wyse 3020 thin client. +config LEDS_AW200XX + tristate "LED support for Awinic AW20036/AW20054/AW20072" + depends on LEDS_CLASS + depends on I2C + depends on OF + help + This option enables support for the AW20036/AW20054/AW20072 LED driver. + It is a 3x12/6x9/6x12 matrix LED driver programmed via + an I2C interface, up to 36/54/72 LEDs or 12/18/24 RGBs, + 3 pattern controllers for auto breathing or group dimming control. + + To compile this driver as a module, choose M here: the module + will be called leds-aw200xx. + config LEDS_AW2013 tristate "LED support for Awinic AW2013" depends on LEDS_CLASS && I2C && OF diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile index 4fd2f92cd198..f611e48cd3f5 100644 --- a/drivers/leds/Makefile +++ b/drivers/leds/Makefile @@ -15,6 +15,7 @@ obj-$(CONFIG_LEDS_AN30259A) += leds-an30259a.o obj-$(CONFIG_LEDS_APU) += leds-apu.o obj-$(CONFIG_LEDS_ARIEL) += leds-ariel.o obj-$(CONFIG_LEDS_ASIC3) += leds-asic3.o +obj-$(CONFIG_LEDS_AW200XX) += leds-aw200xx.o obj-$(CONFIG_LEDS_AW2013) += leds-aw2013.o obj-$(CONFIG_LEDS_BCM6328) += leds-bcm6328.o obj-$(CONFIG_LEDS_BCM6358) += leds-bcm6358.o diff --git a/drivers/leds/leds-aw200xx.c b/drivers/leds/leds-aw200xx.c new file mode 100644 index 000000000000..83cabb85c26f --- /dev/null +++ b/drivers/leds/leds-aw200xx.c @@ -0,0 +1,638 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Awinic AW20036/AW20054/AW20072 LED driver + * + * Copyright (c) 2023, SberDevices. All Rights Reserved. + * + * Author: Martin Kurbanov <mmkurbanov@sberdevices.ru> + */ + +#include <linux/bitfield.h> +#include <linux/i2c.h> +#include <linux/leds.h> +#include <linux/mod_devicetable.h> +#include <linux/module.h> +#include <linux/mutex.h> +#include <linux/regmap.h> + +#define AW200XX_LEDS_MAX 72 +#define AW200XX_PATTERN_MAX 3 +#define AW200XX_DIM_MAX (BIT(6) - 1) +#define AW200XX_FADE_MAX (BIT(8) - 1) +#define AW200XX_IMAX_DEFAULT_uA 60000 +#define AW200XX_IMAX_MAX_uA 160000 +#define AW200XX_IMAX_MIN_uA 3300 + +/* Page 0 */ +#define AW200XX_REG_PAGE0_BASE 0xc000 + +/* Select page register */ +#define AW200XX_REG_PAGE 0xF0 +#define AW200XX_PAGE_MASK (GENMASK(7, 6) | GENMASK(2, 0)) +#define AW200XX_PAGE_SHIFT 0 +#define AW200XX_NUM_PAGES 6 +#define AW200XX_PAGE_SIZE 256 +#define AW200XX_REG(page, reg) \ + (AW200XX_REG_PAGE0_BASE + (page) * AW200XX_PAGE_SIZE + (reg)) +#define AW200XX_REG_MAX \ + AW200XX_REG(AW200XX_NUM_PAGES - 1, AW200XX_PAGE_SIZE - 1) +#define AW200XX_PAGE0 0 +#define AW200XX_PAGE1 1 +#define AW200XX_PAGE2 2 +#define AW200XX_PAGE3 3 +#define AW200XX_PAGE4 4 +#define AW200XX_PAGE5 5 + +/* Chip ID register */ +#define AW200XX_REG_IDR AW200XX_REG(AW200XX_PAGE0, 0x00) +#define AW200XX_IDR_CHIPID 0x18 + +/* Sleep mode register */ +#define AW200XX_REG_SLPCR AW200XX_REG(AW200XX_PAGE0, 0x01) +#define AW200XX_SLPCR_ACTIVE 0x00 + +/* Reset register */ +#define AW200XX_REG_RSTR AW200XX_REG(AW200XX_PAGE0, 0x02) +#define AW200XX_RSTR_RESET 0x01 + +/* Global current configuration register */ +#define AW200XX_REG_GCCR AW200XX_REG(AW200XX_PAGE0, 0x03) +#define AW200XX_GCCR_IMAX_MASK GENMASK(7, 4) +#define AW200XX_GCCR_IMAX(x) ((x) << 4) +#define AW200XX_GCCR_ALLON BIT(3) + +/* Fast clear display control register */ +#define AW200XX_REG_FCD AW200XX_REG(AW200XX_PAGE0, 0x04) +#define AW200XX_FCD_CLEAR 0x01 + +/* Interrupt status register */ +#define AW200XX_REG_ISRFLT AW200XX_REG(AW200XX_PAGE0, 0x0B) +#define AW200XX_ISRFLT_PATIS_MASK GENMASK(6, 4) + +/* Pattern enable control register */ +#define AW200XX_REG_PATCR AW200XX_REG(AW200XX_PAGE0, 0x43) +#define AW200XX_PATCR_PAT_IE_MASK GENMASK(6, 4) +#define AW200XX_PATCR_PAT_IE_ALL AW200XX_PATCR_PAT_IE_MASK +#define AW200XX_PATCR_PAT_ENABLE(x) BIT(x) + +/* + * Maximum breathing level registers + * For patterns 0 - 0x44, 1 - 0x45, 2 - 0x46 (step 1) + */ +#define AW200XX_REG_PAT0_MAX_BREATH AW200XX_REG(AW200XX_PAGE0, 0x44) + +/* + * Minimum breathing level registers + * For patterns 0 - 0x47, 1 - 0x48, 2 - 0x49 (step 1) + */ +#define AW200XX_REG_PAT0_MIN_BREATH AW200XX_REG(AW200XX_PAGE0, 0x47) + +/* + * Template 1 (rise-time) & template 2 (on-time) configuration register + * For patterns 0 - 0x4A, 1 - 0x4E, 2 - 0x52 (step 4) + */ +#define AW200XX_REG_PAT0_T0 AW200XX_REG(AW200XX_PAGE0, 0x4A) + +/* + * Template 3 (fall-time) & template 4 (off-time) configuration register + * For patterns 0 - 0x4B, 1 - 0x4F, 2 - 0x53 (step 4) + */ +#define AW200XX_REG_PAT0_T1 AW200XX_REG(AW200XX_PAGE0, 0x4B) + +/* + * Loop configuration registers: + * loop end point setting (LE) + * loop beginning point setting (LB) + * MSB of loop times (LT) + * For patterns 0 - 0x4C, 1 - 0x50, 2 - 0x54 (step 4) + */ +#define AW200XX_REG_PAT0_T2 AW200XX_REG(AW200XX_PAGE0, 0x4C) +#define AW200XX_REG_PATX_T2(x) (AW200XX_REG_PAT0_T2 + (x)) + +/* + * Loop configuration registers: + * LSB of loop times (LT) + * For patterns 0 - 0x4D, 1 - 0x51, 2 - 0x55 (step 4) + */ +#define AW200XX_REG_PAT0_T3 AW200XX_REG(AW200XX_PAGE0, 0x4D) +#define AW200XX_REG_PATX_T3(x) (AW200XX_REG_PAT0_T3 + (x)) + +#define AW200XX_PAT_T2_LE_MASK GENMASK(7, 6) +#define AW200XX_PAT_T2_LB_MASK GENMASK(5, 4) +#define AW200XX_PAT_T2_LT_MASK GENMASK(3, 0) +#define AW200XX_PAT_T3_LT_MASK GENMASK(7, 0) +#define AW200XX_PAT0_T2_LT_MSB(x) ((x) >> 8) +#define AW200XX_PAT0_T3_LT_LSB(x) ((x) & 0xFF) +#define AW200XX_PAT0_T_LT(msb, lsb) ((msb) << 8 | (lsb)) +#define AW200XX_PAT0_T_LT_MAX (BIT(12) - 1) + +#define AW200XX_PAT_T_STEP 4 + +#define AW200XX_PAT_T1_T3_MASK GENMASK(7, 4) +#define AW200XX_PAT_T2_T4_MASK GENMASK(3, 0) +#define AW200XX_TEMPLATE_TIME_MAX (BIT(4) - 1) + +/* + * Pattern mode configuration register + * For patterns 0 - 0x56, 1 - 0x57, 2 - 0x58 (step 1) + */ +#define AW200XX_REG_PAT0_CFG AW200XX_REG(AW200XX_PAGE0, 0x56) +#define AW200XX_PAT_CFG_MODE_MASK BIT(0) +#define AW200XX_PAT_CFG_RAMP_MASK BIT(1) +#define AW200XX_PAT_CFG_SWITCH_MASK BIT(2) + +/* Start pattern register */ +#define AW200XX_REG_PATGO AW200XX_REG(AW200XX_PAGE0, 0x59) +#define AW200XX_PATGO(x) BIT(x) +#define AW200XX_PATGO_RUN(x, run) ((run) << (x)) +#define AW200XX_PATGO_STATE(x) BIT((x) + 4) + +/* Display size configuration */ +#define AW200XX_REG_DSIZE AW200XX_REG(AW200XX_PAGE0, 0x80) +#define AW200XX_DSIZE_COLUMNS_MAX 12 + +#define AW200XX_LED2REG(x, columns) \ + ((x) + (((x) / (columns)) * (AW200XX_DSIZE_COLUMNS_MAX - (columns)))) + +/* Patern selection register*/ +#define AW200XX_REG_PAT_SELECT(x, columns) \ + AW200XX_REG(AW200XX_PAGE3, AW200XX_LED2REG(x, columns)) +#define AW200XX_PATX_SELECT(x) ((x) + 1) + +/* + * DIM current configuration register (page 4). + * The even address for current DIM configuration. + * The odd address for current FADE configuration + */ +#define AW200XX_REG_DIM(x, columns) \ + AW200XX_REG(AW200XX_PAGE4, AW200XX_LED2REG(x, columns) * 2) +#define AW200XX_REG_DIM2FADE(x) ((x) + 1) +#define AW200XX_REG_FADE(x, columns) (AW200XX_REG_DIM(x, columns) + 1) + +/* Duty ratio of display scan (see p.15 of datasheet for formula) */ +#define AW200XX_DUTY_RATIO(rows) \ + (((592UL * 1000000UL) / 600500UL) * (1000UL / (rows)) / 1000UL) + +struct aw200xx_led { + struct led_classdev cdev; + struct aw200xx *chip; + int dim; + u32 num; +}; + +struct aw200xx { + const struct aw200xx_chipdef *cdef; + struct i2c_client *client; + struct regmap *regmap; + struct mutex mutex; + u32 num_leds; + u32 display_rows; + struct aw200xx_led leds[]; +}; + +struct aw200xx_chipdef { + u32 channels; + u32 display_size_rows_max; + u32 display_size_columns; +}; + +static ssize_t aw200xx_dim_show(struct device *dev, + struct device_attribute *devattr, + char *buf) +{ + struct led_classdev *cdev = dev_get_drvdata(dev); + struct aw200xx_led *led = container_of(cdev, struct aw200xx_led, cdev); + int dim = led->dim; + + if (dim < 0) + return sysfs_emit(buf, "auto\n"); + + return sysfs_emit(buf, "%d\n", dim); +} + +static ssize_t aw200xx_dim_store(struct device *dev, + struct device_attribute *devattr, + const char *buf, size_t count) +{ + struct led_classdev *cdev = dev_get_drvdata(dev); + struct aw200xx_led *led = container_of(cdev, struct aw200xx_led, cdev); + struct aw200xx *chip = led->chip; + u32 columns = chip->cdef->display_size_columns; + int dim; + ssize_t ret; + + if (sysfs_streq(buf, "auto")) { + dim = -1; + } else { + ret = kstrtoint(buf, 0, &dim); + if (ret) + return ret; + + if (dim > AW200XX_DIM_MAX) + return -EINVAL; + } + + mutex_lock(&chip->mutex); + + if (dim >= 0) { + ret = regmap_write(chip->regmap, + AW200XX_REG_DIM(led->num, columns), dim); + if (ret) + goto exit; + } + + led->dim = dim; + ret = count; + +exit: + mutex_unlock(&chip->mutex); + return ret; +} +static DEVICE_ATTR(dim, 0644, aw200xx_dim_show, aw200xx_dim_store); + +static struct attribute *dim_attrs[] = { + &dev_attr_dim.attr, + NULL +}; +ATTRIBUTE_GROUPS(dim); + +static int aw200xx_brightness_set(struct led_classdev *cdev, + enum led_brightness brightness) +{ + struct aw200xx_led *led = container_of(cdev, struct aw200xx_led, cdev); + struct aw200xx *chip = led->chip; + int dim; + u32 reg; + int ret; + + mutex_lock(&chip->mutex); + + reg = AW200XX_REG_DIM(led->num, chip->cdef->display_size_columns); + dim = led->dim; + + if (dim < 0) { + dim = brightness / (AW200XX_FADE_MAX / AW200XX_DIM_MAX); + dim = max(dim, 1); + } + + ret = regmap_write(chip->regmap, reg, dim); + if (ret) + goto error; + + ret = regmap_write(chip->regmap, + AW200XX_REG_DIM2FADE(reg), brightness); + +error: + mutex_unlock(&chip->mutex); + + return ret; +} + +static u32 aw200xx_imax_from_global(const struct aw200xx *const chip, + u32 global_imax_uA) +{ + unsigned long long duty = AW200XX_DUTY_RATIO(chip->display_rows); + + /* The output current of each LED (see p.14 of datasheet for formula) */ + return (duty * global_imax_uA) / 1000U; +} + +static u32 aw200xx_imax_to_global(const struct aw200xx *const chip, + u32 led_imax_uA) +{ + u32 duty = AW200XX_DUTY_RATIO(chip->display_rows); + + /* The output current of each LED (see p.14 of datasheet for formula) */ + return (led_imax_uA * 1000U) / duty; +} + +static int aw200xx_set_imax(const struct aw200xx *const chip, + u32 led_imax_uA) +{ + struct imax_global { + u32 regval; + u32 microamp; + } imaxs[] = { + { 8, 3300 }, + { 9, 6700 }, + { 0, 10000 }, + { 11, 13300 }, + { 1, 20000 }, + { 13, 26700 }, + { 2, 30000 }, + { 3, 40000 }, + { 15, 53300 }, + { 4, 60000 }, + { 5, 80000 }, + { 6, 120000 }, + { 7, 160000 }, + }; + u32 g_imax_uA = aw200xx_imax_to_global(chip, led_imax_uA); + int i = ARRAY_SIZE(imaxs); + + while (i--) { + if (g_imax_uA >= imaxs[i].microamp) + break; + } + if (i < 0) + return -EINVAL; + + return regmap_update_bits(chip->regmap, AW200XX_REG_GCCR, + AW200XX_GCCR_IMAX_MASK, + AW200XX_GCCR_IMAX(imaxs[i].regval)); +} + +static int aw200xx_chip_reset(const struct aw200xx *const chip) +{ + int ret; + + ret = regmap_write(chip->regmap, AW200XX_REG_RSTR, AW200XX_RSTR_RESET); + if (ret) + return ret; + + regcache_mark_dirty(chip->regmap); + return regmap_write(chip->regmap, AW200XX_REG_FCD, AW200XX_FCD_CLEAR); +} + +static int aw200xx_chip_init(const struct aw200xx *const chip) +{ + int ret; + + ret = regmap_write(chip->regmap, AW200XX_REG_DSIZE, + chip->display_rows - 1); + if (ret) + return ret; + + ret = regmap_write(chip->regmap, AW200XX_REG_SLPCR, + AW200XX_SLPCR_ACTIVE); + if (ret) + return ret; + + return regmap_update_bits(chip->regmap, AW200XX_REG_GCCR, + AW200XX_GCCR_ALLON, AW200XX_GCCR_ALLON); +} + +static int aw200xx_chip_check(const struct aw200xx *const chip) +{ + struct device *dev = &chip->client->dev; + u32 chipid; + int ret; + + ret = regmap_read(chip->regmap, AW200XX_REG_IDR, &chipid); + if (ret) + return dev_err_probe(dev, ret, "Failed to read chip ID\n"); + + if (chipid != AW200XX_IDR_CHIPID) + return dev_err_probe(dev, -ENODEV, + "Chip reported wrong ID: %x\n", chipid); + + return 0; +} + +static int aw200xx_probe_dt(struct device *dev, struct aw200xx *chip) +{ + struct fwnode_handle *child; + u32 current_min, current_max, min_uA; + int ret; + int i = 0; + + ret = device_property_read_u32(dev, "awinic,display-rows", + &chip->display_rows); + if (ret) + return dev_err_probe(dev, ret, + "Failed to read 'display-rows' property\n"); + + if (!chip->display_rows || + chip->display_rows > chip->cdef->display_size_rows_max) { + return dev_err_probe(dev, ret, + "Invalid leds display size %u\n", + chip->display_rows); + } + + current_max = aw200xx_imax_from_global(chip, AW200XX_IMAX_MAX_uA); + current_min = aw200xx_imax_from_global(chip, AW200XX_IMAX_MIN_uA); + min_uA = UINT_MAX; + + device_for_each_child_node(dev, child) { + struct led_init_data init_data = {}; + struct aw200xx_led *led; + u32 source, imax; + + ret = fwnode_property_read_u32(child, "reg", &source); + if (ret) { + dev_err(dev, "Missing reg property\n"); + chip->num_leds--; + continue; + } + + if (source >= chip->cdef->channels) { + dev_err(dev, "LED reg %u out of range (max %u)\n", + source, chip->cdef->channels); + chip->num_leds--; + continue; + } + + ret = fwnode_property_read_u32(child, "led-max-microamp", + &imax); + if (ret) { + dev_info(&chip->client->dev, + "DT property led-max-microamp is missing\n"); + } else if (imax < current_min || imax > current_max) { + dev_err(dev, "Invalid value %u for led-max-microamp\n", + imax); + chip->num_leds--; + continue; + } else { + min_uA = min(min_uA, imax); + } + + led = &chip->leds[i]; + led->dim = -1; + led->num = source; + led->chip = chip; + led->cdev.brightness_set_blocking = aw200xx_brightness_set; + led->cdev.groups = dim_groups; + init_data.fwnode = child; + + ret = devm_led_classdev_register_ext(dev, &led->cdev, + &init_data); + if (ret) { + fwnode_handle_put(child); + break; + } + + i++; + } + + if (!chip->num_leds) + return -EINVAL; + + if (min_uA == UINT_MAX) { + min_uA = aw200xx_imax_from_global(chip, + AW200XX_IMAX_DEFAULT_uA); + } + + return aw200xx_set_imax(chip, min_uA); +} + +static const struct regmap_range_cfg aw200xx_ranges[] = { + { + .name = "aw200xx", + .range_min = 0, + .range_max = AW200XX_REG_MAX, + .selector_reg = AW200XX_REG_PAGE, + .selector_mask = AW200XX_PAGE_MASK, + .selector_shift = AW200XX_PAGE_SHIFT, + .window_start = 0, + .window_len = AW200XX_PAGE_SIZE, + }, +}; + +static const struct regmap_range aw200xx_writeonly_ranges[] = { + regmap_reg_range(AW200XX_REG(AW200XX_PAGE1, 0x00), AW200XX_REG_MAX), +}; + +static const struct regmap_access_table aw200xx_readable_table = { + .no_ranges = aw200xx_writeonly_ranges, + .n_no_ranges = ARRAY_SIZE(aw200xx_writeonly_ranges), +}; + +static const struct regmap_range aw200xx_readonly_ranges[] = { + regmap_reg_range(AW200XX_REG_IDR, AW200XX_REG_IDR), + regmap_reg_range(AW200XX_REG_ISRFLT, AW200XX_REG_ISRFLT), +}; + +static const struct regmap_access_table aw200xx_writeable_table = { + .no_ranges = aw200xx_readonly_ranges, + .n_no_ranges = ARRAY_SIZE(aw200xx_readonly_ranges), +}; + +static const struct regmap_range aw200xx_volatile_registers[] = { + regmap_reg_range(AW200XX_REG_ISRFLT, AW200XX_REG_ISRFLT), +}; + +static const struct regmap_access_table aw200xx_volatile_table = { + .yes_ranges = aw200xx_volatile_registers, + .n_yes_ranges = ARRAY_SIZE(aw200xx_volatile_registers), +}; + +static const struct regmap_config aw200xx_regmap_config = { + .reg_bits = 8, + .val_bits = 8, + .max_register = AW200XX_REG_MAX, + .ranges = aw200xx_ranges, + .num_ranges = ARRAY_SIZE(aw200xx_ranges), + .rd_table = &aw200xx_readable_table, + .wr_table = &aw200xx_writeable_table, + .volatile_table = &aw200xx_volatile_table, + .cache_type = REGCACHE_RBTREE, +}; + +static int aw200xx_probe(struct i2c_client *client) +{ + const struct aw200xx_chipdef *cdef; + struct aw200xx *chip; + int count; + int ret; + + cdef = device_get_match_data(&client->dev); + + count = device_get_child_node_count(&client->dev); + if (!count || count > cdef->channels) + return dev_err_probe(&client->dev, -EINVAL, + "Incorrect number of leds (%d)", count); + + chip = devm_kzalloc(&client->dev, struct_size(chip, leds, count), + GFP_KERNEL); + if (!chip) + return -ENOMEM; + + chip->cdef = cdef; + chip->num_leds = count; + chip->client = client; + i2c_set_clientdata(client, chip); + + chip->regmap = devm_regmap_init_i2c(client, &aw200xx_regmap_config); + if (IS_ERR(chip->regmap)) + return PTR_ERR(chip->regmap); + + ret = aw200xx_chip_check(chip); + if (ret) + return ret; + + mutex_init(&chip->mutex); + + /* Need a lock now since after call aw200xx_probe_dt, sysfs nodes created */ + mutex_lock(&chip->mutex); + + ret = aw200xx_probe_dt(&client->dev, chip); + if (ret < 0) + goto exit; + + ret = aw200xx_chip_reset(chip); + if (ret) + goto exit; + + ret = aw200xx_chip_init(chip); + +exit: + mutex_unlock(&chip->mutex); + return ret; +} + +static void aw200xx_remove(struct i2c_client *client) +{ + struct aw200xx *chip = i2c_get_clientdata(client); + + aw200xx_chip_reset(chip); + mutex_destroy(&chip->mutex); +} + +static const struct aw200xx_chipdef aw20036_cdef = { + .channels = 36, + .display_size_rows_max = 3, + .display_size_columns = 12, +}; + +static const struct aw200xx_chipdef aw20054_cdef = { + .channels = 54, + .display_size_rows_max = 6, + .display_size_columns = 9, +}; + +static const struct aw200xx_chipdef aw20072_cdef = { + .channels = 72, + .display_size_rows_max = 6, + .display_size_columns = 12, +}; + +static const struct i2c_device_id aw200xx_id[] = { + { "aw20036" }, + { "aw20054" }, + { "aw20072" }, + {} +}; +MODULE_DEVICE_TABLE(i2c, aw200xx_id); + +static const struct of_device_id aw200xx_match_table[] = { + { .compatible = "awinic,aw20036", .data = &aw20036_cdef, }, + { .compatible = "awinic,aw20054", .data = &aw20054_cdef, }, + { .compatible = "awinic,aw20072", .data = &aw20072_cdef, }, + {} +}; +MODULE_DEVICE_TABLE(of, aw200xx_match_table); + +static struct i2c_driver aw200xx_driver = { + .driver = { + .name = "aw200xx", + .of_match_table = aw200xx_match_table, + }, + .probe_new = aw200xx_probe, + .remove = aw200xx_remove, + .id_table = aw200xx_id, +}; +module_i2c_driver(aw200xx_driver); + +MODULE_AUTHOR("Martin Kurbanov <mmkurbanov@sberdevices.ru>"); +MODULE_DESCRIPTION("AW200XX LED driver"); +MODULE_LICENSE("GPL"); -- 2.37.2 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v3 2/2] leds: add aw20xx driver 2023-03-14 12:02 ` [PATCH v3 2/2] leds: add aw20xx driver Martin Kurbanov @ 2023-03-14 12:12 ` Martin Kurbanov 2023-03-14 16:22 ` Andy Shevchenko 2023-03-14 16:44 ` Andy Shevchenko 2023-03-15 9:59 ` kernel test robot 2 siblings, 1 reply; 12+ messages in thread From: Martin Kurbanov @ 2023-03-14 12:12 UTC (permalink / raw) To: Andy Shevchenko Cc: Pavel Machek, Lee Jones, Rob Herring, Krzysztof Kozlowski, linux-leds, linux-kernel, kernel, devicetree Hello Andy. Thank you for review. I have fixed most of your comments. Please take a look below. On 2023-03-01 00:51, Andy Shevchenko wrote: >> + /* The output current of each LED (see p.14 of datasheet for formula) */ >> + return (duty * global_imax_microamp) / 1000U; > > units.h ? These constants are needed to improve the accuracy of calculations. units.h doesn’t have any helpful definitions to use here. >> +static int aw200xx_set_imax(const struct aw200xx *const chip, >> + u32 led_imax_microamp) >> +{ >> + struct imax_global { >> + u32 regval; >> + u32 microamp; >> + } imaxs[] = { >> + { 8, 3300 }, >> + { 9, 6700 }, >> + { 0, 10000 }, >> + { 11, 13300 }, >> + { 1, 20000 }, >> + { 13, 26700 }, >> + { 2, 30000 }, >> + { 3, 40000 }, >> + { 15, 53300 }, >> + { 4, 60000 }, >> + { 5, 80000 }, >> + { 6, 120000 }, >> + { 7, 160000 }, > > This looks a bit random. Is there any pattern on how value is > connected to the register value? There is no ability to create any pattern here, because this table data doesn’t have any regularity. -- Best Regards, Kurbanov Martin ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v3 2/2] leds: add aw20xx driver 2023-03-14 12:12 ` Martin Kurbanov @ 2023-03-14 16:22 ` Andy Shevchenko 2023-03-14 16:49 ` Andy Shevchenko 0 siblings, 1 reply; 12+ messages in thread From: Andy Shevchenko @ 2023-03-14 16:22 UTC (permalink / raw) To: Martin Kurbanov Cc: Pavel Machek, Lee Jones, Rob Herring, Krzysztof Kozlowski, linux-leds, linux-kernel, kernel, devicetree On Tue, Mar 14, 2023 at 2:12 PM Martin Kurbanov <mmkurbanov@sberdevices.ru> wrote: > > Hello Andy. Thank you for review. > I have fixed most of your comments. Please take a look below. Good, but you can postpone issuing a new version before letting me answer. > On 2023-03-01 00:51, Andy Shevchenko wrote: > >> + /* The output current of each LED (see p.14 of datasheet for formula) */ > >> + return (duty * global_imax_microamp) / 1000U; > > > > units.h ? > > These constants are needed to improve the accuracy of calculations. > units.h doesn’t have any helpful definitions to use here. Okay, let me look at v3 and I will comment there. ... > >> +static int aw200xx_set_imax(const struct aw200xx *const chip, > >> + u32 led_imax_microamp) > >> +{ > >> + struct imax_global { > >> + u32 regval; > >> + u32 microamp; > >> + } imaxs[] = { > >> + { 8, 3300 }, > >> + { 9, 6700 }, > >> + { 0, 10000 }, > >> + { 11, 13300 }, > >> + { 1, 20000 }, > >> + { 13, 26700 }, > >> + { 2, 30000 }, > >> + { 3, 40000 }, > >> + { 15, 53300 }, > >> + { 4, 60000 }, > >> + { 5, 80000 }, > >> + { 6, 120000 }, > >> + { 7, 160000 }, > > > > This looks a bit random. Is there any pattern on how value is > > connected to the register value? > > There is no ability to create any pattern here, because this table data > doesn’t have any regularity. There is a clear pattern. You have two tables, i.e. with multiplier 10000 and second one with multiplier 3333 (table in the datasheet seems bad from a math perspective). And it's even shown correctly in the datasheet. With this mix you missed 10. The coefficient table is 1,2,3,4,6,8,12,16 for both tables. Hence you need one table and two multipliers. Please, rewrite accordingly. -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v3 2/2] leds: add aw20xx driver 2023-03-14 16:22 ` Andy Shevchenko @ 2023-03-14 16:49 ` Andy Shevchenko 0 siblings, 0 replies; 12+ messages in thread From: Andy Shevchenko @ 2023-03-14 16:49 UTC (permalink / raw) To: Martin Kurbanov Cc: Pavel Machek, Lee Jones, Rob Herring, Krzysztof Kozlowski, linux-leds, linux-kernel, kernel, devicetree On Tue, Mar 14, 2023 at 6:22 PM Andy Shevchenko <andy.shevchenko@gmail.com> wrote: > On Tue, Mar 14, 2023 at 2:12 PM Martin Kurbanov > <mmkurbanov@sberdevices.ru> wrote: > > On 2023-03-01 00:51, Andy Shevchenko wrote: ... > > >> +static int aw200xx_set_imax(const struct aw200xx *const chip, > > >> + u32 led_imax_microamp) > > >> +{ > > >> + struct imax_global { > > >> + u32 regval; > > >> + u32 microamp; > > >> + } imaxs[] = { > > >> + { 8, 3300 }, > > >> + { 9, 6700 }, > > >> + { 0, 10000 }, > > >> + { 11, 13300 }, > > >> + { 1, 20000 }, > > >> + { 13, 26700 }, > > >> + { 2, 30000 }, > > >> + { 3, 40000 }, > > >> + { 15, 53300 }, > > >> + { 4, 60000 }, > > >> + { 5, 80000 }, > > >> + { 6, 120000 }, > > >> + { 7, 160000 }, > > > > > > This looks a bit random. Is there any pattern on how value is > > > connected to the register value? > > > > There is no ability to create any pattern here, because this table data > > doesn’t have any regularity. > > There is a clear pattern. > > You have two tables, i.e. with multiplier 10000 and second one with > multiplier 3333 (table in the datasheet seems bad from a math > perspective). And it's even shown correctly in the datasheet. > > With this mix you missed 10. > > The coefficient table is 1,2,3,4,6,8,12,16 for both tables. > > Hence you need one table and two multipliers. > > Please, rewrite accordingly. JFYI: You may see how I killed a table in one driver due to missing understanding that there is a pattern. 9df461eca18f ("spi: pxa2xx: replace ugly table by approximation") -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v3 2/2] leds: add aw20xx driver 2023-03-14 12:02 ` [PATCH v3 2/2] leds: add aw20xx driver Martin Kurbanov 2023-03-14 12:12 ` Martin Kurbanov @ 2023-03-14 16:44 ` Andy Shevchenko 2023-03-15 9:59 ` kernel test robot 2 siblings, 0 replies; 12+ messages in thread From: Andy Shevchenko @ 2023-03-14 16:44 UTC (permalink / raw) To: Martin Kurbanov Cc: Pavel Machek, Lee Jones, Rob Herring, Krzysztof Kozlowski, linux-leds, linux-kernel, kernel, devicetree On Tue, Mar 14, 2023 at 2:03 PM Martin Kurbanov <mmkurbanov@sberdevices.ru> wrote: > > This commit adds support for AWINIC AW20036/AW20054/AW20072 LED driver. > This driver supports following AW200XX features: > - Individual 64-level DIM currents As said, please give a chance reviewer to answer your questions before issuing a new version. You will save yours time in the first place and reviewer's and others' at the second. ... > +config LEDS_AW200XX > + tristate "LED support for Awinic AW20036/AW20054/AW20072" > + depends on LEDS_CLASS > + depends on I2C > + depends on OF I don't see any dependency. Do you? ... > +#include <linux/bitfield.h> bits.h needs to be included, it's not guaranteed to be by any of the present headers in this list. > +#include <linux/i2c.h> > +#include <linux/leds.h> > +#include <linux/mod_devicetable.h> > +#include <linux/module.h> > +#include <linux/mutex.h> > +#include <linux/regmap.h> + container_of.h ... > +#define AW200XX_PAT0_T2_LT_MSB(x) ((x) >> 8) > +#define AW200XX_PAT0_T3_LT_LSB(x) ((x) & 0xFF) > +#define AW200XX_PAT0_T_LT(msb, lsb) ((msb) << 8 | (lsb)) Are these being in use anyhow? Please, do not add dead definitions if they are not currently in use and do not add value to understanding how hardware works for the implemented things. ... > +/* Patern selection register*/ Pattern ... > +/* Duty ratio of display scan (see p.15 of datasheet for formula) */ > +#define AW200XX_DUTY_RATIO(rows) \ > + (((592UL * 1000000UL) / 600500UL) * (1000UL / (rows)) / 1000UL) Instead of referring to a page in the datasheet, quote the formula and a bit of text. So, now as I read it I can tell how it can be improved. The first six 0:s multiplier is actually USEC_PER_SEC in the kernel. The 1000 is your addition which needs to be explained. It may be just MILLI or KILO depending on what you have in mind with it. ... > +struct aw200xx { > + const struct aw200xx_chipdef *cdef; > + struct i2c_client *client; > + struct regmap *regmap; You may derive this from &client->dev, right? The API dev_get_regmap(). > + struct mutex mutex; > + u32 num_leds; > + u32 display_rows; > + struct aw200xx_led leds[]; > +}; ... > + if (sysfs_streq(buf, "auto")) { > + dim = -1; > + } else { > + ret = kstrtoint(buf, 0, &dim); > + if (ret) > + return ret; > + > + if (dim > AW200XX_DIM_MAX) > + return -EINVAL; And if dim is < 0, does ir mean "auto"? If so, is it documented? > + } ... > +static DEVICE_ATTR(dim, 0644, aw200xx_dim_show, aw200xx_dim_store); DEVICE_ATTR_RW() ? ... > + mutex_lock(&chip->mutex); > + > + reg = AW200XX_REG_DIM(led->num, chip->cdef->display_size_columns); > + dim = led->dim; > + This blank line should go before a dim assignment. > + if (dim < 0) { > + dim = brightness / (AW200XX_FADE_MAX / AW200XX_DIM_MAX); > + dim = max(dim, 1); Can it be written in a single assignment? dim = max(brightness / (AW200XX_FADE_MAX / AW200XX_DIM_MAX), 1) > + } ... > +error: In one function it's called the exit, here it's the error, please be consistent and I think the more precise is to use something like 'out_unlock'. > + mutex_unlock(&chip->mutex); > + > + return ret; > +} ... > + /* The output current of each LED (see p.14 of datasheet for formula) */ Again, put more comments. Do not push the reader to find a Datasheet which I do not see the link to in the commit message. > + return (duty * global_imax_uA) / 1000U; So, this seems like the same meaning as 1000 in the macro, correct? Please explain the use of it. The best, if you think units.h doesn't provide it, define in your driver. ... > + /* The output current of each LED (see p.14 of datasheet for formula) */ > + return (led_imax_uA * 1000U) / duty; All the same as per above. ... > + { > + { 8, 3300 }, > + { 9, 6700 }, > + { 0, 10000 }, > + { 11, 13300 }, > + { 1, 20000 }, > + { 13, 26700 }, > + { 2, 30000 }, > + { 3, 40000 }, > + { 15, 53300 }, > + { 4, 60000 }, > + { 5, 80000 }, > + { 6, 120000 }, > + { 7, 160000 }, > + }; Yeah, as per previous email, use exactly what Datasheet provides you. There is a clear pattern and clear math mistakes in the datasheet itself. ... > +static const struct regmap_config aw200xx_regmap_config = { > + .reg_bits = 8, > + .val_bits = 8, > + .max_register = AW200XX_REG_MAX, > + .ranges = aw200xx_ranges, > + .num_ranges = ARRAY_SIZE(aw200xx_ranges), > + .rd_table = &aw200xx_readable_table, > + .wr_table = &aw200xx_writeable_table, > + .volatile_table = &aw200xx_volatile_table, > + .cache_type = REGCACHE_RBTREE, You have a mutex, why do you need to use regmap's lock? > +}; ... > + cdef = device_get_match_data(&client->dev); Is it fine if it's NULL? ... > + chip->cdef = cdef; Can it be assigned directly here w.o temporary variable? -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v3 2/2] leds: add aw20xx driver 2023-03-14 12:02 ` [PATCH v3 2/2] leds: add aw20xx driver Martin Kurbanov 2023-03-14 12:12 ` Martin Kurbanov 2023-03-14 16:44 ` Andy Shevchenko @ 2023-03-15 9:59 ` kernel test robot 2 siblings, 0 replies; 12+ messages in thread From: kernel test robot @ 2023-03-15 9:59 UTC (permalink / raw) To: Martin Kurbanov, Pavel Machek, Lee Jones, Rob Herring, Krzysztof Kozlowski, Andy Shevchenko Cc: oe-kbuild-all, linux-leds, linux-kernel, kernel, devicetree, Martin Kurbanov Hi Martin, Thank you for the patch! Yet something to improve: [auto build test ERROR on pavel-leds/for-next] [cannot apply to lee-leds/for-leds-next robh/for-next linus/master v6.3-rc2 next-20230315] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Martin-Kurbanov/leds-add-aw20xx-driver/20230314-210251 base: git://git.kernel.org/pub/scm/linux/kernel/git/pavel/linux-leds.git for-next patch link: https://lore.kernel.org/r/20230314120252.48263-3-mmkurbanov%40sberdevices.ru patch subject: [PATCH v3 2/2] leds: add aw20xx driver config: mips-allyesconfig (https://download.01.org/0day-ci/archive/20230315/202303151746.f6VVX4iZ-lkp@intel.com/config) compiler: mips-linux-gcc (GCC) 12.1.0 reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # https://github.com/intel-lab-lkp/linux/commit/9760204574f3c53a9753b6daf20125c8552ce8ae git remote add linux-review https://github.com/intel-lab-lkp/linux git fetch --no-tags linux-review Martin-Kurbanov/leds-add-aw20xx-driver/20230314-210251 git checkout 9760204574f3c53a9753b6daf20125c8552ce8ae # save the config file mkdir build_dir && cp config build_dir/.config COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=mips olddefconfig COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=mips SHELL=/bin/bash If you fix the issue, kindly add following tag where applicable | Reported-by: kernel test robot <lkp@intel.com> | Link: https://lore.kernel.org/oe-kbuild-all/202303151746.f6VVX4iZ-lkp@intel.com/ All errors (new ones prefixed by >>): arch/mips/kernel/head.o: in function `kernel_entry': (.ref.text+0xac): relocation truncated to fit: R_MIPS_26 against `start_kernel' init/main.o: in function `set_reset_devices': main.c:(.init.text+0x20): relocation truncated to fit: R_MIPS_26 against `_mcount' main.c:(.init.text+0x30): relocation truncated to fit: R_MIPS_26 against `__sanitizer_cov_trace_pc' init/main.o: in function `debug_kernel': main.c:(.init.text+0xa4): relocation truncated to fit: R_MIPS_26 against `_mcount' main.c:(.init.text+0xb4): relocation truncated to fit: R_MIPS_26 against `__sanitizer_cov_trace_pc' init/main.o: in function `quiet_kernel': main.c:(.init.text+0x128): relocation truncated to fit: R_MIPS_26 against `_mcount' main.c:(.init.text+0x138): relocation truncated to fit: R_MIPS_26 against `__sanitizer_cov_trace_pc' init/main.o: in function `warn_bootconfig': main.c:(.init.text+0x1ac): relocation truncated to fit: R_MIPS_26 against `_mcount' main.c:(.init.text+0x1bc): relocation truncated to fit: R_MIPS_26 against `__sanitizer_cov_trace_pc' init/main.o: in function `init_setup': main.c:(.init.text+0x234): relocation truncated to fit: R_MIPS_26 against `_mcount' main.c:(.init.text+0x254): additional relocation overflows omitted from the output mips-linux-ld: drivers/leds/leds-aw200xx.o: in function `aw200xx_probe_dt': >> leds-aw200xx.c:(.text.aw200xx_probe_dt+0x164): undefined reference to `__udivdi3' >> mips-linux-ld: leds-aw200xx.c:(.text.aw200xx_probe_dt+0x184): undefined reference to `__udivdi3' mips-linux-ld: leds-aw200xx.c:(.text.aw200xx_probe_dt+0x53c): undefined reference to `__udivdi3' -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2023-03-17 8:11 UTC | newest] Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2023-03-14 12:02 [PATCH v3 0/2] leds: add aw20xx driver Martin Kurbanov 2023-03-14 12:02 ` [PATCH v3 1/2] dt-bindings: leds: add binding for aw200xx Martin Kurbanov 2023-03-16 7:11 ` Krzysztof Kozlowski 2023-03-16 16:40 ` Lee Jones 2023-03-16 19:54 ` Martin Kurbanov 2023-03-17 8:09 ` Lee Jones 2023-03-14 12:02 ` [PATCH v3 2/2] leds: add aw20xx driver Martin Kurbanov 2023-03-14 12:12 ` Martin Kurbanov 2023-03-14 16:22 ` Andy Shevchenko 2023-03-14 16:49 ` Andy Shevchenko 2023-03-14 16:44 ` Andy Shevchenko 2023-03-15 9:59 ` kernel test robot
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.