linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] dtc: create tool to diff device trees
@ 2015-12-31  9:12 Frank Rowand
  2016-01-04 21:29 ` Rob Herring
  0 siblings, 1 reply; 4+ messages in thread
From: Frank Rowand @ 2015-12-31  9:12 UTC (permalink / raw)
  To: Rob Herring; +Cc: Grant Likely, david, devicetree, Linux Kernel list

From: Frank Rowand <frank.rowand@sonymobile.com>

Create script to diff device trees.

The device tree can be in any of the forms recognized by the dtc compiler:
  - source
  - binary blob
  - file system tree (from /proc/devicetree)

If the device tree is a source file, then it is pre-processed in the
same way as it would be when built in the linux kernel source tree
before diffing.

Signed-off-by: Frank Rowand <frank.rowand@sonymobile.com>
---

Tools to develop and debug device tree are somewhat inadequate.  This is a
small step in improving the situation.

Rationale for and examples of using the script are provided in slides
1 - 78 of the elce 2015 presentation "Solving Device Tree Issues",
which can be found at:

   http://elinux.org/images/0/04/Dt_debugging_elce_2015_151006_0421.pdf

(The script was named dtdiff instead of dtx_diff in the presentation.)


 scripts/dtc/dtx_diff |  368 +++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 368 insertions(+)

Index: b/scripts/dtc/dtx_diff
===================================================================
--- /dev/null
+++ b/scripts/dtc/dtx_diff
@@ -0,0 +1,368 @@
+#! /bin/bash
+
+# Copyright (C) 2015 Frank Rowand
+#
+# 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; version 2 of the License.
+
+
+usage() {
+
+	# use spaces instead of tabs in the usage message
+	cat >&2 <<eod
+
+Usage:
+
+   `basename $0` DTx
+        decompile DTx
+
+   `basename $0` DTx_1 DTx_2
+        diff DTx_1 and DTx_2
+
+
+       -f           print full dts in diff (--unified=99999)
+       -h           synonym for --help
+       -help        synonym for --help
+      --help        print this message and exit
+       -s SRCTREE   linux kernel source tree is at path SRCTREE
+                        (default is current directory)
+       -S           linux kernel source tree is at root of current git repo
+       -u           unsorted, do not sort DTx
+
+
+Each DTx is processed by the dtc compiler to produce a sorted dts source
+file.  If DTx is a dts source file then it is pre-processed in the same
+manner as done for the compile of the dts source file in the Linux kernel
+build system ('#include' and '/include/' directives are processed).
+
+If two DTx are provided, the resulting dts source files are diffed.
+
+If DTx is a directory, it is treated as a DT subtree, such as
+  /proc/device-tree.
+
+If DTx contains the binary blob magic value in the first four bytes,
+  it is treated as a binary blob (aka .dtb or FDT).
+
+Otherwise DTx is treated as a dts source file (aka .dts).
+
+   If this script is not run from the root of the linux source tree,
+   and DTx utilizes '#include' or '/include/' then the path of the
+   linux source tree can be provided by '-s SRCTREE' or '-S' so that
+   include paths will be set properly.
+
+   The shell variable \${ARCH} must provide the architecture containing
+   the dts source file for include paths to be set properly for '#include'
+   or '/include/' to be processed.
+
+   If DTx_1 and DTx_2 are in different architectures, then this script
+   may not work since \${ARCH} is part of the include path.  Two possible
+   workarounds:
+
+      `basename $0` \\
+          <(ARCH=arch_of_dtx_1 `basename $0` DTx_1) \\
+          <(ARCH=arch_of_dtx_2 `basename $0` DTx_2)
+
+      `basename $0` ARCH=arch_of_dtx_1 DTx_1 >tmp_dtx_1.dts
+      `basename $0` ARCH=arch_of_dtx_2 DTx_2 >tmp_dtx_2.dts
+      `basename $0` tmp_dtx_1.dts tmp_dtx_2.dts
+      rm tmp_dtx_1.dts tmp_dtx_2.dts
+
+   If DTx_1 and DTx_2 are in different directories, then this script will
+   add the path of DTx_1 and DTx_2 to the include paths.  If DTx_2 includes
+   a local file that exists in both the path of DTx_1 and DTx_2 then the
+   file in the path of DTx_1 will incorrectly be included.  Possible
+   workaround:
+
+      `basename $0` DTx_1 >tmp_dtx_1.dts
+      `basename $0` DTx_2 >tmp_dtx_2.dts
+      `basename $0` tmp_dtx_1.dts tmp_dtx_2.dts
+      rm tmp_dtx_1.dts tmp_dtx_2.dts
+
+eod
+}
+
+
+compile_to_dts() {
+
+	dtx="$1"
+
+	if [ -d "${dtx}" ] ; then
+
+		# -----  input is file tree
+
+		if ( ! ${DTC} -I fs ${dtx} ) ; then
+			exit 3
+		fi
+
+	elif [ -f "${dtx}" ] && [ -r "${dtx}" ] ; then
+
+		magic=`hexdump -n 4 -e '/1 "%02x"' ${dtx}`
+		if [ "${magic}" = "d00dfeed" ] ; then
+
+			# -----  input is FDT (binary blob)
+
+			if ( ! ${DTC} -I dtb ${dtx} ) ; then
+				exit 3
+			fi
+
+		else
+
+			# -----  input is DTS (source)
+
+			if ( cpp ${cpp_flags} -x assembler-with-cpp ${dtx} | ${DTC} -I dts ) ; then
+				return
+			fi
+
+			echo ""                                                          >&2
+			echo "Possible hints to resolve the above error:"                >&2
+			echo "  (hints might not fix the problem)"                       >&2
+
+			hint_given=0
+
+			if [ "${ARCH}" = "" ] ; then
+				hint_given=1
+				echo ""                                                      >&2
+				echo "  shell variable \$ARCH not set"                       >&2
+			fi
+
+			dtx_arch=`echo "/${dtx}" | sed -e 's|.*/arch/||' -e 's|/.*||'`
+
+			if [ "${dtx_arch}" != ""  -a "${dtx_arch}" != "${ARCH}" ] ; then
+				hint_given=1
+				echo ""                                                      >&2
+				echo "  architecture ${dtx_arch} is in file path, but"       >&2
+				echo "  does not match shell variable \$ARCH (${ARCH})"      >&2
+			fi
+
+			if [ ! -d ${srctree}/arch/${ARCH} ] ; then
+				hint_given=1
+				echo ""                                                      >&2
+				echo "  ${srctree}/arch/${ARCH}/ does not exist"             >&2
+				echo "  Is \$ARCH='${ARCH}' correct?"                        >&2
+				echo "  Possible fix: use '-s' option"                       >&2
+
+				git_root=`git rev-parse --show-toplevel 2>/dev/null`
+				if [ -d ${git_root}/arch/ ] ; then
+					echo "  Possible fix: use '-S' option"                   >&2
+				fi
+			fi
+
+			if [ $hint_given = 0 ] ; then
+				echo ""                                                      >&2
+				echo "  No hints available."                                 >&2
+			fi
+
+			echo ""                                                          >&2
+
+			exit 3
+
+		fi
+	else
+		echo ""                                                              >&2
+		echo "ERROR: ${dtx} does not exist or is not readable"               >&2
+		echo ""                                                              >&2
+		exit 2
+	fi
+
+}
+
+
+# -----  start of script
+
+cmd_diff=0
+diff_flags="-u"
+dtx_file_1=""
+dtx_file_2=""
+dtc_sort="-s"
+help=0
+srctree=""
+
+
+while [ $# -gt 0 ] ; do
+
+	case $1 in
+
+	-f )
+		diff_flags="--unified=999999"
+		shift
+		;;
+
+	-h | -help | --help )
+		help=1
+		shift
+		;;
+
+	-s )
+		srctree="$2"
+		shift 2
+		;;
+
+	-S )
+		git_root=`git rev-parse --show-toplevel 2>/dev/null`
+		srctree="${git_root}"
+		shift
+		;;
+
+	-u )
+		dtc_sort=""
+		shift
+		;;
+
+	*)
+		if [ "${dtx_file_1}"  = "" ] ; then
+			dtx_file_1="$1"
+		elif [ "${dtx_file_2}" = "" ] ; then
+			dtx_file_2="$1"
+		else
+			echo ""                                                          >&2
+			echo "ERROR: Unexpected parameter: $1"                           >&2
+			echo ""                                                          >&2
+			exit 2
+		fi
+		shift
+		;;
+
+	esac
+
+done
+
+if [ "${srctree}" = "" ] ; then
+	srctree="."
+fi
+
+if [ "${dtx_file_2}" != "" ]; then
+	cmd_diff=1
+fi
+
+if (( ${help} )) ; then
+	usage
+	exit 1
+fi
+
+# this must follow check for ${help}
+if [ "${dtx_file_1}" = "" ]; then
+	echo ""                                                                  >&2
+	echo "ERROR: parameter DTx required"                                     >&2
+	echo ""                                                                  >&2
+	exit 2
+fi
+
+
+# -----  prefer dtc from linux kernel, allow fallback to dtc in $PATH
+
+if [ "${KBUILD_OUTPUT:0:2}" = ".." ] ; then
+	__KBUILD_OUTPUT="${srctree}/${KBUILD_OUTPUT}"
+elif [ "${KBUILD_OUTPUT}" = "" ] ; then
+	__KBUILD_OUTPUT="."
+else
+	__KBUILD_OUTPUT="${KBUILD_OUTPUT}"
+fi
+
+DTC="${__KBUILD_OUTPUT}/scripts/dtc/dtc"
+
+if [ ! -x ${DTC} ] ; then
+	__DTC="dtc"
+	if ( ! which ${__DTC} >/dev/null ) ; then
+
+		# use spaces instead of tabs in the error message
+		cat >&2 <<eod
+
+ERROR: unable to find a 'dtc' program
+
+   Preferred 'dtc' (built from Linux kernel source tree) was not found or
+   is not executable.
+
+      'dtc' is: ${DTC}
+
+      If it does not exist, create it from the root of the Linux source tree:
+
+         'make scripts'.
+
+      If not at the root of the Linux kernel source tree -s SRCTREE or -S
+      may need to be specified to find 'dtc'.
+
+      If 'O=\${dir}' is specified in your Linux builds, this script requires
+      'export KBUILD_OUTPUT=\${dir}' or add \${dir}/scripts/dtc to \$PATH
+      before running.
+
+      If \${KBUILD_OUTPUT} is a relative path, then '-s SRCDIR', -S, or run
+      this script from the root of the Linux kernel source tree is required.
+
+   Fallback '${__DTC}' was also not in \${PATH} or is not executable.
+
+eod
+		exit 2
+	fi
+	DTC=${__DTC}
+fi
+
+
+# -----  cpp and dtc flags same as for linux source tree build of .dtb files,
+#        plus directories of the dtx file(s)
+
+dtx_path_1_dtc_include="-i `dirname ${dtx_file_1}`"
+
+dtx_path_2_dtc_include=""
+if (( ${cmd_diff} )) ; then
+	dtx_path_2_dtc_include="-i `dirname ${dtx_file_2}`"
+fi
+
+cpp_flags="\
+	-nostdinc                                              \
+	-I${srctree}/arch/${ARCH}/boot/dts                     \
+	-I${srctree}/arch/${ARCH}/boot/dts/include             \
+	-I${srctree}/arch/${ARCH}/boot/dts/include/dt-bindings \
+	-I${srctree}/drivers/of/testcase-data                  \
+	-undef -D__DTS__"
+
+_dts_makefile=""
+
+if [ -f ${srctree}/arch/${ARCH}/boot/dts/Makefile ] ; then
+	_dts_makefile="${srctree}/arch/${ARCH}/boot/dts/Makefile"
+elif [ -f ${srctree}/arch/${ARCH}/boot/Makefile ] ; then
+	# arch/powerpc
+	_dts_makefile="${srctree}/arch/${ARCH}/boot/Makefile"
+fi
+
+if [ "${_dts_makefile}" != "" ] ; then
+	arch_dtc_flags=`grep DTC_FLAGS ${_dts_makefile} \
+		| grep -v "^#"                              \
+		| sed -e 's|.*=||' `
+else
+	arch_dtc_flags=""
+fi
+
+dtc_flags="\
+	-i ${srctree}/arch/${ARCH}/boot/dts/ \
+	-i ${srctree}/kernel/dts             \
+	${dtx_path_1_dtc_include}            \
+	${dtx_path_2_dtc_include}            \
+	${arch_dtc_flags}"
+
+DTC="${DTC} ${dtc_flags} -O dts -qq -f ${dtc_sort} -o -"
+
+
+# -----  do the diff or decompile
+
+if (( ${cmd_diff} )) ; then
+
+	diff ${diff_flags} \
+		<(compile_to_dts "${dtx_file_1}") \
+		<(compile_to_dts "${dtx_file_2}")
+
+else
+
+	compile_to_dts "${dtx_file_1}"
+
+fi
+
+
+# ==============================================================================
+# ______________________________________________________________________________
+# - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
+# ______________________________________________________________________________
+#  vi config follows:
+#
+#  add "set modelines" to ~/.exrc or ~/.vimrc to set tabs automatically
+#  ex:set tabstop=4 shiftwidth=4 sts=4:

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] dtc: create tool to diff device trees
  2015-12-31  9:12 [PATCH] dtc: create tool to diff device trees Frank Rowand
@ 2016-01-04 21:29 ` Rob Herring
  2016-01-07  6:15   ` Frank Rowand
  2016-01-07 18:40   ` Frank Rowand
  0 siblings, 2 replies; 4+ messages in thread
