On 8.09.2023 02:30, Anjelique Melendez wrote: > > > On 9/7/2023 1:31 PM, Konrad Dybcio wrote: >> On 7.09.2023 22:26, Konrad Dybcio wrote: >>> On 7.09.2023 21:54, Anjelique Melendez wrote: >>>> >>>> >>>> On 8/30/2023 11:34 AM, Konrad Dybcio wrote: >>>>> On 30.08.2023 20:06, 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@xxxxxxxxxxx> >>>>>> --- >>>>>> drivers/leds/rgb/leds-qcom-lpg.c | 9 ++++++--- >>>>>> 1 file changed, 6 insertions(+), 3 deletions(-) >>>>>> >>>>>> diff --git a/drivers/leds/rgb/leds-qcom-lpg.c b/drivers/leds/rgb/leds-qcom-lpg.c >>>>>> index 90dc27d5eb7c..0b37d3b539f8 100644 >>>>>> --- a/drivers/leds/rgb/leds-qcom-lpg.c >>>>>> +++ b/drivers/leds/rgb/leds-qcom-lpg.c >>>>>> @@ -1672,11 +1672,14 @@ static const struct lpg_data pm8994_lpg_data = { >>>>>> static const struct lpg_data pmi632_lpg_data = { >>>>>> .triled_base = 0xd000, >>>>>> >>>>>> + .lut_size = 64, >>>>>> + .lut_sdam_base = 0x80, >>>>> Is that a predefined space for use with LPG? >>>>> >>>>> Or can it be reclaimed for something else? >>>>> >>>>> Konrad >>>> Yes, this is a predefined space for use with LPG >>> We represent the SDAM as a NVMEM device, generally it would >>> be nice to add all regions within it as subnodes in the devicetree. >> Wait hmm.. we already get it as a nvmem cell.. Or at least that's >> how I understand it (lut_sdam_base == lpg_chan_nvmem->start, pseudocode) >> >> Why don't we access it through the nvmem r/w ops then? >> >> Konrad > I think I might be a little confused on what you are asking so please let > me know if this does not answer your question. > > lut_sdam_base is the offset where lut pattern begins in the SDAM. So when we are writing back > our LED pattern we end up calling nvmem_device_write(lpg_chan_nvmem, lut_sdam_base + offset, 1, brightness). > So far for every single SDAM PPG devices we have seen the lpg_sdam_base be 0x80 and every > LUT SDAM PPG devices (pm8350c) we have seen lpg_sdam_base be 0x45, which is why we > included this value in the lpg_data rather than as a devicetree property since it has > been consistent across a few pmics. > > I am ok if you would like the lut_sdam_base to be moved to a devicetree property. So.. we have a slice of SDAM represented as an NVMEM cell (and that part of SDAM is reserved solely for LPG), and then within that cell, we need to add an additional offset to get to what we want. Correct? What's in LPG_NVMEM_CELL[0:offset-1] then? Konrad