mbox series

[v2,0/6] leds: lm3692x: Allow to set ovp and brigthness mode

Message ID cover.1577271823.git.agx@sigxcpu.org
Headers show
Series leds: lm3692x: Allow to set ovp and brigthness mode | expand

Message

Guido Günther Dec. 25, 2019, 11:07 a.m. UTC 
Overvoltage protection and brightness mode are currently hardcoded
as 29V and disabled in the driver. Make these configurable via DT.

Besides addressing review comments v2 also allows to limit the maximum led
current.

Changes from v1
- As per review comments by Dan Murphy
  https://lore.kernel.org/linux-leds/3d66b07d-b4c5-43e6-4378-d63cc84b8d43@ti.com/
  - Split commits per propoerty
  - Add new properties to DT example too
  - Drop dev_dbg() statements
  - ovp: fix 21V value parsing
  - ovp: Set correct default value if DT parsing fails
- As per review comments by Pavel Machek
  https://lore.kernel.org/linux-leds/20191221191515.GF32732@amd/
  - Fix defaults (which is 29V)
  - Use uV as Unit for ovp property
- Change property name to 'ti,ovp-microvolt' to make it shorter
- Honor led-max-microamp to not exceed the maximum led current

To: Jacek Anaszewski <jacek.anaszewski@gmail.com>,Pavel Machek <pavel@ucw.cz>,Dan Murphy <dmurphy@ti.com>,Rob Herring <robh+dt@kernel.org>,Mark Rutland <mark.rutland@arm.com>,linux-leds@vger.kernel.org,devicetree@vger.kernel.org,linux-kernel@vger.kernel.org


Guido Günther (6):
  dt: bindings: lm3692x: Add ti,ovp-microvolt property
  leds: lm3692x: Allow to configure over voltage protection
  dt: bindings: lm3692x: Add ti,brightness-mapping-exponential property
  leds: lm3692x: Allow to configure brigthness mode
  dt: bindings: lm3692x: Add led-max-microamp property
  leds: lm3692x: Make sure we don't exceed the maximum led current

 .../devicetree/bindings/leds/leds-lm3692x.txt | 11 +++
 drivers/leds/leds-lm3692x.c                   | 67 +++++++++++++++++--
 2 files changed, 72 insertions(+), 6 deletions(-)

Comments

Pavel Machek Dec. 26, 2019, 10:07 a.m. UTC | #1 
On Wed 2019-12-25 12:07:15, Guido Günther wrote:
> Overvoltage protection is currently using the default of 29V.  Make it
> configurable via DT.
> 
> Signed-off-by: Guido Günther <agx@sigxcpu.org>

Acked-by: Pavel	 Machek <pavel@ucw.cz>
Pavel Machek Dec. 26, 2019, 10:13 a.m. UTC | #2 
On Wed 2019-12-25 12:07:19, Guido Günther wrote:
1;2802;0c> The current is given by the formular from page 12 of
> https://www.ti.com/lit/ds/symlink/lm36922.pdf. We use this to limit the
> led's max_brightness using the led-max-microamp DT property.
> 
> The formular for the lm36923 is identical according to the data
sheet.

formula?

>  static int lm3692x_probe_dt(struct lm3692x_led *led)
>  {
>  	struct fwnode_handle *child = NULL;
>  	struct led_init_data init_data = {};
> -	u32 ovp;
> +	u32 ovp, max_cur;
>  	bool exp_mode;
>  	int ret;
>  
> @@ -397,6 +416,10 @@ static int lm3692x_probe_dt(struct lm3692x_led *led)
>  		return ret;
>  	}
>  
> +	fwnode_property_read_u32(child, "led-max-microamp", &max_cur);
> +	led->led_dev.max_brightness = ret ? LED_FULL :
> +		lm3692x_max_brightness(led, max_cur);
> +

Umm. Should ret come from this fwnode_property_read_u32()?

With that fixed,

Acked-by: Pavel Machek <pavel@ucw.cz>

(Feel free to wait for Rob before resending the series, and I guess
you can merge it with the next one).

Best regards,
									Pavel
Guido Günther Dec. 27, 2019, 12:59 p.m. UTC | #3 
Hi,
On Thu, Dec 26, 2019 at 11:13:36AM +0100, Pavel Machek wrote:
> On Wed 2019-12-25 12:07:19, Guido Günther wrote:
> 1;2802;0c> The current is given by the formular from page 12 of
> > https://www.ti.com/lit/ds/symlink/lm36922.pdf. We use this to limit the
> > led's max_brightness using the led-max-microamp DT property.
> > 
> > The formular for the lm36923 is identical according to the data
> sheet.
> 
> formula?
> 
> >  static int lm3692x_probe_dt(struct lm3692x_led *led)
> >  {
> >  	struct fwnode_handle *child = NULL;
> >  	struct led_init_data init_data = {};
> > -	u32 ovp;
> > +	u32 ovp, max_cur;
> >  	bool exp_mode;
> >  	int ret;
> >  
> > @@ -397,6 +416,10 @@ static int lm3692x_probe_dt(struct lm3692x_led *led)
> >  		return ret;
> >  	}
> >  
> > +	fwnode_property_read_u32(child, "led-max-microamp", &max_cur);
> > +	led->led_dev.max_brightness = ret ? LED_FULL :
> > +		lm3692x_max_brightness(led, max_cur);
> > +
> 
> Umm. Should ret come from this fwnode_property_read_u32()?

Argh...i was sure i had that fixed (and tested without setting
led-max-microamp) but it was sitting on another branch. Thanks a lot for
catching that!
 -- Guido

> 
> With that fixed,
> 
> Acked-by: Pavel Machek <pavel@ucw.cz>
> 
> (Feel free to wait for Rob before resending the series, and I guess
> you can merge it with the next one).
> 
> Best regards,
> 									Pavel
> -- 
> (english) http://www.livejournal.com/~pavelmachek
> (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html