From: Rob Herring @ 2016-01-04 21:29 UTC (permalink / raw)
  To: Frank Rowand; +Cc: Grant Likely, David Gibson, devicetree, Linux Kernel list

On Thu, Dec 31, 2015 at 3:12 AM, Frank Rowand <frowand.list@gmail.com> wrote:
> From: Frank Rowand <frank.rowand@sonymobile.com>
>
> Create script to diff device trees.
>
> The device tree can be in any of the forms recognized by the dtc compiler:
>   - source
>   - binary blob
>   - file system tree (from /proc/devicetree)
>
> If the device tree is a source file, then it is pre-processed in the
> same way as it would be when built in the linux kernel source tree
> before diffing.

In general, I'd like to see some of this move to dtc and the kernel
dependencies minimized.

>
> Signed-off-by: Frank Rowand <frank.rowand@sonymobile.com>
> ---
>
> Tools to develop and debug device tree are somewhat inadequate.  This is a
> small step in improving the situation.
>
> Rationale for and examples of using the script are provided in slides
> 1 - 78 of the elce 2015 presentation "Solving Device Tree Issues",
> which can be found at:
>
>    http://elinux.org/images/0/04/Dt_debugging_elce_2015_151006_0421.pdf
>
> (The script was named dtdiff instead of dtx_diff in the presentation.)
>
>
>  scripts/dtc/dtx_diff |  368 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 368 insertions(+)
>
> Index: b/scripts/dtc/dtx_diff
> ===================================================================
> --- /dev/null
> +++ b/scripts/dtc/dtx_diff
> @@ -0,0 +1,368 @@
> +#! /bin/bash

