[PATCH v3 0/2] soc: qcom: pmic_glink: Add support for battery management running on SOCCP

[PATCH v3 0/2] soc: qcom: pmic_glink: Add support for battery management running on SOCCP

[Anjelique Melendez]

System On Chip Control Processor (SOCCP) is a subsystem that can have
battery management firmware running on it to support Type-C/PD and
battery charging. Add support for devices, such as Kaanpali and Glymur, 
which are running battery management on SOCCP.

Changes since v2:
  - Updated bindings dependencies
  - Removed qcom,glymur-pmic-glink compatible string from pmic_glink driver list
  - Link: https://lore.kernel.org/all/20251027212250.3847537-1-anjelique.melendez@oss.qualcomm.com/
Changes since V1:
  - Corrected bindings dependencies 
  - Renamed pmic_glink_data variables
  - Dropped "soc: qcom: pmic_glink: Add support for SOCCP remoteproc channels"
    since it was applied from its original series: https://lore.kernel.org/all/176157405464.8818.5887965202916918883.b4-ty@kernel.org/
  - Link: https://lore.kernel.org/all/20251017003033.268567-1-anjelique.melendez@oss.qualcomm.com/

Anjelique Melendez (2):
  dt-bindings: soc: qcom: qcom,pmic-glink: Add Kaanapali and Glymur
    compatibles
  soc: qcom: pmic_glink: Add charger PDR service path and service name
    to client data

 .../bindings/soc/qcom/qcom,pmic-glink.yaml    |  8 +++
 drivers/soc/qcom/pmic_glink.c                 | 65 ++++++++++++-------
 2 files changed, 49 insertions(+), 24 deletions(-)

-- 
2.34.1

[PATCH v3 1/2] dt-bindings: soc: qcom: qcom,pmic-glink: Add Kaanapali and Glymur compatibles

[Anjelique Melendez]

Kaanapali PMIC glink platforms support features exposed by the SM8550
PMIC glink interface. Document the Kaanpali compatible string to have
fallback on SM8550.

Glymur PMIC glink platforms support features exposed by SM8550,
X1E80100 and Kaanapali PMIC glink interfaces. Document the Glymur
compatible string to have fallbacks on SM8550, X1E80100 and Kaanapali.

Signed-off-by: Anjelique Melendez <anjelique.melendez@oss.qualcomm.com>
---
 .../devicetree/bindings/soc/qcom/qcom,pmic-glink.yaml     | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,pmic-glink.yaml b/Documentation/devicetree/bindings/soc/qcom/qcom,pmic-glink.yaml
index 7085bf88afab..302b338e7012 100644
--- a/Documentation/devicetree/bindings/soc/qcom/qcom,pmic-glink.yaml
+++ b/Documentation/devicetree/bindings/soc/qcom/qcom,pmic-glink.yaml
@@ -37,12 +37,20 @@ properties:
           - const: qcom,pmic-glink
       - items:
           - enum:
+              - qcom,kaanapali-pmic-glink
               - qcom,milos-pmic-glink
               - qcom,sm8650-pmic-glink
               - qcom,sm8750-pmic-glink
               - qcom,x1e80100-pmic-glink
           - const: qcom,sm8550-pmic-glink
           - const: qcom,pmic-glink
+      - items:
+          - enum:
+              - qcom,glymur-pmic-glink
+          - const: qcom,kaanapali-pmic-glink
+          - const: qcom,x1e80100-pmic-glink
+          - const: qcom,sm8550-pmic-glink
+          - const: qcom,pmic-glink
 
   '#address-cells':
     const: 1
-- 
2.34.1

Re: [PATCH v3 1/2] dt-bindings: soc: qcom: qcom,pmic-glink: Add Kaanapali and Glymur compatibles

[Krzysztof Kozlowski]

On Tue, Dec 16, 2025 at 09:56:54PM -0800, Anjelique Melendez wrote:
> Kaanapali PMIC glink platforms support features exposed by the SM8550
> PMIC glink interface. Document the Kaanpali compatible string to have
> fallback on SM8550.
> 
> Glymur PMIC glink platforms support features exposed by SM8550,

We will be repeating the same discussion, probably, because above does
not help me to understand at all. How Glymur can be compatible with
Kaanapali, which is both: compatible with X1E and not compatible with
X1E

> X1E80100 and Kaanapali PMIC glink interfaces. Document the Glymur
> compatible string to have fallbacks on SM8550, X1E80100 and Kaanapali.

Half of your commit msg - this sentence and last sentence from previous
paragraph - is redundant because you repeat what is in the diff. It's
obvious. Imagine please that we can read the diff. But the weirdness of
compatibility is not explained at all.

