Re: [PATCH v8 08/13] coresight: etm4x: fix inconsistencies with sysfs configuration

[Yeoreum Yun]

[...]

> >   static void etm4_enable_sysfs_smp_call(void *info)
> >   {
> >   	struct etm4_enable_arg *arg = info;
> > +	struct etmv4_drvdata *drvdata;
> >   	struct coresight_device *csdev;
> > +	unsigned long cfg_hash;
> > +	int preset;
> >   	if (WARN_ON(!arg))
> >   		return;
> > -	csdev = arg->drvdata->csdev;
> > +	drvdata = arg->drvdata;
> > +	csdev = drvdata->csdev;
> >   	if (!coresight_take_mode(csdev, CS_MODE_SYSFS)) {
> >   		/* Someone is already using the tracer */
> >   		arg->rc = -EBUSY;
> >   		return;
> >   	}
> > -	arg->rc = etm4_enable_hw(arg->drvdata);
> > +	drvdata->active_config = arg->config;
> 
> I see, we copy the sysfs config to init_arg and then apply it here.

Yes it is. Thanks!

> 
> > -	/* The tracer didn't start */
> > +	/* enable any config activated by configfs */
> > +	cscfg_config_sysfs_get_active_cfg(&cfg_hash, &preset);
> > +	if (cfg_hash) {
> > +		arg->rc = cscfg_csdev_enable_active_config(csdev,
> > +							   cfg_hash,
> > +							   preset);
> > +		if (arg->rc)
> > +			goto err;
> > +	}
> > +
> > +	drvdata->trcid = arg->path->trace_id;
> > +
> > +	/* Tracer will never be paused in sysfs mode */
> > +	drvdata->paused = false;
> > +
> > +	arg->rc = etm4_enable_hw(drvdata);
> >   	if (arg->rc) {
> > -		coresight_set_mode(csdev, CS_MODE_DISABLED);
> > -		return;
> > +		cscfg_csdev_disable_active_config(csdev);
> > +		goto err;
> >   	}
> > +	drvdata->sticky_enable = true;
> >   	csdev->path = arg->path;
> > +
> > +	return;
> > +err:
> > +	/* The tracer didn't start */
> > +	coresight_set_mode(csdev, CS_MODE_DISABLED);
> 
> Do we need to reset the active_config ?

I don't think we don't need to reset active_config since
it's a failure path and at the next time, the active_config will
be initialised with the drvdata->config again if next trial is via
sysfs.

If the next trial is perf, it clears active_config in
etm4_parse_event_config().

Therefore, we wouldn't need to reset the active_config before
relasing mode.

> 
> 
> > diff --git a/drivers/hwtracing/coresight/coresight-etm4x.h b/drivers/hwtracing/coresight/coresight-etm4x.h
> > index c0f7da17a186..a974e4d05838 100644
> > --- a/drivers/hwtracing/coresight/coresight-etm4x.h
> > +++ b/drivers/hwtracing/coresight/coresight-etm4x.h
> > @@ -1068,6 +1068,7 @@ struct etmv4_save_state {
> >    *		allows tracing at all ELs. We don't want to compute this
> >    *		at runtime, due to the additional setting of TRFCR_CX when
> >    *		in EL2. Otherwise, 0.
> > + * @active_config:	structure holding current applied configuration parameters.
> >    * @config:	structure holding configuration parameters.
> 
> Please could you update the documentation for "config" -> holding sysfs mode
> configuration.

Thanks I'll update accordingly.

-- 
Sincerely,
Yeoreum Yun