diff mbox series

cmd: ubi: change 'default y' for SUNXI to 'imply' in Kconfig

Message ID 1531116757-5995-1-git-send-email-yamada.masahiro@socionext.com
State Accepted
Commit c6cca10b9c29224b62c93e24389751c487b98a7d
Headers show
Series cmd: ubi: change 'default y' for SUNXI to 'imply' in Kconfig | expand

Commit Message

Masahiro Yamada July 9, 2018, 6:12 a.m. UTC
It is not preferred to put SUNXI-specific code in the common place.

Change it to 'imply' property of ARCH_SUNXI.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

 arch/arm/Kconfig | 1 +
 cmd/Kconfig      | 1 -
 2 files changed, 1 insertion(+), 1 deletion(-)

Comments

Maxime Ripard July 9, 2018, 7:20 a.m. UTC | #1
On Mon, Jul 09, 2018 at 03:12:37PM +0900, Masahiro Yamada wrote:
> It is not preferred to put SUNXI-specific code in the common place.


I think Tom has been saying the exact opposite a number of times,
since there's some circular dependencies issues that arise when using
imply.

Maxime

> Change it to 'imply' property of ARCH_SUNXI.

> 

> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>

> ---

> 

>  arch/arm/Kconfig | 1 +

>  cmd/Kconfig      | 1 -

>  2 files changed, 1 insertion(+), 1 deletion(-)

> 

> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig

> index 7d394f2..a6ce155 100644

> --- a/arch/arm/Kconfig

> +++ b/arch/arm/Kconfig

> @@ -796,6 +796,7 @@ config ARCH_SUNXI

>  	select USB_KEYBOARD if DISTRO_DEFAULTS

>  	select USE_TINY_PRINTF

>  	imply CMD_GPT

> +	imply CMD_UBI if NAND

>  	imply DISTRO_DEFAULTS

>  	imply FAT_WRITE

>  	imply OF_LIBFDT_OVERLAY

> diff --git a/cmd/Kconfig b/cmd/Kconfig

> index aec2090..159ebaa 100644

> --- a/cmd/Kconfig

> +++ b/cmd/Kconfig

> @@ -1792,7 +1792,6 @@ config CMD_UBI

>  	select CRC32

>  	select MTD_UBI

>  	select CMD_MTDPARTS

> -	default y if NAND_SUNXI

>  	help

>  	  UBI is a software layer above MTD layer which admits use of LVM-like

>  	  logical volumes on top of MTD devices, hides some complexities of

> -- 

> 2.7.4

> 


-- 
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com
Masahiro Yamada July 9, 2018, 11:14 a.m. UTC | #2
2018-07-09 16:20 GMT+09:00 Maxime Ripard <maxime.ripard@bootlin.com>:
> On Mon, Jul 09, 2018 at 03:12:37PM +0900, Masahiro Yamada wrote:
>> It is not preferred to put SUNXI-specific code in the common place.
>
> I think Tom has been saying the exact opposite a number of times,
> since there's some circular dependencies issues that arise when using
> imply.


Platforms select/imply necessary drivers or features.
This is what we usually do.


ARCH_SUNXI already uses select / imply in arch/arm/Kconfig.
(Also ARCH_SUNXI does so in arch/arm/mach-sunxi/Kconfig in Linux Kernel)


On the other hand, no drivers or features select / imply ARCH_SUNXI.


To sum up, select or imply should be used
in the following direction.

   Platform  ---(select or imply)--->  features


If you follow this rule,
circular dependency will not happen.