> 
> Signed-off-by: Anjelique Melendez <anjelique.melendez@oss.qualcomm.com>
> ---
>  .../devicetree/bindings/soc/qcom/qcom,pmic-glink.yaml     | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,pmic-glink.yaml b/Documentation/devicetree/bindings/soc/qcom/qcom,pmic-glink.yaml
> index 7085bf88afab..302b338e7012 100644
> --- a/Documentation/devicetree/bindings/soc/qcom/qcom,pmic-glink.yaml
> +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,pmic-glink.yaml
> @@ -37,12 +37,20 @@ properties:
>            - const: qcom,pmic-glink
>        - items:
>            - enum:
> +              - qcom,kaanapali-pmic-glink
>                - qcom,milos-pmic-glink
>                - qcom,sm8650-pmic-glink
>                - qcom,sm8750-pmic-glink
>                - qcom,x1e80100-pmic-glink

This means they are not compatible...

>            - const: qcom,sm8550-pmic-glink
>            - const: qcom,pmic-glink
> +      - items:
> +          - enum:
> +              - qcom,glymur-pmic-glink
> +          - const: qcom,kaanapali-pmic-glink
> +          - const: qcom,x1e80100-pmic-glink

This suggests they are actually compatible.

> +          - const: qcom,sm8550-pmic-glink
> +          - const: qcom,pmic-glink
>  
>    '#address-cells':
>      const: 1
> -- 
> 2.34.1
> 

Re: [PATCH v3 1/2] dt-bindings: soc: qcom: qcom,pmic-glink: Add Kaanapali and Glymur compatibles

[Bjorn Andersson]

On Tue, Dec 16, 2025 at 09:56:54PM -0800, Anjelique Melendez wrote:
> Kaanapali PMIC glink platforms support features exposed by the SM8550
> PMIC glink interface. Document the Kaanpali compatible string to have
> fallback on SM8550.
> 
> Glymur PMIC glink platforms support features exposed by SM8550,
> X1E80100 and Kaanapali PMIC glink interfaces. Document the Glymur
> compatible string to have fallbacks on SM8550, X1E80100 and Kaanapali.

But while the Glymur PMIC GLINK interface does present (almost) the same
features as SM8550 and/or Kaanapali, I don't see them as compatible. The
protocol is different and if nothing else, as shown by patch 2, the life
cycle is different.

> 
> Signed-off-by: Anjelique Melendez <anjelique.melendez@oss.qualcomm.com>
> ---
>  .../devicetree/bindings/soc/qcom/qcom,pmic-glink.yaml     | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,pmic-glink.yaml b/Documentation/devicetree/bindings/soc/qcom/qcom,pmic-glink.yaml
> index 7085bf88afab..302b338e7012 100644
> --- a/Documentation/devicetree/bindings/soc/qcom/qcom,pmic-glink.yaml
> +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,pmic-glink.yaml
> @@ -37,12 +37,20 @@ properties:
>            - const: qcom,pmic-glink
>        - items:
>            - enum:
> +              - qcom,kaanapali-pmic-glink
>                - qcom,milos-pmic-glink
>                - qcom,sm8650-pmic-glink
>                - qcom,sm8750-pmic-glink
>                - qcom,x1e80100-pmic-glink
>            - const: qcom,sm8550-pmic-glink
>            - const: qcom,pmic-glink
> +      - items:
> +          - enum:
> +              - qcom,glymur-pmic-glink
> +          - const: qcom,kaanapali-pmic-glink
> +          - const: qcom,x1e80100-pmic-glink
> +          - const: qcom,sm8550-pmic-glink
> +          - const: qcom,pmic-glink

So this would then result in:

compatible = "qcom,glymur-pmic-glink", "qcom,kaanapali-pmic-glink", "qcom,x1e80100-pmic-glink", "qcom,sm8550-pmic-glink", "qcom,pmic-glink";

Which tells the operating system that it should use the implementation
for the Glymur interface, and if that isn't known the implementation for
the Kaanapali interface, and if that isn't known the implementation for
the Hamoa interface, and if that isn't known the implementation for the
SM8550 interface, and if that isn't known the generic "pmic glink"
interface.

So, in an operating system that doesn't know how to interact with
Glymur, you have stated that Kaanapali is a good second choice. This
does accurately capture the soccp behavior, but looking at the
individual functions we're not compatible with Kaanapali.

If we're in an operating system that doesn't know about Glymur or
Kaanapali, you're stating that the Hamoa implementation is good. I think
this is accurate for several of the features, but the OS will forever
wait for the services to show up.

Etc...

As such, I think the appropriate way to express this is to add two new
"top-level" options:

      - items:
          - const: qcom,kaanapali-pmic-glink
          - const: qcom,pmic-glink
      - items:
          - const: qcom,glymur-pmic-glink
          - const: qcom,pmic-glink

This establishes them as new "base compatibles", which can be used for
derived platforms, by just adding the relevant enums. The alternative is
to add them to the first item (enum + qcom,pmic-glink) and then as soon
as we have derivatives we split it out in enum + kaanapali +
qcom,pmic-glink or enum + glymur + qcom,pmic-glink. But I think this
would be cleaner.

Regards,
Bjorn

>  
>    '#address-cells':
>      const: 1
> -- 
> 2.34.1
> 

