[U-Boot] [PATCH] dm: core: Change platform specific translation-offset handling

[U-Boot] [PATCH] dm: core: Change platform specific translation-offset handling

[Stefan Roese]

Testing has shown that the current DM implementation of a platform /
board specific translation offset, as its needed for the SPL on MVEBU
platforms is buggy. The translation offset is confingured too late,
after the driver bind functions are run. This may result in incorrect
address translations. With the current implementation its not possible
to configure the offset earlier, as the DM code has not run at all.

This patch now removed the set_/get_translation_offset() calls and
moves the translation offset into the GD variable translation_offset.
This variable will get used when CONFIG_TRANSLATION_OFFSET is enabled.
This option is enabled only for MVEBU on ARM32 platforms, where its
currenty needed and configured in the SPL.

Signed-off-by: Stefan Roese <sr@denx.de>
Cc: Pierre Bourdon <delroth@gmail.com>
Cc: Baruch Siach <baruch@tkos.co.il>
Cc: Simon Glass <sjg@chromium.org>
Cc: Heiko Schocher <hs@denx.de>
Cc: Tom Rini <trini@konsulko.com>
---
 arch/arm/mach-mvebu/Kconfig       |  1 +
 arch/arm/mach-mvebu/spl.c         | 12 +++++++++---
 drivers/core/Kconfig              |  9 +++++++++
 drivers/core/fdtaddr.c            |  9 ++++++---
 drivers/core/root.c               | 21 ---------------------
 include/asm-generic/global_data.h |  4 ++++
 include/dm/fdtaddr.h              | 21 ---------------------
 7 files changed, 29 insertions(+), 48 deletions(-)

diff --git a/arch/arm/mach-mvebu/Kconfig b/arch/arm/mach-mvebu/Kconfig
index 7dda04e0e3..6d3429b6b1 100644
--- a/arch/arm/mach-mvebu/Kconfig
+++ b/arch/arm/mach-mvebu/Kconfig
@@ -14,6 +14,7 @@ config ARMADA_32BIT
 	select SPL_OF_CONTROL
 	select SPL_SIMPLE_BUS
 	select SUPPORT_SPL
+	select TRANSLATION_OFFSET
 
 config ARMADA_64BIT
 	bool
diff --git a/arch/arm/mach-mvebu/spl.c b/arch/arm/mach-mvebu/spl.c
index 9dd7c84b68..530b98c1aa 100644
--- a/arch/arm/mach-mvebu/spl.c
+++ b/arch/arm/mach-mvebu/spl.c
@@ -93,15 +93,21 @@ void board_init_f(ulong dummy)
 	 */
 #endif
 
+	/*
+	 * Use special translation offset for SPL. This needs to be
+	 * configured *before* spl_init() is called as this function
+	 * calls dm_init() which calls the bind functions of the
+	 * device drivers. Here the base address needs to be configured
+	 * (translated) correctly.
+	 */
+	gd->translation_offset = 0xd0000000 - 0xf1000000;
+
 	ret = spl_init();
 	if (ret) {
 		debug("spl_init() failed: %d\n", ret);
 		hang();
 	}
 
-	/* Use special translation offset for SPL */
-	dm_set_translation_offset(0xd0000000 - 0xf1000000);
-
 	preloader_console_init();
 
 	timer_init();
diff --git a/drivers/core/Kconfig b/drivers/core/Kconfig
index ddf2fb3fb8..2d195ae35e 100644
--- a/drivers/core/Kconfig
+++ b/drivers/core/Kconfig
@@ -225,6 +225,15 @@ config SPL_OF_TRANSLATE
 	  used for the address translation. This function is faster and
 	  smaller in size than fdt_translate_address().
 
+config TRANSLATION_OFFSET
+	bool "Platforms specific translation offset"
+	depends on DM && OF_CONTROL
+	help
+	  Some platforms need a special address translation. Those
+	  platforms (e.g. mvebu in SPL) can configure a translation
+	  offset by enabling this option and setting the translation_offset
+	  variable in the GD in their platform- / board-specific code.
+
 config OF_ISA_BUS
 	bool
 	depends on OF_TRANSLATE
diff --git a/drivers/core/fdtaddr.c b/drivers/core/fdtaddr.c
index e113f1dd39..c2873861da 100644
--- a/drivers/core/fdtaddr.c
+++ b/drivers/core/fdtaddr.c
@@ -74,13 +74,16 @@ fdt_addr_t devfdt_get_addr_index(struct udevice *dev, int index)
 		}
 	}
 
