public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
* [U-Boot] [PATCH 0/2] Change DM_SEQ_ALIAS to be configurable for SPL
@ 2015-12-11 14:48 Nathan Rossi
  2015-12-11 14:48 ` [U-Boot] [PATCH 1/2] spl: dm: Add SPL_DM_SEQ_ALIAS config option Nathan Rossi
  2015-12-11 14:48 ` [U-Boot] [PATCH 2/2] arm: zynq: Enable SPL_DM_SEQ_ALIAS for all Zynq configs Nathan Rossi
  0 siblings, 2 replies; 14+ messages in thread
From: Nathan Rossi @ 2015-12-11 14:48 UTC (permalink / raw)
  To: u-boot

Add a config option for SPL_DM_SEQ_ALIAS and enable it for the Zynq
targets which require it for SPI flash support in SPL.

Nathan Rossi (2):
  spl: dm: Add SPL_DM_SEQ_ALIAS config option
  arm: zynq: Enable SPL_DM_SEQ_ALIAS for all Zynq configs

 configs/zynq_microzed_defconfig    | 1 +
 configs/zynq_picozed_defconfig     | 1 +
 configs/zynq_zc702_defconfig       | 1 +
 configs/zynq_zc706_defconfig       | 1 +
 configs/zynq_zc770_xm010_defconfig | 1 +
 configs/zynq_zc770_xm011_defconfig | 1 +
 configs/zynq_zc770_xm012_defconfig | 1 +
 configs/zynq_zc770_xm013_defconfig | 1 +
 configs/zynq_zed_defconfig         | 1 +
 configs/zynq_zybo_defconfig        | 1 +
 drivers/core/Kconfig               | 9 +++++++++
 drivers/core/device.c              | 2 +-
 include/config_uncmd_spl.h         | 1 -
 13 files changed, 20 insertions(+), 2 deletions(-)

-- 
2.6.2

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

* [U-Boot] [PATCH 1/2] spl: dm: Add SPL_DM_SEQ_ALIAS config option
  2015-12-11 14:48 [U-Boot] [PATCH 0/2] Change DM_SEQ_ALIAS to be configurable for SPL Nathan Rossi
@ 2015-12-11 14:48 ` Nathan Rossi
  2015-12-11 15:07   ` Marek Vasut
  2015-12-11 15:47   ` Simon Glass
  2015-12-11 14:48 ` [U-Boot] [PATCH 2/2] arm: zynq: Enable SPL_DM_SEQ_ALIAS for all Zynq configs Nathan Rossi
  1 sibling, 2 replies; 14+ messages in thread
From: Nathan Rossi @ 2015-12-11 14:48 UTC (permalink / raw)
  To: u-boot

The Device Model sequence alias feature is required by some Uclasses.
Instead of disabling the feature for all SPL targets allow it to be
configured.

The config option is disabled by default to reduce code size for targets
that are not interested or do not require this feature.

Signed-off-by: Nathan Rossi <nathan@nathanrossi.com>
Cc: Simon Glass <sjg@chromium.org>
Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
Cc: Linus Walleij <linus.walleij@linaro.org>
Cc: Marek Vasut <marex@denx.de>
Cc: Michal Simek <michal.simek@xilinx.com>
---
Based on a small amount of inspection for the Zynq platform, enabling
this config option adds ~1KB of code size.

Also on a side note, this might affect the socfpga target as it forcibly
overrides the #undef from config_uncmd_spl.h in its common header. I
have Cc'd the respective maintainer for this reason.
---
 drivers/core/Kconfig       | 9 +++++++++
 drivers/core/device.c      | 2 +-
 include/config_uncmd_spl.h | 1 -
 3 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/core/Kconfig b/drivers/core/Kconfig
index 53acee0..97aa01e 100644
--- a/drivers/core/Kconfig
+++ b/drivers/core/Kconfig
@@ -60,6 +60,15 @@ config DM_SEQ_ALIAS
 	help
 	  Most boards will have a '/aliases' node containing the path to
 	  numbered devices (e.g. serial0 = &serial0). This feature can be
+	  disabled if it is not required.
+
+config SPL_DM_SEQ_ALIAS
+	bool "Support numbered aliases in device tree in SPL"
+	depends on DM
+	default n
+	help
+	  Most boards will have a '/aliases' node containing the path to
+	  numbered devices (e.g. serial0 = &serial0). This feature can be
 	  disabled if it is not required, to save code space in SPL.
 
 config REGMAP