Re: [PATCH v3 1/2] dt-bindings: soc: qcom: qcom,pmic-glink: Add Kaanapali and Glymur compatibles

[Anjelique Melendez]

On 12/22/2025 11:42 AM, Bjorn Andersson wrote:
> On Tue, Dec 16, 2025 at 09:56:54PM -0800, Anjelique Melendez wrote:
>> Kaanapali PMIC glink platforms support features exposed by the
>> SM8550 PMIC glink interface. Document the Kaanpali compatible
>> string to have fallback on SM8550.
>> 
>> Glymur PMIC glink platforms support features exposed by SM8550, 
>> X1E80100 and Kaanapali PMIC glink interfaces. Document the Glymur 
>> compatible string to have fallbacks on SM8550, X1E80100 and
>> Kaanapali.
> 
> But while the Glymur PMIC GLINK interface does present (almost) the
> same features as SM8550 and/or Kaanapali, I don't see them as
> compatible. The protocol is different and if nothing else, as shown
> by patch 2, the life cycle is different.
> 
>> 
>> Signed-off-by: Anjelique Melendez
>> <anjelique.melendez@oss.qualcomm.com> --- .../devicetree/bindings/
>> soc/qcom/qcom,pmic-glink.yaml     | 8 ++++++++ 1 file changed, 8
>> insertions(+)
>> 
>> diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,pmic-
>> glink.yaml b/Documentation/devicetree/bindings/soc/qcom/qcom,pmic-
>> glink.yaml index 7085bf88afab..302b338e7012 100644 --- a/
>> Documentation/devicetree/bindings/soc/qcom/qcom,pmic-glink.yaml ++
>> + b/Documentation/devicetree/bindings/soc/qcom/qcom,pmic-
>> glink.yaml @@ -37,12 +37,20 @@ properties: - const: qcom,pmic-
>> glink - items: - enum: +              - qcom,kaanapali-pmic-glink 
>> - qcom,milos-pmic-glink - qcom,sm8650-pmic-glink - qcom,sm8750-
>> pmic-glink - qcom,x1e80100-pmic-glink - const: qcom,sm8550-pmic-
>> glink - const: qcom,pmic-glink +      - items: +          - enum: 
>> +              - qcom,glymur-pmic-glink +          - const:
>> qcom,kaanapali-pmic-glink +          - const: qcom,x1e80100-pmic-
>> glink +          - const: qcom,sm8550-pmic-glink +          -
>> const: qcom,pmic-glink
> 
> So this would then result in:
> 
> compatible = "qcom,glymur-pmic-glink", "qcom,kaanapali-pmic-glink",
> "qcom,x1e80100-pmic-glink", "qcom,sm8550-pmic-glink", "qcom,pmic-
> glink";
> 
> Which tells the operating system that it should use the
> implementation for the Glymur interface, and if that isn't known the
> implementation for the Kaanapali interface, and if that isn't known
> the implementation for the Hamoa interface, and if that isn't known
> the implementation for the SM8550 interface, and if that isn't known
> the generic "pmic glink" interface.
> 
> So, in an operating system that doesn't know how to interact with 
> Glymur, you have stated that Kaanapali is a good second choice. This 
> does accurately capture the soccp behavior, but looking at the 
> individual functions we're not compatible with Kaanapali.
> 
> If we're in an operating system that doesn't know about Glymur or 
> Kaanapali, you're stating that the Hamoa implementation is good. I
> think this is accurate for several of the features, but the OS will
> forever wait for the services to show up.
> 
> Etc...
> 
> As such, I think the appropriate way to express this is to add two
> new "top-level" options:
> 
> - items: - const: qcom,kaanapali-pmic-glink - const: qcom,pmic-glink 
> - items: - const: qcom,glymur-pmic-glink - const: qcom,pmic-glink
> 
> This establishes them as new "base compatibles", which can be used
> for derived platforms, by just adding the relevant enums. The
> alternative is to add them to the first item (enum + qcom,pmic-
> glink) and then as soon as we have derivatives we split it out in
> enum + kaanapali + qcom,pmic-glink or enum + glymur + qcom,pmic-
> glink. But I think this would be cleaner.
> 
> Regards, Bjorn
>

I agree that it would make more sense to decouple Kaanapali and Glymur 
however I do have a few follow up questions on how that should look in 
the pmic-glink and glink aux drivers.

1. In v2 we had tried to define Kaanapali and Glymur separately but in 
the pmic-glink driver they were sharing the same data since they both 
have charger FW running on SOCCP instead of ADSP. Krzysztof had 
expressed here, 
https://lore.kernel.org/all/01f419cc-3236-48b9-bd51-e7db07d1e6fe@kernel.org/, 
that having separate Glymur and Kaanapali compatible strings but then 
having them both point to the pmic_glink_kaanapali_data basically infers 
that Kaanapali and Glymur were compatible. There were other issues with 
that binding so I'm not sure if that comment would still stand in this 
case if we create new Glymur and Kaanapali base compatibles. If it does 
would it make more sense if the pmic_glink_data was instead named so 
that it relates to the system that the chg fw is running on?
i.e
{ .compatible="qcom,glymur-pmic-glink",.data=&pmic_glink_soccp_data},
{ .compatible="qcom,kaanapali-pmic-glink",.data=&pmic_glink_soccp_data},
{ .compatible = "qcom,pmic-glink", .data = &pmic_glink_adsp_data },


