diff mbox

[1/1] opal: fix tests dependency on libfdt

Message ID 20170525150341.11726-1-ernunes@redhat.com
State Accepted
Headers show

Commit Message

Erico Nunes May 25, 2017, 3:03 p.m. UTC 
The inclusion of some tests in src/opal/ in the latest fwts release
added a hard dependency on libfdt-devel for all architectures, otherwise
breaking the build.
libfdt-devel may not be available or wanted for all architectures, for
example it is not officially distributed with RHEL x86_64.

fwts already had an optional dependency on libfdt before and this is
handled through the use of HAVE_LIBFDT.
This handling was already done for some of the opal tests but the new
ones were trying to build regardless of HAVE_LIBFDT. This patch moves
the new ones along with the tests which will only be built if
HAVE_LIBFDT gets set during configuration, as they rely heavily on it.

Signed-off-by: Erico Nunes <ernunes@redhat.com>
Cc: Pridhiviraj Paidipeddi <ppaidipe@linux.vnet.ibm.com>
Signed-off-by: Erico Nunes <ernunes@redhat.com>
---
 src/Makefile.am            | 8 +++++---
 src/opal/power_mgmt_info.c | 2 --
 src/opal/reserv_mem.c      | 2 --
 3 files changed, 5 insertions(+), 7 deletions(-)

Comments

Colin Ian King May 25, 2017, 3:52 p.m. UTC | #1 
On 25/05/17 16:03, Erico Nunes wrote:
> The inclusion of some tests in src/opal/ in the latest fwts release
> added a hard dependency on libfdt-devel for all architectures, otherwise
> breaking the build.
> libfdt-devel may not be available or wanted for all architectures, for
> example it is not officially distributed with RHEL x86_64.
> 
> fwts already had an optional dependency on libfdt before and this is
> handled through the use of HAVE_LIBFDT.
> This handling was already done for some of the opal tests but the new
> ones were trying to build regardless of HAVE_LIBFDT. This patch moves
> the new ones along with the tests which will only be built if
> HAVE_LIBFDT gets set during configuration, as they rely heavily on it.
> 
> Signed-off-by: Erico Nunes <ernunes@redhat.com>
> Cc: Pridhiviraj Paidipeddi <ppaidipe@linux.vnet.ibm.com>
> Signed-off-by: Erico Nunes <ernunes@redhat.com>
> ---
>  src/Makefile.am            | 8 +++++---
>  src/opal/power_mgmt_info.c | 2 --
>  src/opal/reserv_mem.c      | 2 --
>  3 files changed, 5 insertions(+), 7 deletions(-)
> 
> diff --git a/src/Makefile.am b/src/Makefile.am
> index 2c2795e..e5537b4 100644
> --- a/src/Makefile.am
> +++ b/src/Makefile.am
> @@ -25,9 +25,12 @@ dt_tests = \
>  	devicetree/dt_base/dt_base.c		\
>  	devicetree/dt_sysinfo/dt_sysinfo.c
>  mem_tests = \
> -	opal/mem_info.c
> +	opal/mem_info.c				\
> +	opal/reserv_mem.c
>  cpu_tests = \
>  	opal/cpu_info.c
> +power_mgmt_tests = \
> +	opal/power_mgmt_info.c
>  endif
>  
>  if HAVE_LIBFDT
> @@ -147,8 +150,6 @@ fwts_SOURCES = main.c 				\
>  	kernel/version/version.c 		\
>  	opal/mtd_info.c				\
>  	opal/prd_info.c				\
> -	opal/power_mgmt_info.c			\
> -	opal/reserv_mem.c			\
>  	pci/aspm/aspm.c 			\
>  	pci/crs/crs.c 				\
>  	pci/maxreadreq/maxreadreq.c 		\
> @@ -166,6 +167,7 @@ fwts_SOURCES = main.c 				\
>  	$(pci_tests)				\
>  	$(mem_tests)				\
>  	$(cpu_tests)				\
> +	$(power_mgmt_tests)			\
>  	$(dt_tests)
>  
>  fwts_LDFLAGS = -lm `pkg-config --libs glib-2.0 gio-2.0`
> diff --git a/src/opal/power_mgmt_info.c b/src/opal/power_mgmt_info.c
> index 5456c43..b83b93b 100644
> --- a/src/opal/power_mgmt_info.c
> +++ b/src/opal/power_mgmt_info.c
> @@ -24,9 +24,7 @@
>  
>  #include "fwts.h"
>  
> -#ifdef HAVE_LIBFDT
>  #include <libfdt.h>
> -#endif
>  
>  #define MAX_PSTATES 256
>  
> diff --git a/src/opal/reserv_mem.c b/src/opal/reserv_mem.c
> index 49e14c4..4199448 100644
> --- a/src/opal/reserv_mem.c
> +++ b/src/opal/reserv_mem.c
> @@ -24,9 +24,7 @@
>  
>  #include "fwts.h"
>  
> -#ifdef HAVE_LIBFDT
>  #include <libfdt.h>
> -#endif
>  
>  #define CONFIG_FILENAME "/usr/local/share/fwts/platform.conf"
>  #define MAXBUF 1024
> 

