linux-leds.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH V3 0/4] Add support for WLED5
@ 2020-03-09 13:25 Kiran Gunda
  2020-03-09 13:25 ` [PATCH V3 1/4] backlight: qcom-wled: convert the wled bindings to .yaml format Kiran Gunda
                   ` (3 more replies)
  0 siblings, 4 replies; 15+ messages in thread
From: Kiran Gunda @ 2020-03-09 13:25 UTC (permalink / raw)
  To: bjorn.andersson, jingoohan1, lee.jones, b.zolnierkie, dri-devel,
	daniel.thompson, jacek.anaszewski, pavel, robh+dt, mark.rutland,
	linux-leds, devicetree, linux-kernel
  Cc: linux-arm-msm, Kiran Gunda


Currently, WLED driver supports only WLED4 peripherals that is present
on pmi8998 and pm660L. This patch series  converts the existing WLED4
bindings from .txt to .yaml format and adds the support for WLED5 peripheral
that is present on PM8150L.

PM8150L WLED supports the following.
    - Two modulators and each sink can use any of the modulator
    - Multiple CABC selection options
    - Multiple brightness width selection (12 bits to 15 bits)

Changes from V1:
	- Rebased on top of the below commit.
	  backlight: qcom-wled: Fix unsigned comparison to zero

Changes from V2:
	- Addressed Bjorn's comments by splitting the WLED4 changes
	  in a seperate patch.
	- Added WLED5 auto calibration support

Kiran Gunda (4):
  backlight: qcom-wled: convert the wled bindings to .yaml format
  backlight: qcom-wled: Add callbacks functions
  backlight: qcom-wled: Add support for WLED5 peripheral in PM8150L
  backlight: qcom-wled: Update auto calibration support for WLED5

 .../bindings/leds/backlight/qcom-wled.txt          | 154 -----
 .../bindings/leds/backlight/qcom-wled.yaml         | 223 ++++++++
 drivers/video/backlight/qcom-wled.c                | 622 ++++++++++++++++++---
 3 files changed, 772 insertions(+), 227 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/leds/backlight/qcom-wled.txt
 create mode 100644 Documentation/devicetree/bindings/leds/backlight/qcom-wled.yaml

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
 a Linux Foundation Collaborative Project

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH V3 1/4] backlight: qcom-wled: convert the wled bindings to .yaml format
  2020-03-09 13:25 [PATCH V3 0/4] Add support for WLED5 Kiran Gunda
@ 2020-03-09 13:25 ` Kiran Gunda
  2020-03-10 15:01   ` Daniel Thompson
  2020-03-10 18:31   ` Rob Herring
  2020-03-09 13:26 ` [PATCH V3 2/4] backlight: qcom-wled: Add callback functions Kiran Gunda
                   ` (2 subsequent siblings)
  3 siblings, 2 replies; 15+ messages in thread
From: Kiran Gunda @ 2020-03-09 13:25 UTC (permalink / raw)
  To: bjorn.andersson, jingoohan1, lee.jones, b.zolnierkie, dri-devel,
	daniel.thompson, jacek.anaszewski, pavel, robh+dt, mark.rutland,
	linux-leds, devicetree, linux-kernel, Dan Murphy
  Cc: linux-arm-msm, Kiran Gunda

Convert the qcom-wled bindings from .txt to .yaml format.

Signed-off-by: Kiran Gunda <kgunda@codeaurora.org>
---
 .../bindings/leds/backlight/qcom-wled.txt          | 154 -----------------
 .../bindings/leds/backlight/qcom-wled.yaml         | 184 +++++++++++++++++++++
 2 files changed, 184 insertions(+), 154 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/leds/backlight/qcom-wled.txt
 create mode 100644 Documentation/devicetree/bindings/leds/backlight/qcom-wled.yaml

diff --git a/Documentation/devicetree/bindings/leds/backlight/qcom-wled.txt b/Documentation/devicetree/bindings/leds/backlight/qcom-wled.txt
deleted file mode 100644
index c06863b..0000000
--- a/Documentation/devicetree/bindings/leds/backlight/qcom-wled.txt
+++ /dev/null
@@ -1,154 +0,0 @@
-Binding for Qualcomm Technologies, Inc. WLED driver
-
-WLED (White Light Emitting Diode) driver is used for controlling display
-backlight that is part of PMIC on Qualcomm Technologies, Inc. reference
-platforms. The PMIC is connected to the host processor via SPMI bus.
-
-- compatible
-	Usage:        required
-	Value type:   <string>
-	Definition:   should be one of:
-			"qcom,pm8941-wled"
-			"qcom,pmi8998-wled"
-			"qcom,pm660l-wled"
-
-- reg
-	Usage:        required
-	Value type:   <prop encoded array>
-	Definition:   Base address of the WLED modules.
-
-- default-brightness
-	Usage:        optional
-	Value type:   <u32>
-	Definition:   brightness value on boot, value from: 0-4095.
-		      Default: 2048
-
-- label
-	Usage:        required
-	Value type:   <string>
-	Definition:   The name of the backlight device
-
-- qcom,cs-out
-	Usage:        optional
-	Value type:   <bool>
-	Definition:   enable current sink output.
-		      This property is supported only for PM8941.
-
-- qcom,cabc
-	Usage:        optional
-	Value type:   <bool>
-	Definition:   enable content adaptive backlight control.
-
-- qcom,ext-gen
-	Usage:        optional
-	Value type:   <bool>
-	Definition:   use externally generated modulator signal to dim.
-		      This property is supported only for PM8941.
-
-- qcom,current-limit
-	Usage:        optional
-	Value type:   <u32>
-	Definition:   mA; per-string current limit; value from 0 to 25 with
-		      1 mA step. Default 20 mA.
-		      This property is supported only for pm8941.
-
-- qcom,current-limit-microamp
-	Usage:        optional
-	Value type:   <u32>
-	Definition:   uA; per-string current limit; value from 0 to 30000 with
-		      2500 uA step. Default 25 mA.
-
-- qcom,current-boost-limit
-	Usage:        optional
-	Value type:   <u32>
-	Definition:   mA; boost current limit.
-		      For pm8941: one of: 105, 385, 525, 805, 980, 1260, 1400,
-		      1680. Default: 805 mA.
-		      For pmi8998: one of: 105, 280, 450, 620, 970, 1150, 1300,
-		      1500. Default: 970 mA.
-
-- qcom,switching-freq
-	Usage:        optional
-	Value type:   <u32>
-	 Definition:   kHz; switching frequency; one of: 600, 640, 685, 738,
-		       800, 872, 960, 1066, 1200, 1371, 1600, 1920, 2400, 3200,
-		       4800, 9600.
-		       Default: for pm8941: 1600 kHz
-				for pmi8998: 800 kHz
-
-- qcom,ovp
-	Usage:        optional
-	Value type:   <u32>
-	Definition:   V; Over-voltage protection limit; one of:
-		      27, 29, 32, 35. Default: 29V
-		      This property is supported only for PM8941.
-
-- qcom,ovp-millivolt
-	Usage:        optional
-	Value type:   <u32>
-	Definition:   mV; Over-voltage protection limit;
-		      For pmi8998: one of 18100, 19600, 29600, 31100.
-		      Default 29600 mV.
-		      If this property is not specified for PM8941, it
-		      falls back to "qcom,ovp" property.
-
-- qcom,num-strings
-	Usage:        optional
-	Value type:   <u32>
-	Definition:   #; number of led strings attached;
-		      value: For PM8941 from 1 to 3. Default: 2
-			     For PMI8998 from 1 to 4.
-
-- interrupts
-	Usage:        optional
-	Value type:   <prop encoded array>
-	Definition:   Interrupts associated with WLED. This should be
-		      "short" and "ovp" interrupts. Interrupts can be
-		      specified as per the encoding listed under
-		      Documentation/devicetree/bindings/spmi/
-		      qcom,spmi-pmic-arb.txt.
-
-- interrupt-names
-	Usage:        optional
-	Value type:   <string>
-	Definition:   Interrupt names associated with the interrupts.
-		      Must be "short" and "ovp". The short circuit detection
-		      is not supported for PM8941.
-
-- qcom,enabled-strings
-	Usage:        optional
-	Value tyoe:   <u32 array>
-	Definition:   Array of the WLED strings numbered from 0 to 3. Each
-		      string of leds are operated individually. Specify the
-		      list of strings used by the device. Any combination of
-		      led strings can be used.
-
-- qcom,external-pfet
-	Usage:        optional
-	Value type:   <bool>
-	Definition:   Specify if external PFET control for short circuit
-		      protection is used. This property is supported only
-		      for PMI8998.
-
-- qcom,auto-string-detection
-	Usage:        optional
-	Value type:   <bool>
-	Definition:   Enables auto-detection of the WLED string configuration.
-		      This feature is not supported for PM8941.
-
-
-Example:
-
-pm8941-wled@d800 {
-	compatible = "qcom,pm8941-wled";
-	reg = <0xd800>;
-	label = "backlight";
-
-	qcom,cs-out;
-	qcom,current-limit = <20>;
-	qcom,current-boost-limit = <805>;
-	qcom,switching-freq = <1600>;
-	qcom,ovp = <29>;
-	qcom,num-strings = <2>;
-	qcom,enabled-strings = <0 1>;
-};
diff --git a/Documentation/devicetree/bindings/leds/backlight/qcom-wled.yaml b/Documentation/devicetree/bindings/leds/backlight/qcom-wled.yaml
new file mode 100644
index 0000000..d334f81
--- /dev/null
+++ b/Documentation/devicetree/bindings/leds/backlight/qcom-wled.yaml
@@ -0,0 +1,184 @@
+# SPDX-License-Identifier: GPL-2.0-only
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/bindings/leds/backlight/qcom-wled.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Binding for Qualcomm Technologies, Inc. WLED driver
+
+maintainers:
+  - Lee Jones <lee.jones@linaro.org>
+
+description: |
+  WLED (White Light Emitting Diode) driver is used for controlling display
+  backlight that is part of PMIC on Qualcomm Technologies, Inc. reference
+  platforms. The PMIC is connected to the host processor via SPMI bus.
+
+properties:
+  compatible :
+    enum:
+       - qcom,pm8941-wled
+       - qcom,pmi8998-wled
+       - qcom,pm660l-wled
+
+  reg:
+    maxItems: 1
+
+  default-brightness:
+    maxItems: 1
+    description:
+      brightness value on boot, value from 0-4095.
+    allOf:
+      - $ref: /schemas/types.yaml#/definitions/uint32
+        default: 2048
+
+  label:
+    maxItems: 1
+    description:
+      The name of the backlight device.
+    allOf:
+      - $ref: /schemas/types.yaml#/definitions/string
+
+  qcom,cs-out:
+    description:
+      enable current sink output.
+      This property is supported only for PM8941.
+    type: boolean
+
+  qcom,cabc:
+    description:
+      enable content adaptive backlight control.
+    type: boolean
+
+  qcom,ext-gen:
+    description:
+      use externally generated modulator signal to dim.
+      This property is supported only for PM8941.
+    type: boolean
+
+  qcom,current-limit:
+    maxItems: 1
+    allOf:
+      - $ref: /schemas/types.yaml#/definitions/uint32
+    description:
+      mA; per-string current limit; value from 0 to 25 with
+      1 mA step. This property is supported only for pm8941.
+    default: 20
+
+  qcom,current-limit-microamp:
+    maxItems: 1
+    allOf:
+      - $ref: /schemas/types.yaml#/definitions/uint32
+    description:
+      uA; per-string current limit; value from 0 to 30000 with
+      2500 uA step.
+    default: 25
+
+  qcom,current-boost-limit:
+    maxItems: 1
+    allOf:
+      - $ref: /schemas/types.yaml#/definitions/uint32
+    description:
+      mA; boost current limit.
+      For pm8941 one of 105, 385, 525, 805, 980, 1260, 1400, 1680.
+      Default, 805 mA.
+      For pmi8998 one of 105, 280, 450, 620, 970, 1150, 1300,
+      1500. Default 970 mA.
+
+  qcom,switching-freq:
+    maxItems: 1
+    allOf:
+      - $ref: /schemas/types.yaml#/definitions/uint32
+    description:
+      kHz; switching frequency; one of 600, 640, 685, 738,
+      800, 872, 960, 1066, 1200, 1371, 1600, 1920, 2400, 3200,
+      4800, 9600.
+      Default for pm8941 1600 kHz
+               for pmi8998 800 kHz
+
+  qcom,ovp:
+    maxItems: 1
+    allOf:
+      - $ref: /schemas/types.yaml#/definitions/uint32
+    description:
+      V; Over-voltage protection limit; one of 27, 29, 32, 35. Default 29V
+      This property is supported only for PM8941.
+
+  qcom,ovp-millivolt:
+    maxItems: 1
+    allOf:
+      - $ref: /schemas/types.yaml#/definitions/uint32
+    description:
+      mV; Over-voltage protection limit;
+      For pmi8998 one of 18100, 19600, 29600, 31100.
+      Default 29600 mV.
+      If this property is not specified for PM8941, it
+      falls back to "qcom,ovp" property.
+
+  qcom,num-strings:
+    maxItems: 1
+    allOf:
+      - $ref: /schemas/types.yaml#/definitions/uint32
+    description:
+      number of led strings attached;
+      value for PM8941 from 1 to 3. Default 2
+      For PMI8998 from 1 to 4.
+
+  interrupts:
+    maxItems: 2
+    description:
+      Interrupts associated with WLED. This should be
+      "short" and "ovp" interrupts. Interrupts can be
+      specified as per the encoding listed under
+      Documentation/devicetree/bindings/spmi/
+      qcom,spmi-pmic-arb.txt.
+
+  interrupt-names:
+    description:
+      Interrupt names associated with the interrupts.
+      Must be "short" and "ovp". The short circuit detection
+      is not supported for PM8941.
+
+  qcom,enabled-strings:
+    maxItems: 1
+    allOf:
+      - $ref: /schemas/types.yaml#/definitions/uint32-array
+    description:
+      Array of the WLED strings numbered from 0 to 3. Each
+      string of leds are operated individually. Specify the
+      list of strings used by the device. Any combination of
+      led strings can be used.
+
+  qcom,external-pfet:
+    description:
+      Specify if external PFET control for short circuit
+      protection is used. This property is supported only
+      for PMI8998.
+    type: boolean
+
+  qcom,auto-string-detection:
+    description:
+      Enables auto-detection of the WLED string configuration.
+      This feature is not supported for PM8941.
+    type: boolean
+
+required:
+  - compatible
+  - reg
+  - label
+
+examples:
+  - |
+    pm8941-wled@d800 {
+        compatible = "qcom,pm8941-wled";
+        reg = <0xd800 0x100>;
+        label = "backlight";
+
+        qcom,cs-out;
+        qcom,current-limit = <20>;
+        qcom,current-boost-limit = <805>;
+        qcom,switching-freq = <1600>;
+        qcom,ovp = <29>;
+        qcom,num-strings = <2>;
+        qcom,enabled-strings = <0 1>;
+     };
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
 a Linux Foundation Collaborative Project

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH V3 2/4] backlight: qcom-wled: Add callback functions
  2020-03-09 13:25 [PATCH V3 0/4] Add support for WLED5 Kiran Gunda
  2020-03-09 13:25 ` [PATCH V3 1/4] backlight: qcom-wled: convert the wled bindings to .yaml format Kiran Gunda
@ 2020-03-09 13:26 ` Kiran Gunda
  2020-03-10 15:27   ` Daniel Thompson
  2020-03-09 13:26 ` [PATCH V3 3/4] backlight: qcom-wled: Add support for WLED5 peripheral in PM8150L Kiran Gunda
  2020-03-09 13:26 ` [PATCH V3 4/4] backlight: qcom-wled: Update auto calibration support for WLED5 Kiran Gunda
  3 siblings, 1 reply; 15+ messages in thread
From: Kiran Gunda @ 2020-03-09 13:26 UTC (permalink / raw)
  To: bjorn.andersson, jingoohan1, lee.jones, b.zolnierkie, dri-devel,
	daniel.thompson, jacek.anaszewski, pavel, robh+dt, mark.rutland,
	linux-leds, devicetree, linux-kernel, Andy Gross, linux-arm-msm,
	linux-fbdev
  Cc: Kiran Gunda

