[PATCH V3 12/18] coresight: tmc: dump system memory content only when needed

[Suzuki K Poulose]

On 25/04/16 15:38, Mathieu Poirier wrote:
> On 25 April 2016 at 05:16, Suzuki K Poulose <Suzuki.Poulose at arm.com> wrote:
>> On 22/04/16 18:14, Mathieu Poirier wrote:
>>>
>>> Calling tmc_etf/etr_dump_hw() is required only when operating from
>>> sysFS.  When working from Perf, the system memory is harvested
>>> from the AUX trace API.
>>>
>>> Signed-off-by: Mathieu Poirier <mathieu.poirier at linaro.org>
>>> ---
>>>    drivers/hwtracing/coresight/coresight-tmc-etf.c | 7 ++++++-
>>>    drivers/hwtracing/coresight/coresight-tmc-etr.c | 7 ++++++-
>>>    2 files changed, 12 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/hwtracing/coresight/coresight-tmc-etf.c
>>> b/drivers/hwtracing/coresight/coresight-tmc-etf.c
>>> index d090a9745c73..25fad93b68d4 100644
>>> --- a/drivers/hwtracing/coresight/coresight-tmc-etf.c
>>> +++ b/drivers/hwtracing/coresight/coresight-tmc-etf.c
>>> @@ -74,7 +74,12 @@ static void tmc_etb_disable_hw(struct tmc_drvdata
>>> *drvdata)
>>>          CS_UNLOCK(drvdata->base);
>>>
>>>          tmc_flush_and_stop(drvdata);
>>> -       tmc_etb_dump_hw(drvdata);
>>> +       /*
>>> +        * When operating in sysFS mode the content of the buffer needs to
>>> be
>>> +        * read before the TMC is disabled.
>>> +        */
>>> +       if (local_read(&drvdata->mode) == CS_MODE_SYSFS)
>>> +               tmc_etb_dump_hw(drvdata);
>>>          tmc_disable_hw(drvdata);
>>>
>>>          CS_LOCK(drvdata->base);
>>> diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c
>>> b/drivers/hwtracing/coresight/coresight-tmc-etr.c
>>> index 8bbbf3ab1387..4b000f4003a2 100644
>>> --- a/drivers/hwtracing/coresight/coresight-tmc-etr.c
>>> +++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c
>>> @@ -76,7 +76,12 @@ static void tmc_etr_disable_hw(struct tmc_drvdata
>>> *drvdata)
>>>          CS_UNLOCK(drvdata->base);
>>>
>>>          tmc_flush_and_stop(drvdata);
>>> -       tmc_etr_dump_hw(drvdata);
>>> +       /*
>>> +        * When operating in sysFS mode the content of the buffer needs to
>>> be
>>> +        * read before the TMC is disabled.
>>> +        */
>>> +       if (local_read(&drvdata->mode) == CS_MODE_SYSFS)
>>> +               tmc_etr_dump_hw(drvdata);
>>>          tmc_disable_hw(drvdata);
>>
>>
>> Nit: It would be cleaner if tmc_etX_disable_hw accepted a bool parameter to
>> decide
>> whether to dump the hw data or not. That way, the callers can make the
>> decision,
>> leaving the disable_hw code unaware of the mode checks.
>
> My goal in pushing the functionality to tmc_etX_disable_hw() was to
> especially keep the the higher layers unaware of sink buffer
> management specifics.  There is two ways to look at this and we simply
> landed on different sides of the fence.  I'm not strongly opinionated
> on this, I'll modify if you're really keen on this.

I am not, you can retain as it is.

Reviewed-by: Suzuki K Poulose <suzuki.poulose at arm.com>

Suzuki