linux-arm-msm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Stanimir Varbanov <stanimir.varbanov@linaro.org>
To: Hans Verkuil <hverkuil@xs4all.nl>,
	Stanimir Varbanov <stanimir.varbanov@linaro.org>,
	Mauro Carvalho Chehab <mchehab@kernel.org>
Cc: Andy Gross <andy.gross@linaro.org>,
	Bjorn Andersson <bjorn.andersson@linaro.org>,
	Stephen Boyd <sboyd@codeaurora.org>,
	Srinivas Kandagatla <srinivas.kandagatla@linaro.org>,
	linux-media@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-arm-msm@vger.kernel.org
Subject: Re: [PATCH v8 01/10] firmware: qcom_scm: Fix to allow COMPILE_TEST-ing
Date: Fri, 5 May 2017 16:23:52 +0300	[thread overview]
Message-ID: <4b4f7977-c934-aba7-1716-ea74d4562baf@linaro.org> (raw)
In-Reply-To: <bd344b32-77a0-9160-7be8-52b88ccd225e@xs4all.nl>

Hi Hans,

On 05/05/2017 02:34 PM, Hans Verkuil wrote:
> On 04/28/17 11:13, 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.
>>
>> To: Andy Gross <andy.gross@linaro.org>
>> Cc: Stephen Boyd <sboyd@codeaurora.org>
>> Cc: Bjorn Andersson <bjorn.andersson@linaro.org>
>> Signed-off-by: Stanimir Varbanov <stanimir.varbanov@linaro.org>
>> ---
>>  drivers/firmware/Kconfig    |  2 +-
>>  drivers/firmware/qcom_scm.h | 72 ++++++++++++++++++++++++++++++++++++++-------
>>  include/linux/qcom_scm.h    | 32 --------------------
>>  3 files changed, 62 insertions(+), 44 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..d2b5723afb3f 100644
>> --- a/drivers/firmware/qcom_scm.h
>> +++ b/drivers/firmware/qcom_scm.h
>> @@ -12,6 +12,7 @@
>>  #ifndef __QCOM_SCM_INT_H
>>  #define __QCOM_SCM_INT_H
>>  
>> +#if IS_ENABLED(CONFIG_ARM) || IS_ENABLED(CONFIG_ARM64)
> 
> This is weird. Shouldn't this be:
> 
> #if IS_ENABLED(CONFIG_QCOM_SCM)

I think no, because if you take a look in the above hunk which adding
COMPILE_TEST in the QCOM_SCM config the CONFIG_QCOM_SCM will be enabled
if compile testing is selected for example on x86 build target.

> 
> If the code in the actual source only works for ARM, then that should be
> handled in that source code, and not in this header IMHO.

Do you mean adding #ifdef in the source file? IMO this will become a
bigger mess.

> 
> Regards,
> 
> 	Hans
> 
>>  #define QCOM_SCM_SVC_BOOT		0x1
>>  #define QCOM_SCM_BOOT_ADDR		0x1
>>  #define QCOM_SCM_BOOT_ADDR_MC		0x11
>> @@ -58,6 +59,66 @@ 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);
>>  
>> +#define QCOM_SCM_SVC_MP			0xc
>> +#define QCOM_SCM_RESTORE_SEC_CFG	2
>> +extern int __qcom_scm_restore_sec_cfg(struct device *dev, u32 device_id,
>> +				      u32 spare);
>> +#define QCOM_SCM_IOMMU_SECURE_PTBL_SIZE	3
>> +#define QCOM_SCM_IOMMU_SECURE_PTBL_INIT	4
>> +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
>> +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; }
>> +#define QCOM_SCM_SVC_HDCP		0x11
>> +#define QCOM_SCM_CMD_HDCP		0x01
>> +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) {}
>> +#define QCOM_SCM_SVC_PIL		0x2
>> +#define QCOM_SCM_PAS_IS_SUPPORTED_CMD	0x7
>> +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; }
>> +extern 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; }
>> +#endif
>> +
>>  /* common error codes */
>>  #define QCOM_SCM_V2_EBUSY	-12
>>  #define QCOM_SCM_ENOMEM		-5
>> @@ -85,15 +146,4 @@ static inline int qcom_scm_remap_error(int err)
>>  	return -EINVAL;
>>  }
>>  
>> -#define QCOM_SCM_SVC_MP			0xc
>> -#define QCOM_SCM_RESTORE_SEC_CFG	2
>> -extern int __qcom_scm_restore_sec_cfg(struct device *dev, u32 device_id,
>> -				      u32 spare);
>> -#define QCOM_SCM_IOMMU_SECURE_PTBL_SIZE	3
>> -#define QCOM_SCM_IOMMU_SECURE_PTBL_INIT	4
>> -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);
>> -
>>  #endif
>> diff --git a/include/linux/qcom_scm.h b/include/linux/qcom_scm.h
>> index e5380471c2cd..b628f735f355 100644
>> --- a/include/linux/qcom_scm.h
>> +++ b/include/linux/qcom_scm.h
>> @@ -23,7 +23,6 @@ struct qcom_scm_hdcp_req {
>>  	u32 val;
>>  };
>>  
>> -#if IS_ENABLED(CONFIG_QCOM_SCM)
>>  extern int qcom_scm_set_cold_boot_addr(void *entry, const cpumask_t *cpus);
>>  extern int qcom_scm_set_warm_boot_addr(void *entry, const cpumask_t *cpus);
>>  extern bool qcom_scm_is_available(void);
>> @@ -43,35 +42,4 @@ extern int qcom_scm_set_remote_state(u32 state, u32 id);
>>  extern int qcom_scm_restore_sec_cfg(u32 device_id, u32 spare);
>>  extern int qcom_scm_iommu_secure_ptbl_size(u32 spare, size_t *size);
>>  extern int qcom_scm_iommu_secure_ptbl_init(u64 addr, u32 size, u32 spare);
>> -#else
>> -static inline
>> -int qcom_scm_set_cold_boot_addr(void *entry, const cpumask_t *cpus)
>> -{
>> -	return -ENODEV;
>> -}
>> -static inline
>> -int qcom_scm_set_warm_boot_addr(void *entry, const cpumask_t *cpus)
>> -{
>> -	return -ENODEV;
>> -}
>> -static inline bool qcom_scm_is_available(void) { return false; }
>> -static inline bool qcom_scm_hdcp_available(void) { return false; }
>> -static inline int qcom_scm_hdcp_req(struct qcom_scm_hdcp_req *req, u32 req_cnt,
>> -				    u32 *resp) { return -ENODEV; }
>> -static inline bool qcom_scm_pas_supported(u32 peripheral) { return false; }
>> -static inline int qcom_scm_pas_init_image(u32 peripheral, const void *metadata,
>> -					  size_t size) { return -ENODEV; }
>> -static inline int qcom_scm_pas_mem_setup(u32 peripheral, phys_addr_t addr,
>> -					 phys_addr_t size) { return -ENODEV; }
>> -static inline int
>> -qcom_scm_pas_auth_and_reset(u32 peripheral) { return -ENODEV; }
>> -static inline int qcom_scm_pas_shutdown(u32 peripheral) { return -ENODEV; }
>> -static inline void qcom_scm_cpu_power_down(u32 flags) {}
>> -static inline u32 qcom_scm_get_version(void) { return 0; }
>> -static inline u32
>> -qcom_scm_set_remote_state(u32 state,u32 id) { return -ENODEV; }
>> -static inline int qcom_scm_restore_sec_cfg(u32 device_id, u32 spare) { return -ENODEV; }
>> -static inline int qcom_scm_iommu_secure_ptbl_size(u32 spare, size_t *size) { return -ENODEV; }
>> -static inline int qcom_scm_iommu_secure_ptbl_init(u64 addr, u32 size, u32 spare) { return -ENODEV; }
>> -#endif
>>  #endif
>>
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

