lkml.org 
[lkml]   [2016]   [Apr]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH V2 13/15] coresight: tmc: implementing TMC-ETF AUX space API
From
Date
On 19/04/16 17:45, Mathieu Poirier wrote:
> On 19 April 2016 at 10:16, Suzuki K Poulose <Suzuki.Poulose@arm.com> wrote:
>> On 12/04/16 18:54, Mathieu Poirier wrote:
>>>
>>> This patch implement the AUX area interfaces required to
>>> use the TMC (configured as an ETF) from the Perf sub-system.
>>>
>>> The heuristic is heavily borrowed from the ETB10 implementation.
>>>
>>> Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
>>> ---
>>> drivers/hwtracing/coresight/coresight-tmc-etf.c | 198
>>> ++++++++++++++++++++++++
>>> drivers/hwtracing/coresight/coresight-tmc.h | 21 +++
>>> 2 files changed, 219 insertions(+)
>>>
>>> diff --git a/drivers/hwtracing/coresight/coresight-tmc-etf.c
>>> b/drivers/hwtracing/coresight/coresight-tmc-etf.c
>>> index a440784e3b27..fff175d4020d 100644
>>> --- a/drivers/hwtracing/coresight/coresight-tmc-etf.c
>>> +++ b/drivers/hwtracing/coresight/coresight-tmc-etf.c
>>> @@ -15,7 +15,9 @@
>>> * this program. If not, see <http://www.gnu.org/licenses/>.
>>> */
>>>
>>> +#include <linux/circ_buf.h>
>>> #include <linux/coresight.h>
>>> +#include <linux/perf_event.h>
>>> #include <linux/slab.h>
>>> #include "coresight-priv.h"
>>> #include "coresight-tmc.h"
>>> @@ -295,9 +297,205 @@ static void tmc_disable_etf_link(struct
>>> coresight_device *csdev,
>>> dev_info(drvdata->dev, "TMC disabled\n");
>>> }
>>>
>>> +static void *tmc_alloc_etf_buffer(struct coresight_device *csdev, int
>>> cpu,
>>> + void **pages, int nr_pages, bool
>>> overwrite)
>>
>>
>>
>>
>>> +
>>> +static void tmc_free_etf_buffer(void *config)
>>> +{
>>
>>
>>> +
>>> +static int tmc_set_etf_buffer(struct coresight_device *csdev,
>>> + struct perf_output_handle *handle,
>>> + void *sink_config)
>>
>>
>>
>>> +static unsigned long tmc_reset_etf_buffer(struct coresight_device *csdev,
>>> + struct perf_output_handle
>>> *handle,
>>> + void *sink_config, bool *lost)
>>> +{
>>
>>
>>
>>> /**
>>> + * struct cs_buffer - keep track of a recording session' specifics
>>> + * @cur: index of the current buffer
>>> + * @nr_pages: max number of pages granted to us
>>> + * @offset: offset within the current buffer
>>> + * @data_size: how much we collected in this run
>>> + * @lost: other than zero if we had a HW buffer wrap around
>>> + * @snapshot: is this run in snapshot mode
>>> + * @data_pages: a handle the ring buffer
>>> + */
>>> +struct cs_tmc_buffers {
>>> + unsigned int cur;
>>> + unsigned int nr_pages;
>>> + unsigned long offset;
>>> + local_t data_size;
>>> + local_t lost;
>>> + bool snapshot;
>>> + void **data_pages;
>>> +};
>>
>>
>>
>> All of the above look exactly the same as what we have in etb10.c (as you
>> have mentioned).
>> Is there any chance we could reuse them under a generic name ?
>
> I toyed with the idea many times...
>
> Today the structures are similar and can be used in both drivers but
> it is only a matter for time (probably months) before someone adds new
> functionality on one side that isn't compatible with the other side.
> When that happens we'll get a bloated struct with fields that aren't
> used, depending on where it gets instantiated. Or the struct will be
> split again, coming back to what we have today.
>

If that happens in future, we know what to do now :)

>>> + * Make sure the new size is aligned in accordance with
>>> the
>>> + * requirement explained above.
>>> + */
>>> + to_read -= handle->size & mask;
>>
>>
>> Shouldn't this be :
>>
>> to_read = handle->size & mask;
>
> You are correct.

This applies to the etb10 code as well. So you might want to fix that as well.

Cheers
Suzuki

\
 
 \ /
  Last update: 2016-04-19 19:01    [W:0.274 / U:0.196 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site