#4150 assigned enhancement

Delete or fix shell command fdt (rtems-fdt.c is untested and not in user documentation)

Reported by: Frank Kuehndel Owned by:
Priority: low Milestone: Indefinite
Component: shell Version:
Severity: normal Keywords: shell, fdt
Cc: Blocked By:
Blocking:

Description

The shell seems to once have a command 'fdt' (flattened device tree). Its code resides in

cpukit/libmisc/rtems-fdt/rtems-fdt.c
cpukit/libmisc/rtems-fdt/rtems-fdt-shell.c
cpukit/include/rtems/rtems-fdt-shell.h

At the time of writing, this code is not used. No code calls this and it is not part of the shell anymore. It is not even listed in the shell command macros in cpukit/include/rtems/shellconfig.h.

Note that RTEMS has a large set of fdt functions which are also used - in cpukit/include/libfdt.h. Yet, these are not a shell command.

I tried to test the 'fdt' shell command but loading the flattened device tree blob from testsuites/libtests/libfdt01/some.c (see testsuites/libtests/libfdt01/some.dts) with 'fdt ld <blob-filename>' resulted in an infinite loop. Yet, maybe I made something wrong. (RTEMS 6, HEAD at df9cc1aee87da6c6ba41d52454fa5f45fba74501, arm/realview_pbx_a9_qemu)

See also bug #3156 "rtems-fdt probably not supported on 64-bit PowerPC"

I suggest to either fix this code and make it a working shell command again (by for example replacing the functions in rtems-fdt.c by those from libfdt.h) or to delete the dead code.

Why not simply letting dead code hang around in the sources?

Because even dead code needs maintenance. New compiler versions, compiler flags, code analysis tools etc. report warnings or errors even in dead code and these issues must be fixed and - if possible - tested. Moreover, some dead code (not 'rtems-fdt.c') uses internals from other "living" RTEMS code. If that living code is changed, the dead code needs to be adapted to these changes. Conclusion: Dead code wastes efforts and makes maintenance harder.

Change History (2)

comment:1 Changed on 10/16/20 at 16:35:53 by Frank Kuehndel

The code is not dead. Chris Johns wrote on the mailing list:

With this command I know users on Zynq platforms that use FDT to map PL logic to drivers. The command is very much in use and is used as a PS to PL testing method. (Reference: Email)

In this case, I suggest the same as Chris did earlier:

  • Add it again to the shell.
  • Add it to the shell documentation.
  • Add a test for it. A simple smoke unit-test which loads an fdt (blob) and invokes the most important functions in file cpukit/libmisc/rtems-fdt/rtems-fdt.c would be great.

comment:2 Changed on 10/16/20 at 17:44:56 by Joel Sherrill

Summary: Delete or fix shell command fdt (rtems-fdt.c is dead code)Delete or fix shell command fdt (rtems-fdt.c is untested and not in user documentation)

Calling this dead code is inappropriate. At best it is deactivated code which has the bug of not having a configuration option and documentation.

RTEMS provides functionality which may be used by applications. Sometimes we have functionality which has no tests or user documentation. That is usually an oversight we should have caught when it was merged. This does not make it dead.

Using this logic for dead code, one could delete a lot of driver code (e.g. can, spacewire, 1553, gpio) and network code because there is no direct test in RTEMS of that.

We should be very careful using terms like dead code which have very specific meanings in the context of safety certification standards. See https://de.mathworks.com/products/polyspace/dead-code-coverage.html for a decent discussion which cites the DO-178 definition.

The bug here is missing documentation and configuration option.

Note: See TracTickets for help on using tickets.