lkml.org 
[lkml]   [2017]   [Nov]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH 07/17] coresight: tmc etr: Add transparent buffer management
From
Date
On 02/11/17 17:48, Mathieu Poirier wrote:
> On Thu, Oct 19, 2017 at 06:15:43PM +0100, Suzuki K Poulose wrote:
>> At the moment we always use contiguous memory for TMC ETR tracing
>> when used from sysfs. The size of the buffer is fixed at boot time
>> and can only be changed by modifiying the DT. With the introduction
>> of SG support we could support really large buffers in that mode.
>> This patch abstracts the buffer used for ETR to switch between a
>> contiguous buffer or a SG table depending on the availability of
>> the memory.
>>
>> This also enables the sysfs mode to use the ETR in SG mode depending
>> on configured the trace buffer size. Also, since ETR will use the
>> new infrastructure to manage the buffer, we can get rid of some
>> of the members in the tmc_drvdata and clean up the fields a bit.
>>
>> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
>> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
>> ---
>> drivers/hwtracing/coresight/coresight-tmc-etr.c | 433 +++++++++++++++++++-----
>> drivers/hwtracing/coresight/coresight-tmc.h | 60 +++-
>> 2 files changed, 403 insertions(+), 90 deletions(-)
>>
>
> [..]
>
>> +
>> +static void tmc_etr_sync_sg_buf(struct etr_buf *etr_buf, u64 rrp, u64 rwp)

>> + w_offset = tmc_sg_get_data_page_offset(table, rwp);
>> + if (w_offset < 0) {
>> + dev_warn(table->dev, "Unable to map RWP %llx to offset\n",
>> + rwp);
>
> dev_warn(table->dev,
> "Unable to map RWP %llx to offset\n", rwq);
>
> It looks a little better and we respect indentation rules. Same for r_offset.
>

>> +static inline int tmc_etr_mode_alloc_buf(int mode,
>> + struct tmc_drvdata *drvdata,
>> + struct etr_buf *etr_buf, int node,
>> + void **pages)
>
> static inline int
> tmc_etr_mode_alloc_buf(int mode,
> struct tmc_drvdata *drvdata,
> struct etr_buf *etr_buf, int node,
> void **pages)

>> + * tmc_alloc_etr_buf: Allocate a buffer use by ETR.
>> + * @drvdata : ETR device details.
>> + * @size : size of the requested buffer.
>> + * @flags : Required properties of the type of buffer.
>> + * @node : Node for memory allocations.
>> + * @pages : An optional list of pages.
>> + */
>> +static struct etr_buf *tmc_alloc_etr_buf(struct tmc_drvdata *drvdata,
>> + ssize_t size, int flags,
>> + int node, void **pages)
>
> Please fix indentation. Also @flags isn't used.
>

Yep, flags is only used later and can move it to the patch where we use it.

>> +{
>> + int rc = -ENOMEM;
>> + bool has_etr_sg, has_iommu;
>> + struct etr_buf *etr_buf;
>> +
>> + has_etr_sg = tmc_etr_has_cap(drvdata, TMC_ETR_SG);
>> + has_iommu = iommu_get_domain_for_dev(drvdata->dev);
>> +
>> + etr_buf = kzalloc(sizeof(*etr_buf), GFP_KERNEL);
>> + if (!etr_buf)
>> + return ERR_PTR(-ENOMEM);
>> +
>> + etr_buf->size = size;
>> +
>> + /*
>> + * If we have to use an existing list of pages, we cannot reliably
>> + * use a contiguous DMA memory (even if we have an IOMMU). Otherwise,
>> + * we use the contiguous DMA memory if :
>> + * a) The ETR cannot use Scatter-Gather.
>> + * b) if not a, we have an IOMMU backup
>
> Please rework the above sentence.

How about :
b) if (a) is not true and we have an IOMMU connected to the ETR.

I will address the other comments on indentation.

Thanks for the detailed look

Cheers
Suzuki

\
 
 \ /
  Last update: 2017-11-03 11:03    [W:0.165 / U:0.732 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site