Add "-e" by default?

> +
> +# Copyright (C) 2015 Frank Rowand
> +#
> +# 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; version 2 of the License.
> +
> +
> +usage() {
> +
> +       # use spaces instead of tabs in the usage message
> +       cat >&2 <<eod
> +
> +Usage:
> +
> +   `basename $0` DTx
> +        decompile DTx
> +
> +   `basename $0` DTx_1 DTx_2
> +        diff DTx_1 and DTx_2
> +
> +
> +       -f           print full dts in diff (--unified=99999)
> +       -h           synonym for --help
> +       -help        synonym for --help
> +      --help        print this message and exit
> +       -s SRCTREE   linux kernel source tree is at path SRCTREE
> +                        (default is current directory)
> +       -S           linux kernel source tree is at root of current git repo

Can't this be detected? There are some cases of kernel trees which are
not in git.

> +       -u           unsorted, do not sort DTx
> +
> +
> +Each DTx is processed by the dtc compiler to produce a sorted dts source
> +file.  If DTx is a dts source file then it is pre-processed in the same
> +manner as done for the compile of the dts source file in the Linux kernel
> +build system ('#include' and '/include/' directives are processed).
> +
> +If two DTx are provided, the resulting dts source files are diffed.
> +
> +If DTx is a directory, it is treated as a DT subtree, such as
> +  /proc/device-tree.
> +
> +If DTx contains the binary blob magic value in the first four bytes,
> +  it is treated as a binary blob (aka .dtb or FDT).
> +
> +Otherwise DTx is treated as a dts source file (aka .dts).
> +
> +   If this script is not run from the root of the linux source tree,
> +   and DTx utilizes '#include' or '/include/' then the path of the
> +   linux source tree can be provided by '-s SRCTREE' or '-S' so that
> +   include paths will be set properly.
> +
> +   The shell variable \${ARCH} must provide the architecture containing
> +   the dts source file for include paths to be set properly for '#include'
> +   or '/include/' to be processed.
> +
> +   If DTx_1 and DTx_2 are in different architectures, then this script
> +   may not work since \${ARCH} is part of the include path.  Two possible
> +   workarounds:
> +
> +      `basename $0` \\
> +          <(ARCH=arch_of_dtx_1 `basename $0` DTx_1) \\
> +          <(ARCH=arch_of_dtx_2 `basename $0` DTx_2)
> +
> +      `basename $0` ARCH=arch_of_dtx_1 DTx_1 >tmp_dtx_1.dts
> +      `basename $0` ARCH=arch_of_dtx_2 DTx_2 >tmp_dtx_2.dts
> +      `basename $0` tmp_dtx_1.dts tmp_dtx_2.dts
> +      rm tmp_dtx_1.dts tmp_dtx_2.dts
> +
> +   If DTx_1 and DTx_2 are in different directories, then this script will
> +   add the path of DTx_1 and DTx_2 to the include paths.  If DTx_2 includes
> +   a local file that exists in both the path of DTx_1 and DTx_2 then the
> +   file in the path of DTx_1 will incorrectly be included.  Possible
> +   workaround:
> +
> +      `basename $0` DTx_1 >tmp_dtx_1.dts
> +      `basename $0` DTx_2 >tmp_dtx_2.dts
> +      `basename $0` tmp_dtx_1.dts tmp_dtx_2.dts
> +      rm tmp_dtx_1.dts tmp_dtx_2.dts
> +
> +eod
> +}
> +
> +
> +compile_to_dts() {
> +
> +       dtx="$1"
> +
> +       if [ -d "${dtx}" ] ; then
> +
> +               # -----  input is file tree
> +
> +               if ( ! ${DTC} -I fs ${dtx} ) ; then
> +                       exit 3
> +               fi
> +
> +       elif [ -f "${dtx}" ] && [ -r "${dtx}" ] ; then
> +
> +               magic=`hexdump -n 4 -e '/1 "%02x"' ${dtx}`
> +               if [ "${magic}" = "d00dfeed" ] ; then

Perhaps this auto-detection should be part of dtc?

> +
> +                       # -----  input is FDT (binary blob)
> +
> +                       if ( ! ${DTC} -I dtb ${dtx} ) ; then
> +                               exit 3
> +                       fi
> +
> +               else
> +
> +                       # -----  input is DTS (source)
> +
> +                       if ( cpp ${cpp_flags} -x assembler-with-cpp ${dtx} | ${DTC} -I dts ) ; then
> +                               return
> +                       fi
> +
> +                       echo ""                                                          >&2
> +                       echo "Possible hints to resolve the above error:"                >&2
> +                       echo "  (hints might not fix the problem)"                       >&2
> +
> +                       hint_given=0
> +
> +                       if [ "${ARCH}" = "" ] ; then
> +                               hint_given=1
> +                               echo ""                                                      >&2
> +                               echo "  shell variable \$ARCH not set"                       >&2
> +                       fi
> +
> +                       dtx_arch=`echo "/${dtx}" | sed -e 's|.*/arch/||' -e 's|/.*||'`
> +
> +                       if [ "${dtx_arch}" != ""  -a "${dtx_arch}" != "${ARCH}" ] ; then
> +                               hint_given=1
> +                               echo ""                                                      >&2
> +                               echo "  architecture ${dtx_arch} is in file path, but"       >&2
> +                               echo "  does not match shell variable \$ARCH (${ARCH})"      >&2
> +                       fi
> +
> +                       if [ ! -d ${srctree}/arch/${ARCH} ] ; then
> +                               hint_given=1
> +                               echo ""                                                      >&2
> +                               echo "  ${srctree}/arch/${ARCH}/ does not exist"             >&2
> +                               echo "  Is \$ARCH='${ARCH}' correct?"                        >&2
> +                               echo "  Possible fix: use '-s' option"                       >&2
> +
> +                               git_root=`git rev-parse --show-toplevel 2>/dev/null`
> +                               if [ -d ${git_root}/arch/ ] ; then
> +                                       echo "  Possible fix: use '-S' option"                   >&2
> +                               fi
> +                       fi
> +
> +                       if [ $hint_given = 0 ] ; then
> +                               echo ""                                                      >&2
> +                               echo "  No hints available."                                 >&2
> +                       fi
> +
> +                       echo ""                                                          >&2
> +
> +                       exit 3
> +
> +               fi
> +       else
> +               echo ""                                                              >&2
> +               echo "ERROR: ${dtx} does not exist or is not readable"               >&2
> +               echo ""                                                              >&2
> +               exit 2
> +       fi
> +
> +}
> +
> +
> +# -----  start of script
> +
> +cmd_diff=0
> +diff_flags="-u"
> +dtx_file_1=""
> +dtx_file_2=""
> +dtc_sort="-s"
> +help=0
> +srctree=""
> +
> +
> +while [ $# -gt 0 ] ; do
> +
> +       case $1 in
> +
> +       -f )
> +               diff_flags="--unified=999999"
> +               shift
> +               ;;
> +
> +       -h | -help | --help )
> +               help=1
> +               shift
> +               ;;
> +
> +       -s )
> +               srctree="$2"
> +               shift 2
> +               ;;
> +
> +       -S )
> +               git_root=`git rev-parse --show-toplevel 2>/dev/null`
> +               srctree="${git_root}"
> +               shift
> +               ;;
> +
> +       -u )
> +               dtc_sort=""
> +               shift
> +               ;;
> +
> +       *)
> +               if [ "${dtx_file_1}"  = "" ] ; then
> +                       dtx_file_1="$1"
> +               elif [ "${dtx_file_2}" = "" ] ; then
> +                       dtx_file_2="$1"
> +               else
> +                       echo ""                                                          >&2
> +                       echo "ERROR: Unexpected parameter: $1"                           >&2
> +                       echo ""                                                          >&2
> +                       exit 2
> +               fi
> +               shift
> +               ;;
> +
> +       esac
> +
> +done
> +
> +if [ "${srctree}" = "" ] ; then
> +       srctree="."
> +fi
> +
> +if [ "${dtx_file_2}" != "" ]; then
> +       cmd_diff=1
> +fi
> +
> +if (( ${help} )) ; then
> +       usage
> +       exit 1
> +fi
> +
> +# this must follow check for ${help}
> +if [ "${dtx_file_1}" = "" ]; then
> +       echo ""                                                                  >&2
> +       echo "ERROR: parameter DTx required"                                     >&2
> +       echo ""                                                                  >&2
> +       exit 2
> +fi
> +
> +
> +# -----  prefer dtc from linux kernel, allow fallback to dtc in $PATH
> +
> +if [ "${KBUILD_OUTPUT:0:2}" = ".." ] ; then
> +       __KBUILD_OUTPUT="${srctree}/${KBUILD_OUTPUT}"
> +elif [ "${KBUILD_OUTPUT}" = "" ] ; then
> +       __KBUILD_OUTPUT="."
> +else
> +       __KBUILD_OUTPUT="${KBUILD_OUTPUT}"
> +fi
> +
> +DTC="${__KBUILD_OUTPUT}/scripts/dtc/dtc"
> +
> +if [ ! -x ${DTC} ] ; then
> +       __DTC="dtc"
> +       if ( ! which ${__DTC} >/dev/null ) ; then
> +
> +               # use spaces instead of tabs in the error message
> +               cat >&2 <<eod
> +
> +ERROR: unable to find a 'dtc' program
> +
> +   Preferred 'dtc' (built from Linux kernel source tree) was not found or
> +   is not executable.
> +
> +      'dtc' is: ${DTC}
> +
> +      If it does not exist, create it from the root of the Linux source tree:
> +
> +         'make scripts'.
> +
> +      If not at the root of the Linux kernel source tree -s SRCTREE or -S
> +      may need to be specified to find 'dtc'.
> +
> +      If 'O=\${dir}' is specified in your Linux builds, this script requires
> +      'export KBUILD_OUTPUT=\${dir}' or add \${dir}/scripts/dtc to \$PATH
> +      before running.
> +
> +      If \${KBUILD_OUTPUT} is a relative path, then '-s SRCDIR', -S, or run
> +      this script from the root of the Linux kernel source tree is required.
> +
> +   Fallback '${__DTC}' was also not in \${PATH} or is not executable.
> +
> +eod
> +               exit 2
> +       fi
> +       DTC=${__DTC}
> +fi
> +
> +
> +# -----  cpp and dtc flags same as for linux source tree build of .dtb files,
> +#        plus directories of the dtx file(s)
> +
> +dtx_path_1_dtc_include="-i `dirname ${dtx_file_1}`"
> +
> +dtx_path_2_dtc_include=""
> +if (( ${cmd_diff} )) ; then
> +       dtx_path_2_dtc_include="-i `dirname ${dtx_file_2}`"
> +fi
> +
> +cpp_flags="\
> +       -nostdinc                                              \
> +       -I${srctree}/arch/${ARCH}/boot/dts                     \
> +       -I${srctree}/arch/${ARCH}/boot/dts/include             \
> +       -I${srctree}/arch/${ARCH}/boot/dts/include/dt-bindings \