2. With Glymur and Kaanapali separated we would also need to update the 
UCSI and Battmngr drivers compatible string lists.

In USCI driver we would need to do:

+ { .compatible ="qcom,glymur-pmic-glink", .data = &quirk_sm8450, },
+ { .compatible = "qcom,kaanapali-pmic-glink",.data = &quirk_sm8450, },
   { .compatible = "qcom,qcm6490-pmic-glink", .data = &quirk_sc8280xp, },
   { .compatible = "qcom,sc8180x-pmic-glink", .data = &quirk_sc8180x, },
   { .compatible = "qcom,sc8280xp-pmic-glink", .data= &quirk_sc8280xp, },
   { .compatible = "qcom,sm8350-pmic-glink", .data = &quirk_sc8180x, },
   { .compatible = "qcom,sm8450-pmic-glink", .data = &quirk_sm8450, },
   { .compatible = "qcom,sm8550-pmic-glink", .data = &quirk_sm8450, },


and in battmgr driver we would need to do:

+ { .compatible = "qcom,glymur-pmic-glink", .data = (void *) X1E80100 },
+ { .compatible = "qcom,kaanapali-pmic-glink", .data = (void *)SM8550 },
   { .compatible = "qcom,sc8180x-pmic-glink", .data = (void *)SC8280XP },
   { .compatible = "qcom,sc8280xp-pmic-glink", .data = (void 
*)SC8280XP},
   { .compatible = "qcom,sm8550-pmic-glink", .data = (void *)SM8550 },
   { .compatible = "qcom,x1e80100-pmic-glink", .data = (void *)X1E80100},


I'm wondering if instead of making changes to both the UCSI and battmngr 
drivers, does it make sense to just add a compatible for X1E to UCSI? I 
briefly discussed this approach in this response: 
https://lore.kernel.org/all/481ec137-87cf-4448-99e9-4a1477f4854d@oss.qualcomm.com/.
With this we could instead define Kaanapali as a fallback to sm8550 and 
Glymur as a fallback to the x1e

Binding would instead look something like this

--- a/Documentation/devicetree/bindings/soc/qcom/qcom,pmic-glink.yaml
+++ b/Documentation/devicetree/bindings/soc/qcom/qcom,pmic-glink.yaml
@@ -29,6 +29,7 @@ properties:
                 - qcom,sm8350-pmic-glink
                 - qcom,sm8450-pmic-glink
                 - qcom,sm8550-pmic-glink
+               - qcom,x1e80100-pmic-glink
             - const: qcom,pmic-glink
         - items:
             - enum:
@@ -38,12 +38,18 @@ properties:
             - const: qcom,pmic-glink
         - items:
             - enum:
+               - qcom,kaanapali-pmic-glink
                 - qcom,milos-pmic-glink
                 - qcom,sm8650-pmic-glink
                 - qcom,sm8750-pmic-glink
                 - qcom,x1e80100-pmic-glink
             - const: qcom,sm8550-pmic-glink
             - const: qcom,pmic-glink
+      - items:
+          - enum:
+              - qcom,glymur-pmic-glink
+          - const: qcom,x1e80100-pmic-glink
+          - const: qcom,pmic-glink

with the UCSI driver change just being:
   { .compatible = "qcom,qcm6490-pmic-glink", .data = &quirk_sc8280xp, },
   { .compatible = "qcom,sc8180x-pmic-glink", .data = &quirk_sc8180x, },
   { .compatible = "qcom,sc8280xp-pmic-glink", .data= &quirk_sc8280xp, },
   { .compatible = "qcom,sm8350-pmic-glink", .data = &quirk_sc8180x, },
   { .compatible = "qcom,sm8450-pmic-glink", .data = &quirk_sm8450, },
   { .compatible = "qcom,sm8550-pmic-glink", .data = &quirk_sm8450, },
+ { .compatible = "qcom,x1e80100-pmic-glink", .data = &quirk_sm8450, },


Thanks,
Anjelique

Re: [PATCH v3 1/2] dt-bindings: soc: qcom: qcom,pmic-glink: Add Kaanapali and Glymur compatibles

[Bjorn Andersson]

