Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752801AbcD1Xoa (ORCPT ); Thu, 28 Apr 2016 19:44:30 -0400 Received: from mail-wm0-f65.google.com ([74.125.82.65]:34551 "EHLO mail-wm0-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752621AbcD1Xo3 convert rfc822-to-8bit (ORCPT ); Thu, 28 Apr 2016 19:44:29 -0400 MIME-Version: 1.0 In-Reply-To: <57229D47.6070704@gmail.com> References: <572284AB.102@gmail.com> <57229D47.6070704@gmail.com> Date: Thu, 28 Apr 2016 16:44:27 -0700 Message-ID: Subject: Re: [PATCH] scripts/dtc: dt_to_config - report kernel config options for a devicetree From: Gaurav Minocha To: Frank Rowand Cc: Rob Herring , Grant Likely , "devicetree@vger.kernel.org" , Linux Kernel list , Geert Uytterhoeven , Pavel Machek Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8BIT Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 8252 Lines: 209 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@10050000 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' => ['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'], >>> + 'arch/mips/mti-malta/malta-dt.c' => ['obj-y'], >>> + 'arch/x86/kernel/devicetree.c' => ['CONFIG_OF'], >>> + 'drivers/mtd/maps/physmap_of.c' => ['CONFIG_MTD_PHYSMAP_OF'], >>> + 'drivers/video/backlight/pwm_bl.c' => ['CONFIG_BACKLIGHT_PWM'], >>> + 'drivers/mfd/syscon.c' => ['CONFIG_MFD_SYSCON'], >> >> I don't understand why some of these are not searchable by compatible strings. > > I will have to get back to you later on this question. > > >>> + >>> + # drivers/usb/host/ehci-ppc-of.c >>> + # drivers/usb/host/ehci-xilinx-of.c >>> + # are included from: >>> + # drivers/usb/host/ehci-hcd.c >>> + # thus the search of Makefile for the included .c files is incorrect >>> + # ehci-hcd.c wraps the includes with ifdef CONFIG_USB_EHCI_HCD_..._OF >>> + # >>> + # similar model for ohci-hcd.c (but no ohci-xilinx-of.c) >>> + # >>> + # similarly, uhci-hcd.c includes uhci-platform.c >>> + >>> + 'drivers/usb/host/ehci-ppc-of.c' => ['CONFIG_USB_EHCI_HCD', >>> + 'CONFIG_USB_EHCI_HCD_PPC_OF'], >>> + 'drivers/usb/host/ohci-ppc-of.c' => ['CONFIG_USB_OHCI_HCD', >>> + 'CONFIG_USB_OHCI_HCD_PPC_OF'], >>> + >>> + 'drivers/usb/host/ehci-xilinx-of.c' => ['CONFIG_USB_EHCI_HCD', >>> + 'CONFIG_USB_EHCI_HCD_XILINX'], >>> + >>> + 'drivers/usb/host/uhci-platform.c' => ['CONFIG_USB_UHCI_HCD', >>> + 'CONFIG_USB_UHCI_PLATFORM'], >>> + >>> + # scan_makefile will find only one of these config options: >>> + # ifneq ($(CONFIG_SOC_IMX6)$(CONFIG_SOC_LS1021A),) >>> + 'arch/arm/mach-imx/platsmp.c' => ['CONFIG_SOC_IMX6 && CONFIG_SMP', >>> + 'CONFIG_SOC_LS1021A && CONFIG_SMP'], >> >> These will never get updated when the files or config options change. >> Conversely, how do I know if I need to add something here? Is this >> list complete or based on testing some set of dts files. IMO, this >> list has to go to merge this. > > That is a concern that I have also. I added a warning flag in the > flags field to indicate when one of these hard-coded values was used. > The flag does not solve the issue, but does make it more visible. > > The list is a result of running against every dts file in the kernel > tree and looking at the reports. > > >>> +); >>> + >>> + >>> +# 'virt/kvm/arm/.*' are controlled by makefiles in other directories, >>> +# using relative paths, such as 'KVM := ../../../virt/kvm'. Do not >>> +# add complexity to find_kconfig() to deal with this. There is a long >>> +# term intent to change the kvm related makefiles to the normal kernel >>> +# style. After that is done, this entry can be removed from the >>> +# driver_black_list. >>> + >>> +@driver_black_list = ( >>> + 'virt/kvm/arm/.*', >>> +); >> >> A small number of exceptions like this I could stomach. >> >> The rest is just write-only language nonsense... >> >> Rob >> >