Currently, the charger PD service path and service name are hard coded
however these paths are not guaranteed to be the same between SOCs. 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 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 627f96ca322e..3042261578aa 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,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_adsp_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_soccp_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_soccp_data },
+ { .compatible = "qcom,kaanapali-pmic-glink", .data = &pmic_glink_soccp_data },
+ { .compatible = "qcom,pmic-glink", .data = &pmic_glink_adsp_data },
{}
};
MODULE_DEVICE_TABLE(of, pmic_glink_of_match);
--
2.34.1On Wed, Jan 14, 2026 at 01:17:59PM -0800, 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 SOCs. For
> example, on Kaanapali and Glymur, Charger FW runs on SOCCP(another subsystem)
None of your commits are properly wrapped. Please use standard IDE/SW
editing tools which solve all such nits. You really should not have
received such review.
> 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 qcom,kaanapali-pmic-glink and qcom,glymur-pmic-glink
> compatible strings.
This is confusing. You either do the changes because something is not
correct OR you do them because they are part of Kaanapali/Glymur. Fixing
a bug AND adding new support are two separate commits.
Find the real rationale wahy you are doing this.
>
> Signed-off-by: Anjelique Melendez <anjelique.melendez@oss.qualcomm.com>
> ---
> drivers/soc/qcom/pmic_glink.c | 66 ++++++++++++++++++++++-------------
> 1 file changed, 42 insertions(+), 24 deletions(-)
...
> -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_adsp_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_soccp_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_soccp_data },
> + { .compatible = "qcom,kaanapali-pmic-glink", .data = &pmic_glink_soccp_data },
So these two are compatible? This should be somewhere clarified.
> + { .compatible = "qcom,pmic-glink", .data = &pmic_glink_adsp_data },
> {}
> };
> MODULE_DEVICE_TABLE(of, pmic_glink_of_match);
> --
> 2.34.1
>
On Thu, Jan 15, 2026 at 10:19:20AM +0100, Krzysztof Kozlowski wrote:
> On Wed, Jan 14, 2026 at 01:17:59PM -0800, 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 SOCs. For
> > example, on Kaanapali and Glymur, Charger FW runs on SOCCP(another subsystem)
>
> None of your commits are properly wrapped. Please use standard IDE/SW
> editing tools which solve all such nits. You really should not have
> received such review.
>
> > 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 qcom,kaanapali-pmic-glink and qcom,glymur-pmic-glink
> > compatible strings.
>
> This is confusing. You either do the changes because something is not
> correct OR you do them because they are part of Kaanapali/Glymur. Fixing
> a bug AND adding new support are two separate commits.
>
> Find the real rationale wahy you are doing this.
>
> >
> > Signed-off-by: Anjelique Melendez <anjelique.melendez@oss.qualcomm.com>
> > ---
> > drivers/soc/qcom/pmic_glink.c | 66 ++++++++++++++++++++++-------------
> > 1 file changed, 42 insertions(+), 24 deletions(-)
>
> > 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_soccp_data },
> > + { .compatible = "qcom,kaanapali-pmic-glink", .data = &pmic_glink_soccp_data },
>
> So these two are compatible? This should be somewhere clarified.
I think a lot of questions (both from the patch authors and patch
reviewers) come from the fact that the actual data is spread between
several drivers (this one, UCSI, charger). I'll take a look at pushing
all the data here and then necessary bits down to aux drivers.
>
> > + { .compatible = "qcom,pmic-glink", .data = &pmic_glink_adsp_data },
> > {}
> > };
> > MODULE_DEVICE_TABLE(of, pmic_glink_of_match);
> > --
> > 2.34.1
> >
--
With best wishes
Dmitry
On Wed, Jan 14, 2026 at 01:17:59PM -0800, 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 SOCs. 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 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(-) > Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com> -- With best wishes Dmitry
© 2016 - 2026 Red Hat, Inc.