diff mbox series

[V2] mfd: qcom-spmi-pmic: Add support for pm6150 and pm6150l

Message ID 1572931309-16250-1-git-send-email-kgunda@codeaurora.org (mailing list archive)
State New, archived
Headers show
Series [V2] mfd: qcom-spmi-pmic: Add support for pm6150 and pm6150l | expand

Commit Message

Kiran Gunda Nov. 5, 2019, 5:21 a.m. UTC
Add the compatibles and PMIC ids for pm6150 and pm6150l PMICs
found on SC7180 based platforms.

Signed-off-by: Kiran Gunda <kgunda@codeaurora.org>
---
 - Changes from V1:
   Sorted the macros and compatibles.

 Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.txt | 2 ++
 drivers/mfd/qcom-spmi-pmic.c                             | 4 ++++
 2 files changed, 6 insertions(+)

Comments

Stephen Boyd Nov. 5, 2019, 7:19 p.m. UTC | #1
Quoting Kiran Gunda (2019-11-04 21:21:49)
> Add the compatibles and PMIC ids for pm6150 and pm6150l PMICs
> found on SC7180 based platforms.
> 
> Signed-off-by: Kiran Gunda <kgunda@codeaurora.org>
> ---
>  - Changes from V1:
>    Sorted the macros and compatibles.

I don't see anything sorted though.

> 
>  Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.txt | 2 ++
>  drivers/mfd/qcom-spmi-pmic.c                             | 4 ++++
>  2 files changed, 6 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.txt b/Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.txt
> index 1437062..b5fc64e 100644
> --- a/Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.txt
> +++ b/Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.txt
> @@ -32,6 +32,8 @@ Required properties:
>                     "qcom,pm8998",
>                     "qcom,pmi8998",
>                     "qcom,pm8005",
> +                  "qcom,pm6150",
> +                  "qcom,pm6150l",

And this looks badly tabbed or something?

>                     or generalized "qcom,spmi-pmic".
>  - reg:             Specifies the SPMI USID slave address for this device.
>                     For more information see:
> diff --git a/drivers/mfd/qcom-spmi-pmic.c b/drivers/mfd/qcom-spmi-pmic.c
> index e8fe705..74b7980 100644
> --- a/drivers/mfd/qcom-spmi-pmic.c
> +++ b/drivers/mfd/qcom-spmi-pmic.c
> @@ -34,6 +34,8 @@
>  #define PM8998_SUBTYPE         0x14
>  #define PMI8998_SUBTYPE                0x15
>  #define PM8005_SUBTYPE         0x18
> +#define PM6150_SUBTYPE         0x28
> +#define PM6150L_SUBTYPE                0x27
>  
>  static const struct of_device_id pmic_spmi_id_table[] = {
>         { .compatible = "qcom,spmi-pmic", .data = (void *)COMMON_SUBTYPE },
> @@ -53,6 +55,8 @@
>         { .compatible = "qcom,pm8998",    .data = (void *)PM8998_SUBTYPE },
>         { .compatible = "qcom,pmi8998",   .data = (void *)PMI8998_SUBTYPE },
>         { .compatible = "qcom,pm8005",    .data = (void *)PM8005_SUBTYPE },
> +       { .compatible = "qcom,pm6150",    .data = (void *)PM6150_SUBTYPE },
> +       { .compatible = "qcom,pm6150l",   .data = (void *)PM6150L_SUBTYPE },
>         { }
>  };
>
Kiran Gunda Nov. 6, 2019, 6:43 a.m. UTC | #2
On 2019-11-06 00:49, Stephen Boyd wrote:
> Quoting Kiran Gunda (2019-11-04 21:21:49)
>> Add the compatibles and PMIC ids for pm6150 and pm6150l PMICs
>> found on SC7180 based platforms.
>> 
>> Signed-off-by: Kiran Gunda <kgunda@codeaurora.org>
>> ---
>>  - Changes from V1:
>>    Sorted the macros and compatibles.
> 
> I don't see anything sorted though.
> 
Sorry .. I might have misunderstood your comment. Let me know if my 
understanding is correct.

>>>> And compatible here.
>>> And on macro name here.

This means you want to sort all the existing compatible and macros in 
alpha numeric order ?

>>>> Please sort on compatible string
This means you want sort in the order how the compatibles are defined ?

