linux-leds.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/2] backlight_lm3630a: add enable_gpios property
@ 2019-09-11 17:21 Andreas Kemnade
  2019-09-11 17:21 ` [PATCH v3 1/2] dt-bindings: backlight: lm3630a: add enable_gpios Andreas Kemnade
  2019-09-11 17:21 ` [PATCH v3 2/2] backlight: lm3630a: add an enable gpio for the HWEN pin Andreas Kemnade
  0 siblings, 2 replies; 8+ messages in thread
From: Andreas Kemnade @ 2019-09-11 17:21 UTC (permalink / raw)
  To: lee.jones, daniel.thompson, jingoohan1, jacek.anaszewski, pavel,
	dmurphy, robh+dt, mark.rutland, b.zolnierkie, dri-devel,
	linux-leds, devicetree, linux-kernel, linux-fbdev,
	H. Nikolaus Schaller
  Cc: Andreas Kemnade

To be able to handle the HWEN pin of the lm3630a, add
an enable gpio to the driver and a property.

Tested on Kobo Clara HD.

Changes in v2:
simplification and reordering

Changes in v3:
added acked-by
removed legacy include

Andreas Kemnade (2):
  dt-bindings: backlight: lm3630a: add enable_gpios
  backlight: lm3630a: add an enable gpio for the HWEN pin

 .../bindings/leds/backlight/lm3630a-backlight.yaml       | 5 +++++
 drivers/video/backlight/lm3630a_bl.c                     | 9 +++++++++
 2 files changed, 14 insertions(+)

-- 
2.20.1


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

* [PATCH v3 1/2] dt-bindings: backlight: lm3630a: add enable_gpios
  2019-09-11 17:21 [PATCH v3 0/2] backlight_lm3630a: add enable_gpios property Andreas Kemnade
@ 2019-09-11 17:21 ` Andreas Kemnade
  2019-09-12 11:39   ` Dan Murphy
  2019-09-11 17:21 ` [PATCH v3 2/2] backlight: lm3630a: add an enable gpio for the HWEN pin Andreas Kemnade
  1 sibling, 1 reply; 8+ messages in thread
From: Andreas Kemnade @ 2019-09-11 17:21 UTC (permalink / raw)
  To: lee.jones, daniel.thompson, jingoohan1, jacek.anaszewski, pavel,
	dmurphy, robh+dt, mark.rutland, b.zolnierkie, dri-devel,
	linux-leds, devicetree, linux-kernel, linux-fbdev,
	H. Nikolaus Schaller
  Cc: Andreas Kemnade

add enable-gpios to describe HWEN pin

Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
Acked-by: Daniel Thompson <daniel.thompson@linaro.org>
---
changes in v2: added example
changes in v3: added Acked-by
 .../bindings/leds/backlight/lm3630a-backlight.yaml           | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/leds/backlight/lm3630a-backlight.yaml b/Documentation/devicetree/bindings/leds/backlight/lm3630a-backlight.yaml
index dc129d9a329e..1fa83feffe16 100644
--- a/Documentation/devicetree/bindings/leds/backlight/lm3630a-backlight.yaml
+++ b/Documentation/devicetree/bindings/leds/backlight/lm3630a-backlight.yaml
@@ -29,6 +29,10 @@ properties:
   '#size-cells':
     const: 0
 
+  enable-gpios:
+    description: GPIO to use to enable/disable the backlight (HWEN pin).
+    maxItems: 1
+
 required:
   - compatible
   - reg
@@ -92,6 +96,7 @@ examples:
     i2c {
         #address-cells = <1>;
         #size-cells = <0>;
+        enable-gpios = <&gpio2 5 GPIO_ACTIVE_HIGH>;
 
         led-controller@38 {
                 compatible = "ti,lm3630a";
-- 
2.20.1


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

* [PATCH v3 2/2] backlight: lm3630a: add an enable gpio for the HWEN pin
  2019-09-11 17:21 [PATCH v3 0/2] backlight_lm3630a: add enable_gpios property Andreas Kemnade
  2019-09-11 17:21 ` [PATCH v3 1/2] dt-bindings: backlight: lm3630a: add enable_gpios Andreas Kemnade
