Messages in this thread | | | Subject | Re: [PATCH 07/17] coresight: tmc etr: Add transparent buffer management | From | Suzuki K Poulose <> | Date | Fri, 3 Nov 2017 10:02:44 +0000 |
| |
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
| |