| Message ID | 1432192507.646890.191605150773.1.gpush@pablo |
|---|---|
| State | Rejected |
| Headers | show |
Thanks Jeremy, I'm on vacation for a few days, so I will give these a thorough review middle of next week. Just one question, what is the expected behaviour if dtc is not installed. Also, I think the device-tree-compiler package should be added to the debian control Colin On 21/05/15 08:15, Jeremy Kerr wrote: > Check that we can compile the device tree with dtc. > > Signed-off-by: Jeremy Kerr <jk@ozlabs.org> > > --- > src/Makefile.am | 1 > src/devicetree/dtc/dtc.c | 76 +++++++++++++++++++++++++++++++++++++++ > 2 files changed, 77 insertions(+) > > diff --git a/src/Makefile.am b/src/Makefile.am > index 62c63ef..4a2622b 100644 > --- a/src/Makefile.am > +++ b/src/Makefile.am > @@ -78,6 +78,7 @@ fwts_SOURCES = main.c \ > cpu/nx/nx.c \ > cpu/msr/msr.c \ > cpu/microcode/microcode.c \ > + devicetree/dtc/dtc.c \ > dmi/dmicheck/dmicheck.c \ > hotkey/hotkey/hotkey.c \ > hpet/hpet_check/hpet_check.c \ > diff --git a/src/devicetree/dtc/dtc.c b/src/devicetree/dtc/dtc.c > new file mode 100644 > index 0000000..c1420dc > --- /dev/null > +++ b/src/devicetree/dtc/dtc.c > @@ -0,0 +1,76 @@ > +/* > + * Copyright (C) 2014 Jeremy Kerr <jk@ozlabs.org> > + * > + * This program is free software; you can redistribute it and/or > + * modify it under the terms of the GNU General Public License > + * as published by the Free Software Foundation; either version 2 > + * of the License, or (at your option) any later version. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + * > + * You should have received a copy of the GNU General Public License > + * along with this program; if not, write to the Free Software > + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA > + * 02110-1301, USA. > + * > + */ > + > +#define _GNU_SOURCE > +#include <assert.h> > +#include <stdio.h> > + > +#include "fwts.h" > + > +static const char *devicetree_path = "/proc/device-tree"; > + > +static int dtc_check_valid(fwts_framework *fw) > +{ > + int rc, status; > + char *command; > + > + rc = asprintf(&command, "dtc -I fs -o /dev/null %s", devicetree_path); > + assert(rc > 0); > + > + rc = fwts_exec(command, &status); > + free(command); > + > + if (rc != FWTS_OK || !WIFEXITED(status) || WEXITSTATUS(status) != 0) { > + fwts_failed(fw, LOG_LEVEL_HIGH, "DeviceTreeDtcParseFail", > + "dtc reports device tree errors"); > + return FWTS_ERROR; > + } > + > + fwts_passed(fw, "dtc test passed"); > + return FWTS_OK; > +} > + > +static int dtc_init(fwts_framework *fw) > +{ > + int rc, status; > + > + rc = fwts_exec("echo '/dts-v1/; / { };' | dtc -o /dev/null", &status); > + > + if (rc != FWTS_OK || !WIFEXITED(status) || WEXITSTATUS(status) != 0) { > + fwts_aborted(fw, "Failed to run dtc"); > + return FWTS_ABORTED; > + } > + > + return FWTS_OK; > +} > + > +static fwts_framework_minor_test dtc_tests[] = { > + { dtc_check_valid, "Check device tree validity with dtc" }, > + { NULL, NULL }, > +}; > + > +static fwts_framework_ops dtc_ops = { > + .description = "Device tree compilation test", > + .minor_tests = dtc_tests, > + .init = dtc_init, > +}; > + > +FWTS_REGISTER_FEATURES("dtc", &dtc_ops, FWTS_TEST_ANYTIME, > + FWTS_FLAG_BATCH, FWTS_FW_FEATURE_DEVICETREE); >
Hi Colin, > I'm on vacation for a few days, so I will give these a thorough review > middle of next week. Great, thanks. Enjoy the vacation! > Just one question, what is the expected behaviour if dtc is not > installed. Also, I think the device-tree-compiler package should be > added to the debian control At present, we just abort from the ->init, we could skip instead if that's cleaner. However, since this test depends on FWTS_FW_FEATURE_DEVICETREE, that'll only happen on dt-based machines. If we do add it to the debian/control dependencies, we might just want that for the powerpc and ppc64el architectures (and maybe ARM?). No use pulling it in for x86. Cheers, Jeremy
On 21/05/15 08:15, Jeremy Kerr wrote: > Check that we can compile the device tree with dtc. > > Signed-off-by: Jeremy Kerr <jk@ozlabs.org> > > --- > src/Makefile.am | 1 > src/devicetree/dtc/dtc.c | 76 +++++++++++++++++++++++++++++++++++++++ > 2 files changed, 77 insertions(+) > > diff --git a/src/Makefile.am b/src/Makefile.am > index 62c63ef..4a2622b 100644 > --- a/src/Makefile.am > +++ b/src/Makefile.am > @@ -78,6 +78,7 @@ fwts_SOURCES = main.c \ > cpu/nx/nx.c \ > cpu/msr/msr.c \ > cpu/microcode/microcode.c \ > + devicetree/dtc/dtc.c \ > dmi/dmicheck/dmicheck.c \ > hotkey/hotkey/hotkey.c \ > hpet/hpet_check/hpet_check.c \ > diff --git a/src/devicetree/dtc/dtc.c b/src/devicetree/dtc/dtc.c > new file mode 100644 > index 0000000..c1420dc > --- /dev/null > +++ b/src/devicetree/dtc/dtc.c > @@ -0,0 +1,76 @@ > +/* > + * Copyright (C) 2014 Jeremy Kerr <jk@ozlabs.org> > + * > + * This program is free software; you can redistribute it and/or > + * modify it under the terms of the GNU General Public License > + * as published by the Free Software Foundation; either version 2 > + * of the License, or (at your option) any later version. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + * > + * You should have received a copy of the GNU General Public License > + * along with this program; if not, write to the Free Software > + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA > + * 02110-1301, USA. > + * > + */ > + > +#define _GNU_SOURCE > +#include <assert.h> > +#include <stdio.h> > + > +#include "fwts.h" > + > +static const char *devicetree_path = "/proc/device-tree"; > + > +static int dtc_check_valid(fwts_framework *fw) > +{ > + int rc, status; > + char *command; > + > + rc = asprintf(&command, "dtc -I fs -o /dev/null %s", devicetree_path); > + assert(rc > 0); I'd prefer not to have asserts, any unexpected errors should be reported via fwts_error() > + > + rc = fwts_exec(command, &status); > + free(command); > + > + if (rc != FWTS_OK || !WIFEXITED(status) || WEXITSTATUS(status) != 0) { > + fwts_failed(fw, LOG_LEVEL_HIGH, "DeviceTreeDtcParseFail", > + "dtc reports device tree errors"); > + return FWTS_ERROR; > + } Is it possible to log the device tree errors in the fwts log, that would be useful rather than just a failure message. > + > + fwts_passed(fw, "dtc test passed"); > + return FWTS_OK; > +} > + > +static int dtc_init(fwts_framework *fw) > +{ > + int rc, status; > + > + rc = fwts_exec("echo '/dts-v1/; / { };' | dtc -o /dev/null", &status); if dtc is a required tool, then it needs to be added into the Debian dependencies > + > + if (rc != FWTS_OK || !WIFEXITED(status) || WEXITSTATUS(status) != 0) { > + fwts_aborted(fw, "Failed to run dtc"); > + return FWTS_ABORTED; > + } > + > + return FWTS_OK; > +} > + > +static fwts_framework_minor_test dtc_tests[] = { > + { dtc_check_valid, "Check device tree validity with dtc" }, > + { NULL, NULL }, > +}; > + > +static fwts_framework_ops dtc_ops = { > + .description = "Device tree compilation test", > + .minor_tests = dtc_tests, > + .init = dtc_init, > +}; > + > +FWTS_REGISTER_FEATURES("dtc", &dtc_ops, FWTS_TEST_ANYTIME, > + FWTS_FLAG_BATCH, FWTS_FW_FEATURE_DEVICETREE); >
diff --git a/src/Makefile.am b/src/Makefile.am index 62c63ef..4a2622b 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -78,6 +78,7 @@ fwts_SOURCES = main.c \ cpu/nx/nx.c \ cpu/msr/msr.c \ cpu/microcode/microcode.c \ + devicetree/dtc/dtc.c \ dmi/dmicheck/dmicheck.c \ hotkey/hotkey/hotkey.c \ hpet/hpet_check/hpet_check.c \ diff --git a/src/devicetree/dtc/dtc.c b/src/devicetree/dtc/dtc.c new file mode 100644 index 0000000..c1420dc --- /dev/null +++ b/src/devicetree/dtc/dtc.c @@ -0,0 +1,76 @@ +/* + * Copyright (C) 2014 Jeremy Kerr <jk@ozlabs.org> + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License + * as published by the Free Software Foundation; either version 2 + * of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA + * 02110-1301, USA. + * + */ + +#define _GNU_SOURCE +#include <assert.h> +#include <stdio.h> + +#include "fwts.h" + +static const char *devicetree_path = "/proc/device-tree"; + +static int dtc_check_valid(fwts_framework *fw) +{ + int rc, status; + char *command; + + rc = asprintf(&command, "dtc -I fs -o /dev/null %s", devicetree_path); + assert(rc > 0); + + rc = fwts_exec(command, &status); + free(command); + + if (rc != FWTS_OK || !WIFEXITED(status) || WEXITSTATUS(status) != 0) { + fwts_failed(fw, LOG_LEVEL_HIGH, "DeviceTreeDtcParseFail", + "dtc reports device tree errors"); + return FWTS_ERROR; + } + + fwts_passed(fw, "dtc test passed"); + return FWTS_OK; +} + +static int dtc_init(fwts_framework *fw) +{ + int rc, status; + + rc = fwts_exec("echo '/dts-v1/; / { };' | dtc -o /dev/null", &status); + + if (rc != FWTS_OK || !WIFEXITED(status) || WEXITSTATUS(status) != 0) { + fwts_aborted(fw, "Failed to run dtc"); + return FWTS_ABORTED; + } + + return FWTS_OK; +} + +static fwts_framework_minor_test dtc_tests[] = { + { dtc_check_valid, "Check device tree validity with dtc" }, + { NULL, NULL }, +}; + +static fwts_framework_ops dtc_ops = { + .description = "Device tree compilation test", + .minor_tests = dtc_tests, + .init = dtc_init, +}; + +FWTS_REGISTER_FEATURES("dtc", &dtc_ops, FWTS_TEST_ANYTIME, + FWTS_FLAG_BATCH, FWTS_FW_FEATURE_DEVICETREE);
Check that we can compile the device tree with dtc. Signed-off-by: Jeremy Kerr <jk@ozlabs.org> --- src/Makefile.am | 1 src/devicetree/dtc/dtc.c | 76 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 77 insertions(+)