kernel doesn't use this path. The less dependency on ARCH, the better.
The dependency in the kernel is artificial.

> +       -I${srctree}/drivers/of/testcase-data                  \
> +       -undef -D__DTS__"
> +
> +_dts_makefile=""
> +
> +if [ -f ${srctree}/arch/${ARCH}/boot/dts/Makefile ] ; then
> +       _dts_makefile="${srctree}/arch/${ARCH}/boot/dts/Makefile"
> +elif [ -f ${srctree}/arch/${ARCH}/boot/Makefile ] ; then
> +       # arch/powerpc
> +       _dts_makefile="${srctree}/arch/${ARCH}/boot/Makefile"

We need to move PPC to be in line with other arches rather than
working around it.

> +fi
> +
> +if [ "${_dts_makefile}" != "" ] ; then
> +       arch_dtc_flags=`grep DTC_FLAGS ${_dts_makefile} \

We should just kill DTC_FLAGS being per arch. Plus it is only used for
padding, so it is irrelevant to this tool.

> +               | grep -v "^#"                              \
> +               | sed -e 's|.*=||' `
> +else
> +       arch_dtc_flags=""
> +fi
> +
> +dtc_flags="\
> +       -i ${srctree}/arch/${ARCH}/boot/dts/ \
> +       -i ${srctree}/kernel/dts             \
> +       ${dtx_path_1_dtc_include}            \
> +       ${dtx_path_2_dtc_include}            \
> +       ${arch_dtc_flags}"
> +
> +DTC="${DTC} ${dtc_flags} -O dts -qq -f ${dtc_sort} -o -"
> +
> +
> +# -----  do the diff or decompile
> +
> +if (( ${cmd_diff} )) ; then
> +
> +       diff ${diff_flags} \
> +               <(compile_to_dts "${dtx_file_1}") \
> +               <(compile_to_dts "${dtx_file_2}")
> +
> +else
> +
> +       compile_to_dts "${dtx_file_1}"
> +
> +fi
> +
> +
> +# ==============================================================================
> +# ______________________________________________________________________________
> +# - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
> +# ______________________________________________________________________________
> +#  vi config follows:
> +#
> +#  add "set modelines" to ~/.exrc or ~/.vimrc to set tabs automatically
> +#  ex:set tabstop=4 shiftwidth=4 sts=4:

