Hi Olof, On 06/19/2017 08:35 AM, Olof Johansson wrote: > Hi, > > On Thu, Jun 8, 2017 at 8:23 AM, Stanimir Varbanov > <stanimir.varbanov@xxxxxxxxxx> wrote: >> Unfortunatly previous attempt to allow consumer drivers to >> use COMPILE_TEST option in Kconfig is not enough, because in the >> past the consumer drivers used 'depends on' Kconfig option but >> now they are using 'select' Kconfig option which means on non ARM >> arch'es compilation is triggered. Thus we need to move the ifdefery >> one level below by touching the private qcom_scm.h header. >> >> Cc: Stephen Boyd <sboyd@xxxxxxxxxxxxxx> >> Cc: Bjorn Andersson <bjorn.andersson@xxxxxxxxxx> >> Signed-off-by: Stanimir Varbanov <stanimir.varbanov@xxxxxxxxxx> >> --- >> This is second version of the patch with comments addressed: >> * proper identation for static inline functions >> * to avoid duplicating defines group them on top of the header >> >> The first version has been part of the venus driver patchset >> v8 and can be found at: >> https://patchwork.kernel.org/patch/9704275/ >> >> drivers/firmware/Kconfig | 2 +- >> drivers/firmware/qcom_scm.h | 122 ++++++++++++++++++++++++++++++++++++-------- >> include/linux/qcom_scm.h | 32 ------------ >> 3 files changed, 101 insertions(+), 55 deletions(-) >> >> diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig >> index 6e4ed5a9c6fd..480578c3691a 100644 >> --- a/drivers/firmware/Kconfig >> +++ b/drivers/firmware/Kconfig >> @@ -204,7 +204,7 @@ config FW_CFG_SYSFS_CMDLINE >> >> config QCOM_SCM >> bool >> - depends on ARM || ARM64 >> + depends on ARM || ARM64 || COMPILE_TEST >> select RESET_CONTROLLER >> >> config QCOM_SCM_32 >> diff --git a/drivers/firmware/qcom_scm.h b/drivers/firmware/qcom_scm.h >> index 9bea691f30fb..60689fc8a567 100644 >> --- a/drivers/firmware/qcom_scm.h >> +++ b/drivers/firmware/qcom_scm.h >> @@ -16,31 +16,20 @@ >> #define QCOM_SCM_BOOT_ADDR 0x1 >> #define QCOM_SCM_BOOT_ADDR_MC 0x11 >> #define QCOM_SCM_SET_REMOTE_STATE 0xa >> -extern int __qcom_scm_set_remote_state(struct device *dev, u32 state, u32 id); >> >> #define QCOM_SCM_FLAG_HLOS 0x01 >> #define QCOM_SCM_FLAG_COLDBOOT_MC 0x02 >> #define QCOM_SCM_FLAG_WARMBOOT_MC 0x04 >> -extern int __qcom_scm_set_warm_boot_addr(struct device *dev, void *entry, >> - const cpumask_t *cpus); >> -extern int __qcom_scm_set_cold_boot_addr(void *entry, const cpumask_t *cpus); >> >> #define QCOM_SCM_CMD_TERMINATE_PC 0x2 >> #define QCOM_SCM_FLUSH_FLAG_MASK 0x3 >> #define QCOM_SCM_CMD_CORE_HOTPLUGGED 0x10 >> -extern void __qcom_scm_cpu_power_down(u32 flags); >> >> #define QCOM_SCM_SVC_INFO 0x6 >> #define QCOM_IS_CALL_AVAIL_CMD 0x1 >> -extern int __qcom_scm_is_call_available(struct device *dev, u32 svc_id, >> - u32 cmd_id); >> >> #define QCOM_SCM_SVC_HDCP 0x11 >> #define QCOM_SCM_CMD_HDCP 0x01 >> -extern int __qcom_scm_hdcp_req(struct device *dev, >> - struct qcom_scm_hdcp_req *req, u32 req_cnt, u32 *resp); >> - >> -extern void __qcom_scm_init(void); >> >> #define QCOM_SCM_SVC_PIL 0x2 >> #define QCOM_SCM_PAS_INIT_IMAGE_CMD 0x1 >> @@ -49,6 +38,27 @@ extern void __qcom_scm_init(void); >> #define QCOM_SCM_PAS_SHUTDOWN_CMD 0x6 >> #define QCOM_SCM_PAS_IS_SUPPORTED_CMD 0x7 >> #define QCOM_SCM_PAS_MSS_RESET 0xa >> + >> +#define QCOM_SCM_SVC_MP 0xc >> +#define QCOM_SCM_RESTORE_SEC_CFG 2 >> + >> +#define QCOM_SCM_IOMMU_SECURE_PTBL_SIZE 3 >> +#define QCOM_SCM_IOMMU_SECURE_PTBL_INIT 4 >> + >> +#define QCOM_SCM_SVC_HDCP 0x11 >> +#define QCOM_SCM_CMD_HDCP 0x01 >> + >> +#if IS_ENABLED(CONFIG_ARM) || IS_ENABLED(CONFIG_ARM64) > > This isn't right -- this should be IS_ENABLED(QCOM_SCM) The idea was to use the __qcom_scm_xxx functions only on ARM and ARM64 and fallback to the empty stubs on every other architecture. > >> +extern int __qcom_scm_set_remote_state(struct device *dev, u32 state, u32 id); >> +extern int __qcom_scm_set_warm_boot_addr(struct device *dev, void *entry, >> + const cpumask_t *cpus); >> +extern int __qcom_scm_set_cold_boot_addr(void *entry, const cpumask_t *cpus); >> +extern void __qcom_scm_cpu_power_down(u32 flags); >> +extern int __qcom_scm_is_call_available(struct device *dev, u32 svc_id, >> + u32 cmd_id); >> +extern int __qcom_scm_hdcp_req(struct device *dev, >> + struct qcom_scm_hdcp_req *req, u32 req_cnt, u32 *resp); >> +extern void __qcom_scm_init(void); >> extern bool __qcom_scm_pas_supported(struct device *dev, u32 peripheral); >> extern int __qcom_scm_pas_init_image(struct device *dev, u32 peripheral, >> dma_addr_t metadata_phys); >> @@ -57,6 +67,85 @@ extern int __qcom_scm_pas_mem_setup(struct device *dev, u32 peripheral, >> extern int __qcom_scm_pas_auth_and_reset(struct device *dev, u32 peripheral); >> extern int __qcom_scm_pas_shutdown(struct device *dev, u32 peripheral); >> extern int __qcom_scm_pas_mss_reset(struct device *dev, bool reset); >> +extern int __qcom_scm_restore_sec_cfg(struct device *dev, u32 device_id, >> + u32 spare); >> +extern int __qcom_scm_iommu_secure_ptbl_size(struct device *dev, u32 spare, >> + size_t *size); >> +extern int __qcom_scm_iommu_secure_ptbl_init(struct device *dev, u64 addr, >> + u32 size, u32 spare); >> +#else /* !ARM and !ARM64 */ >> +static inline int __qcom_scm_set_remote_state(struct device *dev, u32 state, >> + u32 id) >> +{ >> + return -ENODEV; >> +} >> +static inline int __qcom_scm_set_warm_boot_addr(struct device *dev, void *entry, >> + const cpumask_t *cpus) >> +{ >> + return -ENODEV; >> +} >> +static inline int __qcom_scm_set_cold_boot_addr(void *entry, >> + const cpumask_t *cpus) >> +{ >> + return -ENODEV; >> +} >> +static inline void __qcom_scm_cpu_power_down(u32 flags) {} >> +static inline int __qcom_scm_is_call_available(struct device *dev, u32 svc_id, >> + u32 cmd_id) >> +{ >> + return -ENODEV; >> +} >> +static inline int __qcom_scm_hdcp_req(struct device *dev, >> + struct qcom_scm_hdcp_req *req, >> + u32 req_cnt, u32 *resp) >> +{ >> + return -ENODEV; >> +} >> +static inline void __qcom_scm_init(void) {} >> +static inline bool __qcom_scm_pas_supported(struct device *dev, u32 peripheral) >> +{ >> + return false; >> +} >> +static inline int __qcom_scm_pas_init_image(struct device *dev, u32 peripheral, >> + dma_addr_t metadata_phys) >> +{ >> + return -ENODEV; >> +} >> +static inline int __qcom_scm_pas_mem_setup(struct device *dev, u32 peripheral, >> + phys_addr_t addr, phys_addr_t size) >> +{ >> + return -ENODEV; >> +} >> +static inline int __qcom_scm_pas_auth_and_reset(struct device *dev, >> + u32 peripheral) >> +{ >> + return -ENODEV; >> +} >> +static inline int __qcom_scm_pas_shutdown(struct device *dev, u32 peripheral) >> +{ >> + return -ENODEV; >> +} >> +static inline int __qcom_scm_pas_mss_reset(struct device *dev, bool reset) >> +{ >> + return -ENODEV; >> +} >> +static inline int __qcom_scm_restore_sec_cfg(struct device *dev, u32 device_id, >> + u32 spare) >> +{ >> + return -ENODEV; >> +} >> +static inline int __qcom_scm_iommu_secure_ptbl_size(struct device *dev, >> + u32 spare, size_t *size) >> +{ >> + return -ENODEV; >> +} >> +static inline int __qcom_scm_iommu_secure_ptbl_init(struct device *dev, >> + u64 addr, u32 size, >> + u32 spare) >> +{ >> + return -ENODEV; >> +} > > The amount of boilerplate random stubs here indiciates that the > abstraction point for this is wrong. This was the best solution I came to. > > Why are you looking to COMPILE_TEST a very platform-tied driver like > this on other architectures? In fact the aim is to make possible to COMPILE_TEST drivers which are using QCOM_SCM interface. So that qcom_scm driver itself doesn't need compile test coverage, and compile testing qcom_scm is impossible on non-ARM architectures. > > It probably makes more sense to stub the driver->scm interface than > the internal scm interface if what you're looking for is driver > compile_test coverage. Actually, this is the state of qcom_scm if we don't apply the this patch, and it didn't help. Thinking more on that it looks like that adding COMPILE_TEST in 'config QCOM_SCM' is controversial. Arnd, Andy any ideas how to proceed. If this patch is not get merged (and I/we cannot find better solution) the video driver for qualcomm platforms will be rejected for 4.13. -- regards, Stan -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