On Wed, Jan 07, 2026 at 02:36:49PM -0800, Anjelique Melendez wrote:
> 
> 
> On 12/22/2025 11:42 AM, Bjorn Andersson wrote:
> > On Tue, Dec 16, 2025 at 09:56:54PM -0800, Anjelique Melendez wrote:
> > > Kaanapali PMIC glink platforms support features exposed by the
> > > SM8550 PMIC glink interface. Document the Kaanpali compatible
> > > string to have fallback on SM8550.
> > > 
> > > Glymur PMIC glink platforms support features exposed by SM8550,
> > > X1E80100 and Kaanapali PMIC glink interfaces. Document the Glymur
> > > compatible string to have fallbacks on SM8550, X1E80100 and
> > > Kaanapali.
> > 
> > But while the Glymur PMIC GLINK interface does present (almost) the
> > same features as SM8550 and/or Kaanapali, I don't see them as
> > compatible. The protocol is different and if nothing else, as shown
> > by patch 2, the life cycle is different.
> > 
> > > 
> > > Signed-off-by: Anjelique Melendez
> > > <anjelique.melendez@oss.qualcomm.com> --- .../devicetree/bindings/
> > > soc/qcom/qcom,pmic-glink.yaml     | 8 ++++++++ 1 file changed, 8
> > > insertions(+)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,pmic-
> > > glink.yaml b/Documentation/devicetree/bindings/soc/qcom/qcom,pmic-
> > > glink.yaml index 7085bf88afab..302b338e7012 100644 --- a/
> > > Documentation/devicetree/bindings/soc/qcom/qcom,pmic-glink.yaml ++
> > > + b/Documentation/devicetree/bindings/soc/qcom/qcom,pmic-
> > > glink.yaml @@ -37,12 +37,20 @@ properties: - const: qcom,pmic-
> > > glink - items: - enum: +              - qcom,kaanapali-pmic-glink -
> > > qcom,milos-pmic-glink - qcom,sm8650-pmic-glink - qcom,sm8750-
> > > pmic-glink - qcom,x1e80100-pmic-glink - const: qcom,sm8550-pmic-
> > > glink - const: qcom,pmic-glink +      - items: +          - enum: +
> > > - qcom,glymur-pmic-glink +          - const:
> > > qcom,kaanapali-pmic-glink +          - const: qcom,x1e80100-pmic-
> > > glink +          - const: qcom,sm8550-pmic-glink +          -
> > > const: qcom,pmic-glink
> > 
> > So this would then result in:
> > 
> > compatible = "qcom,glymur-pmic-glink", "qcom,kaanapali-pmic-glink",
> > "qcom,x1e80100-pmic-glink", "qcom,sm8550-pmic-glink", "qcom,pmic-
> > glink";
> > 
> > Which tells the operating system that it should use the
> > implementation for the Glymur interface, and if that isn't known the
> > implementation for the Kaanapali interface, and if that isn't known
> > the implementation for the Hamoa interface, and if that isn't known
> > the implementation for the SM8550 interface, and if that isn't known
> > the generic "pmic glink" interface.
> > 
> > So, in an operating system that doesn't know how to interact with
> > Glymur, you have stated that Kaanapali is a good second choice. This
> > does accurately capture the soccp behavior, but looking at the
> > individual functions we're not compatible with Kaanapali.
> > 
> > If we're in an operating system that doesn't know about Glymur or
> > Kaanapali, you're stating that the Hamoa implementation is good. I
> > think this is accurate for several of the features, but the OS will
> > forever wait for the services to show up.
> > 
> > Etc...
> > 
> > As such, I think the appropriate way to express this is to add two
> > new "top-level" options:
> > 
> > - items: - const: qcom,kaanapali-pmic-glink - const: qcom,pmic-glink -
> > items: - const: qcom,glymur-pmic-glink - const: qcom,pmic-glink
> > 
> > This establishes them as new "base compatibles", which can be used
> > for derived platforms, by just adding the relevant enums. The
> > alternative is to add them to the first item (enum + qcom,pmic-
> > glink) and then as soon as we have derivatives we split it out in
> > enum + kaanapali + qcom,pmic-glink or enum + glymur + qcom,pmic-
> > glink. But I think this would be cleaner.
> > 
> > Regards, Bjorn
> > 
> 
> I agree that it would make more sense to decouple Kaanapali and Glymur
> however I do have a few follow up questions on how that should look in the
> pmic-glink and glink aux drivers.
> 
> 1. In v2 we had tried to define Kaanapali and Glymur separately but in the
> pmic-glink driver they were sharing the same data since they both have
> charger FW running on SOCCP instead of ADSP. Krzysztof had expressed here,
> https://lore.kernel.org/all/01f419cc-3236-48b9-bd51-e7db07d1e6fe@kernel.org/,
> that having separate Glymur and Kaanapali compatible strings but then having
> them both point to the pmic_glink_kaanapali_data basically infers that
> Kaanapali and Glymur were compatible.

My main take away from that email thread is Krzysztof is trying to
understand the situation, and isn't provided the explanation about how
the various firmware interfaces relates to each other - and hence what
should be marked as "compatible" with what.

