Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Mime-Version: 1.0
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain; charset=UTF-8
Date:   Wed, 19 Oct 2022 13:09:56 +0200
Message-Id: <CNPUQGP720NA.3KXP115T46HC@otso>
From:   "Luca Weiss" <luca.weiss@fairphone.com>
To:     "Fenglin Wu" <quic_fenglinw@quicinc.com>,
        <linux-arm-msm@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
        <krzysztof.kozlowski@linaro.org>
Cc:     <quic_collinsd@quicinc.com>, <quic_subbaram@quicinc.com>
Subject: Re: [PATCH v3 0/2] Add LED driver for flash module in QCOM PMICs
References: <20221018014024.948731-1-quic_fenglinw@quicinc.com>
 <CNPPXFMBHOPK.2XSBJT5M1TNVP@otso>
 <fbae8ec2-a645-6141-8308-3122c2ca9fab@quicinc.com>
In-Reply-To: <fbae8ec2-a645-6141-8308-3122c2ca9fab@quicinc.com>
Precedence: bulk

Hi Fenglin,

On Wed Oct 19, 2022 at 12:17 PM CEST, Fenglin Wu wrote:
>
>
> On 2022/10/19 15:23, Luca Weiss wrote:
> > Hi Fenglin,
> >=20
> > On Tue Oct 18, 2022 at 3:40 AM CEST, Fenglin Wu wrote:
> >> Initial driver and binding document changes for supporting flash LED
> >> module in Qualcomm Technologies, Inc. PMICs.
> >>
> >=20
> > Thanks for these patches, it's really nice to see drivers like this
> > being sent upstream!
> >=20
> > I've just tried these patches on pm6150l which also is compatible with
> > this driver (and used on sm7225-fairphone-fp4).
> >=20
> > The two different flash LEDs on the device I could adjust as expected
> > using sysfs:
> >=20
> > $ echo 255 > /sys/class/leds/yellow:flash-0/brightness
> > $ echo 255 > /sys/class/leds/white:flash-0/brightness
> >=20
> > Also lower brightness values resulted in lower brightness on the LED, s=
o
> > all is good here!
> >=20
> > But for flash usage, I couldn't figure out how to use it, doing the
> > following resulted in no change on the LED.
> >=20
> > $ cat /sys/class/leds/white:flash-0/max_flash_brightness
> > 1000000
> > $ echo 1000000 > /sys/class/leds/white:flash-0/flash_brightness
> >=20
> > Here's my LED definition:
> >=20
> >    led-0 {
> >      function =3D LED_FUNCTION_FLASH;
> >      color =3D <LED_COLOR_ID_YELLOW>;
> >      led-sources =3D <1>;
> >      led-max-microamp =3D <180000>;
> >      flash-max-microamp =3D <1000000>;
> >      flash-max-timeout-us =3D <1280000>;
> >    };
> >=20
> >  From values are from msm-4.19 kernel:
> >=20
> >    qcom,flash_0 {
> >      qcom,current-ma =3D <1000>; // =3D> flash-max-microamp
> >      qcom,duration-ms =3D <1280>; // =3D> flash-max-timeout-us
> >      qcom,id =3D <0>; // =3D> led-sources?
> >    };
> >=20
> >    qcom,torch_0 {
> >      qcom,current-ma =3D <180>; // =3D> led-max-microamp
> >      qcom,id =3D <0>; // =3D> led-sources?
> >    };
> >=20
> > Could you please let me know how flash is supposed to work or if I
> > maybe have messed up some setting here?
> >=20
> > Regards
> > Luca
>
> Hi Luca,
>
> Thanks for testing the driver at your end.
> The "brightness" node is for enabling/disable/adjusting brightness when=
=20
> the LED is working in torch mode, the nodes for enabling/adjusting the=20
> LED behavior in flash mode are "flash_brightness" "flash_timeout"=20
> "flash_strobe".
> You can strobe the flash by "echo 1 > flash_strobe" directly and the=20
> default brightness/timeout value will be used, or you can update the=20
> settings with "echo xxx > flash_brightness; echo xxx > flash_timeout"=20
> then strobe the LED with "echo 1 > flash_strobe". Please remember you=20
> always need to "echo 0 > flash_strobe" 1st if you want to enable it again=
.
> Thanks

Indeed with flash_strobe it works as expected!

Tested-by: Luca Weiss <luca.weiss@fairphone.com> # sm7225-fairphone-fp4 + p=
m6150l

Thanks again,
Luca

>
> Fenglin
> >=20
> >> Changes in V3:
> >>    1. Updated the driver to use regmap_field for register access.
> >>    2. Adressed the review comments in binding document change.
> >>
> >> Changes in V2:
> >>    1. Addressed review comments in binding change, thanks Krzysztof!
> >>    2. Updated driver to address the compilation issue reported by
> >>       kernel test robot.
> >>
> >>
> >> Fenglin Wu (2):
> >>    leds: flash: add driver to support flash LED module in QCOM PMICs
> >>    dt-bindings: add bindings for QCOM flash LED
> >>
> >>   .../bindings/leds/qcom,spmi-flash-led.yaml    | 116 +++
> >>   drivers/leds/flash/Kconfig                    |  15 +
> >>   drivers/leds/flash/Makefile                   |   1 +
> >>   drivers/leds/flash/leds-qcom-flash.c          | 700 ++++++++++++++++=
++
> >>   4 files changed, 832 insertions(+)
> >>   create mode 100644 Documentation/devicetree/bindings/leds/qcom,spmi-=
flash-led.yaml
> >>   create mode 100644 drivers/leds/flash/leds-qcom-flash.c
> >>
> >> --=20
> >> 2.25.1
> >=20