> Maxime
>
>> Change it to 'imply' property of ARCH_SUNXI.
>>
>> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
>> ---
>>
>>  arch/arm/Kconfig | 1 +
>>  cmd/Kconfig      | 1 -
>>  2 files changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
>> index 7d394f2..a6ce155 100644
>> --- a/arch/arm/Kconfig
>> +++ b/arch/arm/Kconfig
>> @@ -796,6 +796,7 @@ config ARCH_SUNXI
>>       select USB_KEYBOARD if DISTRO_DEFAULTS
>>       select USE_TINY_PRINTF
>>       imply CMD_GPT
>> +     imply CMD_UBI if NAND
>>       imply DISTRO_DEFAULTS
>>       imply FAT_WRITE
>>       imply OF_LIBFDT_OVERLAY
>> diff --git a/cmd/Kconfig b/cmd/Kconfig
>> index aec2090..159ebaa 100644
>> --- a/cmd/Kconfig
>> +++ b/cmd/Kconfig
>> @@ -1792,7 +1792,6 @@ config CMD_UBI
>>       select CRC32
>>       select MTD_UBI
>>       select CMD_MTDPARTS
>> -     default y if NAND_SUNXI
>>       help
>>         UBI is a software layer above MTD layer which admits use of LVM-like
>>         logical volumes on top of MTD devices, hides some complexities of
>> --
>> 2.7.4
>>
>
> --
> Maxime Ripard, Bootlin (formerly Free Electrons)
> Embedded Linux and Kernel engineering
> https://bootlin.com
>
> _______________________________________________
> U-Boot mailing list
> U-Boot@lists.denx.de
> https://lists.denx.de/listinfo/u-boot
>
Tom Rini July 9, 2018, 1:10 p.m. UTC | #3
On Mon, Jul 09, 2018 at 08:14:28PM +0900, Masahiro Yamada wrote:
> 2018-07-09 16:20 GMT+09:00 Maxime Ripard <maxime.ripard@bootlin.com>:

> > On Mon, Jul 09, 2018 at 03:12:37PM +0900, Masahiro Yamada wrote:

> >> It is not preferred to put SUNXI-specific code in the common place.

> >

> > I think Tom has been saying the exact opposite a number of times,

> > since there's some circular dependencies issues that arise when using

> > imply.

> 

> 

> Platforms select/imply necessary drivers or features.

> This is what we usually do.

> 

> 

> ARCH_SUNXI already uses select / imply in arch/arm/Kconfig.

> (Also ARCH_SUNXI does so in arch/arm/mach-sunxi/Kconfig in Linux Kernel)

> 

> 

> On the other hand, no drivers or features select / imply ARCH_SUNXI.

> 

> 

> To sum up, select or imply should be used

> in the following direction.

> 

>    Platform  ---(select or imply)--->  features

> 

> 

> If you follow this rule,

> circular dependency will not happen.


I think what ends up being confusing about this is that we must do:
    default 0xDEADBEEF if ARCH_FOO

as we cannot:
    imply FOO_OPTION 0xDEADBEEF if FOO_DRIVER

-- 
Tom
Tom Rini Aug. 26, 2018, 9:01 p.m. UTC | #4
On Mon, Jul 09, 2018 at 03:12:37PM +0900, Masahiro Yamada wrote:

> It is not preferred to put SUNXI-specific code in the common place.

> 

> Change it to 'imply' property of ARCH_SUNXI.

> 

> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>


Applied to u-boot/master, thanks!

-- 
Tom
diff mbox series

Patch

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 7d394f2..a6ce155 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -796,6 +796,7 @@  config ARCH_SUNXI
 	select USB_KEYBOARD if DISTRO_DEFAULTS
 	select USE_TINY_PRINTF
 	imply CMD_GPT
+	imply CMD_UBI if NAND
 	imply DISTRO_DEFAULTS
 	imply FAT_WRITE
 	imply OF_LIBFDT_OVERLAY
diff --git a/cmd/Kconfig b/cmd/Kconfig
index aec2090..159ebaa 100644
--- a/cmd/Kconfig
+++ b/cmd/Kconfig
@@ -1792,7 +1792,6 @@  config CMD_UBI
 	select CRC32
 	select MTD_UBI
 	select CMD_MTDPARTS
-	default y if NAND_SUNXI
 	help
 	  UBI is a software layer above MTD layer which admits use of LVM-like
 	  logical volumes on top of MTD devices, hides some complexities of