+#if defined(CONFIG_TRANSLATION_OFFSET)
 	/*
 	 * Some platforms need a special address translation. Those
 	 * platforms (e.g. mvebu in SPL) can configure a translation
-	 * offset in the DM by calling dm_set_translation_offset() that
-	 * will get added to all addresses returned by devfdt_get_addr().
+	 * offset by setting this value in the GD and enaling this
+	 * feature via CONFIG_TRANSLATION_OFFSET. This value will
+	 * get added to all addresses returned by devfdt_get_addr().
 	 */
-	addr += dm_get_translation_offset();
+	addr += gd->translation_offset;
+#endif
 
 	return addr;
 #else
diff --git a/drivers/core/root.c b/drivers/core/root.c
index e6ec7faf37..8fa096648e 100644
--- a/drivers/core/root.c
+++ b/drivers/core/root.c
@@ -25,10 +25,6 @@
 
 DECLARE_GLOBAL_DATA_PTR;
 
-struct root_priv {
-	fdt_addr_t translation_offset;	/* optional translation offset */
-};
-
 static const struct driver_info root_info = {
 	.name		= "root_driver",
 };
@@ -52,22 +48,6 @@ void dm_fixup_for_gd_move(struct global_data *new_gd)
 	}
 }
 
-fdt_addr_t dm_get_translation_offset(void)
-{
-	struct udevice *root = dm_root();
-	struct root_priv *priv = dev_get_priv(root);
-
-	return priv->translation_offset;
-}
-
-void dm_set_translation_offset(fdt_addr_t offs)
-{
-	struct udevice *root = dm_root();
-	struct root_priv *priv = dev_get_priv(root);
-
-	priv->translation_offset = offs;
-}
-
 #if defined(CONFIG_NEEDS_MANUAL_RELOC)
 void fix_drivers(void)
 {
@@ -420,7 +400,6 @@ int dm_init_and_scan(bool pre_reloc_only)
 U_BOOT_DRIVER(root_driver) = {
 	.name	= "root_driver",
 	.id	= UCLASS_ROOT,
-	.priv_auto_alloc_size = sizeof(struct root_priv),
 };
 
 /* This is the root uclass */
diff --git a/include/asm-generic/global_data.h b/include/asm-generic/global_data.h
index 78dcf40bff..65ee3e5d5a 100644
--- a/include/asm-generic/global_data.h
+++ b/include/asm-generic/global_data.h
@@ -20,6 +20,7 @@
  */
 
 #ifndef __ASSEMBLY__
+#include <fdtdec.h>
 #include <membuff.h>
 #include <linux/list.h>
 
@@ -133,6 +134,9 @@ typedef struct global_data {
 	struct spl_handoff *spl_handoff;
 # endif
 #endif
+#if defined(CONFIG_TRANSLATION_OFFSET)
+	fdt_addr_t translation_offset;	/* optional translation offset */
+#endif
 } gd_t;
 #endif
 
diff --git a/include/dm/fdtaddr.h b/include/dm/fdtaddr.h
index c171d9bc2f..3bc2599b6c 100644
--- a/include/dm/fdtaddr.h
+++ b/include/dm/fdtaddr.h
@@ -120,25 +120,4 @@ fdt_addr_t devfdt_get_addr_size_index(struct udevice *dev, int index,
  */
 fdt_addr_t devfdt_get_addr_name(struct udevice *dev, const char *name);
 
-/**
- * dm_set_translation_offset() - Set translation offset
- * @offs: Translation offset
- *
- * Some platforms need a special address translation. Those
- * platforms (e.g. mvebu in SPL) can configure a translation
- * offset in the DM by calling this function. It will be
- * added to all addresses returned in devfdt_get_addr().
- */
-void dm_set_translation_offset(fdt_addr_t offs);
-
-/**
- * dm_get_translation_offset() - Get translation offset
- *
- * This function returns the translation offset that can
- * be configured by calling dm_set_translation_offset().
- *
- * @return translation offset for the device address (0 as default).
- */
-fdt_addr_t dm_get_translation_offset(void);
-
 #endif
-- 
2.21.0

[U-Boot] [PATCH] dm: core: Change platform specific translation-offset handling

[Pierre Bourdon]

On Fri, Apr 12, 2019 at 4:42 PM Stefan Roese <sr@denx.de> wrote:
> Testing has shown that the current DM implementation of a platform /
> board specific translation offset, as its needed for the SPL on MVEBU
> platforms is buggy. The translation offset is confingured too late,
> after the driver bind functions are run. This may result in incorrect
> address translations. With the current implementation its not possible
> to configure the offset earlier, as the DM code has not run at all.
>
> This patch now removed the set_/get_translation_offset() calls and
> moves the translation offset into the GD variable translation_offset.
> This variable will get used when CONFIG_TRANSLATION_OFFSET is enabled.
> This option is enabled only for MVEBU on ARM32 platforms, where its
> currenty needed and configured in the SPL.

Verified to work on Turris Omnia. No freeze at boot time in SPL mode,
i2c0 is properly enumerated.

> Signed-off-by: Stefan Roese <sr@denx.de>
> Cc: Pierre Bourdon <delroth@gmail.com>
> Cc: Baruch Siach <baruch@tkos.co.il>
> Cc: Simon Glass <sjg@chromium.org>
> Cc: Heiko Schocher <hs@denx.de>
> Cc: Tom Rini <trini@konsulko.com>

Tested-by: Pierre Bourdon <delroth@gmail.com>

-- 
Pierre Bourdon <delroth@gmail.com>
Software Engineer @ Zürich, Switzerland
https://delroth.net/

[U-Boot] [PATCH] dm: core: Change platform specific translation-offset handling

[Baruch Siach]

Hi Stefan,

On Fri, Apr 12 2019, Stefan Roese wrote:
> Testing has shown that the current DM implementation of a platform /
> board specific translation offset, as its needed for the SPL on MVEBU
> platforms is buggy. The translation offset is confingured too late,
> after the driver bind functions are run. This may result in incorrect
> address translations. With the current implementation its not possible
> to configure the offset earlier, as the DM code has not run at all.
>
> This patch now removed the set_/get_translation_offset() calls and
> moves the translation offset into the GD variable translation_offset.
> This variable will get used when CONFIG_TRANSLATION_OFFSET is enabled.
> This option is enabled only for MVEBU on ARM32 platforms, where its
> currenty needed and configured in the SPL.
>
> Signed-off-by: Stefan Roese <sr@denx.de>
> Cc: Pierre Bourdon <delroth@gmail.com>
> Cc: Baruch Siach <baruch@tkos.co.il>
> Cc: Simon Glass <sjg@chromium.org>
> Cc: Heiko Schocher <hs@denx.de>
> Cc: Tom Rini <trini@konsulko.com>

Thanks. This fixes boot on Clearfog when I2C is enabled in SPL.

Tested-by: Baruch Siach <baruch@tkos.co.il>

baruch

-- 
     http://baruch.siach.name/blog/                  ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch at tkos.co.il - tel: +972.52.368.4656, http://www.tkos.co.il -

[U-Boot] [PATCH] dm: core: Change platform specific translation-offset handling

[sjg at google.com]

Hi Stefan,

On Fri, Apr 12 2019, Stefan Roese wrote:
> Testing has shown that the current DM implementation of a platform /
> board specific translation offset, as its needed for the SPL on MVEBU
> platforms is buggy. The translation offset is confingured too late,
> after the driver bind functions are run. This may result in incorrect
> address translations. With the current implementation its not possible
> to configure the offset earlier, as the DM code has not run at all.
>
> This patch now removed the set_/get_translation_offset() calls and
> moves the translation offset into the GD variable translation_offset.
> This variable will get used when CONFIG_TRANSLATION_OFFSET is enabled.
> This option is enabled only for MVEBU on ARM32 platforms, where its
> currenty needed and configured in the SPL.
>
> Signed-off-by: Stefan Roese <sr@denx.de>
> Cc: Pierre Bourdon <delroth@gmail.com>
> Cc: Baruch Siach <baruch@tkos.co.il>
> Cc: Simon Glass <sjg@chromium.org>
> Cc: Heiko Schocher <hs@denx.de>
> Cc: Tom Rini <trini@konsulko.com>

Thanks. This fixes boot on Clearfog when I2C is enabled in SPL.

Tested-by: Baruch Siach <baruch@tkos.co.il>

baruch

-- 
     http://baruch.siach.name/blog/                  ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch at tkos.co.il - tel: +972.52.368.4656, http://www.tkos.co.il -

Applied to u-boot-dm, thanks!