Re: [PATCH V2 1/6] coresight: etb10: Properly set AUX buffer head in snapshot mode

Mathieu Poirier
2019-05-15T14:29:09Z

Good day Suzuki,

On Wed, 15 May 2019 at 03:45, Suzuki K Poulose <suzuki.poulose@arm.com> wrote:
>
> Hi Mathieu,
>
> On 14/05/2019 20:40, Mathieu Poirier wrote:
> > Unify amongst sink drivers how the AUX ring buffer head is communicated
> > to user space. That way the same algorithm in cs_etm_find_snapshot()
>
> I would leave the userspace tool's function name out of the commit description
> and the comment below. We could instead say: "That way the same algorithm can be
> used by the userspace tool to determine the position and the size of the latest
> snapshot data."

I purposely added the name of the function there so that people can
quickly find it and avoid any misunderstanding about the code in
question. But I also have the same information as a comment in the
code, which should be sufficient. I'll fix it.

>
> > can be used to determine where the latest data is and how much of it
> > to access.
> >
> > Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
> > ---
> > drivers/hwtracing/coresight/coresight-etb10.c | 13 +++++++------
> > 1 file changed, 7 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/hwtracing/coresight/coresight-etb10.c b/drivers/hwtracing/coresight/coresight-etb10.c
> > index 4ee4c80a4354..60e753b1768d 100644
> > --- a/drivers/hwtracing/coresight/coresight-etb10.c
> > +++ b/drivers/hwtracing/coresight/coresight-etb10.c
> > @@ -548,13 +548,14 @@ static unsigned long etb_update_buffer(struct coresight_device *csdev,
> > writel_relaxed(0x0, drvdata->base + ETB_RAM_WRITE_POINTER);
> >
> > /*
> > - * In snapshot mode we have to update the handle->head to point
> > - * to the new location.
> > + * In snapshot mode we simply increment the head by the number of byte
> > + * that were written. User space function cs_etm_find_snapshot() will
> > + * figure out how many bytes to get from the AUX buffer based on the
> > + * position of the head.
> > */
> > - if (buf->snapshot) {
> > - handle->head = (cur * PAGE_SIZE) + offset;
> > - to_read = buf->nr_pages << PAGE_SHIFT;
> > - }
> > + if (buf->snapshot)
> > + handle->head += to_read;
> > +
> > __etb_enable_hw(drvdata);
> > CS_LOCK(drvdata->base);
> > out:
>
> Otherwise:
>
> Reviewed-by: Suzuki K Poulose <suzuki.poulose@arm.com>

Is this for all the kernel space patches or just this one?