Kernel standard is 8 characters. I assume that applies to bash files too.

Rob

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] dtc: create tool to diff device trees
  2016-01-04 21:29 ` Rob Herring
@ 2016-01-07  6:15   ` Frank Rowand
  2016-01-07 18:40   ` Frank Rowand
  1 sibling, 0 replies; 4+ messages in thread
From: Frank Rowand @ 2016-01-07  6:15 UTC (permalink / raw)
  To: Rob Herring; +Cc: Grant Likely, David Gibson, devicetree, Linux Kernel list

Hi Rob,

Thanks for the comments.

On 1/4/2016 1:29 PM, Rob Herring wrote:
> On Thu, Dec 31, 2015 at 3:12 AM, Frank Rowand <frowand.list@gmail.com> wrote:
>> From: Frank Rowand <frank.rowand@sonymobile.com>
>>
>> Create script to diff device trees.
>>
>> The device tree can be in any of the forms recognized by the dtc compiler:
>>   - source
>>   - binary blob
>>   - file system tree (from /proc/devicetree)
>>
>> If the device tree is a source file, then it is pre-processed in the
>> same way as it would be when built in the linux kernel source tree
>> before diffing.
> 
> In general, I'd like to see some of this move to dtc and the kernel
> dependencies minimized.
> 
>>
>> Signed-off-by: Frank Rowand <frank.rowand@sonymobile.com>
>> ---
>>
>> Tools to develop and debug device tree are somewhat inadequate.  This is a
>> small step in improving the situation.
>>
>> Rationale for and examples of using the script are provided in slides
>> 1 - 78 of the elce 2015 presentation "Solving Device Tree Issues",
>> which can be found at:
>>
>>    http://elinux.org/images/0/04/Dt_debugging_elce_2015_151006_0421.pdf
>>
>> (The script was named dtdiff instead of dtx_diff in the presentation.)
>>
>>
>>  scripts/dtc/dtx_diff |  368 +++++++++++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 368 insertions(+)
>>
>> Index: b/scripts/dtc/dtx_diff
>> ===================================================================
>> --- /dev/null
>> +++ b/scripts/dtc/dtx_diff
>> @@ -0,0 +1,368 @@
>> +#! /bin/bash
> 
> Add "-e" by default?

Nope.  There are several commands in the script that can fail in
normal operation, but the script needs to continue.

> 
>> +
>> +# Copyright (C) 2015 Frank Rowand
>> +#
>> +# 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; version 2 of the License.
>> +
>> +
>> +usage() {
>> +
>> +       # use spaces instead of tabs in the usage message
>> +       cat >&2 <<eod
>> +
>> +Usage:
>> +
>> +   `basename $0` DTx
>> +        decompile DTx
>> +
>> +   `basename $0` DTx_1 DTx_2
>> +        diff DTx_1 and DTx_2
>> +
>> +
>> +       -f           print full dts in diff (--unified=99999)
>> +       -h           synonym for --help
>> +       -help        synonym for --help
>> +      --help        print this message and exit
>> +       -s SRCTREE   linux kernel source tree is at path SRCTREE
>> +                        (default is current directory)
>> +       -S           linux kernel source tree is at root of current git repo
> 
> Can't this be detected? There are some cases of kernel trees which are
> not in git.

-S is just an easier option to use than -s if the script is run from within
the linux kernel source tree if it is a git repo.  If the script is run
from the root of the tree then there is no need to specify -s or -S -- it
just works.

> 
>> +       -u           unsorted, do not sort DTx
>> +
>> +
>> +Each DTx is processed by the dtc compiler to produce a sorted dts source
>> +file.  If DTx is a dts source file then it is pre-processed in the same
>> +manner as done for the compile of the dts source file in the Linux kernel
>> +build system ('#include' and '/include/' directives are processed).
>> +
>> +If two DTx are provided, the resulting dts source files are diffed.
>> +
>> +If DTx is a directory, it is treated as a DT subtree, such as
>> +  /proc/device-tree.
>> +
>> +If DTx contains the binary blob magic value in the first four bytes,
>> +  it is treated as a binary blob (aka .dtb or FDT).
>> +
>> +Otherwise DTx is treated as a dts source file (aka .dts).
>> +
>> +   If this script is not run from the root of the linux source tree,
>> +   and DTx utilizes '#include' or '/include/' then the path of the
>> +   linux source tree can be provided by '-s SRCTREE' or '-S' so that
>> +   include paths will be set properly.
>> +
>> +   The shell variable \${ARCH} must provide the architecture containing
>> +   the dts source file for include paths to be set properly for '#include'
>> +   or '/include/' to be processed.
>> +
>> +   If DTx_1 and DTx_2 are in different architectures, then this script
>> +   may not work since \${ARCH} is part of the include path.  Two possible
>> +   workarounds:
>> +
>> +      `basename $0` \\
>> +          <(ARCH=arch_of_dtx_1 `basename $0` DTx_1) \\
>> +          <(ARCH=arch_of_dtx_2 `basename $0` DTx_2)
>> +
>> +      `basename $0` ARCH=arch_of_dtx_1 DTx_1 >tmp_dtx_1.dts
>> +      `basename $0` ARCH=arch_of_dtx_2 DTx_2 >tmp_dtx_2.dts
>> +      `basename $0` tmp_dtx_1.dts tmp_dtx_2.dts
>> +      rm tmp_dtx_1.dts tmp_dtx_2.dts
>> +
>> +   If DTx_1 and DTx_2 are in different directories, then this script will
>> +   add the path of DTx_1 and DTx_2 to the include paths.  If DTx_2 includes
>> +   a local file that exists in both the path of DTx_1 and DTx_2 then the
>> +   file in the path of DTx_1 will incorrectly be included.  Possible
>> +   workaround:
>> +
>> +      `basename $0` DTx_1 >tmp_dtx_1.dts
>> +      `basename $0` DTx_2 >tmp_dtx_2.dts
>> +      `basename $0` tmp_dtx_1.dts tmp_dtx_2.dts
>> +      rm tmp_dtx_1.dts tmp_dtx_2.dts
>> +
>> +eod
>> +}
>> +
>> +
>> +compile_to_dts() {
>> +
>> +       dtx="$1"
>> +
>> +       if [ -d "${dtx}" ] ; then
>> +
>> +               # -----  input is file tree
>> +
>> +               if ( ! ${DTC} -I fs ${dtx} ) ; then
>> +                       exit 3
>> +               fi
>> +
>> +       elif [ -f "${dtx}" ] && [ -r "${dtx}" ] ; then
>> +
>> +               magic=`hexdump -n 4 -e '/1 "%02x"' ${dtx}`
>> +               if [ "${magic}" = "d00dfeed" ] ; then
> 
> Perhaps this auto-detection should be part of dtc?

The script needs to invoke dtc differently for a source dts (it
needs to preprocess with cpp) so we need this check here
anyway.

> 
>> +
>> +                       # -----  input is FDT (binary blob)
>> +
>> +                       if ( ! ${DTC} -I dtb ${dtx} ) ; then
>> +                               exit 3
>> +                       fi
>> +
>> +               else
>> +
>> +                       # -----  input is DTS (source)
>> +
>> +                       if ( cpp ${cpp_flags} -x assembler-with-cpp ${dtx} | ${DTC} -I dts ) ; then
>> +                               return
>> +                       fi
>> +
>> +                       echo ""                                                          >&2
>> +                       echo "Possible hints to resolve the above error:"                >&2
>> +                       echo "  (hints might not fix the problem)"                       >&2
>> +
>> +                       hint_given=0
>> +
>> +                       if [ "${ARCH}" = "" ] ; then
>> +                               hint_given=1
>> +                               echo ""                                                      >&2
>> +                               echo "  shell variable \$ARCH not set"                       >&2
>> +                       fi
>> +
>> +                       dtx_arch=`echo "/${dtx}" | sed -e 's|.*/arch/||' -e 's|/.*||'`
>> +
>> +                       if [ "${dtx_arch}" != ""  -a "${dtx_arch}" != "${ARCH}" ] ; then
>> +                               hint_given=1
>> +                               echo ""                                                      >&2
>> +                               echo "  architecture ${dtx_arch} is in file path, but"       >&2
>> +                               echo "  does not match shell variable \$ARCH (${ARCH})"      >&2
>> +                       fi
>> +
>> +                       if [ ! -d ${srctree}/arch/${ARCH} ] ; then
>> +                               hint_given=1
>> +                               echo ""                                                      >&2
>> +                               echo "  ${srctree}/arch/${ARCH}/ does not exist"             >&2
>> +                               echo "  Is \$ARCH='${ARCH}' correct?"                        >&2
>> +                               echo "  Possible fix: use '-s' option"                       >&2
>> +
>> +                               git_root=`git rev-parse --show-toplevel 2>/dev/null`
>> +                               if [ -d ${git_root}/arch/ ] ; then
>> +                                       echo "  Possible fix: use '-S' option"                   >&2
>> +                               fi
>> +                       fi
>> +
>> +                       if [ $hint_given = 0 ] ; then
>> +                               echo ""                                                      >&2
>> +                               echo "  No hints available."                                 >&2
>> +                       fi
>> +
>> +                       echo ""                                                          >&2
>> +
>> +                       exit 3
>> +
>> +               fi
>> +       else
>> +               echo ""                                                              >&2
>> +               echo "ERROR: ${dtx} does not exist or is not readable"               >&2
>> +               echo ""                                                              >&2
>> +               exit 2
>> +       fi
>> +
>> +}
>> +
>> +
>> +# -----  start of script
>> +
>> +cmd_diff=0
>> +diff_flags="-u"
>> +dtx_file_1=""
>> +dtx_file_2=""
>> +dtc_sort="-s"
>> +help=0
>> +srctree=""
>> +
>> +
>> +while [ $# -gt 0 ] ; do
>> +
>> +       case $1 in
>> +
>> +       -f )
>> +               diff_flags="--unified=999999"
>> +               shift
>> +               ;;
>> +
>> +       -h | -help | --help )
>> +               help=1
>> +               shift
>> +               ;;
>> +
>> +       -s )
>> +               srctree="$2"
>> +               shift 2
>> +               ;;
>> +
>> +       -S )
>> +               git_root=`git rev-parse --show-toplevel 2>/dev/null`
>> +               srctree="${git_root}"
>> +               shift
>> +               ;;
>> +
>> +       -u )
>> +               dtc_sort=""
>> +               shift
>> +               ;;
>> +
>> +       *)
>> +               if [ "${dtx_file_1}"  = "" ] ; then
>> +                       dtx_file_1="$1"
>> +               elif [ "${dtx_file_2}" = "" ] ; then
>> +                       dtx_file_2="$1"
>> +               else
>> +                       echo ""                                                          >&2
>> +                       echo "ERROR: Unexpected parameter: $1"                           >&2
>> +                       echo ""                                                          >&2
>> +                       exit 2
>> +               fi
>> +               shift
>> +               ;;
>> +
>> +       esac
>> +
>> +done
>> +
>> +if [ "${srctree}" = "" ] ; then
>> +       srctree="."
>> +fi
>> +
>> +if [ "${dtx_file_2}" != "" ]; then
>> +       cmd_diff=1
>> +fi
>> +
>> +if (( ${help} )) ; then
>> +       usage
>> +       exit 1
>> +fi
>> +
>> +# this must follow check for ${help}
>> +if [ "${dtx_file_1}" = "" ]; then
>> +       echo ""                                                                  >&2
>> +       echo "ERROR: parameter DTx required"                                     >&2
>> +       echo ""                                                                  >&2
>> +       exit 2
>> +fi
>> +
>> +
>> +# -----  prefer dtc from linux kernel, allow fallback to dtc in $PATH
>> +
>> +if [ "${KBUILD_OUTPUT:0:2}" = ".." ] ; then
>> +       __KBUILD_OUTPUT="${srctree}/${KBUILD_OUTPUT}"
>> +elif [ "${KBUILD_OUTPUT}" = "" ] ; then
>> +       __KBUILD_OUTPUT="."
>> +else
>> +       __KBUILD_OUTPUT="${KBUILD_OUTPUT}"
>> +fi
>> +
>> +DTC="${__KBUILD_OUTPUT}/scripts/dtc/dtc"
>> +
>> +if [ ! -x ${DTC} ] ; then
>> +       __DTC="dtc"
>> +       if ( ! which ${__DTC} >/dev/null ) ; then
>> +
>> +               # use spaces instead of tabs in the error message
>> +               cat >&2 <<eod
>> +
>> +ERROR: unable to find a 'dtc' program
>> +
>> +   Preferred 'dtc' (built from Linux kernel source tree) was not found or
>> +   is not executable.
>> +
>> +      'dtc' is: ${DTC}
>> +
>> +      If it does not exist, create it from the root of the Linux source tree:
>> +
>> +         'make scripts'.
>> +
>> +      If not at the root of the Linux kernel source tree -s SRCTREE or -S
>> +      may need to be specified to find 'dtc'.
>> +
>> +      If 'O=\${dir}' is specified in your Linux builds, this script requires
>> +      'export KBUILD_OUTPUT=\${dir}' or add \${dir}/scripts/dtc to \$PATH
>> +      before running.
>> +
>> +      If \${KBUILD_OUTPUT} is a relative path, then '-s SRCDIR', -S, or run
>> +      this script from the root of the Linux kernel source tree is required.
>> +
>> +   Fallback '${__DTC}' was also not in \${PATH} or is not executable.
>> +
>> +eod
>> +               exit 2
>> +       fi
>> +       DTC=${__DTC}
>> +fi
>> +
>> +
>> +# -----  cpp and dtc flags same as for linux source tree build of .dtb files,
>> +#        plus directories of the dtx file(s)
>> +
>> +dtx_path_1_dtc_include="-i `dirname ${dtx_file_1}`"
>> +
>> +dtx_path_2_dtc_include=""
>> +if (( ${cmd_diff} )) ; then
>> +       dtx_path_2_dtc_include="-i `dirname ${dtx_file_2}`"
>> +fi
>> +
>> +cpp_flags="\
>> +       -nostdinc                                              \
>> +       -I${srctree}/arch/${ARCH}/boot/dts                     \
>> +       -I${srctree}/arch/${ARCH}/boot/dts/include             \
>> +       -I${srctree}/arch/${ARCH}/boot/dts/include/dt-bindings \
> 
> kernel doesn't use this path. The less dependency on ARCH, the better.
> The dependency in the kernel is artificial.

Yep, ARCH is a pain.  But it is used in the kernel build of dt files.
See "dtc_cpp_flags" in scripts/Makefile.lib.  I'm not sure why I
have dt-bindings in there though.  I'll have to check that.


> 
>> +       -I${srctree}/drivers/of/testcase-data                  \
>> +       -undef -D__DTS__"
>> +
>> +_dts_makefile=""
>> +
>> +if [ -f ${srctree}/arch/${ARCH}/boot/dts/Makefile ] ; then
>> +       _dts_makefile="${srctree}/arch/${ARCH}/boot/dts/Makefile"
>> +elif [ -f ${srctree}/arch/${ARCH}/boot/Makefile ] ; then
>> +       # arch/powerpc
>> +       _dts_makefile="${srctree}/arch/${ARCH}/boot/Makefile"
> 
> We need to move PPC to be in line with other arches rather than
> working around it.
> 
>> +fi
>> +
>> +if [ "${_dts_makefile}" != "" ] ; then
>> +       arch_dtc_flags=`grep DTC_FLAGS ${_dts_makefile} \
> 
> We should just kill DTC_FLAGS being per arch. Plus it is only used for
> padding, so it is irrelevant to this tool.