Add cabc_config, sync_toggle, wled_ovp_fault_status and wled_ovp_delay
callback functions to prepare the driver for adding WLED5 support.

Signed-off-by: Kiran Gunda <kgunda@codeaurora.org>
---
 drivers/video/backlight/qcom-wled.c | 196 +++++++++++++++++++++++-------------
 1 file changed, 126 insertions(+), 70 deletions(-)

diff --git a/drivers/video/backlight/qcom-wled.c b/drivers/video/backlight/qcom-wled.c
index 3d276b3..b73f273 100644
--- a/drivers/video/backlight/qcom-wled.c
+++ b/drivers/video/backlight/qcom-wled.c
@@ -128,6 +128,7 @@ struct wled_config {
 	bool cs_out_en;
 	bool ext_gen;
 	bool cabc;
+	bool en_cabc;
 	bool external_pfet;
 	bool auto_detection_enabled;
 };
@@ -147,14 +148,20 @@ struct wled {
 	u32 max_brightness;
 	u32 short_count;
 	u32 auto_detect_count;
+	u32 version;
 	bool disabled_by_short;
 	bool has_short_detect;
+	bool cabc_disabled;
 	int short_irq;
 	int ovp_irq;
 
 	struct wled_config cfg;
 	struct delayed_work ovp_work;
 	int (*wled_set_brightness)(struct wled *wled, u16 brightness);
+	int (*cabc_config)(struct wled *wled, bool enable);
+	int (*wled_sync_toggle)(struct wled *wled);
+	int (*wled_ovp_fault_status)(struct wled *wled, bool *fault_set);
+	int (*wled_ovp_delay)(struct wled *wled);
 };
 
 static int wled3_set_brightness(struct wled *wled, u16 brightness)
@@ -237,7 +244,7 @@ static int wled_module_enable(struct wled *wled, int val)
 	return 0;
 }
 
-static int wled_sync_toggle(struct wled *wled)
+static int wled3_sync_toggle(struct wled *wled)
 {
 	int rc;
 	unsigned int mask = GENMASK(wled->max_string_count - 1, 0);
@@ -255,6 +262,46 @@ static int wled_sync_toggle(struct wled *wled)
 	return rc;
 }
 
+static int wled4_ovp_fault_status(struct wled *wled, bool *fault_set)
+{
+	int rc;
+	u32 int_rt_sts, fault_sts;
+
+	*fault_set = false;
+	rc = regmap_read(wled->regmap,
+			wled->ctrl_addr + WLED3_CTRL_REG_INT_RT_STS,
+			&int_rt_sts);
+	if (rc < 0) {
+		dev_err(wled->dev, "Failed to read INT_RT_STS rc=%d\n", rc);
+		return rc;
+	}
+
+	rc = regmap_read(wled->regmap,
+			wled->ctrl_addr + WLED3_CTRL_REG_FAULT_STATUS,
+			&fault_sts);
+	if (rc < 0) {
+		dev_err(wled->dev, "Failed to read FAULT_STATUS rc=%d\n", rc);
+		return rc;
+	}
+
+	if (int_rt_sts & WLED3_CTRL_REG_OVP_FAULT_STATUS)
+		*fault_set = true;
+
+	if (fault_sts & WLED3_CTRL_REG_OVP_FAULT_BIT)
+		*fault_set = true;
+
+	if (*fault_set)
+		dev_dbg(wled->dev, "WLED OVP fault detected, int_rt_sts=0x%x fault_sts=0x%x\n",
+			int_rt_sts, fault_sts);
+
+	return rc;
+}
+
+static int wled4_ovp_delay(struct wled *wled)
+{
+	return WLED_SOFT_START_DLY_US;
+}
+
 static int wled_update_status(struct backlight_device *bl)
 {
 	struct wled *wled = bl_get_data(bl);
@@ -275,7 +322,7 @@ static int wled_update_status(struct backlight_device *bl)
 			goto unlock_mutex;
 		}
 
-		rc = wled_sync_toggle(wled);
+		rc = wled->wled_sync_toggle(wled);
 		if (rc < 0) {
 			dev_err(wled->dev, "wled sync failed rc:%d\n", rc);
 			goto unlock_mutex;
@@ -298,6 +345,31 @@ static int wled_update_status(struct backlight_device *bl)
 	return rc;
 }
 
+static int wled4_cabc_config(struct wled *wled, bool enable)
+{
+	int i, j, rc;
+	u8 val;
+
+	if (wled->cabc_disabled)
+		return 0;
+
+	for (i = 0; i < wled->cfg.num_strings; i++) {
+		j = wled->cfg.enabled_strings[i];
+
+		val = enable ? WLED4_SINK_REG_STR_CABC_MASK : 0;
+		rc = regmap_update_bits(wled->regmap, wled->sink_addr +
+					WLED4_SINK_REG_STR_CABC(j),
+					WLED4_SINK_REG_STR_CABC_MASK, val);
+		if (rc < 0)
+			return rc;
+	}
+
+	if (!wled->cfg.en_cabc)
+		wled->cabc_disabled = true;
+
+	return 0;
+}
+
 #define WLED_SHORT_DLY_MS			20
 #define WLED_SHORT_CNT_MAX			5
 #define WLED_SHORT_RESET_CNT_DLY_US		USEC_PER_SEC
@@ -345,9 +417,10 @@ static irqreturn_t wled_short_irq_handler(int irq, void *_wled)
 
 static void wled_auto_string_detection(struct wled *wled)
 {
-	int rc = 0, i;
-	u32 sink_config = 0, int_sts;
+	int rc = 0, i, delay_time_us;
+	u32 sink_config = 0;
 	u8 sink_test = 0, sink_valid = 0, val;
+	bool fault_set;
 
 	/* Read configured sink configuration */
 	rc = regmap_read(wled->regmap, wled->sink_addr +
@@ -376,14 +449,9 @@ static void wled_auto_string_detection(struct wled *wled)
 	}
 
 	if (wled->cfg.cabc) {
-		for (i = 0; i < wled->cfg.num_strings; i++) {
-			rc = regmap_update_bits(wled->regmap, wled->sink_addr +
-						WLED4_SINK_REG_STR_CABC(i),
-						WLED4_SINK_REG_STR_CABC_MASK,
-						0);
-			if (rc < 0)
-				goto failed_detect;
-		}
+		rc = wled->cabc_config(wled, 0);
+		if (rc < 0)
+			goto failed_detect;
 	}
 
 	/* Disable all sinks */
@@ -427,18 +495,17 @@ static void wled_auto_string_detection(struct wled *wled)
 			goto failed_detect;
 		}
 
-		usleep_range(WLED_SOFT_START_DLY_US,
-			     WLED_SOFT_START_DLY_US + 1000);
+		delay_time_us = wled->wled_ovp_delay(wled);
+		usleep_range(delay_time_us, delay_time_us + 1000);
 
-		rc = regmap_read(wled->regmap, wled->ctrl_addr +
-				 WLED3_CTRL_REG_INT_RT_STS, &int_sts);
+		rc = wled->wled_ovp_fault_status(wled, &fault_set);
 		if (rc < 0) {
-			dev_err(wled->dev, "Error in reading WLED3_CTRL_INT_RT_STS rc=%d\n",
+			dev_err(wled->dev, "Error in getting OVP fault_sts, rc=%d\n",
 				rc);
 			goto failed_detect;
 		}
 
-		if (int_sts & WLED3_CTRL_REG_OVP_FAULT_STATUS)
+		if (fault_set)
 			dev_dbg(wled->dev, "WLED OVP fault detected with SINK %d\n",
 				i + 1);
 		else
@@ -478,30 +545,30 @@ static void wled_auto_string_detection(struct wled *wled)
 	}
 
 	/* Enable valid sinks */