I've not got the resources to test this. Pridhiviraj does this work for you?

Colin
Erico Nunes May 26, 2017, 4:21 p.m. UTC | #2 
Hi Colin,

On 05/25/2017 05:52 PM, Colin Ian King wrote:
> I've not got the resources to test this. Pridhiviraj does this work for you?

I understand that the opal tests being touched here are targeted for
power platforms, but this patch should only impact the fwts build. So, I
see the following tests that can be done on x86_64:

- Build fwts without libfdt installed in the system: broken without this
patch. With the patch, the build succeeds and the mem_info and
power_mgmt tests are not included because libfdt is unavailable. They do
not show up in 'fwts --show-tests'.

- Build fwts with libfdt (and libfdt headers): not afected, the mem_info
and power_mgmt tests show up in 'fwts --show-tests':
mem_info        OPAL MEM Info
power_mgmt      OPAL Processor Power Management DT Validation Tests


Optional libfdt dependency was already in place for other DT-dependant
tests such as 'dt_base  Base device tree validity check' or 'reserv_mem
OPAL Reserved memory DT Validation Test'. So this patch only places
mem_info and power_mgmt correctly in the build to avoid turning libfdt
from being optional to being mandatory needlessly.


Erico
ppaidipe May 29, 2017, 6:41 a.m. UTC | #3 
On 2017-05-26 21:51, Erico Nunes wrote:
> Hi Colin,
> 
> On 05/25/2017 05:52 PM, Colin Ian King wrote:
>> I've not got the resources to test this. Pridhiviraj does this work 
>> for you?
> 
> I understand that the opal tests being touched here are targeted for
> power platforms, but this patch should only impact the fwts build. So, 
> I
> see the following tests that can be done on x86_64:
> 
> - Build fwts without libfdt installed in the system: broken without 
> this
> patch. With the patch, the build succeeds and the mem_info and
> power_mgmt tests are not included because libfdt is unavailable. They 
> do
> not show up in 'fwts --show-tests'.
> 
> - Build fwts with libfdt (and libfdt headers): not afected, the 
> mem_info
> and power_mgmt tests show up in 'fwts --show-tests':
> mem_info        OPAL MEM Info
> power_mgmt      OPAL Processor Power Management DT Validation Tests
> 
> 
> Optional libfdt dependency was already in place for other DT-dependant
> tests such as 'dt_base  Base device tree validity check' or 'reserv_mem
> OPAL Reserved memory DT Validation Test'. So this patch only places
> mem_info and power_mgmt correctly in the build to avoid turning libfdt
> from being optional to being mandatory needlessly.
> 


Thanks Erico for the patch. I have tested it on Power system.
fwts builds and runs fine with the both OPAL tests being built.
So both the tests are working fine.

[console-pexpect]#./src/fwts --show-tests
Batch tests:
  bmc_info        BMC Info
  cpu_info        OPAL CPU Info
  cpufreq         CPU frequency scaling tests.
  dt_base         Base device tree validity check
  dt_sysinfo      Device tree system information test
  klog            Scan kernel log for errors and warnings.
  maxreadreq      Test firmware has set PCI Express MaxReadReq to a 
higher value on non-motherboard devices.
  mem_info        OPAL MEM Info
  mtd_info        OPAL MTD Info
  olog            Run OLOG scan and analysis checks.
  oops            Scan kernel log for Oopses.
  power_mgmt      OPAL Processor Power Management DT Validation Tests
  prd_info        OPAL Processor Recovery Diagnostics Info
  reserv_mem      OPAL Reserved memory DT Validation Test
  version         Gather kernel system information.
[console-pexpect]#./src/fwts power_mgmt
Running 1 tests, results appended to results.log
Test: OPAL Processor Power Management DT Validation Tests
   OPAL Processor Frequency States Info                    1 passed
   OPAL Processor Idle States Info                         1 passed
