Hi Olof, On 06/19/2017 02:25 PM, Stanimir Varbanov wrote: > 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. > Currently the dependences are: VIDEO_QCOM_VENUS selects QCOM_MDT_LOADER QCOM_MDT_LOADER selects QCOM_SCM And I came to this, diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig index 9fca977ef18d..b8657c561eae 100644 --- a/drivers/soc/qcom/Kconfig +++ b/drivers/soc/qcom/Kconfig @@ -12,7 +12,7 @@ config QCOM_GSBI config QCOM_MDT_LOADER tristate - select QCOM_SCM + depends on QCOM_SCM || COMPILE_TEST config QCOM_PM bool "Qualcomm Power Management" -- 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