diff --git a/drivers/core/device.c b/drivers/core/device.c
index 758f390..b237b88 100644
--- a/drivers/core/device.c
+++ b/drivers/core/device.c
@@ -64,7 +64,7 @@ int device_bind(struct udevice *parent, const struct driver *drv,
 
 	dev->seq = -1;
 	dev->req_seq = -1;
-	if (CONFIG_IS_ENABLED(OF_CONTROL) && IS_ENABLED(CONFIG_DM_SEQ_ALIAS)) {
+	if (CONFIG_IS_ENABLED(OF_CONTROL) && CONFIG_IS_ENABLED(DM_SEQ_ALIAS)) {
 		/*
 		* Some devices, such as a SPI bus, I2C bus and serial ports
 		* are numbered using aliases.
diff --git a/include/config_uncmd_spl.h b/include/config_uncmd_spl.h
index 6e299f6..3b198ae 100644
--- a/include/config_uncmd_spl.h
+++ b/include/config_uncmd_spl.h
@@ -29,7 +29,6 @@
 #endif
 
 #undef CONFIG_DM_WARN
-#undef CONFIG_DM_SEQ_ALIAS
 #undef CONFIG_DM_STDIO
 
 #endif /* CONFIG_SPL_BUILD */
-- 
2.6.2

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

* [U-Boot] [PATCH 2/2] arm: zynq: Enable SPL_DM_SEQ_ALIAS for all Zynq configs
  2015-12-11 14:48 [U-Boot] [PATCH 0/2] Change DM_SEQ_ALIAS to be configurable for SPL Nathan Rossi
  2015-12-11 14:48 ` [U-Boot] [PATCH 1/2] spl: dm: Add SPL_DM_SEQ_ALIAS config option Nathan Rossi
@ 2015-12-11 14:48 ` Nathan Rossi
  1 sibling, 0 replies; 14+ messages in thread
From: Nathan Rossi @ 2015-12-11 14:48 UTC (permalink / raw)
  To: u-boot

This feature is required in SPL to enable support for loading from SPI
flash when the device is booted from QSPI.

Signed-off-by: Nathan Rossi <nathan@nathanrossi.com>
Cc: Michal Simek <michal.simek@xilinx.com>
---
 configs/zynq_microzed_defconfig    | 1 +
 configs/zynq_picozed_defconfig     | 1 +
 configs/zynq_zc702_defconfig       | 1 +
 configs/zynq_zc706_defconfig       | 1 +
 configs/zynq_zc770_xm010_defconfig | 1 +
 configs/zynq_zc770_xm011_defconfig | 1 +
 configs/zynq_zc770_xm012_defconfig | 1 +
 configs/zynq_zc770_xm013_defconfig | 1 +
 configs/zynq_zed_defconfig         | 1 +
 configs/zynq_zybo_defconfig        | 1 +
 10 files changed, 10 insertions(+)

diff --git a/configs/zynq_microzed_defconfig b/configs/zynq_microzed_defconfig
index c68efc8..35d2285 100644
--- a/configs/zynq_microzed_defconfig
+++ b/configs/zynq_microzed_defconfig
@@ -3,6 +3,7 @@ CONFIG_ARCH_ZYNQ=y
 CONFIG_TARGET_ZYNQ_MICROZED=y
 CONFIG_DEFAULT_DEVICE_TREE="zynq-microzed"
 CONFIG_SPL=y
+CONFIG_SPL_DM_SEQ_ALIAS=y
 CONFIG_FIT=y
 CONFIG_FIT_VERBOSE=y
 CONFIG_FIT_SIGNATURE=y
diff --git a/configs/zynq_picozed_defconfig b/configs/zynq_picozed_defconfig
index 62eb79f..dfe70e6 100644
--- a/configs/zynq_picozed_defconfig
+++ b/configs/zynq_picozed_defconfig
@@ -3,6 +3,7 @@ CONFIG_ARCH_ZYNQ=y
 CONFIG_TARGET_ZYNQ_PICOZED=y
 CONFIG_DEFAULT_DEVICE_TREE="zynq-picozed"
 CONFIG_SPL=y
+CONFIG_SPL_DM_SEQ_ALIAS=y
 # CONFIG_CMD_IMLS is not set
 # CONFIG_CMD_FLASH is not set
 CONFIG_CMD_GPIO=y
diff --git a/configs/zynq_zc702_defconfig b/configs/zynq_zc702_defconfig
index 5261b73..95d9774 100644
--- a/configs/zynq_zc702_defconfig
+++ b/configs/zynq_zc702_defconfig
@@ -2,6 +2,7 @@ CONFIG_ARM=y
 CONFIG_ARCH_ZYNQ=y
 CONFIG_DEFAULT_DEVICE_TREE="zynq-zc702"
 CONFIG_SPL=y
+CONFIG_SPL_DM_SEQ_ALIAS=y
 CONFIG_FIT=y
 CONFIG_FIT_VERBOSE=y
 CONFIG_FIT_SIGNATURE=y
diff --git a/configs/zynq_zc706_defconfig b/configs/zynq_zc706_defconfig
index 2e525b4..59166e0 100644
--- a/configs/zynq_zc706_defconfig
+++ b/configs/zynq_zc706_defconfig
@@ -3,6 +3,7 @@ CONFIG_ARCH_ZYNQ=y
 CONFIG_TARGET_ZYNQ_ZC706=y
 CONFIG_DEFAULT_DEVICE_TREE="zynq-zc706"
 CONFIG_SPL=y
+CONFIG_SPL_DM_SEQ_ALIAS=y
 CONFIG_FIT=y
 CONFIG_FIT_VERBOSE=y
 CONFIG_FIT_SIGNATURE=y
diff --git a/configs/zynq_zc770_xm010_defconfig b/configs/zynq_zc770_xm010_defconfig
index 6f2ad17..6bb79f7 100644
--- a/configs/zynq_zc770_xm010_defconfig
+++ b/configs/zynq_zc770_xm010_defconfig
@@ -3,6 +3,7 @@ CONFIG_ARCH_ZYNQ=y
 CONFIG_TARGET_ZYNQ_ZC770=y
 CONFIG_DEFAULT_DEVICE_TREE="zynq-zc770-xm010"
 CONFIG_SPL=y
+CONFIG_SPL_DM_SEQ_ALIAS=y
 CONFIG_FIT=y
 CONFIG_FIT_VERBOSE=y
 CONFIG_FIT_SIGNATURE=y
diff --git a/configs/zynq_zc770_xm011_defconfig b/configs/zynq_zc770_xm011_defconfig
index d20b3ed..241f1bf 100644
--- a/configs/zynq_zc770_xm011_defconfig
+++ b/configs/zynq_zc770_xm011_defconfig
@@ -3,6 +3,7 @@ CONFIG_ARCH_ZYNQ=y
 CONFIG_TARGET_ZYNQ_ZC770=y
 CONFIG_DEFAULT_DEVICE_TREE="zynq-zc770-xm011"
 CONFIG_SPL=y
+CONFIG_SPL_DM_SEQ_ALIAS=y
 CONFIG_FIT=y
 CONFIG_FIT_VERBOSE=y
 CONFIG_FIT_SIGNATURE=y
diff --git a/configs/zynq_zc770_xm012_defconfig b/configs/zynq_zc770_xm012_defconfig
index 4e963a4..e9a337c 100644
--- a/configs/zynq_zc770_xm012_defconfig
+++ b/configs/zynq_zc770_xm012_defconfig
@@ -3,6 +3,7 @@ CONFIG_ARCH_ZYNQ=y
 CONFIG_TARGET_ZYNQ_ZC770=y
 CONFIG_DEFAULT_DEVICE_TREE="zynq-zc770-xm012"
 CONFIG_SPL=y
+CONFIG_SPL_DM_SEQ_ALIAS=y
 CONFIG_FIT=y
 CONFIG_FIT_VERBOSE=y
 CONFIG_FIT_SIGNATURE=y
diff --git a/configs/zynq_zc770_xm013_defconfig b/configs/zynq_zc770_xm013_defconfig
index f2d8f14..e410f6d 100644
--- a/configs/zynq_zc770_xm013_defconfig
+++ b/configs/zynq_zc770_xm013_defconfig
@@ -3,6 +3,7 @@ CONFIG_ARCH_ZYNQ=y
 CONFIG_TARGET_ZYNQ_ZC770=y
 CONFIG_DEFAULT_DEVICE_TREE="zynq-zc770-xm013"
 CONFIG_SPL=y
+CONFIG_SPL_DM_SEQ_ALIAS=y
 CONFIG_FIT=y
 CONFIG_FIT_VERBOSE=y
 CONFIG_FIT_SIGNATURE=y
diff --git a/configs/zynq_zed_defconfig b/configs/zynq_zed_defconfig
index 2e7c68d..bc1051a 100644
--- a/configs/zynq_zed_defconfig
+++ b/configs/zynq_zed_defconfig
@@ -3,6 +3,7 @@ CONFIG_ARCH_ZYNQ=y
 CONFIG_TARGET_ZYNQ_ZED=y
 CONFIG_DEFAULT_DEVICE_TREE="zynq-zed"
 CONFIG_SPL=y
+CONFIG_SPL_DM_SEQ_ALIAS=y
 CONFIG_FIT=y
 CONFIG_FIT_VERBOSE=y
 CONFIG_FIT_SIGNATURE=y
diff --git a/configs/zynq_zybo_defconfig b/configs/zynq_zybo_defconfig
index d443e92..704f703 100644
--- a/configs/zynq_zybo_defconfig
+++ b/configs/zynq_zybo_defconfig
@@ -3,6 +3,7 @@ CONFIG_ARCH_ZYNQ=y
 CONFIG_TARGET_ZYNQ_ZYBO=y
 CONFIG_DEFAULT_DEVICE_TREE="zynq-zybo"
 CONFIG_SPL=y
+CONFIG_SPL_DM_SEQ_ALIAS=y
 CONFIG_FIT=y
 CONFIG_FIT_VERBOSE=y
 CONFIG_FIT_SIGNATURE=y
-- 
2.6.2

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

* [U-Boot] [PATCH 1/2] spl: dm: Add SPL_DM_SEQ_ALIAS config option
  2015-12-11 14:48 ` [U-Boot] [PATCH 1/2] spl: dm: Add SPL_DM_SEQ_ALIAS config option Nathan Rossi
@ 2015-12-11 15:07   ` Marek Vasut
  2015-12-11 15:46     ` Michal Simek
  2015-12-11 15:47   ` Simon Glass
  1 sibling, 1 reply; 14+ messages in thread
From: Marek Vasut @ 2015-12-11 15:07 UTC (permalink / raw)
  To: u-boot

On Friday, December 11, 2015 at 03:48:09 PM, Nathan Rossi wrote:
> The Device Model sequence alias feature is required by some Uclasses.
> Instead of disabling the feature for all SPL targets allow it to be
> configured.
> 
> The config option is disabled by default to reduce code size for targets
> that are not interested or do not require this feature.
> 
> Signed-off-by: Nathan Rossi <nathan@nathanrossi.com>
> Cc: Simon Glass <sjg@chromium.org>
> Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Cc: Marek Vasut <marex@denx.de>
> Cc: Michal Simek <michal.simek@xilinx.com>
> ---
> Based on a small amount of inspection for the Zynq platform, enabling
> this config option adds ~1KB of code size.
> 
> Also on a side note, this might affect the socfpga target as it forcibly
> overrides the #undef from config_uncmd_spl.h in its common header. I
> have Cc'd the respective maintainer for this reason.

The fix for SoCFPGA is easy -- enable the SPL_DM_SEQ_ALIAS in configs/socfpga*.
It is needed for booting from QSPI NOR.

> ---
>  drivers/core/Kconfig       | 9 +++++++++
>  drivers/core/device.c      | 2 +-
>  include/config_uncmd_spl.h | 1 -
>  3 files changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/core/Kconfig b/drivers/core/Kconfig
> index 53acee0..97aa01e 100644
> --- a/drivers/core/Kconfig
> +++ b/drivers/core/Kconfig
> @@ -60,6 +60,15 @@ config DM_SEQ_ALIAS
>  	help
>  	  Most boards will have a '/aliases' node containing the path to
>  	  numbered devices (e.g. serial0 = &serial0). This feature can be
> +	  disabled if it is not required.
> +
> +config SPL_DM_SEQ_ALIAS
> +	bool "Support numbered aliases in device tree in SPL"
> +	depends on DM
> +	default n
> +	help
> +	  Most boards will have a '/aliases' node containing the path to
> +	  numbered devices (e.g. serial0 = &serial0). This feature can be
>  	  disabled if it is not required, to save code space in SPL.
> 
>  config REGMAP
> diff --git a/drivers/core/device.c b/drivers/core/device.c
> index 758f390..b237b88 100644
> --- a/drivers/core/device.c
> +++ b/drivers/core/device.c
> @@ -64,7 +64,7 @@ int device_bind(struct udevice *parent, const struct
> driver *drv,
> 
>  	dev->seq = -1;
>  	dev->req_seq = -1;
> -	if (CONFIG_IS_ENABLED(OF_CONTROL) && IS_ENABLED(CONFIG_DM_SEQ_ALIAS)) {
> +	if (CONFIG_IS_ENABLED(OF_CONTROL) && CONFIG_IS_ENABLED(DM_SEQ_ALIAS)) {
>  		/*
>  		* Some devices, such as a SPI bus, I2C bus and serial ports
>  		* are numbered using aliases.
> diff --git a/include/config_uncmd_spl.h b/include/config_uncmd_spl.h
> index 6e299f6..3b198ae 100644
> --- a/include/config_uncmd_spl.h
> +++ b/include/config_uncmd_spl.h
> @@ -29,7 +29,6 @@
>  #endif
> 
>  #undef CONFIG_DM_WARN
> -#undef CONFIG_DM_SEQ_ALIAS
>  #undef CONFIG_DM_STDIO
> 
>  #endif /* CONFIG_SPL_BUILD */

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

* [U-Boot] [PATCH 1/2] spl: dm: Add SPL_DM_SEQ_ALIAS config option
  2015-12-11 15:07   ` Marek Vasut
@ 2015-12-11 15:46     ` Michal Simek
  2015-12-11 17:32       ` Marek Vasut
  0 siblings, 1 reply; 14+ messages in thread
From: Michal Simek @ 2015-12-11 15:46 UTC (permalink / raw)
  To: u-boot

On 11.12.2015 16:07, Marek Vasut wrote:
> On Friday, December 11, 2015 at 03:48:09 PM, Nathan Rossi wrote:
>> The Device Model sequence alias feature is required by some Uclasses.
>> Instead of disabling the feature for all SPL targets allow it to be
>> configured.
>>
>> The config option is disabled by default to reduce code size for targets
>> that are not interested or do not require this feature.
>>
>> Signed-off-by: Nathan Rossi <nathan@nathanrossi.com>
>> Cc: Simon Glass <sjg@chromium.org>
>> Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
>> Cc: Linus Walleij <linus.walleij@linaro.org>
>> Cc: Marek Vasut <marex@denx.de>
>> Cc: Michal Simek <michal.simek@xilinx.com>
>> ---
>> Based on a small amount of inspection for the Zynq platform, enabling
>> this config option adds ~1KB of code size.
>>
>> Also on a side note, this might affect the socfpga target as it forcibly
>> overrides the #undef from config_uncmd_spl.h in its common header. I
>> have Cc'd the respective maintainer for this reason.
> 
> The fix for SoCFPGA is easy -- enable the SPL_DM_SEQ_ALIAS in configs/socfpga*.
> It is needed for booting from QSPI NOR.

That's probably not the best solution. But of course we can use it.
IRC Stefan had the same problem.

Thanks,
Michal

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

* [U-Boot] [PATCH 1/2] spl: dm: Add SPL_DM_SEQ_ALIAS config option
  2015-12-11 14:48 ` [U-Boot] [PATCH 1/2] spl: dm: Add SPL_DM_SEQ_ALIAS config option Nathan Rossi
  2015-12-11 15:07   ` Marek Vasut
@ 2015-12-11 15:47   ` Simon Glass
  1 sibling, 0 replies; 14+ messages in thread
From: Simon Glass @ 2015-12-11 15:47 UTC (permalink / raw)
  To: u-boot

On 11 December 2015 at 07:48, Nathan Rossi <nathan@nathanrossi.com> wrote:
>
> The Device Model sequence alias feature is required by some Uclasses.
> Instead of disabling the feature for all SPL targets allow it to be
> configured.
>
> The config option is disabled by default to reduce code size for targets
> that are not interested or do not require this feature.
>
> Signed-off-by: Nathan Rossi <nathan@nathanrossi.com>
> Cc: Simon Glass <sjg@chromium.org>
> Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Cc: Marek Vasut <marex@denx.de>
> Cc: Michal Simek <michal.simek@xilinx.com>
> ---
> Based on a small amount of inspection for the Zynq platform, enabling
> this config option adds ~1KB of code size.
>
> Also on a side note, this might affect the socfpga target as it forcibly
> overrides the #undef from config_uncmd_spl.h in its common header. I
> have Cc'd the respective maintainer for this reason.
> ---
>  drivers/core/Kconfig       | 9 +++++++++
>  drivers/core/device.c      | 2 +-
>  include/config_uncmd_spl.h | 1 -
>  3 files changed, 10 insertions(+), 2 deletions(-)

Acked-by: Simon Glass <sjg@chromium.org>

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

* [U-Boot] [PATCH 1/2] spl: dm: Add SPL_DM_SEQ_ALIAS config option
  2015-12-11 15:46     ` Michal Simek
@ 2015-12-11 17:32       ` Marek Vasut
  2015-12-12 12:05         ` Stefan Roese
  0 siblings, 1 reply; 14+ messages in thread
From: Marek Vasut @ 2015-12-11 17:32 UTC (permalink / raw)
  To: u-boot

On Friday, December 11, 2015 at 04:46:40 PM, Michal Simek wrote:
> On 11.12.2015 16:07, Marek Vasut wrote:
> > On Friday, December 11, 2015 at 03:48:09 PM, Nathan Rossi wrote:
> >> The Device Model sequence alias feature is required by some Uclasses.
> >> Instead of disabling the feature for all SPL targets allow it to be
> >> configured.
> >> 
> >> The config option is disabled by default to reduce code size for targets
> >> that are not interested or do not require this feature.
> >> 
> >> Signed-off-by: Nathan Rossi <nathan@nathanrossi.com>
> >> Cc: Simon Glass <sjg@chromium.org>
> >> Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
> >> Cc: Linus Walleij <linus.walleij@linaro.org>
> >> Cc: Marek Vasut <marex@denx.de>
> >> Cc: Michal Simek <michal.simek@xilinx.com>
> >> ---
> >> Based on a small amount of inspection for the Zynq platform, enabling
> >> this config option adds ~1KB of code size.
> >> 
> >> Also on a side note, this might affect the socfpga target as it forcibly
> >> overrides the #undef from config_uncmd_spl.h in its common header. I
> >> have Cc'd the respective maintainer for this reason.
> > 
> > The fix for SoCFPGA is easy -- enable the SPL_DM_SEQ_ALIAS in
> > configs/socfpga*. It is needed for booting from QSPI NOR.
> 
> That's probably not the best solution. But of course we can use it.
> IRC Stefan had the same problem.

So what is the solution ?

Best regards,
Marek Vasut

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

* [U-Boot] [PATCH 1/2] spl: dm: Add SPL_DM_SEQ_ALIAS config option
  2015-12-11 17:32       ` Marek Vasut
@ 2015-12-12 12:05         ` Stefan Roese
  2015-12-12 14:08           ` Nathan Rossi
  0 siblings, 1 reply; 14+ messages in thread
From: Stefan Roese @ 2015-12-12 12:05 UTC (permalink / raw)
  To: u-boot

On 11.12.2015 18:32, Marek Vasut wrote:
> On Friday, December 11, 2015 at 04:46:40 PM, Michal Simek wrote:
>> On 11.12.2015 16:07, Marek Vasut wrote:
>>> On Friday, December 11, 2015 at 03:48:09 PM, Nathan Rossi wrote:
>>>> The Device Model sequence alias feature is required by some Uclasses.
>>>> Instead of disabling the feature for all SPL targets allow it to be
>>>> configured.
>>>>
>>>> The config option is disabled by default to reduce code size for targets
>>>> that are not interested or do not require this feature.
>>>>
>>>> Signed-off-by: Nathan Rossi <nathan@nathanrossi.com>
>>>> Cc: Simon Glass <sjg@chromium.org>
>>>> Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
>>>> Cc: Linus Walleij <linus.walleij@linaro.org>
>>>> Cc: Marek Vasut <marex@denx.de>
>>>> Cc: Michal Simek <michal.simek@xilinx.com>
>>>> ---
>>>> Based on a small amount of inspection for the Zynq platform, enabling
>>>> this config option adds ~1KB of code size.
>>>>
>>>> Also on a side note, this might affect the socfpga target as it forcibly
>>>> overrides the #undef from config_uncmd_spl.h in its common header. I
>>>> have Cc'd the respective maintainer for this reason.
>>>
>>> The fix for SoCFPGA is easy -- enable the SPL_DM_SEQ_ALIAS in
>>> configs/socfpga*. It is needed for booting from QSPI NOR.
>>
>> That's probably not the best solution. But of course we can use it.
>> IRC Stefan had the same problem.
>
> So what is the solution ?

I added

#define CONFIG_DM_SEQ_ALIAS            1

to the common config header for MVEBU. If its possible to set this
via Kconfig in a way where its not #undef'ed by config_uncmd_spl.h,
that would be even better.

Thanks,
Stefan

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

* [U-Boot] [PATCH 1/2] spl: dm: Add SPL_DM_SEQ_ALIAS config option
  2015-12-12 12:05         ` Stefan Roese
@ 2015-12-12 14:08           ` Nathan Rossi
  2015-12-12 15:38             ` Marek Vasut
  2015-12-14  6:26             ` Stefan Roese
  0 siblings, 2 replies; 14+ messages in thread
From: Nathan Rossi @ 2015-12-12 14:08 UTC (permalink / raw)
  To: u-boot

On Sat, Dec 12, 2015 at 10:05 PM, Stefan Roese <sr@denx.de> wrote:
> On 11.12.2015 18:32, Marek Vasut wrote:
>>
>> On Friday, December 11, 2015 at 04:46:40 PM, Michal Simek wrote:
>>>
>>> On 11.12.2015 16:07, Marek Vasut wrote:
>>>>
>>>> On Friday, December 11, 2015 at 03:48:09 PM, Nathan Rossi wrote:
>>>>>
>>>>> The Device Model sequence alias feature is required by some Uclasses.
>>>>> Instead of disabling the feature for all SPL targets allow it to be
>>>>> configured.
>>>>>
>>>>> The config option is disabled by default to reduce code size for
>>>>> targets
>>>>> that are not interested or do not require this feature.
>>>>>
>>>>> Signed-off-by: Nathan Rossi <nathan@nathanrossi.com>
>>>>> Cc: Simon Glass <sjg@chromium.org>
>>>>> Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
>>>>> Cc: Linus Walleij <linus.walleij@linaro.org>
>>>>> Cc: Marek Vasut <marex@denx.de>
>>>>> Cc: Michal Simek <michal.simek@xilinx.com>
>>>>> ---
>>>>> Based on a small amount of inspection for the Zynq platform, enabling
>>>>> this config option adds ~1KB of code size.
>>>>>
>>>>> Also on a side note, this might affect the socfpga target as it
>>>>> forcibly
>>>>> overrides the #undef from config_uncmd_spl.h in its common header. I
>>>>> have Cc'd the respective maintainer for this reason.
>>>>
>>>>
>>>> The fix for SoCFPGA is easy -- enable the SPL_DM_SEQ_ALIAS in
>>>> configs/socfpga*. It is needed for booting from QSPI NOR.

I will add a patch to this series, depending on how it is agreed to
enable this option.

>>>
>>>
>>> That's probably not the best solution. But of course we can use it.
>>> IRC Stefan had the same problem.

So would a better solution be to force the ARCH_ZYNQ config (and
others ARCH_SOCFPGA/ARCH_MVEBU) to 'select SPL_DM_SEQ_ALIAS' instead
of enabling them separately for each defconfig? like is done for
SPL_OF_CONTROL/etc.

>>
>>
>> So what is the solution ?
>
>
> I added
>
> #define CONFIG_DM_SEQ_ALIAS            1
>
> to the common config header for MVEBU. If its possible to set this
> via Kconfig in a way where its not #undef'ed by config_uncmd_spl.h,
> that would be even better.

As with socfpga, would you like me to do the same for MVEBU, add a
patch to this series to enable it for MVEBU?

Regards,
Nathan

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

* [U-Boot] [PATCH 1/2] spl: dm: Add SPL_DM_SEQ_ALIAS config option
  2015-12-12 14:08           ` Nathan Rossi
@ 2015-12-12 15:38             ` Marek Vasut
  2015-12-14  6:26             ` Stefan Roese
  1 sibling, 0 replies; 14+ messages in thread
From: Marek Vasut @ 2015-12-12 15:38 UTC (permalink / raw)
  To: u-boot

On Saturday, December 12, 2015 at 03:08:21 PM, Nathan Rossi wrote:
> On Sat, Dec 12, 2015 at 10:05 PM, Stefan Roese <sr@denx.de> wrote:
> > On 11.12.2015 18:32, Marek Vasut wrote:
> >> On Friday, December 11, 2015 at 04:46:40 PM, Michal Simek wrote:
> >>> On 11.12.2015 16:07, Marek Vasut wrote:
> >>>> On Friday, December 11, 2015 at 03:48:09 PM, Nathan Rossi wrote:
> >>>>> The Device Model sequence alias feature is required by some Uclasses.
> >>>>> Instead of disabling the feature for all SPL targets allow it to be
> >>>>> configured.
> >>>>> 
> >>>>> The config option is disabled by default to reduce code size for
> >>>>> targets
> >>>>> that are not interested or do not require this feature.
> >>>>> 
> >>>>> Signed-off-by: Nathan Rossi <nathan@nathanrossi.com>
> >>>>> Cc: Simon Glass <sjg@chromium.org>
> >>>>> Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
> >>>>> Cc: Linus Walleij <linus.walleij@linaro.org>
> >>>>> Cc: Marek Vasut <marex@denx.de>
> >>>>> Cc: Michal Simek <michal.simek@xilinx.com>
> >>>>> ---
> >>>>> Based on a small amount of inspection for the Zynq platform, enabling
> >>>>> this config option adds ~1KB of code size.
> >>>>> 
> >>>>> Also on a side note, this might affect the socfpga target as it
> >>>>> forcibly
> >>>>> overrides the #undef from config_uncmd_spl.h in its common header. I
> >>>>> have Cc'd the respective maintainer for this reason.
> >>>> 
> >>>> The fix for SoCFPGA is easy -- enable the SPL_DM_SEQ_ALIAS in
> >>>> configs/socfpga*. It is needed for booting from QSPI NOR.
> 
> I will add a patch to this series, depending on how it is agreed to
> enable this option.
> 
> >>> That's probably not the best solution. But of course we can use it.
> >>> IRC Stefan had the same problem.
> 
> So would a better solution be to force the ARCH_ZYNQ config (and
> others ARCH_SOCFPGA/ARCH_MVEBU) to 'select SPL_DM_SEQ_ALIAS' instead
> of enabling them separately for each defconfig? like is done for
> SPL_OF_CONTROL/etc.

That might make sense, yes.

But, I wonder if we can do without DM_SEQ_ALIAS altogether for the SPI NOR boot?

> >> So what is the solution ?
> > 
> > I added
> > 
> > #define CONFIG_DM_SEQ_ALIAS            1
> > 
> > to the common config header for MVEBU. If its possible to set this
> > via Kconfig in a way where its not #undef'ed by config_uncmd_spl.h,
> > that would be even better.
> 
> As with socfpga, would you like me to do the same for MVEBU, add a
> patch to this series to enable it for MVEBU?
> 
> Regards,
> Nathan

Best regards,
Marek Vasut

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

* [U-Boot] [PATCH 1/2] spl: dm: Add SPL_DM_SEQ_ALIAS config option
  2015-12-12 14:08           ` Nathan Rossi
  2015-12-12 15:38             ` Marek Vasut
@ 2015-12-14  6:26             ` Stefan Roese
  2015-12-14  7:29               ` Michal Simek
  1 sibling, 1 reply; 14+ messages in thread
From: Stefan Roese @ 2015-12-14  6:26 UTC (permalink / raw)
  To: u-boot

On 12.12.2015 15:08, Nathan Rossi wrote:
> On Sat, Dec 12, 2015 at 10:05 PM, Stefan Roese <sr@denx.de> wrote:
>> On 11.12.2015 18:32, Marek Vasut wrote:
>>>
>>> On Friday, December 11, 2015 at 04:46:40 PM, Michal Simek wrote:
>>>>
>>>> On 11.12.2015 16:07, Marek Vasut wrote:
>>>>>
>>>>> On Friday, December 11, 2015 at 03:48:09 PM, Nathan Rossi wrote:
>>>>>>
>>>>>> The Device Model sequence alias feature is required by some Uclasses.
>>>>>> Instead of disabling the feature for all SPL targets allow it to be
>>>>>> configured.
>>>>>>
>>>>>> The config option is disabled by default to reduce code size for
>>>>>> targets
>>>>>> that are not interested or do not require this feature.
>>>>>>
>>>>>> Signed-off-by: Nathan Rossi <nathan@nathanrossi.com>
>>>>>> Cc: Simon Glass <sjg@chromium.org>
>>>>>> Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
>>>>>> Cc: Linus Walleij <linus.walleij@linaro.org>
>>>>>> Cc: Marek Vasut <marex@denx.de>
>>>>>> Cc: Michal Simek <michal.simek@xilinx.com>
>>>>>> ---
>>>>>> Based on a small amount of inspection for the Zynq platform, enabling
>>>>>> this config option adds ~1KB of code size.
>>>>>>
>>>>>> Also on a side note, this might affect the socfpga target as it
>>>>>> forcibly
>>>>>> overrides the #undef from config_uncmd_spl.h in its common header. I
>>>>>> have Cc'd the respective maintainer for this reason.
>>>>>
>>>>>
>>>>> The fix for SoCFPGA is easy -- enable the SPL_DM_SEQ_ALIAS in
>>>>> configs/socfpga*. It is needed for booting from QSPI NOR.
>
> I will add a patch to this series, depending on how it is agreed to
> enable this option.
>
>>>>
>>>>
>>>> That's probably not the best solution. But of course we can use it.
>>>> IRC Stefan had the same problem.
>
> So would a better solution be to force the ARCH_ZYNQ config (and
> others ARCH_SOCFPGA/ARCH_MVEBU) to 'select SPL_DM_SEQ_ALIAS' instead
> of enabling them separately for each defconfig? like is done for
> SPL_OF_CONTROL/etc.

Yes, please. Autoselecting this option is definitely better.

>>>
>>>
>>> So what is the solution ?
>>
>>
>> I added
>>
>> #define CONFIG_DM_SEQ_ALIAS            1
>>
>> to the common config header for MVEBU. If its possible to set this
>> via Kconfig in a way where its not #undef'ed by config_uncmd_spl.h,
>> that would be even better.
>
> As with socfpga, would you like me to do the same for MVEBU, add a
> patch to this series to enable it for MVEBU?

Yes, please do so.

Thanks,
Stefan

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

* [U-Boot] [PATCH 1/2] spl: dm: Add SPL_DM_SEQ_ALIAS config option
  2015-12-14  6:26             ` Stefan Roese
@ 2015-12-14  7:29               ` Michal Simek
  2015-12-30  5:52                 ` Nathan Rossi
  0 siblings, 1 reply; 14+ messages in thread
From: Michal Simek @ 2015-12-14  7:29 UTC (permalink / raw)
  To: u-boot

On 14.12.2015 07:26, Stefan Roese wrote:
> On 12.12.2015 15:08, Nathan Rossi wrote:
>> On Sat, Dec 12, 2015 at 10:05 PM, Stefan Roese <sr@denx.de> wrote:
>>> On 11.12.2015 18:32, Marek Vasut wrote:
>>>>
>>>> On Friday, December 11, 2015 at 04:46:40 PM, Michal Simek wrote:
>>>>>
>>>>> On 11.12.2015 16:07, Marek Vasut wrote:
>>>>>>
>>>>>> On Friday, December 11, 2015 at 03:48:09 PM, Nathan Rossi wrote:
>>>>>>>
>>>>>>> The Device Model sequence alias feature is required by some
>>>>>>> Uclasses.
>>>>>>> Instead of disabling the feature for all SPL targets allow it to be
>>>>>>> configured.
>>>>>>>
>>>>>>> The config option is disabled by default to reduce code size for
>>>>>>> targets
>>>>>>> that are not interested or do not require this feature.
>>>>>>>
>>>>>>> Signed-off-by: Nathan Rossi <nathan@nathanrossi.com>
>>>>>>> Cc: Simon Glass <sjg@chromium.org>
>>>>>>> Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
>>>>>>> Cc: Linus Walleij <linus.walleij@linaro.org>
>>>>>>> Cc: Marek Vasut <marex@denx.de>
>>>>>>> Cc: Michal Simek <michal.simek@xilinx.com>
>>>>>>> ---
>>>>>>> Based on a small amount of inspection for the Zynq platform,
>>>>>>> enabling
>>>>>>> this config option adds ~1KB of code size.
>>>>>>>
>>>>>>> Also on a side note, this might affect the socfpga target as it
>>>>>>> forcibly
>>>>>>> overrides the #undef from config_uncmd_spl.h in its common header. I
>>>>>>> have Cc'd the respective maintainer for this reason.
>>>>>>
>>>>>>
>>>>>> The fix for SoCFPGA is easy -- enable the SPL_DM_SEQ_ALIAS in
>>>>>> configs/socfpga*. It is needed for booting from QSPI NOR.
>>
>> I will add a patch to this series, depending on how it is agreed to
>> enable this option.
>>
>>>>>
>>>>>
>>>>> That's probably not the best solution. But of course we can use it.
>>>>> IRC Stefan had the same problem.
>>
>> So would a better solution be to force the ARCH_ZYNQ config (and
>> others ARCH_SOCFPGA/ARCH_MVEBU) to 'select SPL_DM_SEQ_ALIAS' instead
>> of enabling them separately for each defconfig? like is done for
>> SPL_OF_CONTROL/etc.
> 
> Yes, please. Autoselecting this option is definitely better.

TBH I don't think this is the best solution. We should select things
which are required all the time. For zynq case this selection is
necessary just for qspi boot mode. There are other boot modes which
doesn't require it that's why for these mods this is just additional code.
If you think that this is need/good to select by default for your SoC
then this can be done separately.

Note: Agree with Marek that will be good to check where exactly the
problem is that this config option is necessary to enable even for case
where only on qspi is in the system.

Acked-by: Michal Simek <michal.simek@xilinx.com>

Thanks,
Michal

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

* [U-Boot] [PATCH 1/2] spl: dm: Add SPL_DM_SEQ_ALIAS config option
  2015-12-14  7:29               ` Michal Simek
@ 2015-12-30  5:52                 ` Nathan Rossi
  2015-12-30  9:02                   ` Marek Vasut
  0 siblings, 1 reply; 14+ messages in thread
From: Nathan Rossi @ 2015-12-30  5:52 UTC (permalink / raw)
  To: u-boot

On Mon, Dec 14, 2015 at 5:29 PM, Michal Simek <michal.simek@xilinx.com> wrote:
> On 14.12.2015 07:26, Stefan Roese wrote:
>> On 12.12.2015 15:08, Nathan Rossi wrote:
>>> On Sat, Dec 12, 2015 at 10:05 PM, Stefan Roese <sr@denx.de> wrote:
>>>> On 11.12.2015 18:32, Marek Vasut wrote:
>>>>>
>>>>> On Friday, December 11, 2015 at 04:46:40 PM, Michal Simek wrote:
>>>>>>
>>>>>> On 11.12.2015 16:07, Marek Vasut wrote:
>>>>>>>
>>>>>>> On Friday, December 11, 2015 at 03:48:09 PM, Nathan Rossi wrote:
>>>>>>>>
>>>>>>>> The Device Model sequence alias feature is required by some
>>>>>>>> Uclasses.
>>>>>>>> Instead of disabling the feature for all SPL targets allow it to be
>>>>>>>> configured.
>>>>>>>>
>>>>>>>> The config option is disabled by default to reduce code size for
>>>>>>>> targets
>>>>>>>> that are not interested or do not require this feature.
>>>>>>>>
>>>>>>>> Signed-off-by: Nathan Rossi <nathan@nathanrossi.com>
>>>>>>>> Cc: Simon Glass <sjg@chromium.org>
>>>>>>>> Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
>>>>>>>> Cc: Linus Walleij <linus.walleij@linaro.org>
>>>>>>>> Cc: Marek Vasut <marex@denx.de>
>>>>>>>> Cc: Michal Simek <michal.simek@xilinx.com>
>>>>>>>> ---
>>>>>>>> Based on a small amount of inspection for the Zynq platform,
>>>>>>>> enabling
>>>>>>>> this config option adds ~1KB of code size.
>>>>>>>>
>>>>>>>> Also on a side note, this might affect the socfpga target as it
>>>>>>>> forcibly
>>>>>>>> overrides the #undef from config_uncmd_spl.h in its common header. I
>>>>>>>> have Cc'd the respective maintainer for this reason.
>>>>>>>
>>>>>>>
>>>>>>> The fix for SoCFPGA is easy -- enable the SPL_DM_SEQ_ALIAS in
>>>>>>> configs/socfpga*. It is needed for booting from QSPI NOR.
>>>
>>> I will add a patch to this series, depending on how it is agreed to
>>> enable this option.
>>>
>>>>>>
>>>>>>
>>>>>> That's probably not the best solution. But of course we can use it.
>>>>>> IRC Stefan had the same problem.
>>>
>>> So would a better solution be to force the ARCH_ZYNQ config (and
>>> others ARCH_SOCFPGA/ARCH_MVEBU) to 'select SPL_DM_SEQ_ALIAS' instead
>>> of enabling them separately for each defconfig? like is done for
>>> SPL_OF_CONTROL/etc.
>>
>> Yes, please. Autoselecting this option is definitely better.
>
> TBH I don't think this is the best solution. We should select things
> which are required all the time. For zynq case this selection is
> necessary just for qspi boot mode. There are other boot modes which
> doesn't require it that's why for these mods this is just additional code.
> If you think that this is need/good to select by default for your SoC
> then this can be done separately.

Sorry for the slow response/follow up on this.

So as I understand it makes sense for this to be configured slightly
differently depending on when the boot options that need this are
available. I guess this also makes sense as SPI is not the only user
of the seq alias feature.

I am sorting out v2 of this series for the additional patches to add
the config for socfpga and mvebu, just want to clarify that this would
be the preferred way (given recent comments) to enable it for the
corresponding platforms:
 * socfpga -> enable in each defconfig
 * mvebu -> enable in arch/arm/Kconfig for ARCH_MVEBU (aka auto selecting)

>
> Note: Agree with Marek that will be good to check where exactly the
> problem is that this config option is necessary to enable even for case
> where only on qspi is in the system.

I had a bit of a look into this, it would require changes to allow for
it to fall-back to finding and using the first device which is matched
for the uclass (instead of relying on just CONFIG_SF_DEFAULT_BUS), it
would probably be an intrusive change to sf-uclass. A simpler solution
might be to just default seq numbers to 0 when seq alias is not
enabled? would allow for the change to work with other uclasses that
use seq alias.

Regards,
Nathan

>
> Acked-by: Michal Simek <michal.simek@xilinx.com>
>
> Thanks,
> Michal

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

* [U-Boot] [PATCH 1/2] spl: dm: Add SPL_DM_SEQ_ALIAS config option
  2015-12-30  5:52                 ` Nathan Rossi
@ 2015-12-30  9:02                   ` Marek Vasut
  0 siblings, 0 replies; 14+ messages in thread
From: Marek Vasut @ 2015-12-30  9:02 UTC (permalink / raw)
  To: u-boot

On Wednesday, December 30, 2015 at 06:52:24 AM, Nathan Rossi wrote:
[...]
> > TBH I don't think this is the best solution. We should select things
> > which are required all the time. For zynq case this selection is
> > necessary just for qspi boot mode. There are other boot modes which
> > doesn't require it that's why for these mods this is just additional
> > code. If you think that this is need/good to select by default for your
> > SoC then this can be done separately.
> 
> Sorry for the slow response/follow up on this.
> 
> So as I understand it makes sense for this to be configured slightly
> differently depending on when the boot options that need this are
> available. I guess this also makes sense as SPI is not the only user
> of the seq alias feature.
> 
> I am sorting out v2 of this series for the additional patches to add
> the config for socfpga and mvebu, just want to clarify that this would
> be the preferred way (given recent comments) to enable it for the
> corresponding platforms:
>  * socfpga -> enable in each defconfig

If the feature is imperative for the platform to work, just enable it in
Kconfig too, just like on mvebu.

>  * mvebu -> enable in arch/arm/Kconfig for ARCH_MVEBU (aka auto selecting)
> 
> > Note: Agree with Marek that will be good to check where exactly the
> > problem is that this config option is necessary to enable even for case
> > where only on qspi is in the system.
> 
> I had a bit of a look into this, it would require changes to allow for
> it to fall-back to finding and using the first device which is matched
> for the uclass (instead of relying on just CONFIG_SF_DEFAULT_BUS), it
> would probably be an intrusive change to sf-uclass. A simpler solution
> might be to just default seq numbers to 0 when seq alias is not
> enabled? would allow for the change to work with other uclasses that
> use seq alias.
> 
> Regards,
> Nathan
> 
> > Acked-by: Michal Simek <michal.simek@xilinx.com>
> > 
> > Thanks,
> > Michal

Best regards,
Marek Vasut

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

end of thread, other threads:[~2015-12-30  9:02 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-12-11 14:48 [U-Boot] [PATCH 0/2] Change DM_SEQ_ALIAS to be configurable for SPL Nathan Rossi
2015-12-11 14:48 ` [U-Boot] [PATCH 1/2] spl: dm: Add SPL_DM_SEQ_ALIAS config option Nathan Rossi
2015-12-11 15:07   ` Marek Vasut
2015-12-11 15:46     ` Michal Simek
2015-12-11 17:32       ` Marek Vasut
2015-12-12 12:05         ` Stefan Roese
2015-12-12 14:08           ` Nathan Rossi
2015-12-12 15:38             ` Marek Vasut
2015-12-14  6:26             ` Stefan Roese
2015-12-14  7:29               ` Michal Simek
2015-12-30  5:52                 ` Nathan Rossi
2015-12-30  9:02                   ` Marek Vasut
2015-12-11 15:47   ` Simon Glass
2015-12-11 14:48 ` [U-Boot] [PATCH 2/2] arm: zynq: Enable SPL_DM_SEQ_ALIAS for all Zynq configs Nathan Rossi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox