[PATCH v3 0/2] leds: add aw20xx driver

[PATCH v3 0/2] leds: add aw20xx driver

[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

[PATCH v3 1/2] dt-bindings: leds: add binding for aw200xx

[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

[PATCH v3 2/2] leds: add aw20xx driver

[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

Re: [PATCH v3 2/2] leds: add aw20xx driver

[Martin Kurbanov]

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

Re: [PATCH v3 2/2] leds: add aw20xx driver

[Andy Shevchenko]

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

Re: [PATCH v3 2/2] leds: add aw20xx driver

[Andy Shevchenko]

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

Re: [PATCH v3 2/2] leds: add aw20xx driver

[Andy Shevchenko]

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

Re: [PATCH v3 2/2] leds: add aw20xx driver

[kernel test robot]

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

Re: [PATCH v3 1/2] dt-bindings: leds: add binding for aw200xx

[Krzysztof Kozlowski]

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

Re: [PATCH v3 1/2] dt-bindings: leds: add binding for aw200xx

[Lee Jones]

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 [李琼斯]

Re: [PATCH v3 1/2] dt-bindings: leds: add binding for aw200xx

[Martin Kurbanov]

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

Re: [PATCH v3 1/2] dt-bindings: leds: add binding for aw200xx

[Lee Jones]

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 [李琼斯]