Hi Mathieu,



On 12 June 2016 at 19:17, Mathieu Poirier <mathieu.poirier@linaro.org> wrote:
On 3 June 2016 at 02:26, Mike Leach <mike.leach@linaro.org> wrote:
> Feature patch set adding the ETMv3 instruction decoder to the
> main arm-dev branch.
>
> Changes add the ETMV3 packet decode objects, along with updates to
> C-API and test programs

I have reviewed this patchset and commented on a few things.  As a
general rule I think most patchset commit message need to be enhanced
to be more descriptive - it will certainly help other people
understand the work that was done.


that's fair enough - quite a few of the messages were around adding new files and weren't that descriptive. I'll look to improve this in future.

 
>
> This patchset will be applied to the arm-dev trunk unless significant changes
> are warranted. Stable release branch opencsd-0v003 will then be created.

Did you try to modify our current code base to reflect the changes in this set?
 

My ETMv3 development branch was rebased on the latest arm-dev / opencsd-0v002, tested and patchset generated.
The patchset was then applied to the arm-dev branch and re-tested to ensure it applied cleanly.

 
If not I think this would be the next thing to do.  Otherwise having a
new 0v003 won't help people much and will likely create a lot of
confusion.

Not sure what you mean here. notwithstanding some possible slight changes from the comments on the patchset, the library builds and both test programs included in the library run tests correctly.
I do not see where the confusion can arise, unless you are referring to the build issue in perf.
 
At the very least, we can't cut a new release without
having an updated HOWTO.md, and from what I understand
perf-opencsd-4.7-rc1 won't compile with these changes.

That is the case - without the patch I created last week to fix the build issue - the build does fail.

We have a choice here - the HOWTO.md can remain claiming compatibility with  0v002, or we can update to claim compatibility with 0v003, and assume that the patch has been appiled to the perf-opencsd-4.7-rc1 branch.

Normally I wouldn't be gating the a release of the library on correct client operation - but as the API is still in flux and perf is part of the combined solution, I think it is reasonable to have fixes to perf available at the same time as the OpenCSD release.

Regards

Mike



Thanks,
Mathieu

