All of lore.kernel.org
 help / color / mirror / Atom feed
From: Simon Guinot <simon.guinot-jKBdWWKqtFpg9hUCZPvPmw@public.gmane.org>
To: Jacek Anaszewski <j.anaszewski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
Cc: Bryan Wu <cooloney-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>,
	Richard Purdie <rpurdie-Fm38FmjxZ/leoWH0uzbU5w@public.gmane.org>,
	Jason Cooper <jason-NLaQJdtUoK4Be96aLqz0jA@public.gmane.org>,
	Andrew Lunn <andrew-g2DYL2Zd6BY@public.gmane.org>,
	Gregory Clement
	<gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>,
	Sebastian Hesselbarth
	<sebastian.hesselbarth-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>,
	Vincent Donnefort
	<vdonnefort-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>,
	Yoann Sculo <yoann-SEL7PEOQfnBQFI55V6+gNQ@public.gmane.org>,
	Linus Walleij
	<linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>,
	Alexandre Courbot
	<gnurou-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>,
	Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>,
	linux-leds-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Subject: Re: [PATCH v6 5/5] leds: netxbig: set led_classdev max_brightness
Date: Mon, 28 Sep 2015 13:50:33 +0200	[thread overview]
Message-ID: <20150928115033.GZ7306@kw.sim.vm.gnt> (raw)
In-Reply-To: <5609133A.3050802-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>

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

On Mon, Sep 28, 2015 at 12:15:22PM +0200, Jacek Anaszewski wrote:
> On 09/28/2015 11:19 AM, Simon Guinot wrote:
> >On Mon, Sep 28, 2015 at 10:02:35AM +0200, Jacek Anaszewski wrote:
> >>Hi Simon,
> >
> >Hi Jacek,
> >
> >>
> >>Does your device support reading the brightness currently set?
> >
> >No it don't.
> >
> >>If so, it would be good to implement brightness_get op, because
> >>AFAIR you mentioned that the firmware you are working with sets
> >>always maximum brightness value. Having the op implemented would
> >>allow to find this out.
> >
> >I don't understand how this can help. I mean, the only issue is that at
> >startup the initial LED state is unknown. And the software brightness
> >value could be false. But once the LED is configured, the brightness
> >values for software and hardware are synchronized. The brightness value
> >is stored/cached in led_classdev and it can be retrieved by the user via
> >sysfs...
> >
> >For my own knowledge, is there some interest in having brightness_get(),
> >aside of guessing the LED initial state ?
> 
> Some LED controllers can adjust brightness in case battery voltage level
> falls below some threshold.

OK, thanks for the explanation.

> 
> >What does the embedded firmware is writing 255 or 0 into the brightness
> >sysfs attribute. The max_brightness value is ignored. After this patch,
> >writing 255 and 0 still allows to configure the LED in the same way:
> >maximum brightness or off. Thus, I believe there is no compatibility
> >issue.
> 
> LED core always assures that brightness value passed to brightness_set
> op does not exceed max_brightness value. So, now after executing
> "echo 255 > brightness", LED core will adjust it to max_brightness
> (e.g. 7) before passing to brightness_set.
> 
> In the message [1], you mentioned that "LEDs are only enabled at their
> maximum level", so IIUC following is possible:
> 
> #echo 3 > "brightness"
> 
> firmware sets brightness to max_brightness from DT (e.g. 7), but
> 
> #cat brightness
> #3
> 
> Is it true?

Oh no sorry, it is a misunderstanding. By "LEDs are only enabled at
their maximum level", I was meaning "LEDs are only enabled at their
maximum level by the LaCie stock firmware". The firmware don't make
use of the different hardware brightness levels available. But the
feature works perfectly. If you write 3 into sysfs "brightness", then
you get the third brightness level.

OK, I understand you remark now. Sorry for not being very clear in a
first place.

Simon

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

WARNING: multiple messages have this Message-ID (diff)
From: simon.guinot@sequanux.org (Simon Guinot)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v6 5/5] leds: netxbig: set led_classdev max_brightness
Date: Mon, 28 Sep 2015 13:50:33 +0200	[thread overview]
Message-ID: <20150928115033.GZ7306@kw.sim.vm.gnt> (raw)
In-Reply-To: <5609133A.3050802@samsung.com>

On Mon, Sep 28, 2015 at 12:15:22PM +0200, Jacek Anaszewski wrote:
> On 09/28/2015 11:19 AM, Simon Guinot wrote:
> >On Mon, Sep 28, 2015 at 10:02:35AM +0200, Jacek Anaszewski wrote:
> >>Hi Simon,
> >
> >Hi Jacek,
> >
> >>
> >>Does your device support reading the brightness currently set?
> >
> >No it don't.
> >
> >>If so, it would be good to implement brightness_get op, because
> >>AFAIR you mentioned that the firmware you are working with sets
> >>always maximum brightness value. Having the op implemented would
> >>allow to find this out.
> >
> >I don't understand how this can help. I mean, the only issue is that at
> >startup the initial LED state is unknown. And the software brightness
> >value could be false. But once the LED is configured, the brightness
> >values for software and hardware are synchronized. The brightness value
> >is stored/cached in led_classdev and it can be retrieved by the user via
> >sysfs...
> >
> >For my own knowledge, is there some interest in having brightness_get(),
> >aside of guessing the LED initial state ?
> 
> Some LED controllers can adjust brightness in case battery voltage level
> falls below some threshold.

