linux-kernel - Re: [PATCH] soc: qcom: pmic_glink: Add support for SOCCP remoteproc channels

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: <ds6sirdrarxropjbf4h32lv3gjmmq6rprhzg2unijmsiozetzt@uukv6wckb7ko>
Date: Wed, 3 Sep 2025 02:57:15 +0300
From: Dmitry Baryshkov <dmitry.baryshkov@....qualcomm.com>
To: Anjelique Melendez <anjelique.melendez@....qualcomm.com>
Cc: andersson@...nel.org, konradybcio@...nel.org,
        linux-arm-msm@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] soc: qcom: pmic_glink: Add support for SOCCP remoteproc
 channels

On Tue, Sep 02, 2025 at 02:45:44PM -0700, Anjelique Melendez wrote:
> Add support for charger FW running on SOCCP by adding the
> "PMIC_RTR_SOCCP_APPS" channel name to the rpmsg_match list and
> updating notify_clients logic.
> 
> SOCCP does not have multiple PDs and hence PDR is not supported. So, if
> the subsystem comes up/down, rpmsg driver would be probed or removed.
> Use that for notifiying clients of pmic_glink for PDR events.
> 
> Signed-off-by: Anjelique Melendez <anjelique.melendez@....qualcomm.com>
> ---
>  drivers/soc/qcom/pmic_glink.c | 28 +++++++++++++++++++++-------
>  1 file changed, 21 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/soc/qcom/pmic_glink.c b/drivers/soc/qcom/pmic_glink.c
> index c0a4be5df926..bcd17fc05544 100644
> --- a/drivers/soc/qcom/pmic_glink.c
> +++ b/drivers/soc/qcom/pmic_glink.c
> @@ -17,6 +17,11 @@
>  
>  #define PMIC_GLINK_SEND_TIMEOUT (5 * HZ)
>  
> +enum {
> +	PMIC_GLINK_PDR_UNAVAIL = 0,
> +	PMIC_GLINK_PDR_AVAIL,

No need for this, just use boolean instead.

> +};
> +
>  enum {
>  	PMIC_GLINK_CLIENT_BATT = 0,
>  	PMIC_GLINK_CLIENT_ALTMODE,
> @@ -39,6 +44,7 @@ struct pmic_glink {
>  	struct mutex state_lock;
>  	unsigned int client_state;
>  	unsigned int pdr_state;
> +	bool pdr_available;
>  
>  	/* serializing clients list updates */
>  	spinlock_t client_lock;
> @@ -203,17 +209,17 @@ static void pmic_glink_del_aux_device(struct pmic_glink *pg,
>  	auxiliary_device_uninit(aux);
>  }
>  
> -static void pmic_glink_state_notify_clients(struct pmic_glink *pg)
> +static void pmic_glink_state_notify_clients(struct pmic_glink *pg, unsigned int state)
>  {
>  	struct pmic_glink_client *client;
>  	unsigned int new_state = pg->client_state;
>  	unsigned long flags;
>  
>  	if (pg->client_state != SERVREG_SERVICE_STATE_UP) {
> -		if (pg->pdr_state == SERVREG_SERVICE_STATE_UP && pg->ept)
> +		if (state == SERVREG_SERVICE_STATE_UP && pg->ept)
>  			new_state = SERVREG_SERVICE_STATE_UP;
>  	} else {
> -		if (pg->pdr_state == SERVREG_SERVICE_STATE_DOWN || !pg->ept)
> +		if (state == SERVREG_SERVICE_STATE_DOWN || !pg->ept)
>  			new_state = SERVREG_SERVICE_STATE_DOWN;
>  	}
>  
> @@ -233,7 +239,7 @@ static void pmic_glink_pdr_callback(int state, char *svc_path, void *priv)
>  	guard(mutex)(&pg->state_lock);
>  	pg->pdr_state = state;
>  
> -	pmic_glink_state_notify_clients(pg);
> +	pmic_glink_state_notify_clients(pg, state);
>  }
>  
>  static int pmic_glink_rpmsg_probe(struct rpmsg_device *rpdev)
> @@ -246,10 +252,14 @@ static int pmic_glink_rpmsg_probe(struct rpmsg_device *rpdev)
>  		return dev_err_probe(&rpdev->dev, -ENODEV, "no pmic_glink device to attach to\n");
>  
>  	dev_set_drvdata(&rpdev->dev, pg);
> +	pg->pdr_available = rpdev->id.driver_data;
>  
>  	guard(mutex)(&pg->state_lock);
>  	pg->ept = rpdev->ept;
> -	pmic_glink_state_notify_clients(pg);
> +	if (pg->pdr_available)

I think it would be easier to do:

	if (!pg->pdr_available)
		pg->pdr_state = SERVREG_SERVICE_STATE_UP;

	pmic_glink_state_notify_clients(pg, pg->pdr_state);

> +		pmic_glink_state_notify_clients(pg, pg->pdr_state);
> +	else
> +		pmic_glink_state_notify_clients(pg, SERVREG_SERVICE_STATE_UP);
>  
>  	return 0;
>  }
> @@ -265,11 +275,15 @@ static void pmic_glink_rpmsg_remove(struct rpmsg_device *rpdev)
>  
>  	guard(mutex)(&pg->state_lock);
>  	pg->ept = NULL;
> -	pmic_glink_state_notify_clients(pg);
> +	if (pg->pdr_available)
> +		pmic_glink_state_notify_clients(pg, pg->pdr_state);
> +	else
> +		pmic_glink_state_notify_clients(pg, SERVREG_SERVICE_STATE_DOWN);

likewise

>  }
>  
>  static const struct rpmsg_device_id pmic_glink_rpmsg_id_match[] = {
> -	{ "PMIC_RTR_ADSP_APPS" },
> +	{.name = "PMIC_RTR_ADSP_APPS", .driver_data = PMIC_GLINK_PDR_AVAIL },
> +	{.name = "PMIC_RTR_SOCCP_APPS", .driver_data = PMIC_GLINK_PDR_UNAVAIL },
>  	{}
>  };
>  
> -- 
> 2.34.1
> 

-- 
With best wishes
Dmitry