| Message ID | 20200415201420.15958-1-mike.leach@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | perf: cs-etm: Update to build with latest opencsd version. | expand |
Hi Mike, On Wed, Apr 15, 2020 at 09:14:20PM +0100, Mike Leach wrote: > OpenCSD version v0.14.0 adds in a new output element. This is represented > by a new value in the generic element type enum, which must be added to > the handling code in perf cs-etm-decoder to prevent build errors due to > build options on the perf project. > > This element is not currently used by the perf decoder. > > Tested on Linux 5.7-rc1. > > Signed-off-by: Mike Leach <mike.leach@linaro.org> > --- > tools/perf/util/cs-etm-decoder/cs-etm-decoder.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c > index cd92a99eb89d..da4737cbc2ab 100644 > --- a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c > +++ b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c > @@ -564,6 +564,10 @@ static ocsd_datapath_resp_t cs_etm_decoder__gen_trace_elem_printer( > resp = cs_etm_decoder__set_tid(etmq, packet_queue, > elem, trace_chan_id); > break; > + /* Unused packet types */ > +#if OCSD_VER_NUM >= 0x0E00 > + case OCSD_GEN_TRC_ELEM_I_RANGE_NOPATH: > +#endif I don't think use macros to distinguish OpenCSD version number is a good idea, this will get more and more code to checking version number if later have more these kinds improvement and finally it's hard to maintain. Sugget just simply add the new case for OCSD_GEN_TRC_ELEM_I_RANGE_NOPATH, considering if user uses an old version OpenCSD and doesn't output this new element, the new added case doesn't introduce issue for old OpenCSD lib. Futhermore, suggest to change the code in tools/build/feature/test-libopencsd.c, so can reflect the kernel 5.7 to require OpenCSD v0.14.0 or later version when build perf. Thanks, Leo > case OCSD_GEN_TRC_ELEM_ADDR_NACC: > case OCSD_GEN_TRC_ELEM_CYCLE_COUNT: > case OCSD_GEN_TRC_ELEM_ADDR_UNKNOWN: > -- > 2.17.1 > > _______________________________________________ > CoreSight mailing list > CoreSight@lists.linaro.org > https://lists.linaro.org/mailman/listinfo/coresight
On Thu, 16 Apr 2020 at 20:11, Leo Yan <leo.yan@linaro.org> wrote: > > Hi Mike, > > On Wed, Apr 15, 2020 at 09:14:20PM +0100, Mike Leach wrote: > > OpenCSD version v0.14.0 adds in a new output element. This is represented > > by a new value in the generic element type enum, which must be added to > > the handling code in perf cs-etm-decoder to prevent build errors due to > > build options on the perf project. > > > > This element is not currently used by the perf decoder. > > > > Tested on Linux 5.7-rc1. > > > > Signed-off-by: Mike Leach <mike.leach@linaro.org> > > --- > > tools/perf/util/cs-etm-decoder/cs-etm-decoder.c | 4 ++++ > > 1 file changed, 4 insertions(+) > > > > diff --git a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c > > index cd92a99eb89d..da4737cbc2ab 100644 > > --- a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c > > +++ b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c > > @@ -564,6 +564,10 @@ static ocsd_datapath_resp_t cs_etm_decoder__gen_trace_elem_printer( > > resp = cs_etm_decoder__set_tid(etmq, packet_queue, > > elem, trace_chan_id); > > break; > > + /* Unused packet types */ > > +#if OCSD_VER_NUM >= 0x0E00 > > + case OCSD_GEN_TRC_ELEM_I_RANGE_NOPATH: > > +#endif > > I don't think use macros to distinguish OpenCSD version number is a > good idea, this will get more and more code to checking version number > if later have more these kinds improvement and finally it's hard to > maintain. I agree. > > Sugget just simply add the new case for > OCSD_GEN_TRC_ELEM_I_RANGE_NOPATH, considering if user uses an old version > OpenCSD and doesn't output this new element, the new added case doesn't > introduce issue for old OpenCSD lib. > > Futhermore, suggest to change the code in > tools/build/feature/test-libopencsd.c, so can reflect the kernel 5.7 > to require OpenCSD v0.14.0 or later version when build perf. As Leo pointed out, I think we should just continue dealing with new versions of the library in test-libopencsd.c. > > Thanks, > Leo > > > case OCSD_GEN_TRC_ELEM_ADDR_NACC: > > case OCSD_GEN_TRC_ELEM_CYCLE_COUNT: > > case OCSD_GEN_TRC_ELEM_ADDR_UNKNOWN: > > -- > > 2.17.1 > > > > _______________________________________________ > > CoreSight mailing list > > CoreSight@lists.linaro.org > > https://lists.linaro.org/mailman/listinfo/coresight
HI Mathieu, Leo. Agreed - I'll fix up to use only the latest Thanks Mike On Tue, 21 Apr 2020 at 17:52, Mathieu Poirier <mathieu.poirier@linaro.org> wrote: > > On Thu, 16 Apr 2020 at 20:11, Leo Yan <leo.yan@linaro.org> wrote: > > > > Hi Mike, > > > > On Wed, Apr 15, 2020 at 09:14:20PM +0100, Mike Leach wrote: > > > OpenCSD version v0.14.0 adds in a new output element. This is represented > > > by a new value in the generic element type enum, which must be added to > > > the handling code in perf cs-etm-decoder to prevent build errors due to > > > build options on the perf project. > > > > > > This element is not currently used by the perf decoder. > > > > > > Tested on Linux 5.7-rc1. > > > > > > Signed-off-by: Mike Leach <mike.leach@linaro.org> > > > --- > > > tools/perf/util/cs-etm-decoder/cs-etm-decoder.c | 4 ++++ > > > 1 file changed, 4 insertions(+) > > > > > > diff --git a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c > > > index cd92a99eb89d..da4737cbc2ab 100644 > > > --- a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c > > > +++ b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c > > > @@ -564,6 +564,10 @@ static ocsd_datapath_resp_t cs_etm_decoder__gen_trace_elem_printer( > > > resp = cs_etm_decoder__set_tid(etmq, packet_queue, > > > elem, trace_chan_id); > > > break; > > > + /* Unused packet types */ > > > +#if OCSD_VER_NUM >= 0x0E00 > > > + case OCSD_GEN_TRC_ELEM_I_RANGE_NOPATH: > > > +#endif > > > > I don't think use macros to distinguish OpenCSD version number is a > > good idea, this will get more and more code to checking version number > > if later have more these kinds improvement and finally it's hard to > > maintain. > > I agree. > > > > > Sugget just simply add the new case for > > OCSD_GEN_TRC_ELEM_I_RANGE_NOPATH, considering if user uses an old version > > OpenCSD and doesn't output this new element, the new added case doesn't > > introduce issue for old OpenCSD lib. > > > > Futhermore, suggest to change the code in > > tools/build/feature/test-libopencsd.c, so can reflect the kernel 5.7 > > to require OpenCSD v0.14.0 or later version when build perf. > > As Leo pointed out, I think we should just continue dealing with new > versions of the library in test-libopencsd.c. > > > > > Thanks, > > Leo > > > > > case OCSD_GEN_TRC_ELEM_ADDR_NACC: > > > case OCSD_GEN_TRC_ELEM_CYCLE_COUNT: > > > case OCSD_GEN_TRC_ELEM_ADDR_UNKNOWN: > > > -- > > > 2.17.1 > > > > > > _______________________________________________ > > > CoreSight mailing list > > > CoreSight@lists.linaro.org > > > https://lists.linaro.org/mailman/listinfo/coresight
diff --git a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c index cd92a99eb89d..da4737cbc2ab 100644 --- a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c +++ b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c @@ -564,6 +564,10 @@ static ocsd_datapath_resp_t cs_etm_decoder__gen_trace_elem_printer( resp = cs_etm_decoder__set_tid(etmq, packet_queue, elem, trace_chan_id); break; + /* Unused packet types */ +#if OCSD_VER_NUM >= 0x0E00 + case OCSD_GEN_TRC_ELEM_I_RANGE_NOPATH: +#endif case OCSD_GEN_TRC_ELEM_ADDR_NACC: case OCSD_GEN_TRC_ELEM_CYCLE_COUNT: case OCSD_GEN_TRC_ELEM_ADDR_UNKNOWN:
OpenCSD version v0.14.0 adds in a new output element. This is represented by a new value in the generic element type enum, which must be added to the handling code in perf cs-etm-decoder to prevent build errors due to build options on the perf project. This element is not currently used by the perf decoder. Tested on Linux 5.7-rc1. Signed-off-by: Mike Leach <mike.leach@linaro.org> --- tools/perf/util/cs-etm-decoder/cs-etm-decoder.c | 4 ++++ 1 file changed, 4 insertions(+)