@ 2019-09-11 17:21 ` Andreas Kemnade
  2019-09-12  9:38   ` Daniel Thompson
  2019-09-12 11:40   ` Dan Murphy
  1 sibling, 2 replies; 8+ messages in thread
From: Andreas Kemnade @ 2019-09-11 17:21 UTC (permalink / raw)
  To: lee.jones, daniel.thompson, jingoohan1, jacek.anaszewski, pavel,
	dmurphy, robh+dt, mark.rutland, b.zolnierkie, dri-devel,
	linux-leds, devicetree, linux-kernel, linux-fbdev,
	H. Nikolaus Schaller
  Cc: Andreas Kemnade

For now just enable it in the probe function to allow i2c
access. Disabling also means resetting the register values
to default and according to the datasheet does not give
power savings.

Tested on Kobo Clara HD.

Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
---
changes in v2:
- simplification
- correct gpio direction initialisation

changes in v3:
- removed legacy include

 drivers/video/backlight/lm3630a_bl.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/video/backlight/lm3630a_bl.c b/drivers/video/backlight/lm3630a_bl.c
index 8f84f3684f04..d9e67b9b2571 100644
--- a/drivers/video/backlight/lm3630a_bl.c
+++ b/drivers/video/backlight/lm3630a_bl.c
@@ -12,6 +12,7 @@
 #include <linux/uaccess.h>
 #include <linux/interrupt.h>
 #include <linux/regmap.h>
+#include <linux/gpio/consumer.h>
 #include <linux/pwm.h>
 #include <linux/platform_data/lm3630a_bl.h>
 
@@ -48,6 +49,7 @@ struct lm3630a_chip {
 	struct lm3630a_platform_data *pdata;
 	struct backlight_device *bleda;
 	struct backlight_device *bledb;
+	struct gpio_desc *enable_gpio;
 	struct regmap *regmap;
 	struct pwm_device *pwmd;
 };
@@ -535,6 +537,13 @@ static int lm3630a_probe(struct i2c_client *client,
 	}
 	pchip->pdata = pdata;
 
+	pchip->enable_gpio = devm_gpiod_get_optional(&client->dev, "enable",
+						GPIOD_OUT_HIGH);
+	if (IS_ERR(pchip->enable_gpio)) {
+		rval = PTR_ERR(pchip->enable_gpio);
+		return rval;
+	}
+
 	/* chip initialize */
 	rval = lm3630a_chip_init(pchip);
 	if (rval < 0) {
-- 
2.20.1


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

* Re: [PATCH v3 2/2] backlight: lm3630a: add an enable gpio for the HWEN pin
  2019-09-11 17:21 ` [PATCH v3 2/2] backlight: lm3630a: add an enable gpio for the HWEN pin Andreas Kemnade
@ 2019-09-12  9:38   ` Daniel Thompson
  2019-09-12 11:40   ` Dan Murphy
  1 sibling, 0 replies; 8+ messages in thread
From: Daniel Thompson @ 2019-09-12  9:38 UTC (permalink / raw)
  To: Andreas Kemnade
  Cc: lee.jones, jingoohan1, jacek.anaszewski, pavel, dmurphy, robh+dt,
	mark.rutland, b.zolnierkie, dri-devel, linux-leds, devicetree,
	linux-kernel, linux-fbdev, H. Nikolaus Schaller

On Wed, Sep 11, 2019 at 07:21:06PM +0200, Andreas Kemnade wrote:
> For now just enable it in the probe function to allow i2c
> access. Disabling also means resetting the register values
> to default and according to the datasheet does not give
> power savings.
> 
> Tested on Kobo Clara HD.
> 
> Signed-off-by: Andreas Kemnade <andreas@kemnade.info>

Make sure Dan is happy w.r.t. his review comments but if this driver is
unchanged when you spin v4 (for the DT changes) then feel free to add:

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


Daniel.

