linux-kernel - Re: [PATCH] remoteproc: qcom: pas: Conclude the rename from adsp

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 for Android: free password hash cracker in your pocket

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

Message-ID: <v765tixx2jqqx5rxylqmtulhotxbld5xvjrvo376hyzelmgrop@mqr4fcqdcrfj>
Date: Thu, 5 Jun 2025 15:27:19 -0500
From: Bjorn Andersson <andersson@...nel.org>
To: Luca Weiss <luca@...aweiss.eu>
Cc: Bjorn Andersson <bjorn.andersson@....qualcomm.com>, 
	Mathieu Poirier <mathieu.poirier@...aro.org>, linux-arm-msm@...r.kernel.org, linux-remoteproc@...r.kernel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] remoteproc: qcom: pas: Conclude the rename from adsp

On Thu, Jun 05, 2025 at 06:01:07PM +0200, Luca Weiss wrote:
> Hi Bjorn,
> 
> Awesome to see this being cleaned up!
> 
> On 05-06-2025 5:23 p.m., Bjorn Andersson wrote:
> > The change that renamed the driver from "adsp" to "pas" didn't change
> > any of the implementation. The result is an aesthetic eyesore, and
> > confusing to many.
> > 
> > Conclude the rename of the driver, by updating function, structures and
> > variable names to match what the driver actually is. The "Hexagon v5" is
> > also dropped from the name and Kconfig, as this isn't correct either.
> > 
> > No functional change.
> > 
> > Fixes: 9e004f97161d ("remoteproc: qcom: Rename Hexagon v5 PAS driver")
> > Signed-off-by: Bjorn Andersson <bjorn.andersson@....qualcomm.com>
> > ---
> >   drivers/remoteproc/Kconfig          |  11 +-
> >   drivers/remoteproc/qcom_q6v5_adsp.c |  46 +--
> 
> Actually looking through that driver, it's not just adsp-pil but also
> supports cdsp-pil and wpss-pil, so long-term that should probably be renamed
> to qcom_q6v5_pil.c? Not for this patch though obviously.
> 

There's another "convention" at play here, the "pas" refers to the
secure service doing authentication. The alternative to this is to do
everything in Linux, which we conveniently refer to (here) as "pil".

Also, per the commit text the "q6v5" isn't accurate anymore, so perhaps
this should just be the qcom_pas driver - or the qcom_pil driver.

I think the change as proposed is a good middle ground, we get things
cleaned up and aligned without affecting anything outside the
implementation.

> >   drivers/remoteproc/qcom_q6v5_pas.c  | 617 ++++++++++++++++++------------------
> >   3 files changed, 334 insertions(+), 340 deletions(-)
> 
> <snip>
> 
> > diff --git a/drivers/remoteproc/qcom_q6v5_pas.c b/drivers/remoteproc/qcom_q6v5_pas.c
> > index b306f223127c452f8f2d85aa0fc98db2d684feae..b0fc372ff0a9e032d784b1a4403ffeea5d0f9a00 100644
> > --- a/drivers/remoteproc/qcom_q6v5_pas.c
> > +++ b/drivers/remoteproc/qcom_q6v5_pas.c
> > @@ -1,6 +1,6 @@
> >   // SPDX-License-Identifier: GPL-2.0-only
> >   /*
> > - * Qualcomm ADSP/SLPI Peripheral Image Loader for MSM8974 and MSM8996
> > + * Qualcomm Peripahal Authentication Service remoteproc driver
> 
> typo Peripahal
> 

Thank you

> >    *
> >    * Copyright (C) 2016 Linaro Ltd
> >    * Copyright (C) 2014 Sony Mobile Communications AB
[..]
> > -static int adsp_unprepare(struct rproc *rproc)
> > +static int qcom_pas_unprepare(struct rproc *rproc)
> >   {
> > -	struct qcom_adsp *adsp = rproc->priv;
> > +	struct qcom_pas *pas = rproc->priv;
> >   	/*
> > -	 * adsp_load() did pass pas_metadata to the SCM driver for storing
> > +	 * pas_load() did pass pas_metadata to the SCM driver for storing
> 
> qcom_pas_load?
> 

Of course, thanks for spotting those.

[..]
> > -static const struct of_device_id adsp_of_match[] = {
> > +static const struct of_device_id qcom_pas_of_match[] = {
> >   	{ .compatible = "qcom,msm8226-adsp-pil", .data = &msm8996_adsp_resource},
> >   	{ .compatible = "qcom,msm8953-adsp-pil", .data = &msm8996_adsp_resource},
> >   	{ .compatible = "qcom,msm8974-adsp-pil", .data = &adsp_resource_init},
> 
> Not really for this patch but shouldn't those compatibles also be -pas?
> 

Per the naming convention later adopted, they should have been "-pas".
But changing that would break backwards compatibility with exiting DT,
so we'll leave that as is.

Thanks,
Bjorn