Re: [PATCH v6 09/13] coresight: etm4x: missing cscfg_csdev_disable_active_config() in perf enable

[Yeoreum Yun <yeoreum.yun@arm.com>]

Hi Leo,

> On Wed, Apr 22, 2026 at 02:21:59PM +0100, Yeoreum Yun wrote:
> 
> [...]
> 
> > @@ -895,6 +895,8 @@ static int etm4_parse_event_config(struct coresight_device *csdev,
> >  			 * Missing BB support could cause silent decode errors
> >  			 * so fail to open if it's not supported.
> >  			 */
> > +			if (cfg_hash)
> > +				cscfg_csdev_disable_active_config(csdev);
> 
> I prefer do a bit refactoring for this.
> 
> we just save cfg_hash and cfg_preset into drvdata in
> etm4_parse_event_config():
> 
>   drvdata->cfg_hash = ATTR_CFG_GET_FLD(attr, configid);
>   if (drvdata->cfg_hash)
>         drvdata->preset = ATTR_CFG_GET_FLD(attr, preset);
> 
> Then create two helpers:
> 
>   etm4_cscfg_enable(csdev) {
>     struct etmv4_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
> 
>     return cscfg_csdev_enable_active_config(csdev, drvdata->cfg_hash,
>                                             drvdata->preset);
>   }
> 
>   etm4_cscfg_disable(csdev) {
>     cscfg_csdev_disable_active_config(csdev);
>   }
> 
> These helpers will be used by etm4_{enable|disable}_perf()
> and etm4_{enable|disable}_sysfs(). This might benefit the future cscfg
> refactoring.

I think this seems some over-engineering since the
etm4_cscfg_enable/disable() just an wrapper for
cscfg_csdev_enable/disable_active_config() but just increase size of drvdata.

It's not late to delay when we do refactoring the cscfg
and at that time, we can consider some place to save cfg_hash and
preset. If we do right now, personally, there seems no benefit for this.

Am I missing something?

Thanks.

[...]

-- 
Sincerely,
Yeoreum Yun