[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <s6xfgmrv7axotpozgq43qxsflr7muzg4snff5coadkqbvfcofq@ktbqljjlue4l>
Date: Thu, 15 Jan 2026 22:00:55 +0200
From: Dmitry Baryshkov <dmitry.baryshkov@....qualcomm.com>
To: Krzysztof Kozlowski <krzk@...nel.org>
Cc: Anjelique Melendez <anjelique.melendez@....qualcomm.com>,
andersson@...nel.org, konradybcio@...nel.org, robh@...nel.org,
krzk+dt@...nel.org, conor+dt@...nel.org,
heikki.krogerus@...ux.intel.com, gregkh@...uxfoundation.org,
abel.vesa@...aro.org, linux-arm-msm@...r.kernel.org,
devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-usb@...r.kernel.org
Subject: Re: [PATCH v4 4/4] soc: qcom: pmic_glink: Add charger PDR service
path and service name to client data
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@....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
Powered by blists - more mailing lists