LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Baolin Wang <baolin.wang@linaro.org>
To: Pavel Machek <pavel@ucw.cz>
Cc: Jacek Anaszewski <jacek.anaszewski@gmail.com>,
	Rob Herring <robh+dt@kernel.org>,
	Mark Rutland <mark.rutland@arm.com>,
	xiaotong.lu@spreadtrum.com, Mark Brown <broonie@kernel.org>,
	linux-leds@vger.kernel.org, DTML <devicetree@vger.kernel.org>,
	LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v2 2/2] leds: Add Spreadtrum SC27xx breathing light controller driver
Date: Thu, 10 May 2018 11:12:32 +0800	[thread overview]
Message-ID: <CAMz4kuLYP1aHTAsgN0rkqV8Zj12V4-d1pECTn8x537a9fLRndw@mail.gmail.com> (raw)
In-Reply-To: <20180509142539.GB25131@amd>

Hi Pavel,

On 9 May 2018 at 22:25, Pavel Machek <pavel@ucw.cz> wrote:
> On Tue 2018-05-08 13:39:45, Baolin Wang wrote:
>> From: Xiaotong Lu <xiaotong.lu@spreadtrum.com>
>>
>> This patch adds Spreadtrum SC27xx PMIC series breathing light controller
>> driver, which can support 3 LEDs. Each LED can work at normal PWM mode
>> and breathing mode.
>>
>> diff --git a/Documentation/ABI/testing/sysfs-class-led-driver-sc27xx b/Documentation/ABI/testing/sysfs-class-led-driver-sc27xx
>> new file mode 100644
>> index 0000000..22166fb
>> --- /dev/null
>> +++ b/Documentation/ABI/testing/sysfs-class-led-driver-sc27xx
>> @@ -0,0 +1,19 @@
>> +What:                /sys/class/leds/<led>/rise_time
>> +What:                /sys/class/leds/<led>/high_time
>> +What:                /sys/class/leds/<led>/fall_time
>> +What:                /sys/class/leds/<led>/low_time
>> +Date:                May 2018
>> +KernelVersion:       4.18
>> +Contact:     Xiaotong Lu <xiaotong.lu@spreadtrum.com>
>> +Description:
>> +             Set the pattern generator rise, high, fall and low
>> +             times (0..63). It's unit is 0.125s, it should be > 0.
>> +
>> +             1 - 125 ms
>> +             2 - 250 ms
>> +             3 - 375 ms
>> +             ...
>> +             ...
>> +             ...
>> +             62 - 7.75 s
>> +             63 - 7.875 s
>
> How does this interact with triggers? With manually setting
> brightness? Are the pattern generators independend for the LEDs?

With depending on what mode user selected. If user set the
raise/high/fall/low time values, that means the LED will work at
breathing mode, and we set brightness to turn on/off the breathing
mode LED.

Yes, they are independent for each LED.

>
> Can you generate white breathing pattern? If so, how?

Yes, firstly we should set the raise/high/fall/low time values for red
LED, blue LED and green LED (each LED time values are same), then set
one brightness value to turn on the red LED, blue LED and green LED.

>
> How do you select between normal and breathing modes?

As I explained, when user set the raise/high/fall/low time values,
that means user selects the breathing mode of the LED. Otherwise the
LED works at normal mode.

>
> I'd specify times in miliseconds or something, this is way too
> hardware specific.

But our hardware specification defines magic numbers, not times values
in milliseconds. So I think we should follow our specification's
definition, and user should also follow the same rules.

>
> Now... functionality like this is common between many LED
> controllers. N900 could do this kind of "breathing", too, and it also
> supports other patterns.
>
> I believe we need interface common between different LED controllers.
>
> And I guess it would be easiest if you dropped this part from initial
> merge.
>
> Thanks and best regards,
>
>                                                                 Pavel
> --
> (english) http://www.livejournal.com/~pavelmachek
> (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html



-- 
Baolin.wang
Best Regards

  parent reply	other threads:[~2018-05-10  3:12 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-05-08  5:39 [PATCH v2 1/2] dt-bindings: leds: Add SC27xx breathing light controller documentation Baolin Wang
2018-05-08  5:39 ` [PATCH v2 2/2] leds: Add Spreadtrum SC27xx breathing light controller driver Baolin Wang
2018-05-08 20:54   ` Jacek Anaszewski
2018-05-09  2:34     ` Baolin Wang
2018-05-09 14:25   ` Pavel Machek
2018-05-09 19:40     ` Jacek Anaszewski
2018-05-10 11:37       ` Pavel Machek
2018-05-10 19:41         ` Jacek Anaszewski
2018-05-12  8:35           ` Pavel Machek
2018-05-12 20:44             ` Jacek Anaszewski
2018-05-13  2:19               ` Baolin Wang
2018-06-21  7:25               ` Baolin Wang
2018-05-10  3:12     ` Baolin Wang [this message]
2018-05-08 15:43 ` [PATCH v2 1/2] dt-bindings: leds: Add SC27xx breathing light controller documentation Rob Herring
2018-05-09 14:25 ` Pavel Machek
2018-05-09 20:09   ` Jacek Anaszewski
2018-05-10  1:55   ` Baolin Wang

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=CAMz4kuLYP1aHTAsgN0rkqV8Zj12V4-d1pECTn8x537a9fLRndw@mail.gmail.com \
    --to=baolin.wang@linaro.org \
    --cc=broonie@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=jacek.anaszewski@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-leds@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=pavel@ucw.cz \
    --cc=robh+dt@kernel.org \
    --cc=xiaotong.lu@spreadtrum.com \
    --subject='Re: [PATCH v2 2/2] leds: Add Spreadtrum SC27xx breathing light controller driver' \
    /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

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