[console-pexpect]#./src/fwts reserv_mem
Running 1 tests, results appended to results.log
Test: OPAL Reserved memory DT Validation Test
   OPAL Reserved memory DT Validation Info                 2 failed
[console-pexpect]#git log
commit 19ab61314d6357b34fe81d9d4f0df4d6a905a262
Author: Erico Nunes <ernunes@redhat.com>
Date:   Thu May 25 17:03:41 2017 +0200

     opal: fix tests dependency on libfdt

     The inclusion of some tests in src/opal/ in the latest fwts release
     added a hard dependency on libfdt-devel for all architectures, 
otherwise
     breaking the build.
     libfdt-devel may not be available or wanted for all architectures, 
for
     example it is not officially distributed with RHEL x86_64.

     fwts already had an optional dependency on libfdt before and this is
     handled through the use of HAVE_LIBFDT.
     This handling was already done for some of the opal tests but the 
new
     ones were trying to build regardless of HAVE_LIBFDT. This patch 
moves
     the new ones along with the tests which will only be built if
     HAVE_LIBFDT gets set during configuration, as they rely heavily on 
it.

     Signed-off-by: Erico Nunes <ernunes@redhat.com>
     Cc: Pridhiviraj Paidipeddi <ppaidipe@linux.vnet.ibm.com>
     Signed-off-by: Erico Nunes <ernunes@redhat.com>

commit f4d8710dc55923de438fba912addea14bf5449ee
Author: Ivan Hu <ivan.hu@canonical.com>
Date:   Wed May 24 12:04:59 2017 +0800


Sorry Colin for the late reply.

Thanks
Pridhiviraj

> 
> Erico
Alex Hung May 29, 2017, 11:24 p.m. UTC | #4 
On 2017-05-28 11:41 PM, ppaidipe wrote:
> On 2017-05-26 21:51, Erico Nunes wrote:
>> Hi Colin,
>>
>> On 05/25/2017 05:52 PM, Colin Ian King wrote:
>>> I've not got the resources to test this. Pridhiviraj does this work 
>>> for you?
>>
>> I understand that the opal tests being touched here are targeted for
>> power platforms, but this patch should only impact the fwts build. So, I
>> see the following tests that can be done on x86_64:
>>
>> - Build fwts without libfdt installed in the system: broken without this
>> patch. With the patch, the build succeeds and the mem_info and
>> power_mgmt tests are not included because libfdt is unavailable. They do
>> not show up in 'fwts --show-tests'.
>>
>> - Build fwts with libfdt (and libfdt headers): not afected, the mem_info
>> and power_mgmt tests show up in 'fwts --show-tests':
>> mem_info        OPAL MEM Info
>> power_mgmt      OPAL Processor Power Management DT Validation Tests
>>
>>
>> Optional libfdt dependency was already in place for other DT-dependant
>> tests such as 'dt_base  Base device tree validity check' or 'reserv_mem
>> OPAL Reserved memory DT Validation Test'. So this patch only places
>> mem_info and power_mgmt correctly in the build to avoid turning libfdt
>> from being optional to being mandatory needlessly.
>>
> 
> 
> Thanks Erico for the patch. I have tested it on Power system.
> fwts builds and runs fine with the both OPAL tests being built.
> So both the tests are working fine.
> 
> [console-pexpect]#./src/fwts --show-tests
> Batch tests:
>   bmc_info        BMC Info
>   cpu_info        OPAL CPU Info
>   cpufreq         CPU frequency scaling tests.
>   dt_base         Base device tree validity check
>   dt_sysinfo      Device tree system information test
>   klog            Scan kernel log for errors and warnings.
>   maxreadreq      Test firmware has set PCI Express MaxReadReq to a 
> higher value on non-motherboard devices.
>   mem_info        OPAL MEM Info
>   mtd_info        OPAL MTD Info
>   olog            Run OLOG scan and analysis checks.
>   oops            Scan kernel log for Oopses.
>   power_mgmt      OPAL Processor Power Management DT Validation Tests
>   prd_info        OPAL Processor Recovery Diagnostics Info
>   reserv_mem      OPAL Reserved memory DT Validation Test
>   version         Gather kernel system information.
> [console-pexpect]#./src/fwts power_mgmt
> Running 1 tests, results appended to results.log
> Test: OPAL Processor Power Management DT Validation Tests
>    OPAL Processor Frequency States Info                    1 passed
>    OPAL Processor Idle States Info                         1 passed
> [console-pexpect]#./src/fwts reserv_mem
> Running 1 tests, results appended to results.log
> Test: OPAL Reserved memory DT Validation Test
>    OPAL Reserved memory DT Validation Info                 2 failed
> [console-pexpect]#git log
> commit 19ab61314d6357b34fe81d9d4f0df4d6a905a262
> Author: Erico Nunes <ernunes@redhat.com>
> Date:   Thu May 25 17:03:41 2017 +0200
> 
>      opal: fix tests dependency on libfdt
> 
>      The inclusion of some tests in src/opal/ in the latest fwts release
>      added a hard dependency on libfdt-devel for all architectures, 
> otherwise
>      breaking the build.
>      libfdt-devel may not be available or wanted for all architectures, for
>      example it is not officially distributed with RHEL x86_64.
> 
>      fwts already had an optional dependency on libfdt before and this is
>      handled through the use of HAVE_LIBFDT.
>      This handling was already done for some of the opal tests but the new
>      ones were trying to build regardless of HAVE_LIBFDT. This patch moves
>      the new ones along with the tests which will only be built if
>      HAVE_LIBFDT gets set during configuration, as they rely heavily on it.
> 
>      Signed-off-by: Erico Nunes <ernunes@redhat.com>
>      Cc: Pridhiviraj Paidipeddi <ppaidipe@linux.vnet.ibm.com>
>      Signed-off-by: Erico Nunes <ernunes@redhat.com>
> 
> commit f4d8710dc55923de438fba912addea14bf5449ee
> Author: Ivan Hu <ivan.hu@canonical.com>
> Date:   Wed May 24 12:04:59 2017 +0800
> 
> 
> Sorry Colin for the late reply.
> 
> Thanks
> Pridhiviraj
> 
>>
>> Erico
> 
> 