> There were other issues with that
> binding so I'm not sure if that comment would still stand in this case if we
> create new Glymur and Kaanapali base compatibles. If it does would it make
> more sense if the pmic_glink_data was instead named so that it relates to
> the system that the chg fw is running on?
> i.e
> { .compatible="qcom,glymur-pmic-glink",.data=&pmic_glink_soccp_data},
> { .compatible="qcom,kaanapali-pmic-glink",.data=&pmic_glink_soccp_data},
> { .compatible = "qcom,pmic-glink", .data = &pmic_glink_adsp_data },
> 

Absolutely, the fact that the variable is called
"pmic_glink_sm8450_client_mask" doesn't help anyone to understand that
it means "yes we have battery, altmode, and ucsi".

In fact, those bit fields was introduced when we didn't support all
three clients across all targets. As the code looks today, that whole
mechanism should be thrown out (or we should introduce some system that
exercises it).

> 
> 2. With Glymur and Kaanapali separated we would also need to update the UCSI
> and Battmngr drivers compatible string lists.
> 
> In USCI driver we would need to do:
> 
> + { .compatible ="qcom,glymur-pmic-glink", .data = &quirk_sm8450, },
> + { .compatible = "qcom,kaanapali-pmic-glink",.data = &quirk_sm8450, },

Looks reasonable, but as you suggest below, better to add an Hamoa
compatible and rely on the fallback to that. This will also allow us to
stop claiming that Hamoa is compatible with SM8550 - just because they
have the same USCI quirks...

>   { .compatible = "qcom,qcm6490-pmic-glink", .data = &quirk_sc8280xp, },
>   { .compatible = "qcom,sc8180x-pmic-glink", .data = &quirk_sc8180x, },
>   { .compatible = "qcom,sc8280xp-pmic-glink", .data= &quirk_sc8280xp, },
>   { .compatible = "qcom,sm8350-pmic-glink", .data = &quirk_sc8180x, },
>   { .compatible = "qcom,sm8450-pmic-glink", .data = &quirk_sm8450, },
>   { .compatible = "qcom,sm8550-pmic-glink", .data = &quirk_sm8450, },
> 
> 
> and in battmgr driver we would need to do:
> 
> + { .compatible = "qcom,glymur-pmic-glink", .data = (void *) X1E80100 },
> + { .compatible = "qcom,kaanapali-pmic-glink", .data = (void *)SM8550 },

Looks good as well, and shows my point of why Glymur and Hamoa isn't
"compatible" with SM8550.

>   { .compatible = "qcom,sc8180x-pmic-glink", .data = (void *)SC8280XP },
>   { .compatible = "qcom,sc8280xp-pmic-glink", .data = (void *)SC8280XP},
>   { .compatible = "qcom,sm8550-pmic-glink", .data = (void *)SM8550 },
>   { .compatible = "qcom,x1e80100-pmic-glink", .data = (void *)X1E80100},
> 
> 
> I'm wondering if instead of making changes to both the UCSI and battmngr
> drivers, does it make sense to just add a compatible for X1E to UCSI? I
> briefly discussed this approach in this response: https://lore.kernel.org/all/481ec137-87cf-4448-99e9-4a1477f4854d@oss.qualcomm.com/.
> With this we could instead define Kaanapali as a fallback to sm8550 and
> Glymur as a fallback to the x1e
> 

Yes, this would be better.

> Binding would instead look something like this
> 
> --- a/Documentation/devicetree/bindings/soc/qcom/qcom,pmic-glink.yaml
> +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,pmic-glink.yaml
> @@ -29,6 +29,7 @@ properties:
>                 - qcom,sm8350-pmic-glink
>                 - qcom,sm8450-pmic-glink
>                 - qcom,sm8550-pmic-glink
> +               - qcom,x1e80100-pmic-glink
>             - const: qcom,pmic-glink
>         - items:
>             - enum:
> @@ -38,12 +38,18 @@ properties:
>             - const: qcom,pmic-glink
>         - items:
>             - enum:
> +               - qcom,kaanapali-pmic-glink
>                 - qcom,milos-pmic-glink
>                 - qcom,sm8650-pmic-glink
>                 - qcom,sm8750-pmic-glink
>                 - qcom,x1e80100-pmic-glink
>             - const: qcom,sm8550-pmic-glink
>             - const: qcom,pmic-glink
> +      - items:
> +          - enum:
> +              - qcom,glymur-pmic-glink
> +          - const: qcom,x1e80100-pmic-glink
> +          - const: qcom,pmic-glink

This matches my expectations (all three hunks).

PS. We should remove qcom,x1e80100-pmic-glink from the grouping with
qcom,sm8550-pmic-glink, but let's do that later.

> 
> with the UCSI driver change just being:
>   { .compatible = "qcom,qcm6490-pmic-glink", .data = &quirk_sc8280xp, },
>   { .compatible = "qcom,sc8180x-pmic-glink", .data = &quirk_sc8180x, },
>   { .compatible = "qcom,sc8280xp-pmic-glink", .data= &quirk_sc8280xp, },
>   { .compatible = "qcom,sm8350-pmic-glink", .data = &quirk_sc8180x, },
>   { .compatible = "qcom,sm8450-pmic-glink", .data = &quirk_sm8450, },
>   { .compatible = "qcom,sm8550-pmic-glink", .data = &quirk_sm8450, },
> + { .compatible = "qcom,x1e80100-pmic-glink", .data = &quirk_sm8450, },

