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 and
qcom,glymur-pmic-glink compatible strings.
Signed-off-by: Anjelique Melendez <anjelique.melendez@oss.qualcomm.com>
---
drivers/soc/qcom/pmic_glink.c | 66 ++++++++++++++++++++++-------------
1 file changed, 42 insertions(+), 24 deletions(-)
diff --git a/drivers/soc/qcom/pmic_glink.c b/drivers/soc/qcom/pmic_glink.c
index c0a4be5df926..aa5ba9a0285e 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;
+ char *charger_pdr_service_name;
+ 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;
@@ -285,7 +291,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;
@@ -302,12 +307,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),
@@ -315,27 +318,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);
@@ -345,13 +351,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);
@@ -365,23 +371,35 @@ 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_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 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 of_device_id pmic_glink_of_match[] = {
- { .compatible = "qcom,pmic-glink", .data = &pmic_glink_sm8450_client_mask },
+ { .compatible = "qcom,glymur-pmic-glink", .data = &pmic_glink_kaanapali_data },
+ { .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.1On 25-10-27 14:22:50, 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 and
> qcom,glymur-pmic-glink compatible strings.
>
> Signed-off-by: Anjelique Melendez <anjelique.melendez@oss.qualcomm.com>
> ---
> drivers/soc/qcom/pmic_glink.c | 66 ++++++++++++++++++++++-------------
> 1 file changed, 42 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/soc/qcom/pmic_glink.c b/drivers/soc/qcom/pmic_glink.c
> index c0a4be5df926..aa5ba9a0285e 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;
> + char *charger_pdr_service_name;
> + 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;
> @@ -285,7 +291,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;
> @@ -302,12 +307,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),
> @@ -315,27 +318,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;
> + }
> }
But this does nothing on Kaanapali and Glymur. Am I wrong?
Yes, you do not have a charger PD on Glymur, but you do have an ssr,
for which you do need to register a notifier instead.
You need to be doing something like this:
https://gitlab.com/Linaro/arm64-laptops/linux/-/commit/2cd84e303d263d8fd5de3730714a16c29cc6788b
On 10/28/2025 10:20 AM, Abel Vesa wrote:
> On 25-10-27 14:22:50, Anjelique Melendez wrote:
>> - 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;
>> + }
>> }
>
> But this does nothing on Kaanapali and Glymur. Am I wrong?
>
> Yes, you do not have a charger PD on Glymur, but you do have an ssr,
> for which you do need to register a notifier instead.
>
> You need to be doing something like this:
> https://gitlab.com/Linaro/arm64-laptops/linux/-/commit/2cd84e303d263d8fd5de3730714a16c29cc6788b
Please take a look at this change, just applied:
https://lore.kernel.org/all/20250919175025.2988948-1-anjelique.melendez@oss.qualcomm.com/.
On 25-10-28 16:30:28, Anjelique Melendez wrote:
>
>
> On 10/28/2025 10:20 AM, Abel Vesa wrote:
> > On 25-10-27 14:22:50, Anjelique Melendez wrote:
>
> > > - 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;
> > > + }
> > > }
> >
> > But this does nothing on Kaanapali and Glymur. Am I wrong?
> >
> > Yes, you do not have a charger PD on Glymur, but you do have an ssr,
> > for which you do need to register a notifier instead.
> >
> > You need to be doing something like this:
> > https://gitlab.com/Linaro/arm64-laptops/linux/-/commit/2cd84e303d263d8fd5de3730714a16c29cc6788b
>
> Please take a look at this change, just applied: https://lore.kernel.org/all/20250919175025.2988948-1-anjelique.melendez@oss.qualcomm.com/.
Fair enough. I think your approach is even cleaner.
Thanks.
On 10/27/25 10:22 PM, 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 and > qcom,glymur-pmic-glink compatible strings. > > Signed-off-by: Anjelique Melendez <anjelique.melendez@oss.qualcomm.com> > --- I think this change disqualifies Glymur from having a fallback to 8550, since it couldn't have worked without ignoring the PDR Konrad
© 2016 - 2026 Red Hat, Inc.