Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.34 as permitted sender) client-ip=23.128.96.34;
Message-ID: <a2781978-2081-f4cb-dfe3-0489860dab8e@quicinc.com>
Date:   Thu, 12 Oct 2023 14:50:42 -0700
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101
 Thunderbird/91.5.0
Subject: Re: [PATCH v5 0/7] Add support for LUT PPG
Content-Language: en-US
To:     Luca Weiss <luca.weiss@fairphone.com>, <pavel@ucw.cz>,
        <lee@kernel.org>, <thierry.reding@gmail.com>, <robh+dt@kernel.org>,
        <krzysztof.kozlowski+dt@linaro.org>, <conor+dt@kernel.org>,
        <agross@kernel.org>, <andersson@kernel.org>
CC:     <konrad.dybcio@linaro.org>, <u.kleine-koenig@pengutronix.de>,
        <quic_subbaram@quicinc.com>, <quic_gurus@quicinc.com>,
        <linux-leds@vger.kernel.org>, <devicetree@vger.kernel.org>,
        <linux-kernel@vger.kernel.org>, <linux-arm-msm@vger.kernel.org>,
        <linux-pwm@vger.kernel.org>, <kernel@quicinc.com>
References: <20230929003901.15086-1-quic_amelende@quicinc.com>
 <CVX5ZUGU9BVE.2TA819U1AI6BZ@otso>
From:   Anjelique Melendez <quic_amelende@quicinc.com>
In-Reply-To: <CVX5ZUGU9BVE.2TA819U1AI6BZ@otso>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
Precedence: bulk


On 10/1/2023 7:15 AM, Luca Weiss wrote:
> On Fri Sep 29, 2023 at 2:38 AM CEST, Anjelique Melendez wrote:
>> In certain PMICs, LUT pattern and LPG configuration is stored in SDAM
>> modules instead of LUT peripheral. This feature is called PPG.
>>
>> This change series adds support for PPG. Thanks!
[..]
>>
>> Tested-by: Luca Weiss <luca.weiss@fairphone.com> # sdm632-fairphone-fp3 (pmi632)
> 
> Hi Anjelique,
> 
> Actually I've retested this now on PMI632 (and also realized that my
> previous tests weren't correct and wasn't actually using hw_pattern).
> 
> Using the following commands (after boot) I'm expecting to get a
> 500ms on 500ms off blinking pattern between white (255 255 255) and off
> (0 0 0).
> 
>   echo pattern > /sys/class/leds/rgb:status/trigger
>   echo -1 > /sys/class/leds/rgb:status/repeat
> 
>   echo "255 255 255" > /sys/class/leds/rgb:status/multi_intensity
>   echo "255 500 255 0 0 500 0 0" > /sys/class/leds/rgb:status/hw_pattern
> 
> What I actually see is it blinking between cyan (0 255 255) and red (255
> 0 0).
> At some point after playing with many patterns I got it to actually
> cycle between white and off, but I couldn't reproduce this again (or I
> didn't try hard enough).
> 
> 
> But with this example it correctly blinks red on-off.
> 
>   echo "255 0 0" > /sys/class/leds/rgb:status/multi_intensity
>   echo "255 500 255 0 0 500 0 0" > /sys/class/leds/rgb:status/hw_pattern
> 
> With "0 255 0" and "0 0 255" the other colors also work fine, it's just
> the combinations that seem somewhat broken.
> 
> Regards
> Luca
> 
> 
Hi Luca,

Thanks for testing again and the feedback!
Looks like for multicolor devices there is a small concurrency issue with
enabling pattern at the same time for all the led channels. This could be
why you observed your device blinking between red (255 0 0) and cyan (0 255 255),
instead of seeing all channels (255 255 255) blink.
The fix I'm planing to include in the next series is is to disable the multicolor led
channels first, then configure all channels, and finally re-enable channels
so that pattern is triggered at the same time for each all of the channels.

I am currently testing with pm8350c device so if you are able to test next series
on pmi632 it would be very appreciated!

Thanks,
Anjelique