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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <5uyhz4uv7ccsj7cevimqrsoywne6f5ax5j2t4uosz6a7due4ac@3x4ouutt5nwy>
Date: Tue, 28 Oct 2025 19:20:13 +0200
From: Abel Vesa <abel.vesa@...aro.org>
To: Anjelique Melendez <anjelique.melendez@....qualcomm.com>
Cc: andersson@...nel.org, konradybcio@...nel.org, robh@...nel.org, 
	krzk+dt@...nel.org, conor+dt@...nel.org, linux-arm-msm@...r.kernel.org, 
	devicetree@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 2/2] soc: qcom: pmic_glink: Add charger PDR service
 path and service name to client data

On 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@....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