Re: [PATCH] scripts/dtc: dt_to_config - report kernel config options for a devicetree
On Sat, Apr 30, 2016 at 1:38 PM, Rob Herring <robh...@kernel.org> wrote: > On Fri, Apr 29, 2016 at 1:39 AM, Gaurav Minocha > <gaurav.minocha...@gmail.com> wrote: >> On Thu, Apr 28, 2016 at 3:32 PM, Rob Herring <robh...@kernel.org> wrote: >>> On Thu, Apr 28, 2016 at 4:46 PM, Frank Rowand <frowand.l...@gmail.com> >>> wrote: >>>> From: Frank Rowand <frank.row...@am.sony.com> >>>> >>>> Determining which kernel config options need to be enabled for a >>>> given devicetree can be a painful process. Create a new tool to >>>> find the drivers that may match a devicetree node compatible, >>>> find the kernel config options that enable the driver, and >>>> optionally report whether the kernel config option is enabled. >>> >>> I would find this more useful to output a config fragment with all the >>> options enabled. The hard part there is enabling the options a given >>> option is dependent on which I don't think kbuild takes care of. >> >> Do you mean to generate something like .config? If yes, then IMO it would >> not be a correct configuration file. > > No, only a config fragment which is just some subset of config > options. Then you could have a generic defconfig plus the fragment for > a platform to get a working setup. Okay, that seems like a useful option, that should be part of the tool itself. Will discuss with Frank once he is back. Thanks! > > Rob
Re: [PATCH] scripts/dtc: dt_to_config - report kernel config options for a devicetree
On Sat, Apr 30, 2016 at 1:38 PM, Rob Herring wrote: > On Fri, Apr 29, 2016 at 1:39 AM, Gaurav Minocha > wrote: >> On Thu, Apr 28, 2016 at 3:32 PM, Rob Herring wrote: >>> On Thu, Apr 28, 2016 at 4:46 PM, Frank Rowand >>> wrote: >>>> From: Frank Rowand >>>> >>>> Determining which kernel config options need to be enabled for a >>>> given devicetree can be a painful process. Create a new tool to >>>> find the drivers that may match a devicetree node compatible, >>>> find the kernel config options that enable the driver, and >>>> optionally report whether the kernel config option is enabled. >>> >>> I would find this more useful to output a config fragment with all the >>> options enabled. The hard part there is enabling the options a given >>> option is dependent on which I don't think kbuild takes care of. >> >> Do you mean to generate something like .config? If yes, then IMO it would >> not be a correct configuration file. > > No, only a config fragment which is just some subset of config > options. Then you could have a generic defconfig plus the fragment for > a platform to get a working setup. Okay, that seems like a useful option, that should be part of the tool itself. Will discuss with Frank once he is back. Thanks! > > Rob
Re: [PATCH] scripts/dtc: dt_to_config - report kernel config options for a devicetree
On Thu, Apr 28, 2016 at 3:32 PM, Rob Herring <robh...@kernel.org> wrote: > On Thu, Apr 28, 2016 at 4:46 PM, Frank Rowand <frowand.l...@gmail.com> wrote: >> From: Frank Rowand <frank.row...@am.sony.com> >> >> Determining which kernel config options need to be enabled for a >> given devicetree can be a painful process. Create a new tool to >> find the drivers that may match a devicetree node compatible, >> find the kernel config options that enable the driver, and >> optionally report whether the kernel config option is enabled. > > I would find this more useful to output a config fragment with all the > options enabled. The hard part there is enabling the options a given > option is dependent on which I don't think kbuild takes care of. Do you mean to generate something like .config? If yes, then IMO it would not be a correct configuration file. > >> Signed-off-by: Gaurav Minocha <gaurav.minocha...@gmail.com> >> Signed-off-by: Frank Rowand <frank.row...@am.sony.com> >> >> --- >> scripts/dtc/dt_to_config | 1061 >> +++ >> 1 file changed, 1061 insertions(+) >> >> Index: b/scripts/dtc/dt_to_config >> === >> --- /dev/null >> +++ b/scripts/dtc/dt_to_config >> @@ -0,0 +1,1061 @@ >> +#!/usr/bin/perl > > I don't do perl... > >> + >> +# Copyright 2016 by Frank Rowand >> +# Š Copyright 2016 by Gaurav Minocha > ^ > Is this supposed to be a copyright symbol? > >> +# >> +# This file is subject to the terms and conditions of the GNU General Public >> +# License v2. > > [...] > >> +# - magic compatibles, do not have a driver >> +# >> +# Will not search for drivers for these compatibles. >> + >> +%compat_white_list = ( >> + 'fixed-partitions' => '1', > > Enabling CONFIG_MTD would be useful. > >> + 'none' => '1', > > Is this an actual string used somewhere? > >> + 'pci' => '1', > > ditto? > >> + 'simple-bus'=> '1', >> +); >> + >> +# magic compatibles, have a driver >> +# >> +# Will not search for drivers for these compatibles. >> +# Will instead use the drivers and config options listed here. >> +# >> +# If you add an entry to this hash, add the corresponding entry >> +# to %driver_config_hard_code_list. >> +# >> +# These compatibles have a very large number of false positives. > > What does this mean? > >> +# >> +# 'hardcoded_no_driver' is a magic value. Other code knows this >> +# magic value. Do not use 'no_driver' here! >> +# >> +# TODO: Revisit each 'hardcoded_no_driver' to see how the compatible >> +# is used. Are there drivers that can be provided? >> + >> +%driver_hard_code_list = ( >> + 'cache' => ['hardcoded_no_driver'], >> + 'eeprom'=> ['hardcoded_no_driver'], >> + 'gpio' => ['hardcoded_no_driver'], >> + 'gpios' => ['drivers/leds/leds-tca6507.c'], >> + 'gpio-keys' => ['drivers/input/keyboard/gpio_keys.c'], >> + 'i2c-gpio' => ['drivers/i2c/busses/i2c-gpio.c'], >> + 'isa' => ['arch/mips/mti-malta/malta-dt.c', >> +'arch/x86/kernel/devicetree.c'], >> + 'led' => ['hardcoded_no_driver'], >> + 'm25p32'=> ['hardcoded_no_driver'], >> + 'm25p64'=> ['hardcoded_no_driver'], >> + 'm25p80'=> ['hardcoded_no_driver'], >> + 'mtd_ram' => ['drivers/mtd/maps/physmap_of.c'], >> + 'pwm-backlight' => ['drivers/video/backlight/pwm_bl.c'], >> + 'spidev'=> ['hardcoded_no_driver'], >> + 'syscon'=> ['drivers/mfd/syscon.c'], >> + 'tlv320aic23' => ['hardcoded_no_driver'], >> + 'wm8731'=> ['hardcoded_no_driver'], >> +); >> + >> +%driver_config_hard_code_list = ( >> + >> + # this one needed even if %driver_hard_code_list is empty >> + 'no_driver' => ['no_config'], >> + 'hardcoded_no_driver' => ['no_config'], >> + >> + 'drivers/leds/leds-tca6507.c' => ['CONFIG_LEDS_TCA6507'], >> + 'drivers/input/key
Re: [PATCH] scripts/dtc: dt_to_config - report kernel config options for a devicetree
On Thu, Apr 28, 2016 at 3:32 PM, Rob Herring wrote: > On Thu, Apr 28, 2016 at 4:46 PM, Frank Rowand wrote: >> From: Frank Rowand >> >> Determining which kernel config options need to be enabled for a >> given devicetree can be a painful process. Create a new tool to >> find the drivers that may match a devicetree node compatible, >> find the kernel config options that enable the driver, and >> optionally report whether the kernel config option is enabled. > > I would find this more useful to output a config fragment with all the > options enabled. The hard part there is enabling the options a given > option is dependent on which I don't think kbuild takes care of. Do you mean to generate something like .config? If yes, then IMO it would not be a correct configuration file. > >> Signed-off-by: Gaurav Minocha >> Signed-off-by: Frank Rowand >> >> --- >> scripts/dtc/dt_to_config | 1061 >> +++ >> 1 file changed, 1061 insertions(+) >> >> Index: b/scripts/dtc/dt_to_config >> === >> --- /dev/null >> +++ b/scripts/dtc/dt_to_config >> @@ -0,0 +1,1061 @@ >> +#!/usr/bin/perl > > I don't do perl... > >> + >> +# Copyright 2016 by Frank Rowand >> +# Š Copyright 2016 by Gaurav Minocha > ^ > Is this supposed to be a copyright symbol? > >> +# >> +# This file is subject to the terms and conditions of the GNU General Public >> +# License v2. > > [...] > >> +# - magic compatibles, do not have a driver >> +# >> +# Will not search for drivers for these compatibles. >> + >> +%compat_white_list = ( >> + 'fixed-partitions' => '1', > > Enabling CONFIG_MTD would be useful. > >> + 'none' => '1', > > Is this an actual string used somewhere? > >> + 'pci' => '1', > > ditto? > >> + 'simple-bus'=> '1', >> +); >> + >> +# magic compatibles, have a driver >> +# >> +# Will not search for drivers for these compatibles. >> +# Will instead use the drivers and config options listed here. >> +# >> +# If you add an entry to this hash, add the corresponding entry >> +# to %driver_config_hard_code_list. >> +# >> +# These compatibles have a very large number of false positives. > > What does this mean? > >> +# >> +# 'hardcoded_no_driver' is a magic value. Other code knows this >> +# magic value. Do not use 'no_driver' here! >> +# >> +# TODO: Revisit each 'hardcoded_no_driver' to see how the compatible >> +# is used. Are there drivers that can be provided? >> + >> +%driver_hard_code_list = ( >> + 'cache' => ['hardcoded_no_driver'], >> + 'eeprom'=> ['hardcoded_no_driver'], >> + 'gpio' => ['hardcoded_no_driver'], >> + 'gpios' => ['drivers/leds/leds-tca6507.c'], >> + 'gpio-keys' => ['drivers/input/keyboard/gpio_keys.c'], >> + 'i2c-gpio' => ['drivers/i2c/busses/i2c-gpio.c'], >> + 'isa' => ['arch/mips/mti-malta/malta-dt.c', >> +'arch/x86/kernel/devicetree.c'], >> + 'led' => ['hardcoded_no_driver'], >> + 'm25p32'=> ['hardcoded_no_driver'], >> + 'm25p64'=> ['hardcoded_no_driver'], >> + 'm25p80'=> ['hardcoded_no_driver'], >> + 'mtd_ram' => ['drivers/mtd/maps/physmap_of.c'], >> + 'pwm-backlight' => ['drivers/video/backlight/pwm_bl.c'], >> + 'spidev'=> ['hardcoded_no_driver'], >> + 'syscon'=> ['drivers/mfd/syscon.c'], >> + 'tlv320aic23' => ['hardcoded_no_driver'], >> + 'wm8731'=> ['hardcoded_no_driver'], >> +); >> + >> +%driver_config_hard_code_list = ( >> + >> + # this one needed even if %driver_hard_code_list is empty >> + 'no_driver' => ['no_config'], >> + 'hardcoded_no_driver' => ['no_config'], >> + >> + 'drivers/leds/leds-tca6507.c' => ['CONFIG_LEDS_TCA6507'], >> + 'drivers/input/keyboard/gpio_keys.c'=> ['CONFIG_KEYBOARD_GPIO'], >> + 'drivers/i2c/busses/i2c-gpio.c' => ['CONFIG_I2C_GPIO'], >> +
Re: [PATCH] scripts/dtc: dt_to_config - report kernel config options for a devicetree
On Thu, Apr 28, 2016 at 4:31 PM, Frank Rowand <frowand.l...@gmail.com> wrote: > On 4/28/2016 3:32 PM, Rob Herring wrote: >> On Thu, Apr 28, 2016 at 4:46 PM, Frank Rowand <frowand.l...@gmail.com> wrote: >>> From: Frank Rowand <frank.row...@am.sony.com> >>> >>> Determining which kernel config options need to be enabled for a >>> given devicetree can be a painful process. Create a new tool to >>> find the drivers that may match a devicetree node compatible, >>> find the kernel config options that enable the driver, and >>> optionally report whether the kernel config option is enabled. >> >> I would find this more useful to output a config fragment with all the >> options enabled. The hard part there is enabling the options a given >> option is dependent on which I don't think kbuild takes care of. >> >>> Signed-off-by: Gaurav Minocha <gaurav.minocha...@gmail.com> >>> Signed-off-by: Frank Rowand <frank.row...@am.sony.com> >>> >>> --- >>> scripts/dtc/dt_to_config | 1061 >>> +++ >>> 1 file changed, 1061 insertions(+) >>> >>> Index: b/scripts/dtc/dt_to_config >>> ======= >>> --- /dev/null >>> +++ b/scripts/dtc/dt_to_config >>> @@ -0,0 +1,1061 @@ >>> +#!/usr/bin/perl >> >> I don't do perl... >> >>> + >>> +# Copyright 2016 by Frank Rowand >>> +# Š Copyright 2016 by Gaurav Minocha >> ^ >> Is this supposed to be a copyright symbol? > > Yes. Gaurav, can we drop this symbol? > Yes, Of course. > >>> +# >>> +# This file is subject to the terms and conditions of the GNU General >>> Public >>> +# License v2. >> >> [...] >> >>> +# - magic compatibles, do not have a driver >>> +# >>> +# Will not search for drivers for these compatibles. >>> + >>> +%compat_white_list = ( >>> + 'fixed-partitions' => '1', >> >> Enabling CONFIG_MTD would be useful. > > Thanks! I'll dig deeper, but it likes like maybe > CONFIG_MTD_OF_PARTS also. > > >>> + 'none' => '1', >> >> Is this an actual string used somewhere? > > Yes. Looking at the output from running this against all .dts > files has led to some interesting information. > > An example of compatible = "none" is node mct@1005 from > arch/arm/boot/dts/exynos4210-universal_c210.dts > >> >>> + 'pci' => '1', >> >> ditto? > > arch/x86/platform/ce4100/falconfalls.dts, node pci@3fc > > >>> + 'simple-bus'=> '1', >>> +); >>> + >>> +# magic compatibles, have a driver >>> +# >>> +# Will not search for drivers for these compatibles. >>> +# Will instead use the drivers and config options listed here. >>> +# >>> +# If you add an entry to this hash, add the corresponding entry >>> +# to %driver_config_hard_code_list. >>> +# >>> +# These compatibles have a very large number of false positives. >> >> What does this mean? > > That means that a _lot_ of garbage, bogus matches are reported, > creating a lot of noise in the report. > > >>> +# >>> +# 'hardcoded_no_driver' is a magic value. Other code knows this >>> +# magic value. Do not use 'no_driver' here! >>> +# >>> +# TODO: Revisit each 'hardcoded_no_driver' to see how the compatible >>> +# is used. Are there drivers that can be provided? >>> + >>> +%driver_hard_code_list = ( >>> + 'cache' => ['hardcoded_no_driver'], >>> + 'eeprom'=> ['hardcoded_no_driver'], >>> + 'gpio' => ['hardcoded_no_driver'], >>> + 'gpios' => ['drivers/leds/leds-tca6507.c'], >>> + 'gpio-keys' => ['drivers/input/keyboard/gpio_keys.c'], >>> + 'i2c-gpio' => ['drivers/i2c/busses/i2c-gpio.c'], >>> + 'isa' => ['arch/mips/mti-malta/malta-dt.c', >>> +'arch/x86/kernel/devicetree.c'], >>> + 'led' => ['hardcoded_no_driver'], >>> + 'm25p32'=> ['hardcoded_no_driver'], >>> + 'm25p64'=> ['hardcoded_no_driver'], >>> + 'm25p80'
Re: [PATCH] scripts/dtc: dt_to_config - report kernel config options for a devicetree
On Thu, Apr 28, 2016 at 4:31 PM, Frank Rowand wrote: > On 4/28/2016 3:32 PM, Rob Herring wrote: >> On Thu, Apr 28, 2016 at 4:46 PM, Frank Rowand wrote: >>> From: Frank Rowand >>> >>> Determining which kernel config options need to be enabled for a >>> given devicetree can be a painful process. Create a new tool to >>> find the drivers that may match a devicetree node compatible, >>> find the kernel config options that enable the driver, and >>> optionally report whether the kernel config option is enabled. >> >> I would find this more useful to output a config fragment with all the >> options enabled. The hard part there is enabling the options a given >> option is dependent on which I don't think kbuild takes care of. >> >>> Signed-off-by: Gaurav Minocha >>> Signed-off-by: Frank Rowand >>> >>> --- >>> scripts/dtc/dt_to_config | 1061 >>> +++ >>> 1 file changed, 1061 insertions(+) >>> >>> Index: b/scripts/dtc/dt_to_config >>> === >>> --- /dev/null >>> +++ b/scripts/dtc/dt_to_config >>> @@ -0,0 +1,1061 @@ >>> +#!/usr/bin/perl >> >> I don't do perl... >> >>> + >>> +# Copyright 2016 by Frank Rowand >>> +# Š Copyright 2016 by Gaurav Minocha >> ^ >> Is this supposed to be a copyright symbol? > > Yes. Gaurav, can we drop this symbol? > Yes, Of course. > >>> +# >>> +# This file is subject to the terms and conditions of the GNU General >>> Public >>> +# License v2. >> >> [...] >> >>> +# - magic compatibles, do not have a driver >>> +# >>> +# Will not search for drivers for these compatibles. >>> + >>> +%compat_white_list = ( >>> + 'fixed-partitions' => '1', >> >> Enabling CONFIG_MTD would be useful. > > Thanks! I'll dig deeper, but it likes like maybe > CONFIG_MTD_OF_PARTS also. > > >>> + 'none' => '1', >> >> Is this an actual string used somewhere? > > Yes. Looking at the output from running this against all .dts > files has led to some interesting information. > > An example of compatible = "none" is node mct@1005 from > arch/arm/boot/dts/exynos4210-universal_c210.dts > >> >>> + 'pci' => '1', >> >> ditto? > > arch/x86/platform/ce4100/falconfalls.dts, node pci@3fc > > >>> + 'simple-bus'=> '1', >>> +); >>> + >>> +# magic compatibles, have a driver >>> +# >>> +# Will not search for drivers for these compatibles. >>> +# Will instead use the drivers and config options listed here. >>> +# >>> +# If you add an entry to this hash, add the corresponding entry >>> +# to %driver_config_hard_code_list. >>> +# >>> +# These compatibles have a very large number of false positives. >> >> What does this mean? > > That means that a _lot_ of garbage, bogus matches are reported, > creating a lot of noise in the report. > > >>> +# >>> +# 'hardcoded_no_driver' is a magic value. Other code knows this >>> +# magic value. Do not use 'no_driver' here! >>> +# >>> +# TODO: Revisit each 'hardcoded_no_driver' to see how the compatible >>> +# is used. Are there drivers that can be provided? >>> + >>> +%driver_hard_code_list = ( >>> + 'cache' => ['hardcoded_no_driver'], >>> + 'eeprom'=> ['hardcoded_no_driver'], >>> + 'gpio' => ['hardcoded_no_driver'], >>> + 'gpios' => ['drivers/leds/leds-tca6507.c'], >>> + 'gpio-keys' => ['drivers/input/keyboard/gpio_keys.c'], >>> + 'i2c-gpio' => ['drivers/i2c/busses/i2c-gpio.c'], >>> + 'isa' => ['arch/mips/mti-malta/malta-dt.c', >>> +'arch/x86/kernel/devicetree.c'], >>> + 'led' => ['hardcoded_no_driver'], >>> + 'm25p32'=> ['hardcoded_no_driver'], >>> + 'm25p64'=> ['hardcoded_no_driver'], >>> + 'm25p80'=> ['hardcoded_no_driver'], >>> + 'mtd_ram' => ['drivers/mtd/maps/physmap_of.c'], >>> + 'pwm-backlight'
Re: [PATCH 4/5] Documentation: rename of_selftest.txt to of_unittest.txt
Please use -M flag while sending rename patch, as mentioned in my other mail.
On Tue, Mar 10, 2015 at 8:37 PM, Wang Long wrote:
> Since the test of the devicetree's OF api use unittest as
> its name. so we should rename of_selftest.txt to of_unittest.txt.
>
> Signed-off-by: Wang Long
> ---
> Documentation/devicetree/of_selftest.txt | 198
> ---
> Documentation/devicetree/of_unittest.txt | 198
> +++
> 2 files changed, 198 insertions(+), 198 deletions(-)
> delete mode 100644 Documentation/devicetree/of_selftest.txt
> create mode 100644 Documentation/devicetree/of_unittest.txt
>
> diff --git a/Documentation/devicetree/of_selftest.txt
> b/Documentation/devicetree/of_selftest.txt
> deleted file mode 100644
> index d79a6bc..000
> --- a/Documentation/devicetree/of_selftest.txt
> +++ /dev/null
> @@ -1,198 +0,0 @@
> -Open Firmware Device Tree Unittest
> ---
> -
> -Author: Gaurav Minocha
> -
> -1. Introduction
> -
> -This document explains how the test data required for executing OF unittest
> -is attached to the live tree dynamically, independent of the machine's
> -architecture.
> -
> -It is recommended to read the following documents before moving ahead.
> -
> -[1] Documentation/devicetree/usage-model.txt
> -[2] http://www.devicetree.org/Device_Tree_Usage
> -
> -OF Selftest has been designed to test the interface (include/linux/of.h)
> -provided to device driver developers to fetch the device information..etc.
> -from the unflattened device tree data structure. This interface is used by
> -most of the device drivers in various use cases.
> -
> -
> -2. Test-data
> -
> -The Device Tree Source file (drivers/of/unittest-data/testcases.dts) contains
> -the test data required for executing the unit tests automated in
> -drivers/of/unittest.c. Currently, following Device Tree Source Include files
> -(.dtsi) are included in testcases.dts:
> -
> -drivers/of/unittest-data/tests-interrupts.dtsi
> -drivers/of/unittest-data/tests-platform.dtsi
> -drivers/of/unittest-data/tests-phandle.dtsi
> -drivers/of/unittest-data/tests-match.dtsi
> -drivers/of/unittest-data/tests-overlay.dtsi
> -
> -When the kernel is build with OF_SELFTEST enabled, then the following make
> rule
> -
> -$(obj)/%.dtb: $(src)/%.dts FORCE
> - $(call if_changed_dep, dtc)
> -
> -is used to compile the DT source file (testcases.dts) into a binary blob
> -(testcases.dtb), also referred as flattened DT.
> -
> -After that, using the following rule the binary blob above is wrapped as an
> -assembly file (testcases.dtb.S).
> -
> -$(obj)/%.dtb.S: $(obj)/%.dtb
> - $(call cmd, dt_S_dtb)
> -
> -The assembly file is compiled into an object file (testcases.dtb.o), and is
> -linked into the kernel image.
> -
> -
> -2.1. Adding the test data
> -
> -Un-flattened device tree structure:
> -
> -Un-flattened device tree consists of connected device_node(s) in form of a
> tree
> -structure described below.
> -
> -// following struct members are used to construct the tree
> -struct device_node {
> -...
> -struct device_node *parent;
> -struct device_node *child;
> -struct device_node *sibling;
> -...
> - };
> -
> -Figure 1, describes a generic structure of machine's un-flattened device tree
> -considering only child and sibling pointers. There exists another pointer,
> -*parent, that is used to traverse the tree in the reverse direction. So, at
> -a particular level the child node and all the sibling nodes will have a
> parent
> -pointer pointing to a common node (e.g. child1, sibling2, sibling3,
> sibling4's
> -parent points to root node)
> -
> -root ('/')
> - |
> -child1 -> sibling2 -> sibling3 -> sibling4 -> null
> - | | | |
> - | | | null
> - | | |
> - | |child31 -> sibling32 -> null
> - | | | |
> - | | null null
> - | |
> - | child21 -> sibling22 -> sibling23 -> null
> - | | ||
> - |null null null
> - |
> -child11 -> sibling12 -> sibling13 -> sibling14 -> null
> - | | ||
> - | | | null
> - | | |
> - nullnull child131 -> null
> - |
> - null
> -
> -Figure 1: Generic structure of un-flattened device tree
> -
> -
> -Before executing O
Re: [PATCH 4/5] Documentation: rename of_selftest.txt to of_unittest.txt
Please use -M flag while sending rename patch, as mentioned in my other mail.
On Tue, Mar 10, 2015 at 8:37 PM, Wang Long long.wangl...@huawei.com wrote:
Since the test of the devicetree's OF api use unittest as
its name. so we should rename of_selftest.txt to of_unittest.txt.
Signed-off-by: Wang Long long.wangl...@huawei.com
---
Documentation/devicetree/of_selftest.txt | 198
---
Documentation/devicetree/of_unittest.txt | 198
+++
2 files changed, 198 insertions(+), 198 deletions(-)
delete mode 100644 Documentation/devicetree/of_selftest.txt
create mode 100644 Documentation/devicetree/of_unittest.txt
diff --git a/Documentation/devicetree/of_selftest.txt
b/Documentation/devicetree/of_selftest.txt
deleted file mode 100644
index d79a6bc..000
--- a/Documentation/devicetree/of_selftest.txt
+++ /dev/null
@@ -1,198 +0,0 @@
-Open Firmware Device Tree Unittest
---
-
-Author: Gaurav Minocha gaurav.minocha...@gmail.com
-
-1. Introduction
-
-This document explains how the test data required for executing OF unittest
-is attached to the live tree dynamically, independent of the machine's
-architecture.
-
-It is recommended to read the following documents before moving ahead.
-
-[1] Documentation/devicetree/usage-model.txt
-[2] http://www.devicetree.org/Device_Tree_Usage
-
-OF Selftest has been designed to test the interface (include/linux/of.h)
-provided to device driver developers to fetch the device information..etc.
-from the unflattened device tree data structure. This interface is used by
-most of the device drivers in various use cases.
-
-
-2. Test-data
-
-The Device Tree Source file (drivers/of/unittest-data/testcases.dts) contains
-the test data required for executing the unit tests automated in
-drivers/of/unittest.c. Currently, following Device Tree Source Include files
-(.dtsi) are included in testcases.dts:
-
-drivers/of/unittest-data/tests-interrupts.dtsi
-drivers/of/unittest-data/tests-platform.dtsi
-drivers/of/unittest-data/tests-phandle.dtsi
-drivers/of/unittest-data/tests-match.dtsi
-drivers/of/unittest-data/tests-overlay.dtsi
-
-When the kernel is build with OF_SELFTEST enabled, then the following make
rule
-
-$(obj)/%.dtb: $(src)/%.dts FORCE
- $(call if_changed_dep, dtc)
-
-is used to compile the DT source file (testcases.dts) into a binary blob
-(testcases.dtb), also referred as flattened DT.
-
-After that, using the following rule the binary blob above is wrapped as an
-assembly file (testcases.dtb.S).
-
-$(obj)/%.dtb.S: $(obj)/%.dtb
- $(call cmd, dt_S_dtb)
-
-The assembly file is compiled into an object file (testcases.dtb.o), and is
-linked into the kernel image.
-
-
-2.1. Adding the test data
-
-Un-flattened device tree structure:
-
-Un-flattened device tree consists of connected device_node(s) in form of a
tree
-structure described below.
-
-// following struct members are used to construct the tree
-struct device_node {
-...
-struct device_node *parent;
-struct device_node *child;
-struct device_node *sibling;
-...
- };
-
-Figure 1, describes a generic structure of machine's un-flattened device tree
-considering only child and sibling pointers. There exists another pointer,
-*parent, that is used to traverse the tree in the reverse direction. So, at
-a particular level the child node and all the sibling nodes will have a
parent
-pointer pointing to a common node (e.g. child1, sibling2, sibling3,
sibling4's
-parent points to root node)
-
-root ('/')
- |
-child1 - sibling2 - sibling3 - sibling4 - null
- | | | |
- | | | null
- | | |
- | |child31 - sibling32 - null
- | | | |
- | | null null
- | |
- | child21 - sibling22 - sibling23 - null
- | | ||
- |null null null
- |
-child11 - sibling12 - sibling13 - sibling14 - null
- | | ||
- | | | null
- | | |
- nullnull child131 - null
- |
- null
-
-Figure 1: Generic structure of un-flattened device tree
-
-
-Before executing OF unittest, it is required to attach the test data to
-machine's device tree (if present). So, when unittest_data_add() is called,
-at first it reads the flattened device tree data linked into the kernel image
-via the following kernel symbols:
-
-__dtb_testcases_begin - address marking the start of test data blob
-__dtb_testcases_end - address marking the end of test data blob
-
-Secondly, it calls of_fdt_unflatten_tree() to unflatten the flattened
-blob. And finally, if the machine's
Re: [PATCH] of: Fix NULL dereference in selftest removal code
Thanks for the fix, I just noted that you increased NO_OF_NODES to 3 in
following phandle resolver patch.
So, with NO_OF_NODES = 3, the following patch works fine on arm, x86
and powerpc.
On Wed, Oct 1, 2014 at 9:02 AM, Grant Likely wrote:
> The selftest code removes its testcase data from the live tree when
> exiting, but if the testcases data tree contains an empty child of the
> root, then it causes an oops due to a NULL dereference. The reason is
> that the code tries to directly dereference the child pointer without
> checking first if a child is actually there.
>
> The solution is to pass the parent node into detach_node_and_children()
> instead of trying to pass the child. This required removing the code
> that attempts to remove all of the sibling nodes in
> detach_node_and_children(), which was never sensible in the first place.
>
> At the same time add a check to make sure the bounds of the nodes list
> are not exceeded by the testdata tree. If they are then abort.
>
> Signed-off-by: Grant Likely
> Cc: Gaurav Minocha
> ---
> drivers/of/selftest.c | 9 +++--
> 1 file changed, 3 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/of/selftest.c b/drivers/of/selftest.c
> index a737cb5974de..883e60b04eb5 100644
> --- a/drivers/of/selftest.c
> +++ b/drivers/of/selftest.c
> @@ -637,6 +637,8 @@ static int attach_node_and_children(struct device_node
> *np)
> dup = np;
>
> while (dup) {
> + if (WARN_ON(last_node_index >= NO_OF_NODES))
> + return -EINVAL;
> nodes[last_node_index++] = dup;
> dup = dup->sibling;
> }
> @@ -717,10 +719,6 @@ static void detach_node_and_children(struct device_node
> *np)
> {
> while (np->child)
> detach_node_and_children(np->child);
> -
> - while (np->sibling)
> - detach_node_and_children(np->sibling);
> -
> of_detach_node(np);
> }
>
> @@ -749,8 +747,7 @@ static void selftest_data_remove(void)
> if (nodes[last_node_index]) {
> np =
> of_find_node_by_path(nodes[last_node_index]->full_name);
> if (strcmp(np->full_name, "/aliases") != 0) {
> - detach_node_and_children(np->child);
> - of_detach_node(np);
> + detach_node_and_children(np);
> } else {
> for_each_property_of_node(np, prop) {
> if (strcmp(prop->name,
> "testcase-alias") == 0)
> --
> 1.9.1
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] of: Fix NULL dereference in selftest removal code
Thanks for the fix, I just noted that you increased NO_OF_NODES to 3 in
following phandle resolver patch.
So, with NO_OF_NODES = 3, the following patch works fine on arm, x86
and powerpc.
On Wed, Oct 1, 2014 at 9:02 AM, Grant Likely grant.lik...@linaro.org wrote:
The selftest code removes its testcase data from the live tree when
exiting, but if the testcases data tree contains an empty child of the
root, then it causes an oops due to a NULL dereference. The reason is
that the code tries to directly dereference the child pointer without
checking first if a child is actually there.
The solution is to pass the parent node into detach_node_and_children()
instead of trying to pass the child. This required removing the code
that attempts to remove all of the sibling nodes in
detach_node_and_children(), which was never sensible in the first place.
At the same time add a check to make sure the bounds of the nodes list
are not exceeded by the testdata tree. If they are then abort.
Signed-off-by: Grant Likely grant.lik...@linaro.org
Cc: Gaurav Minocha gaurav.minocha...@gmail.com
---
drivers/of/selftest.c | 9 +++--
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/drivers/of/selftest.c b/drivers/of/selftest.c
index a737cb5974de..883e60b04eb5 100644
--- a/drivers/of/selftest.c
+++ b/drivers/of/selftest.c
@@ -637,6 +637,8 @@ static int attach_node_and_children(struct device_node
*np)
dup = np;
while (dup) {
+ if (WARN_ON(last_node_index = NO_OF_NODES))
+ return -EINVAL;
nodes[last_node_index++] = dup;
dup = dup-sibling;
}
@@ -717,10 +719,6 @@ static void detach_node_and_children(struct device_node
*np)
{
while (np-child)
detach_node_and_children(np-child);
-
- while (np-sibling)
- detach_node_and_children(np-sibling);
-
of_detach_node(np);
}
@@ -749,8 +747,7 @@ static void selftest_data_remove(void)
if (nodes[last_node_index]) {
np =
of_find_node_by_path(nodes[last_node_index]-full_name);
if (strcmp(np-full_name, /aliases) != 0) {
- detach_node_and_children(np-child);
- of_detach_node(np);
+ detach_node_and_children(np);
} else {
for_each_property_of_node(np, prop) {
if (strcmp(prop-name,
testcase-alias) == 0)
--
1.9.1
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [OF test] BUG: unable to handle kernel NULL pointer dereference at 00000038
On Wed, Aug 27, 2014 at 6:04 AM, Fengguang Wu wrote: > Greetings, > > 0day kernel testing robot got the below dmesg and the first bad commit is It failed because CONFIG_SYSFS isn't enabled in your configuration. Will discuss with Grant and patch it soon. Thanks! > > git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master > commit b951f9dc7f25fc1e39aafda5edb4b47b38285d9f > Author: Gaurav Minocha > AuthorDate: Sat Jul 26 12:48:50 2014 -0700 > Commit: Grant Likely > CommitDate: Sat Aug 16 09:03:56 2014 +0100 > > Enabling OF selftest to run without machine's devicetree > > If there is no devicetree present, this patch adds the selftest > data as a live devicetree. It also removes the same after the > testcase execution is complete. > > Tested with and without machine's devicetree. > > Signed-off-by: Gaurav Minocha > Signed-off-by: Grant Likely > > +--+++---+ > | | b5f2a8c026 | > b951f9dc7f | next-20140827 | > +--+++---+ > | boot_successes | 60 | 0 >| 0 | > | boot_failures| 0 | 20 >| 11| > | BUG:unable_to_handle_kernel_NULL_pointer_dereference | 0 | 20 >| 11| > | Oops | 0 | 20 >| 11| > | EIP_is_at_kernfs_find_ns | 0 | 20 >| 11| > | Kernel_panic-not_syncing:Fatal_exception | 0 | 20 >| 11| > | backtrace:of_selftest| 0 | 20 >| 11| > | backtrace:kernel_init_freeable | 0 | 20 >| 11| > +--+++---+ > > [2.779062] rtc-test rtc-test.0: setting system clock to 2014-08-27 > 12:23:45 UTC (1409142225) > [2.779708] BIOS EDD facility v0.16 2004-Jun-25, 0 devices found > [2.780171] EDD information not available. > [2.780575] BUG: unable to handle kernel NULL pointer dereference at > 0038 > [2.781133] IP: [] kernfs_find_ns+0xd/0xe4 > [2.781518] *pde = > [2.781747] Oops: [#1] PREEMPT > [2.782037] Modules linked in: > [2.782291] CPU: 0 PID: 1 Comm: swapper Not tainted 3.16.0-04165-gb951f9d > #8 > [2.782799] task: c34c1000 ti: c34c4000 task.ti: c34c4000 > [2.783200] EIP: 0060:[] EFLAGS: 00010282 CPU: 0 > [2.783599] EIP is at kernfs_find_ns+0xd/0xe4 > [2.783921] EAX: EBX: ECX: EDX: b158c20a > [2.784164] ESI: EDI: c34f0df0 EBP: c34c5e94 ESP: c34c5e80 > [2.784164] DS: 007b ES: 007b FS: GS: SS: 0068 > [2.784164] CR0: 8005003b CR2: 0038 CR3: 01675000 CR4: 06d0 > [2.784164] Stack: > [2.784164] 36b0 b0085008 b158c20a c34f0df0 c34c5ea8 > b10e6674 > [2.784164] b158c20a c34c5ec0 b13552ea b158c20a b0088000 > b0088000 > [2.784164] c34c5ed0 b13563df b164a3b5 b0088000 c34c5f34 b164a456 > b13de13e 0003 > [2.784164] Call Trace: > [2.784164] [] kernfs_find_and_get_ns+0x25/0x3d > [2.784164] [] safe_name+0x4d/0x70 > [2.784164] [] __of_attach_node_sysfs+0x2d/0xa5 > [2.784164] [] ? of_selftest_platform_populate+0x1ca/0x1ca > [2.784164] [] of_selftest+0xa1/0xf46 > [2.784164] [] ? _raw_spin_unlock_irqrestore+0x39/0x54 > [2.784164] [] ? trace_hardirqs_on+0xb/0xd > [2.784164] [] ? slob_free+0x217/0x21f > [2.784164] [] ? of_selftest_platform_populate+0x1ca/0x1ca > [2.784164] [] ? of_selftest_platform_populate+0x1ca/0x1ca > [2.784164] [] do_one_initcall+0xce/0x160 > [2.784164] [] ? do_early_param+0x51/0x75 > [2.784164] [] ? parse_args+0x182/0x23b > [2.784164] [] kernel_init_freeable+0x184/0x20e > [2.784164] [] kernel_init+0x8/0xb8 > [2.784164] [] ret_from_kernel_thread+0x20/0x30 > [2.784164] [] ? rest_init+0xa0/0xa0 > [2.784164] Code: 5e 2f 00 89 d8 e8 21 b9 fd ff 85 c0 0f 95 c0 0f b6 c0 eb > 06 b8 f6 ff ff ff c3 5b 5e 5d c3 55 89 e5 57 56 89 c6 53 89 cb 83 ec 08 <8b> > 78 38 66 8b 40 4c 89 55 f0 66 c1 e8 05 83 e0 01 83 3d 94 9c > [2.784164] EIP: [] kernfs_find_ns+0xd/0xe4 SS:ESP 0068:c34c5e80 > [2.784164] CR2: 0038 > [2.784
Re: [OF test] BUG: unable to handle kernel NULL pointer dereference at 00000038
On Wed, Aug 27, 2014 at 6:04 AM, Fengguang Wu fengguang...@intel.com wrote: Greetings, 0day kernel testing robot got the below dmesg and the first bad commit is It failed because CONFIG_SYSFS isn't enabled in your configuration. Will discuss with Grant and patch it soon. Thanks! git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master commit b951f9dc7f25fc1e39aafda5edb4b47b38285d9f Author: Gaurav Minocha gaurav.minocha...@gmail.com AuthorDate: Sat Jul 26 12:48:50 2014 -0700 Commit: Grant Likely grant.lik...@linaro.org CommitDate: Sat Aug 16 09:03:56 2014 +0100 Enabling OF selftest to run without machine's devicetree If there is no devicetree present, this patch adds the selftest data as a live devicetree. It also removes the same after the testcase execution is complete. Tested with and without machine's devicetree. Signed-off-by: Gaurav Minocha gaurav.minocha...@gmail.com Signed-off-by: Grant Likely grant.lik...@linaro.org +--+++---+ | | b5f2a8c026 | b951f9dc7f | next-20140827 | +--+++---+ | boot_successes | 60 | 0 | 0 | | boot_failures| 0 | 20 | 11| | BUG:unable_to_handle_kernel_NULL_pointer_dereference | 0 | 20 | 11| | Oops | 0 | 20 | 11| | EIP_is_at_kernfs_find_ns | 0 | 20 | 11| | Kernel_panic-not_syncing:Fatal_exception | 0 | 20 | 11| | backtrace:of_selftest| 0 | 20 | 11| | backtrace:kernel_init_freeable | 0 | 20 | 11| +--+++---+ [2.779062] rtc-test rtc-test.0: setting system clock to 2014-08-27 12:23:45 UTC (1409142225) [2.779708] BIOS EDD facility v0.16 2004-Jun-25, 0 devices found [2.780171] EDD information not available. [2.780575] BUG: unable to handle kernel NULL pointer dereference at 0038 [2.781133] IP: [b10e6578] kernfs_find_ns+0xd/0xe4 [2.781518] *pde = [2.781747] Oops: [#1] PREEMPT [2.782037] Modules linked in: [2.782291] CPU: 0 PID: 1 Comm: swapper Not tainted 3.16.0-04165-gb951f9d #8 [2.782799] task: c34c1000 ti: c34c4000 task.ti: c34c4000 [2.783200] EIP: 0060:[b10e6578] EFLAGS: 00010282 CPU: 0 [2.783599] EIP is at kernfs_find_ns+0xd/0xe4 [2.783921] EAX: EBX: ECX: EDX: b158c20a [2.784164] ESI: EDI: c34f0df0 EBP: c34c5e94 ESP: c34c5e80 [2.784164] DS: 007b ES: 007b FS: GS: SS: 0068 [2.784164] CR0: 8005003b CR2: 0038 CR3: 01675000 CR4: 06d0 [2.784164] Stack: [2.784164] 36b0 b0085008 b158c20a c34f0df0 c34c5ea8 b10e6674 [2.784164] b158c20a c34c5ec0 b13552ea b158c20a b0088000 b0088000 [2.784164] c34c5ed0 b13563df b164a3b5 b0088000 c34c5f34 b164a456 b13de13e 0003 [2.784164] Call Trace: [2.784164] [b10e6674] kernfs_find_and_get_ns+0x25/0x3d [2.784164] [b13552ea] safe_name+0x4d/0x70 [2.784164] [b13563df] __of_attach_node_sysfs+0x2d/0xa5 [2.784164] [b164a3b5] ? of_selftest_platform_populate+0x1ca/0x1ca [2.784164] [b164a456] of_selftest+0xa1/0xf46 [2.784164] [b13de13e] ? _raw_spin_unlock_irqrestore+0x39/0x54 [2.784164] [b104b60c] ? trace_hardirqs_on+0xb/0xd [2.784164] [b10a6390] ? slob_free+0x217/0x21f [2.784164] [b164a3b5] ? of_selftest_platform_populate+0x1ca/0x1ca [2.784164] [b164a3b5] ? of_selftest_platform_populate+0x1ca/0x1ca [2.784164] [b100045b] do_one_initcall+0xce/0x160 [2.784164] [b1626400] ? do_early_param+0x51/0x75 [2.784164] [b103eeb9] ? parse_args+0x182/0x23b [2.784164] [b1626beb] kernel_init_freeable+0x184/0x20e [2.784164] [b13d5b3c] kernel_init+0x8/0xb8 [2.784164] [b13de840] ret_from_kernel_thread+0x20/0x30 [2.784164] [b13d5b34] ? rest_init+0xa0/0xa0 [2.784164] Code: 5e 2f 00 89 d8 e8 21 b9 fd ff 85 c0 0f 95 c0 0f b6 c0 eb 06 b8 f6 ff ff ff c3 5b 5e 5d c3 55 89 e5 57 56 89 c6 53 89 cb 83 ec 08 8b 78 38 66 8b 40 4c 89 55 f0 66 c1 e8 05 83 e0 01 83 3d 94 9c [2.784164] EIP: [b10e6578] kernfs_find_ns+0xd/0xe4 SS:ESP 0068:c34c5e80 [2.784164] CR2: 0038 [2.784164] ---[ end trace 411ad12a024bcda1 ]--- [2.784164] Kernel panic - not syncing: Fatal exception