Thanks for testing it.

Acked-by: Alex Hung <alex.hung@canonical.com>
Colin Ian King May 30, 2017, 8:26 a.m. UTC | #5 
On 29/05/17 07:41, ppaidipe wrote:
> On 2017-05-26 21:51, Erico Nunes wrote:
>> Hi Colin,
>>
>> On 05/25/2017 05:52 PM, Colin Ian King wrote:
>>> I've not got the resources to test this. Pridhiviraj does this work
>>> for you?
>>
>> I understand that the opal tests being touched here are targeted for
>> power platforms, but this patch should only impact the fwts build. So, I
>> see the following tests that can be done on x86_64:
>>
>> - Build fwts without libfdt installed in the system: broken without this
>> patch. With the patch, the build succeeds and the mem_info and
>> power_mgmt tests are not included because libfdt is unavailable. They do
>> not show up in 'fwts --show-tests'.
>>
>> - Build fwts with libfdt (and libfdt headers): not afected, the mem_info
>> and power_mgmt tests show up in 'fwts --show-tests':
>> mem_info        OPAL MEM Info
>> power_mgmt      OPAL Processor Power Management DT Validation Tests
>>
>>
>> Optional libfdt dependency was already in place for other DT-dependant
>> tests such as 'dt_base  Base device tree validity check' or 'reserv_mem
>> OPAL Reserved memory DT Validation Test'. So this patch only places
>> mem_info and power_mgmt correctly in the build to avoid turning libfdt
>> from being optional to being mandatory needlessly.
>>
> 
> 
> Thanks Erico for the patch. I have tested it on Power system.
> fwts builds and runs fine with the both OPAL tests being built.
> So both the tests are working fine.
> 
> [console-pexpect]#./src/fwts --show-tests
> Batch tests:
>  bmc_info        BMC Info
>  cpu_info        OPAL CPU Info
>  cpufreq         CPU frequency scaling tests.
>  dt_base         Base device tree validity check
>  dt_sysinfo      Device tree system information test
>  klog            Scan kernel log for errors and warnings.
>  maxreadreq      Test firmware has set PCI Express MaxReadReq to a
> higher value on non-motherboard devices.
>  mem_info        OPAL MEM Info
>  mtd_info        OPAL MTD Info
>  olog            Run OLOG scan and analysis checks.
>  oops            Scan kernel log for Oopses.
>  power_mgmt      OPAL Processor Power Management DT Validation Tests
>  prd_info        OPAL Processor Recovery Diagnostics Info
>  reserv_mem      OPAL Reserved memory DT Validation Test
>  version         Gather kernel system information.
> [console-pexpect]#./src/fwts power_mgmt
> Running 1 tests, results appended to results.log
> Test: OPAL Processor Power Management DT Validation Tests
>   OPAL Processor Frequency States Info                    1 passed
>   OPAL Processor Idle States Info                         1 passed
> [console-pexpect]#./src/fwts reserv_mem
> Running 1 tests, results appended to results.log
> Test: OPAL Reserved memory DT Validation Test
>   OPAL Reserved memory DT Validation Info                 2 failed
> [console-pexpect]#git log
> commit 19ab61314d6357b34fe81d9d4f0df4d6a905a262
> Author: Erico Nunes <ernunes@redhat.com>
> Date:   Thu May 25 17:03:41 2017 +0200
> 
>     opal: fix tests dependency on libfdt
> 
>     The inclusion of some tests in src/opal/ in the latest fwts release
>     added a hard dependency on libfdt-devel for all architectures,
> otherwise
>     breaking the build.
>     libfdt-devel may not be available or wanted for all architectures, for
>     example it is not officially distributed with RHEL x86_64.
> 
>     fwts already had an optional dependency on libfdt before and this is
>     handled through the use of HAVE_LIBFDT.
>     This handling was already done for some of the opal tests but the new
>     ones were trying to build regardless of HAVE_LIBFDT. This patch moves
>     the new ones along with the tests which will only be built if
>     HAVE_LIBFDT gets set during configuration, as they rely heavily on it.
> 
>     Signed-off-by: Erico Nunes <ernunes@redhat.com>
>     Cc: Pridhiviraj Paidipeddi <ppaidipe@linux.vnet.ibm.com>
>     Signed-off-by: Erico Nunes <ernunes@redhat.com>
> 
> commit f4d8710dc55923de438fba912addea14bf5449ee
> Author: Ivan Hu <ivan.hu@canonical.com>
> Date:   Wed May 24 12:04:59 2017 +0800
> 
> 
> Sorry Colin for the late reply.
> 
> Thanks
> Pridhiviraj
> 
>>
>> Erico
> 
> 
Thanks for the patch and for testing.

