Re: [PATCH v2 0/4] Add support for QCOM SPMI Flash LEDs

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Nicolas.

On 1/26/21 3:03 PM, Nícolas F. R. A. Prado wrote:
Hi,

this patch series adds support for Qualcomm's SPMI Flash LEDs present in the
PM8941 PMIC. It is used as part of MSM8974 based devices, like the Nexus 5
(hammerhead), as a camera flash or as a lantern when in torch mode.

Patch 1 adds the dt-bindings for the driver, together with a header for the
values of some properties.

Patch 2 adds the driver, which was ported from downstream [1], and is now using
the flash LED class framework.

Patch 3 enables the driver as a module in qcom_defconfig, and also enables
CONFIG_LEDS_CLASS_FLASH since it is required by the driver.

Patch 4 adds the device tree nodes configuring the driver in the pm8941 dtsi.

After the feedback I received from the v1 RFC patch (thank you Jacek and
Bjorn!), I implemented the flash LED class framework, renamed the driver to
qcom-spmi-flash and added the dt-bindings. I also did a whole lot of cleanup.

Some caveats:
- I still didn't implement get_strobe() and get_fault() for the flash LEDs,
   because I'm still not sure how to do it. get_strobe() in particular I'm not
   even sure if is possible, since after the flash turns off automatically after
   the timeout, I don't see any change in the SPMI registers. So I'm unsure how
   one would get the current strobe state.

strobe_get is optional - you can leave it uninitialized if there is no
obvious way to get strobe status.

Regarding faults - I see you have FLASH_FAULT_DETECT but have no
information on its impact whatsoever. Usually devices report the faults
by settings some register bits and then we can map those errors to
LED flash framework generic errors.

- I have yet to add the V4L2 flash wrapper for the flash LEDs. I still didn't do
   it because I wasn't sure if it was needed, so wanted to double check. But
   being a camera flash it seems that would be useful. Also, it would be great if
   someone could point me how I would go about testing the flash usage through
   V4L2.

You need a V4L2 media device driver with which this driver would
register a V4L2 flash LED sub-device. Such a device is usually
implemented for platform ISP devices. Provided it is present in the
mainline you would have to associate this driver DT node with the
media device DT node. Then you can test the V4L2 Flash control with
v4l2-ctl or yavta user space tools.

Let's skip the V4L2 support for now - it can be added later, if needed.

Another thing worth mentioning: for v1 the dt nodes were added in hammerhead's
dts (just to simplify testing), but I have now moved them to pm8941's dtsi,
since it was like that in downstream. So if folks using devices based on
PM8941/MSM8974 other than the Nexus 5 could test it, that would be great, since
I have only tested on the Nexus 5.

v1 RFC: https://lore.kernel.org/lkml/20201106165737.1029106-1-nfraprado@xxxxxxxxxxxxxx/

[1] https://github.com/AICP/kernel_lge_hammerhead/blob/n7.1/drivers/leds/leds-qpnp.c

Nícolas F. R. A. Prado (4):
   dt-bindings: leds: Add binding for qcom-spmi-flash
   leds: Add driver for QCOM SPMI Flash LEDs
   ARM: qcom_defconfig: Enable QCOM SPMI Flash LEDs
   ARM: dts: qcom: pm8941: Add nodes for QCOM SPMI Flash LEDs

  .../bindings/leds/leds-qcom-spmi-flash.yaml   |   94 ++
  arch/arm/boot/dts/qcom-pm8941.dtsi            |   38 +
  arch/arm/configs/qcom_defconfig               |    2 +
  drivers/leds/Kconfig                          |    8 +
  drivers/leds/Makefile                         |    1 +
  drivers/leds/leds-qcom-spmi-flash.c           | 1153 +++++++++++++++++
  .../dt-bindings/leds/leds-qcom-spmi-flash.h   |   15 +
  7 files changed, 1311 insertions(+)
  create mode 100644 Documentation/devicetree/bindings/leds/leds-qcom-spmi-flash.yaml
  create mode 100644 drivers/leds/leds-qcom-spmi-flash.c
  create mode 100644 include/dt-bindings/leds/leds-qcom-spmi-flash.h


--
Best regards,
Jacek Anaszewski



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux