[U-Boot] [PATCH 1/2] RFC: dm: Kconfig: Move all driver model CONFIGs to Kconfig

[Simon Glass <sjg@chromium.org>]

Hi Masahiro,

On 11 December 2014 at 10:37, Masahiro YAMADA <yamada.m@jp.panasonic.com> wrote:
> Hi Simon,
>
>
>
>>
>> diff --git a/Kconfig b/Kconfig
>> index 153ee2b..d4ca152 100644
>> --- a/Kconfig
>> +++ b/Kconfig
>> @@ -158,3 +158,5 @@ source "drivers/Kconfig"
>>  source "fs/Kconfig"
>>
>>  source "lib/Kconfig"
>> +
>> +source "test/dm/Kconfig"
>
> Perhaps we might want "test/Kconfig" in the future.
>
>
>
>
>
>> --- a/arch/arm/cpu/armv7/tegra-common/Kconfig
>> +++ b/arch/arm/cpu/armv7/tegra-common/Kconfig
>> @@ -20,6 +20,27 @@ endchoice
>>  config USE_PRIVATE_LIBGCC
>>         default y if SPL_BUILD
>>
>> +config DM
>> +       default y
>> +
>> +config SPL_DM
>> +       default y
>> +
>> +config DM_SERIAL
>> +       default y
>> +
>> +config DM_SPI
>> +       default y
>> +
>> +config DM_SPI_FLASH
>> +       default y
>> +
>> +config DM_I2C
>> +       default y
>> +
>> +config DM_GPIO
>> +       default y
>> +
>
>
> Are all of these user-configurable?
>
> I mean, do Tegra boards work fine without CONFIG_DM_* ?
> I doubt it because you have already converted Tegra drivers.
>
>
> - Use "select" if the option is mandatory for that board
>
> - Use "default y" if the option is still optional, but recommended.
>
>

OK will change this for tegra.

>
>
>
>
>
>>  source "arch/arm/cpu/armv7/tegra20/Kconfig"
>>  source "arch/arm/cpu/armv7/tegra30/Kconfig"
>>  source "arch/arm/cpu/armv7/tegra114/Kconfig"
>> diff --git a/arch/sandbox/Kconfig b/arch/sandbox/Kconfig
>> index 3057325..1b4cbce 100644
>> --- a/arch/sandbox/Kconfig
>> +++ b/arch/sandbox/Kconfig
>> @@ -10,4 +10,43 @@ config SYS_BOARD
>>  config SYS_CONFIG_NAME
>>         default "sandbox"
>>
>> +config DM
>> +       default y
>> +
>> +config DM_GPIO
>> +       default y
>> +
>> +config DM_SERIAL
>> +       default y
>> +
>> +config CMD_DEMO
>> +       default y
>> +
>> +config CMD_DM
>> +       default y
>> +
>> +config DM_DEMO
>> +       default y
>> +
>> +config DM_DEMO_SIMPLE
>> +       default y
>> +
>> +config DM_DEMO_SHAPE
>> +       default y
>> +
>> +config DM_TEST
>> +       default y
>> +
>> +config DM_CROS_EC
>> +       default y
>> +
>> +config DM_SPI
>> +       default y
>> +
>> +config DM_SPI_FLASH
>> +       default y
>> +
>> +config DM_I2C
>> +       default y
>> +
>
>
> Why do you like this syntax?
>
>  - If the option is not user-configurable, use "select"
>  - If the option is user-configurable and you want change the default value,
>    add it to configs/sandbox_defconfig
>
>

OK I'll split these into two types.

>
>
>
>
>
>> diff --git a/board/samsung/goni/Kconfig b/board/samsung/goni/Kconfig
>> index cbbf5a9..006e864 100644
>> --- a/board/samsung/goni/Kconfig
>> +++ b/board/samsung/goni/Kconfig
>> @@ -12,4 +12,13 @@ config SYS_SOC
>>  config SYS_CONFIG_NAME
>>         default "s5p_goni"
>>
>> +config DM
>> +       default y
>> +
>> +config DM_GPIO
>> +       default y
>> +
>> +config DM_SERIAL
>> +       default y
>> +
>
>
> "select" or "_defconfig"
>

Will go with select here.

>
>
>> diff --git a/common/Kconfig b/common/Kconfig
>> index fd84fa0..174bf4f 100644
>> --- a/common/Kconfig
>> +++ b/common/Kconfig
>> @@ -153,6 +153,29 @@ endmenu
>>
>>  menu "Device access commands"
>>
>> +config CMD_DM
>> +       bool "dm - Access to driver model information"
>> +       depends on DM
>> +       default y
>> +       help
>> +         Provides access to driver model data structures and information,
>> +         such as a list of devices, list of uclasses and the state of each
>> +         device (e.g. activated). This is not required for operation, but
>> +         can be useful to see the state of driver model for debugging or
>> +         interest.
>> +
>> +config CMD_DEMO
>> +       bool "demo - Demonstration commands for driver model"
>> +       depends on DM
>> +       help
>> +         Procides a 'demo' command which can be used to play around with
>> +         driver model. To use this properly you will need to enable one or
>> +         both of the demo devices (DM_DEMO_SHAPE and DM_DEMO_SIMPLE).
>> +         Otherwise you will always get an empty list of devices. The demo
>> +         devices are defined in the sandbox device tree, so the easiest
>> +         option is to use sandbox and pass the -d point to sandbox's
>> +         u-boot.dtb file.
>> +
>
>
> These seems rather "Misc commands".
>

They are driver-model-related though.

>
>
>>  config CMD_LOADB
>>         bool "loadb"
>>         help
>> diff --git a/configs/Linksprite_pcDuino3_fdt_defconfig b/configs/Linksprite_pcDuino3_fdt_defconfig
>> index a33f3a7..931b5dd 100644
>> --- a/configs/Linksprite_pcDuino3_fdt_defconfig
>> +++ b/configs/Linksprite_pcDuino3_fdt_defconfig
>> @@ -2,6 +2,8 @@ CONFIG_SPL=y
>>  CONFIG_SYS_EXTRA_OPTIONS="AXP209_POWER,SUNXI_GMAC,AHCI,SATAPWR=SUNXI_GPH(2),USB_EHCI"
>>  CONFIG_FDTFILE="sun7i-a20-pcduino3.dtb"
>>  CONFIG_DM=y
>> +CONFIG_DM_GPIO=y
>> +CONFIG_DM_SERIAL=y
>>  CONFIG_DEFAULT_DEVICE_TREE="sun7i-a20-pcduino3"
>>  CONFIG_OF_CONTROL=y
>>  CONFIG_OF_SEPARATE=y
>
> Here, you use _defconfig.
>
> What is the difference?

This board shares config with others so I can't change the defaults.
In fact this board still uses CONFIG_SYS_EXTRA_OPTIONS which is
unfortunate a) because it should use device tree for GPIOs and b)
because we are trying to deprecate these.

>
>
>
>> diff --git a/drivers/core/Kconfig b/drivers/core/Kconfig
>> index d2799dc..2ae2cc4 100644
>> --- a/drivers/core/Kconfig
>> +++ b/drivers/core/Kconfig
>> @@ -2,5 +2,123 @@ config DM
>>         bool "Enable Driver Model"
>>         depends on !SPL_BUILD
>>         help
>> -         This config option enables Driver Model.
>> -         To use legacy drivers, say N.
>> +         This config option enables Driver Model. This brings in the core
>> +         support, including scanning of platform data on start-up. If
>> +         CONFIG_OF_CONTROL is enabled, the device tree will be scanned also
>> +         when available.
>> +
>> +config DM_SERIAL
>> +       bool "Enable Driver Model for serial"
>> +       depends on DM
>> +       help
>> +         Enable driver model for serial. This replaces
>> +         drivers/serial/serial.c with the serial uclass, which
>> +         implements serial_putc() etc. The uclass interface is
>> +         defined in include/serial.h.
>> +
>> +config DM_GPIO
>> +       bool "Enable Driver Model for GPIO"
>> +       depends on DM
>> +       help
>> +         Enable driver model for GPIO access. The standard GPIO
>> +         interface (gpio_get_value(), etc.) is then implemented by
>> +         the GPIO uclass. Drivers provide methods to query the
>> +         particular GPIOs that they provide. The uclass interface
>> +         is defined in include/asm-generic/gpio.h.
>>
>> +config DM_SPI
>> +       bool "Enable Driver Model for SPI"
>> +       depends on DM
>> +       help
>> +         Enable driver model for SPI. The SPI slave interface
>> +         (spi_setup_slave(), spi_xfer(), etc.) is then implemented by
>> +         the SPI uclass. Drivers provide methods to access the SPI
>> +         buses that they control. The uclass interface is defined in
>> +         include/spi.h. The existing spi_slave structure is attached
>> +         as 'parent data' to every slave on each bus. Slaves
>> +         typically use driver-private data instead of extending the
>> +         spi_slave structure.
>> +
>
>
>
> DM_{SERIAL,GPIO,SPI} is already in drivers/{serial,gpio,spi}/Kconfig.
>
> IMHO, I think drivers/*/Kconfig would be a suitable place,
> but it is OK if you want to move them.

Yes I agree, let's leave them there. Maybe I didn't notice that you
had already done this work.

>
> Anyway, these configs are temporary.
> When we finish DM conversion, CONFIG_DM* will go away.

Yes, looking forward to it. I'm planning to start deprecation for
serial and GPIO soon.

>
>
>
>> new file mode 100644
>> index 0000000..4234fe4
>> --- /dev/null
>> +++ b/drivers/demo/Kconfig
>> @@ -0,0 +1,26 @@
>> +config DM_DEMO
>> +       bool "Enable demo uclass support"
>> +       depends on DM
>> +       help
>> +         This uclass allows you to play around with driver model. It provides
>> +         an interface to a couple of demo devices. You can access it using
>> +         the 'demo' command or by calling the uclass functions from your
>> +         own code.
>> +
>> +config DM_DEMO_SIMPLE
>> +       bool "Enable simple demo device for driver model"
>> +       depends on DM
>
> depends on DM_DEMO
>
>> +       help
>> +         This device allows you to play around with driver model. It prints
>> +         a message when the 'demo hello' command is executed which targets
>> +         this device. It can be used to help understand how driver model
>> +         works.
>> +
>> +config DM_DEMO_SHAPE
>> +       bool "Enable shape demo device for driver model"
>> +       depends on DM
>
> Ditto.
>
>

OK

>
>
>
>
>> +++ b/test/dm/Kconfig
>> @@ -0,0 +1,8 @@
>> +config DM_TEST
>> +       bool "Enable driver model test command"
>> +       depends on SANDBOX
>
> also depends on CMD_DM ??

Yes, that seem right.

Regards,
Simon