>> 
>>  Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.txt | 2 ++
>>  drivers/mfd/qcom-spmi-pmic.c                             | 4 ++++
>>  2 files changed, 6 insertions(+)
>> 
>> diff --git a/Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.txt 
>> b/Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.txt
>> index 1437062..b5fc64e 100644
>> --- a/Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.txt
>> +++ b/Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.txt
>> @@ -32,6 +32,8 @@ Required properties:
>>                     "qcom,pm8998",
>>                     "qcom,pmi8998",
>>                     "qcom,pm8005",
>> +                  "qcom,pm6150",
>> +                  "qcom,pm6150l",
> 
> And this looks badly tabbed or something?
> 
My bad, I used tabs. Will correct it in next post.
>>                     or generalized "qcom,spmi-pmic".
>>  - reg:             Specifies the SPMI USID slave address for this 
>> device.
>>                     For more information see:
>> diff --git a/drivers/mfd/qcom-spmi-pmic.c 
>> b/drivers/mfd/qcom-spmi-pmic.c
>> index e8fe705..74b7980 100644
>> --- a/drivers/mfd/qcom-spmi-pmic.c
>> +++ b/drivers/mfd/qcom-spmi-pmic.c
>> @@ -34,6 +34,8 @@
>>  #define PM8998_SUBTYPE         0x14
>>  #define PMI8998_SUBTYPE                0x15
>>  #define PM8005_SUBTYPE         0x18
>> +#define PM6150_SUBTYPE         0x28
>> +#define PM6150L_SUBTYPE                0x27
>> 
>>  static const struct of_device_id pmic_spmi_id_table[] = {
>>         { .compatible = "qcom,spmi-pmic", .data = (void 
>> *)COMMON_SUBTYPE },
>> @@ -53,6 +55,8 @@
>>         { .compatible = "qcom,pm8998",    .data = (void 
>> *)PM8998_SUBTYPE },
>>         { .compatible = "qcom,pmi8998",   .data = (void 
>> *)PMI8998_SUBTYPE },
>>         { .compatible = "qcom,pm8005",    .data = (void 
>> *)PM8005_SUBTYPE },
>> +       { .compatible = "qcom,pm6150",    .data = (void 
>> *)PM6150_SUBTYPE },
>> +       { .compatible = "qcom,pm6150l",   .data = (void 
>> *)PM6150L_SUBTYPE },
>>         { }
>>  };
>>
Stephen Boyd Nov. 6, 2019, 4:38 p.m. UTC | #3
Quoting Kiran Gunda (2019-11-04 21:21:49)
> diff --git a/Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.txt b/Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.txt
> index 1437062..b5fc64e 100644
> --- a/Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.txt
> +++ b/Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.txt
> @@ -32,6 +32,8 @@ Required properties:
>                     "qcom,pm8998",
>                     "qcom,pmi8998",
>                     "qcom,pm8005",
> +                  "qcom,pm6150",
> +                  "qcom,pm6150l",

This seems to match the compatible list in the driver. Can you convert
this binding to YAML and then sort this compatible string list
alpha-numberically? Two patches, one to convert to YAML and sort and
another patch to add these new compatible strings.

>                     or generalized "qcom,spmi-pmic".
>  - reg:             Specifies the SPMI USID slave address for this device.
>                     For more information see:
> diff --git a/drivers/mfd/qcom-spmi-pmic.c b/drivers/mfd/qcom-spmi-pmic.c
> index e8fe705..74b7980 100644
> --- a/drivers/mfd/qcom-spmi-pmic.c
> +++ b/drivers/mfd/qcom-spmi-pmic.c
> @@ -34,6 +34,8 @@
>  #define PM8998_SUBTYPE         0x14
>  #define PMI8998_SUBTYPE                0x15
>  #define PM8005_SUBTYPE         0x18
> +#define PM6150_SUBTYPE         0x28
> +#define PM6150L_SUBTYPE                0x27

This list looks to be sorted based on id number, so just swap the two
here.