>
> Mike Leach (9):
>   OCSD response enum value name corrected.
>   ETMv3 packet decoder
>   Refactor etmv3 config class and etmv3 packet class to improve
>     encapsulation.
>   ETMv3 decoder - created code follower generic class.
>   ETMv3 decoder updates.
>   TC2 snapshot update.
>   ETMv3 Decoder Updates
>   ETMv3 : update to linux build makefile
>   update readme to reflect correct decoder support.
>
>  README.md                                          |     6 +-
>  decoder/build/linux/ref_trace_decode_lib/makefile  |     5 +-
>  .../ref_trace_decode_lib.vcxproj                   |     6 +-
>  .../ref_trace_decode_lib.vcxproj.filters           |    18 +-
>  decoder/include/c_api/opencsd_c_api.h              |    13 +
>  decoder/include/common/ocsd_code_follower.h        |   231 +
>  decoder/include/common/ocsd_dcd_tree_elem.h        |     4 +-
>  decoder/include/common/ocsd_gen_elem_list.h        |   153 +
>  decoder/include/common/ocsd_pe_context.h           |   116 +
>  decoder/include/common/trc_gen_elem.h              |    82 +-
>  decoder/include/etmv3/etmv3_decoder.h              |     1 +
>  decoder/include/etmv3/trc_cmp_cfg_etmv3.h          |    58 +-
>  decoder/include/etmv3/trc_pkt_decode_etmv3.h       |   179 +-
>  decoder/include/etmv3/trc_pkt_elem_etmv3.h         |   115 +-
>  decoder/include/etmv3/trc_pkt_types_etmv3.h        |    12 +-
>  decoder/include/ocsd_if_types.h                    |     8 +-
>  decoder/include/trc_gen_elem_types.h               |     4 +-
>  decoder/include/trc_pkt_types.h                    |     2 +-
>  decoder/source/c_api/ocsd_c_api.cpp                |    21 +-
>  decoder/source/etmv3/trc_cmp_cfg_etmv3.cpp         |    17 +-
>  decoder/source/etmv3/trc_pkt_decode_etmv3.cpp      |   553 +-
>  decoder/source/etmv3/trc_pkt_elem_etmv3.cpp        |   267 +-
>  decoder/source/etmv3/trc_pkt_proc_etmv3_impl.cpp   |     9 +-
>  decoder/source/etmv4/trc_pkt_decode_etmv4i.cpp     |    16 +-
>  decoder/source/etmv4/trc_pkt_proc_etmv4i_impl.cpp  |     2 +-
>  decoder/source/ocsd_code_follower.cpp              |   154 +
>  decoder/source/ocsd_dcd_tree.cpp                   |    29 +-
>  decoder/source/ocsd_error.cpp                      |     3 +-
>  decoder/source/ocsd_gen_elem_list.cpp              |   168 +
>  decoder/source/ptm/trc_pkt_decode_ptm.cpp          |     2 +-
>  decoder/source/ptm/trc_pkt_proc_ptm.cpp            |     2 +-
>  decoder/source/stm/trc_pkt_proc_stm.cpp            |     2 +-
>  decoder/source/trc_frame_deformatter.cpp           |     2 +-
>  decoder/source/trc_gen_elem.cpp                    |    19 +-
>  .../snapshot_parser_lib/source/ss_to_dcdtree.cpp   |    13 +-
>  decoder/tests/snapshots/TC2/ds5-dumps/etm_0.txt    |  7379 ------------
>  decoder/tests/snapshots/TC2/ds5-dumps/etm_0x10.txt |  7381 ------------
>  decoder/tests/snapshots/TC2/ds5-dumps/etm_0x11.txt |  7612 ------------
>  decoder/tests/snapshots/TC2/ds5-dumps/etm_0x12.txt |  1980 ----
>  .../tests/snapshots/TC2/ds5-dumps/etmv3_0x10.txt   |  7518 ++++++++++++
>  .../tests/snapshots/TC2/ds5-dumps/etmv3_0x11.txt   |  7728 +++++++++++++
>  .../tests/snapshots/TC2/ds5-dumps/etmv3_0x12.txt   |  2001 ++++
>  decoder/tests/snapshots/TC2/ds5-dumps/ptm_0x13.txt | 11553 -------------------
>  .../tests/snapshots/TC2/ds5-dumps/ptmv1_0x13.txt   |  9913 ++++++++++++++++
>  decoder/tests/source/simple_pkt_c_api.c            |    18 +-
>  45 files changed, 29197 insertions(+), 36178 deletions(-)
>  create mode 100644 decoder/include/common/ocsd_code_follower.h
>  create mode 100644 decoder/include/common/ocsd_gen_elem_list.h
>  create mode 100644 decoder/include/common/ocsd_pe_context.h
>  create mode 100644 decoder/source/ocsd_code_follower.cpp
>  create mode 100644 decoder/source/ocsd_gen_elem_list.cpp
>  delete mode 100644 decoder/tests/snapshots/TC2/ds5-dumps/etm_0.txt
>  delete mode 100644 decoder/tests/snapshots/TC2/ds5-dumps/etm_0x10.txt
>  delete mode 100644 decoder/tests/snapshots/TC2/ds5-dumps/etm_0x11.txt
>  delete mode 100644 decoder/tests/snapshots/TC2/ds5-dumps/etm_0x12.txt
>  create mode 100644 decoder/tests/snapshots/TC2/ds5-dumps/etmv3_0x10.txt
>  create mode 100644 decoder/tests/snapshots/TC2/ds5-dumps/etmv3_0x11.txt
>  create mode 100644 decoder/tests/snapshots/TC2/ds5-dumps/etmv3_0x12.txt
>  delete mode 100644 decoder/tests/snapshots/TC2/ds5-dumps/ptm_0x13.txt
>  create mode 100644 decoder/tests/snapshots/TC2/ds5-dumps/ptmv1_0x13.txt
>
> --
> 1.9.1
>
> _______________________________________________
> CoreSight mailing list
> CoreSight@lists.linaro.org
> https://lists.linaro.org/mailman/listinfo/coresight



--
Mike Leach
Principal Engineer, ARM Ltd.
Blackburn Design Centre. UK