This series depends on [1], which adds driver for Qualcomm LPG.
[1] https://patchwork.kernel.org/project/linux-arm-msm/list/?series=505483&archive=both&state=*
satya priya (3):
dt-bindings: leds: Add pm8350c pmic support
leds: Add pm8350c support to Qualcomm LPG driver
arm64: dts: qcom: pm8350c: Add pwm support
Documentation/devicetree/bindings/leds/leds-qcom-lpg.yaml | 1 +
arch/arm64/boot/dts/qcom/pm8350c.dtsi | 6 ++++++
drivers/leds/rgb/leds-qcom-lpg.c | 10 ++++++++++
3 files changed, 17 insertions(+)
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
Add pwm support for PM8350C pmic.
Signed-off-by: satya priya <[email protected]>
---
arch/arm64/boot/dts/qcom/pm8350c.dtsi | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/pm8350c.dtsi b/arch/arm64/boot/dts/qcom/pm8350c.dtsi
index e1b75ae..ecdae55 100644
--- a/arch/arm64/boot/dts/qcom/pm8350c.dtsi
+++ b/arch/arm64/boot/dts/qcom/pm8350c.dtsi
@@ -29,6 +29,12 @@
interrupt-controller;
#interrupt-cells = <2>;
};
+
+ pm8350c_pwm4: pwm {
+ compatible = "qcom,pm8350c-pwm";
+ #pwm-cells = <2>;
+ status = "okay";
+ };
};
};
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
On Mon, Sep 06, 2021 at 04:11:07PM +0530, satya priya wrote:
> Add pwm support for PM8350C pmic.
>
> Signed-off-by: satya priya <[email protected]>
> ---
> arch/arm64/boot/dts/qcom/pm8350c.dtsi | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/pm8350c.dtsi b/arch/arm64/boot/dts/qcom/pm8350c.dtsi
> index e1b75ae..ecdae55 100644
> --- a/arch/arm64/boot/dts/qcom/pm8350c.dtsi
> +++ b/arch/arm64/boot/dts/qcom/pm8350c.dtsi
> @@ -29,6 +29,12 @@
> interrupt-controller;
> #interrupt-cells = <2>;
> };
> +
> + pm8350c_pwm4: pwm {
What does the '4' represent, an internal channel number? It should
probably be omitted if the PM8350 only has a single output PWM
port.
> + compatible = "qcom,pm8350c-pwm";
> + #pwm-cells = <2>;
> + status = "okay";
I don't think it should be enabled by default, there may be boards with
the PM8350C that don't use the PWM.
Quoting satya priya (2021-09-06 03:41:07)
> Add pwm support for PM8350C pmic.
>
> Signed-off-by: satya priya <[email protected]>
> ---
> arch/arm64/boot/dts/qcom/pm8350c.dtsi | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/pm8350c.dtsi b/arch/arm64/boot/dts/qcom/pm8350c.dtsi
> index e1b75ae..ecdae55 100644
> --- a/arch/arm64/boot/dts/qcom/pm8350c.dtsi
> +++ b/arch/arm64/boot/dts/qcom/pm8350c.dtsi
> @@ -29,6 +29,12 @@
> interrupt-controller;
> #interrupt-cells = <2>;
> };
> +
> + pm8350c_pwm4: pwm {
> + compatible = "qcom,pm8350c-pwm";
Shouldn't there be a reg property?
> + #pwm-cells = <2>;
> + status = "okay";
> + };
> };
> };
>
> --
> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
> of Code Aurora Forum, hosted by The Linux Foundation
>
On 2021-09-08 09:04, Stephen Boyd wrote:
> Quoting satya priya (2021-09-06 03:41:07)
>> Add pwm support for PM8350C pmic.
>>
>> Signed-off-by: satya priya <[email protected]>
>> ---
>> arch/arm64/boot/dts/qcom/pm8350c.dtsi | 6 ++++++
>> 1 file changed, 6 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/pm8350c.dtsi
>> b/arch/arm64/boot/dts/qcom/pm8350c.dtsi
>> index e1b75ae..ecdae55 100644
>> --- a/arch/arm64/boot/dts/qcom/pm8350c.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/pm8350c.dtsi
>> @@ -29,6 +29,12 @@
>> interrupt-controller;
>> #interrupt-cells = <2>;
>> };
>> +
>> + pm8350c_pwm4: pwm {
>> + compatible = "qcom,pm8350c-pwm";
>
> Shouldn't there be a reg property?
>
The bindings do not specify reg property. I think it is because we are
adding the base address in struct "pm8350c_pwm_data".
>> + #pwm-cells = <2>;
>> + status = "okay";
>> + };
>> };
>> };
>>
>> --
>> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a
>> member
>> of Code Aurora Forum, hosted by The Linux Foundation
>>
On 2021-09-07 23:46, Matthias Kaehlcke wrote:
> On Mon, Sep 06, 2021 at 04:11:07PM +0530, satya priya wrote:
>> Add pwm support for PM8350C pmic.
>>
>> Signed-off-by: satya priya <[email protected]>
>> ---
>> arch/arm64/boot/dts/qcom/pm8350c.dtsi | 6 ++++++
>> 1 file changed, 6 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/pm8350c.dtsi
>> b/arch/arm64/boot/dts/qcom/pm8350c.dtsi
>> index e1b75ae..ecdae55 100644
>> --- a/arch/arm64/boot/dts/qcom/pm8350c.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/pm8350c.dtsi
>> @@ -29,6 +29,12 @@
>> interrupt-controller;
>> #interrupt-cells = <2>;
>> };
>> +
>> + pm8350c_pwm4: pwm {
>
> What does the '4' represent, an internal channel number? It should
> probably be omitted if the PM8350 only has a single output PWM
> port.
>
pm8350c has four PWMs, but I think we can drop the '4' here.
>> + compatible = "qcom,pm8350c-pwm";
>> + #pwm-cells = <2>;
>> + status = "okay";
>
> I don't think it should be enabled by default, there may be boards with
> the PM8350C that don't use the PWM.
Okay.
On Wed 08 Sep 08:29 PDT 2021, Matthias Kaehlcke wrote:
> On Wed, Sep 08, 2021 at 02:37:39PM +0530, [email protected] wrote:
> > On 2021-09-07 23:46, Matthias Kaehlcke wrote:
> > > On Mon, Sep 06, 2021 at 04:11:07PM +0530, satya priya wrote:
> > > > Add pwm support for PM8350C pmic.
> > > >
> > > > Signed-off-by: satya priya <[email protected]>
> > > > ---
> > > > arch/arm64/boot/dts/qcom/pm8350c.dtsi | 6 ++++++
> > > > 1 file changed, 6 insertions(+)
> > > >
> > > > diff --git a/arch/arm64/boot/dts/qcom/pm8350c.dtsi
> > > > b/arch/arm64/boot/dts/qcom/pm8350c.dtsi
> > > > index e1b75ae..ecdae55 100644
> > > > --- a/arch/arm64/boot/dts/qcom/pm8350c.dtsi
> > > > +++ b/arch/arm64/boot/dts/qcom/pm8350c.dtsi
> > > > @@ -29,6 +29,12 @@
> > > > interrupt-controller;
> > > > #interrupt-cells = <2>;
> > > > };
> > > > +
> > > > + pm8350c_pwm4: pwm {
> > >
> > > What does the '4' represent, an internal channel number? It should
> > > probably be omitted if the PM8350 only has a single output PWM
> > > port.
> > >
> >
> > pm8350c has four PWMs, but I think we can drop the '4' here.
>
> Why is only one PWM exposed if the PMIC has for of them? Why number 4
> and not one of the others?
The node should represent all 4 channels, which ones the board uses is
captured in how they are bound to other clients - or defines as LEDs by
additional child nodes.
Regards,
Bjorn
On Wed, Sep 08, 2021 at 02:37:39PM +0530, [email protected] wrote:
> On 2021-09-07 23:46, Matthias Kaehlcke wrote:
> > On Mon, Sep 06, 2021 at 04:11:07PM +0530, satya priya wrote:
> > > Add pwm support for PM8350C pmic.
> > >
> > > Signed-off-by: satya priya <[email protected]>
> > > ---
> > > arch/arm64/boot/dts/qcom/pm8350c.dtsi | 6 ++++++
> > > 1 file changed, 6 insertions(+)
> > >
> > > diff --git a/arch/arm64/boot/dts/qcom/pm8350c.dtsi
> > > b/arch/arm64/boot/dts/qcom/pm8350c.dtsi
> > > index e1b75ae..ecdae55 100644
> > > --- a/arch/arm64/boot/dts/qcom/pm8350c.dtsi
> > > +++ b/arch/arm64/boot/dts/qcom/pm8350c.dtsi
> > > @@ -29,6 +29,12 @@
> > > interrupt-controller;
> > > #interrupt-cells = <2>;
> > > };
> > > +
> > > + pm8350c_pwm4: pwm {
> >
> > What does the '4' represent, an internal channel number? It should
> > probably be omitted if the PM8350 only has a single output PWM
> > port.
> >
>
> pm8350c has four PWMs, but I think we can drop the '4' here.
Why is only one PWM exposed if the PMIC has for of them? Why number 4
and not one of the others?
On 2021-09-08 22:38, Bjorn Andersson wrote:
> On Wed 08 Sep 08:29 PDT 2021, Matthias Kaehlcke wrote:
>
>> On Wed, Sep 08, 2021 at 02:37:39PM +0530, [email protected] wrote:
>> > On 2021-09-07 23:46, Matthias Kaehlcke wrote:
>> > > On Mon, Sep 06, 2021 at 04:11:07PM +0530, satya priya wrote:
>> > > > Add pwm support for PM8350C pmic.
>> > > >
>> > > > Signed-off-by: satya priya <[email protected]>
>> > > > ---
>> > > > arch/arm64/boot/dts/qcom/pm8350c.dtsi | 6 ++++++
>> > > > 1 file changed, 6 insertions(+)
>> > > >
>> > > > diff --git a/arch/arm64/boot/dts/qcom/pm8350c.dtsi
>> > > > b/arch/arm64/boot/dts/qcom/pm8350c.dtsi
>> > > > index e1b75ae..ecdae55 100644
>> > > > --- a/arch/arm64/boot/dts/qcom/pm8350c.dtsi
>> > > > +++ b/arch/arm64/boot/dts/qcom/pm8350c.dtsi
>> > > > @@ -29,6 +29,12 @@
>> > > > interrupt-controller;
>> > > > #interrupt-cells = <2>;
>> > > > };
>> > > > +
>> > > > + pm8350c_pwm4: pwm {
>> > >
>> > > What does the '4' represent, an internal channel number? It should
>> > > probably be omitted if the PM8350 only has a single output PWM
>> > > port.
>> > >
>> >
>> > pm8350c has four PWMs, but I think we can drop the '4' here.
>>
>> Why is only one PWM exposed if the PMIC has for of them? Why number 4
>> and not one of the others?
>
pwm4 is used for backlight support on kodiak crd board, so I mentioned
4, thinking 4 nodes should be present for 4 pwms.
but I see that we need to represent all the four channels as one node.
will drop the '4' in next version.
Thanks,
Satya Priya
> The node should represent all 4 channels, which ones the board uses is
> captured in how they are bound to other clients - or defines as LEDs by
> additional child nodes.
>
> Regards,
> Bjorn