Re: [PATCH v4 2/2] leds: qcom-lpg: Add PM660L configuration and compatible
On 2023-12-19 10:17:21, Marijn Suijten wrote:
> Hi Johan and Lee,
>
> On 2023-12-19 09:22:43, Johan Hovold wrote:
> > Hi Marijn and Lee,
> >
> > On Tue, Jul 19, 2022 at 11:18:48PM +0200, 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
> > > Reviewed-by: Bjorn Andersson
> > > ---
> > >
> > > Changes since v3:
> > > - Rebased on -next;
> > > - (series) dropped DTS patches that have been applied through the
> > > Qualcomm DTS tree, leaving only leds changes (driver and
> > > accompanying dt-bindings).
> >
> > > diff --git a/drivers/leds/rgb/leds-qcom-lpg.c
> > > b/drivers/leds/rgb/leds-qcom-lpg.c
> > > index 02f51cc61837..102ab0c33887 100644
> > > --- a/drivers/leds/rgb/leds-qcom-lpg.c
> > > +++ b/drivers/leds/rgb/leds-qcom-lpg.c
> > > @@ -1304,6 +1304,23 @@ static int lpg_remove(struct platform_device *pdev)
> > > return 0;
> > > }
> > >
> > > +static const struct lpg_data pm660l_lpg_data = {
> > > + .lut_base = 0xb000,
> > > + .lut_size = 49,
> > > +
> > > + .triled_base = 0xd000,
> > > + .triled_has_atc_ctl = true,
> > > + .triled_has_src_sel = true,
> > > +
> > > + .num_channels = 4,
> > > + .channels = (const struct lpg_channel_data[]) {
> > > + { .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[]) {
> > > @@ -1424,6 +1441,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 },
> >
> > When reviewing the Qualcomm SPMI PMIC bindings I noticed that this patch
> > was never picked up by the LEDs maintainer, while the binding and dtsi
> > changes made it in:
> >
> >
> > https://lore.kernel.org/r/20220719211848.1653920-2-marijn.suij...@somainline.org
> >
> > Looks like it may still apply cleanly, but otherwise, would you mind
> > rebasing and resending so that Lee can pick this one up?
> >
> > Johan
>
> Coincidentally I haven't touched this device/platform for months... until last
> weekend where I noticed the same. It does not apply cleanly and I had to
> solve
> some conflicts:
>
> https://github.com/SoMainline/linux/commit/8ec5d02eaffcec24fcab6a989ab117a5b72b96b6
>
> I'll gladly resend this!
Apologies for taking more time than necessary. According to b4 the patch should
become available at:
https://lore.kernel.org/r/20240204-pm660l-lpg-v5-1-2f54d1a08...@somainline.org
> Note that I have one more unmerged leds patch around, that hasn't been looked
> at either. Would it help to send this once again, perhaps with more
> reviewers/
> testing (Johan, would you mind taking a look too)?
>
> https://lore.kernel.org/linux-leds/20220719213034.1664056-1-marijn.suij...@somainline.org/
I'll continue looking into clarifying this patch before resending it.
- Marijn
Re: [PATCH v4 2/2] leds: qcom-lpg: Add PM660L configuration and compatible
On Tue, Dec 19, 2023 at 11:06:08AM +0100, Marijn Suijten wrote: > On 2023-12-19 10:33:25, Johan Hovold wrote: > > On Tue, Dec 19, 2023 at 10:17:16AM +0100, Marijn Suijten wrote: > > > > > Note that I have one more unmerged leds patch around, that hasn't been > > > looked > > > at either. Would it help to send this once again, perhaps with more > > > reviewers/ > > > testing (Johan, would you mind taking a look too)? > > > > > > https://lore.kernel.org/linux-leds/20220719213034.1664056-1-marijn.suij...@somainline.org/ > > > > Yes, I suggest you resend that one too so that it ends up in Lee's > > inbox. > > I will rebase, test and resend it too. Just asking if you notice any glaring > issues with this patch, as it won't be the first time it has been resent after > not being looked at for some time. I haven't look at this code before and only skimmed it now, but your patch looks correct to me. Johan
Re: [PATCH v4 2/2] leds: qcom-lpg: Add PM660L configuration and compatible
On 2023-12-19 10:33:25, Johan Hovold wrote: > On Tue, Dec 19, 2023 at 10:17:16AM +0100, Marijn Suijten wrote: > > > Note that I have one more unmerged leds patch around, that hasn't been > > looked > > at either. Would it help to send this once again, perhaps with more > > reviewers/ > > testing (Johan, would you mind taking a look too)? > > > > https://lore.kernel.org/linux-leds/20220719213034.1664056-1-marijn.suij...@somainline.org/ > > Yes, I suggest you resend that one too so that it ends up in Lee's > inbox. I will rebase, test and resend it too. Just asking if you notice any glaring issues with this patch, as it won't be the first time it has been resent after not being looked at for some time. - Marijn
Re: [PATCH v4 2/2] leds: qcom-lpg: Add PM660L configuration and compatible
Hi Johan and Lee,
On 2023-12-19 09:22:43, Johan Hovold wrote:
> Hi Marijn and Lee,
>
> On Tue, Jul 19, 2022 at 11:18:48PM +0200, 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
> > Reviewed-by: Bjorn Andersson
> > ---
> >
> > Changes since v3:
> > - Rebased on -next;
> > - (series) dropped DTS patches that have been applied through the
> > Qualcomm DTS tree, leaving only leds changes (driver and
> > accompanying dt-bindings).
>
> > diff --git a/drivers/leds/rgb/leds-qcom-lpg.c
> > b/drivers/leds/rgb/leds-qcom-lpg.c
> > index 02f51cc61837..102ab0c33887 100644
> > --- a/drivers/leds/rgb/leds-qcom-lpg.c
> > +++ b/drivers/leds/rgb/leds-qcom-lpg.c
> > @@ -1304,6 +1304,23 @@ static int lpg_remove(struct platform_device *pdev)
> > return 0;
> > }
> >
> > +static const struct lpg_data pm660l_lpg_data = {
> > + .lut_base = 0xb000,
> > + .lut_size = 49,
> > +
> > + .triled_base = 0xd000,
> > + .triled_has_atc_ctl = true,
> > + .triled_has_src_sel = true,
> > +
> > + .num_channels = 4,
> > + .channels = (const struct lpg_channel_data[]) {
> > + { .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[]) {
> > @@ -1424,6 +1441,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 },
>
> When reviewing the Qualcomm SPMI PMIC bindings I noticed that this patch
> was never picked up by the LEDs maintainer, while the binding and dtsi
> changes made it in:
>
>
> https://lore.kernel.org/r/20220719211848.1653920-2-marijn.suij...@somainline.org
>
> Looks like it may still apply cleanly, but otherwise, would you mind
> rebasing and resending so that Lee can pick this one up?
>
> Johan
Coincidentally I haven't touched this device/platform for months... until last
weekend where I noticed the same. It does not apply cleanly and I had to solve
some conflicts:
https://github.com/SoMainline/linux/commit/8ec5d02eaffcec24fcab6a989ab117a5b72b96b6
I'll gladly resend this!
Note that I have one more unmerged leds patch around, that hasn't been looked
at either. Would it help to send this once again, perhaps with more reviewers/
testing (Johan, would you mind taking a look too)?
https://lore.kernel.org/linux-leds/20220719213034.1664056-1-marijn.suij...@somainline.org/
Thanks!
- Marijn
Re: [PATCH v4 2/2] leds: qcom-lpg: Add PM660L configuration and compatible
On Tue, Dec 19, 2023 at 10:17:16AM +0100, Marijn Suijten wrote: > Note that I have one more unmerged leds patch around, that hasn't been looked > at either. Would it help to send this once again, perhaps with more > reviewers/ > testing (Johan, would you mind taking a look too)? > > https://lore.kernel.org/linux-leds/20220719213034.1664056-1-marijn.suij...@somainline.org/ Yes, I suggest you resend that one too so that it ends up in Lee's inbox. Johan
Re: [PATCH v4 2/2] leds: qcom-lpg: Add PM660L configuration and compatible
Hi Marijn and Lee,
On Tue, Jul 19, 2022 at 11:18:48PM +0200, 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
> Reviewed-by: Bjorn Andersson
> ---
>
> Changes since v3:
> - Rebased on -next;
> - (series) dropped DTS patches that have been applied through the
> Qualcomm DTS tree, leaving only leds changes (driver and
> accompanying dt-bindings).
> diff --git a/drivers/leds/rgb/leds-qcom-lpg.c
> b/drivers/leds/rgb/leds-qcom-lpg.c
> index 02f51cc61837..102ab0c33887 100644
> --- a/drivers/leds/rgb/leds-qcom-lpg.c
> +++ b/drivers/leds/rgb/leds-qcom-lpg.c
> @@ -1304,6 +1304,23 @@ static int lpg_remove(struct platform_device *pdev)
> return 0;
> }
>
> +static const struct lpg_data pm660l_lpg_data = {
> + .lut_base = 0xb000,
> + .lut_size = 49,
> +
> + .triled_base = 0xd000,
> + .triled_has_atc_ctl = true,
> + .triled_has_src_sel = true,
> +
> + .num_channels = 4,
> + .channels = (const struct lpg_channel_data[]) {
> + { .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[]) {
> @@ -1424,6 +1441,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 },
When reviewing the Qualcomm SPMI PMIC bindings I noticed that this patch
was never picked up by the LEDs maintainer, while the binding and dtsi
changes made it in:
https://lore.kernel.org/r/20220719211848.1653920-2-marijn.suij...@somainline.org
Looks like it may still apply cleanly, but otherwise, would you mind
rebasing and resending so that Lee can pick this one up?
Johan