>  
>  static const struct of_device_id pmic_spmi_id_table[] = {
>         { .compatible = "qcom,spmi-pmic", .data = (void *)COMMON_SUBTYPE },
> @@ -53,6 +55,8 @@
>         { .compatible = "qcom,pm8998",    .data = (void *)PM8998_SUBTYPE },
>         { .compatible = "qcom,pmi8998",   .data = (void *)PMI8998_SUBTYPE },
>         { .compatible = "qcom,pm8005",    .data = (void *)PM8005_SUBTYPE },
> +       { .compatible = "qcom,pm6150",    .data = (void *)PM6150_SUBTYPE },
> +       { .compatible = "qcom,pm6150l",   .data = (void *)PM6150L_SUBTYPE },

This is also sorted based on .data value, so swap the two here too.

>         { }
Stephen Boyd Nov. 6, 2019, 4:38 p.m. UTC | #4
Quoting kgunda@codeaurora.org (2019-11-05 22:43:59)
> On 2019-11-06 00:49, Stephen Boyd wrote:
> > Quoting Kiran Gunda (2019-11-04 21:21:49)
> >> Add the compatibles and PMIC ids for pm6150 and pm6150l PMICs
> >> found on SC7180 based platforms.
> >> 
> >> Signed-off-by: Kiran Gunda <kgunda@codeaurora.org>
> >> ---
> >>  - Changes from V1:
> >>    Sorted the macros and compatibles.
> > 
> > I don't see anything sorted though.
> > 
> Sorry .. I might have misunderstood your comment. Let me know if my 
> understanding is correct.
> 
> >>>> And compatible here.
> >>> And on macro name here.
> 
> This means you want to sort all the existing compatible and macros in 
> alpha numeric order ?

Sorry I also got confused on what the driver is doing. I replied on the
original patch with what is preferred.
Kiran Gunda Nov. 11, 2019, 6:14 a.m. UTC | #5
On 2019-11-06 22:08, Stephen Boyd wrote:
> Quoting Kiran Gunda (2019-11-04 21:21:49)
>> diff --git a/Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.txt 
>> b/Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.txt
>> index 1437062..b5fc64e 100644
>> --- a/Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.txt
>> +++ b/Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.txt
>> @@ -32,6 +32,8 @@ Required properties:
>>                     "qcom,pm8998",
>>                     "qcom,pmi8998",
>>                     "qcom,pm8005",
>> +                  "qcom,pm6150",
>> +                  "qcom,pm6150l",
> 
> This seems to match the compatible list in the driver. Can you convert
> this binding to YAML and then sort this compatible string list
> alpha-numberically? Two patches, one to convert to YAML and sort and
> another patch to add these new compatible strings.
> 
Sure. Will do it.
>>                     or generalized "qcom,spmi-pmic".
>>  - reg:             Specifies the SPMI USID slave address for this 
>> device.
>>                     For more information see:
>> diff --git a/drivers/mfd/qcom-spmi-pmic.c 
>> b/drivers/mfd/qcom-spmi-pmic.c
>> index e8fe705..74b7980 100644
>> --- a/drivers/mfd/qcom-spmi-pmic.c
>> +++ b/drivers/mfd/qcom-spmi-pmic.c
>> @@ -34,6 +34,8 @@
>>  #define PM8998_SUBTYPE         0x14
>>  #define PMI8998_SUBTYPE                0x15
>>  #define PM8005_SUBTYPE         0x18
>> +#define PM6150_SUBTYPE         0x28
>> +#define PM6150L_SUBTYPE                0x27
> 
> This list looks to be sorted based on id number, so just swap the two
> here.
> 
Ok. Will do it in next post.
>> 
>>  static const struct of_device_id pmic_spmi_id_table[] = {
>>         { .compatible = "qcom,spmi-pmic", .data = (void 
>> *)COMMON_SUBTYPE },
>> @@ -53,6 +55,8 @@
>>         { .compatible = "qcom,pm8998",    .data = (void 
>> *)PM8998_SUBTYPE },
>>         { .compatible = "qcom,pmi8998",   .data = (void 
>> *)PMI8998_SUBTYPE },
>>         { .compatible = "qcom,pm8005",    .data = (void 
>> *)PM8005_SUBTYPE },
>> +       { .compatible = "qcom,pm6150",    .data = (void 
>> *)PM6150_SUBTYPE },
>> +       { .compatible = "qcom,pm6150l",   .data = (void 
>> *)PM6150L_SUBTYPE },
> 
> This is also sorted based on .data value, so swap the two here too.
> 
Ok. Will do it in next post.
>>         { }
Kiran Gunda Nov. 11, 2019, 6:15 a.m. UTC | #6
On 2019-11-06 22:08, Stephen Boyd wrote:
> Quoting kgunda@codeaurora.org (2019-11-05 22:43:59)
>> On 2019-11-06 00:49, Stephen Boyd wrote:
>> > Quoting Kiran Gunda (2019-11-04 21:21:49)
>> >> Add the compatibles and PMIC ids for pm6150 and pm6150l PMICs
>> >> found on SC7180 based platforms.
>> >>
>> >> Signed-off-by: Kiran Gunda <kgunda@codeaurora.org>
>> >> ---
>> >>  - Changes from V1:
>> >>    Sorted the macros and compatibles.
>> >
>> > I don't see anything sorted though.
>> >
>> Sorry .. I might have misunderstood your comment. Let me know if my
>> understanding is correct.
>> 
>> >>>> And compatible here.
>> >>> And on macro name here.
>> 
>> This means you want to sort all the existing compatible and macros in
>> alpha numeric order ?
> 
> Sorry I also got confused on what the driver is doing. I replied on the
> original patch with what is preferred.
Ok.. I just replied to that.
Lee Jones Nov. 11, 2019, 11:28 a.m. UTC | #7
On Tue, 05 Nov 2019, Kiran Gunda wrote:

> Add the compatibles and PMIC ids for pm6150 and pm6150l PMICs
> found on SC7180 based platforms.
> 
> Signed-off-by: Kiran Gunda <kgunda@codeaurora.org>
> ---
>  - Changes from V1:
>    Sorted the macros and compatibles.
> 
>  Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.txt | 2 ++
>  drivers/mfd/qcom-spmi-pmic.c                             | 4 ++++
>  2 files changed, 6 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.txt b/Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.txt
> index 1437062..b5fc64e 100644
> --- a/Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.txt
> +++ b/Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.txt
> @@ -32,6 +32,8 @@ Required properties:
>                     "qcom,pm8998",
>                     "qcom,pmi8998",
>                     "qcom,pm8005",
> +		   "qcom,pm6150",
> +		   "qcom,pm6150l",

Tabbing looks off.
Kiran Gunda Nov. 12, 2019, 9:03 a.m. UTC | #8
On 2019-11-11 16:58, Lee Jones wrote:
> On Tue, 05 Nov 2019, Kiran Gunda wrote:
> 
>> Add the compatibles and PMIC ids for pm6150 and pm6150l PMICs
>> found on SC7180 based platforms.
>> 
>> Signed-off-by: Kiran Gunda <kgunda@codeaurora.org>
>> ---
>>  - Changes from V1:
>>    Sorted the macros and compatibles.
>> 
>>  Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.txt | 2 ++
>>  drivers/mfd/qcom-spmi-pmic.c                             | 4 ++++
>>  2 files changed, 6 insertions(+)
>> 
>> diff --git a/Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.txt 
>> b/Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.txt
>> index 1437062..b5fc64e 100644
>> --- a/Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.txt
>> +++ b/Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.txt
>> @@ -32,6 +32,8 @@ Required properties:
>>                     "qcom,pm8998",
>>                     "qcom,pmi8998",
>>                     "qcom,pm8005",
>> +		   "qcom,pm6150",
>> +		   "qcom,pm6150l",
> 
> Tabbing looks off.
yes. Placed a tab mistakenly. Going to address in next post.
Matthias Kaehlcke Jan. 21, 2020, 7:34 p.m. UTC | #9
Hi Kiran,

What is the status of this patch? It has outstanding comments and I
couldn't find a later version. Do you plan to post a v3 in the near
future?

Thanks

Matthias

On Wed, Nov 06, 2019 at 08:38:53AM -0800, Stephen Boyd wrote:
> Quoting kgunda@codeaurora.org (2019-11-05 22:43:59)
> > On 2019-11-06 00:49, Stephen Boyd wrote:
> > > Quoting Kiran Gunda (2019-11-04 21:21:49)
> > >> Add the compatibles and PMIC ids for pm6150 and pm6150l PMICs
> > >> found on SC7180 based platforms.
> > >> 
> > >> Signed-off-by: Kiran Gunda <kgunda@codeaurora.org>
> > >> ---
> > >>  - Changes from V1:
> > >>    Sorted the macros and compatibles.
> > > 
> > > I don't see anything sorted though.
> > > 
> > Sorry .. I might have misunderstood your comment. Let me know if my 
> > understanding is correct.
> > 
> > >>>> And compatible here.
> > >>> And on macro name here.
> > 
> > This means you want to sort all the existing compatible and macros in 
> > alpha numeric order ?
> 
> Sorry I also got confused on what the driver is doing. I replied on the
> original patch with what is preferred.
>
Kiran Gunda Jan. 23, 2020, 5:55 a.m. UTC | #10
Hi Matthias,
Sorry for the delay...

Yes. I submit the V3 shortly.

Just to note that this is a good to have patch and not critical.
We are using a "qcom,spmi-pmic" compatible in the device tree files, so 
using the pm6150/pm6150l is not a mandatory.

Thanks,
Kiran

On 2020-01-22 01:04, Matthias Kaehlcke wrote:
> Hi Kiran,
> 
> What is the status of this patch? It has outstanding comments and I
> couldn't find a later version. Do you plan to post a v3 in the near
> future?
> 
> Thanks
> 
> Matthias
> 
> On Wed, Nov 06, 2019 at 08:38:53AM -0800, Stephen Boyd wrote:
>> Quoting kgunda@codeaurora.org (2019-11-05 22:43:59)
>> > On 2019-11-06 00:49, Stephen Boyd wrote:
>> > > Quoting Kiran Gunda (2019-11-04 21:21:49)
>> > >> Add the compatibles and PMIC ids for pm6150 and pm6150l PMICs
>> > >> found on SC7180 based platforms.
>> > >>
>> > >> Signed-off-by: Kiran Gunda <kgunda@codeaurora.org>
>> > >> ---
>> > >>  - Changes from V1:
>> > >>    Sorted the macros and compatibles.
>> > >
>> > > I don't see anything sorted though.
>> > >
>> > Sorry .. I might have misunderstood your comment. Let me know if my
>> > understanding is correct.
>> >
>> > >>>> And compatible here.
>> > >>> And on macro name here.
>> >
>> > This means you want to sort all the existing compatible and macros in
>> > alpha numeric order ?
>> 
>> Sorry I also got confused on what the driver is doing. I replied on 
>> the
>> original patch with what is preferred.
>>
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.txt b/Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.txt
index 1437062..b5fc64e 100644
--- a/Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.txt
+++ b/Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.txt
@@ -32,6 +32,8 @@  Required properties:
                    "qcom,pm8998",
                    "qcom,pmi8998",
                    "qcom,pm8005",
+		   "qcom,pm6150",
+		   "qcom,pm6150l",
                    or generalized "qcom,spmi-pmic".
 - reg:             Specifies the SPMI USID slave address for this device.
                    For more information see:
diff --git a/drivers/mfd/qcom-spmi-pmic.c b/drivers/mfd/qcom-spmi-pmic.c
index e8fe705..74b7980 100644
--- a/drivers/mfd/qcom-spmi-pmic.c
+++ b/drivers/mfd/qcom-spmi-pmic.c
@@ -34,6 +34,8 @@ 
 #define PM8998_SUBTYPE		0x14
 #define PMI8998_SUBTYPE		0x15
 #define PM8005_SUBTYPE		0x18
+#define PM6150_SUBTYPE		0x28
+#define PM6150L_SUBTYPE		0x27
 
 static const struct of_device_id pmic_spmi_id_table[] = {
 	{ .compatible = "qcom,spmi-pmic", .data = (void *)COMMON_SUBTYPE },
@@ -53,6 +55,8 @@ 
 	{ .compatible = "qcom,pm8998",    .data = (void *)PM8998_SUBTYPE },
 	{ .compatible = "qcom,pmi8998",   .data = (void *)PMI8998_SUBTYPE },
 	{ .compatible = "qcom,pm8005",    .data = (void *)PM8005_SUBTYPE },
+	{ .compatible = "qcom,pm6150",    .data = (void *)PM6150_SUBTYPE },
+	{ .compatible = "qcom,pm6150l",   .data = (void *)PM6150L_SUBTYPE },
 	{ }
 };