| Message ID | 20230929003901.15086-1-quic_amelende@quicinc.com |
|---|---|
| Headers | show |
| Series | Add support for LUT PPG | expand |
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! > > Changes since v4: > - Patch 3/7 > - Get rid of r/w helpers > - Use regmap_read_poll_timeout() in qcom_pbs_wait_for_ack() > - Update error path in qcom_pbs_trigger_event() > - Fix reverse christmas tree > - Patch 4/7 > - Get rid of r/w helpers > - Update variables to use "sdam" instead of "nvmem" > - Fix comments > - Fix reverse christmas tree > - Update lpg_pattern_set() logic > - Patch 5/7 > - Removed sdam_lut_base from lpg_data > Changes since v3: > - Patch 4/7 > - Fix function returns > - Move register definition to top of file > - Revert max_brightness and probe accidental changes > - Combine init_sdam() and parse_sdam() > - Change error prints in probe to use dev_err_probe > - Remove ppg_en variable > - Update when pbs triggers are set/cleared > - Patch 6/7 > - Remove use of nvmem_count > - Move register definition to top of file > - Remove lpg_get_sdam_lut_idx() > Changes since v2: > - Patch 1/7 > - Fix dt_binding_check error > - Rename binding file to match compatible > - Iclude SoC specific comptaibles > - Patch 2/7 > - Update nvmem-names list > - Patch 3/7 > - Update EXPORT_SYMBOL to EXPORT_SYMBOL_GPL > - Fix return/break logic in qcom_pbs_wait_for_ack() > - Update iterators to be int > - Add constants > - Fix function calls in qcom_pbs_trigger_event() > - Remove unnessary comments > - Return -EPROBE_DEFER from get_pbs_client_device() > Changes since v1: > - Patch 1/7 > - Fix dt_binding_check errors > - Update binding description > - Path 2/7 > - Fix dt_binding_check errors > - Update per variant constraints > - Update nvmem description > - Patch 3/7 > - Update get_pbs_client_device() > - Drop use of printk > - Remove unused function > > 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 > > Anjelique Melendez (7): > dt-bindings: soc: qcom: Add qcom,pbs bindings > dt-bindings: leds: leds-qcom-lpg: Add support for LPG PPG > soc: qcom: add QCOM PBS driver > leds: rgb: leds-qcom-lpg: Add support for single SDAM PPG > leds: rgb: leds-qcom-lpg: Update PMI632 lpg_data to support PPG > leds: rgb: leds-qcom-lpg: Include support for PPG with dedicated LUT > SDAM > leds: rgb: Update PM8350C lpg_data to support two-nvmem PPG Scheme > > .../bindings/leds/leds-qcom-lpg.yaml | 89 ++++- > .../bindings/soc/qcom/qcom,pbs.yaml | 46 +++ > drivers/leds/rgb/leds-qcom-lpg.c | 359 ++++++++++++++++-- > drivers/soc/qcom/Kconfig | 9 + > drivers/soc/qcom/Makefile | 1 + > drivers/soc/qcom/qcom-pbs.c | 243 ++++++++++++ > include/linux/soc/qcom/qcom-pbs.h | 30 ++ > 7 files changed, 749 insertions(+), 28 deletions(-) > create mode 100644 Documentation/devicetree/bindings/soc/qcom/qcom,pbs.yaml > create mode 100644 drivers/soc/qcom/qcom-pbs.c > create mode 100644 include/linux/soc/qcom/qcom-pbs.h
On Thu, 28 Sep 2023, Anjelique Melendez wrote: > Update the pmi632 lpg_data struct so that pmi632 devices use PPG > for LUT pattern. > > Signed-off-by: Anjelique Melendez <quic_amelende@quicinc.com> > --- > drivers/leds/rgb/leds-qcom-lpg.c | 8 +++++--- > 1 file changed, 5 insertions(+), 3 deletions(-) Reviewed-by: Lee Jones <lee@kernel.org> > diff --git a/drivers/leds/rgb/leds-qcom-lpg.c b/drivers/leds/rgb/leds-qcom-lpg.c > index 4d87686f916c..a6cea6bd7167 100644 > --- a/drivers/leds/rgb/leds-qcom-lpg.c > +++ b/drivers/leds/rgb/leds-qcom-lpg.c > @@ -1660,11 +1660,13 @@ static const struct lpg_data pm8994_lpg_data = { > static const struct lpg_data pmi632_lpg_data = { > .triled_base = 0xd000, > > + .lut_size = 64, > + > .num_channels = 5, > .channels = (const struct lpg_channel_data[]) { > - { .base = 0xb300, .triled_mask = BIT(7) }, > - { .base = 0xb400, .triled_mask = BIT(6) }, > - { .base = 0xb500, .triled_mask = BIT(5) }, > + { .base = 0xb300, .triled_mask = BIT(7), .sdam_offset = 0x48 }, > + { .base = 0xb400, .triled_mask = BIT(6), .sdam_offset = 0x56 }, > + { .base = 0xb500, .triled_mask = BIT(5), .sdam_offset = 0x64 }, > { .base = 0xb600 }, > { .base = 0xb700 }, > }, > -- > 2.41.0 >
On Thu, 28 Sep 2023, Anjelique Melendez wrote: > Update the pm8350c lpg_data struct so that pm8350c devices are treated as > PWM devices that support two-nvmem PPG scheme. > > Signed-off-by: Anjelique Melendez <quic_amelende@quicinc.com> > --- > drivers/leds/rgb/leds-qcom-lpg.c | 8 +++++--- > 1 file changed, 5 insertions(+), 3 deletions(-) Reviewed-by: Lee Jones <lee@kernel.org> > diff --git a/drivers/leds/rgb/leds-qcom-lpg.c b/drivers/leds/rgb/leds-qcom-lpg.c > index 910c7cf740cc..8962ea13df29 100644 > --- a/drivers/leds/rgb/leds-qcom-lpg.c > +++ b/drivers/leds/rgb/leds-qcom-lpg.c > @@ -1800,11 +1800,13 @@ static const struct lpg_data pm8150l_lpg_data = { > static const struct lpg_data pm8350c_pwm_data = { > .triled_base = 0xef00, > > + .lut_size = 122, > + > .num_channels = 4, > .channels = (const struct lpg_channel_data[]) { > - { .base = 0xe800, .triled_mask = BIT(7) }, > - { .base = 0xe900, .triled_mask = BIT(6) }, > - { .base = 0xea00, .triled_mask = BIT(5) }, > + { .base = 0xe800, .triled_mask = BIT(7), .sdam_offset = 0x48 }, > + { .base = 0xe900, .triled_mask = BIT(6), .sdam_offset = 0x56 }, > + { .base = 0xea00, .triled_mask = BIT(5), .sdam_offset = 0x64 }, > { .base = 0xeb00 }, > }, > }; > -- > 2.41.0 >
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
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! Changes since v4: - Patch 3/7 - Get rid of r/w helpers - Use regmap_read_poll_timeout() in qcom_pbs_wait_for_ack() - Update error path in qcom_pbs_trigger_event() - Fix reverse christmas tree - Patch 4/7 - Get rid of r/w helpers - Update variables to use "sdam" instead of "nvmem" - Fix comments - Fix reverse christmas tree - Update lpg_pattern_set() logic - Patch 5/7 - Removed sdam_lut_base from lpg_data Changes since v3: - Patch 4/7 - Fix function returns - Move register definition to top of file - Revert max_brightness and probe accidental changes - Combine init_sdam() and parse_sdam() - Change error prints in probe to use dev_err_probe - Remove ppg_en variable - Update when pbs triggers are set/cleared - Patch 6/7 - Remove use of nvmem_count - Move register definition to top of file - Remove lpg_get_sdam_lut_idx() Changes since v2: - Patch 1/7 - Fix dt_binding_check error - Rename binding file to match compatible - Iclude SoC specific comptaibles - Patch 2/7 - Update nvmem-names list - Patch 3/7 - Update EXPORT_SYMBOL to EXPORT_SYMBOL_GPL - Fix return/break logic in qcom_pbs_wait_for_ack() - Update iterators to be int - Add constants - Fix function calls in qcom_pbs_trigger_event() - Remove unnessary comments - Return -EPROBE_DEFER from get_pbs_client_device() Changes since v1: - Patch 1/7 - Fix dt_binding_check errors - Update binding description - Path 2/7 - Fix dt_binding_check errors - Update per variant constraints - Update nvmem description - Patch 3/7 - Update get_pbs_client_device() - Drop use of printk - Remove unused function Tested-by: Luca Weiss <luca.weiss@fairphone.com> # sdm632-fairphone-fp3 (pmi632) Anjelique Melendez (7): dt-bindings: soc: qcom: Add qcom,pbs bindings dt-bindings: leds: leds-qcom-lpg: Add support for LPG PPG soc: qcom: add QCOM PBS driver leds: rgb: leds-qcom-lpg: Add support for single SDAM PPG leds: rgb: leds-qcom-lpg: Update PMI632 lpg_data to support PPG leds: rgb: leds-qcom-lpg: Include support for PPG with dedicated LUT SDAM leds: rgb: Update PM8350C lpg_data to support two-nvmem PPG Scheme .../bindings/leds/leds-qcom-lpg.yaml | 89 ++++- .../bindings/soc/qcom/qcom,pbs.yaml | 46 +++ drivers/leds/rgb/leds-qcom-lpg.c | 359 ++++++++++++++++-- drivers/soc/qcom/Kconfig | 9 + drivers/soc/qcom/Makefile | 1 + drivers/soc/qcom/qcom-pbs.c | 243 ++++++++++++ include/linux/soc/qcom/qcom-pbs.h | 30 ++ 7 files changed, 749 insertions(+), 28 deletions(-) create mode 100644 Documentation/devicetree/bindings/soc/qcom/qcom,pbs.yaml create mode 100644 drivers/soc/qcom/qcom-pbs.c create mode 100644 include/linux/soc/qcom/qcom-pbs.h