| Message ID | 1454981583-31872-22-git-send-email-al.stone@linaro.org |
|---|---|
| State | Rejected |
| Headers | show |
On 09/02/16 01:33, Al Stone wrote: > Now that the tests have been resequenced, added to, and generally > overhauled, clean up some variables in test1 that are no longer > useful. > > Signed-off-by: Al Stone <al.stone@linaro.org> > --- > src/acpi/fadt/fadt.c | 4 ---- > 1 file changed, 4 deletions(-) > > diff --git a/src/acpi/fadt/fadt.c b/src/acpi/fadt/fadt.c > index 05205cb..fbc71fd 100644 > --- a/src/acpi/fadt/fadt.c > +++ b/src/acpi/fadt/fadt.c > @@ -1514,8 +1514,6 @@ static void acpi_table_check_fadt_sleep_status_reg(fwts_framework *fw) > > static int fadt_test1(fwts_framework *fw) > { > - bool passed = true; > - > acpi_table_check_fadt_firmware_ctrl(fw); > acpi_table_check_fadt_dsdt(fw); > acpi_table_check_fadt_reserved(fw); > @@ -1589,8 +1587,6 @@ static int fadt_test1(fwts_framework *fw) > */ > fwts_log_info(fw, "FADT Hypervisor Vendor Identity is %" PRIu64, > fadt->hypervisor_id); > - if (passed) > - fwts_passed(fw, "No issues found in FADT table."); > > return FWTS_OK; > } > Acked-by: Colin Ian King <colin.king@canonical.com> Thanks Al for all these improvements. Are there any fwts-test patches to come later? Colin
On 02/09/2016 05:34 AM, Colin Ian King wrote: > On 09/02/16 01:33, Al Stone wrote: >> Now that the tests have been resequenced, added to, and generally >> overhauled, clean up some variables in test1 that are no longer >> useful. >> >> Signed-off-by: Al Stone <al.stone@linaro.org> >> --- >> src/acpi/fadt/fadt.c | 4 ---- >> 1 file changed, 4 deletions(-) >> >> diff --git a/src/acpi/fadt/fadt.c b/src/acpi/fadt/fadt.c >> index 05205cb..fbc71fd 100644 >> --- a/src/acpi/fadt/fadt.c >> +++ b/src/acpi/fadt/fadt.c >> @@ -1514,8 +1514,6 @@ static void acpi_table_check_fadt_sleep_status_reg(fwts_framework *fw) >> >> static int fadt_test1(fwts_framework *fw) >> { >> - bool passed = true; >> - >> acpi_table_check_fadt_firmware_ctrl(fw); >> acpi_table_check_fadt_dsdt(fw); >> acpi_table_check_fadt_reserved(fw); >> @@ -1589,8 +1587,6 @@ static int fadt_test1(fwts_framework *fw) >> */ >> fwts_log_info(fw, "FADT Hypervisor Vendor Identity is %" PRIu64, >> fadt->hypervisor_id); >> - if (passed) >> - fwts_passed(fw, "No issues found in FADT table."); >> >> return FWTS_OK; >> } >> > Acked-by: Colin Ian King <colin.king@canonical.com> > > Thanks Al for all these improvements. Are there any fwts-test patches > to come later? > > Colin > Thanks for all the review. This turned out a lot bigger than I thought it might so I appreciate the patience involved. I did run make check but I did not get any regression test failures that I had not already seen and reported or fixed (there's a previous series called "Update several regression tests" that still needs to be ACKd and pulled in, btw). I'll double check that, of course, and send anything I find along. Or, were you expecting new sections in fwts-test? I hadn't really thought that about it, if that's what's being asked....is there a rule of thumb the project follows that applies here?
On 09/02/16 23:30, Al Stone wrote: > On 02/09/2016 05:34 AM, Colin Ian King wrote: >> On 09/02/16 01:33, Al Stone wrote: >>> Now that the tests have been resequenced, added to, and generally >>> overhauled, clean up some variables in test1 that are no longer >>> useful. >>> >>> Signed-off-by: Al Stone <al.stone@linaro.org> >>> --- >>> src/acpi/fadt/fadt.c | 4 ---- >>> 1 file changed, 4 deletions(-) >>> >>> diff --git a/src/acpi/fadt/fadt.c b/src/acpi/fadt/fadt.c >>> index 05205cb..fbc71fd 100644 >>> --- a/src/acpi/fadt/fadt.c >>> +++ b/src/acpi/fadt/fadt.c >>> @@ -1514,8 +1514,6 @@ static void acpi_table_check_fadt_sleep_status_reg(fwts_framework *fw) >>> >>> static int fadt_test1(fwts_framework *fw) >>> { >>> - bool passed = true; >>> - >>> acpi_table_check_fadt_firmware_ctrl(fw); >>> acpi_table_check_fadt_dsdt(fw); >>> acpi_table_check_fadt_reserved(fw); >>> @@ -1589,8 +1587,6 @@ static int fadt_test1(fwts_framework *fw) >>> */ >>> fwts_log_info(fw, "FADT Hypervisor Vendor Identity is %" PRIu64, >>> fadt->hypervisor_id); >>> - if (passed) >>> - fwts_passed(fw, "No issues found in FADT table."); >>> >>> return FWTS_OK; >>> } >>> >> Acked-by: Colin Ian King <colin.king@canonical.com> >> >> Thanks Al for all these improvements. Are there any fwts-test patches >> to come later? >> >> Colin >> > > Thanks for all the review. This turned out a lot bigger than I thought > it might so I appreciate the patience involved. > > I did run make check but I did not get any regression test failures that > I had not already seen and reported or fixed (there's a previous series > called "Update several regression tests" that still needs to be ACKd and > pulled in, btw). I'll double check that, of course, and send anything > I find along. > > Or, were you expecting new sections in fwts-test? I hadn't really thought > that about it, if that's what's being asked....is there a rule of thumb > the project follows that applies here? > No worries about extra tests for now. Let's see how this patch set shakes down on a range of firmware over the next few release cycles. Colin
On 02/12/2016 03:09 AM, Colin Ian King wrote: > On 09/02/16 23:30, Al Stone wrote: >> On 02/09/2016 05:34 AM, Colin Ian King wrote: >>> On 09/02/16 01:33, Al Stone wrote: >>>> Now that the tests have been resequenced, added to, and generally >>>> overhauled, clean up some variables in test1 that are no longer >>>> useful. >>>> >>>> Signed-off-by: Al Stone <al.stone@linaro.org> >>>> --- >>>> src/acpi/fadt/fadt.c | 4 ---- >>>> 1 file changed, 4 deletions(-) >>>> >>>> diff --git a/src/acpi/fadt/fadt.c b/src/acpi/fadt/fadt.c >>>> index 05205cb..fbc71fd 100644 >>>> --- a/src/acpi/fadt/fadt.c >>>> +++ b/src/acpi/fadt/fadt.c >>>> @@ -1514,8 +1514,6 @@ static void acpi_table_check_fadt_sleep_status_reg(fwts_framework *fw) >>>> >>>> static int fadt_test1(fwts_framework *fw) >>>> { >>>> - bool passed = true; >>>> - >>>> acpi_table_check_fadt_firmware_ctrl(fw); >>>> acpi_table_check_fadt_dsdt(fw); >>>> acpi_table_check_fadt_reserved(fw); >>>> @@ -1589,8 +1587,6 @@ static int fadt_test1(fwts_framework *fw) >>>> */ >>>> fwts_log_info(fw, "FADT Hypervisor Vendor Identity is %" PRIu64, >>>> fadt->hypervisor_id); >>>> - if (passed) >>>> - fwts_passed(fw, "No issues found in FADT table."); >>>> >>>> return FWTS_OK; >>>> } >>>> >>> Acked-by: Colin Ian King <colin.king@canonical.com> >>> >>> Thanks Al for all these improvements. Are there any fwts-test patches >>> to come later? >>> >>> Colin >>> >> >> Thanks for all the review. This turned out a lot bigger than I thought >> it might so I appreciate the patience involved. >> >> I did run make check but I did not get any regression test failures that >> I had not already seen and reported or fixed (there's a previous series >> called "Update several regression tests" that still needs to be ACKd and >> pulled in, btw). I'll double check that, of course, and send anything >> I find along. >> >> Or, were you expecting new sections in fwts-test? I hadn't really thought >> that about it, if that's what's being asked....is there a rule of thumb >> the project follows that applies here? >> > No worries about extra tests for now. Let's see how this patch set > shakes down on a range of firmware over the next few release cycles. > > Colin > Okey dokey. Shall I send a v2 of this set or are we copacetic?
On 2016-02-09 09:33 AM, Al Stone wrote: > Now that the tests have been resequenced, added to, and generally > overhauled, clean up some variables in test1 that are no longer > useful. > > Signed-off-by: Al Stone <al.stone@linaro.org> > --- > src/acpi/fadt/fadt.c | 4 ---- > 1 file changed, 4 deletions(-) > > diff --git a/src/acpi/fadt/fadt.c b/src/acpi/fadt/fadt.c > index 05205cb..fbc71fd 100644 > --- a/src/acpi/fadt/fadt.c > +++ b/src/acpi/fadt/fadt.c > @@ -1514,8 +1514,6 @@ static void acpi_table_check_fadt_sleep_status_reg(fwts_framework *fw) > > static int fadt_test1(fwts_framework *fw) > { > - bool passed = true; > - > acpi_table_check_fadt_firmware_ctrl(fw); > acpi_table_check_fadt_dsdt(fw); > acpi_table_check_fadt_reserved(fw); > @@ -1589,8 +1587,6 @@ static int fadt_test1(fwts_framework *fw) > */ > fwts_log_info(fw, "FADT Hypervisor Vendor Identity is %" PRIu64, > fadt->hypervisor_id); > - if (passed) > - fwts_passed(fw, "No issues found in FADT table."); > > return FWTS_OK; > } > Acked-by: Alex Hung <alex.hung@canonical.com>
diff --git a/src/acpi/fadt/fadt.c b/src/acpi/fadt/fadt.c index 05205cb..fbc71fd 100644 --- a/src/acpi/fadt/fadt.c +++ b/src/acpi/fadt/fadt.c @@ -1514,8 +1514,6 @@ static void acpi_table_check_fadt_sleep_status_reg(fwts_framework *fw) static int fadt_test1(fwts_framework *fw) { - bool passed = true; - acpi_table_check_fadt_firmware_ctrl(fw); acpi_table_check_fadt_dsdt(fw); acpi_table_check_fadt_reserved(fw); @@ -1589,8 +1587,6 @@ static int fadt_test1(fwts_framework *fw) */ fwts_log_info(fw, "FADT Hypervisor Vendor Identity is %" PRIu64, fadt->hypervisor_id); - if (passed) - fwts_passed(fw, "No issues found in FADT table."); return FWTS_OK; }
Now that the tests have been resequenced, added to, and generally overhauled, clean up some variables in test1 that are no longer useful. Signed-off-by: Al Stone <al.stone@linaro.org> --- src/acpi/fadt/fadt.c | 4 ---- 1 file changed, 4 deletions(-)