[PATCH 4/6] qcom-scm: add ocmem support

[Rob Clark]

On Tue, Sep 29, 2015 at 6:33 PM, Stephen Boyd <sboyd at codeaurora.org> wrote:
> On 09/29, Rob Clark wrote:
>> On Tue, Sep 29, 2015 at 5:38 PM, Stephen Boyd <sboyd at codeaurora.org> wrote:
>> > On 09/29, Rob Clark wrote:
>> >> diff --git a/drivers/firmware/qcom_scm-32.c b/drivers/firmware/qcom_scm-32.c
>> >> index c1e4325..e1ac97f 100644
>> >> --- a/drivers/firmware/qcom_scm-32.c
>> >> +++ b/drivers/firmware/qcom_scm-32.c
>> >> @@ -500,6 +500,59 @@ int __qcom_scm_hdcp_req(struct qcom_scm_hdcp_req *req, u32 req_cnt, u32 *resp)
>> >>               req, req_cnt * sizeof(*req), resp, sizeof(*resp));
>> >>  }
>> >>
>> >> +int __qcom_scm_ocmem_secure_cfg(unsigned sec_id)
>> >> +{
>> >> +     int ret, scm_ret = 0;
>> >> +     struct msm_scm_sec_cfg {
>> >
>> > We've left these as anonymous structs for things like
>> > qcom_scm_set_boot_addr(), maybe we should do the same here.
>> >
>> >> +             __le32 id;
>> >> +             __le32 spare;
>> >
>> > Also, the iommu driver would use this API and it uses this
>> > "spare" element, so perhaps this whole function should be renamed
>> > to be more generic and take two values. Downstream the function
>> > is called scm_restore_sec_cfg, so maybe something similar.  And
>> > the service id is MP for "memory protection", so
>> > QCOM_SCM_OCMEM_SECURE_SVC could be QCOM_SCM_MEMORY_PROTECTION?
>>
>> heh,
>>
>>     #define SCM_SVC_MP            0xC
>>     #define IOMMU_SECURE_CFG    2
>>
>> vs.
>>
>>     #define OCMEM_SECURE_SVC_ID 12
>>     #define OCMEM_SECURE_CFG_ID 0x2
>>
>> that wasn't obscure at all!
>
> :)
>
>>
>> Maybe then there is a better name than spare?  Looks like downstream
>> iommu calls it cb_num?
>
> Yeah I think that's the only use to indicate which context bank
> it is. Maybe we can have a single id configure API and a special
> iommu context bank API that both funnel into the same private two
> number API. Otherwise we have a bunch of callers passing 0 for
> the second argument because they don't care.

so fwiw, I went thru all the downstream scm_call() callers..  there
are a lot of callers to SCM_SVC_MP service (through a couple different
#defines), but most of them are different cmd-id's.  The ones using
SECURE_CFG (0x2) are:

  * dwc3_msm_restore_sec_config()
  * ocmem_restore_sec_program()
  * msm_iommu_sec_program_iommu()

so we have two points passing in zero for ctx-bank, one that does not.
I don't think it is worth having two API's to save hard-coding zero in
two places ;-)

BR,
-R

> --
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
> a Linux Foundation Collaborative Project