Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Fri, 28 Jan 2022 18:53:24 -0600
From:   Bjorn Andersson <bjorn.andersson@linaro.org>
To:     Marijn Suijten <marijn.suijten@somainline.org>
Cc:     Pavel Machek <pavel@ucw.cz>, Rob Herring <robh+dt@kernel.org>,
        Andy Gross <agross@kernel.org>,
        Thierry Reding <thierry.reding@gmail.com>,
        Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>,
        Lee Jones <lee.jones@linaro.org>, 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,
        Yassine Oudjana <y.oudjana@protonmail.com>,
        Luca Weiss <luca@z3ntu.xyz>,
        Subbaraman Narayanamurthy <subbaram@codeaurora.org>
Subject: Re: [PATCH v10 2/2] leds: Add driver for Qualcomm LPG
Message-ID: <YfSQBOHkwCKMGrbu@yoga>
References: <20211010043912.136640-1-bjorn.andersson@linaro.org>
 <20211010043912.136640-2-bjorn.andersson@linaro.org>
 <YXL0DyyPkS4/wfB7@ripper>
 <20211027211928.tjybwy2lokj6eoun@SoMainline.org>
 <20211027212709.4ma5uzy5titmgzqv@SoMainline.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20211027212709.4ma5uzy5titmgzqv@SoMainline.org>
Precedence: bulk

On Wed 27 Oct 16:27 CDT 2021, Marijn Suijten wrote:

> On 2021-10-27 23:19:30, Marijn Suijten wrote:
> > Hi Bjorn,
> > 
> > On 2021-10-22 10:25:35, Bjorn Andersson wrote:
> > > On Sat 09 Oct 21:39 PDT 2021, Bjorn Andersson wrote:
> > > 
> > > > The Light Pulse Generator (LPG) is a PWM-block found in a wide range of
> > > > PMICs from Qualcomm. These PMICs typically comes with 1-8 LPG instances,
> > > > with their output being routed to various other components, such as
> > > > current sinks or GPIOs.
> > > > 
> > > > Each LPG instance can operate on fixed parameters or based on a shared
> > > > lookup-table, altering the duty cycle over time. This provides the means
> > > > for hardware assisted transitions of LED brightness.
> > > > 
> > > > A typical use case for the fixed parameter mode is to drive a PWM
> > > > backlight control signal, the driver therefor allows each LPG instance
> > > > to be exposed to the kernel either through the LED framework or the PWM
> > > > framework.
> > > > 
> > > > A typical use case for the LED configuration is to drive RGB LEDs in
> > > > smartphones etc, for which the driver support multiple channels to be
> > > > ganged up to a MULTICOLOR LED. In this configuration the pattern
> > > > generators will be synchronized, to allow for multi-color patterns.
> > > > 
> > > > Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> > > > ---
> > > 
> > > Any feedback on this?
> > 
> > I asked in #linux-msm whether anything is wrong with the patterns,
> > since my Sony Discovery (sdm630 with a pm660l) blinks way quicker on a
> > pattern that's supposed to stay on for 1s and off for 1s:
> > 
> >     echo "0 1000 255 1000" > /sys/class/leds/rgb\:status/hw_pattern
> > 
> > It however seems to be broken in the same way on an older version now
> > (this might be v9 or v8) which I don't remember to be the case.  Can you
> > double-check if this is all working fine on your side?  If so, I'll have
> > to find some time to debug it on my end.
> > 
> > Thanks!
> > - Marijn
> 
> Another thing I just ran into: on both patch revisions the colors are
> flipped.  multi_index reports "red green glue", but the values written
> to multi_intensity correspond to "blue green red" instead.  Is it the
> same on your side?
> 

I booted one of my 8974 devices with RGB LED and the colors matches my
expectations. Can you confirm that your mapping in the DT node is
correct?

E.g. with pm8941 the mapping should be "backwards":

lpg {
    ...;
    rgb-led {
        color = <LED_COLOR_ID_RGB>;
        function = LED_FUNCTION_STATUS;

        #address-cells = <1>;
        #size-cells = <0>;

        led@1 {
            reg = <7>;
            color = <LED_COLOR_ID_RED>;
        };

        led@2 {
            reg = <6>;
            color = <LED_COLOR_ID_GREEN>;
        };

        led@3 {
            reg = <5>;
            color = <LED_COLOR_ID_BLUE>;
        };
};

Regards,
Bjorn