Re: [PATCH v2 2/5] coresight: etm4x: exclude ss_status from drvdata->config

[Yeoreum Yun]

Hi Jie,

>
>
> On 4/10/2026 3:43 PM, Yeoreum Yun wrote:
> > The purpose of TRCSSCSRn register is to show status of
> > the corresponding Single-shot Comparator Control and input supports.
> > That means writable field's purpose for reset or restore from idle status
> > not for configuration.
> >
> > Therefore, exclude ss_status from drvdata->config, move it to etm4x_caps.
> > This includes remove TRCSSCRn from configurable item and
> > remove saving in etm4_disable_hw().
> >
> > Signed-off-by: Yeoreum Yun <yeoreum.yun@xxxxxxx>
> > ---
> >   .../hwtracing/coresight/coresight-etm4x-cfg.c  |  1 -
> >   .../hwtracing/coresight/coresight-etm4x-core.c | 18 +++++-------------
> >   .../coresight/coresight-etm4x-sysfs.c          |  7 ++-----
> >   drivers/hwtracing/coresight/coresight-etm4x.h  |  3 ++-
> >   4 files changed, 9 insertions(+), 20 deletions(-)
> >
> > diff --git a/drivers/hwtracing/coresight/coresight-etm4x-cfg.c b/drivers/hwtracing/coresight/coresight-etm4x-cfg.c
> > index c302072b293a..d14d7c8a23e5 100644
> > --- a/drivers/hwtracing/coresight/coresight-etm4x-cfg.c
> > +++ b/drivers/hwtracing/coresight/coresight-etm4x-cfg.c
> > @@ -86,7 +86,6 @@ static int etm4_cfg_map_reg_offset(struct etmv4_drvdata *drvdata,
> >   		off_mask =  (offset & GENMASK(11, 5));
> >   		do {
> >   			CHECKREGIDX(TRCSSCCRn(0), ss_ctrl, idx, off_mask);
> > -			CHECKREGIDX(TRCSSCSRn(0), ss_status, idx, off_mask);
> >   			CHECKREGIDX(TRCSSPCICRn(0), ss_pe_cmp, idx, off_mask);
> >   		} while (0);
> >   	} else if ((offset >= TRCCIDCVRn(0)) && (offset <= TRCVMIDCVRn(7))) {
> > diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> > index 6443f3717b37..b7abb171f523 100644
> > --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
> > +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> > @@ -91,7 +91,7 @@ static bool etm4x_sspcicrn_present(struct etmv4_drvdata *drvdata, int n)
> >   	const struct etmv4_caps *caps = &drvdata->caps;
> >   	return (n < caps->nr_ss_cmp) && caps->nr_pe &&
> > -	       (drvdata->config.ss_status[n] & TRCSSCSRn_PC);
> > +	       (caps->ss_status[n] & TRCSSCSRn_PC);
> >   }
> >   u64 etm4x_sysreg_read(u32 offset, bool _relaxed, bool _64bit)
> > @@ -571,11 +571,9 @@ static int etm4_enable_hw(struct etmv4_drvdata *drvdata)
> >   		etm4x_relaxed_write32(csa, config->res_ctrl[i], TRCRSCTLRn(i));
> >   	for (i = 0; i < caps->nr_ss_cmp; i++) {
> > -		/* always clear status bit on restart if using single-shot */
> > -		if (config->ss_ctrl[i] || config->ss_pe_cmp[i])
> > -			config->ss_status[i] &= ~TRCSSCSRn_STATUS;
> >   		etm4x_relaxed_write32(csa, config->ss_ctrl[i], TRCSSCCRn(i));
> > -		etm4x_relaxed_write32(csa, config->ss_status[i], TRCSSCSRn(i));
> > +		/* always clear status bit on restart if using single-shot */
>
> the clear step for the status bit has been removed. but the comment still
> here.
>
> I think we cannot remove the clear step. The hardware requires the STATUS
> bit to be cleared before re-arming a single-shot comparator; failing to do
> so means the comparator will not fire on the next trace session

This one is intended. if you see the patch:

+  /* always clear status bit on restart if using single-shot */
+  etm4x_relaxed_write32(csa, caps->ss_status[i], TRCSSCSRn(i));

cap->ss_status[i] is initialised by etm4_init_arch_data() cleared
STATUS & PENDING bit. so the comment is still valid.


Thanks.

[...]

--
Sincerely,
Yeoreum Yun