Yes it is irrelevant.  The only reason I included it was just to future
proof for if anyone snuck anything else into the DTC_FLAGS in the future.
I'll just remove this section (resolving the issue with working around
the location of the PPC makefile too).

> 
>> +               | grep -v "^#"                              \
>> +               | sed -e 's|.*=||' `
>> +else
>> +       arch_dtc_flags=""
>> +fi
>> +
>> +dtc_flags="\
>> +       -i ${srctree}/arch/${ARCH}/boot/dts/ \
>> +       -i ${srctree}/kernel/dts             \
>> +       ${dtx_path_1_dtc_include}            \
>> +       ${dtx_path_2_dtc_include}            \
>> +       ${arch_dtc_flags}"
>> +
>> +DTC="${DTC} ${dtc_flags} -O dts -qq -f ${dtc_sort} -o -"
>> +
>> +
>> +# -----  do the diff or decompile
>> +
>> +if (( ${cmd_diff} )) ; then
>> +
>> +       diff ${diff_flags} \
>> +               <(compile_to_dts "${dtx_file_1}") \
>> +               <(compile_to_dts "${dtx_file_2}")
>> +
>> +else
>> +
>> +       compile_to_dts "${dtx_file_1}"
>> +
>> +fi
>> +
>> +
>> +# ==============================================================================
>> +# ______________________________________________________________________________
>> +# - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
>> +# ______________________________________________________________________________
>> +#  vi config follows:
>> +#
>> +#  add "set modelines" to ~/.exrc or ~/.vimrc to set tabs automatically
>> +#  ex:set tabstop=4 shiftwidth=4 sts=4:
> 
> Kernel standard is 8 characters. I assume that applies to bash files too.

I'll reformat to 8 characters.

> 
> Rob

-Frank


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] dtc: create tool to diff device trees
  2016-01-04 21:29 ` Rob Herring
  2016-01-07  6:15   ` Frank Rowand
@ 2016-01-07 18:40   ` Frank Rowand
  1 sibling, 0 replies; 4+ messages in thread
