On Fri, Oct 30, 2020 at 05:50:35PM +0000, Alexandru Elisei wrote:
> Hi Eric,
>
> On 10/30/20 5:09 PM, Auger Eric wrote:
> > Hi Alexandru,
> >
> > On 10/30/20 4:59 PM, Alexandru Elisei wrote:
> > [..]
> >>>> + spe.align = 1 << spe.align;
> >>>> +
> >>>> + pmsidr = read_sysreg_s(SYS_PMSIDR_EL1);
> >>>> +
> >>>> + interval = (pmsidr >> SYS_PMSIDR_EL1_INTERVAL_SHIFT) & SYS_PMSIDR_EL1_INTERVAL_MASK;
> >>>> + spe.min_interval = spe_min_interval(interval);
> >>>> +
> >>>> + spe.max_record_size = (pmsidr >> SYS_PMSIDR_EL1_MAXSIZE_SHIFT) & \
> >>>> + SYS_PMSIDR_EL1_MAXSIZE_MASK;
> >>>> + spe.max_record_size = 1 << spe.max_record_size;
> >>>> +
> >>>> + spe.countsize = (pmsidr >> SYS_PMSIDR_EL1_COUNTSIZE_SHIFT) & \
> >>>> + SYS_PMSIDR_EL1_COUNTSIZE_MASK;
> >>>> +
> >>>> + spe.fl_cap = pmsidr & BIT(SYS_PMSIDR_EL1_FL_SHIFT);
> >>>> + spe.ft_cap = pmsidr & BIT(SYS_PMSIDR_EL1_FT_SHIFT);
> >>>> + spe.fe_cap = pmsidr & BIT(SYS_PMSIDR_EL1_FE_SHIFT);
> >>> Why did you remove the report_info() section? I think those human
> >>> readable info can be useful.
> >> I made them part of the test. Since the architecture says they are 1, I think
> >> making sure their value matches is more useful than printing something that the
> >> architecture guarantees.
> > OK for those caps which are always 1 anyway but I was more thinking about
> >
> > report_info("Align= %d bytes, Min Interval=%d Single record Max Size =
> > %d bytes", spe.align, spe.min_interval, spe.maxsize);
> >
> > I'd prefer to keep it.
>
> Oh, I think I see what you mean, I chose to print them using printf in main().
> This is very similar to what the timer test does, only it does it directly in
> main(), instead of calling another function (print_timer_info(), in the case of
> the timer test). I can move the printfs to spe_probe() if that's what you prefer.
Please use report_info(). When tests fail it's popular to diff the failing
results against the passing results. Some output changes each run, even
in the same environment, and some output really isn't that important. We
should keep important, non-test test results in INFO messages. Then, when
diffing, we can filter out anything without a prefix.
(BTW, I was cc'ed, or maybe bcc'ed, on this series from the beginning.)
Thanks,
drew
>
> Thanks,
>
> Alex
>