-	for (i = 0; i < wled->cfg.num_strings; i++) {
-		if (wled->cfg.cabc) {
-			rc = regmap_update_bits(wled->regmap, wled->sink_addr +
-						WLED4_SINK_REG_STR_CABC(i),
-						WLED4_SINK_REG_STR_CABC_MASK,
-						WLED4_SINK_REG_STR_CABC_MASK);
-			if (rc < 0)
+	if (wled->version == 4) {
+		for (i = 0; i < wled->cfg.num_strings; i++) {
+			if (sink_config &
+			    BIT(WLED4_SINK_REG_CURR_SINK_SHFT + i))
+				val = WLED4_SINK_REG_STR_MOD_MASK;
+			else
+				/* Disable modulator_en for unused sink */
+				val = 0x0;
+
+			rc = regmap_write(wled->regmap, wled->sink_addr +
+					  WLED4_SINK_REG_STR_MOD_EN(i), val);
+			if (rc < 0) {
+				dev_err(wled->dev, "Failed to configure MODULATOR_EN rc=%d\n",
+					rc);
 				goto failed_detect;
-		}
-
-		if (sink_config & BIT(WLED4_SINK_REG_CURR_SINK_SHFT + i))
-			val = WLED4_SINK_REG_STR_MOD_MASK;
-		else
-			val = 0x0; /* Disable modulator_en for unused sink */
-
-		rc = regmap_write(wled->regmap, wled->sink_addr +
-				  WLED4_SINK_REG_STR_MOD_EN(i), val);
-		if (rc < 0) {
-			dev_err(wled->dev, "Failed to configure MODULATOR_EN rc=%d\n",
-				rc);
-			goto failed_detect;
+			}
 		}
 	}
 
+	/* Enable CABC if it needs to be enabled */
+	rc = wled->cabc_config(wled, true);
+	if (rc < 0)
+		goto failed_detect;
+
 	/* Restore the feedback setting */
 	rc = regmap_write(wled->regmap,
 			  wled->ctrl_addr + WLED3_CTRL_REG_FEEDBACK_CONTROL, 0);
@@ -570,29 +637,19 @@ static bool wled_auto_detection_required(struct wled *wled)
 static int wled_auto_detection_at_init(struct wled *wled)
 {
 	int rc;
-	u32 fault_status, rt_status;
+	bool fault_set;
 
 	if (!wled->cfg.auto_detection_enabled)
 		return 0;
 
-	rc = regmap_read(wled->regmap,
-			 wled->ctrl_addr + WLED3_CTRL_REG_INT_RT_STS,
-			 &rt_status);
+	rc = wled->wled_ovp_fault_status(wled, &fault_set);
 	if (rc < 0) {
-		dev_err(wled->dev, "Failed to read RT status rc=%d\n", rc);
-		return rc;
-	}
-
-	rc = regmap_read(wled->regmap,
-			 wled->ctrl_addr + WLED3_CTRL_REG_FAULT_STATUS,
-			 &fault_status);
-	if (rc < 0) {
-		dev_err(wled->dev, "Failed to read fault status rc=%d\n", rc);
+		dev_err(wled->dev, "Error in getting OVP fault_sts, rc=%d\n",
+			rc);
 		return rc;
 	}
 
-	if ((rt_status & WLED3_CTRL_REG_OVP_FAULT_STATUS) ||
-	    (fault_status & WLED3_CTRL_REG_OVP_FAULT_BIT)) {
+	if (fault_set) {
 		mutex_lock(&wled->lock);
 		wled_auto_string_detection(wled);
 		mutex_unlock(&wled->lock);
@@ -811,17 +868,12 @@ static int wled4_setup(struct wled *wled)
 					wled->cfg.string_i_limit);
 		if (rc < 0)
 			return rc;
-
-		addr = wled->sink_addr +
-				WLED4_SINK_REG_STR_CABC(j);
-		rc = regmap_update_bits(wled->regmap, addr,
-					WLED4_SINK_REG_STR_CABC_MASK,
-					wled->cfg.cabc ?
-					WLED4_SINK_REG_STR_CABC_MASK : 0);
-		if (rc < 0)
-			return rc;
 	}
 
+	rc = wled4_cabc_config(wled, wled->cfg.en_cabc);
+	if (rc < 0)
+		return rc;
+
 	rc = regmap_update_bits(wled->regmap, wled->ctrl_addr +
 				WLED3_CTRL_REG_MOD_EN,
 				WLED3_CTRL_REG_MOD_EN_MASK,
@@ -835,7 +887,7 @@ static int wled4_setup(struct wled *wled)
 	if (rc < 0)
 		return rc;
 
-	rc = wled_sync_toggle(wled);
+	rc = wled->wled_sync_toggle(wled);
 	if (rc < 0) {
 		dev_err(wled->dev, "Failed to toggle sync reg rc:%d\n", rc);
 		return rc;
@@ -951,7 +1003,7 @@ static u32 wled_values(const struct wled_var_cfg *cfg, u32 idx)
 	return idx;
 }
 
-static int wled_configure(struct wled *wled, int version)
+static int wled_configure(struct wled *wled)
 {
 	struct wled_config *cfg = &wled->cfg;
 	struct device *dev = wled->dev;
@@ -1035,12 +1087,13 @@ static int wled_configure(struct wled *wled, int version)
 	if (rc)
 		wled->name = devm_kasprintf(dev, GFP_KERNEL, "%pOFn", dev->of_node);
 
-	switch (version) {
+	switch (wled->version) {
 	case 3:
 		u32_opts = wled3_opts;
 		size = ARRAY_SIZE(wled3_opts);
 		*cfg = wled3_config_defaults;
 		wled->wled_set_brightness = wled3_set_brightness;
+		wled->wled_sync_toggle = wled3_sync_toggle;
 		wled->max_string_count = 3;
 		wled->sink_addr = wled->ctrl_addr;
 		break;
@@ -1050,6 +1103,10 @@ static int wled_configure(struct wled *wled, int version)
 		size = ARRAY_SIZE(wled4_opts);
 		*cfg = wled4_config_defaults;
 		wled->wled_set_brightness = wled4_set_brightness;
+		wled->wled_sync_toggle = wled3_sync_toggle;
+		wled->cabc_config = wled4_cabc_config;
+		wled->wled_ovp_fault_status = wled4_ovp_fault_status;
+		wled->wled_ovp_delay = wled4_ovp_delay;
 		wled->max_string_count = 4;
 
 		prop_addr = of_get_address(dev->of_node, 1, NULL, NULL);
@@ -1186,7 +1243,6 @@ static int wled_probe(struct platform_device *pdev)
 	struct backlight_device *bl;
 	struct wled *wled;
 	struct regmap *regmap;
-	int version;
 	u32 val;
 	int rc;
 
@@ -1203,18 +1259,18 @@ static int wled_probe(struct platform_device *pdev)
 	wled->regmap = regmap;
 	wled->dev = &pdev->dev;
 
-	version = (uintptr_t)of_device_get_match_data(&pdev->dev);
-	if (!version) {
+	wled->version = (uintptr_t)of_device_get_match_data(&pdev->dev);
+	if (!wled->version) {
 		dev_err(&pdev->dev, "Unknown device version\n");
 		return -ENODEV;
 	}
 
 	mutex_init(&wled->lock);
-	rc = wled_configure(wled, version);
+	rc = wled_configure(wled);
 	if (rc)
 		return rc;
 
-	switch (version) {
+	switch (wled->version) {
 	case 3:
 		wled->cfg.auto_detection_enabled = false;
 		rc = wled3_setup(wled);
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
 a Linux Foundation Collaborative Project

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH V3 3/4] backlight: qcom-wled: Add support for WLED5 peripheral in PM8150L
  2020-03-09 13:25 [PATCH V3 0/4] Add support for WLED5 Kiran Gunda
  2020-03-09 13:25 ` [PATCH V3 1/4] backlight: qcom-wled: convert the wled bindings to .yaml format Kiran Gunda
  2020-03-09 13:26 ` [PATCH V3 2/4] backlight: qcom-wled: Add callback functions Kiran Gunda
@ 2020-03-09 13:26 ` Kiran Gunda
  2020-03-10 15:45   ` Daniel Thompson
  2020-03-09 13:26 ` [PATCH V3 4/4] backlight: qcom-wled: Update auto calibration support for WLED5 Kiran Gunda
  3 siblings, 1 reply; 15+ messages in thread
From: Kiran Gunda @ 2020-03-09 13:26 UTC (permalink / raw)
  To: bjorn.andersson, jingoohan1, lee.jones, b.zolnierkie, dri-devel,
	daniel.thompson, jacek.anaszewski, pavel, robh+dt, mark.rutland,
	linux-leds, devicetree, linux-kernel, Dan Murphy, Andy Gross,
	linux-arm-msm, linux-fbdev
  Cc: Kiran Gunda

Add support for WLED5 peripheral that is present on PM8150L PMICs.

PM8150L WLED supports the following:
    - Two modulators and each sink can use any of the modulator
    - Multiple CABC selection options
    - Multiple brightness width selection (12 bits to 15 bits)

Signed-off-by: Kiran Gunda <kgunda@codeaurora.org>
---
 .../bindings/leds/backlight/qcom-wled.yaml         |  39 +++
 drivers/video/backlight/qcom-wled.c                | 336 ++++++++++++++++++++-
 2 files changed, 374 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/leds/backlight/qcom-wled.yaml b/Documentation/devicetree/bindings/leds/backlight/qcom-wled.yaml
index d334f81..e0dadc4 100644
--- a/Documentation/devicetree/bindings/leds/backlight/qcom-wled.yaml
+++ b/Documentation/devicetree/bindings/leds/backlight/qcom-wled.yaml
@@ -20,6 +20,7 @@ properties:
        - qcom,pm8941-wled
        - qcom,pmi8998-wled
        - qcom,pm660l-wled
+       - qcom,pm8150l-wled
 
   reg:
     maxItems: 1
@@ -28,10 +29,23 @@ properties:
     maxItems: 1
     description:
       brightness value on boot, value from 0-4095.
+      For pm8150l this value vary from 0-4095 or 0-32767
+      depending on the brightness control mode. If CABC is
+      enabled 0-4095 range is used.
     allOf:
       - $ref: /schemas/types.yaml#/definitions/uint32
         default: 2048
 
+  max-brightness:
+    maxItems: 1
+    description:
+      Maximum brightness level. Allowed values are,
+      for pmi8998 it is  0-4095.
+      For pm8150l, this can be either 4095 or 32767.
+      If CABC is enabled, this is capped to 4095.
+    allOf:
+      - $ref: /schemas/types.yaml#/definitions/uint32
+
   label:
     maxItems: 1
     description:
@@ -124,6 +138,31 @@ properties:
       value for PM8941 from 1 to 3. Default 2
       For PMI8998 from 1 to 4.
 
+  qcom,modulator-sel:
+    maxItems: 1
+    allOf:
+      - $ref: /schemas/types.yaml#/definitions/uint32
+    description:
+      Selects the modulator used for brightness modulation.
+      Allowed values are,
+               0 - Modulator A
+               1 - Modulator B
+      If not specified, then modulator A will be used by default.
+      This property is applicable only to WLED5 peripheral.
+
+  qcom,cabc-sel:
+    maxItems: 1
+    allOf:
+      - $ref: /schemas/types.yaml#/definitions/uint32
+    description:
+      Selects the CABC pin signal used for brightness modulation.
+      Allowed values are,
+              0 - CABC disabled
+              1 - CABC 1
+              2 - CABC 2
+              3 - External signal (e.g. LPG) is used for dimming
+      This property is applicable only to WLED5 peripheral.
+
   interrupts:
     maxItems: 2
     description:
diff --git a/drivers/video/backlight/qcom-wled.c b/drivers/video/backlight/qcom-wled.c
index b73f273..edbbcb2 100644
--- a/drivers/video/backlight/qcom-wled.c
+++ b/drivers/video/backlight/qcom-wled.c
@@ -19,6 +19,8 @@
 #define WLED_DEFAULT_BRIGHTNESS				2048
 #define WLED_SOFT_START_DLY_US				10000
 #define WLED3_SINK_REG_BRIGHT_MAX			0xFFF
+#define WLED5_SINK_REG_BRIGHT_MAX_12B			0xFFF
+#define WLED5_SINK_REG_BRIGHT_MAX_15B			0x7FFF
 
 /* WLED3/WLED4 control registers */
 #define WLED3_CTRL_REG_FAULT_STATUS			0x08
@@ -40,6 +42,7 @@
 
 #define WLED3_CTRL_REG_OVP				0x4d
 #define  WLED3_CTRL_REG_OVP_MASK			GENMASK(1, 0)
+#define  WLED5_CTRL_REG_OVP_MASK			GENMASK(3, 0)
 
 #define WLED3_CTRL_REG_ILIMIT				0x4e
 #define  WLED3_CTRL_REG_ILIMIT_MASK			GENMASK(2, 0)
@@ -101,6 +104,40 @@
 
 #define WLED4_SINK_REG_BRIGHT(n)			(0x57 + (n * 0x10))
 
+/* WLED5 specific sink registers */
+#define WLED5_SINK_REG_MOD_A_EN				0x50
+#define WLED5_SINK_REG_MOD_B_EN				0x60
+#define  WLED5_SINK_REG_MOD_EN_MASK			BIT(7)
+
+#define WLED5_SINK_REG_MOD_A_SRC_SEL			0x51
+#define WLED5_SINK_REG_MOD_B_SRC_SEL			0x61
+#define  WLED5_SINK_REG_MOD_SRC_SEL_HIGH		0
+#define  WLED5_SINK_REG_MOD_SRC_SEL_EXT			0x03
+#define  WLED5_SINK_REG_MOD_SRC_SEL_MASK		GENMASK(1, 0)
+
+#define WLED5_SINK_REG_MOD_A_BRIGHTNESS_WIDTH_SEL	0x52
+#define WLED5_SINK_REG_MOD_B_BRIGHTNESS_WIDTH_SEL	0x62
+#define  WLED5_SINK_REG_BRIGHTNESS_WIDTH_12B		0
+#define  WLED5_SINK_REG_BRIGHTNESS_WIDTH_15B		1
+
+#define WLED5_SINK_REG_MOD_A_BRIGHTNESS_LSB		0x53
+#define WLED5_SINK_REG_MOD_A_BRIGHTNESS_MSB		0x54
+#define WLED5_SINK_REG_MOD_B_BRIGHTNESS_LSB		0x63
+#define WLED5_SINK_REG_MOD_B_BRIGHTNESS_MSB		0x64
+
+#define WLED5_SINK_REG_MOD_SYNC_BIT			0x65
+#define  WLED5_SINK_REG_SYNC_MOD_A_BIT			BIT(0)
+#define  WLED5_SINK_REG_SYNC_MOD_B_BIT			BIT(1)
+#define  WLED5_SINK_REG_SYNC_MASK			GENMASK(1, 0)
+
+/* WLED5 specific per-'string' registers below */
+#define WLED5_SINK_REG_STR_FULL_SCALE_CURR(n)		(0x72 + (n * 0x10))
+
+#define WLED5_SINK_REG_STR_SRC_SEL(n)			(0x73 + (n * 0x10))
+#define  WLED5_SINK_REG_SRC_SEL_MOD_A			0
+#define  WLED5_SINK_REG_SRC_SEL_MOD_B			1
+#define  WLED5_SINK_REG_SRC_SEL_MASK			GENMASK(1, 0)
+
 struct wled_var_cfg {
 	const u32 *values;
 	u32 (*fn)(u32);
@@ -125,6 +162,8 @@ struct wled_config {
 	u32 num_strings;
 	u32 string_i_limit;
 	u32 enabled_strings[WLED_MAX_STRINGS];
+	u32 mod_sel;
+	u32 cabc_sel;
 	bool cs_out_en;
 	bool ext_gen;
 	bool cabc;
@@ -164,6 +203,27 @@ struct wled {
 	int (*wled_ovp_delay)(struct wled *wled);
 };
 
+enum wled5_mod_sel {
+	MOD_A,
+	MOD_B,
+	MOD_MAX,
+};
+
+static const u8 wled5_brightness_reg[MOD_MAX] = {
+	[MOD_A] = WLED5_SINK_REG_MOD_A_BRIGHTNESS_LSB,
+	[MOD_B] = WLED5_SINK_REG_MOD_B_BRIGHTNESS_LSB,
+};
+
+static const u8 wled5_src_sel_reg[MOD_MAX] = {
+	[MOD_A] = WLED5_SINK_REG_MOD_A_SRC_SEL,
+	[MOD_B] = WLED5_SINK_REG_MOD_B_SRC_SEL,
+};
+
+static const u8 wled5_brt_wid_sel_reg[MOD_MAX] = {
+	[MOD_A] = WLED5_SINK_REG_MOD_A_BRIGHTNESS_WIDTH_SEL,
+	[MOD_B] = WLED5_SINK_REG_MOD_B_BRIGHTNESS_WIDTH_SEL,
+};
+
 static int wled3_set_brightness(struct wled *wled, u16 brightness)
 {
 	int rc, i;
@@ -182,6 +242,25 @@ static int wled3_set_brightness(struct wled *wled, u16 brightness)
 	return 0;
 }
 
+static int wled5_set_brightness(struct wled *wled, u16 brightness)
+{
+	int rc, offset;
+	u16 low_limit = wled->max_brightness * 1 / 1000;
+	u8 v[2];
+
+	/* WLED5's lower limit is 0.1% */
+	if (brightness < low_limit)
+		brightness = low_limit;
+
+	v[0] = brightness & 0xff;
+	v[1] = (brightness >> 8) & 0x7f;
+
+	offset = wled5_brightness_reg[wled->cfg.mod_sel];
+	rc = regmap_bulk_write(wled->regmap, wled->sink_addr + offset,
+			       v, 2);
+	return rc;
+}
+
 static int wled4_set_brightness(struct wled *wled, u16 brightness)
 {
 	int rc, i;
@@ -244,6 +323,24 @@ static int wled_module_enable(struct wled *wled, int val)
 	return 0;
 }
 
+static int wled5_sync_toggle(struct wled *wled)
+{
+	int rc;
+	u8 val;
+
+	val = (wled->cfg.mod_sel == MOD_A) ? WLED5_SINK_REG_SYNC_MOD_A_BIT :
+					     WLED5_SINK_REG_SYNC_MOD_B_BIT;
+	rc = regmap_update_bits(wled->regmap,
+				wled->sink_addr + WLED5_SINK_REG_MOD_SYNC_BIT,
+				WLED5_SINK_REG_SYNC_MASK, val);
+	if (rc < 0)
+		return rc;
+
+	return regmap_update_bits(wled->regmap,
+				  wled->sink_addr + WLED5_SINK_REG_MOD_SYNC_BIT,
+				  WLED5_SINK_REG_SYNC_MASK, 0);
+}
+
 static int wled3_sync_toggle(struct wled *wled)
 {
 	int rc;
@@ -345,6 +442,29 @@ static int wled_update_status(struct backlight_device *bl)
 	return rc;
 }
 
+static int wled5_cabc_config(struct wled *wled, bool enable)
+{
+	int rc, offset;
+	u8 reg;
+
+	if (wled->cabc_disabled)
+		return 0;
+
+	reg = enable ? wled->cfg.cabc_sel : 0;
+	offset = wled5_src_sel_reg[wled->cfg.mod_sel];
+	rc = regmap_update_bits(wled->regmap, wled->sink_addr + offset,
+				WLED5_SINK_REG_MOD_SRC_SEL_MASK, reg);
+	if (rc < 0) {
+		pr_err("Error in configuring CABC rc=%d\n", rc);
+		return rc;
+	}
+
+	if (!wled->cfg.cabc_sel)
+		wled->cabc_disabled = true;
+
+	return 0;
+}
+
 static int wled4_cabc_config(struct wled *wled, bool enable)
 {
 	int i, j, rc;
@@ -909,6 +1029,115 @@ static const struct wled_config wled4_config_defaults = {
 	.auto_detection_enabled = false,
 };
 
+static int wled5_setup(struct wled *wled)
+{
+	int rc, temp, i, j;
+	u8 sink_en = 0;
+	u16 addr;
+	u32 val;
+
+	rc = regmap_update_bits(wled->regmap,
+				wled->ctrl_addr + WLED3_CTRL_REG_OVP,
+				WLED5_CTRL_REG_OVP_MASK, wled->cfg.ovp);
+	if (rc < 0)
+		return rc;
+
+	rc = regmap_update_bits(wled->regmap,
+				wled->ctrl_addr + WLED3_CTRL_REG_ILIMIT,
+				WLED3_CTRL_REG_ILIMIT_MASK,
+				wled->cfg.boost_i_limit);
+	if (rc < 0)
+		return rc;
+
+	rc = regmap_update_bits(wled->regmap,
+				wled->ctrl_addr + WLED3_CTRL_REG_FREQ,
+				WLED3_CTRL_REG_FREQ_MASK,
+				wled->cfg.switch_freq);
+	if (rc < 0)
+		return rc;
+
+	/* Per sink/string configuration */
+	for (i = 0; i < wled->cfg.num_strings; ++i) {
+		j = wled->cfg.enabled_strings[i];
+		addr = wled->sink_addr +
+				WLED4_SINK_REG_STR_FULL_SCALE_CURR(j);
+		rc = regmap_update_bits(wled->regmap, addr,
+					WLED4_SINK_REG_STR_FULL_SCALE_CURR_MASK,
+					wled->cfg.string_i_limit);
+		if (rc < 0)
+			return rc;
+
+		addr = wled->sink_addr + WLED5_SINK_REG_STR_SRC_SEL(j);
+		rc = regmap_update_bits(wled->regmap, addr,
+					WLED5_SINK_REG_SRC_SEL_MASK,
+					wled->cfg.mod_sel == MOD_A ?
+					WLED5_SINK_REG_SRC_SEL_MOD_A :
+					WLED5_SINK_REG_SRC_SEL_MOD_B);
+
+		temp = j + WLED4_SINK_REG_CURR_SINK_SHFT;
+		sink_en |= 1 << temp;
+	}
+
+	rc = wled5_cabc_config(wled, wled->cfg.cabc_sel ? true : false);
+	if (rc < 0)
+		return rc;
+
+	/* Enable one of the modulators A or B based on mod_sel */
+	addr = wled->sink_addr + WLED5_SINK_REG_MOD_A_EN;
+	val = (wled->cfg.mod_sel == MOD_A) ? WLED5_SINK_REG_MOD_EN_MASK : 0;
+	rc = regmap_update_bits(wled->regmap, addr,
+				WLED5_SINK_REG_MOD_EN_MASK, val);
+	if (rc < 0)
+		return rc;
+
+	addr = wled->sink_addr + WLED5_SINK_REG_MOD_B_EN;
+	val = (wled->cfg.mod_sel == MOD_B) ? WLED5_SINK_REG_MOD_EN_MASK : 0;
+	rc = regmap_update_bits(wled->regmap, addr,
+				WLED5_SINK_REG_MOD_EN_MASK, val);
+	if (rc < 0)
+		return rc;
+
+	addr = wled->sink_addr + wled5_brt_wid_sel_reg[wled->cfg.mod_sel];
+	val = (wled->max_brightness == WLED5_SINK_REG_BRIGHT_MAX_15B) ?
+		 WLED5_SINK_REG_BRIGHTNESS_WIDTH_15B :
+		 WLED5_SINK_REG_BRIGHTNESS_WIDTH_12B;
+	rc = regmap_write(wled->regmap, addr, val);
+	if (rc < 0)
+		return rc;
+
+	rc = regmap_update_bits(wled->regmap,
+				wled->sink_addr + WLED4_SINK_REG_CURR_SINK,
+				WLED4_SINK_REG_CURR_SINK_MASK, sink_en);
+	if (rc < 0)
+		return rc;
+
+	/* This updates only FSC configuration in WLED5 */
+	rc = wled->wled_sync_toggle(wled);
+	if (rc < 0) {
+		pr_err("Failed to toggle sync reg rc:%d\n", rc);
+		return rc;
+	}
+
+	rc = wled_auto_detection_at_init(wled);
+	if (rc < 0)
+		return rc;
+
+	return 0;
+}
+
+static const struct wled_config wled5_config_defaults = {
+	.boost_i_limit = 5,
+	.string_i_limit = 10,
+	.ovp = 4,
+	.num_strings = 4,
+	.switch_freq = 11,
+	.mod_sel = 0,
+	.cabc_sel = 0,
+	.cabc = false,
+	.external_pfet = false,
+	.auto_detection_enabled = false,
+};
+
 static const u32 wled3_boost_i_limit_values[] = {
 	105, 385, 525, 805, 980, 1260, 1400, 1680,
 };
@@ -927,6 +1156,16 @@ static const struct wled_var_cfg wled4_boost_i_limit_cfg = {
 	.size = ARRAY_SIZE(wled4_boost_i_limit_values),
 };
 
+static inline u32 wled5_boost_i_limit_values_fn(u32 idx)
+{
+	return 525 + (idx * 175);
+}
+
+static const struct wled_var_cfg wled5_boost_i_limit_cfg = {
+	.fn = wled5_boost_i_limit_values_fn,
+	.size = 8,
+};
+
 static const u32 wled3_ovp_values[] = {
 	35, 32, 29, 27,
 };
@@ -945,6 +1184,21 @@ static const struct wled_var_cfg wled4_ovp_cfg = {
 	.size = ARRAY_SIZE(wled4_ovp_values),
 };
 
+static inline u32 wled5_ovp_values_fn(u32 idx)
+{
+	/*
+	 * 0000 - 38.5 V
+	 * 0001 - 37 V ..
+	 * 1111 - 16 V
+	 */
+	return 38500 - (idx * 1500);
+}
+
+static const struct wled_var_cfg wled5_ovp_cfg = {
+	.fn = wled5_ovp_values_fn,
+	.size = 16,
+};
+
 static u32 wled3_num_strings_values_fn(u32 idx)
 {
 	return idx + 1;
@@ -992,6 +1246,14 @@ static const struct wled_var_cfg wled4_string_cfg = {
 	.size = 16,
 };
 
+static const struct wled_var_cfg wled5_mod_sel_cfg = {
+	.size = 2,
+};
+
+static const struct wled_var_cfg wled5_cabc_sel_cfg = {
+	.size = 4,
+};
+
 static u32 wled_values(const struct wled_var_cfg *cfg, u32 idx)
 {
 	if (idx >= cfg->size)
@@ -1068,6 +1330,44 @@ static int wled_configure(struct wled *wled)
 		},
 	};
 
+	const struct wled_u32_opts wled5_opts[] = {
+		{
+			.name = "qcom,current-boost-limit",
+			.val_ptr = &cfg->boost_i_limit,
+			.cfg = &wled5_boost_i_limit_cfg,
+		},
+		{
+			.name = "qcom,current-limit-microamp",
+			.val_ptr = &cfg->string_i_limit,
+			.cfg = &wled4_string_i_limit_cfg,
+		},
+		{
+			.name = "qcom,ovp-millivolt",
+			.val_ptr = &cfg->ovp,
+			.cfg = &wled5_ovp_cfg,
+		},
+		{
+			.name = "qcom,switching-freq",
+			.val_ptr = &cfg->switch_freq,
+			.cfg = &wled3_switch_freq_cfg,
+		},
+		{
+			.name = "qcom,num-strings",
+			.val_ptr = &cfg->num_strings,
+			.cfg = &wled4_num_strings_cfg,
+		},
+		{
+			.name = "qcom,modulator-sel",
+			.val_ptr = &cfg->mod_sel,
+			.cfg = &wled5_mod_sel_cfg,
+		},
+		{
+			.name = "qcom,cabc-sel",
+			.val_ptr = &cfg->cabc_sel,
+			.cfg = &wled5_cabc_sel_cfg,
+		},
+	};
+
 	const struct wled_bool_opts bool_opts[] = {
 		{ "qcom,cs-out", &cfg->cs_out_en, },
 		{ "qcom,ext-gen", &cfg->ext_gen, },
@@ -1117,6 +1417,23 @@ static int wled_configure(struct wled *wled)
 		wled->sink_addr = be32_to_cpu(*prop_addr);
 		break;
 
+	case 5:
+		u32_opts = wled5_opts;
+		size = ARRAY_SIZE(wled5_opts);
+		*cfg = wled5_config_defaults;
+		wled->wled_set_brightness = wled5_set_brightness;
+		wled->wled_sync_toggle = wled5_sync_toggle;
+		wled->cabc_config = wled5_cabc_config;
+		wled->max_string_count = 4;
+
+		prop_addr = of_get_address(dev->of_node, 1, NULL, NULL);
+		if (!prop_addr) {
+			dev_err(wled->dev, "invalid IO resources\n");
+			return -EINVAL;
+		}
+		wled->sink_addr = be32_to_cpu(*prop_addr);
+		break;
+
 	default:
 		dev_err(wled->dev, "Invalid WLED version\n");
 		return -EINVAL;
@@ -1270,6 +1587,10 @@ static int wled_probe(struct platform_device *pdev)
 	if (rc)
 		return rc;
 
+	val = WLED3_SINK_REG_BRIGHT_MAX;
+	of_property_read_u32(pdev->dev.of_node, "max-brightness", &val);
+	wled->max_brightness = val;
+
 	switch (wled->version) {
 	case 3:
 		wled->cfg.auto_detection_enabled = false;
@@ -1289,6 +1610,18 @@ static int wled_probe(struct platform_device *pdev)
 		}
 		break;
 
+	case 5:
+		wled->has_short_detect = true;
+		if (wled->cfg.cabc_sel)
+			wled->max_brightness = WLED5_SINK_REG_BRIGHT_MAX_12B;
+
+		rc = wled5_setup(wled);
+		if (rc) {
+			dev_err(&pdev->dev, "wled5_setup failed\n");
+			return rc;
+		}
+		break;
+
 	default:
 		dev_err(wled->dev, "Invalid WLED version\n");
 		break;
@@ -1310,7 +1643,7 @@ static int wled_probe(struct platform_device *pdev)
 	memset(&props, 0, sizeof(struct backlight_properties));
 	props.type = BACKLIGHT_RAW;
 	props.brightness = val;
-	props.max_brightness = WLED3_SINK_REG_BRIGHT_MAX;
+	props.max_brightness = wled->max_brightness;
 	bl = devm_backlight_device_register(&pdev->dev, wled->name,
 					    &pdev->dev, wled,
 					    &wled_ops, &props);
@@ -1333,6 +1666,7 @@ static const struct of_device_id wled_match_table[] = {
 	{ .compatible = "qcom,pm8941-wled", .data = (void *)3 },
 	{ .compatible = "qcom,pmi8998-wled", .data = (void *)4 },
 	{ .compatible = "qcom,pm660l-wled", .data = (void *)4 },
+	{ .compatible = "qcom,pm8150l-wled", .data = (void *)5 },
 	{}
 };
 MODULE_DEVICE_TABLE(of, wled_match_table);
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
 a Linux Foundation Collaborative Project

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH V3 4/4] backlight: qcom-wled: Update auto calibration support for WLED5
  2020-03-09 13:25 [PATCH V3 0/4] Add support for WLED5 Kiran Gunda
                   ` (2 preceding siblings ...)
  2020-03-09 13:26 ` [PATCH V3 3/4] backlight: qcom-wled: Add support for WLED5 peripheral in PM8150L Kiran Gunda
@ 2020-03-09 13:26 ` Kiran Gunda
  3 siblings, 0 replies; 15+ messages in thread
From: Kiran Gunda @ 2020-03-09 13:26 UTC (permalink / raw)
  To: bjorn.andersson, jingoohan1, lee.jones, b.zolnierkie, dri-devel,
	daniel.thompson, jacek.anaszewski, pavel, robh+dt, mark.rutland,
	linux-leds, devicetree, linux-kernel, Andy Gross, linux-arm-msm,
	linux-fbdev
  Cc: Kiran Gunda

Currently, auto calibration logic checks only for OVP_FAULT bit
to be set in FAULT_STATUS register to detect OVP fault. This works
well for WLED4 type. However, WLED5 type has OVP_PRE_ALARM bit
which can indicate a potential OVP fault. Use that as well for
detecting OVP fault and run auto calibration to fix the sink
configuration.

Signed-off-by: Kiran Gunda <kgunda@codeaurora.org>
---
 drivers/video/backlight/qcom-wled.c | 90 ++++++++++++++++++++++++++++++++++++-
 1 file changed, 88 insertions(+), 2 deletions(-)

diff --git a/drivers/video/backlight/qcom-wled.c b/drivers/video/backlight/qcom-wled.c
index edbbcb2..5079f1f 100644
--- a/drivers/video/backlight/qcom-wled.c
+++ b/drivers/video/backlight/qcom-wled.c
@@ -27,6 +27,7 @@
 #define  WLED3_CTRL_REG_ILIM_FAULT_BIT			BIT(0)
 #define  WLED3_CTRL_REG_OVP_FAULT_BIT			BIT(1)
 #define  WLED4_CTRL_REG_SC_FAULT_BIT			BIT(2)
+#define  WLED5_CTRL_REG_OVP_PRE_ALARM_BIT		BIT(4)
 
 #define WLED3_CTRL_REG_INT_RT_STS			0x10
 #define  WLED3_CTRL_REG_OVP_FAULT_STATUS		BIT(1)
@@ -104,6 +105,10 @@
 
 #define WLED4_SINK_REG_BRIGHT(n)			(0x57 + (n * 0x10))
 
+/* WLED5 specific control registers */
+#define WLED5_CTRL_REG_OVP_INT_CTL			0x5f
+#define  WLED5_CTRL_REG_OVP_INT_TIMER_MASK		GENMASK(2, 0)
+
 /* WLED5 specific sink registers */
 #define WLED5_SINK_REG_MOD_A_EN				0x50
 #define WLED5_SINK_REG_MOD_B_EN				0x60
@@ -394,11 +399,67 @@ static int wled4_ovp_fault_status(struct wled *wled, bool *fault_set)
 	return rc;
 }
 
+static int wled5_ovp_fault_status(struct wled *wled, bool *fault_set)
+{
+	int rc;
+	u32 int_rt_sts, fault_sts;
+
+	*fault_set = false;
+	rc = regmap_read(wled->regmap,
+			wled->ctrl_addr + WLED3_CTRL_REG_INT_RT_STS,
+			&int_rt_sts);
+	if (rc < 0) {
+		dev_err(wled->dev, "Failed to read INT_RT_STS rc=%d\n", rc);
+		return rc;
+	}
+
+	rc = regmap_read(wled->regmap,
+			wled->ctrl_addr + WLED3_CTRL_REG_FAULT_STATUS,
+			&fault_sts);
+	if (rc < 0) {
+		dev_err(wled->dev, "Failed to read FAULT_STATUS rc=%d\n", rc);
+		return rc;
+	}
+
+	if (int_rt_sts & WLED3_CTRL_REG_OVP_FAULT_STATUS)
+		*fault_set = true;
+
+	if (fault_sts & (WLED3_CTRL_REG_OVP_FAULT_BIT |
+			       WLED5_CTRL_REG_OVP_PRE_ALARM_BIT))
+		*fault_set = true;
+
+	if (*fault_set)
+		dev_dbg(wled->dev, "WLED OVP fault detected, int_rt_sts=0x%x fault_sts=0x%x\n",
+			int_rt_sts, fault_sts);
+
+	return rc;
+}
+
 static int wled4_ovp_delay(struct wled *wled)
 {
 	return WLED_SOFT_START_DLY_US;
 }
 
+static int wled5_ovp_delay(struct wled *wled)
+{
+	int rc, delay_us;
+	u32 val;
+	u8 ovp_timer_ms[8] = {1, 2, 4, 8, 12, 16, 20, 24};
+
+	/* For WLED5, get the delay based on OVP timer */
+	rc = regmap_read(wled->regmap, wled->ctrl_addr +
+			 WLED5_CTRL_REG_OVP_INT_CTL, &val);
+	if (rc < 0)
+		delay_us =
+		ovp_timer_ms[val & WLED5_CTRL_REG_OVP_INT_TIMER_MASK] * 1000;
+	else
+		delay_us = 2 * WLED_SOFT_START_DLY_US;
+
+	dev_dbg(wled->dev, "delay_time_us: %d\n", *delay_us);
+
+	return delay_us;
+}
+
 static int wled_update_status(struct backlight_device *bl)
 {
 	struct wled *wled = bl_get_data(bl);
@@ -736,9 +797,32 @@ static bool wled_auto_detection_required(struct wled *wled)
 	if (!wled->auto_detection_ovp_count) {
 		wled->start_ovp_fault_time = ktime_get();
 		wled->auto_detection_ovp_count++;
-	} else {
+		return false;
+	}
+
+	if (wled->version == 5) {
+		/*
+		 * WLED5 has OVP fault density interrupt configuration i.e. to
+		 * count the number of OVP alarms for a certain duration before
+		 * triggering OVP fault interrupt. By default, number of OVP
+		 * fault events counted before an interrupt is fired is 32 and
+		 * the time interval is 12 ms. If we see more than one OVP fault
+		 * interrupt, then that should qualify for a real OVP fault
+		 * condition to run auto calibration algorithm.
+		 */
+
+		if (wled->auto_detection_ovp_count > 1) {
+			elapsed_time_us = ktime_us_delta(ktime_get(),
+					wled->start_ovp_fault_time);
+			wled->auto_detection_ovp_count = 0;
+			dev_dbg(wled->dev, "Elapsed time: %lld us\n",
+				elapsed_time_us);
+			return true;
+		}
+		wled->auto_detection_ovp_count++;
+	} else if (wled->version == 4) {
 		elapsed_time_us = ktime_us_delta(ktime_get(),
-						 wled->start_ovp_fault_time);
+					 wled->start_ovp_fault_time);
 		if (elapsed_time_us > WLED_AUTO_DETECT_CNT_DLY_US)
 			wled->auto_detection_ovp_count = 0;
 		else
@@ -1424,6 +1508,8 @@ static int wled_configure(struct wled *wled)
 		wled->wled_set_brightness = wled5_set_brightness;
 		wled->wled_sync_toggle = wled5_sync_toggle;
 		wled->cabc_config = wled5_cabc_config;
+		wled->wled_ovp_fault_status = wled5_ovp_fault_status;
+		wled->wled_ovp_delay = wled5_ovp_delay;
 		wled->max_string_count = 4;
 
 		prop_addr = of_get_address(dev->of_node, 1, NULL, NULL);
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
 a Linux Foundation Collaborative Project

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH V3 1/4] backlight: qcom-wled: convert the wled bindings to .yaml format
  2020-03-09 13:25 ` [PATCH V3 1/4] backlight: qcom-wled: convert the wled bindings to .yaml format Kiran Gunda
@ 2020-03-10 15:01   ` Daniel Thompson
  2020-03-11  7:00     ` kgunda
  2020-03-10 18:31   ` Rob Herring
  1 sibling, 1 reply; 15+ messages in thread
From: Daniel Thompson @ 2020-03-10 15:01 UTC (permalink / raw)
  To: Kiran Gunda
  Cc: bjorn.andersson, jingoohan1, lee.jones, b.zolnierkie, dri-devel,
	jacek.anaszewski, pavel, robh+dt, mark.rutland, linux-leds,
	devicetree, linux-kernel, Dan Murphy, linux-arm-msm

On Mon, Mar 09, 2020 at 06:55:59PM +0530, Kiran Gunda wrote:
> Convert the qcom-wled bindings from .txt to .yaml format.
> 
> Signed-off-by: Kiran Gunda <kgunda@codeaurora.org>

Acked-by: Daniel Thompson <daniel.thompson@linaro.org>


> ---
>  .../bindings/leds/backlight/qcom-wled.txt          | 154 -----------------
>  .../bindings/leds/backlight/qcom-wled.yaml         | 184 +++++++++++++++++++++
>  2 files changed, 184 insertions(+), 154 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/leds/backlight/qcom-wled.txt
>  create mode 100644 Documentation/devicetree/bindings/leds/backlight/qcom-wled.yaml
> 
> diff --git a/Documentation/devicetree/bindings/leds/backlight/qcom-wled.txt b/Documentation/devicetree/bindings/leds/backlight/qcom-wled.txt
> deleted file mode 100644
> index c06863b..0000000
> --- a/Documentation/devicetree/bindings/leds/backlight/qcom-wled.txt
> +++ /dev/null
> @@ -1,154 +0,0 @@
> -Binding for Qualcomm Technologies, Inc. WLED driver
> -
> -WLED (White Light Emitting Diode) driver is used for controlling display
> -backlight that is part of PMIC on Qualcomm Technologies, Inc. reference
> -platforms. The PMIC is connected to the host processor via SPMI bus.
> -
> -- compatible
> -	Usage:        required
> -	Value type:   <string>
> -	Definition:   should be one of:
> -			"qcom,pm8941-wled"
> -			"qcom,pmi8998-wled"
> -			"qcom,pm660l-wled"
> -
> -- reg
> -	Usage:        required
> -	Value type:   <prop encoded array>
> -	Definition:   Base address of the WLED modules.
> -
> -- default-brightness
> -	Usage:        optional
> -	Value type:   <u32>
> -	Definition:   brightness value on boot, value from: 0-4095.
> -		      Default: 2048
> -
> -- label
> -	Usage:        required
> -	Value type:   <string>
> -	Definition:   The name of the backlight device
> -
> -- qcom,cs-out
> -	Usage:        optional
> -	Value type:   <bool>
> -	Definition:   enable current sink output.
> -		      This property is supported only for PM8941.
> -
> -- qcom,cabc
> -	Usage:        optional
> -	Value type:   <bool>
> -	Definition:   enable content adaptive backlight control.
> -
> -- qcom,ext-gen
> -	Usage:        optional
> -	Value type:   <bool>
> -	Definition:   use externally generated modulator signal to dim.
> -		      This property is supported only for PM8941.
> -
> -- qcom,current-limit
> -	Usage:        optional
> -	Value type:   <u32>
> -	Definition:   mA; per-string current limit; value from 0 to 25 with
> -		      1 mA step. Default 20 mA.
> -		      This property is supported only for pm8941.
> -
> -- qcom,current-limit-microamp
> -	Usage:        optional
> -	Value type:   <u32>
> -	Definition:   uA; per-string current limit; value from 0 to 30000 with
> -		      2500 uA step. Default 25 mA.
> -
> -- qcom,current-boost-limit
> -	Usage:        optional
> -	Value type:   <u32>
> -	Definition:   mA; boost current limit.
> -		      For pm8941: one of: 105, 385, 525, 805, 980, 1260, 1400,
> -		      1680. Default: 805 mA.
> -		      For pmi8998: one of: 105, 280, 450, 620, 970, 1150, 1300,
> -		      1500. Default: 970 mA.
> -
> -- qcom,switching-freq
> -	Usage:        optional
> -	Value type:   <u32>
> -	 Definition:   kHz; switching frequency; one of: 600, 640, 685, 738,
> -		       800, 872, 960, 1066, 1200, 1371, 1600, 1920, 2400, 3200,
> -		       4800, 9600.
> -		       Default: for pm8941: 1600 kHz
> -				for pmi8998: 800 kHz
> -
> -- qcom,ovp
> -	Usage:        optional
> -	Value type:   <u32>
> -	Definition:   V; Over-voltage protection limit; one of:
> -		      27, 29, 32, 35. Default: 29V
> -		      This property is supported only for PM8941.
> -
> -- qcom,ovp-millivolt
> -	Usage:        optional
> -	Value type:   <u32>
> -	Definition:   mV; Over-voltage protection limit;
> -		      For pmi8998: one of 18100, 19600, 29600, 31100.
> -		      Default 29600 mV.
> -		      If this property is not specified for PM8941, it
> -		      falls back to "qcom,ovp" property.
> -
> -- qcom,num-strings
> -	Usage:        optional
> -	Value type:   <u32>
> -	Definition:   #; number of led strings attached;
> -		      value: For PM8941 from 1 to 3. Default: 2
> -			     For PMI8998 from 1 to 4.
> -
> -- interrupts
> -	Usage:        optional
> -	Value type:   <prop encoded array>
> -	Definition:   Interrupts associated with WLED. This should be
> -		      "short" and "ovp" interrupts. Interrupts can be
> -		      specified as per the encoding listed under
> -		      Documentation/devicetree/bindings/spmi/
> -		      qcom,spmi-pmic-arb.txt.
> -
> -- interrupt-names
> -	Usage:        optional
> -	Value type:   <string>
> -	Definition:   Interrupt names associated with the interrupts.
> -		      Must be "short" and "ovp". The short circuit detection
> -		      is not supported for PM8941.
> -
> -- qcom,enabled-strings
> -	Usage:        optional
> -	Value tyoe:   <u32 array>
> -	Definition:   Array of the WLED strings numbered from 0 to 3. Each
> -		      string of leds are operated individually. Specify the
> -		      list of strings used by the device. Any combination of
> -		      led strings can be used.
> -
> -- qcom,external-pfet
> -	Usage:        optional
> -	Value type:   <bool>
> -	Definition:   Specify if external PFET control for short circuit
> -		      protection is used. This property is supported only
> -		      for PMI8998.
> -
> -- qcom,auto-string-detection
> -	Usage:        optional
> -	Value type:   <bool>
> -	Definition:   Enables auto-detection of the WLED string configuration.
> -		      This feature is not supported for PM8941.
> -
> -
> -Example:
> -
> -pm8941-wled@d800 {
> -	compatible = "qcom,pm8941-wled";
> -	reg = <0xd800>;
> -	label = "backlight";
> -
> -	qcom,cs-out;
> -	qcom,current-limit = <20>;
> -	qcom,current-boost-limit = <805>;
> -	qcom,switching-freq = <1600>;
> -	qcom,ovp = <29>;
> -	qcom,num-strings = <2>;
> -	qcom,enabled-strings = <0 1>;
> -};
> diff --git a/Documentation/devicetree/bindings/leds/backlight/qcom-wled.yaml b/Documentation/devicetree/bindings/leds/backlight/qcom-wled.yaml
> new file mode 100644
> index 0000000..d334f81
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/leds/backlight/qcom-wled.yaml
> @@ -0,0 +1,184 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/bindings/leds/backlight/qcom-wled.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Binding for Qualcomm Technologies, Inc. WLED driver
> +
> +maintainers:
> +  - Lee Jones <lee.jones@linaro.org>
> +
> +description: |
> +  WLED (White Light Emitting Diode) driver is used for controlling display
> +  backlight that is part of PMIC on Qualcomm Technologies, Inc. reference
> +  platforms. The PMIC is connected to the host processor via SPMI bus.
> +
> +properties:
> +  compatible :
> +    enum:
> +       - qcom,pm8941-wled
> +       - qcom,pmi8998-wled
> +       - qcom,pm660l-wled
> +
> +  reg:
> +    maxItems: 1
> +
> +  default-brightness:
> +    maxItems: 1
> +    description:
> +      brightness value on boot, value from 0-4095.
> +    allOf:
> +      - $ref: /schemas/types.yaml#/definitions/uint32
> +        default: 2048
> +
> +  label:
> +    maxItems: 1
> +    description:
> +      The name of the backlight device.
> +    allOf:
> +      - $ref: /schemas/types.yaml#/definitions/string
> +
> +  qcom,cs-out:
> +    description:
> +      enable current sink output.
> +      This property is supported only for PM8941.
> +    type: boolean
> +
> +  qcom,cabc:
> +    description:
> +      enable content adaptive backlight control.
> +    type: boolean
> +
> +  qcom,ext-gen:
> +    description:
> +      use externally generated modulator signal to dim.
> +      This property is supported only for PM8941.
> +    type: boolean
> +
> +  qcom,current-limit:
> +    maxItems: 1
> +    allOf:
> +      - $ref: /schemas/types.yaml#/definitions/uint32
> +    description:
> +      mA; per-string current limit; value from 0 to 25 with
> +      1 mA step. This property is supported only for pm8941.
> +    default: 20
> +
> +  qcom,current-limit-microamp:
> +    maxItems: 1
> +    allOf:
> +      - $ref: /schemas/types.yaml#/definitions/uint32
> +    description:
> +      uA; per-string current limit; value from 0 to 30000 with
> +      2500 uA step.
> +    default: 25
> +
> +  qcom,current-boost-limit:
> +    maxItems: 1
> +    allOf:
> +      - $ref: /schemas/types.yaml#/definitions/uint32
> +    description:
> +      mA; boost current limit.
> +      For pm8941 one of 105, 385, 525, 805, 980, 1260, 1400, 1680.
> +      Default, 805 mA.
> +      For pmi8998 one of 105, 280, 450, 620, 970, 1150, 1300,
> +      1500. Default 970 mA.
> +
> +  qcom,switching-freq:
> +    maxItems: 1
> +    allOf:
> +      - $ref: /schemas/types.yaml#/definitions/uint32
> +    description:
> +      kHz; switching frequency; one of 600, 640, 685, 738,
> +      800, 872, 960, 1066, 1200, 1371, 1600, 1920, 2400, 3200,
> +      4800, 9600.
> +      Default for pm8941 1600 kHz
> +               for pmi8998 800 kHz
> +
> +  qcom,ovp:
> +    maxItems: 1
> +    allOf:
> +      - $ref: /schemas/types.yaml#/definitions/uint32
> +    description:
> +      V; Over-voltage protection limit; one of 27, 29, 32, 35. Default 29V
> +      This property is supported only for PM8941.
> +
> +  qcom,ovp-millivolt:
> +    maxItems: 1
> +    allOf:
> +      - $ref: /schemas/types.yaml#/definitions/uint32
> +    description:
> +      mV; Over-voltage protection limit;
> +      For pmi8998 one of 18100, 19600, 29600, 31100.
> +      Default 29600 mV.
> +      If this property is not specified for PM8941, it
> +      falls back to "qcom,ovp" property.
> +
> +  qcom,num-strings:
> +    maxItems: 1
> +    allOf:
> +      - $ref: /schemas/types.yaml#/definitions/uint32
> +    description:
> +      number of led strings attached;
> +      value for PM8941 from 1 to 3. Default 2
> +      For PMI8998 from 1 to 4.
> +
> +  interrupts:
> +    maxItems: 2
> +    description:
> +      Interrupts associated with WLED. This should be
> +      "short" and "ovp" interrupts. Interrupts can be
> +      specified as per the encoding listed under
> +      Documentation/devicetree/bindings/spmi/
> +      qcom,spmi-pmic-arb.txt.
> +
> +  interrupt-names:
> +    description:
> +      Interrupt names associated with the interrupts.
> +      Must be "short" and "ovp". The short circuit detection
> +      is not supported for PM8941.
> +
> +  qcom,enabled-strings:
> +    maxItems: 1
> +    allOf:
> +      - $ref: /schemas/types.yaml#/definitions/uint32-array
> +    description:
> +      Array of the WLED strings numbered from 0 to 3. Each
> +      string of leds are operated individually. Specify the
> +      list of strings used by the device. Any combination of
> +      led strings can be used.
> +
> +  qcom,external-pfet:
> +    description:
> +      Specify if external PFET control for short circuit
> +      protection is used. This property is supported only
> +      for PMI8998.
> +    type: boolean
> +
> +  qcom,auto-string-detection:
> +    description:
> +      Enables auto-detection of the WLED string configuration.
> +      This feature is not supported for PM8941.
> +    type: boolean
> +
> +required:
> +  - compatible
> +  - reg
> +  - label
> +
> +examples:
> +  - |
> +    pm8941-wled@d800 {
> +        compatible = "qcom,pm8941-wled";
> +        reg = <0xd800 0x100>;
> +        label = "backlight";
> +
> +        qcom,cs-out;
> +        qcom,current-limit = <20>;
> +        qcom,current-boost-limit = <805>;
> +        qcom,switching-freq = <1600>;
> +        qcom,ovp = <29>;
> +        qcom,num-strings = <2>;
> +        qcom,enabled-strings = <0 1>;
> +     };
> -- 
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
>  a Linux Foundation Collaborative Project

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH V3 2/4] backlight: qcom-wled: Add callback functions
  2020-03-09 13:26 ` [PATCH V3 2/4] backlight: qcom-wled: Add callback functions Kiran Gunda
@ 2020-03-10 15:27   ` Daniel Thompson
  2020-03-11  6:41     ` kgunda
  0 siblings, 1 reply; 15+ messages in thread
From: Daniel Thompson @ 2020-03-10 15:27 UTC (permalink / raw)
  To: Kiran Gunda
  Cc: bjorn.andersson, jingoohan1, lee.jones, b.zolnierkie, dri-devel,
	jacek.anaszewski, pavel, robh+dt, mark.rutland, linux-leds,
	devicetree, linux-kernel, Andy Gross, linux-arm-msm, linux-fbdev

On Mon, Mar 09, 2020 at 06:56:00PM +0530, Kiran Gunda wrote:
> Add cabc_config, sync_toggle, wled_ovp_fault_status and wled_ovp_delay
> callback functions to prepare the driver for adding WLED5 support.
> 
> Signed-off-by: Kiran Gunda <kgunda@codeaurora.org>

Overall this code would a lot easier to review if
> ---
>  drivers/video/backlight/qcom-wled.c | 196 +++++++++++++++++++++++-------------
>  1 file changed, 126 insertions(+), 70 deletions(-)
> 
> diff --git a/drivers/video/backlight/qcom-wled.c b/drivers/video/backlight/qcom-wled.c
> index 3d276b3..b73f273 100644
> --- a/drivers/video/backlight/qcom-wled.c
> +++ b/drivers/video/backlight/qcom-wled.c
> @@ -128,6 +128,7 @@ struct wled_config {
>  	bool cs_out_en;
>  	bool ext_gen;
>  	bool cabc;
> +	bool en_cabc;

Does this ever get set to true?

>  	bool external_pfet;
>  	bool auto_detection_enabled;
>  };
> @@ -147,14 +148,20 @@ struct wled {
>  	u32 max_brightness;
>  	u32 short_count;
>  	u32 auto_detect_count;
> +	u32 version;
>  	bool disabled_by_short;
>  	bool has_short_detect;
> +	bool cabc_disabled;
>  	int short_irq;
>  	int ovp_irq;
>  
>  	struct wled_config cfg;
>  	struct delayed_work ovp_work;
>  	int (*wled_set_brightness)(struct wled *wled, u16 brightness);
> +	int (*cabc_config)(struct wled *wled, bool enable);
> +	int (*wled_sync_toggle)(struct wled *wled);
> +	int (*wled_ovp_fault_status)(struct wled *wled, bool *fault_set);
> +	int (*wled_ovp_delay)(struct wled *wled);

Let's get some doc comments explaining what these callbacks do (and
which versions they apply to).

cabc_config() in particular appears to have a very odd interface for
wled4.  It looks like it relies on being initially called with enable
set a particular way and prevents itself from acting again. Therefore if
the comment you end up writing doesn't sound "right" then please also
fix the API!

Finally, why is everything except cabc_config() prefixed with wled?


Daniel.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH V3 3/4] backlight: qcom-wled: Add support for WLED5 peripheral in PM8150L
  2020-03-09 13:26 ` [PATCH V3 3/4] backlight: qcom-wled: Add support for WLED5 peripheral in PM8150L Kiran Gunda
@ 2020-03-10 15:45   ` Daniel Thompson
  2020-03-11  7:00     ` kgunda
  0 siblings, 1 reply; 15+ messages in thread
From: Daniel Thompson @ 2020-03-10 15:45 UTC (permalink / raw)
  To: Kiran Gunda
  Cc: bjorn.andersson, jingoohan1, lee.jones, b.zolnierkie, dri-devel,
	jacek.anaszewski, pavel, robh+dt, mark.rutland, linux-leds,
	devicetree, linux-kernel, Dan Murphy, Andy Gross, linux-arm-msm,
	linux-fbdev

On Mon, Mar 09, 2020 at 06:56:01PM +0530, Kiran Gunda wrote:
> Add support for WLED5 peripheral that is present on PM8150L PMICs.
> 
> PM8150L WLED supports the following:
>     - Two modulators and each sink can use any of the modulator
>     - Multiple CABC selection options
>     - Multiple brightness width selection (12 bits to 15 bits)
> 
> Signed-off-by: Kiran Gunda <kgunda@codeaurora.org>

Mostly just style comments below...


> ---
>  .../bindings/leds/backlight/qcom-wled.yaml         |  39 +++
>  drivers/video/backlight/qcom-wled.c                | 336 ++++++++++++++++++++-

Shouldn't the bindings and driver be separate?


>  2 files changed, 374 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/leds/backlight/qcom-wled.yaml b/Documentation/devicetree/bindings/leds/backlight/qcom-wled.yaml
> index d334f81..e0dadc4 100644
> --- a/Documentation/devicetree/bindings/leds/backlight/qcom-wled.yaml
> +++ b/Documentation/devicetree/bindings/leds/backlight/qcom-wled.yaml
> @@ -20,6 +20,7 @@ properties:
>         - qcom,pm8941-wled
>         - qcom,pmi8998-wled
>         - qcom,pm660l-wled
> +       - qcom,pm8150l-wled
>  
>    reg:
>      maxItems: 1
> @@ -28,10 +29,23 @@ properties:
>      maxItems: 1
>      description:
>        brightness value on boot, value from 0-4095.
> +      For pm8150l this value vary from 0-4095 or 0-32767
> +      depending on the brightness control mode. If CABC is
> +      enabled 0-4095 range is used.

Is this a pm8150l restriction or a WLED5 restriction (will other WLED5
have different ranges)?


>      allOf:
>        - $ref: /schemas/types.yaml#/definitions/uint32
>          default: 2048
>  
> +  max-brightness:
> +    maxItems: 1
> +    description:
> +      Maximum brightness level. Allowed values are,
> +      for pmi8998 it is  0-4095.
> +      For pm8150l, this can be either 4095 or 32767.

Ditto.


> +      If CABC is enabled, this is capped to 4095.
> +    allOf:
> +      - $ref: /schemas/types.yaml#/definitions/uint32
> +
>    label:
>      maxItems: 1
>      description:
> @@ -124,6 +138,31 @@ properties:
>        value for PM8941 from 1 to 3. Default 2
>        For PMI8998 from 1 to 4.
>  
> +  qcom,modulator-sel:
> +    maxItems: 1
> +    allOf:
> +      - $ref: /schemas/types.yaml#/definitions/uint32
> +    description:
> +      Selects the modulator used for brightness modulation.
> +      Allowed values are,
> +               0 - Modulator A
> +               1 - Modulator B
> +      If not specified, then modulator A will be used by default.
> +      This property is applicable only to WLED5 peripheral.
> +
> +  qcom,cabc-sel:
> +    maxItems: 1
> +    allOf:
> +      - $ref: /schemas/types.yaml#/definitions/uint32
> +    description:
> +      Selects the CABC pin signal used for brightness modulation.
> +      Allowed values are,
> +              0 - CABC disabled
> +              1 - CABC 1
> +              2 - CABC 2
> +              3 - External signal (e.g. LPG) is used for dimming
> +      This property is applicable only to WLED5 peripheral.
> +
>    interrupts:
>      maxItems: 2
>      description:
> diff --git a/drivers/video/backlight/qcom-wled.c b/drivers/video/backlight/qcom-wled.c
> index b73f273..edbbcb2 100644
> --- a/drivers/video/backlight/qcom-wled.c
> +++ b/drivers/video/backlight/qcom-wled.c
> @@ -19,6 +19,8 @@
>  #define WLED_DEFAULT_BRIGHTNESS				2048
>  #define WLED_SOFT_START_DLY_US				10000
>  #define WLED3_SINK_REG_BRIGHT_MAX			0xFFF
> +#define WLED5_SINK_REG_BRIGHT_MAX_12B			0xFFF
> +#define WLED5_SINK_REG_BRIGHT_MAX_15B			0x7FFF
>  
>  /* WLED3/WLED4 control registers */
>  #define WLED3_CTRL_REG_FAULT_STATUS			0x08
> @@ -40,6 +42,7 @@
>  
>  #define WLED3_CTRL_REG_OVP				0x4d
>  #define  WLED3_CTRL_REG_OVP_MASK			GENMASK(1, 0)
> +#define  WLED5_CTRL_REG_OVP_MASK			GENMASK(3, 0)
>  
>  #define WLED3_CTRL_REG_ILIMIT				0x4e
>  #define  WLED3_CTRL_REG_ILIMIT_MASK			GENMASK(2, 0)
> @@ -101,6 +104,40 @@
>  
>  #define WLED4_SINK_REG_BRIGHT(n)			(0x57 + (n * 0x10))
>  
> +/* WLED5 specific sink registers */
> +#define WLED5_SINK_REG_MOD_A_EN				0x50
> +#define WLED5_SINK_REG_MOD_B_EN				0x60
> +#define  WLED5_SINK_REG_MOD_EN_MASK			BIT(7)
> +
> +#define WLED5_SINK_REG_MOD_A_SRC_SEL			0x51
> +#define WLED5_SINK_REG_MOD_B_SRC_SEL			0x61
> +#define  WLED5_SINK_REG_MOD_SRC_SEL_HIGH		0
> +#define  WLED5_SINK_REG_MOD_SRC_SEL_EXT			0x03
> +#define  WLED5_SINK_REG_MOD_SRC_SEL_MASK		GENMASK(1, 0)
> +
> +#define WLED5_SINK_REG_MOD_A_BRIGHTNESS_WIDTH_SEL	0x52
> +#define WLED5_SINK_REG_MOD_B_BRIGHTNESS_WIDTH_SEL	0x62
> +#define  WLED5_SINK_REG_BRIGHTNESS_WIDTH_12B		0
> +#define  WLED5_SINK_REG_BRIGHTNESS_WIDTH_15B		1
> +
> +#define WLED5_SINK_REG_MOD_A_BRIGHTNESS_LSB		0x53
> +#define WLED5_SINK_REG_MOD_A_BRIGHTNESS_MSB		0x54
> +#define WLED5_SINK_REG_MOD_B_BRIGHTNESS_LSB		0x63
> +#define WLED5_SINK_REG_MOD_B_BRIGHTNESS_MSB		0x64
> +
> +#define WLED5_SINK_REG_MOD_SYNC_BIT			0x65
> +#define  WLED5_SINK_REG_SYNC_MOD_A_BIT			BIT(0)
> +#define  WLED5_SINK_REG_SYNC_MOD_B_BIT			BIT(1)
> +#define  WLED5_SINK_REG_SYNC_MASK			GENMASK(1, 0)
> +
> +/* WLED5 specific per-'string' registers below */
> +#define WLED5_SINK_REG_STR_FULL_SCALE_CURR(n)		(0x72 + (n * 0x10))
> +
> +#define WLED5_SINK_REG_STR_SRC_SEL(n)			(0x73 + (n * 0x10))
> +#define  WLED5_SINK_REG_SRC_SEL_MOD_A			0
> +#define  WLED5_SINK_REG_SRC_SEL_MOD_B			1
> +#define  WLED5_SINK_REG_SRC_SEL_MASK			GENMASK(1, 0)
> +
>  struct wled_var_cfg {
>  	const u32 *values;
>  	u32 (*fn)(u32);
> @@ -125,6 +162,8 @@ struct wled_config {
>  	u32 num_strings;
>  	u32 string_i_limit;
>  	u32 enabled_strings[WLED_MAX_STRINGS];

> +	u32 mod_sel;
> +	u32 cabc_sel;

Please explain cabc_sel (wled5) versus cabc_en (wled4).

>  	bool cs_out_en;
>  	bool ext_gen;
>  	bool cabc;
> @@ -164,6 +203,27 @@ struct wled {
>  	int (*wled_ovp_delay)(struct wled *wled);
>  };
>  
> +enum wled5_mod_sel {
> +	MOD_A,
> +	MOD_B,
> +	MOD_MAX,
> +};
> +
> +static const u8 wled5_brightness_reg[MOD_MAX] = {
> +	[MOD_A] = WLED5_SINK_REG_MOD_A_BRIGHTNESS_LSB,
> +	[MOD_B] = WLED5_SINK_REG_MOD_B_BRIGHTNESS_LSB,
> +};
> +
> +static const u8 wled5_src_sel_reg[MOD_MAX] = {
> +	[MOD_A] = WLED5_SINK_REG_MOD_A_SRC_SEL,
> +	[MOD_B] = WLED5_SINK_REG_MOD_B_SRC_SEL,
> +};
> +
> +static const u8 wled5_brt_wid_sel_reg[MOD_MAX] = {
> +	[MOD_A] = WLED5_SINK_REG_MOD_A_BRIGHTNESS_WIDTH_SEL,
> +	[MOD_B] = WLED5_SINK_REG_MOD_B_BRIGHTNESS_WIDTH_SEL,
> +};
> +
>  static int wled3_set_brightness(struct wled *wled, u16 brightness)
>  {
>  	int rc, i;
> @@ -182,6 +242,25 @@ static int wled3_set_brightness(struct wled *wled, u16 brightness)
>  	return 0;
>  }
>  
> +static int wled5_set_brightness(struct wled *wled, u16 brightness)
> +{
> +	int rc, offset;
> +	u16 low_limit = wled->max_brightness * 1 / 1000;
> +	u8 v[2];
> +
> +	/* WLED5's lower limit is 0.1% */
> +	if (brightness < low_limit)
> +		brightness = low_limit;
> +
> +	v[0] = brightness & 0xff;
> +	v[1] = (brightness >> 8) & 0x7f;
> +
> +	offset = wled5_brightness_reg[wled->cfg.mod_sel];
> +	rc = regmap_bulk_write(wled->regmap, wled->sink_addr + offset,
> +			       v, 2);
> +	return rc;
> +}
> +

Can we keep the same ordering throughout the file (wled3, wled4, wled5)?
Most of the wled5 callbacks seem to have been inserted above wled4.


Daniel.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH V3 1/4] backlight: qcom-wled: convert the wled bindings to .yaml format
  2020-03-09 13:25 ` [PATCH V3 1/4] backlight: qcom-wled: convert the wled bindings to .yaml format Kiran Gunda
  2020-03-10 15:01   ` Daniel Thompson
@ 2020-03-10 18:31   ` Rob Herring
  2020-03-11  7:03     ` kgunda
  1 sibling, 1 reply; 15+ messages in thread
From: Rob Herring @ 2020-03-10 18:31 UTC (permalink / raw)
  To: Kiran Gunda
  Cc: bjorn.andersson, jingoohan1, lee.jones, b.zolnierkie, dri-devel,
	daniel.thompson, jacek.anaszewski, pavel, robh+dt, mark.rutland,
	linux-leds, devicetree, linux-kernel, Dan Murphy, linux-arm-msm,
	Kiran Gunda

On Mon,  9 Mar 2020 18:55:59 +0530, Kiran Gunda wrote:
> Convert the qcom-wled bindings from .txt to .yaml format.
> 
> Signed-off-by: Kiran Gunda <kgunda@codeaurora.org>
> ---
>  .../bindings/leds/backlight/qcom-wled.txt          | 154 -----------------
>  .../bindings/leds/backlight/qcom-wled.yaml         | 184 +++++++++++++++++++++
>  2 files changed, 184 insertions(+), 154 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/leds/backlight/qcom-wled.txt
>  create mode 100644 Documentation/devicetree/bindings/leds/backlight/qcom-wled.yaml
> 

My bot found errors running 'make dt_binding_check' on your patch:

Documentation/devicetree/bindings/leds/backlight/qcom-wled.yaml: $id: relative path/filename doesn't match actual path or filename
	expected: http://devicetree.org/schemas/leds/backlight/qcom-wled.yaml#

See https://patchwork.ozlabs.org/patch/1251567
Please check and re-submit.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH V3 2/4] backlight: qcom-wled: Add callback functions
  2020-03-10 15:27   ` Daniel Thompson
@ 2020-03-11  6:41     ` kgunda
  2020-03-11 10:30       ` Daniel Thompson
  0 siblings, 1 reply; 15+ messages in thread
From: kgunda @ 2020-03-11  6:41 UTC (permalink / raw)
  To: Daniel Thompson
  Cc: bjorn.andersson, jingoohan1, lee.jones, b.zolnierkie, dri-devel,
	jacek.anaszewski, pavel, robh+dt, mark.rutland, linux-leds,
	devicetree, linux-kernel, Andy Gross, linux-arm-msm, linux-fbdev,
	linux-arm-msm-owner

On 2020-03-10 20:57, Daniel Thompson wrote:
> On Mon, Mar 09, 2020 at 06:56:00PM +0530, Kiran Gunda wrote:
>> Add cabc_config, sync_toggle, wled_ovp_fault_status and wled_ovp_delay
>> callback functions to prepare the driver for adding WLED5 support.
>> 
>> Signed-off-by: Kiran Gunda <kgunda@codeaurora.org>
> 
> Overall this code would a lot easier to review if
>> ---
>>  drivers/video/backlight/qcom-wled.c | 196 
>> +++++++++++++++++++++++-------------
>>  1 file changed, 126 insertions(+), 70 deletions(-)
>> 
>> diff --git a/drivers/video/backlight/qcom-wled.c 
>> b/drivers/video/backlight/qcom-wled.c
>> index 3d276b3..b73f273 100644
>> --- a/drivers/video/backlight/qcom-wled.c
>> +++ b/drivers/video/backlight/qcom-wled.c
>> @@ -128,6 +128,7 @@ struct wled_config {
>>  	bool cs_out_en;
>>  	bool ext_gen;
>>  	bool cabc;
>> +	bool en_cabc;
> 
> Does this ever get set to true?
> 
Yes. If user wants use the cabc pin to control the brightness and
use the "qcom,cabc" DT property in the device tree.

>>  	bool external_pfet;
>>  	bool auto_detection_enabled;
>>  };
>> @@ -147,14 +148,20 @@ struct wled {
>>  	u32 max_brightness;
>>  	u32 short_count;
>>  	u32 auto_detect_count;
>> +	u32 version;
>>  	bool disabled_by_short;
>>  	bool has_short_detect;
>> +	bool cabc_disabled;
>>  	int short_irq;
>>  	int ovp_irq;
>> 
>>  	struct wled_config cfg;
>>  	struct delayed_work ovp_work;
>>  	int (*wled_set_brightness)(struct wled *wled, u16 brightness);
>> +	int (*cabc_config)(struct wled *wled, bool enable);
>> +	int (*wled_sync_toggle)(struct wled *wled);
>> +	int (*wled_ovp_fault_status)(struct wled *wled, bool *fault_set);
>> +	int (*wled_ovp_delay)(struct wled *wled);
> 
> Let's get some doc comments explaining what these callbacks do (and
> which versions they apply to).
> 
Sure. I will update it in the commit text in next post.

> cabc_config() in particular appears to have a very odd interface for
> wled4.  It looks like it relies on being initially called with enable
> set a particular way and prevents itself from acting again. Therefore 
> if
> the comment you end up writing doesn't sound "right" then please also
> fix the API!
> 
Actually this variable is useful for WLED5, where the default HW state 
is
CABC1 enabled mode. So, if the user doesn't want to use the CABC we are 
configuring
the HW state to "0" based on the DT property and then setting a flag to 
not enable it again.
This is not needed for WLED4. I will remove it for WLED4 in next post.

> Finally, why is everything except cabc_config() prefixed with wled?
> 
It is typo. I will correct it in the next post.
> 
> Daniel.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH V3 3/4] backlight: qcom-wled: Add support for WLED5 peripheral in PM8150L
  2020-03-10 15:45   ` Daniel Thompson
@ 2020-03-11  7:00     ` kgunda
  0 siblings, 0 replies; 15+ messages in thread
From: kgunda @ 2020-03-11  7:00 UTC (permalink / raw)
  To: Daniel Thompson
  Cc: bjorn.andersson, jingoohan1, lee.jones, b.zolnierkie, dri-devel,
	jacek.anaszewski, pavel, robh+dt, mark.rutland, linux-leds,
	devicetree, linux-kernel, Dan Murphy, Andy Gross, linux-arm-msm,
	linux-fbdev, linux-arm-msm-owner

On 2020-03-10 21:15, Daniel Thompson wrote:
> On Mon, Mar 09, 2020 at 06:56:01PM +0530, Kiran Gunda wrote:
>> Add support for WLED5 peripheral that is present on PM8150L PMICs.
>> 
>> PM8150L WLED supports the following:
>>     - Two modulators and each sink can use any of the modulator
>>     - Multiple CABC selection options
>>     - Multiple brightness width selection (12 bits to 15 bits)
>> 
>> Signed-off-by: Kiran Gunda <kgunda@codeaurora.org>
> 
> Mostly just style comments below...
> 
> 
>> ---
>>  .../bindings/leds/backlight/qcom-wled.yaml         |  39 +++
>>  drivers/video/backlight/qcom-wled.c                | 336 
>> ++++++++++++++++++++-
> 
> Shouldn't the bindings and driver be separate?
> 
> 
Ok. I will split it out in to a separate patch in next post.
>>  2 files changed, 374 insertions(+), 1 deletion(-)
>> 
>> diff --git 
>> a/Documentation/devicetree/bindings/leds/backlight/qcom-wled.yaml 
>> b/Documentation/devicetree/bindings/leds/backlight/qcom-wled.yaml
>> index d334f81..e0dadc4 100644
>> --- a/Documentation/devicetree/bindings/leds/backlight/qcom-wled.yaml
>> +++ b/Documentation/devicetree/bindings/leds/backlight/qcom-wled.yaml
>> @@ -20,6 +20,7 @@ properties:
>>         - qcom,pm8941-wled
>>         - qcom,pmi8998-wled
>>         - qcom,pm660l-wled
>> +       - qcom,pm8150l-wled
>> 
>>    reg:
>>      maxItems: 1
>> @@ -28,10 +29,23 @@ properties:
>>      maxItems: 1
>>      description:
>>        brightness value on boot, value from 0-4095.
>> +      For pm8150l this value vary from 0-4095 or 0-32767
>> +      depending on the brightness control mode. If CABC is
>> +      enabled 0-4095 range is used.
> 
> Is this a pm8150l restriction or a WLED5 restriction (will other WLED5
> have different ranges)?
> 
It is a WLED5 restriction which is used in pm8150l PMIC.
> 
>>      allOf:
>>        - $ref: /schemas/types.yaml#/definitions/uint32
>>          default: 2048
>> 
>> +  max-brightness:
>> +    maxItems: 1
>> +    description:
>> +      Maximum brightness level. Allowed values are,
>> +      for pmi8998 it is  0-4095.
>> +      For pm8150l, this can be either 4095 or 32767.
> 
> Ditto.
> 
It is a WLED5 restriction which is used in pm8150l PMIC.
> 
>> +      If CABC is enabled, this is capped to 4095.
>> +    allOf:
>> +      - $ref: /schemas/types.yaml#/definitions/uint32
>> +
>>    label:
>>      maxItems: 1
>>      description:
>> @@ -124,6 +138,31 @@ properties:
>>        value for PM8941 from 1 to 3. Default 2
>>        For PMI8998 from 1 to 4.
>> 
>> +  qcom,modulator-sel:
>> +    maxItems: 1
>> +    allOf:
>> +      - $ref: /schemas/types.yaml#/definitions/uint32
>> +    description:
>> +      Selects the modulator used for brightness modulation.
>> +      Allowed values are,
>> +               0 - Modulator A
>> +               1 - Modulator B
>> +      If not specified, then modulator A will be used by default.
>> +      This property is applicable only to WLED5 peripheral.
>> +
>> +  qcom,cabc-sel:
>> +    maxItems: 1
>> +    allOf:
>> +      - $ref: /schemas/types.yaml#/definitions/uint32
>> +    description:
>> +      Selects the CABC pin signal used for brightness modulation.
>> +      Allowed values are,
>> +              0 - CABC disabled
>> +              1 - CABC 1
>> +              2 - CABC 2
>> +              3 - External signal (e.g. LPG) is used for dimming
>> +      This property is applicable only to WLED5 peripheral.
>> +
>>    interrupts:
>>      maxItems: 2
>>      description:
>> diff --git a/drivers/video/backlight/qcom-wled.c 
>> b/drivers/video/backlight/qcom-wled.c
>> index b73f273..edbbcb2 100644
>> --- a/drivers/video/backlight/qcom-wled.c
>> +++ b/drivers/video/backlight/qcom-wled.c
>> @@ -19,6 +19,8 @@
>>  #define WLED_DEFAULT_BRIGHTNESS				2048
>>  #define WLED_SOFT_START_DLY_US				10000
>>  #define WLED3_SINK_REG_BRIGHT_MAX			0xFFF
>> +#define WLED5_SINK_REG_BRIGHT_MAX_12B			0xFFF
>> +#define WLED5_SINK_REG_BRIGHT_MAX_15B			0x7FFF
>> 
>>  /* WLED3/WLED4 control registers */
>>  #define WLED3_CTRL_REG_FAULT_STATUS			0x08
>> @@ -40,6 +42,7 @@
>> 
>>  #define WLED3_CTRL_REG_OVP				0x4d
>>  #define  WLED3_CTRL_REG_OVP_MASK			GENMASK(1, 0)
>> +#define  WLED5_CTRL_REG_OVP_MASK			GENMASK(3, 0)
>> 
>>  #define WLED3_CTRL_REG_ILIMIT				0x4e
>>  #define  WLED3_CTRL_REG_ILIMIT_MASK			GENMASK(2, 0)
>> @@ -101,6 +104,40 @@
>> 
>>  #define WLED4_SINK_REG_BRIGHT(n)			(0x57 + (n * 0x10))
>> 
>> +/* WLED5 specific sink registers */
>> +#define WLED5_SINK_REG_MOD_A_EN				0x50
>> +#define WLED5_SINK_REG_MOD_B_EN				0x60
>> +#define  WLED5_SINK_REG_MOD_EN_MASK			BIT(7)
>> +
>> +#define WLED5_SINK_REG_MOD_A_SRC_SEL			0x51
>> +#define WLED5_SINK_REG_MOD_B_SRC_SEL			0x61
>> +#define  WLED5_SINK_REG_MOD_SRC_SEL_HIGH		0
>> +#define  WLED5_SINK_REG_MOD_SRC_SEL_EXT			0x03
>> +#define  WLED5_SINK_REG_MOD_SRC_SEL_MASK		GENMASK(1, 0)
>> +
>> +#define WLED5_SINK_REG_MOD_A_BRIGHTNESS_WIDTH_SEL	0x52
>> +#define WLED5_SINK_REG_MOD_B_BRIGHTNESS_WIDTH_SEL	0x62
>> +#define  WLED5_SINK_REG_BRIGHTNESS_WIDTH_12B		0
>> +#define  WLED5_SINK_REG_BRIGHTNESS_WIDTH_15B		1
>> +
>> +#define WLED5_SINK_REG_MOD_A_BRIGHTNESS_LSB		0x53
>> +#define WLED5_SINK_REG_MOD_A_BRIGHTNESS_MSB		0x54
>> +#define WLED5_SINK_REG_MOD_B_BRIGHTNESS_LSB		0x63
>> +#define WLED5_SINK_REG_MOD_B_BRIGHTNESS_MSB		0x64
>> +
>> +#define WLED5_SINK_REG_MOD_SYNC_BIT			0x65
>> +#define  WLED5_SINK_REG_SYNC_MOD_A_BIT			BIT(0)
>> +#define  WLED5_SINK_REG_SYNC_MOD_B_BIT			BIT(1)
>> +#define  WLED5_SINK_REG_SYNC_MASK			GENMASK(1, 0)
>> +
>> +/* WLED5 specific per-'string' registers below */
>> +#define WLED5_SINK_REG_STR_FULL_SCALE_CURR(n)		(0x72 + (n * 0x10))
>> +
>> +#define WLED5_SINK_REG_STR_SRC_SEL(n)			(0x73 + (n * 0x10))
>> +#define  WLED5_SINK_REG_SRC_SEL_MOD_A			0
>> +#define  WLED5_SINK_REG_SRC_SEL_MOD_B			1
>> +#define  WLED5_SINK_REG_SRC_SEL_MASK			GENMASK(1, 0)
>> +
>>  struct wled_var_cfg {
>>  	const u32 *values;
>>  	u32 (*fn)(u32);
>> @@ -125,6 +162,8 @@ struct wled_config {
>>  	u32 num_strings;
>>  	u32 string_i_limit;
>>  	u32 enabled_strings[WLED_MAX_STRINGS];
> 
>> +	u32 mod_sel;
>> +	u32 cabc_sel;
> 
> Please explain cabc_sel (wled5) versus cabc_en (wled4).
> 
Ok. WLED5 has 2 CABC modules, from which one can be selected/enabled.
wled4 has only one CABC module and it is just enabled based on the user 
input.
I will add a comment in next post.

>>  	bool cs_out_en;
>>  	bool ext_gen;
>>  	bool cabc;
>> @@ -164,6 +203,27 @@ struct wled {
>>  	int (*wled_ovp_delay)(struct wled *wled);
>>  };
>> 
>> +enum wled5_mod_sel {
>> +	MOD_A,
>> +	MOD_B,
>> +	MOD_MAX,
>> +};
>> +
>> +static const u8 wled5_brightness_reg[MOD_MAX] = {
>> +	[MOD_A] = WLED5_SINK_REG_MOD_A_BRIGHTNESS_LSB,
>> +	[MOD_B] = WLED5_SINK_REG_MOD_B_BRIGHTNESS_LSB,
>> +};
>> +
>> +static const u8 wled5_src_sel_reg[MOD_MAX] = {
>> +	[MOD_A] = WLED5_SINK_REG_MOD_A_SRC_SEL,
>> +	[MOD_B] = WLED5_SINK_REG_MOD_B_SRC_SEL,
>> +};
>> +
>> +static const u8 wled5_brt_wid_sel_reg[MOD_MAX] = {
>> +	[MOD_A] = WLED5_SINK_REG_MOD_A_BRIGHTNESS_WIDTH_SEL,
>> +	[MOD_B] = WLED5_SINK_REG_MOD_B_BRIGHTNESS_WIDTH_SEL,
>> +};
>> +
>>  static int wled3_set_brightness(struct wled *wled, u16 brightness)
>>  {
>>  	int rc, i;
>> @@ -182,6 +242,25 @@ static int wled3_set_brightness(struct wled 
>> *wled, u16 brightness)
>>  	return 0;
>>  }
>> 
>> +static int wled5_set_brightness(struct wled *wled, u16 brightness)
>> +{
>> +	int rc, offset;
>> +	u16 low_limit = wled->max_brightness * 1 / 1000;
>> +	u8 v[2];
>> +
>> +	/* WLED5's lower limit is 0.1% */
>> +	if (brightness < low_limit)
>> +		brightness = low_limit;
>> +
>> +	v[0] = brightness & 0xff;
>> +	v[1] = (brightness >> 8) & 0x7f;
>> +
>> +	offset = wled5_brightness_reg[wled->cfg.mod_sel];
>> +	rc = regmap_bulk_write(wled->regmap, wled->sink_addr + offset,
>> +			       v, 2);
>> +	return rc;
>> +}
>> +
> 
> Can we keep the same ordering throughout the file (wled3, wled4, 
> wled5)?
> Most of the wled5 callbacks seem to have been inserted above wled4.
> 
Sure. Will do it in next post.
> 
> Daniel.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH V3 1/4] backlight: qcom-wled: convert the wled bindings to .yaml format
  2020-03-10 15:01   ` Daniel Thompson
@ 2020-03-11  7:00     ` kgunda
  0 siblings, 0 replies; 15+ messages in thread
From: kgunda @ 2020-03-11  7:00 UTC (permalink / raw)
  To: Daniel Thompson
  Cc: bjorn.andersson, jingoohan1, lee.jones, b.zolnierkie, dri-devel,
	jacek.anaszewski, pavel, robh+dt, mark.rutland, linux-leds,
	devicetree, linux-kernel, Dan Murphy, linux-arm-msm,
	linux-arm-msm-owner

On 2020-03-10 20:31, Daniel Thompson wrote:
> On Mon, Mar 09, 2020 at 06:55:59PM +0530, Kiran Gunda wrote:
>> Convert the qcom-wled bindings from .txt to .yaml format.
>> 
>> Signed-off-by: Kiran Gunda <kgunda@codeaurora.org>
> 
> Acked-by: Daniel Thompson <daniel.thompson@linaro.org>
> 
Thanks.
> 
>> ---
>>  .../bindings/leds/backlight/qcom-wled.txt          | 154 
>> -----------------
>>  .../bindings/leds/backlight/qcom-wled.yaml         | 184 
>> +++++++++++++++++++++
>>  2 files changed, 184 insertions(+), 154 deletions(-)
>>  delete mode 100644 
>> Documentation/devicetree/bindings/leds/backlight/qcom-wled.txt
>>  create mode 100644 
>> Documentation/devicetree/bindings/leds/backlight/qcom-wled.yaml
>> 
>> diff --git 
>> a/Documentation/devicetree/bindings/leds/backlight/qcom-wled.txt 
>> b/Documentation/devicetree/bindings/leds/backlight/qcom-wled.txt
>> deleted file mode 100644
>> index c06863b..0000000
>> --- a/Documentation/devicetree/bindings/leds/backlight/qcom-wled.txt
>> +++ /dev/null
>> @@ -1,154 +0,0 @@
>> -Binding for Qualcomm Technologies, Inc. WLED driver
>> -
>> -WLED (White Light Emitting Diode) driver is used for controlling 
>> display
>> -backlight that is part of PMIC on Qualcomm Technologies, Inc. 
>> reference
>> -platforms. The PMIC is connected to the host processor via SPMI bus.
>> -
>> -- compatible
>> -	Usage:        required
>> -	Value type:   <string>
>> -	Definition:   should be one of:
>> -			"qcom,pm8941-wled"
>> -			"qcom,pmi8998-wled"
>> -			"qcom,pm660l-wled"
>> -
>> -- reg
>> -	Usage:        required
>> -	Value type:   <prop encoded array>
>> -	Definition:   Base address of the WLED modules.
>> -
>> -- default-brightness
>> -	Usage:        optional
>> -	Value type:   <u32>
>> -	Definition:   brightness value on boot, value from: 0-4095.
>> -		      Default: 2048
>> -
>> -- label
>> -	Usage:        required
>> -	Value type:   <string>
>> -	Definition:   The name of the backlight device
>> -
>> -- qcom,cs-out
>> -	Usage:        optional
>> -	Value type:   <bool>
>> -	Definition:   enable current sink output.
>> -		      This property is supported only for PM8941.
>> -
>> -- qcom,cabc
>> -	Usage:        optional
>> -	Value type:   <bool>
>> -	Definition:   enable content adaptive backlight control.
>> -
>> -- qcom,ext-gen
>> -	Usage:        optional
>> -	Value type:   <bool>
>> -	Definition:   use externally generated modulator signal to dim.
>> -		      This property is supported only for PM8941.
>> -
>> -- qcom,current-limit
>> -	Usage:        optional
>> -	Value type:   <u32>
>> -	Definition:   mA; per-string current limit; value from 0 to 25 with
>> -		      1 mA step. Default 20 mA.
>> -		      This property is supported only for pm8941.
>> -
>> -- qcom,current-limit-microamp
>> -	Usage:        optional
>> -	Value type:   <u32>
>> -	Definition:   uA; per-string current limit; value from 0 to 30000 
>> with
>> -		      2500 uA step. Default 25 mA.
>> -
>> -- qcom,current-boost-limit
>> -	Usage:        optional
>> -	Value type:   <u32>
>> -	Definition:   mA; boost current limit.
>> -		      For pm8941: one of: 105, 385, 525, 805, 980, 1260, 1400,
>> -		      1680. Default: 805 mA.
>> -		      For pmi8998: one of: 105, 280, 450, 620, 970, 1150, 1300,
>> -		      1500. Default: 970 mA.
>> -
>> -- qcom,switching-freq
>> -	Usage:        optional
>> -	Value type:   <u32>
>> -	 Definition:   kHz; switching frequency; one of: 600, 640, 685, 738,
>> -		       800, 872, 960, 1066, 1200, 1371, 1600, 1920, 2400, 3200,
>> -		       4800, 9600.
>> -		       Default: for pm8941: 1600 kHz
>> -				for pmi8998: 800 kHz
>> -
>> -- qcom,ovp
>> -	Usage:        optional
>> -	Value type:   <u32>
>> -	Definition:   V; Over-voltage protection limit; one of:
>> -		      27, 29, 32, 35. Default: 29V
>> -		      This property is supported only for PM8941.
>> -
>> -- qcom,ovp-millivolt
>> -	Usage:        optional
>> -	Value type:   <u32>
>> -	Definition:   mV; Over-voltage protection limit;
>> -		      For pmi8998: one of 18100, 19600, 29600, 31100.
>> -		      Default 29600 mV.
>> -		      If this property is not specified for PM8941, it
>> -		      falls back to "qcom,ovp" property.
>> -
>> -- qcom,num-strings
>> -	Usage:        optional
>> -	Value type:   <u32>
>> -	Definition:   #; number of led strings attached;
>> -		      value: For PM8941 from 1 to 3. Default: 2
>> -			     For PMI8998 from 1 to 4.
>> -
>> -- interrupts
>> -	Usage:        optional
>> -	Value type:   <prop encoded array>
>> -	Definition:   Interrupts associated with WLED. This should be
>> -		      "short" and "ovp" interrupts. Interrupts can be
>> -		      specified as per the encoding listed under
>> -		      Documentation/devicetree/bindings/spmi/
>> -		      qcom,spmi-pmic-arb.txt.
>> -
>> -- interrupt-names
>> -	Usage:        optional
>> -	Value type:   <string>
>> -	Definition:   Interrupt names associated with the interrupts.
>> -		      Must be "short" and "ovp". The short circuit detection
>> -		      is not supported for PM8941.
>> -
>> -- qcom,enabled-strings
>> -	Usage:        optional
>> -	Value tyoe:   <u32 array>
>> -	Definition:   Array of the WLED strings numbered from 0 to 3. Each
>> -		      string of leds are operated individually. Specify the
>> -		      list of strings used by the device. Any combination of
>> -		      led strings can be used.
>> -
>> -- qcom,external-pfet
>> -	Usage:        optional
>> -	Value type:   <bool>
>> -	Definition:   Specify if external PFET control for short circuit
>> -		      protection is used. This property is supported only
>> -		      for PMI8998.
>> -
>> -- qcom,auto-string-detection
>> -	Usage:        optional
>> -	Value type:   <bool>
>> -	Definition:   Enables auto-detection of the WLED string 
>> configuration.
>> -		      This feature is not supported for PM8941.
>> -
>> -
>> -Example:
>> -
>> -pm8941-wled@d800 {
>> -	compatible = "qcom,pm8941-wled";
>> -	reg = <0xd800>;
>> -	label = "backlight";
>> -
>> -	qcom,cs-out;
>> -	qcom,current-limit = <20>;
>> -	qcom,current-boost-limit = <805>;
>> -	qcom,switching-freq = <1600>;
>> -	qcom,ovp = <29>;
>> -	qcom,num-strings = <2>;
>> -	qcom,enabled-strings = <0 1>;
>> -};
>> diff --git 
>> a/Documentation/devicetree/bindings/leds/backlight/qcom-wled.yaml 
>> b/Documentation/devicetree/bindings/leds/backlight/qcom-wled.yaml
>> new file mode 100644
>> index 0000000..d334f81
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/leds/backlight/qcom-wled.yaml
>> @@ -0,0 +1,184 @@
>> +# SPDX-License-Identifier: GPL-2.0-only
>> +%YAML 1.2
>> +---
>> +$id: 
>> http://devicetree.org/schemas/bindings/leds/backlight/qcom-wled.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Binding for Qualcomm Technologies, Inc. WLED driver
>> +
>> +maintainers:
>> +  - Lee Jones <lee.jones@linaro.org>
>> +
>> +description: |
>> +  WLED (White Light Emitting Diode) driver is used for controlling 
>> display
>> +  backlight that is part of PMIC on Qualcomm Technologies, Inc. 
>> reference
>> +  platforms. The PMIC is connected to the host processor via SPMI 
>> bus.
>> +
>> +properties:
>> +  compatible :
>> +    enum:
>> +       - qcom,pm8941-wled
>> +       - qcom,pmi8998-wled
>> +       - qcom,pm660l-wled
>> +
>> +  reg:
>> +    maxItems: 1
>> +
>> +  default-brightness:
>> +    maxItems: 1
>> +    description:
>> +      brightness value on boot, value from 0-4095.
>> +    allOf:
>> +      - $ref: /schemas/types.yaml#/definitions/uint32
>> +        default: 2048
>> +
>> +  label:
>> +    maxItems: 1
>> +    description:
>> +      The name of the backlight device.
>> +    allOf:
>> +      - $ref: /schemas/types.yaml#/definitions/string
>> +
>> +  qcom,cs-out:
>> +    description:
>> +      enable current sink output.
>> +      This property is supported only for PM8941.
>> +    type: boolean
>> +
>> +  qcom,cabc:
>> +    description:
>> +      enable content adaptive backlight control.
>> +    type: boolean
>> +
>> +  qcom,ext-gen:
>> +    description:
>> +      use externally generated modulator signal to dim.
>> +      This property is supported only for PM8941.
>> +    type: boolean
>> +
>> +  qcom,current-limit:
>> +    maxItems: 1
>> +    allOf:
>> +      - $ref: /schemas/types.yaml#/definitions/uint32
>> +    description:
>> +      mA; per-string current limit; value from 0 to 25 with
>> +      1 mA step. This property is supported only for pm8941.
>> +    default: 20
>> +
>> +  qcom,current-limit-microamp:
>> +    maxItems: 1
>> +    allOf:
>> +      - $ref: /schemas/types.yaml#/definitions/uint32
>> +    description:
>> +      uA; per-string current limit; value from 0 to 30000 with
>> +      2500 uA step.
>> +    default: 25
>> +
>> +  qcom,current-boost-limit:
>> +    maxItems: 1
>> +    allOf:
>> +      - $ref: /schemas/types.yaml#/definitions/uint32
>> +    description:
>> +      mA; boost current limit.
>> +      For pm8941 one of 105, 385, 525, 805, 980, 1260, 1400, 1680.
>> +      Default, 805 mA.
>> +      For pmi8998 one of 105, 280, 450, 620, 970, 1150, 1300,
>> +      1500. Default 970 mA.
>> +
>> +  qcom,switching-freq:
>> +    maxItems: 1
>> +    allOf:
>> +      - $ref: /schemas/types.yaml#/definitions/uint32
>> +    description:
>> +      kHz; switching frequency; one of 600, 640, 685, 738,
>> +      800, 872, 960, 1066, 1200, 1371, 1600, 1920, 2400, 3200,
>> +      4800, 9600.
>> +      Default for pm8941 1600 kHz
>> +               for pmi8998 800 kHz
>> +
>> +  qcom,ovp:
>> +    maxItems: 1
>> +    allOf:
>> +      - $ref: /schemas/types.yaml#/definitions/uint32
>> +    description:
>> +      V; Over-voltage protection limit; one of 27, 29, 32, 35. 
>> Default 29V
>> +      This property is supported only for PM8941.
>> +
>> +  qcom,ovp-millivolt:
>> +    maxItems: 1
>> +    allOf:
>> +      - $ref: /schemas/types.yaml#/definitions/uint32
>> +    description:
>> +      mV; Over-voltage protection limit;
>> +      For pmi8998 one of 18100, 19600, 29600, 31100.
>> +      Default 29600 mV.
>> +      If this property is not specified for PM8941, it
>> +      falls back to "qcom,ovp" property.
>> +
>> +  qcom,num-strings:
>> +    maxItems: 1
>> +    allOf:
>> +      - $ref: /schemas/types.yaml#/definitions/uint32
>> +    description:
>> +      number of led strings attached;
>> +      value for PM8941 from 1 to 3. Default 2
>> +      For PMI8998 from 1 to 4.
>> +
>> +  interrupts:
>> +    maxItems: 2
>> +    description:
>> +      Interrupts associated with WLED. This should be
>> +      "short" and "ovp" interrupts. Interrupts can be
>> +      specified as per the encoding listed under
>> +      Documentation/devicetree/bindings/spmi/
>> +      qcom,spmi-pmic-arb.txt.
>> +
>> +  interrupt-names:
>> +    description:
>> +      Interrupt names associated with the interrupts.
>> +      Must be "short" and "ovp". The short circuit detection
>> +      is not supported for PM8941.
>> +
>> +  qcom,enabled-strings:
>> +    maxItems: 1
>> +    allOf:
>> +      - $ref: /schemas/types.yaml#/definitions/uint32-array
>> +    description:
>> +      Array of the WLED strings numbered from 0 to 3. Each
>> +      string of leds are operated individually. Specify the
>> +      list of strings used by the device. Any combination of
>> +      led strings can be used.
>> +
>> +  qcom,external-pfet:
>> +    description:
>> +      Specify if external PFET control for short circuit
>> +      protection is used. This property is supported only
>> +      for PMI8998.
>> +    type: boolean
>> +
>> +  qcom,auto-string-detection:
>> +    description:
>> +      Enables auto-detection of the WLED string configuration.
>> +      This feature is not supported for PM8941.
>> +    type: boolean
>> +
>> +required:
>> +  - compatible
>> +  - reg
>> +  - label
>> +
>> +examples:
>> +  - |
>> +    pm8941-wled@d800 {
>> +        compatible = "qcom,pm8941-wled";
>> +        reg = <0xd800 0x100>;
>> +        label = "backlight";
>> +
>> +        qcom,cs-out;
>> +        qcom,current-limit = <20>;
>> +        qcom,current-boost-limit = <805>;
>> +        qcom,switching-freq = <1600>;
>> +        qcom,ovp = <29>;
>> +        qcom,num-strings = <2>;
>> +        qcom,enabled-strings = <0 1>;
>> +     };
>> --
>> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora 
>> Forum,
>>  a Linux Foundation Collaborative Project

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH V3 1/4] backlight: qcom-wled: convert the wled bindings to .yaml format
  2020-03-10 18:31   ` Rob Herring
@ 2020-03-11  7:03     ` kgunda
  0 siblings, 0 replies; 15+ messages in thread
From: kgunda @ 2020-03-11  7:03 UTC (permalink / raw)
  To: Rob Herring
  Cc: bjorn.andersson, jingoohan1, lee.jones, b.zolnierkie, dri-devel,
	daniel.thompson, jacek.anaszewski, pavel, robh+dt, mark.rutland,
	linux-leds, devicetree, linux-kernel, Dan Murphy, linux-arm-msm,
	linux-arm-msm-owner

On 2020-03-11 00:01, Rob Herring wrote:
> On Mon,  9 Mar 2020 18:55:59 +0530, Kiran Gunda wrote:
>> Convert the qcom-wled bindings from .txt to .yaml format.
>> 
>> Signed-off-by: Kiran Gunda <kgunda@codeaurora.org>
>> ---
>>  .../bindings/leds/backlight/qcom-wled.txt          | 154 
>> -----------------
>>  .../bindings/leds/backlight/qcom-wled.yaml         | 184 
>> +++++++++++++++++++++
>>  2 files changed, 184 insertions(+), 154 deletions(-)
>>  delete mode 100644 
>> Documentation/devicetree/bindings/leds/backlight/qcom-wled.txt
>>  create mode 100644 
>> Documentation/devicetree/bindings/leds/backlight/qcom-wled.yaml
>> 
> 
> My bot found errors running 'make dt_binding_check' on your patch:
> 
> Documentation/devicetree/bindings/leds/backlight/qcom-wled.yaml: $id:
> relative path/filename doesn't match actual path or filename
> 	expected: http://devicetree.org/schemas/leds/backlight/qcom-wled.yaml#
> 
> See https://patchwork.ozlabs.org/patch/1251567
> Please check and re-submit.
I will fix it in next post.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH V3 2/4] backlight: qcom-wled: Add callback functions
  2020-03-11  6:41     ` kgunda
@ 2020-03-11 10:30       ` Daniel Thompson
  2020-03-23 15:36         ` kgunda
  0 siblings, 1 reply; 15+ messages in thread
From: Daniel Thompson @ 2020-03-11 10:30 UTC (permalink / raw)
  To: kgunda
  Cc: bjorn.andersson, jingoohan1, lee.jones, b.zolnierkie, dri-devel,
	jacek.anaszewski, pavel, robh+dt, mark.rutland, linux-leds,
	devicetree, linux-kernel, Andy Gross, linux-arm-msm, linux-fbdev,
	linux-arm-msm-owner

On Wed, Mar 11, 2020 at 12:11:00PM +0530, kgunda@codeaurora.org wrote:
> On 2020-03-10 20:57, Daniel Thompson wrote:
> > On Mon, Mar 09, 2020 at 06:56:00PM +0530, Kiran Gunda wrote:
> > > Add cabc_config, sync_toggle, wled_ovp_fault_status and wled_ovp_delay
> > > callback functions to prepare the driver for adding WLED5 support.
> > > 
> > > Signed-off-by: Kiran Gunda <kgunda@codeaurora.org>
> > 
> > Overall this code would a lot easier to review if
> > > ---
> > >  drivers/video/backlight/qcom-wled.c | 196
> > > +++++++++++++++++++++++-------------
> > >  1 file changed, 126 insertions(+), 70 deletions(-)
> > > 
> > > diff --git a/drivers/video/backlight/qcom-wled.c
> > > b/drivers/video/backlight/qcom-wled.c
> > > index 3d276b3..b73f273 100644
> > > --- a/drivers/video/backlight/qcom-wled.c
> > > +++ b/drivers/video/backlight/qcom-wled.c
> > > @@ -128,6 +128,7 @@ struct wled_config {
> > >  	bool cs_out_en;
> > >  	bool ext_gen;
> > >  	bool cabc;
> > > +	bool en_cabc;
> > 
> > Does this ever get set to true?
> > 
> Yes. If user wants use the cabc pin to control the brightness and
> use the "qcom,cabc" DT property in the device tree.

That sounds like what you intended the code to do!

Is the code that does this present in the patch? I could not find
it.


Daniel.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH V3 2/4] backlight: qcom-wled: Add callback functions
  2020-03-11 10:30       ` Daniel Thompson
@ 2020-03-23 15:36         ` kgunda
  0 siblings, 0 replies; 15+ messages in thread
From: kgunda @ 2020-03-23 15:36 UTC (permalink / raw)
  To: Daniel Thompson
  Cc: bjorn.andersson, jingoohan1, lee.jones, b.zolnierkie, dri-devel,
	jacek.anaszewski, pavel, robh+dt, mark.rutland, linux-leds,
	devicetree, linux-kernel, Andy Gross, linux-arm-msm, linux-fbdev,
	linux-arm-msm-owner

On 2020-03-11 16:00, Daniel Thompson wrote:
> On Wed, Mar 11, 2020 at 12:11:00PM +0530, kgunda@codeaurora.org wrote:
>> On 2020-03-10 20:57, Daniel Thompson wrote:
>> > On Mon, Mar 09, 2020 at 06:56:00PM +0530, Kiran Gunda wrote:
>> > > Add cabc_config, sync_toggle, wled_ovp_fault_status and wled_ovp_delay
>> > > callback functions to prepare the driver for adding WLED5 support.
>> > >
>> > > Signed-off-by: Kiran Gunda <kgunda@codeaurora.org>
>> >
>> > Overall this code would a lot easier to review if
>> > > ---
>> > >  drivers/video/backlight/qcom-wled.c | 196
>> > > +++++++++++++++++++++++-------------
>> > >  1 file changed, 126 insertions(+), 70 deletions(-)
>> > >
>> > > diff --git a/drivers/video/backlight/qcom-wled.c
>> > > b/drivers/video/backlight/qcom-wled.c
>> > > index 3d276b3..b73f273 100644
>> > > --- a/drivers/video/backlight/qcom-wled.c
>> > > +++ b/drivers/video/backlight/qcom-wled.c
>> > > @@ -128,6 +128,7 @@ struct wled_config {
>> > >  	bool cs_out_en;
>> > >  	bool ext_gen;
>> > >  	bool cabc;
>> > > +	bool en_cabc;
>> >
>> > Does this ever get set to true?
>> >
>> Yes. If user wants use the cabc pin to control the brightness and
>> use the "qcom,cabc" DT property in the device tree.
> 
> That sounds like what you intended the code to do!
> 
> Is the code that does this present in the patch? I could not find
> it.
> 
okay... It's my bad. We already have the "cabc" for this. I will remove 
the en_cabc in
next series.
> 
> Daniel.

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2020-03-23 15:36 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-09 13:25 [PATCH V3 0/4] Add support for WLED5 Kiran Gunda
2020-03-09 13:25 ` [PATCH V3 1/4] backlight: qcom-wled: convert the wled bindings to .yaml format Kiran Gunda
2020-03-10 15:01   ` Daniel Thompson
2020-03-11  7:00     ` kgunda
2020-03-10 18:31   ` Rob Herring
2020-03-11  7:03     ` kgunda
2020-03-09 13:26 ` [PATCH V3 2/4] backlight: qcom-wled: Add callback functions Kiran Gunda
2020-03-10 15:27   ` Daniel Thompson
2020-03-11  6:41     ` kgunda
2020-03-11 10:30       ` Daniel Thompson
2020-03-23 15:36         ` kgunda
2020-03-09 13:26 ` [PATCH V3 3/4] backlight: qcom-wled: Add support for WLED5 peripheral in PM8150L Kiran Gunda
2020-03-10 15:45   ` Daniel Thompson
2020-03-11  7:00     ` kgunda
2020-03-09 13:26 ` [PATCH V3 4/4] backlight: qcom-wled: Update auto calibration support for WLED5 Kiran Gunda

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).