2022-07-19 21:37:55

by Marijn Suijten

[permalink] [raw]
Subject: [PATCH v4 2/2] leds: qcom-lpg: Add PM660L configuration and compatible

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 <[email protected]>
Reviewed-by: Bjorn Andersson <[email protected]>
---

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).

v3: https://lore.kernel.org/linux-arm-msm/[email protected]/

Changes since v2:
- Constify channels struct-array (Bjorn);
- Correct LUT size to 49 slots (Bjorn).

v2: https://lore.kernel.org/linux-leds/[email protected]/T/#u

Changes since v1:
- Rebased to pick up pm8350c in the diff-context (Pavel).

v1: https://lore.kernel.org/linux-leds/[email protected]/T/#u

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 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 },
--
2.37.1


2023-12-19 08:23:01

by Johan Hovold

[permalink] [raw]
Subject: 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 <[email protected]>
> Reviewed-by: Bjorn Andersson <[email protected]>
> ---
>
> 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/[email protected]

Looks like it may still apply cleanly, but otherwise, would you mind
rebasing and resending so that Lee can pick this one up?

Johan

2023-12-19 09:26:55

by Marijn Suijten

[permalink] [raw]
Subject: 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 <[email protected]>
> > Reviewed-by: Bjorn Andersson <[email protected]>
> > ---
> >
> > 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/[email protected]
>
> 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/[email protected]/

Thanks!

- Marijn

2023-12-19 09:33:39

by Johan Hovold

[permalink] [raw]
Subject: 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/[email protected]/

Yes, I suggest you resend that one too so that it ends up in Lee's
inbox.

Johan

2023-12-19 10:06:28

by Marijn Suijten

[permalink] [raw]
Subject: 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/[email protected]/
>
> 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

2023-12-20 08:10:11

by Johan Hovold

[permalink] [raw]
Subject: 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/[email protected]/
> >
> > 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

2024-02-04 17:29:56

by Marijn Suijten

[permalink] [raw]
Subject: 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 <[email protected]>
> > > Reviewed-by: Bjorn Andersson <[email protected]>
> > > ---
> > >
> > > 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/[email protected]
> >
> > 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/[email protected]

> 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/[email protected]/

I'll continue looking into clarifying this patch before resending it.

- Marijn