From: Frank Rowand @ 2016-01-07 18:40 UTC (permalink / raw)
  To: Rob Herring; +Cc: Grant Likely, David Gibson, devicetree, Linux Kernel list

On 1/4/2016 1:29 PM, Rob Herring wrote:
> On Thu, Dec 31, 2015 at 3:12 AM, Frank Rowand <frowand.list@gmail.com> wrote:
>> From: Frank Rowand <frank.rowand@sonymobile.com>
>>
>> Create script to diff device trees.
>>
>> The device tree can be in any of the forms recognized by the dtc compiler:
>>   - source
>>   - binary blob
>>   - file system tree (from /proc/devicetree)
>>
>> If the device tree is a source file, then it is pre-processed in the
>> same way as it would be when built in the linux kernel source tree
>> before diffing.
> 
> In general, I'd like to see some of this move to dtc and the kernel
> dependencies minimized.
> 

I didn't address this comment in my first reply.  Didn't mean to gloss
over it.

I agree with the sentiment, but I'm not sure how to act on it.  I don't
think that dtc should be changed to be Linux specific (but if the dtc
maintainers think differently I would follow their lead).  You did
provide at least one example of minimizing kernel dependencies
(removing the arch specific Makefile dtc flags), but beyond that
I'm not sure what can be done.

On the positive side, we can avoid adding more kernel dependencies.

< snip >

-Frank

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2016-01-07 18:40 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-12-31  9:12 [PATCH] dtc: create tool to diff device trees Frank Rowand
2016-01-04 21:29 ` Rob Herring
2016-01-07  6:15   ` Frank Rowand
2016-01-07 18:40   ` Frank Rowand

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).