Hi Pavel, On Thu, Mar 3, 2022 at 2:10 PM Doug Anderson <dianders@xxxxxxxxxxxx> wrote: > > Hi, > > On Thu, Mar 3, 2022 at 1:41 PM Bjorn Andersson > <bjorn.andersson@xxxxxxxxxx> 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 supports 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. > > > > The idea of modelling this as a LED driver ontop of a PWM driver was > > considered, but setting the properties related to patterns does not fit > > in the PWM API. Similarly the idea of just duplicating the lower bits in > > a PWM and LED driver separately was considered, but this would not allow > > the PWM channels and LEDs to be configured on a per-board basis. The > > driver implements the more complex LED interface, and provides a PWM > > interface on the side of that, in the same driver. > > > > Signed-off-by: Bjorn Andersson <bjorn.andersson@xxxxxxxxxx> > > --- > > > > Changes since v13: > > - Fixed mixed space/tab indentation in documentation > > - Added 0 as to lpg_clk_rates[] to match the hardware state, to avoid + 1 in > > lpg_apply_freq() and - 1 in lpg_pwm_get_state() > > - Don't divide with 0 if current clock is 0 in lpg_pwm_get_state(), just return > > period = duty = 0 in this case > > - Renamed "clk" in struct lpg_channel to clk_sel > > - Renamed "pre_div" in struct lpg_channel to pre_div_sel > > > > Changes since v12: > > - Initialize ret in lpg_pwm_apply() > > > > Changes since v11: > > - Extended commit message to cover decision to put pwm_chip in the LED driver > > - Added Documentation, in particular for the hw_pattern format > > - Added a lock to synchronize requests from LED and PWM frameworks > > - Turned out that the 9bit selector differs per channel in some PMICs, so > > replaced bitmask in lpg_data with lookup based on QPNP SUBTYPE > > - Fixed kerneldoc for the struct device pointer in struct lpg > > - Rewrote conditional in lut_free() to make it easier to read > > - Corrected and deduplicated max_period expression in lpg_calc_freq() > > - Extended nom/dom to numerator/denominator in lpg_calc_freq() > > - Replaced 1 << 9 with LPG_RESOLUTION in one more place in lpg_calc_freq() > > - Use FIELD_PREP() in lpg_apply_freq() as masks was introduced for reading the > > same in get_state() > > - Cleaned up the pattern format, to allow specifying both low and high pause > > with and without pingpong mode. > > - Only update frequency and pwm_value if PWM channel is enabled in lpg_pwm_apply > > - Make lpg_pwm_get_state() read the hardware state, in order to pick up e.g. > > bootloader backlight configuration > > - Use devm_bitmap_zalloc() to allocate the lut_bitmap > > - Use dev_err_probe() in lpg_probe() > > - Extended Kconfig help text to mention module name and satisfy checkpatch > > > > Documentation/leds/leds-qcom-lpg.rst | 76 ++ > > drivers/leds/Kconfig | 3 + > > drivers/leds/Makefile | 3 + > > drivers/leds/rgb/Kconfig | 18 + > > drivers/leds/rgb/Makefile | 3 + > > drivers/leds/rgb/leds-qcom-lpg.c | 1405 ++++++++++++++++++++++++++ > > 6 files changed, 1508 insertions(+) > > Gets rid of the KASAN error and PWM still works for me, so happy to add back: > > Tested-by: Douglas Anderson <dianders@xxxxxxxxxxxx> > > I haven't done a full review of the driver but I did a once-over of > the changes between v12 and v13 and they look good to me. With v5.18-rc1 released, this seems like it would be an ideal time to land this driver and its bindings in a for-next branch for the leds subsystem. Is there anything blocking it? Are you the right person to land them? Ideally the bindings / driver (patch #1 and #2) from Satya's series [1] could land right atop it since it's ready too? [1] https://lore.kernel.org/r/1645509309-16142-1-git-send-email-quic_c_skakit@xxxxxxxxxxx/ Thanks! -Doug