On 2022-05-10 22:54:07, Bjorn Andersson wrote:
> On Sat 07 May 17:11 CDT 2022, Marijn Suijten wrote:
>
> > Inherit PM660L PMIC LPG/triled block configuration from downstream
> > drivers and DT sources, consisting of a triled block with automatic
> > trickle charge control and source selection, three colored led channels
> > belonging to the synchronized triled block and one loose PWM channel.
> >
> > Signed-off-by: Marijn Suijten <marijn.suijten@xxxxxxxxxxxxxx>
> > ---
> > drivers/leds/rgb/leds-qcom-lpg.c | 18 ++++++++++++++++++
> > 1 file changed, 18 insertions(+)
> >
> > diff --git a/drivers/leds/rgb/leds-qcom-lpg.c b/drivers/leds/rgb/leds-qcom-lpg.c
> > index cfa3362b2457..30c12ac8eed4 100644
> > --- a/drivers/leds/rgb/leds-qcom-lpg.c
> > +++ b/drivers/leds/rgb/leds-qcom-lpg.c
> > @@ -1271,6 +1271,23 @@ static int lpg_remove(struct platform_device *pdev)
> > return 0;
> > }
> >
> > +static const struct lpg_data pm660l_lpg_data = {
> > + .lut_base = 0xb000,
> > + .lut_size = 0x100,
>
> The documentation tells me that you have 49 entries of LUT on the
> PM660L.
Downstream DT sources report the full 0x100 range starting at 0xb000 is
used, before the first channel starts at 0xb100:
https://git.codelinaro.org/clo/la/kernel/msm-4.14/-/blob/LA.UM.8.2.1.r1-06200-sdm660.0/arch/arm64/boot/dts/qcom/pm660l.dtsi#L84-85
However, every LUT entry appears to be two bytes in size so this should
at least be halved.
The driver does seem to add another 0x42 on top of this base address:
https://git.codelinaro.org/clo/la/kernel/msm-4.14/-/blob/LA.UM.8.2.1.r1-06200-sdm660.0/drivers/pwm/pwm-qti-lpg.c#L104
(Your LPG driver adds 0x40 - I've seen both used in our downstream
driver) yet that leaves (0x100-0x42) / 2 = 95 spots.
This is still significantly higher than 49. Is part of this register
range used for something else, inaccesible for LUT-value readings or is
the counter hardware in the LPG limiting this?
The driver codes in a max length of 47:
https://git.codelinaro.org/clo/la/kernel/msm-4.14/-/blob/LA.UM.8.2.1.r1-06200-sdm660.0/drivers/pwm/pwm-qti-lpg.c#L108
If you're confident about this I'll bump it down to 49 or try validating
what happens if higher registers are used.
> > +
> > + .triled_base = 0xd000,
> > + .triled_has_atc_ctl = true,
> > + .triled_has_src_sel = true,
> > +
> > + .num_channels = 4,
> > + .channels = (struct lpg_channel_data[]) {
>
> This can be const
Thanks for spotting!
- Marijn
>
> Regards,
> Bjorn
>
> > + { .base = 0xb100, .triled_mask = BIT(5) },
> > + { .base = 0xb200, .triled_mask = BIT(6) },
> > + { .base = 0xb300, .triled_mask = BIT(7) },
> > + { .base = 0xb400 },
> > + },
> > +};
> > +
> > static const struct lpg_data pm8916_pwm_data = {
> > .num_channels = 1,
> > .channels = (const struct lpg_channel_data[]) {
> > @@ -1391,6 +1408,7 @@ static const struct lpg_data pm8350c_pwm_data = {
> > };
> >
> > static const struct of_device_id lpg_of_table[] = {
> > + { .compatible = "qcom,pm660l-lpg", .data = &pm660l_lpg_data },
> > { .compatible = "qcom,pm8150b-lpg", .data = &pm8150b_lpg_data },
> > { .compatible = "qcom,pm8150l-lpg", .data = &pm8150l_lpg_data },
> > { .compatible = "qcom,pm8350c-pwm", .data = &pm8350c_pwm_data },
> > --
> > 2.36.0
> >
