Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1235977 > unrolled thread

Re: [RFC PATCH 01/20] coresight: etm3x: splitting 'etm_enable_hw()' operations

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [RFC PATCH 01/20] coresight: etm3x: splitting 'etm_enable_hw()' operations Alexander Shishkin <alexander.shishkin@linux.intel.com> - 2015-09-30 12:00 +0200
    Re: [RFC PATCH 01/20] coresight: etm3x: splitting 'etm_enable_hw()' operations Mathieu Poirier <mathieu.poirier@linaro.org> - 2015-10-02 00:30 +0200
      Re: [RFC PATCH 01/20] coresight: etm3x: splitting 'etm_enable_hw()' operations Alexander Shishkin <alexander.shishkin@linux.intel.com> - 2015-10-02 06:50 +0200

#1235977 — Re: [RFC PATCH 01/20] coresight: etm3x: splitting 'etm_enable_hw()' operations

Mathieu Poirier <mathieu.poirier@linaro.org> writes:

> -static void etm_enable_hw(void *info)
> +static void etm_power_up_cpu(void *info)
>  {
> -	int i;
> -	u32 etmcr;
> -	struct etm_drvdata *drvdata = info;
> +	struct coresight_device *csdev = info;
> +	struct etm_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
> +
> +	WARN_ON(drvdata->cpu != smp_processor_id());

Maybe WARN_ON_ONCE() is sufficient here (and other similar places).

> +static void etm_power_down_cpu(void *info)
> +{
> +	struct coresight_device *csdev = info;
> +	struct etm_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
> +
> +	WARN_ON(drvdata->cpu != smp_processor_id());

Likewise.

> +/**
> + * etm_configure_cpu - configure ETM registers
> + * @csdev - the etm that needs to be configure.
> + *
> + * Applies a configuration set to the ETM registers _without_ enabling the
> + * tracer.  This function needs to be executed on the CPU who's tracer is
> + * being configured.
> + */
> +static void etm_configure_cpu(void *info)
> +{
> +	int i;
> +	u32 etmcr;
> +	struct coresight_device *csdev = info;
> +	struct etm_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
> +
> +	WARN_ON(drvdata->cpu != smp_processor_id());

Likewise.

> +
> +	CS_UNLOCK(drvdata->base);
>  	etm_set_prog(drvdata);
>  
>  	etmcr = etm_readl(drvdata, ETMCR);
> -	etmcr &= (ETMCR_PWD_DWN | ETMCR_ETM_PRG);
>  	etmcr |= drvdata->port_size;
>  	etm_writel(drvdata, drvdata->ctrl | etmcr, ETMCR);
>  	etm_writel(drvdata, drvdata->trigger_event, ETMTRIGGER);

Most of these things can also be bypassed, as at least initially perf
events won't be using trigger/sequencer configurations, so we could
simply clear all these things out when a first perf event is created
(which would also disallow any sysfs poking around the etm/etb) and not
worry about them in the pmu callbacks.

Regards,
--
Alex
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1237794

On 30 September 2015 at 03:58, Alexander Shishkin
<alexander.shishkin@linux.intel.com> wrote:
> Mathieu Poirier <mathieu.poirier@linaro.org> writes:
>
>> -static void etm_enable_hw(void *info)
>> +static void etm_power_up_cpu(void *info)
>>  {
>> -     int i;
>> -     u32 etmcr;
>> -     struct etm_drvdata *drvdata = info;
>> +     struct coresight_device *csdev = info;
>> +     struct etm_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
>> +
>> +     WARN_ON(drvdata->cpu != smp_processor_id());
>
> Maybe WARN_ON_ONCE() is sufficient here (and other similar places).

Yes, let's go with that.

>
>> +static void etm_power_down_cpu(void *info)
>> +{
>> +     struct coresight_device *csdev = info;
>> +     struct etm_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
>> +
>> +     WARN_ON(drvdata->cpu != smp_processor_id());
>
> Likewise.
>
>> +/**
>> + * etm_configure_cpu - configure ETM registers
>> + * @csdev - the etm that needs to be configure.
>> + *
>> + * Applies a configuration set to the ETM registers _without_ enabling the
>> + * tracer.  This function needs to be executed on the CPU who's tracer is
>> + * being configured.
>> + */
>> +static void etm_configure_cpu(void *info)
>> +{
>> +     int i;
>> +     u32 etmcr;
>> +     struct coresight_device *csdev = info;
>> +     struct etm_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
>> +
>> +     WARN_ON(drvdata->cpu != smp_processor_id());
>
> Likewise.
>
>> +
>> +     CS_UNLOCK(drvdata->base);
>>       etm_set_prog(drvdata);
>>
>>       etmcr = etm_readl(drvdata, ETMCR);
>> -     etmcr &= (ETMCR_PWD_DWN | ETMCR_ETM_PRG);
>>       etmcr |= drvdata->port_size;
>>       etm_writel(drvdata, drvdata->ctrl | etmcr, ETMCR);
>>       etm_writel(drvdata, drvdata->trigger_event, ETMTRIGGER);
>
> Most of these things can also be bypassed, as at least initially perf
> events won't be using trigger/sequencer configurations, so we could
> simply clear all these things out when a first perf event is created
> (which would also disallow any sysfs poking around the etm/etb) and not
> worry about them in the pmu callbacks.

I don't want to restrain configuration options to what is available
through Perf's cmd line.  The sysFS interface is there to complement
what is currently not available.  Poking around in the sysFS registers
is allowed for a long as a tracer hasn't been commissioned by Perf.  I
do agree though that a mechanism need to be set in place to prevent
changing configuration values once Perf has taken control of a tracer.


>
> Regards,
> --
> Alex
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1237898

Mathieu Poirier <mathieu.poirier@linaro.org> writes:

> On 30 September 2015 at 03:58, Alexander Shishkin
> <alexander.shishkin@linux.intel.com> wrote:
>> Most of these things can also be bypassed, as at least initially perf
>> events won't be using trigger/sequencer configurations, so we could
>> simply clear all these things out when a first perf event is created
>> (which would also disallow any sysfs poking around the etm/etb) and not
>> worry about them in the pmu callbacks.
>
> I don't want to restrain configuration options to what is available
> through Perf's cmd line.  The sysFS interface is there to complement
> what is currently not available.

In practice this means that part of your trace configuration will be
global and part -- per event, which can indeed work if you only have one
event at a time, but again, makes using perf infrastructure redundant
for this driver. For multiple events this doesn't make much sense.

Regards,
--
Alex
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web