On Thu, Aug 21, 2025 at 03:36:26PM +0100, Bryan O'Donoghue wrote:
> On 19/08/2025 17:54, Mukesh Ojha wrote:
> > Simplify qcom_scm_pas_init_image() by making the memory
> > allocation, copy and free work in a separate function
> > then the actual SMC call.
>
> then is temporal
> than is disjunctive
>
> you mean than here, not then.
Thanks, its a typo.
>
> >
> > Signed-off-by: Mukesh Ojha <mukesh.o...@oss.qualcomm.com>
> > ---
> > drivers/firmware/qcom/qcom_scm.c | 59 ++++++++++++++++++--------------
> > 1 file changed, 34 insertions(+), 25 deletions(-)
> >
> > diff --git a/drivers/firmware/qcom/qcom_scm.c
> > b/drivers/firmware/qcom/qcom_scm.c
> > index 9a5b34f5bacb..7827699e277c 100644
> > --- a/drivers/firmware/qcom/qcom_scm.c
> > +++ b/drivers/firmware/qcom/qcom_scm.c
> > @@ -584,6 +584,38 @@ void *qcom_scm_pas_ctx_init(struct device *dev, u32
> > peripheral, phys_addr_t mem_
> > }
> > EXPORT_SYMBOL_GPL(qcom_scm_pas_ctx_init);
> > +static int __qcom_scm_pas_init_image(u32 peripheral, dma_addr_t mdata_phys,
> > + void *metadata, size_t size,
> > + struct qcom_scm_res *res)
> > +{
> > + int ret;
> > + struct qcom_scm_desc desc = {
> > + .svc = QCOM_SCM_SVC_PIL,
> > + .cmd = QCOM_SCM_PIL_PAS_INIT_IMAGE,
> > + .arginfo = QCOM_SCM_ARGS(2, QCOM_SCM_VAL, QCOM_SCM_RW),
> > + .args[0] = peripheral,
> > + .owner = ARM_SMCCC_OWNER_SIP,
> > + };
>
> A minor detail but please reverse christmas tree your defintions and try to
> make int ret come last.
Sure.
>
> > +
> > + ret = qcom_scm_clk_enable();
> > + if (ret)
> > + return ret;
> > +
> > + ret = qcom_scm_bw_enable();
> > + if (ret)
> > + goto disable_clk;
> > +
> > + desc.args[1] = mdata_phys;
> > +
> > + ret = qcom_scm_call(__scm->dev, &desc, res);
> > + qcom_scm_bw_disable();
> > +
> > +disable_clk:
> > + qcom_scm_clk_disable();
> > +
> > + return ret;
> > +}
> > +
> > /**
> > * qcom_scm_pas_init_image() - Initialize peripheral authentication
> > service
> > * state machine for a given peripheral,
> > using the
> > @@ -604,17 +636,10 @@ EXPORT_SYMBOL_GPL(qcom_scm_pas_ctx_init);
> > int qcom_scm_pas_init_image(u32 peripheral, const void *metadata, size_t
> > size,
> > struct qcom_scm_pas_metadata *ctx)
> > {
> > + struct qcom_scm_res res;
> > dma_addr_t mdata_phys;
> > void *mdata_buf;
> > int ret;
> > - struct qcom_scm_desc desc = {
> > - .svc = QCOM_SCM_SVC_PIL,
> > - .cmd = QCOM_SCM_PIL_PAS_INIT_IMAGE,
> > - .arginfo = QCOM_SCM_ARGS(2, QCOM_SCM_VAL, QCOM_SCM_RW),
> > - .args[0] = peripheral,
> > - .owner = ARM_SMCCC_OWNER_SIP,
> > - };
> > - struct qcom_scm_res res;
> > /*
> > * During the scm call memory protection will be enabled for the meta
> > @@ -635,23 +660,7 @@ int qcom_scm_pas_init_image(u32 peripheral, const void
> > *metadata, size_t size,
> > memcpy(mdata_buf, metadata, size);
> > - ret = qcom_scm_clk_enable();
> > - if (ret)
> > - goto out;
> > -
> > - ret = qcom_scm_bw_enable();
> > - if (ret)
> > - goto disable_clk;
> > -
> > - desc.args[1] = mdata_phys;
> > -
> > - ret = qcom_scm_call(__scm->dev, &desc, &res);
> > - qcom_scm_bw_disable();
> > -
> > -disable_clk:
> > - qcom_scm_clk_disable();
> > -
> > -out:
> > + ret = __qcom_scm_pas_init_image(peripheral, mdata_phys, mdata_buf,
> > size, &res);
> > if (ret < 0 || !ctx) {
> > dma_free_coherent(__scm->dev, size, mdata_buf, mdata_phys);
> > } else if (ctx) {
>
> With those changes.
>
> Reviewed-by: Bryan O'Donoghue <bryan.odonog...@linaro.org>
--
-Mukesh Ojha