Acked-by: Colin Ian King <colin.king@canonical.com>
diff mbox

Patch

diff --git a/src/Makefile.am b/src/Makefile.am
index 2c2795e..e5537b4 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -25,9 +25,12 @@  dt_tests = \
 	devicetree/dt_base/dt_base.c		\
 	devicetree/dt_sysinfo/dt_sysinfo.c
 mem_tests = \
-	opal/mem_info.c
+	opal/mem_info.c				\
+	opal/reserv_mem.c
 cpu_tests = \
 	opal/cpu_info.c
+power_mgmt_tests = \
+	opal/power_mgmt_info.c
 endif
 
 if HAVE_LIBFDT
@@ -147,8 +150,6 @@  fwts_SOURCES = main.c 				\
 	kernel/version/version.c 		\
 	opal/mtd_info.c				\
 	opal/prd_info.c				\
-	opal/power_mgmt_info.c			\
-	opal/reserv_mem.c			\
 	pci/aspm/aspm.c 			\
 	pci/crs/crs.c 				\
 	pci/maxreadreq/maxreadreq.c 		\
@@ -166,6 +167,7 @@  fwts_SOURCES = main.c 				\
 	$(pci_tests)				\
 	$(mem_tests)				\
 	$(cpu_tests)				\
+	$(power_mgmt_tests)			\
 	$(dt_tests)
 
 fwts_LDFLAGS = -lm `pkg-config --libs glib-2.0 gio-2.0`
diff --git a/src/opal/power_mgmt_info.c b/src/opal/power_mgmt_info.c
index 5456c43..b83b93b 100644
--- a/src/opal/power_mgmt_info.c
+++ b/src/opal/power_mgmt_info.c
@@ -24,9 +24,7 @@ 
 
 #include "fwts.h"
 
-#ifdef HAVE_LIBFDT
 #include <libfdt.h>
-#endif
 
 #define MAX_PSTATES 256
 
diff --git a/src/opal/reserv_mem.c b/src/opal/reserv_mem.c
index 49e14c4..4199448 100644
--- a/src/opal/reserv_mem.c
+++ b/src/opal/reserv_mem.c
@@ -24,9 +24,7 @@ 
 
 #include "fwts.h"
 
-#ifdef HAVE_LIBFDT
 #include <libfdt.h>
-#endif
 
 #define CONFIG_FILENAME "/usr/local/share/fwts/platform.conf"
 #define MAXBUF 1024