-- 
regards,
Stan

  reply	other threads:[~2017-05-05 13:23 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-04-28  9:13 [PATCH v8 00/10] Qualcomm video decoder/encoder driver Stanimir Varbanov
2017-04-28  9:13 ` [PATCH v8 01/10] firmware: qcom_scm: Fix to allow COMPILE_TEST-ing Stanimir Varbanov
2017-05-02 19:22   ` Bjorn Andersson
2017-05-05 11:34   ` Hans Verkuil
2017-05-05 13:23     ` Stanimir Varbanov [this message]
2017-05-05 14:30       ` Hans Verkuil
2017-04-28  9:13 ` [PATCH v8 02/10] media: v4l2-mem2mem: extend m2m APIs for more accurate buffer management Stanimir Varbanov
2017-05-05 11:36   ` Hans Verkuil
2017-04-28  9:13 ` [PATCH v8 03/10] doc: DT: venus: binding document for Qualcomm video driver Stanimir Varbanov
2017-04-28  9:13 ` [PATCH v8 04/10] MAINTAINERS: Add Qualcomm Venus video accelerator driver Stanimir Varbanov
2017-04-28  9:13 ` [PATCH v8 05/10] media: venus: adding core part and helper functions Stanimir Varbanov
2017-04-28 22:02   ` Jordan Crouse
2017-04-29 20:22     ` Bjorn Andersson
2017-05-02  9:17       ` Stanimir Varbanov
2017-05-03 13:46         ` Jordan Crouse
2017-04-29 22:21   ` Sakari Ailus
2017-05-02  8:52     ` Stanimir Varbanov
2017-05-02 18:52       ` Bjorn Andersson
2017-05-05 13:29         ` Stanimir Varbanov
2017-04-28  9:13 ` [PATCH v8 06/10] media: venus: vdec: add video decoder files Stanimir Varbanov
2017-04-28  9:13 ` [PATCH v8 07/10] media: venus: venc: add video encoder files Stanimir Varbanov
2017-04-28  9:13 ` [PATCH v8 08/10] media: venus: hfi: add Host Firmware Interface (HFI) Stanimir Varbanov
2017-04-28  9:13 ` [PATCH v8 09/10] media: venus: hfi: add Venus HFI files Stanimir Varbanov
2017-04-28  9:13 ` [PATCH v8 10/10] media: venus: enable building of Venus video driver Stanimir Varbanov
2017-05-05 12:44 ` [PATCH v8 00/10] Qualcomm video decoder/encoder driver Hans Verkuil
2017-05-05 13:42   ` Stanimir Varbanov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4b4f7977-c934-aba7-1716-ea74d4562baf@linaro.org \
    --to=stanimir.varbanov@linaro.org \
    --cc=andy.gross@linaro.org \
    --cc=bjorn.andersson@linaro.org \
    --cc=hverkuil@xs4all.nl \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=mchehab@kernel.org \
    --cc=sboyd@codeaurora.org \
    --cc=srinivas.kandagatla@linaro.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).