| Message ID | 20220701145815.2037993-2-bhupesh.sharma@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Daniel Lezcano |
| Headers | show |
| Series | Add support for tsens controller reinit via trustzone | expand |
On Fri 01 Jul 09:58 CDT 2022, Bhupesh Sharma wrote: Please update $subject to match the most uses prefix for the qcom_scm driver. > Some versions of QCoM tsens controller might enter a s/QCoM/Qualcomm/ please. > 'bad state' while running stability tests causing sensor > temperatures/interrupts status to be in an 'invalid' state. > > It is recommended to re-initialize the tsens controller > via trustzone (secure registers) using scm call(s) when that > happens. > > Add support for the same in the qcom_scm driver. > > Cc: Amit Kucheria <amitk@kernel.org> > Cc: Thara Gopinath <thara.gopinath@gmail.com> > Cc: linux-pm@vger.kernel.org > Cc: linux-arm-msm@vger.kernel.org > Signed-off-by: Bhupesh Sharma <bhupesh.sharma@linaro.org> > --- > drivers/firmware/qcom_scm.c | 17 +++++++++++++++++ > drivers/firmware/qcom_scm.h | 4 ++++ > include/linux/qcom_scm.h | 2 ++ > 3 files changed, 23 insertions(+) > > diff --git a/drivers/firmware/qcom_scm.c b/drivers/firmware/qcom_scm.c > index 3163660fa8e2..0bc7cc466218 100644 > --- a/drivers/firmware/qcom_scm.c > +++ b/drivers/firmware/qcom_scm.c > @@ -796,6 +796,23 @@ int qcom_scm_mem_protect_video_var(u32 cp_start, u32 cp_size, > } > EXPORT_SYMBOL(qcom_scm_mem_protect_video_var); > > +int qcom_scm_tsens_reinit(int *tsens_ret) > +{ > + unsigned int ret; qcom_scm_call() returns negative numbers on error, so this should be signed. > + struct qcom_scm_desc desc = { const? > + .svc = QCOM_SCM_SVC_TSENS, > + .cmd = QCOM_SCM_TSENS_INIT_ID, > + }; > + struct qcom_scm_res res; > + > + ret = qcom_scm_call(__scm->dev, &desc, &res); > + if (tsens_ret) > + *tsens_ret = res.result[0]; Most similar qcom_scm functions use negative return value for errors and positive (including 0) values for the returned data. Looking at patch 3, the only thing you seem to care about is tsens_ret being 0 or not, so I do think you would be fine returning both using the return value. Regards, Bjorn > + > + return ret; > +} > +EXPORT_SYMBOL(qcom_scm_tsens_reinit); > + > static int __qcom_scm_assign_mem(struct device *dev, phys_addr_t mem_region, > size_t mem_sz, phys_addr_t src, size_t src_sz, > phys_addr_t dest, size_t dest_sz) > diff --git a/drivers/firmware/qcom_scm.h b/drivers/firmware/qcom_scm.h > index 0d51eef2472f..495fa00230c7 100644 > --- a/drivers/firmware/qcom_scm.h > +++ b/drivers/firmware/qcom_scm.h > @@ -94,6 +94,10 @@ extern int scm_legacy_call(struct device *dev, const struct qcom_scm_desc *desc, > #define QCOM_SCM_PIL_PAS_IS_SUPPORTED 0x07 > #define QCOM_SCM_PIL_PAS_MSS_RESET 0x0a > > +/* TSENS Services and Function IDs */ > +#define QCOM_SCM_SVC_TSENS 0x1E > +#define QCOM_SCM_TSENS_INIT_ID 0x5 > + > #define QCOM_SCM_SVC_IO 0x05 > #define QCOM_SCM_IO_READ 0x01 > #define QCOM_SCM_IO_WRITE 0x02 > diff --git a/include/linux/qcom_scm.h b/include/linux/qcom_scm.h > index f8335644a01a..f8c9eb739df1 100644 > --- a/include/linux/qcom_scm.h > +++ b/include/linux/qcom_scm.h > @@ -124,4 +124,6 @@ extern int qcom_scm_lmh_dcvsh(u32 payload_fn, u32 payload_reg, u32 payload_val, > extern int qcom_scm_lmh_profile_change(u32 profile_id); > extern bool qcom_scm_lmh_dcvsh_available(void); > > +extern int qcom_scm_tsens_reinit(int *tsens_ret); > + > #endif > -- > 2.35.3 >
Hi Bjorn, Thanks for your review. On 7/19/22 8:28 AM, Bjorn Andersson wrote: > On Fri 01 Jul 09:58 CDT 2022, Bhupesh Sharma wrote: > > Please update $subject to match the most uses prefix for the qcom_scm > driver. > >> Some versions of QCoM tsens controller might enter a > > s/QCoM/Qualcomm/ please. Ok. >> 'bad state' while running stability tests causing sensor >> temperatures/interrupts status to be in an 'invalid' state. >> >> It is recommended to re-initialize the tsens controller >> via trustzone (secure registers) using scm call(s) when that >> happens. >> >> Add support for the same in the qcom_scm driver. >> >> Cc: Amit Kucheria <amitk@kernel.org> >> Cc: Thara Gopinath <thara.gopinath@gmail.com> >> Cc: linux-pm@vger.kernel.org >> Cc: linux-arm-msm@vger.kernel.org >> Signed-off-by: Bhupesh Sharma <bhupesh.sharma@linaro.org> >> --- >> drivers/firmware/qcom_scm.c | 17 +++++++++++++++++ >> drivers/firmware/qcom_scm.h | 4 ++++ >> include/linux/qcom_scm.h | 2 ++ >> 3 files changed, 23 insertions(+) >> >> diff --git a/drivers/firmware/qcom_scm.c b/drivers/firmware/qcom_scm.c >> index 3163660fa8e2..0bc7cc466218 100644 >> --- a/drivers/firmware/qcom_scm.c >> +++ b/drivers/firmware/qcom_scm.c >> @@ -796,6 +796,23 @@ int qcom_scm_mem_protect_video_var(u32 cp_start, u32 cp_size, >> } >> EXPORT_SYMBOL(qcom_scm_mem_protect_video_var); >> >> +int qcom_scm_tsens_reinit(int *tsens_ret) >> +{ >> + unsigned int ret; > > qcom_scm_call() returns negative numbers on error, so this should be > signed. Ok. >> + struct qcom_scm_desc desc = { > > const? Sure. >> + .svc = QCOM_SCM_SVC_TSENS, >> + .cmd = QCOM_SCM_TSENS_INIT_ID, >> + }; >> + struct qcom_scm_res res; >> + >> + ret = qcom_scm_call(__scm->dev, &desc, &res); >> + if (tsens_ret) >> + *tsens_ret = res.result[0]; > > Most similar qcom_scm functions use negative return value for errors and > positive (including 0) values for the returned data. > > Looking at patch 3, the only thing you seem to care about is tsens_ret > being 0 or not, so I do think you would be fine returning both using the > return value. Ok, let me double check the same and fix the same in v2. Regards, Bhupesh >> + >> + return ret; >> +} >> +EXPORT_SYMBOL(qcom_scm_tsens_reinit); >> + >> static int __qcom_scm_assign_mem(struct device *dev, phys_addr_t mem_region, >> size_t mem_sz, phys_addr_t src, size_t src_sz, >> phys_addr_t dest, size_t dest_sz) >> diff --git a/drivers/firmware/qcom_scm.h b/drivers/firmware/qcom_scm.h >> index 0d51eef2472f..495fa00230c7 100644 >> --- a/drivers/firmware/qcom_scm.h >> +++ b/drivers/firmware/qcom_scm.h >> @@ -94,6 +94,10 @@ extern int scm_legacy_call(struct device *dev, const struct qcom_scm_desc *desc, >> #define QCOM_SCM_PIL_PAS_IS_SUPPORTED 0x07 >> #define QCOM_SCM_PIL_PAS_MSS_RESET 0x0a >> >> +/* TSENS Services and Function IDs */ >> +#define QCOM_SCM_SVC_TSENS 0x1E >> +#define QCOM_SCM_TSENS_INIT_ID 0x5 >> + >> #define QCOM_SCM_SVC_IO 0x05 >> #define QCOM_SCM_IO_READ 0x01 >> #define QCOM_SCM_IO_WRITE 0x02 >> diff --git a/include/linux/qcom_scm.h b/include/linux/qcom_scm.h >> index f8335644a01a..f8c9eb739df1 100644 >> --- a/include/linux/qcom_scm.h >> +++ b/include/linux/qcom_scm.h >> @@ -124,4 +124,6 @@ extern int qcom_scm_lmh_dcvsh(u32 payload_fn, u32 payload_reg, u32 payload_val, >> extern int qcom_scm_lmh_profile_change(u32 profile_id); >> extern bool qcom_scm_lmh_dcvsh_available(void); >> >> +extern int qcom_scm_tsens_reinit(int *tsens_ret); >> + >> #endif >> -- >> 2.35.3 >>
diff --git a/drivers/firmware/qcom_scm.c b/drivers/firmware/qcom_scm.c index 3163660fa8e2..0bc7cc466218 100644 --- a/drivers/firmware/qcom_scm.c +++ b/drivers/firmware/qcom_scm.c @@ -796,6 +796,23 @@ int qcom_scm_mem_protect_video_var(u32 cp_start, u32 cp_size, } EXPORT_SYMBOL(qcom_scm_mem_protect_video_var); +int qcom_scm_tsens_reinit(int *tsens_ret) +{ + unsigned int ret; + struct qcom_scm_desc desc = { + .svc = QCOM_SCM_SVC_TSENS, + .cmd = QCOM_SCM_TSENS_INIT_ID, + }; + struct qcom_scm_res res; + + ret = qcom_scm_call(__scm->dev, &desc, &res); + if (tsens_ret) + *tsens_ret = res.result[0]; + + return ret; +} +EXPORT_SYMBOL(qcom_scm_tsens_reinit); + static int __qcom_scm_assign_mem(struct device *dev, phys_addr_t mem_region, size_t mem_sz, phys_addr_t src, size_t src_sz, phys_addr_t dest, size_t dest_sz) diff --git a/drivers/firmware/qcom_scm.h b/drivers/firmware/qcom_scm.h index 0d51eef2472f..495fa00230c7 100644 --- a/drivers/firmware/qcom_scm.h +++ b/drivers/firmware/qcom_scm.h @@ -94,6 +94,10 @@ extern int scm_legacy_call(struct device *dev, const struct qcom_scm_desc *desc, #define QCOM_SCM_PIL_PAS_IS_SUPPORTED 0x07 #define QCOM_SCM_PIL_PAS_MSS_RESET 0x0a +/* TSENS Services and Function IDs */ +#define QCOM_SCM_SVC_TSENS 0x1E +#define QCOM_SCM_TSENS_INIT_ID 0x5 + #define QCOM_SCM_SVC_IO 0x05 #define QCOM_SCM_IO_READ 0x01 #define QCOM_SCM_IO_WRITE 0x02 diff --git a/include/linux/qcom_scm.h b/include/linux/qcom_scm.h index f8335644a01a..f8c9eb739df1 100644 --- a/include/linux/qcom_scm.h +++ b/include/linux/qcom_scm.h @@ -124,4 +124,6 @@ extern int qcom_scm_lmh_dcvsh(u32 payload_fn, u32 payload_reg, u32 payload_val, extern int qcom_scm_lmh_profile_change(u32 profile_id); extern bool qcom_scm_lmh_dcvsh_available(void); +extern int qcom_scm_tsens_reinit(int *tsens_ret); + #endif
Some versions of QCoM tsens controller might enter a 'bad state' while running stability tests causing sensor temperatures/interrupts status to be in an 'invalid' state. It is recommended to re-initialize the tsens controller via trustzone (secure registers) using scm call(s) when that happens. Add support for the same in the qcom_scm driver. Cc: Amit Kucheria <amitk@kernel.org> Cc: Thara Gopinath <thara.gopinath@gmail.com> Cc: linux-pm@vger.kernel.org Cc: linux-arm-msm@vger.kernel.org Signed-off-by: Bhupesh Sharma <bhupesh.sharma@linaro.org> --- drivers/firmware/qcom_scm.c | 17 +++++++++++++++++ drivers/firmware/qcom_scm.h | 4 ++++ include/linux/qcom_scm.h | 2 ++ 3 files changed, 23 insertions(+)