From patchwork Mon Jun 19 12:19:58 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stanimir Varbanov X-Patchwork-Id: 9795821 X-Patchwork-Delegate: agross@codeaurora.org Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id AE8D0600C5 for ; Mon, 19 Jun 2017 12:20:04 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 9784528435 for ; Mon, 19 Jun 2017 12:20:04 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 8C29028453; Mon, 19 Jun 2017 12:20:04 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.8 required=2.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_HI,T_DKIM_INVALID autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id B70852844C for ; Mon, 19 Jun 2017 12:20:03 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754023AbdFSMUC (ORCPT ); Mon, 19 Jun 2017 08:20:02 -0400 Received: from mail-wr0-f181.google.com ([209.85.128.181]:36134 "EHLO mail-wr0-f181.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751108AbdFSMUB (ORCPT ); Mon, 19 Jun 2017 08:20:01 -0400 Received: by mail-wr0-f181.google.com with SMTP id c11so16120000wrc.3 for ; Mon, 19 Jun 2017 05:20:00 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=subject:to:references:cc:from:message-id:date:user-agent :mime-version:in-reply-to:content-transfer-encoding; bh=kNXKonX3wZbZAuuoNlQ9hKjeJAtwVPxJECY7LXYEJ8A=; b=i+8DS62C0KN1arGqUuvk/56yjDuE06RS8oQy6pMy0ufuPuRlxIehzTKG2rhjo5Rm6F 3uAktanrz9Xne72HK+RFHP9Hr7zZh9XTCfhcDZaqApAtuG8AfmRTmZhRfS7UJwCvaPqr XPSgXnSEHrEvN8t75Kk81TK77gKt+UrsKhn0o= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:references:cc:from:message-id:date :user-agent:mime-version:in-reply-to:content-transfer-encoding; bh=kNXKonX3wZbZAuuoNlQ9hKjeJAtwVPxJECY7LXYEJ8A=; b=KZp1Oxqy6/Qp7yldSZzBRNxWda+NChreg08KkaBUKpIGphzTdPnmF65ysunzoRiXye M1Ux49y72aMLYPSqO1Un2nGIiGf9ml0d1/OYmZaKP9WVCJql3Utb+2Bll4hyH+NEGeZs Kd/yhZkVdHUYFaVnp9t8McByfJLvinqvaq4p2OGL0GUq9Y3DzSROk58OojBvGeQIO+O1 ZPVnjFGk8G+N0XMvA7QD3zzDpyV5j8ERD6NX8VihwROyuRKWGB6EcVqWFsZBlFKsbZ7F NtxmqyyMwDvRoQ9NRuwo/yw5rBZ+m3kQu0Epn2fJGTiqVz+ljGUFcx+KI5Mfa5uCn9BJ FRfQ== X-Gm-Message-State: AKS2vOz22n57j1/EgkR4Rgg9HmLCFOCdlobJSz+vT8Mg16aKrAxRWpW+ oE/4tJk8P/mjxXlr X-Received: by 10.80.178.5 with SMTP id o5mr17040242edd.89.1497874799957; Mon, 19 Jun 2017 05:19:59 -0700 (PDT) Received: from [192.168.27.209] ([37.157.136.206]) by smtp.googlemail.com with ESMTPSA id b30sm5750840edd.6.2017.06.19.05.19.58 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 19 Jun 2017 05:19:59 -0700 (PDT) Subject: Re: [PATCH v2] firmware: qcom_scm: Fix to allow COMPILE_TEST-ing To: Stanimir Varbanov , Olof Johansson , Andy Gross , Arnd Bergmann References: <1496935418-14142-1-git-send-email-stanimir.varbanov@linaro.org> <9e0b85dd-3621-cd8c-7ec6-67e319cab071@linaro.org> Cc: "linux-kernel@vger.kernel.org" , linux-arm-msm , Stephen Boyd , Bjorn Andersson From: Stanimir Varbanov Message-ID: <58ebc743-953b-e2f4-08ca-fa1642f27a5b@linaro.org> Date: Mon, 19 Jun 2017 15:19:58 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.5.1 MIME-Version: 1.0 In-Reply-To: <9e0b85dd-3621-cd8c-7ec6-67e319cab071@linaro.org> Sender: linux-arm-msm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-arm-msm@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP 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 >> 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 >>> Cc: Bjorn Andersson >>> Signed-off-by: Stanimir Varbanov >>> --- >>> 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"