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

[Krzysztof Kozlowski]

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@xxxxxxxxxxxxxxxx>
> ---
>  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
>