Yes, that looks good and if we inherited the quirks from Hamoa, this
looks correct to me.

Regards,
Bjorn

> 
> 
> Thanks,
> Anjelique

[PATCH v3 2/2] soc: qcom: pmic_glink: Add charger PDR service path and service name to client data

[Anjelique Melendez]

Currently, the charger PD service path and service name are hard coded
however these paths are not guaranteed to be the same between PMICs. For
example, on Kaanapali and Glymur, Charger FW runs on SOCCP(another subsystem)
which does not have any specific charger PDs defined.

Define charger PDR service path and service name as client data so that
each PMIC generation can properly define these paths.

While at it, add the qcom,kaanapali-pmic-glink compatible string.

Signed-off-by: Anjelique Melendez <anjelique.melendez@oss.qualcomm.com>
---
 drivers/soc/qcom/pmic_glink.c | 65 ++++++++++++++++++++++-------------
 1 file changed, 41 insertions(+), 24 deletions(-)

diff --git a/drivers/soc/qcom/pmic_glink.c b/drivers/soc/qcom/pmic_glink.c
index 627f96ca322e..3ea8fec3b32b 100644
--- a/drivers/soc/qcom/pmic_glink.c
+++ b/drivers/soc/qcom/pmic_glink.c
@@ -23,13 +23,19 @@ enum {
 	PMIC_GLINK_CLIENT_UCSI,
 };
 
