Re: [PATCH V1 2/4] qcom: spmi-wled: Add support for short circuit handling

[kgunda]

On 2017-12-05 10:05, Bjorn Andersson wrote:

On Thu 16 Nov 04:18 PST 2017, Kiran Gunda wrote:


Handle the short circuit(SC) interrupt and check if the SC interrupt
is valid. Re-enable the module to check if it goes away. Disable the
module altogether if the SC event persists.

Signed-off-by: Kiran Gunda 
---
 .../bindings/leds/backlight/qcom-spmi-wled.txt |  22 
 drivers/video/backlight/qcom-spmi-wled.c   | 126 
-

 2 files changed, 142 insertions(+), 6 deletions(-)

diff --git 
a/Documentation/devicetree/bindings/leds/backlight/qcom-spmi-wled.txt 
b/Documentation/devicetree/bindings/leds/backlight/qcom-spmi-wled.txt

index f1ea25b..768608c 100644
--- 
a/Documentation/devicetree/bindings/leds/backlight/qcom-spmi-wled.txt
+++ 
b/Documentation/devicetree/bindings/leds/backlight/qcom-spmi-wled.txt
@@ -74,6 +74,26 @@ The PMIC is connected to the host processor via 
SPMI bus.

Definition: Specify if cabc (content adaptive backlight control) is
needed.

+- qcom,ext-pfet-sc-pro-en


Please use readable names, rather than a bunch of abbreviations.


Ok. Will address it in next series.

+   Usage:  optional
+   Value type: 
+   Definition: Specify if external PFET control for short circuit
+   protection is needed.


What does this mean? At least change the wording to "...protection is
used".


Ok. Will address it in next series.

+
+- interrupts
+   Usage:  optional
+   Value type: 
+   Definition: Interrupts associated with WLED. 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: 
+   Definition: Interrupt names associated with the interrupts.
+   Must be "sc-irq".


This is obviously an irq, so no need to include that in the name. I
would also prefer if you use the name "short" to make this easier to
read.


Ok. Will address it in next series.