OK, thanks for the explanation.

> 
> >What does the embedded firmware is writing 255 or 0 into the brightness
> >sysfs attribute. The max_brightness value is ignored. After this patch,
> >writing 255 and 0 still allows to configure the LED in the same way:
> >maximum brightness or off. Thus, I believe there is no compatibility
> >issue.
> 
> LED core always assures that brightness value passed to brightness_set
> op does not exceed max_brightness value. So, now after executing
> "echo 255 > brightness", LED core will adjust it to max_brightness
> (e.g. 7) before passing to brightness_set.
> 
> In the message [1], you mentioned that "LEDs are only enabled at their
> maximum level", so IIUC following is possible:
> 
> #echo 3 > "brightness"
> 
> firmware sets brightness to max_brightness from DT (e.g. 7), but
> 
> #cat brightness
> #3
> 
> Is it true?

Oh no sorry, it is a misunderstanding. By "LEDs are only enabled at
their maximum level", I was meaning "LEDs are only enabled at their
maximum level by the LaCie stock firmware". The firmware don't make
use of the different hardware brightness levels available. But the
feature works perfectly. If you write 3 into sysfs "brightness", then
you get the third brightness level.

OK, I understand you remark now. Sorry for not being very clear in a
first place.

Simon
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150928/fb5c942d/attachment.sig>

  parent reply	other threads:[~2015-09-28 11:50 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-09-26 21:02 [PATCH v6 0/5] Add DT support for netxbig LEDs Simon Guinot
2015-09-26 21:02 ` Simon Guinot
2015-09-26 21:02 ` [PATCH v6 1/5] leds: netxbig: add device tree binding Simon Guinot
2015-09-26 21:02   ` Simon Guinot
2015-09-26 21:02 ` [PATCH v6 2/5] ARM: Kirkwood: add LED DT entries for netxbig boards Simon Guinot
2015-09-26 21:02   ` Simon Guinot
2015-09-26 21:02 ` [PATCH v6 3/5] ARM: mvebu: remove static LED setup " Simon Guinot
2015-09-26 21:02   ` Simon Guinot
2015-09-26 21:02 ` [PATCH v6 4/5] leds: netxbig: convert to use the devm_ functions Simon Guinot
2015-09-26 21:02   ` Simon Guinot
     [not found] ` <1443301358-2131-1-git-send-email-simon.guinot-jKBdWWKqtFpg9hUCZPvPmw@public.gmane.org>
2015-09-26 21:02   ` [PATCH v6 5/5] leds: netxbig: set led_classdev max_brightness Simon Guinot
2015-09-26 21:02     ` Simon Guinot
2015-09-28  8:02     ` Jacek Anaszewski
2015-09-28  8:02       ` Jacek Anaszewski
2015-09-28  9:19       ` Simon Guinot
2015-09-28  9:19         ` Simon Guinot
2015-09-28 10:15         ` Jacek Anaszewski
2015-09-28 10:15           ` Jacek Anaszewski
     [not found]           ` <5609133A.3050802-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
2015-09-28 11:50             ` Simon Guinot [this message]
2015-09-28 11:50               ` Simon Guinot
2015-09-28 12:24               ` Jacek Anaszewski
2015-09-28 12:24                 ` Jacek Anaszewski
2015-09-28 13:25                 ` Simon Guinot
2015-09-28 13:25                   ` Simon Guinot
2015-09-28 13:43                   ` Jacek Anaszewski
2015-09-28 13:43                     ` Jacek Anaszewski
2015-10-09  9:43 ` [PATCH v6 0/5] Add DT support for netxbig LEDs Jacek Anaszewski
2015-10-09  9:43   ` Jacek Anaszewski
2015-10-15  7:16   ` Gregory CLEMENT
2015-10-15  7:16     ` Gregory CLEMENT
2015-10-15  8:11     ` Jacek Anaszewski
2015-10-15  8:11       ` Jacek Anaszewski

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20150928115033.GZ7306@kw.sim.vm.gnt \
    --to=simon.guinot-jkbdwwkqtfpg9huczpvpmw@public.gmane.org \
    --cc=andrew-g2DYL2Zd6BY@public.gmane.org \
    --cc=cooloney-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org \
    --cc=devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=gnurou-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org \
    --cc=gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org \
    --cc=j.anaszewski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org \
    --cc=jason-NLaQJdtUoK4Be96aLqz0jA@public.gmane.org \
    --cc=linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org \
    --cc=linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org \
    --cc=linux-leds-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org \
    --cc=rpurdie-Fm38FmjxZ/leoWH0uzbU5w@public.gmane.org \
    --cc=sebastian.hesselbarth-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org \
    --cc=vdonnefort-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org \
    --cc=yoann-SEL7PEOQfnBQFI55V6+gNQ@public.gmane.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.