| Message ID | 1617694853-6881-4-git-send-email-pmorel@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | s390x: Testing SSCH, CSCH and HSCH for errors | expand |
On Tue, 6 Apr 2021 09:40:40 +0200 Pierre Morel <pmorel@linux.ibm.com> wrote: > We will have to test if a device is present for every tests > in the future. > Let's provide separate the first tests from the test loop and > skip the remaining tests if no device is present. What about the following patch description: "We keep adding tests that act upon a concrete device, and we have to test that a device is present for all of those. Instead, just skip all of the tests requiring a device if we were not able to set it up in the first place. The enumeration test will already have failed in that case." > > Signed-off-by: Pierre Morel <pmorel@linux.ibm.com> > --- > s390x/css.c | 36 ++++++++++++++---------------------- > 1 file changed, 14 insertions(+), 22 deletions(-) > (...) > @@ -336,8 +316,6 @@ static struct { > void (*func)(void); > } tests[] = { > /* The css_init test is needed to initialize the CSS Characteristics */ If you remove the css_init test from this list, the above comment does not make sense anymore :) > - { "initialize CSS (chsc)", css_init }, > - { "enumerate (stsch)", test_enumerate }, > { "enable (msch)", test_enable }, > { "sense (ssch/tsch)", test_sense }, > { "measurement block (schm)", test_schm }, > @@ -352,11 +330,25 @@ int main(int argc, char *argv[]) > > report_prefix_push("Channel Subsystem"); > enable_io_isc(0x80 >> IO_SCH_ISC); > + > + report_prefix_push("initialize CSS (chsc)"); > + css_init(); > + report_prefix_pop(); > + > + report_prefix_push("enumerate (stsch)"); > + test_enumerate(); > + report_prefix_pop(); Could we maybe have two lists of tests: one that don't require a device, and one that does? > + > + if (!test_device_sid) > + goto end; In any case, I think we should log an explicit message that we skip the remaining tests because of no device being available. > + > for (i = 0; tests[i].name; i++) { > report_prefix_push(tests[i].name); > tests[i].func(); > report_prefix_pop(); > } > + > +end: > report_prefix_pop(); > > return report_summary();
On 4/6/21 2:44 PM, Cornelia Huck wrote: > On Tue, 6 Apr 2021 09:40:40 +0200 > Pierre Morel <pmorel@linux.ibm.com> wrote: > >> We will have to test if a device is present for every tests >> in the future. >> Let's provide separate the first tests from the test loop and >> skip the remaining tests if no device is present. > > What about the following patch description: > > "We keep adding tests that act upon a concrete device, and we have to > test that a device is present for all of those. > > Instead, just skip all of the tests requiring a device if we were not > able to set it up in the first place. The enumeration test will already > have failed in that case." ok yes better. > >> >> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com> >> --- >> s390x/css.c | 36 ++++++++++++++---------------------- >> 1 file changed, 14 insertions(+), 22 deletions(-) >> > > (...) > >> @@ -336,8 +316,6 @@ static struct { >> void (*func)(void); >> } tests[] = { >> /* The css_init test is needed to initialize the CSS Characteristics */ > > If you remove the css_init test from this list, the above comment does > not make sense anymore :) grrr I thought I did remove this. will do. > >> - { "initialize CSS (chsc)", css_init }, >> - { "enumerate (stsch)", test_enumerate }, >> { "enable (msch)", test_enable }, >> { "sense (ssch/tsch)", test_sense }, >> { "measurement block (schm)", test_schm }, >> @@ -352,11 +330,25 @@ int main(int argc, char *argv[]) >> >> report_prefix_push("Channel Subsystem"); >> enable_io_isc(0x80 >> IO_SCH_ISC); >> + >> + report_prefix_push("initialize CSS (chsc)"); >> + css_init(); >> + report_prefix_pop(); >> + >> + report_prefix_push("enumerate (stsch)"); >> + test_enumerate(); >> + report_prefix_pop(); > > Could we maybe have two lists of tests: one that don't require a > device, and one that does? Yes, I can do that. > >> + >> + if (!test_device_sid) >> + goto end; > > In any case, I think we should log an explicit message that we skip the > remaining tests because of no device being available. OK, I will re-arrange the test order. Thanks, Pierre
diff --git a/s390x/css.c b/s390x/css.c index c340c53..17a6e1d 100644 --- a/s390x/css.c +++ b/s390x/css.c @@ -41,11 +41,6 @@ static void test_enable(void) { int cc; - if (!test_device_sid) { - report_skip("No device"); - return; - } - cc = css_enable(test_device_sid, IO_SCH_ISC); report(cc == 0, "Enable subchannel %08x", test_device_sid); @@ -62,11 +57,6 @@ static void test_sense(void) int ret; int len; - if (!test_device_sid) { - report_skip("No device"); - return; - } - ret = css_enable(test_device_sid, IO_SCH_ISC); if (ret) { report(0, "Could not enable the subchannel: %08x", @@ -218,11 +208,6 @@ static void test_schm_fmt0(void) struct measurement_block_format0 *mb0; int shared_mb_size = 2 * sizeof(struct measurement_block_format0); - if (!test_device_sid) { - report_skip("No device"); - return; - } - /* Allocate zeroed Measurement block */ mb0 = alloc_io_mem(shared_mb_size, 0); if (!mb0) { @@ -289,11 +274,6 @@ static void test_schm_fmt1(void) { struct measurement_block_format1 *mb1; - if (!test_device_sid) { - report_skip("No device"); - return; - } - if (!css_test_general_feature(CSSC_EXTENDED_MEASUREMENT_BLOCK)) { report_skip("Extended measurement block not available"); return; @@ -336,8 +316,6 @@ static struct { void (*func)(void); } tests[] = { /* The css_init test is needed to initialize the CSS Characteristics */ - { "initialize CSS (chsc)", css_init }, - { "enumerate (stsch)", test_enumerate }, { "enable (msch)", test_enable }, { "sense (ssch/tsch)", test_sense }, { "measurement block (schm)", test_schm }, @@ -352,11 +330,25 @@ int main(int argc, char *argv[]) report_prefix_push("Channel Subsystem"); enable_io_isc(0x80 >> IO_SCH_ISC); + + report_prefix_push("initialize CSS (chsc)"); + css_init(); + report_prefix_pop(); + + report_prefix_push("enumerate (stsch)"); + test_enumerate(); + report_prefix_pop(); + + if (!test_device_sid) + goto end; + for (i = 0; tests[i].name; i++) { report_prefix_push(tests[i].name); tests[i].func(); report_prefix_pop(); } + +end: report_prefix_pop(); return report_summary();
We will have to test if a device is present for every tests in the future. Let's provide separate the first tests from the test loop and skip the remaining tests if no device is present. Signed-off-by: Pierre Morel <pmorel@linux.ibm.com> --- s390x/css.c | 36 ++++++++++++++---------------------- 1 file changed, 14 insertions(+), 22 deletions(-)