+
 Example:

 qcom-wled@d800 {
@@ -82,6 +102,8 @@ qcom-wled@d800 {
reg-names = "qcom-wled-ctrl-base", "qcom-wled-sink-base";
label = "backlight";

+   interrupts = <0x3 0xd8 0x2 IRQ_TYPE_EDGE_RISING>;


We tend to write these on the form , 
please

follow this.


Ok. Will address it in next series.

+   interrupt-names = "sc-irq";
qcom,fs-current-limit = <25000>;
qcom,current-boost-limit = <970>;
qcom,switching-freq = <800>;
diff --git a/drivers/video/backlight/qcom-spmi-wled.c 
b/drivers/video/backlight/qcom-spmi-wled.c

index 14c3adc..7dbaaa7 100644
--- a/drivers/video/backlight/qcom-spmi-wled.c
+++ b/drivers/video/backlight/qcom-spmi-wled.c
@@ -11,6 +11,9 @@
  * GNU General Public License for more details.
  */

+#include 
+#include 
+#include 
 #include 
 #include 
 #include 
@@ -23,7 +26,13 @@
 #define QCOM_WLED_DEFAULT_BRIGHTNESS   2048
 #define  QCOM_WLED_MAX_BRIGHTNESS  4095

+#define QCOM_WLED_SC_DLY_MS20
+#define QCOM_WLED_SC_CNT_MAX   5
+#define QCOM_WLED_SC_RESET_CNT_DLY_US  100


With times of this ballpark you can just use jiffies, with this just
being HZ.


Ok. Will address it in next series.

+
 /* WLED control registers */
+#define QCOM_WLED_CTRL_FAULT_STATUS0x08
+
 #define QCOM_WLED_CTRL_MOD_ENABLE  0x46
 #define  QCOM_WLED_CTRL_MOD_EN_MASKBIT(7)
 #define  QCOM_WLED_CTRL_MODULE_EN_SHIFT7
@@ -37,6 +46,15 @@
 #define QCOM_WLED_CTRL_ILIM0x4e
 #define  QCOM_WLED_CTRL_ILIM_MASK  GENMASK(2, 0)

+#define QCOM_WLED_CTRL_SHORT_PROTECT   0x5e
+#define  QCOM_WLED_CTRL_SHORT_EN_MASK  BIT(7)
+
+#define QCOM_WLED_CTRL_SEC_ACCESS  0xd0
+#define  QCOM_WLED_CTRL_SEC_UNLOCK 0xa5
+
+#define QCOM_WLED_CTRL_TEST1   0xe2
+#define  QCOM_WLED_EXT_FET_DTEST2  0x09
+
 /* WLED sink registers */
 #define QCOM_WLED_SINK_CURR_SINK_EN0x46
 #define  QCOM_WLED_SINK_CURR_SINK_MASK GENMASK(7, 4)
@@ -71,19 +89,23 @@ struct qcom_wled_config {
u32 switch_freq;
u32 fs_current;
u32 string_cfg;
+   int sc_irq;


Keep data parsed directly from DT in the config and move this to
qcom_wled.


Ok. Will address it in next series.

bool en_cabc;
+   bool ext_pfet_sc_pro_en;


This name is long and hard to parse. "external_pfet" would be much
easier to read.


Ok. Will address it in next series.

 };

 struct qcom_wled {
const char *name;
struct platform_device *pdev;
struct regmap *regmap;
+   struct mutex lock;
+   struct qcom_wled_config cfg;
+   ktime_t last_sc_event_time;
u16 sink_addr;
u16 ctrl_addr;
u32 brightness;
+   u32 

Re: [PATCH V1 2/4] qcom: spmi-wled: Add support for short circuit handling

[kgunda]

On 2017-12-05 10:05, Bjorn Andersson wrote:

On Thu 16 Nov 04:18 PST 2017, Kiran Gunda wrote:


Handle the short circuit(SC) interrupt and check if the SC interrupt
is valid. Re-enable the module to check if it goes away. Disable the
module altogether if the SC event persists.

Signed-off-by: Kiran Gunda 
---
 .../bindings/leds/backlight/qcom-spmi-wled.txt |  22 
 drivers/video/backlight/qcom-spmi-wled.c   | 126 
-

 2 files changed, 142 insertions(+), 6 deletions(-)

diff --git 
a/Documentation/devicetree/bindings/leds/backlight/qcom-spmi-wled.txt 
b/Documentation/devicetree/bindings/leds/backlight/qcom-spmi-wled.txt

index f1ea25b..768608c 100644
--- 
a/Documentation/devicetree/bindings/leds/backlight/qcom-spmi-wled.txt
+++ 
b/Documentation/devicetree/bindings/leds/backlight/qcom-spmi-wled.txt
@@ -74,6 +74,26 @@ The PMIC is connected to the host processor via 
SPMI bus.

Definition: Specify if cabc (content adaptive backlight control) is
needed.

+- qcom,ext-pfet-sc-pro-en


Please use readable names, rather than a bunch of abbreviations.


Ok. Will address it in next series.

+   Usage:  optional
+   Value type: 
+   Definition: Specify if external PFET control for short circuit
+   protection is needed.


What does this mean? At least change the wording to "...protection is
used".


Ok. Will address it in next series.

+
+- interrupts
+   Usage:  optional
+   Value type: 
+   Definition: Interrupts associated with WLED. 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: 
+   Definition: Interrupt names associated with the interrupts.
+   Must be "sc-irq".


This is obviously an irq, so no need to include that in the name. I
would also prefer if you use the name "short" to make this easier to
read.


Ok. Will address it in next series.

+
 Example:

 qcom-wled@d800 {
@@ -82,6 +102,8 @@ qcom-wled@d800 {
reg-names = "qcom-wled-ctrl-base", "qcom-wled-sink-base";
label = "backlight";

+   interrupts = <0x3 0xd8 0x2 IRQ_TYPE_EDGE_RISING>;


We tend to write these on the form , 
please

follow this.


Ok. Will address it in next series.

+   interrupt-names = "sc-irq";
qcom,fs-current-limit = <25000>;
qcom,current-boost-limit = <970>;
qcom,switching-freq = <800>;
diff --git a/drivers/video/backlight/qcom-spmi-wled.c 
b/drivers/video/backlight/qcom-spmi-wled.c

index 14c3adc..7dbaaa7 100644
--- a/drivers/video/backlight/qcom-spmi-wled.c
+++ b/drivers/video/backlight/qcom-spmi-wled.c
@@ -11,6 +11,9 @@
  * GNU General Public License for more details.
  */

+#include 
+#include 
+#include 
 #include 
 #include 
 #include 
@@ -23,7 +26,13 @@
 #define QCOM_WLED_DEFAULT_BRIGHTNESS   2048
 #define  QCOM_WLED_MAX_BRIGHTNESS  4095

+#define QCOM_WLED_SC_DLY_MS20
+#define QCOM_WLED_SC_CNT_MAX   5
+#define QCOM_WLED_SC_RESET_CNT_DLY_US  100


With times of this ballpark you can just use jiffies, with this just
being HZ.


Ok. Will address it in next series.

+
 /* WLED control registers */
+#define QCOM_WLED_CTRL_FAULT_STATUS0x08
+
 #define QCOM_WLED_CTRL_MOD_ENABLE  0x46
 #define  QCOM_WLED_CTRL_MOD_EN_MASKBIT(7)
 #define  QCOM_WLED_CTRL_MODULE_EN_SHIFT7
@@ -37,6 +46,15 @@
 #define QCOM_WLED_CTRL_ILIM0x4e
 #define  QCOM_WLED_CTRL_ILIM_MASK  GENMASK(2, 0)

+#define QCOM_WLED_CTRL_SHORT_PROTECT   0x5e
+#define  QCOM_WLED_CTRL_SHORT_EN_MASK  BIT(7)
+
+#define QCOM_WLED_CTRL_SEC_ACCESS  0xd0
+#define  QCOM_WLED_CTRL_SEC_UNLOCK 0xa5
+
+#define QCOM_WLED_CTRL_TEST1   0xe2
+#define  QCOM_WLED_EXT_FET_DTEST2  0x09
+
 /* WLED sink registers */
 #define QCOM_WLED_SINK_CURR_SINK_EN0x46
 #define  QCOM_WLED_SINK_CURR_SINK_MASK GENMASK(7, 4)
@@ -71,19 +89,23 @@ struct qcom_wled_config {
u32 switch_freq;
u32 fs_current;
u32 string_cfg;
+   int sc_irq;


Keep data parsed directly from DT in the config and move this to
qcom_wled.


Ok. Will address it in next series.

bool en_cabc;
+   bool ext_pfet_sc_pro_en;


This name is long and hard to parse. "external_pfet" would be much
easier to read.


Ok. Will address it in next series.

 };

 struct qcom_wled {
const char *name;
struct platform_device *pdev;
struct regmap *regmap;
+   struct mutex lock;
+   struct qcom_wled_config cfg;
+   ktime_t last_sc_event_time;
u16 sink_addr;
u16 ctrl_addr;
u32 brightness;
+   u32 sc_count;
bool prev_state;
-
-   struct 

Re: [PATCH V1 2/4] qcom: spmi-wled: Add support for short circuit handling

[Bjorn Andersson]

On Thu 16 Nov 04:18 PST 2017, Kiran Gunda wrote:

> Handle the short circuit(SC) interrupt and check if the SC interrupt
> is valid. Re-enable the module to check if it goes away. Disable the
> module altogether if the SC event persists.
> 
> Signed-off-by: Kiran Gunda 
> ---
>  .../bindings/leds/backlight/qcom-spmi-wled.txt |  22 
>  drivers/video/backlight/qcom-spmi-wled.c   | 126 
> -
>  2 files changed, 142 insertions(+), 6 deletions(-)
> 
> diff --git 
> a/Documentation/devicetree/bindings/leds/backlight/qcom-spmi-wled.txt 
> b/Documentation/devicetree/bindings/leds/backlight/qcom-spmi-wled.txt
> index f1ea25b..768608c 100644
> --- a/Documentation/devicetree/bindings/leds/backlight/qcom-spmi-wled.txt
> +++ b/Documentation/devicetree/bindings/leds/backlight/qcom-spmi-wled.txt
> @@ -74,6 +74,26 @@ The PMIC is connected to the host processor via SPMI bus.
>   Definition: Specify if cabc (content adaptive backlight control) is
>   needed.
>  
> +- qcom,ext-pfet-sc-pro-en

Please use readable names, rather than a bunch of abbreviations.

> + Usage:  optional
> + Value type: 
> + Definition: Specify if external PFET control for short circuit
> + protection is needed.

What does this mean? At least change the wording to "...protection is
used".

> +
> +- interrupts
> + Usage:  optional
> + Value type: 
> + Definition: Interrupts associated with WLED. 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: 
> + Definition: Interrupt names associated with the interrupts.
> + Must be "sc-irq".

This is obviously an irq, so no need to include that in the name. I
would also prefer if you use the name "short" to make this easier to
read.

> +
>  Example:
>  
>  qcom-wled@d800 {
> @@ -82,6 +102,8 @@ qcom-wled@d800 {
>   reg-names = "qcom-wled-ctrl-base", "qcom-wled-sink-base";
>   label = "backlight";
>  
> + interrupts = <0x3 0xd8 0x2 IRQ_TYPE_EDGE_RISING>;

We tend to write these on the form , please
follow this.

> + interrupt-names = "sc-irq";
>   qcom,fs-current-limit = <25000>;
>   qcom,current-boost-limit = <970>;
>   qcom,switching-freq = <800>;
> diff --git a/drivers/video/backlight/qcom-spmi-wled.c 
> b/drivers/video/backlight/qcom-spmi-wled.c
> index 14c3adc..7dbaaa7 100644
> --- a/drivers/video/backlight/qcom-spmi-wled.c
> +++ b/drivers/video/backlight/qcom-spmi-wled.c
> @@ -11,6 +11,9 @@
>   * GNU General Public License for more details.
>   */
>  
> +#include 
> +#include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -23,7 +26,13 @@
>  #define QCOM_WLED_DEFAULT_BRIGHTNESS 2048
>  #define  QCOM_WLED_MAX_BRIGHTNESS4095
>  
> +#define QCOM_WLED_SC_DLY_MS  20
> +#define QCOM_WLED_SC_CNT_MAX 5
> +#define QCOM_WLED_SC_RESET_CNT_DLY_US100

With times of this ballpark you can just use jiffies, with this just
being HZ.

> +
>  /* WLED control registers */
> +#define QCOM_WLED_CTRL_FAULT_STATUS  0x08
> +
>  #define QCOM_WLED_CTRL_MOD_ENABLE0x46
>  #define  QCOM_WLED_CTRL_MOD_EN_MASK  BIT(7)
>  #define  QCOM_WLED_CTRL_MODULE_EN_SHIFT  7
> @@ -37,6 +46,15 @@
>  #define QCOM_WLED_CTRL_ILIM  0x4e
>  #define  QCOM_WLED_CTRL_ILIM_MASKGENMASK(2, 0)
>  
> +#define QCOM_WLED_CTRL_SHORT_PROTECT 0x5e
> +#define  QCOM_WLED_CTRL_SHORT_EN_MASKBIT(7)
> +
> +#define QCOM_WLED_CTRL_SEC_ACCESS0xd0
> +#define  QCOM_WLED_CTRL_SEC_UNLOCK   0xa5
> +
> +#define QCOM_WLED_CTRL_TEST1 0xe2
> +#define  QCOM_WLED_EXT_FET_DTEST20x09
> +
>  /* WLED sink registers */
>  #define QCOM_WLED_SINK_CURR_SINK_EN  0x46
>  #define  QCOM_WLED_SINK_CURR_SINK_MASK   GENMASK(7, 4)
> @@ -71,19 +89,23 @@ struct qcom_wled_config {
>   u32 switch_freq;
>   u32 fs_current;
>   u32 string_cfg;
> + int sc_irq;

Keep data parsed directly from DT in the config and move this to
qcom_wled.

>   bool en_cabc;
> + bool ext_pfet_sc_pro_en;

This name is long and hard to parse. "external_pfet" would be much
easier to read.

>  };
>  
>  struct qcom_wled {
>   const char *name;
>   struct platform_device *pdev;
>   struct regmap *regmap;
> + struct mutex lock;
> + struct qcom_wled_config cfg;
> + ktime_t last_sc_event_time;
>   u16 sink_addr;
>   u16 ctrl_addr;
>   u32 brightness;
> + u32 sc_count;
>   bool prev_state;
> -
> - struct qcom_wled_config cfg;

Moving this seems unnecessary.

>  };
>  
>  static int qcom_wled_module_enable(struct qcom_wled *wled, 

Re: [PATCH V1 2/4] qcom: spmi-wled: Add support for short circuit handling

[Bjorn Andersson]

On Thu 16 Nov 04:18 PST 2017, Kiran Gunda wrote:

> Handle the short circuit(SC) interrupt and check if the SC interrupt
> is valid. Re-enable the module to check if it goes away. Disable the
> module altogether if the SC event persists.
> 
> Signed-off-by: Kiran Gunda 
> ---
>  .../bindings/leds/backlight/qcom-spmi-wled.txt |  22 
>  drivers/video/backlight/qcom-spmi-wled.c   | 126 
> -
>  2 files changed, 142 insertions(+), 6 deletions(-)
> 
> diff --git 
> a/Documentation/devicetree/bindings/leds/backlight/qcom-spmi-wled.txt 
> b/Documentation/devicetree/bindings/leds/backlight/qcom-spmi-wled.txt
> index f1ea25b..768608c 100644
> --- a/Documentation/devicetree/bindings/leds/backlight/qcom-spmi-wled.txt
> +++ b/Documentation/devicetree/bindings/leds/backlight/qcom-spmi-wled.txt
> @@ -74,6 +74,26 @@ The PMIC is connected to the host processor via SPMI bus.
>   Definition: Specify if cabc (content adaptive backlight control) is
>   needed.
>  
> +- qcom,ext-pfet-sc-pro-en

Please use readable names, rather than a bunch of abbreviations.

> + Usage:  optional
> + Value type: 
> + Definition: Specify if external PFET control for short circuit
> + protection is needed.

What does this mean? At least change the wording to "...protection is
used".

> +
> +- interrupts
> + Usage:  optional
> + Value type: 
> + Definition: Interrupts associated with WLED. 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: 
> + Definition: Interrupt names associated with the interrupts.
> + Must be "sc-irq".

This is obviously an irq, so no need to include that in the name. I
would also prefer if you use the name "short" to make this easier to
read.

> +
>  Example:
>  
>  qcom-wled@d800 {
> @@ -82,6 +102,8 @@ qcom-wled@d800 {
>   reg-names = "qcom-wled-ctrl-base", "qcom-wled-sink-base";
>   label = "backlight";
>  
> + interrupts = <0x3 0xd8 0x2 IRQ_TYPE_EDGE_RISING>;

We tend to write these on the form , please
follow this.

> + interrupt-names = "sc-irq";
>   qcom,fs-current-limit = <25000>;
>   qcom,current-boost-limit = <970>;
>   qcom,switching-freq = <800>;
> diff --git a/drivers/video/backlight/qcom-spmi-wled.c 
> b/drivers/video/backlight/qcom-spmi-wled.c
> index 14c3adc..7dbaaa7 100644
> --- a/drivers/video/backlight/qcom-spmi-wled.c
> +++ b/drivers/video/backlight/qcom-spmi-wled.c
> @@ -11,6 +11,9 @@
>   * GNU General Public License for more details.
>   */
>  
> +#include 
> +#include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -23,7 +26,13 @@
>  #define QCOM_WLED_DEFAULT_BRIGHTNESS 2048
>  #define  QCOM_WLED_MAX_BRIGHTNESS4095
>  
> +#define QCOM_WLED_SC_DLY_MS  20
> +#define QCOM_WLED_SC_CNT_MAX 5
> +#define QCOM_WLED_SC_RESET_CNT_DLY_US100

With times of this ballpark you can just use jiffies, with this just
being HZ.

> +
>  /* WLED control registers */
> +#define QCOM_WLED_CTRL_FAULT_STATUS  0x08
> +
>  #define QCOM_WLED_CTRL_MOD_ENABLE0x46
>  #define  QCOM_WLED_CTRL_MOD_EN_MASK  BIT(7)
>  #define  QCOM_WLED_CTRL_MODULE_EN_SHIFT  7
> @@ -37,6 +46,15 @@
>  #define QCOM_WLED_CTRL_ILIM  0x4e
>  #define  QCOM_WLED_CTRL_ILIM_MASKGENMASK(2, 0)
>  
> +#define QCOM_WLED_CTRL_SHORT_PROTECT 0x5e
> +#define  QCOM_WLED_CTRL_SHORT_EN_MASKBIT(7)
> +
> +#define QCOM_WLED_CTRL_SEC_ACCESS0xd0
> +#define  QCOM_WLED_CTRL_SEC_UNLOCK   0xa5
> +
> +#define QCOM_WLED_CTRL_TEST1 0xe2
> +#define  QCOM_WLED_EXT_FET_DTEST20x09
> +
>  /* WLED sink registers */
>  #define QCOM_WLED_SINK_CURR_SINK_EN  0x46
>  #define  QCOM_WLED_SINK_CURR_SINK_MASK   GENMASK(7, 4)
> @@ -71,19 +89,23 @@ struct qcom_wled_config {
>   u32 switch_freq;
>   u32 fs_current;
>   u32 string_cfg;
> + int sc_irq;

Keep data parsed directly from DT in the config and move this to
qcom_wled.

>   bool en_cabc;
> + bool ext_pfet_sc_pro_en;

This name is long and hard to parse. "external_pfet" would be much
easier to read.

>  };
>  
>  struct qcom_wled {
>   const char *name;
>   struct platform_device *pdev;
>   struct regmap *regmap;
> + struct mutex lock;
> + struct qcom_wled_config cfg;
> + ktime_t last_sc_event_time;
>   u16 sink_addr;
>   u16 ctrl_addr;
>   u32 brightness;
> + u32 sc_count;
>   bool prev_state;
> -
> - struct qcom_wled_config cfg;

Moving this seems unnecessary.

>  };
>  
>  static int qcom_wled_module_enable(struct qcom_wled *wled, int val)
> @@ -157,25 +179,26 @@ static int 

Re: [PATCH V1 2/4] qcom: spmi-wled: Add support for short circuit handling

[kgunda]

On 2017-11-18 02:00, Rob Herring wrote:

On Thu, Nov 16, 2017 at 05:48:35PM +0530, Kiran Gunda wrote:

Handle the short circuit(SC) interrupt and check if the SC interrupt
is valid. Re-enable the module to check if it goes away. Disable the
module altogether if the SC event persists.

Signed-off-by: Kiran Gunda 
---
 .../bindings/leds/backlight/qcom-spmi-wled.txt |  22 


... and also put all the binding in one patch. I want to review the 
full

view of the h/w, not piecemeal.


Sure. Will send the next series with the changes suggested.
 drivers/video/backlight/qcom-spmi-wled.c   | 126 
-

 2 files changed, 142 insertions(+), 6 deletions(-)

Re: [PATCH V1 2/4] qcom: spmi-wled: Add support for short circuit handling

[kgunda]

On 2017-11-18 02:00, Rob Herring wrote:

On Thu, Nov 16, 2017 at 05:48:35PM +0530, Kiran Gunda wrote:

Handle the short circuit(SC) interrupt and check if the SC interrupt
is valid. Re-enable the module to check if it goes away. Disable the
module altogether if the SC event persists.

Signed-off-by: Kiran Gunda 
---
 .../bindings/leds/backlight/qcom-spmi-wled.txt |  22 


... and also put all the binding in one patch. I want to review the 
full

view of the h/w, not piecemeal.


Sure. Will send the next series with the changes suggested.
 drivers/video/backlight/qcom-spmi-wled.c   | 126 
-

 2 files changed, 142 insertions(+), 6 deletions(-)

Re: [PATCH V1 2/4] qcom: spmi-wled: Add support for short circuit handling

[Rob Herring]

On Thu, Nov 16, 2017 at 05:48:35PM +0530, Kiran Gunda wrote:
> Handle the short circuit(SC) interrupt and check if the SC interrupt
> is valid. Re-enable the module to check if it goes away. Disable the
> module altogether if the SC event persists.
> 
> Signed-off-by: Kiran Gunda 
> ---
>  .../bindings/leds/backlight/qcom-spmi-wled.txt |  22 

... and also put all the binding in one patch. I want to review the full 
view of the h/w, not piecemeal.

>  drivers/video/backlight/qcom-spmi-wled.c   | 126 
> -
>  2 files changed, 142 insertions(+), 6 deletions(-)

Re: [PATCH V1 2/4] qcom: spmi-wled: Add support for short circuit handling

[Rob Herring]

On Thu, Nov 16, 2017 at 05:48:35PM +0530, Kiran Gunda wrote:
> Handle the short circuit(SC) interrupt and check if the SC interrupt
> is valid. Re-enable the module to check if it goes away. Disable the
> module altogether if the SC event persists.
> 
> Signed-off-by: Kiran Gunda 
> ---
>  .../bindings/leds/backlight/qcom-spmi-wled.txt |  22 

... and also put all the binding in one patch. I want to review the full 
view of the h/w, not piecemeal.

>  drivers/video/backlight/qcom-spmi-wled.c   | 126 
> -
>  2 files changed, 142 insertions(+), 6 deletions(-)