Re: [PATCH v10 3/5] coresight: cti: add Qualcomm extended CTI identification and quirks

[Mike Leach <mike.leach@arm.com>]

On 6/15/26 13:32, Yingchao Deng wrote:
> Qualcomm implements an extended variant of the ARM CoreSight CTI with a
> different register layout and vendor-specific behavior. While the
> programming model remains largely compatible, the register offsets differ
> from the standard ARM CTI and require explicit handling.
> 
> Detect Qualcomm CTIs via the DEVARCH register and record this in the CTI
> driver data. Introduce a small mapping layer to translate standard CTI
> register offsets to Qualcomm-specific offsets, allowing the rest of the
> driver to use a common register access path.
> 
> Additionally, handle a Qualcomm-specific quirk where the hardware does
> not implement the CoreSight Claim tag protocol. Instead of clearing the
> CLAIMSET register at probe time, bypass the claim/disclaim operations
> entirely for Qualcomm CTIs by wrapping coresight_claim_device(),
> coresight_disclaim_device_unlocked() and coresight_clear_self_claim_tag()
> in thin helpers that early-return when is_qcom_cti is set.
> 
> No functional change is intended for standard ARM CTI devices.
> 
> Co-developed-by: Jinlong Mao <jinlong.mao@oss.qualcomm.com>
> Signed-off-by: Jinlong Mao <jinlong.mao@oss.qualcomm.com>
> Signed-off-by: Yingchao Deng <yingchao.deng@oss.qualcomm.com>
> ---
>   drivers/hwtracing/coresight/coresight-cti-core.c | 47 +++++++++++++++--
>   drivers/hwtracing/coresight/coresight-cti.h      | 64 +++++++++++++++++++++++-
>   2 files changed, 105 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/hwtracing/coresight/coresight-cti-core.c b/drivers/hwtracing/coresight/coresight-cti-core.c
> index fa758c535ccb..5b83dd4e603b 100644
> --- a/drivers/hwtracing/coresight/coresight-cti-core.c
> +++ b/drivers/hwtracing/coresight/coresight-cti-core.c
> @@ -73,6 +73,35 @@ void cti_write_all_hw_regs(struct cti_drvdata *drvdata)
>   	CS_LOCK(drvdata->base);
>   }
>   
> +/*
> + * Qualcomm CTIs do not implement the CoreSight Claim tag protocol, so
> + * bypass coresight_clear_self_claim_tag() for them.
> + */
> +static void cti_clear_self_claim_tag(struct cti_drvdata *drvdata,
> +				     struct csdev_access *csa)
> +{
> +	if (drvdata->is_qcom_cti)
> +		return;
> +
> +	coresight_clear_self_claim_tag(csa);
> +}
> +
> +static int cti_claim_device(struct cti_drvdata *drvdata)
> +{
> +	if (drvdata->is_qcom_cti)
> +		return 0;
> +
> +	return coresight_claim_device(drvdata->csdev);
> +}
> +
> +static void cti_unclaim_device_unlocked(struct cti_drvdata *drvdata)
> +{
> +	if (drvdata->is_qcom_cti)
> +		return;
> +
> +	coresight_disclaim_device_unlocked(drvdata->csdev);
> +}
> +
>   /* write regs to hardware and enable */
>   static int cti_enable_hw(struct cti_drvdata *drvdata)
>   {
> @@ -86,7 +115,7 @@ static int cti_enable_hw(struct cti_drvdata *drvdata)
>   		goto cti_state_unchanged;
>   
>   	/* claim the device */
> -	rc = coresight_claim_device(drvdata->csdev);
> +	rc = cti_claim_device(drvdata);
>   	if (rc)
>   		return rc;
>   
> @@ -101,7 +130,6 @@ static int cti_enable_hw(struct cti_drvdata *drvdata)
>   static int cti_disable_hw(struct cti_drvdata *drvdata)
>   {
>   	struct cti_config *config = &drvdata->config;
> -	struct coresight_device *csdev = drvdata->csdev;
>   
>   	guard(raw_spinlock_irqsave)(&drvdata->spinlock);
>   
> @@ -118,7 +146,7 @@ static int cti_disable_hw(struct cti_drvdata *drvdata)
>   	/* disable CTI */
>   	writel_relaxed(0, drvdata->base + CTICONTROL);
>   
> -	coresight_disclaim_device_unlocked(csdev);
> +	cti_unclaim_device_unlocked(drvdata);
>   	CS_LOCK(drvdata->base);
>   	return 0;
>   }
> @@ -144,6 +172,9 @@ void cti_write_intack(struct device *dev, u32 ackval)
>   /* DEVID[19:16] - number of CTM channels */
>   #define CTI_DEVID_CTMCHANNELS(devid_val) ((int) BMVAL(devid_val, 16, 19))
>   
> +/* DEVARCH[31:21] - ARCHITECT */
> +#define CTI_DEVARCH_ARCHITECT(devarch_val) ((int)BMVAL(devarch_val, 21, 31))
> +
>   static int cti_set_default_config(struct device *dev,
>   				  struct cti_drvdata *drvdata)
>   {
> @@ -684,6 +715,7 @@ static int cti_probe(struct amba_device *adev, const struct amba_id *id)
>   	struct coresight_desc cti_desc = { 0 };
>   	struct coresight_platform_data *pdata = NULL;
>   	struct resource *res = &adev->res;
> +	u32 devarch;
>   
>   	/* driver data*/
>   	drvdata = devm_kzalloc(dev, sizeof(*drvdata), GFP_KERNEL);
> @@ -708,6 +740,10 @@ static int cti_probe(struct amba_device *adev, const struct amba_id *id)
>   
>   	raw_spin_lock_init(&drvdata->spinlock);
>   
> +	devarch = readl_relaxed(drvdata->base + CORESIGHT_DEVARCH);
> +	if (CTI_DEVARCH_ARCHITECT(devarch) == QCOM_ARCHITECT)
> +		drvdata->is_qcom_cti = true;
> +
>   	/* initialise CTI driver config values */
>   	ret = cti_set_default_config(dev, drvdata);
>   	if (ret)
> @@ -753,7 +789,7 @@ static int cti_probe(struct amba_device *adev, const struct amba_id *id)
>   	cti_desc.groups = drvdata->ctidev.con_groups;
>   	cti_desc.dev = dev;
>   
> -	coresight_clear_self_claim_tag(&cti_desc.access);
> +	cti_clear_self_claim_tag(drvdata, &cti_desc.access);
>   	drvdata->csdev = coresight_register(&cti_desc);
>   	if (IS_ERR(drvdata->csdev))
>   		return PTR_ERR(drvdata->csdev);
> @@ -767,7 +803,8 @@ static int cti_probe(struct amba_device *adev, const struct amba_id *id)
>   
>   	/* all done - dec pm refcount */
>   	pm_runtime_put(&adev->dev);
> -	dev_info(&drvdata->csdev->dev, "CTI initialized\n");
> +	dev_info(&drvdata->csdev->dev,
> +		 "%sCTI initialized\n", drvdata->is_qcom_cti ? "QCOM " : "");
>   	return 0;
>   }
>   
> diff --git a/drivers/hwtracing/coresight/coresight-cti.h b/drivers/hwtracing/coresight/coresight-cti.h
> index 634bdce5cdfd..4b6fd6b55114 100644
> --- a/drivers/hwtracing/coresight/coresight-cti.h
> +++ b/drivers/hwtracing/coresight/coresight-cti.h
> @@ -54,10 +54,36 @@ struct fwnode_handle;
>   /*
>    * CTI CSSoc 600 has a max of 32 trigger signals per direction.
>    * CTI CSSoc 400 has 8 IO triggers - other CTIs can be impl def.
> + * QCOM CTI support up to 128 trigger signals per direction.
>    * Max of in and out defined in the DEVID register.
>    * - pick up actual number used from .dts parameters if present.

I haven't seen anything in these patches that actually does check for 
max CTIINOUT values in .dts.

Would it not be better to have a QCOM_CTIINOUTENMAX and set the max 
value in config based on the is_qcom_cti flag?

Otherwise it appears that the standard arm CTIs are going to create a 
whole lot of unneeded/unused registers

Regards

Mike

>    */
> -#define CTIINOUTEN_MAX		32
> +#define CTIINOUTEN_MAX		128
> +
> +/* QCOM CTI extension */
> +#define QCOM_ARCHITECT		0x477
> +
> +#define QCOM_CTIINTACK		0x020
> +#define QCOM_CTIAPPSET		0x004
> +#define QCOM_CTIAPPCLEAR	0x008
> +#define QCOM_CTIAPPPULSE	0x00C
> +#define QCOM_CTIINEN		0x400
> +#define QCOM_CTIOUTEN		0x800
> +#define QCOM_CTITRIGINSTATUS	0x040
> +#define QCOM_CTITRIGOUTSTATUS	0x060
> +#define QCOM_CTICHINSTATUS	0x080
> +#define QCOM_CTICHOUTSTATUS	0x084
> +#define QCOM_CTIGATE		0x088
> +#define QCOM_ASICCTL		0x08C
> +/* Integration test registers */
> +#define QCOM_ITCHINACK		0xE70
> +#define QCOM_ITTRIGINACK	0xE80
> +#define QCOM_ITCHOUT		0xE74
> +#define QCOM_ITTRIGOUT		0xEA0
> +#define QCOM_ITCHOUTACK		0xE78
> +#define QCOM_ITTRIGOUTACK	0xEC0
> +#define QCOM_ITCHIN		0xE7C
> +#define QCOM_ITTRIGIN		0xEE0
>   
>   /**
>    * Group of related trigger signals
> @@ -168,6 +194,9 @@ struct cti_config {
>    * @spinlock:	Control data access to one at a time.
>    * @config:	Configuration data for this CTI device.
>    * @node:	List entry of this device in the list of CTI devices.
> + * @is_qcom_cti: True if this CTI is a Qualcomm vendor-specific
> + *		 variant that requires register offset translation
> + *		 via cti_qcom_reg_off().
>    */
>   struct cti_drvdata {
>   	void __iomem *base;
> @@ -176,6 +205,7 @@ struct cti_drvdata {
>   	raw_spinlock_t spinlock;
>   	struct cti_config config;
>   	struct list_head node;
> +	bool is_qcom_cti;
>   };
>   
>   /*
> @@ -229,9 +259,41 @@ struct coresight_platform_data *
>   coresight_cti_get_platform_data(struct device *dev);
>   const char *cti_plat_get_node_name(struct fwnode_handle *fwnode);
>   
> +static inline u32 cti_qcom_reg_off(u32 offset)
> +{
> +	switch (offset) {
> +	case CTIINTACK:		return QCOM_CTIINTACK;
> +	case CTIAPPSET:		return QCOM_CTIAPPSET;
> +	case CTIAPPCLEAR:	return QCOM_CTIAPPCLEAR;
> +	case CTIAPPPULSE:	return QCOM_CTIAPPPULSE;
> +	case CTIINEN:		return QCOM_CTIINEN;
> +	case CTIOUTEN:		return QCOM_CTIOUTEN;
> +	case CTITRIGINSTATUS:	return QCOM_CTITRIGINSTATUS;
> +	case CTITRIGOUTSTATUS:	return QCOM_CTITRIGOUTSTATUS;
> +	case CTICHINSTATUS:	return QCOM_CTICHINSTATUS;
> +	case CTICHOUTSTATUS:	return QCOM_CTICHOUTSTATUS;
> +	case CTIGATE:		return QCOM_CTIGATE;
> +	case ASICCTL:		return QCOM_ASICCTL;
> +	case ITCHINACK:		return QCOM_ITCHINACK;
> +	case ITTRIGINACK:	return QCOM_ITTRIGINACK;
> +	case ITCHOUT:		return QCOM_ITCHOUT;
> +	case ITTRIGOUT:		return QCOM_ITTRIGOUT;
> +	case ITCHOUTACK:	return QCOM_ITCHOUTACK;
> +	case ITTRIGOUTACK:	return QCOM_ITTRIGOUTACK;
> +	case ITCHIN:		return QCOM_ITCHIN;
> +	case ITTRIGIN:		return QCOM_ITTRIGIN;
> +
> +	default:
> +		return offset;
> +	}
> +}
> +
>   static inline void __iomem *__reg_addr(struct cti_drvdata *drvdata,
>   				       u32 off, u32 index)
>   {
> +	if (unlikely(drvdata->is_qcom_cti))
> +		off = cti_qcom_reg_off(off);
> +
>   	return drvdata->base + off + index * sizeof(u32);
>   }
>   
>