| Message ID | 20240820055618.267554-6-quic_gokulsri@quicinc.com |
|---|---|
| State | New |
| Headers | show |
| Series | remove unnecessary q6 clocks | expand |
On Tue, Aug 20, 2024 at 11:26:18AM GMT, Gokul Sriram Palanisamy wrote: > From: Manikanta Mylavarapu <quic_mmanikan@quicinc.com> > > IPQ5332 security software running under trustzone > requires metadata size. With V2 cmd, pass metadata > size as well. Documentation says commit messages should be wrapped at 75 characters, not 50... Please improve the second sentence here, "v2 cmd" is coming out of nowhere. Say that there is a new command with a size parameter added. Is this operation available on all targets, or is it IPQ-specific? I don't see the relationship between this patch and the cover letter subject "remove unnecessary q6 clocks". Should this have been send on its own? > > Signed-off-by: Manikanta Mylavarapu <quic_mmanikan@quicinc.com> > Signed-off-by: Gokul Sriram Palanisamy <quic_gokulsri@quicinc.com> > --- > Changes in v7: > - No changes. > - Rebased on top of linux-next. > > Changes in v6: > - Rebased on linux-next > > Changes in v5: > - Rebased on linux-next > > Changes in v4: > - Rebased on linux-next > > drivers/firmware/qcom/qcom_scm.c | 8 ++++++++ > drivers/firmware/qcom/qcom_scm.h | 1 + > 2 files changed, 9 insertions(+) > > diff --git a/drivers/firmware/qcom/qcom_scm.c b/drivers/firmware/qcom/qcom_scm.c > index e60bef68401c..aa559fd01932 100644 > --- a/drivers/firmware/qcom/qcom_scm.c > +++ b/drivers/firmware/qcom/qcom_scm.c > @@ -607,6 +607,14 @@ int qcom_scm_pas_init_image(u32 peripheral, const void *metadata, size_t size, > > desc.args[1] = mdata_phys; > > + if (__qcom_scm_is_call_available(__scm->dev, QCOM_SCM_SVC_PIL, > + QCOM_SCM_PAS_INIT_IMAGE_V2)) { > + desc.cmd = QCOM_SCM_PAS_INIT_IMAGE_V2; > + desc.arginfo = > + QCOM_SCM_ARGS(3, QCOM_SCM_VAL, QCOM_SCM_RW, QCOM_SCM_VAL); > + desc.args[2] = size; > + } > + Please avoid default initialization and then conditionally overwrite parts of the values. Make a clear: if (v2 availble) { prepare v2 request; } else { prepare v1 request; } Regards, Bjorn > ret = qcom_scm_call(__scm->dev, &desc, &res); > qcom_scm_bw_disable(); > > diff --git a/drivers/firmware/qcom/qcom_scm.h b/drivers/firmware/qcom/qcom_scm.h > index 685b8f59e7a6..008b59cbad36 100644 > --- a/drivers/firmware/qcom/qcom_scm.h > +++ b/drivers/firmware/qcom/qcom_scm.h > @@ -96,6 +96,7 @@ struct qcom_tzmem_pool *qcom_scm_get_tzmem_pool(void); > > #define QCOM_SCM_SVC_PIL 0x02 > #define QCOM_SCM_PIL_PAS_INIT_IMAGE 0x01 > +#define QCOM_SCM_PAS_INIT_IMAGE_V2 0x1a > #define QCOM_SCM_PIL_PAS_MEM_SETUP 0x02 > #define QCOM_SCM_PIL_PAS_AUTH_AND_RESET 0x05 > #define QCOM_SCM_PIL_PAS_SHUTDOWN 0x06 > -- > 2.34.1 >
On 10/23/2024 9:22 PM, Bjorn Andersson wrote: > On Tue, Aug 20, 2024 at 11:26:18AM GMT, Gokul Sriram Palanisamy wrote: >> From: Manikanta Mylavarapu <quic_mmanikan@quicinc.com> >> >> IPQ5332 security software running under trustzone >> requires metadata size. With V2 cmd, pass metadata >> size as well. > Documentation says commit messages should be wrapped at 75 characters, > not 50... > > Please improve the second sentence here, "v2 cmd" is coming out of > nowhere. Say that there is a new command with a size parameter added. > > Is this operation available on all targets, or is it IPQ-specific? > > > I don't see the relationship between this patch and the cover letter > subject "remove unnecessary q6 clocks". Should this have been send on > its own? > >> Signed-off-by: Manikanta Mylavarapu <quic_mmanikan@quicinc.com> >> Signed-off-by: Gokul Sriram Palanisamy <quic_gokulsri@quicinc.com> >> --- >> Changes in v7: >> - No changes. >> - Rebased on top of linux-next. >> >> Changes in v6: >> - Rebased on linux-next >> >> Changes in v5: >> - Rebased on linux-next >> >> Changes in v4: >> - Rebased on linux-next >> >> drivers/firmware/qcom/qcom_scm.c | 8 ++++++++ >> drivers/firmware/qcom/qcom_scm.h | 1 + >> 2 files changed, 9 insertions(+) >> >> diff --git a/drivers/firmware/qcom/qcom_scm.c b/drivers/firmware/qcom/qcom_scm.c >> index e60bef68401c..aa559fd01932 100644 >> --- a/drivers/firmware/qcom/qcom_scm.c >> +++ b/drivers/firmware/qcom/qcom_scm.c >> @@ -607,6 +607,14 @@ int qcom_scm_pas_init_image(u32 peripheral, const void *metadata, size_t size, >> >> desc.args[1] = mdata_phys; >> >> + if (__qcom_scm_is_call_available(__scm->dev, QCOM_SCM_SVC_PIL, >> + QCOM_SCM_PAS_INIT_IMAGE_V2)) { >> + desc.cmd = QCOM_SCM_PAS_INIT_IMAGE_V2; >> + desc.arginfo = >> + QCOM_SCM_ARGS(3, QCOM_SCM_VAL, QCOM_SCM_RW, QCOM_SCM_VAL); >> + desc.args[2] = size; >> + } >> + > Please avoid default initialization and then conditionally overwrite > parts of the values. Make a clear: > > if (v2 availble) { > prepare v2 request; > } else { > prepare v1 request; > } > > Regards, > Bjorn sure, will address. Thank you. Regards, Gokul >> ret = qcom_scm_call(__scm->dev, &desc, &res); >> qcom_scm_bw_disable(); >> >> diff --git a/drivers/firmware/qcom/qcom_scm.h b/drivers/firmware/qcom/qcom_scm.h >> index 685b8f59e7a6..008b59cbad36 100644 >> --- a/drivers/firmware/qcom/qcom_scm.h >> +++ b/drivers/firmware/qcom/qcom_scm.h >> @@ -96,6 +96,7 @@ struct qcom_tzmem_pool *qcom_scm_get_tzmem_pool(void); >> >> #define QCOM_SCM_SVC_PIL 0x02 >> #define QCOM_SCM_PIL_PAS_INIT_IMAGE 0x01 >> +#define QCOM_SCM_PAS_INIT_IMAGE_V2 0x1a >> #define QCOM_SCM_PIL_PAS_MEM_SETUP 0x02 >> #define QCOM_SCM_PIL_PAS_AUTH_AND_RESET 0x05 >> #define QCOM_SCM_PIL_PAS_SHUTDOWN 0x06 >> -- >> 2.34.1 >>
diff --git a/drivers/firmware/qcom/qcom_scm.c b/drivers/firmware/qcom/qcom_scm.c index e60bef68401c..aa559fd01932 100644 --- a/drivers/firmware/qcom/qcom_scm.c +++ b/drivers/firmware/qcom/qcom_scm.c @@ -607,6 +607,14 @@ int qcom_scm_pas_init_image(u32 peripheral, const void *metadata, size_t size, desc.args[1] = mdata_phys; + if (__qcom_scm_is_call_available(__scm->dev, QCOM_SCM_SVC_PIL, + QCOM_SCM_PAS_INIT_IMAGE_V2)) { + desc.cmd = QCOM_SCM_PAS_INIT_IMAGE_V2; + desc.arginfo = + QCOM_SCM_ARGS(3, QCOM_SCM_VAL, QCOM_SCM_RW, QCOM_SCM_VAL); + desc.args[2] = size; + } + ret = qcom_scm_call(__scm->dev, &desc, &res); qcom_scm_bw_disable(); diff --git a/drivers/firmware/qcom/qcom_scm.h b/drivers/firmware/qcom/qcom_scm.h index 685b8f59e7a6..008b59cbad36 100644 --- a/drivers/firmware/qcom/qcom_scm.h +++ b/drivers/firmware/qcom/qcom_scm.h @@ -96,6 +96,7 @@ struct qcom_tzmem_pool *qcom_scm_get_tzmem_pool(void); #define QCOM_SCM_SVC_PIL 0x02 #define QCOM_SCM_PIL_PAS_INIT_IMAGE 0x01 +#define QCOM_SCM_PAS_INIT_IMAGE_V2 0x1a #define QCOM_SCM_PIL_PAS_MEM_SETUP 0x02 #define QCOM_SCM_PIL_PAS_AUTH_AND_RESET 0x05 #define QCOM_SCM_PIL_PAS_SHUTDOWN 0x06