> ---
> changes in v2:
> - simplification
> - correct gpio direction initialisation
> 
> changes in v3:
> - removed legacy include
> 
>  drivers/video/backlight/lm3630a_bl.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/video/backlight/lm3630a_bl.c b/drivers/video/backlight/lm3630a_bl.c
> index 8f84f3684f04..d9e67b9b2571 100644
> --- a/drivers/video/backlight/lm3630a_bl.c
> +++ b/drivers/video/backlight/lm3630a_bl.c
> @@ -12,6 +12,7 @@
>  #include <linux/uaccess.h>
>  #include <linux/interrupt.h>
>  #include <linux/regmap.h>
> +#include <linux/gpio/consumer.h>
>  #include <linux/pwm.h>
>  #include <linux/platform_data/lm3630a_bl.h>
>  
> @@ -48,6 +49,7 @@ struct lm3630a_chip {
>  	struct lm3630a_platform_data *pdata;
>  	struct backlight_device *bleda;
>  	struct backlight_device *bledb;
> +	struct gpio_desc *enable_gpio;
>  	struct regmap *regmap;
>  	struct pwm_device *pwmd;
>  };
> @@ -535,6 +537,13 @@ static int lm3630a_probe(struct i2c_client *client,
>  	}
>  	pchip->pdata = pdata;
>  
> +	pchip->enable_gpio = devm_gpiod_get_optional(&client->dev, "enable",
> +						GPIOD_OUT_HIGH);
> +	if (IS_ERR(pchip->enable_gpio)) {
> +		rval = PTR_ERR(pchip->enable_gpio);
> +		return rval;
> +	}
> +
>  	/* chip initialize */
>  	rval = lm3630a_chip_init(pchip);
>  	if (rval < 0) {
> -- 
> 2.20.1
> 

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

* Re: [PATCH v3 1/2] dt-bindings: backlight: lm3630a: add enable_gpios
  2019-09-11 17:21 ` [PATCH v3 1/2] dt-bindings: backlight: lm3630a: add enable_gpios Andreas Kemnade
@ 2019-09-12 11:39   ` Dan Murphy
  2019-09-12 14:58     ` Andreas Kemnade
  0 siblings, 1 reply; 8+ messages in thread
From: Dan Murphy @ 2019-09-12 11:39 UTC (permalink / raw)
  To: Andreas Kemnade, lee.jones, daniel.thompson, jingoohan1,
	jacek.anaszewski, pavel, robh+dt, mark.rutland, b.zolnierkie,
	dri-devel, linux-leds, devicetree, linux-kernel, linux-fbdev,
	H. Nikolaus Schaller

Andreas

On 9/11/19 12:21 PM, Andreas Kemnade wrote:
> add enable-gpios to describe HWEN pin
>
> Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
> Acked-by: Daniel Thompson <daniel.thompson@linaro.org>
> ---
> changes in v2: added example
> changes in v3: added Acked-by
>   .../bindings/leds/backlight/lm3630a-backlight.yaml           | 5 +++++
>   1 file changed, 5 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/leds/backlight/lm3630a-backlight.yaml b/Documentation/devicetree/bindings/leds/backlight/lm3630a-backlight.yaml
> index dc129d9a329e..1fa83feffe16 100644
> --- a/Documentation/devicetree/bindings/leds/backlight/lm3630a-backlight.yaml
> +++ b/Documentation/devicetree/bindings/leds/backlight/lm3630a-backlight.yaml
> @@ -29,6 +29,10 @@ properties:
>     '#size-cells':
>       const: 0
>   
> +  enable-gpios:
> +    description: GPIO to use to enable/disable the backlight (HWEN pin).
> +    maxItems: 1
> +
>   required:
>     - compatible
>     - reg
> @@ -92,6 +96,7 @@ examples:
>       i2c {
>           #address-cells = <1>;
>           #size-cells = <0>;
> +        enable-gpios = <&gpio2 5 GPIO_ACTIVE_HIGH>;
>   
>           led-controller@38 {
>                   compatible = "ti,lm3630a";

Looks good to me

Reviewed-by: Dan Murphy <dmurphy@ti.com>


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

* Re: [PATCH v3 2/2] backlight: lm3630a: add an enable gpio for the HWEN pin
  2019-09-11 17:21 ` [PATCH v3 2/2] backlight: lm3630a: add an enable gpio for the HWEN pin Andreas Kemnade
  2019-09-12  9:38   ` Daniel Thompson
@ 2019-09-12 11:40   ` Dan Murphy
  1 sibling, 0 replies; 8+ messages in thread
From: Dan Murphy @ 2019-09-12 11:40 UTC (permalink / raw)
  To: Andreas Kemnade, lee.jones, daniel.thompson, jingoohan1,
	jacek.anaszewski, pavel, robh+dt, mark.rutland, b.zolnierkie,
	dri-devel, linux-leds, devicetree, linux-kernel, linux-fbdev,
	H. Nikolaus Schaller

Andreas

On 9/11/19 12:21 PM, Andreas Kemnade wrote:
> For now just enable it in the probe function to allow i2c
> access. Disabling also means resetting the register values
> to default and according to the datasheet does not give
> power savings.
>
> Tested on Kobo Clara HD.
>
> Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
> ---
> changes in v2:
> - simplification
> - correct gpio direction initialisation
>
> changes in v3:
> - removed legacy include
>
>   drivers/video/backlight/lm3630a_bl.c | 9 +++++++++
>   1 file changed, 9 insertions(+)
>
> diff --git a/drivers/video/backlight/lm3630a_bl.c b/drivers/video/backlight/lm3630a_bl.c
> index 8f84f3684f04..d9e67b9b2571 100644
> --- a/drivers/video/backlight/lm3630a_bl.c
> +++ b/drivers/video/backlight/lm3630a_bl.c
> @@ -12,6 +12,7 @@
>   #include <linux/uaccess.h>
>   #include <linux/interrupt.h>
>   #include <linux/regmap.h>
> +#include <linux/gpio/consumer.h>
>   #include <linux/pwm.h>
>   #include <linux/platform_data/lm3630a_bl.h>
>   
> @@ -48,6 +49,7 @@ struct lm3630a_chip {
>   	struct lm3630a_platform_data *pdata;
>   	struct backlight_device *bleda;
>   	struct backlight_device *bledb;
> +	struct gpio_desc *enable_gpio;
>   	struct regmap *regmap;
>   	struct pwm_device *pwmd;
>   };
> @@ -535,6 +537,13 @@ static int lm3630a_probe(struct i2c_client *client,
>   	}
>   	pchip->pdata = pdata;
>   
> +	pchip->enable_gpio = devm_gpiod_get_optional(&client->dev, "enable",
> +						GPIOD_OUT_HIGH);
> +	if (IS_ERR(pchip->enable_gpio)) {
> +		rval = PTR_ERR(pchip->enable_gpio);
> +		return rval;
> +	}
> +
>   	/* chip initialize */
>   	rval = lm3630a_chip_init(pchip);
>   	if (rval < 0) {

Thanks for the explanation

It looks good to me

Reviewed-by: Dan Murphy <dmurphy@ti.com>


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

* Re: [PATCH v3 1/2] dt-bindings: backlight: lm3630a: add enable_gpios
  2019-09-12 11:39   ` Dan Murphy
@ 2019-09-12 14:58     ` Andreas Kemnade
  2019-09-12 15:03       ` Dan Murphy
  0 siblings, 1 reply; 8+ messages in thread
From: Andreas Kemnade @ 2019-09-12 14:58 UTC (permalink / raw)
  To: Dan Murphy
  Cc: lee.jones, daniel.thompson, jingoohan1, jacek.anaszewski, pavel,
	robh+dt, mark.rutland, b.zolnierkie, dri-devel, linux-leds,
	devicetree, linux-kernel, linux-fbdev, H. Nikolaus Schaller

[-- Attachment #1: Type: text/plain, Size: 1595 bytes --]

On Thu, 12 Sep 2019 06:39:50 -0500
Dan Murphy <dmurphy@ti.com> wrote:

> Andreas
> 
> On 9/11/19 12:21 PM, Andreas Kemnade wrote:
> > add enable-gpios to describe HWEN pin
> >
> > Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
> > Acked-by: Daniel Thompson <daniel.thompson@linaro.org>
> > ---
> > changes in v2: added example
> > changes in v3: added Acked-by
> >   .../bindings/leds/backlight/lm3630a-backlight.yaml           | 5 +++++
> >   1 file changed, 5 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/leds/backlight/lm3630a-backlight.yaml b/Documentation/devicetree/bindings/leds/backlight/lm3630a-backlight.yaml
> > index dc129d9a329e..1fa83feffe16 100644
> > --- a/Documentation/devicetree/bindings/leds/backlight/lm3630a-backlight.yaml
> > +++ b/Documentation/devicetree/bindings/leds/backlight/lm3630a-backlight.yaml
> > @@ -29,6 +29,10 @@ properties:
> >     '#size-cells':
> >       const: 0
> >   
> > +  enable-gpios:
> > +    description: GPIO to use to enable/disable the backlight (HWEN pin).
> > +    maxItems: 1
> > +
> >   required:
> >     - compatible
> >     - reg
> > @@ -92,6 +96,7 @@ examples:
> >       i2c {
> >           #address-cells = <1>;
> >           #size-cells = <0>;
> > +        enable-gpios = <&gpio2 5 GPIO_ACTIVE_HIGH>;
> >   
> >           led-controller@38 {
> >                   compatible = "ti,lm3630a";  
> 
> Looks good to me
> 
well, the enable-gpios is still at the same place as in v2. This was sent
before your comments to v2 have been arrived.

Regards,
Andreas

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

* Re: [PATCH v3 1/2] dt-bindings: backlight: lm3630a: add enable_gpios
  2019-09-12 14:58     ` Andreas Kemnade
@ 2019-09-12 15:03       ` Dan Murphy
  0 siblings, 0 replies; 8+ messages in thread
From: Dan Murphy @ 2019-09-12 15:03 UTC (permalink / raw)
  To: Andreas Kemnade
  Cc: lee.jones, daniel.thompson, jingoohan1, jacek.anaszewski, pavel,
	robh+dt, mark.rutland, b.zolnierkie, dri-devel, linux-leds,
	devicetree, linux-kernel, linux-fbdev, H. Nikolaus Schaller

Andreas

On 9/12/19 9:58 AM, Andreas Kemnade wrote:
> On Thu, 12 Sep 2019 06:39:50 -0500
> Dan Murphy <dmurphy@ti.com> wrote:
>
>> Andreas
>>
>> On 9/11/19 12:21 PM, Andreas Kemnade wrote:
>>> add enable-gpios to describe HWEN pin
>>>
>>> Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
>>> Acked-by: Daniel Thompson <daniel.thompson@linaro.org>
>>> ---
>>> changes in v2: added example
>>> changes in v3: added Acked-by
>>>    .../bindings/leds/backlight/lm3630a-backlight.yaml           | 5 +++++
>>>    1 file changed, 5 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/leds/backlight/lm3630a-backlight.yaml b/Documentation/devicetree/bindings/leds/backlight/lm3630a-backlight.yaml
>>> index dc129d9a329e..1fa83feffe16 100644
>>> --- a/Documentation/devicetree/bindings/leds/backlight/lm3630a-backlight.yaml
>>> +++ b/Documentation/devicetree/bindings/leds/backlight/lm3630a-backlight.yaml
>>> @@ -29,6 +29,10 @@ properties:
>>>      '#size-cells':
>>>        const: 0
>>>    
>>> +  enable-gpios:
>>> +    description: GPIO to use to enable/disable the backlight (HWEN pin).
>>> +    maxItems: 1
>>> +
>>>    required:
>>>      - compatible
>>>      - reg
>>> @@ -92,6 +96,7 @@ examples:
>>>        i2c {
>>>            #address-cells = <1>;
>>>            #size-cells = <0>;
>>> +        enable-gpios = <&gpio2 5 GPIO_ACTIVE_HIGH>;
>>>    
>>>            led-controller@38 {
>>>                    compatible = "ti,lm3630a";
>> Looks good to me
>>
> well, the enable-gpios is still at the same place as in v2. This was sent
> before your comments to v2 have been arrived.

Ah I overlooked that.  Yeah that still needs to move I assumed you moved it.

Dan


> Regards,
> Andreas

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

end of thread, other threads:[~2019-09-12 15:03 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-11 17:21 [PATCH v3 0/2] backlight_lm3630a: add enable_gpios property Andreas Kemnade
2019-09-11 17:21 ` [PATCH v3 1/2] dt-bindings: backlight: lm3630a: add enable_gpios Andreas Kemnade
2019-09-12 11:39   ` Dan Murphy
2019-09-12 14:58     ` Andreas Kemnade
2019-09-12 15:03       ` Dan Murphy
2019-09-11 17:21 ` [PATCH v3 2/2] backlight: lm3630a: add an enable gpio for the HWEN pin Andreas Kemnade
2019-09-12  9:38   ` Daniel Thompson
2019-09-12 11:40   ` Dan Murphy

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).