+struct pmic_glink_data {
+	unsigned long	client_mask;
+	const char	*charger_pdr_service_name;
+	const char	*charger_pdr_service_path;
+};
+
 struct pmic_glink {
 	struct device *dev;
 	struct pdr_handle *pdr;
 
 	struct rpmsg_endpoint *ept;
 
-	unsigned long client_mask;
+	const struct pmic_glink_data *data;
 
 	struct auxiliary_device altmode_aux;
 	struct auxiliary_device ps_aux;
@@ -292,7 +298,6 @@ static struct rpmsg_driver pmic_glink_rpmsg_driver = {
 
 static int pmic_glink_probe(struct platform_device *pdev)
 {
-	const unsigned long *match_data;
 	struct pdr_service *service;
 	struct pmic_glink *pg;
 	int ret;
@@ -309,12 +314,10 @@ static int pmic_glink_probe(struct platform_device *pdev)
 	spin_lock_init(&pg->client_lock);
 	mutex_init(&pg->state_lock);
 
-	match_data = (unsigned long *)of_device_get_match_data(&pdev->dev);
-	if (!match_data)
+	pg->data = of_device_get_match_data(&pdev->dev);
+	if (!pg->data)
 		return -EINVAL;
 
-	pg->client_mask = *match_data;
-
 	pg->pdr = pdr_handle_alloc(pmic_glink_pdr_callback, pg);
 	if (IS_ERR(pg->pdr)) {
 		ret = dev_err_probe(&pdev->dev, PTR_ERR(pg->pdr),
@@ -322,27 +325,30 @@ static int pmic_glink_probe(struct platform_device *pdev)
 		return ret;
 	}
 
-	if (pg->client_mask & BIT(PMIC_GLINK_CLIENT_UCSI)) {
+	if (pg->data->client_mask & BIT(PMIC_GLINK_CLIENT_UCSI)) {
 		ret = pmic_glink_add_aux_device(pg, &pg->ucsi_aux, "ucsi");
 		if (ret)
 			goto out_release_pdr_handle;
 	}
-	if (pg->client_mask & BIT(PMIC_GLINK_CLIENT_ALTMODE)) {
+	if (pg->data->client_mask & BIT(PMIC_GLINK_CLIENT_ALTMODE)) {
 		ret = pmic_glink_add_aux_device(pg, &pg->altmode_aux, "altmode");
 		if (ret)
 			goto out_release_ucsi_aux;
 	}
-	if (pg->client_mask & BIT(PMIC_GLINK_CLIENT_BATT)) {
+	if (pg->data->client_mask & BIT(PMIC_GLINK_CLIENT_BATT)) {
 		ret = pmic_glink_add_aux_device(pg, &pg->ps_aux, "power-supply");
 		if (ret)
 			goto out_release_altmode_aux;
 	}
 
-	service = pdr_add_lookup(pg->pdr, "tms/servreg", "msm/adsp/charger_pd");
-	if (IS_ERR(service)) {
-		ret = dev_err_probe(&pdev->dev, PTR_ERR(service),
-				    "failed adding pdr lookup for charger_pd\n");
-		goto out_release_aux_devices;
+	if (pg->data->charger_pdr_service_name && pg->data->charger_pdr_service_path) {
+		service = pdr_add_lookup(pg->pdr, pg->data->charger_pdr_service_name,
+					 pg->data->charger_pdr_service_path);
+		if (IS_ERR(service)) {
+			ret = dev_err_probe(&pdev->dev, PTR_ERR(service),
+					    "failed adding pdr lookup for charger_pd\n");
+			goto out_release_aux_devices;
+		}
 	}
 
 	mutex_lock(&__pmic_glink_lock);
@@ -352,13 +358,13 @@ static int pmic_glink_probe(struct platform_device *pdev)
 	return 0;
 
 out_release_aux_devices:
-	if (pg->client_mask & BIT(PMIC_GLINK_CLIENT_BATT))
+	if (pg->data->client_mask & BIT(PMIC_GLINK_CLIENT_BATT))
 		pmic_glink_del_aux_device(pg, &pg->ps_aux);
 out_release_altmode_aux:
-	if (pg->client_mask & BIT(PMIC_GLINK_CLIENT_ALTMODE))
+	if (pg->data->client_mask & BIT(PMIC_GLINK_CLIENT_ALTMODE))
 		pmic_glink_del_aux_device(pg, &pg->altmode_aux);
 out_release_ucsi_aux:
-	if (pg->client_mask & BIT(PMIC_GLINK_CLIENT_UCSI))
+	if (pg->data->client_mask & BIT(PMIC_GLINK_CLIENT_UCSI))
 		pmic_glink_del_aux_device(pg, &pg->ucsi_aux);
 out_release_pdr_handle:
 	pdr_handle_release(pg->pdr);
@@ -372,23 +378,34 @@ static void pmic_glink_remove(struct platform_device *pdev)
 
 	pdr_handle_release(pg->pdr);
 
-	if (pg->client_mask & BIT(PMIC_GLINK_CLIENT_BATT))
+	if (pg->data->client_mask & BIT(PMIC_GLINK_CLIENT_BATT))
 		pmic_glink_del_aux_device(pg, &pg->ps_aux);
-	if (pg->client_mask & BIT(PMIC_GLINK_CLIENT_ALTMODE))
+	if (pg->data->client_mask & BIT(PMIC_GLINK_CLIENT_ALTMODE))
 		pmic_glink_del_aux_device(pg, &pg->altmode_aux);
-	if (pg->client_mask & BIT(PMIC_GLINK_CLIENT_UCSI))
+	if (pg->data->client_mask & BIT(PMIC_GLINK_CLIENT_UCSI))
 		pmic_glink_del_aux_device(pg, &pg->ucsi_aux);
 
 	guard(mutex)(&__pmic_glink_lock);
 	__pmic_glink = NULL;
 }
 
-static const unsigned long pmic_glink_sm8450_client_mask = BIT(PMIC_GLINK_CLIENT_BATT) |
-							   BIT(PMIC_GLINK_CLIENT_ALTMODE) |
-							   BIT(PMIC_GLINK_CLIENT_UCSI);
+static const struct pmic_glink_data pmic_glink_kaanapali_data = {
+	.client_mask = BIT(PMIC_GLINK_CLIENT_BATT) |
+		       BIT(PMIC_GLINK_CLIENT_ALTMODE) |
+		       BIT(PMIC_GLINK_CLIENT_UCSI),
+};
+
+static const struct pmic_glink_data pmic_glink_sm8450_data = {
+	.client_mask = BIT(PMIC_GLINK_CLIENT_BATT) |
+		       BIT(PMIC_GLINK_CLIENT_ALTMODE) |
+		       BIT(PMIC_GLINK_CLIENT_UCSI),
+	.charger_pdr_service_name = "tms/servreg",
+	.charger_pdr_service_path = "msm/adsp/charger_pd",
+};
 
 static const struct of_device_id pmic_glink_of_match[] = {
-	{ .compatible = "qcom,pmic-glink", .data = &pmic_glink_sm8450_client_mask },
+	{ .compatible = "qcom,kaanapali-pmic-glink", .data = &pmic_glink_kaanapali_data },
+	{ .compatible = "qcom,pmic-glink", .data = &pmic_glink_sm8450_data },
 	{}
 };
 MODULE_DEVICE_TABLE(of, pmic_glink_of_match);
-- 
2.34.1

Re: [PATCH v3 2/2] soc: qcom: pmic_glink: Add charger PDR service path and service name to client data

[Konrad Dybcio]

On 12/17/25 6:56 AM, Anjelique Melendez wrote:
> Currently, the charger PD service path and service name are hard coded
> however these paths are not guaranteed to be the same between PMICs. For
> example, on Kaanapali and Glymur, Charger FW runs on SOCCP(another subsystem)
> which does not have any specific charger PDs defined.
> 
> Define charger PDR service path and service name as client data so that
> each PMIC generation can properly define these paths.
> 
> While at it, add the qcom,kaanapali-pmic-glink compatible string.
> 
> Signed-off-by: Anjelique Melendez <anjelique.melendez@oss.qualcomm.com>
> ---

Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>

Konrad