diff mbox series

[v4,2/2] leds: qcom-lpg: Add PM660L configuration and compatible

Message ID 20220719211848.1653920-2-marijn.suijten@somainline.org
State New
Headers show
Series [v4,1/2] dt-bindings: leds: qcom-lpg: Add compatible for PM660L LPG block | expand

Commit Message

Marijn Suijten July 19, 2022, 9:18 p.m. UTC 
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@somainline.org>
Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---

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/20220511190718.764445-2-marijn.suijten@somainline.org/

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

v2: https://lore.kernel.org/linux-leds/20220507221123.2201668-1-marijn.suijten@somainline.org/T/#u

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

v1: https://lore.kernel.org/linux-leds/20220504205704.699500-1-marijn.suijten@somainline.org/T/#u

 drivers/leds/rgb/leds-qcom-lpg.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

Comments

Johan Hovold Dec. 19, 2023, 8:22 a.m. UTC | #1 
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 <marijn.suijten@somainline.org>
> Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> ---
> 
> 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.suijten@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
Marijn Suijten Dec. 19, 2023, 9:17 a.m. UTC | #2 
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 <marijn.suijten@somainline.org>
> > Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> > ---
> > 
> > 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.suijten@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.suijten@somainline.org/

Thanks!

- Marijn
Johan Hovold Dec. 19, 2023, 9:33 a.m. UTC | #3 
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.suijten@somainline.org/

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

Johan
Marijn Suijten Dec. 19, 2023, 10:06 a.m. UTC | #4 
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.suijten@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
Johan Hovold Dec. 20, 2023, 8:09 a.m. UTC | #5 
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.suijten@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
Marijn Suijten Feb. 4, 2024, 5:29 p.m. UTC | #6 
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 <marijn.suijten@somainline.org>
> > > Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> > > ---
> > > 
> > > 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.suijten@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-2f54d1a0894b@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.suijten@somainline.org/

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

- Marijn
diff mbox series

Patch

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