[PATCHv2 0/5] GE Bx50v3 cleanups

[PATCHv2 0/5] GE Bx50v3 cleanups

[Sebastian Reichel]

Here are some further cleanups for the GE Bx50v3 board(s):

PATCH 1+2: cleanup preprocessor usage in common/image-fit.c,
           introduced after feedback from Simon Glass in PATCHv1

PATCH 3+4: replace magic numbers by analyzing name string from
           the fitImage config node

PATCH 5: rely on PHY driver as much as possible

Changes since PATCHv1:
 * introduce patch 1 & 2
 * update patch 3 to avoid #if

-- Sebastian

Sebastian Reichel (5):
  compiler.h: add host_build()
  image: cleanup pre-processor usage
  image: support board_fit_config_name_match
  board: ge: bx50v3: remove confidx magic numbers
  board: ge: bx50v3: cleanup phy config

 arch/arm/dts/imx6q-ba16.dtsi |  11 ++
 board/ge/bx50v3/bx50v3.c     |  51 ++++-----
 common/image-fit.c           | 212 +++++++++++++++++------------------
 include/compiler.h           |   9 ++
 include/configs/ge_bx50v3.h  |   4 +-
 include/image.h              |   4 +
 6 files changed, 153 insertions(+), 138 deletions(-)

-- 
2.29.2

[PATCHv2 1/5] compiler.h: add host_build()

[Sebastian Reichel]

Add a host_build() function, so that it's possible to
check for software being build with USE_HOSTCC without
relying on preprocessor conditions. In other words

 #ifdef USE_HOSTCC
 	host_only_code();
 #endif

can be written like this instead:

 if (host_build())
 	host_only_code();

This improves code readability and test coverage and
compiler will eleminate this unreachable code.

Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
 include/compiler.h | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/include/compiler.h b/include/compiler.h
index 90b7afae5376..27b9843497a4 100644
--- a/include/compiler.h
+++ b/include/compiler.h
@@ -6,6 +6,7 @@
 #define __COMPILER_H__
 
 #include <stddef.h>
+#include <stdbool.h>
 
 #ifdef USE_HOSTCC
 
@@ -150,4 +151,12 @@ typedef unsigned long int uintptr_t;
 #define MEM_SUPPORT_64BIT_DATA	0
 #endif
 
+static inline bool host_build(void) {
+#ifdef USE_HOSTCC
+	return true;
+#else
+	return false;
+#endif
+}
+
 #endif
-- 
2.29.2

[PATCHv2 1/5] compiler.h: add host_build()

[Simon Glass]

On Mon, 14 Dec 2020 at 16:42, Sebastian Reichel
<sebastian.reichel@collabora.com> wrote:
>
> Add a host_build() function, so that it's possible to
> check for software being build with USE_HOSTCC without
> relying on preprocessor conditions. In other words
>
>  #ifdef USE_HOSTCC
>         host_only_code();
>  #endif
>
> can be written like this instead:
>
>  if (host_build())
>         host_only_code();
>
> This improves code readability and test coverage and
> compiler will eleminate this unreachable code.

eliminate

>
> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
> ---
>  include/compiler.h | 9 +++++++++
>  1 file changed, 9 insertions(+)
>

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

[PATCHv2 1/5] compiler.h: add host_build()

[sbabic at denx.de]

> Add a host_build() function, so that it's possible to
> check for software being build with USE_HOSTCC without
> relying on preprocessor conditions. In other words
>  #ifdef USE_HOSTCC
>  	host_only_code();
>  #endif
> can be written like this instead:
>  if (host_build())
>  	host_only_code();
> This improves code readability and test coverage and
> compiler will eleminate this unreachable code.
> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
> Reviewed-by: Simon Glass <sjg@chromium.org>
Applied to u-boot-imx, master, thanks !

Best regards,
Stefano Babic

-- 
=====================================================================
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================

[PATCHv2 2/5] image: cleanup pre-processor usage

[Sebastian Reichel]

Replace most #ifdef checks for USE_HOSTCC and CONFIG_*
with normal if instructions.

Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
 common/image-fit.c | 193 +++++++++++++++++++++------------------------
 include/image.h    |   4 +
 2 files changed, 96 insertions(+), 101 deletions(-)

diff --git a/common/image-fit.c b/common/image-fit.c
index c82d4d8015f0..1f382d87e207 100644
--- a/common/image-fit.c
+++ b/common/image-fit.c
@@ -15,7 +15,6 @@
 #include <u-boot/crc.h>
 #else
 #include <linux/compiler.h>
-#include <linux/kconfig.h>
 #include <common.h>
 #include <errno.h>
 #include <log.h>
@@ -28,6 +27,7 @@ DECLARE_GLOBAL_DATA_PTR;
 #include <bootm.h>
 #include <image.h>
 #include <bootstage.h>
+#include <linux/kconfig.h>
 #include <u-boot/crc.h>
 #include <u-boot/md5.h>
 #include <u-boot/sha1.h>
@@ -486,16 +486,16 @@ void fit_image_print(const void *fit, int image_noffset, const char *p)
 
 	ret = fit_image_get_data_and_size(fit, image_noffset, &data, &size);
 
-#ifndef USE_HOSTCC
-	printf("%s  Data Start:   ", p);
-	if (ret) {
-		printf("unavailable\n");
-	} else {
-		void *vdata = (void *)data;
+	if (!host_build()) {
+		printf("%s  Data Start:   ", p);
+		if (ret) {
+			printf("unavailable\n");
+		} else {
+			void *vdata = (void *)data;
 
-		printf("0x%08lx\n", (ulong)map_to_sysmem(vdata));
+			printf("0x%08lx\n", (ulong)map_to_sysmem(vdata));
+		}
 	}
-#endif
 
 	printf("%s  Data Size:    ", p);
 	if (ret)
@@ -1420,7 +1420,6 @@ int fit_all_image_verify(const void *fit)
 	return 1;
 }
 
-#ifdef CONFIG_FIT_CIPHER
 static int fit_image_uncipher(const void *fit, int image_noffset,
 			      void **data, size_t *size)
 {
@@ -1444,7 +1443,6 @@ static int fit_image_uncipher(const void *fit, int image_noffset,
  out:
 	return ret;
 }
-#endif /* CONFIG_FIT_CIPHER */
 
 /**
  * fit_image_check_os - check whether image node is of a given os type
@@ -1486,9 +1484,8 @@ int fit_image_check_arch(const void *fit, int noffset, uint8_t arch)
 	uint8_t image_arch;
 	int aarch32_support = 0;
 
-#ifdef CONFIG_ARM64_SUPPORT_AARCH32
-	aarch32_support = 1;
-#endif
+	if (IS_ENABLED(CONFIG_ARM64_SUPPORT_AARCH32))
+		aarch32_support = 1;
 
 	if (fit_image_get_arch(fit, noffset, &image_arch))
 		return 0;
@@ -1977,13 +1974,13 @@ int fit_image_load(bootm_headers_t *images, ulong addr,
 	}
 
 	bootstage_mark(bootstage_id + BOOTSTAGE_SUB_CHECK_ARCH);
-#if !defined(USE_HOSTCC) && !defined(CONFIG_SANDBOX)
-	if (!fit_image_check_target_arch(fit, noffset)) {
-		puts("Unsupported Architecture\n");
-		bootstage_error(bootstage_id + BOOTSTAGE_SUB_CHECK_ARCH);
-		return -ENOEXEC;
+	if (!host_build() && IS_ENABLED(CONFIG_SANDBOX)) {
+		if (!fit_image_check_target_arch(fit, noffset)) {
+			puts("Unsupported Architecture\n");
+			bootstage_error(bootstage_id + BOOTSTAGE_SUB_CHECK_ARCH);
+			return -ENOEXEC;
+		}
 	}
-#endif
 
 #ifndef USE_HOSTCC
 	fit_image_get_arch(fit, noffset, &os_arch);
@@ -2029,9 +2026,8 @@ int fit_image_load(bootm_headers_t *images, ulong addr,
 		return -ENOENT;
 	}
 
-#ifdef CONFIG_FIT_CIPHER
 	/* Decrypt data before uncompress/move */
-	if (IMAGE_ENABLE_DECRYPT) {
+	if (IS_ENABLED(CONFIG_FIT_CIPHER) && IMAGE_ENABLE_DECRYPT) {
 		puts("   Decrypting Data ... ");
 		if (fit_image_uncipher(fit, noffset, &buf, &size)) {
 			puts("Error\n");
@@ -2039,12 +2035,10 @@ int fit_image_load(bootm_headers_t *images, ulong addr,
 		}
 		puts("OK\n");
 	}
-#endif
 
-#if !defined(USE_HOSTCC) && defined(CONFIG_FIT_IMAGE_POST_PROCESS)
 	/* perform any post-processing on the image data */
-	board_fit_image_post_process(&buf, &size);
-#endif
+	if (!host_build() && IS_ENABLED(CONFIG_FIT_IMAGE_POST_PROCESS))
+		board_fit_image_post_process(&buf, &size);
 
 	len = (ulong)size;
 
@@ -2166,14 +2160,12 @@ int boot_get_fdt_fit(bootm_headers_t *images, ulong addr,
 	char *fit_uname_config_copy = NULL;
 	char *next_config = NULL;
 	ulong load, len;
-#ifdef CONFIG_OF_LIBFDT_OVERLAY
 	ulong image_start, image_end;
 	ulong ovload, ovlen;
 	const char *uconfig;
 	const char *uname;
 	void *base, *ov;
 	int i, err, noffset, ov_noffset;
-#endif
 
 	fit_uname = fit_unamep ? *fit_unamep : NULL;
 
@@ -2212,84 +2204,83 @@ int boot_get_fdt_fit(bootm_headers_t *images, ulong addr,
 		goto out;
 
 	/* we need to apply overlays */
-
-#ifdef CONFIG_OF_LIBFDT_OVERLAY
-	image_start = addr;
-	image_end = addr + fit_get_size(fit);
-	/* verify that relocation took place by load address not being in fit */
-	if (load >= image_start && load < image_end) {
-		/* check is simplified; fit load checks for overlaps */
-		printf("Overlayed FDT requires relocation\n");
-		fdt_noffset = -EBADF;
-		goto out;
-	}
-
-	base = map_sysmem(load, len);
-
-	/* apply extra configs in FIT first, followed by args */
-	for (i = 1; ; i++) {
-		if (i < count) {
-			noffset = fit_conf_get_prop_node_index(fit, cfg_noffset,
-							       FIT_FDT_PROP, i);
-			uname = fit_get_name(fit, noffset, NULL);
-			uconfig = NULL;
-		} else {
-			if (!next_config)
-				break;
-			uconfig = next_config;
-			next_config = strchr(next_config, '#');
-			if (next_config)
-				*next_config++ = '\0';
-			uname = NULL;
-
-			/*
-			 * fit_image_load() would load the first FDT from the
-			 * extra config only when uconfig is specified.
-			 * Check if the extra config contains multiple FDTs and
-			 * if so, load them.
-			 */
-			cfg_noffset = fit_conf_get_node(fit, uconfig);
-
-			i = 0;
-			count = fit_conf_get_prop_node_count(fit, cfg_noffset,
-							     FIT_FDT_PROP);
+	if (IS_ENABLED(CONFIG_OF_LIBFDT_OVERLAY)) {
+		image_start = addr;
+		image_end = addr + fit_get_size(fit);
+		/* verify that relocation took place by load address not being in fit */
+		if (load >= image_start && load < image_end) {
+			/* check is simplified; fit load checks for overlaps */
+			printf("Overlayed FDT requires relocation\n");
+			fdt_noffset = -EBADF;
+			goto out;
 		}
 
-		debug("%d: using uname=%s uconfig=%s\n", i, uname, uconfig);
+		base = map_sysmem(load, len);
+
+		/* apply extra configs in FIT first, followed by args */
+		for (i = 1; ; i++) {
+			if (i < count) {
+				noffset = fit_conf_get_prop_node_index(fit, cfg_noffset,
+								       FIT_FDT_PROP, i);
+				uname = fit_get_name(fit, noffset, NULL);
+				uconfig = NULL;
+			} else {
+				if (!next_config)
+					break;
+				uconfig = next_config;
+				next_config = strchr(next_config, '#');
+				if (next_config)
+					*next_config++ = '\0';
+				uname = NULL;
+
+				/*
+				 * fit_image_load() would load the first FDT from the
+				 * extra config only when uconfig is specified.
+				 * Check if the extra config contains multiple FDTs and
+				 * if so, load them.
+				 */
+				cfg_noffset = fit_conf_get_node(fit, uconfig);
+
+				i = 0;
+				count = fit_conf_get_prop_node_count(fit, cfg_noffset,
+								     FIT_FDT_PROP);
+			}
 
-		ov_noffset = fit_image_load(images,
-			addr, &uname, &uconfig,
-			arch, IH_TYPE_FLATDT,
-			BOOTSTAGE_ID_FIT_FDT_START,
-			FIT_LOAD_REQUIRED, &ovload, &ovlen);
-		if (ov_noffset < 0) {
-			printf("load of %s failed\n", uname);
-			continue;
-		}
-		debug("%s loaded at 0x%08lx len=0x%08lx\n",
-				uname, ovload, ovlen);
-		ov = map_sysmem(ovload, ovlen);
-
-		base = map_sysmem(load, len + ovlen);
-		err = fdt_open_into(base, base, len + ovlen);
-		if (err < 0) {
-			printf("failed on fdt_open_into\n");
-			fdt_noffset = err;
-			goto out;
-		}
-		/* the verbose method prints out messages on error */
-		err = fdt_overlay_apply_verbose(base, ov);
-		if (err < 0) {
-			fdt_noffset = err;
-			goto out;
+			debug("%d: using uname=%s uconfig=%s\n", i, uname, uconfig);
+
+			ov_noffset = fit_image_load(images,
+				addr, &uname, &uconfig,
+				arch, IH_TYPE_FLATDT,
+				BOOTSTAGE_ID_FIT_FDT_START,
+				FIT_LOAD_REQUIRED, &ovload, &ovlen);
+			if (ov_noffset < 0) {
+				printf("load of %s failed\n", uname);
+				continue;
+			}
+			debug("%s loaded@0x%08lx len=0x%08lx\n",
+					uname, ovload, ovlen);
+			ov = map_sysmem(ovload, ovlen);
+
+			base = map_sysmem(load, len + ovlen);
+			err = fdt_open_into(base, base, len + ovlen);
+			if (err < 0) {
+				printf("failed on fdt_open_into\n");
+				fdt_noffset = err;
+				goto out;
+			}
+			/* the verbose method prints out messages on error */
+			err = fdt_overlay_apply_verbose(base, ov);
+			if (err < 0) {
+				fdt_noffset = err;
+				goto out;
+			}
+			fdt_pack(base);
+			len = fdt_totalsize(base);
 		}
-		fdt_pack(base);
-		len = fdt_totalsize(base);
+	} else {
+		printf("config with overlays but CONFIG_OF_LIBFDT_OVERLAY not set\n");
+		fdt_noffset = -EBADF;
 	}
-#else
-	printf("config with overlays but CONFIG_OF_LIBFDT_OVERLAY not set\n");
-	fdt_noffset = -EBADF;
-#endif
 
 out:
 	if (datap)
diff --git a/include/image.h b/include/image.h
index 00bc03bebece..41473dbb9cdf 100644
--- a/include/image.h
+++ b/include/image.h
@@ -1552,6 +1552,10 @@ int board_fit_config_name_match(const char *name);
  * @return no return value (failure should be handled internally)
  */
 void board_fit_image_post_process(void **p_image, size_t *p_size);
+#else
+static inline void board_fit_image_post_process(void **p_image, size_t *p_size)
+{
+}
 #endif /* CONFIG_SPL_FIT_IMAGE_POST_PROCESS */
 
 #define FDT_ERROR	((ulong)(-1))
-- 
2.29.2

[PATCHv2 2/5] image: cleanup pre-processor usage

[Simon Glass]

Hi Sebastian,

On Mon, 14 Dec 2020 at 16:42, Sebastian Reichel
<sebastian.reichel@collabora.com> wrote:
>
> Replace most #ifdef checks for USE_HOSTCC and CONFIG_*
> with normal if instructions.
>
> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
> ---
>  common/image-fit.c | 193 +++++++++++++++++++++------------------------
>  include/image.h    |   4 +
>  2 files changed, 96 insertions(+), 101 deletions(-)

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

>
> diff --git a/common/image-fit.c b/common/image-fit.c
> index c82d4d8015f0..1f382d87e207 100644
> --- a/common/image-fit.c
> +++ b/common/image-fit.c
> @@ -15,7 +15,6 @@
>  #include <u-boot/crc.h>
>  #else
>  #include <linux/compiler.h>
> -#include <linux/kconfig.h>
>  #include <common.h>
>  #include <errno.h>
>  #include <log.h>
> @@ -28,6 +27,7 @@ DECLARE_GLOBAL_DATA_PTR;
>  #include <bootm.h>
>  #include <image.h>
>  #include <bootstage.h>
> +#include <linux/kconfig.h>
>  #include <u-boot/crc.h>
>  #include <u-boot/md5.h>
>  #include <u-boot/sha1.h>
> @@ -486,16 +486,16 @@ void fit_image_print(const void *fit, int image_noffset, const char *p)
>
>         ret = fit_image_get_data_and_size(fit, image_noffset, &data, &size);
>
> -#ifndef USE_HOSTCC
> -       printf("%s  Data Start:   ", p);
> -       if (ret) {
> -               printf("unavailable\n");
> -       } else {
> -               void *vdata = (void *)data;
> +       if (!host_build()) {
> +               printf("%s  Data Start:   ", p);
> +               if (ret) {
> +                       printf("unavailable\n");
> +               } else {
> +                       void *vdata = (void *)data;
>
> -               printf("0x%08lx\n", (ulong)map_to_sysmem(vdata));
> +                       printf("0x%08lx\n", (ulong)map_to_sysmem(vdata));
> +               }
>         }
> -#endif
>
>         printf("%s  Data Size:    ", p);
>         if (ret)
> @@ -1420,7 +1420,6 @@ int fit_all_image_verify(const void *fit)
>         return 1;
>  }
>
> -#ifdef CONFIG_FIT_CIPHER
>  static int fit_image_uncipher(const void *fit, int image_noffset,
>                               void **data, size_t *size)
>  {
> @@ -1444,7 +1443,6 @@ static int fit_image_uncipher(const void *fit, int image_noffset,
>   out:
>         return ret;
>  }
> -#endif /* CONFIG_FIT_CIPHER */
>
>  /**
>   * fit_image_check_os - check whether image node is of a given os type
> @@ -1486,9 +1484,8 @@ int fit_image_check_arch(const void *fit, int noffset, uint8_t arch)
>         uint8_t image_arch;
>         int aarch32_support = 0;
>
> -#ifdef CONFIG_ARM64_SUPPORT_AARCH32
> -       aarch32_support = 1;
> -#endif
> +       if (IS_ENABLED(CONFIG_ARM64_SUPPORT_AARCH32))
> +               aarch32_support = 1;
>
>         if (fit_image_get_arch(fit, noffset, &image_arch))
>                 return 0;
> @@ -1977,13 +1974,13 @@ int fit_image_load(bootm_headers_t *images, ulong addr,
>         }
>
>         bootstage_mark(bootstage_id + BOOTSTAGE_SUB_CHECK_ARCH);
> -#if !defined(USE_HOSTCC) && !defined(CONFIG_SANDBOX)
> -       if (!fit_image_check_target_arch(fit, noffset)) {
> -               puts("Unsupported Architecture\n");
> -               bootstage_error(bootstage_id + BOOTSTAGE_SUB_CHECK_ARCH);
> -               return -ENOEXEC;
> +       if (!host_build() && IS_ENABLED(CONFIG_SANDBOX)) {
> +               if (!fit_image_check_target_arch(fit, noffset)) {
> +                       puts("Unsupported Architecture\n");
> +                       bootstage_error(bootstage_id + BOOTSTAGE_SUB_CHECK_ARCH);
> +                       return -ENOEXEC;
> +               }
>         }
> -#endif
>
>  #ifndef USE_HOSTCC
>         fit_image_get_arch(fit, noffset, &os_arch);
> @@ -2029,9 +2026,8 @@ int fit_image_load(bootm_headers_t *images, ulong addr,
>                 return -ENOENT;
>         }
>
> -#ifdef CONFIG_FIT_CIPHER
>         /* Decrypt data before uncompress/move */
> -       if (IMAGE_ENABLE_DECRYPT) {
> +       if (IS_ENABLED(CONFIG_FIT_CIPHER) && IMAGE_ENABLE_DECRYPT) {
>                 puts("   Decrypting Data ... ");
>                 if (fit_image_uncipher(fit, noffset, &buf, &size)) {
>                         puts("Error\n");
> @@ -2039,12 +2035,10 @@ int fit_image_load(bootm_headers_t *images, ulong addr,
>                 }
>                 puts("OK\n");
>         }
> -#endif
>
> -#if !defined(USE_HOSTCC) && defined(CONFIG_FIT_IMAGE_POST_PROCESS)
>         /* perform any post-processing on the image data */
> -       board_fit_image_post_process(&buf, &size);
> -#endif
> +       if (!host_build() && IS_ENABLED(CONFIG_FIT_IMAGE_POST_PROCESS))
> +               board_fit_image_post_process(&buf, &size);
>
>         len = (ulong)size;
>
> @@ -2166,14 +2160,12 @@ int boot_get_fdt_fit(bootm_headers_t *images, ulong addr,
>         char *fit_uname_config_copy = NULL;
>         char *next_config = NULL;
>         ulong load, len;
> -#ifdef CONFIG_OF_LIBFDT_OVERLAY
>         ulong image_start, image_end;
>         ulong ovload, ovlen;
>         const char *uconfig;
>         const char *uname;
>         void *base, *ov;
>         int i, err, noffset, ov_noffset;
> -#endif
>
>         fit_uname = fit_unamep ? *fit_unamep : NULL;
>
> @@ -2212,84 +2204,83 @@ int boot_get_fdt_fit(bootm_headers_t *images, ulong addr,
>                 goto out;
>
>         /* we need to apply overlays */
> -
> -#ifdef CONFIG_OF_LIBFDT_OVERLAY
> -       image_start = addr;
> -       image_end = addr + fit_get_size(fit);
> -       /* verify that relocation took place by load address not being in fit */
> -       if (load >= image_start && load < image_end) {
> -               /* check is simplified; fit load checks for overlaps */
> -               printf("Overlayed FDT requires relocation\n");
> -               fdt_noffset = -EBADF;
> -               goto out;
> -       }
> -
> -       base = map_sysmem(load, len);
> -
> -       /* apply extra configs in FIT first, followed by args */
> -       for (i = 1; ; i++) {
> -               if (i < count) {
> -                       noffset = fit_conf_get_prop_node_index(fit, cfg_noffset,
> -                                                              FIT_FDT_PROP, i);
> -                       uname = fit_get_name(fit, noffset, NULL);
> -                       uconfig = NULL;
> -               } else {
> -                       if (!next_config)
> -                               break;
> -                       uconfig = next_config;
> -                       next_config = strchr(next_config, '#');
> -                       if (next_config)
> -                               *next_config++ = '\0';
> -                       uname = NULL;
> -
> -                       /*
> -                        * fit_image_load() would load the first FDT from the
> -                        * extra config only when uconfig is specified.
> -                        * Check if the extra config contains multiple FDTs and
> -                        * if so, load them.
> -                        */
> -                       cfg_noffset = fit_conf_get_node(fit, uconfig);
> -
> -                       i = 0;
> -                       count = fit_conf_get_prop_node_count(fit, cfg_noffset,
> -                                                            FIT_FDT_PROP);
> +       if (IS_ENABLED(CONFIG_OF_LIBFDT_OVERLAY)) {

The odd thing about the host build is that it normally does not have
access to the CONFIG options. Presumably
IS_ENABLED(CONFIG_OF_LIBFDT_OVERLAY) is therefore 0

However, in general on the host we want to enable all features
relevant to the host tools.

if (host_build() || IS_ENABLED(CONFIG_OF_LIBFDT_OVERLAY))

(BTW this is a general comment and I don't think it affects this
patch...if you do adjust things it should be done after this series,
since from what I can tell your series does not change behaviour)

Regards,
Simon

[PATCHv2 3/5] image: support board_fit_config_name_match

[Sebastian Reichel]

Support reusing board_fit_config_name_match() to automatically
select a sensible default configuration for booting fitImages
using 'bootm'.

Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
 common/image-fit.c | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/common/image-fit.c b/common/image-fit.c
index 1f382d87e207..b3aeff8c5e4a 100644
--- a/common/image-fit.c
+++ b/common/image-fit.c
@@ -1738,12 +1738,19 @@ int fit_conf_get_node(const void *fit, const char *conf_uname)
 	if (conf_uname == NULL) {
 		/* get configuration unit name from the default property */
 		debug("No configuration specified, trying default...\n");
-		conf_uname = (char *)fdt_getprop(fit, confs_noffset,
-						 FIT_DEFAULT_PROP, &len);
-		if (conf_uname == NULL) {
-			fit_get_debug(fit, confs_noffset, FIT_DEFAULT_PROP,
-				      len);
-			return len;
+		if (!host_build() && IS_ENABLED(CONFIG_MULTI_DTB_FIT)) {
+			noffset = fit_find_config_node(fit);
+			if (noffset < 0)
+				return noffset;
+			conf_uname = fdt_get_name(fit, noffset, NULL);
+		} else {
+			conf_uname = (char *)fdt_getprop(fit, confs_noffset,
+							 FIT_DEFAULT_PROP, &len);
+			if (conf_uname == NULL) {
+				fit_get_debug(fit, confs_noffset, FIT_DEFAULT_PROP,
+					      len);
+				return len;
+			}
 		}
 		debug("Found default configuration: '%s'\n", conf_uname);
 	}
-- 
2.29.2

[PATCHv2 3/5] image: support board_fit_config_name_match

[Simon Glass]

On Mon, 14 Dec 2020 at 16:42, Sebastian Reichel
<sebastian.reichel@collabora.com> wrote:
>
> Support reusing board_fit_config_name_match() to automatically
> select a sensible default configuration for booting fitImages
> using 'bootm'.
>
> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
> ---
>  common/image-fit.c | 19 +++++++++++++------
>  1 file changed, 13 insertions(+), 6 deletions(-)
>

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

Can you see if you can add a test for this case?

[PATCHv2 3/5] image: support board_fit_config_name_match

[Stefano Babic]

On 15.12.20 00:41, Sebastian Reichel wrote:
> Support reusing board_fit_config_name_match() to automatically
> select a sensible default configuration for booting fitImages
> using 'bootm'.
> 

For reasons I have not understood, this patch breaks "sometimes" the build :

+common/image-fit.c: In function 'boot_get_fdt_fit':
+common/image-fit.c:2279:10: error: implicit declaration of function
'fdt_overlay_apply_verbose'; did you mean 'fdt_overlay_apply_node'?
[-Werror=implicit-function-declaration]
+ 2279 |    err = fdt_overlay_apply_verbose(base, ov);
+      |          ^~~~~~~~~~~~~~~~~~~~~~~~~
+      |          fdt_overlay_apply_node
+cc1: all warnings being treated as errors

https://gitlab.denx.de/u-boot/custodians/u-boot-imx/-/jobs/193967

And it looks to me that fdt_support.h is included.

This happens if I run buildman locally, too. But it is not
deterministic, sometimes it succeed.

snapper9g20 and snapper9260 seem to trigger more frequently the issues
(but they are not the ones). So I applied the patches related to the
board, but I left this out.

Best regards,
Stefano Babic

> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
> ---
>  common/image-fit.c | 19 +++++++++++++------
>  1 file changed, 13 insertions(+), 6 deletions(-)
> 
> diff --git a/common/image-fit.c b/common/image-fit.c
> index 1f382d87e207..b3aeff8c5e4a 100644
> --- a/common/image-fit.c
> +++ b/common/image-fit.c
> @@ -1738,12 +1738,19 @@ int fit_conf_get_node(const void *fit, const char *conf_uname)
>  	if (conf_uname == NULL) {
>  		/* get configuration unit name from the default property */
>  		debug("No configuration specified, trying default...\n");
> -		conf_uname = (char *)fdt_getprop(fit, confs_noffset,
> -						 FIT_DEFAULT_PROP, &len);
> -		if (conf_uname == NULL) {
> -			fit_get_debug(fit, confs_noffset, FIT_DEFAULT_PROP,
> -				      len);
> -			return len;
> +		if (!host_build() && IS_ENABLED(CONFIG_MULTI_DTB_FIT)) {
> +			noffset = fit_find_config_node(fit);
> +			if (noffset < 0)
> +				return noffset;
> +			conf_uname = fdt_get_name(fit, noffset, NULL);
> +		} else {
> +			conf_uname = (char *)fdt_getprop(fit, confs_noffset,
> +							 FIT_DEFAULT_PROP, &len);
> +			if (conf_uname == NULL) {
> +				fit_get_debug(fit, confs_noffset, FIT_DEFAULT_PROP,
> +					      len);
> +				return len;
> +			}
>  		}
>  		debug("Found default configuration: '%s'\n", conf_uname);
>  	}
> 


-- 
=====================================================================
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================

[PATCHv2 3/5] image: support board_fit_config_name_match

[Sebastian Reichel]

Hi,

On Mon, Dec 28, 2020 at 12:39:57PM +0100, Stefano Babic wrote:
> On 15.12.20 00:41, Sebastian Reichel wrote:
> > Support reusing board_fit_config_name_match() to automatically
> > select a sensible default configuration for booting fitImages
> > using 'bootm'.
> > 
> 
> For reasons I have not understood, this patch breaks "sometimes" the build :
> 
> +common/image-fit.c: In function 'boot_get_fdt_fit':
> +common/image-fit.c:2279:10: error: implicit declaration of function
> 'fdt_overlay_apply_verbose'; did you mean 'fdt_overlay_apply_node'?
> [-Werror=implicit-function-declaration]
> + 2279 |    err = fdt_overlay_apply_verbose(base, ov);
> +      |          ^~~~~~~~~~~~~~~~~~~~~~~~~
> +      |          fdt_overlay_apply_node
> +cc1: all warnings being treated as errors
> 
> https://gitlab.denx.de/u-boot/custodians/u-boot-imx/-/jobs/193967
> 
> And it looks to me that fdt_support.h is included.
> 
> This happens if I run buildman locally, too. But it is not
> deterministic, sometimes it succeed.
> 
> snapper9g20 and snapper9260 seem to trigger more frequently the issues
> (but they are not the ones).

I found the root cause. It's not this patch, but the previous one.
fdt_support.h is included, but in it all declartions only happen
if OF_LIBFDT is defined. My previous patch changes the code block
calling fdt_overlay_apply_verbose() from ifdef OF_LIBFDT_OVERLAY
to IS_ENABLED(OF_LIBFDT_OVERLAY). Note, that OF_LIBFDT_OVERLAY
requires OF_LIBFDT.

I'm about to send PATCHv3 dropping the OF_LIBFDT_OVERLAY related
cleanup.

> So I applied the patches related to the board, but I left this
> out.

Not applying this patch results in GE Bx50v3 boards no longer
booting properly with the default config :(

-- Sebastian

> Best regards,
> Stefano Babic
> 
> > Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
> > ---
> >  common/image-fit.c | 19 +++++++++++++------
> >  1 file changed, 13 insertions(+), 6 deletions(-)
> > 
> > diff --git a/common/image-fit.c b/common/image-fit.c
> > index 1f382d87e207..b3aeff8c5e4a 100644
> > --- a/common/image-fit.c
> > +++ b/common/image-fit.c
> > @@ -1738,12 +1738,19 @@ int fit_conf_get_node(const void *fit, const char *conf_uname)
> >  	if (conf_uname == NULL) {
> >  		/* get configuration unit name from the default property */
> >  		debug("No configuration specified, trying default...\n");
> > -		conf_uname = (char *)fdt_getprop(fit, confs_noffset,
> > -						 FIT_DEFAULT_PROP, &len);
> > -		if (conf_uname == NULL) {
> > -			fit_get_debug(fit, confs_noffset, FIT_DEFAULT_PROP,
> > -				      len);
> > -			return len;
> > +		if (!host_build() && IS_ENABLED(CONFIG_MULTI_DTB_FIT)) {
> > +			noffset = fit_find_config_node(fit);
> > +			if (noffset < 0)
> > +				return noffset;
> > +			conf_uname = fdt_get_name(fit, noffset, NULL);
> > +		} else {
> > +			conf_uname = (char *)fdt_getprop(fit, confs_noffset,
> > +							 FIT_DEFAULT_PROP, &len);
> > +			if (conf_uname == NULL) {
> > +				fit_get_debug(fit, confs_noffset, FIT_DEFAULT_PROP,
> > +					      len);
> > +				return len;
> > +			}
> >  		}
> >  		debug("Found default configuration: '%s'\n", conf_uname);
> >  	}
> > 
> 
> 
> -- 
> =====================================================================
> DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
> =====================================================================
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20210104/83b8334d/attachment.sig>

[PATCHv2 3/5] image: support board_fit_config_name_match

[Stefano Babic]

Hi Sebastian,

On 04.01.21 20:27, Sebastian Reichel wrote:
> Hi,
> 
> On Mon, Dec 28, 2020 at 12:39:57PM +0100, Stefano Babic wrote:
>> On 15.12.20 00:41, Sebastian Reichel wrote:
>>> Support reusing board_fit_config_name_match() to automatically
>>> select a sensible default configuration for booting fitImages
>>> using 'bootm'.
>>>
>>
>> For reasons I have not understood, this patch breaks "sometimes" the build :
>>
>> +common/image-fit.c: In function 'boot_get_fdt_fit':
>> +common/image-fit.c:2279:10: error: implicit declaration of function
>> 'fdt_overlay_apply_verbose'; did you mean 'fdt_overlay_apply_node'?
>> [-Werror=implicit-function-declaration]
>> + 2279 |    err = fdt_overlay_apply_verbose(base, ov);
>> +      |          ^~~~~~~~~~~~~~~~~~~~~~~~~
>> +      |          fdt_overlay_apply_node
>> +cc1: all warnings being treated as errors
>>
>> https://gitlab.denx.de/u-boot/custodians/u-boot-imx/-/jobs/193967
>>
>> And it looks to me that fdt_support.h is included.
>>
>> This happens if I run buildman locally, too. But it is not
>> deterministic, sometimes it succeed.
>>
>> snapper9g20 and snapper9260 seem to trigger more frequently the issues
>> (but they are not the ones).
> 
> I found the root cause. It's not this patch, but the previous one.
> fdt_support.h is included, but in it all declartions only happen
> if OF_LIBFDT is defined. My previous patch changes the code block
> calling fdt_overlay_apply_verbose() from ifdef OF_LIBFDT_OVERLAY
> to IS_ENABLED(OF_LIBFDT_OVERLAY). Note, that OF_LIBFDT_OVERLAY
> requires OF_LIBFDT.

Good catch !

> 
> I'm about to send PATCHv3 dropping the OF_LIBFDT_OVERLAY related
> cleanup.
> 

Thanks !

>> So I applied the patches related to the board, but I left this
>> out.
> 
> Not applying this patch results in GE Bx50v3 boards no longer
> booting properly with the default config :(
> 

I will then apply it and ask Tom to include it in the release - thanks !

Best regards,
Stefano

> -- Sebastian
> 
>> Best regards,
>> Stefano Babic
>>
>>> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
>>> ---
>>>   common/image-fit.c | 19 +++++++++++++------
>>>   1 file changed, 13 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/common/image-fit.c b/common/image-fit.c
>>> index 1f382d87e207..b3aeff8c5e4a 100644
>>> --- a/common/image-fit.c
>>> +++ b/common/image-fit.c
>>> @@ -1738,12 +1738,19 @@ int fit_conf_get_node(const void *fit, const char *conf_uname)
>>>   	if (conf_uname == NULL) {
>>>   		/* get configuration unit name from the default property */
>>>   		debug("No configuration specified, trying default...\n");
>>> -		conf_uname = (char *)fdt_getprop(fit, confs_noffset,
>>> -						 FIT_DEFAULT_PROP, &len);
>>> -		if (conf_uname == NULL) {
>>> -			fit_get_debug(fit, confs_noffset, FIT_DEFAULT_PROP,
>>> -				      len);
>>> -			return len;
>>> +		if (!host_build() && IS_ENABLED(CONFIG_MULTI_DTB_FIT)) {
>>> +			noffset = fit_find_config_node(fit);
>>> +			if (noffset < 0)
>>> +				return noffset;
>>> +			conf_uname = fdt_get_name(fit, noffset, NULL);
>>> +		} else {
>>> +			conf_uname = (char *)fdt_getprop(fit, confs_noffset,
>>> +							 FIT_DEFAULT_PROP, &len);
>>> +			if (conf_uname == NULL) {
>>> +				fit_get_debug(fit, confs_noffset, FIT_DEFAULT_PROP,
>>> +					      len);
>>> +				return len;
>>> +			}
>>>   		}
>>>   		debug("Found default configuration: '%s'\n", conf_uname);
>>>   	}
>>>
>>
>>
>> -- 
>> =====================================================================
>> DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
>> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
>> Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
>> =====================================================================

-- 
=====================================================================
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================

[PATCHv2 3/5] image: support board_fit_config_name_match

[Sebastian Reichel]

Hi Stefano,

On Mon, Jan 04, 2021 at 08:41:33PM +0100, Stefano Babic wrote:
> > I'm about to send PATCHv3 dropping the OF_LIBFDT_OVERLAY related
> > cleanup.
> 
> Thanks !
> 
> > > So I applied the patches related to the board, but I left this
> > > out.
> > 
> > Not applying this patch results in GE Bx50v3 boards no longer
> > booting properly with the default config :(
> > 
> 
> I will then apply it and ask Tom to include it in the release - thanks!

Thanks, this is PATCHv3 for the remaining patches:

https://patchwork.ozlabs.org/project/uboot/list/?series=222707

-- Sebastian
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20210105/8f9be3b6/attachment.sig>

[PATCHv2 3/5] image: support board_fit_config_name_match

[Stefano Babic]

Hi Sebastian,

On 05.01.21 18:27, Sebastian Reichel wrote:
> Hi Stefano,
> 
> On Mon, Jan 04, 2021 at 08:41:33PM +0100, Stefano Babic wrote:
>>> I'm about to send PATCHv3 dropping the OF_LIBFDT_OVERLAY related
>>> cleanup.
>>
>> Thanks !
>>
>>>> So I applied the patches related to the board, but I left this
>>>> out.
>>>
>>> Not applying this patch results in GE Bx50v3 boards no longer
>>> booting properly with the default config :(
>>>
>>
>> I will then apply it and ask Tom to include it in the release - thanks!
> 
> Thanks, this is PATCHv3 for the remaining patches:
>  > https://patchwork.ozlabs.org/project/uboot/list/?series=222707

Yes, I saw them and I asked Tom to pick them up ;-)

Stefano

> 
> -- Sebastian
> 

-- 
=====================================================================
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================

[PATCHv2 4/5] board: ge: bx50v3: remove confidx magic numbers

[Sebastian Reichel]

Instead of hardcoding index magic numbers in the board code,
also rely on board_fit_config_name_match choosing the right
config for the fitImage containing the kernel.

Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
 board/ge/bx50v3/bx50v3.c    | 16 ++++++++++------
 include/configs/ge_bx50v3.h |  4 ++--
 2 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/board/ge/bx50v3/bx50v3.c b/board/ge/bx50v3/bx50v3.c
index 4754647fb4ad..a3ae037a82ef 100644
--- a/board/ge/bx50v3/bx50v3.c
+++ b/board/ge/bx50v3/bx50v3.c
@@ -356,15 +356,12 @@ static void process_vpd(struct vpd_cache *vpd)
 
 	switch (vpd->product_id) {
 	case VPD_PRODUCT_B450:
-		env_set("confidx", "1");
 		i210_index = 1;
 		break;
 	case VPD_PRODUCT_B650:
-		env_set("confidx", "2");
 		i210_index = 1;
 		break;
 	case VPD_PRODUCT_B850:
-		env_set("confidx", "3");
 		i210_index = 2;
 		break;
 	}
@@ -554,16 +551,23 @@ int ft_board_setup(void *blob, struct bd_info *bd)
 
 int board_fit_config_name_match(const char *name)
 {
+	const char *machine = name;
+
 	if (!vpd.is_read)
 		return strcmp(name, "imx6q-bx50v3");
 
+	if (!strncmp(machine, "Boot ", 5))
+		machine += 5;
+	if (!strncmp(machine, "imx6q-", 6))
+		machine += 6;
+
 	switch (vpd.product_id) {
 	case VPD_PRODUCT_B450:
-		return strcmp(name, "imx6q-b450v3");
+		return strcasecmp(machine, "b450v3");
 	case VPD_PRODUCT_B650:
-		return strcmp(name, "imx6q-b650v3");
+		return strcasecmp(machine, "b650v3");
 	case VPD_PRODUCT_B850:
-		return strcmp(name, "imx6q-b850v3");
+		return strcasecmp(machine, "b850v3");
 	default:
 		return -1;
 	}
diff --git a/include/configs/ge_bx50v3.h b/include/configs/ge_bx50v3.h
index e5c580b3f910..2d854af9a06d 100644
--- a/include/configs/ge_bx50v3.h
+++ b/include/configs/ge_bx50v3.h
@@ -62,7 +62,7 @@
         "networkboot=" \
                 "run setnetworkboot; " \
                 "nfs ${loadaddr} /srv/nfs/fitImage; " \
-                "bootm ${loadaddr}#conf@${confidx}\0" \
+                "bootm ${loadaddr}\0" \
 
 #define CONFIG_NETWORKBOOTCOMMAND \
 	"run networkboot; " \
@@ -111,7 +111,7 @@
 	"doboot=" \
 		"echo Booting from ${dev}:${devnum}:${partnum} ...; " \
 		"run setargs; " \
-		"bootm ${loadaddr}#conf@${confidx}\0" \
+		"bootm ${loadaddr}\0" \
 	"tryboot=" \
 		"setenv partnum 1; run hasfirstboot || setenv partnum 2; " \
 		"run loadimage || run swappartitions && run loadimage || " \
-- 
2.29.2

[PATCHv2 4/5] board: ge: bx50v3: remove confidx magic numbers

[sbabic at denx.de]

> Instead of hardcoding index magic numbers in the board code,
> also rely on board_fit_config_name_match choosing the right
> config for the fitImage containing the kernel.
> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
Applied to u-boot-imx, master, thanks !

Best regards,
Stefano Babic

-- 
=====================================================================
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================

[PATCHv2 5/5] board: ge: bx50v3: cleanup phy config

[Sebastian Reichel]

The current PHY rework does the following things:

1. Configure 125MHz clock
2. Setup the TX clock delay (RX is enabled by default),
3. Setup reserved bits to avoid voltage peak

The clock delays are nowadays already configured by the
PHY driver (in ar803x_delay_config). The code for that
can simply be dropped. The clock speed can also be
configured by the PHY driver by adding the device tree
property "qca,clk-out-frequency".

What is left is setting up the undocumented reserved bits
to avoid the voltage peak problem. I slightly improved its
documentation while updating the board's PHY rework code.

Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
 arch/arm/dts/imx6q-ba16.dtsi | 11 +++++++++++
 board/ge/bx50v3/bx50v3.c     | 35 ++++++++++++-----------------------
 2 files changed, 23 insertions(+), 23 deletions(-)

diff --git a/arch/arm/dts/imx6q-ba16.dtsi b/arch/arm/dts/imx6q-ba16.dtsi
index 7d8f61f2fd79..9da2bb6e869e 100644
--- a/arch/arm/dts/imx6q-ba16.dtsi
+++ b/arch/arm/dts/imx6q-ba16.dtsi
@@ -174,6 +174,17 @@
 	pinctrl-0 = <&pinctrl_enet>;
 	phy-mode = "rgmii-id";
 	status = "okay";
+	phy-handle = <&phy0>;
+
+	mdio {
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		phy0: ethernet-phy at 4 {
+			reg = <4>;
+			qca,clk-out-frequency = <125000000>;
+		};
+	};
 };
 
 &hdmi {
diff --git a/board/ge/bx50v3/bx50v3.c b/board/ge/bx50v3/bx50v3.c
index a3ae037a82ef..3ea9425fd1ea 100644
--- a/board/ge/bx50v3/bx50v3.c
+++ b/board/ge/bx50v3/bx50v3.c
@@ -47,6 +47,10 @@ DECLARE_GLOBAL_DATA_PTR;
 #define VPD_PRODUCT_B650 2
 #define VPD_PRODUCT_B450 3
 
+#define AR8033_DBG_REG_ADDR		0x1d
+#define AR8033_DBG_REG_DATA		0x1e
+#define AR8033_SERDES_REG		0x5
+
 static int productid;  /* Default to generic. */
 static struct vpd_cache vpd;
 
@@ -61,31 +65,16 @@ int dram_init(void)
 	return 0;
 }
 
-static int mx6_rgmii_rework(struct phy_device *phydev)
-{
-	/* Configure AR8033 to ouput a 125MHz clk from CLK_25M */
-	/* set device address 0x7 */
-	phy_write(phydev, MDIO_DEVAD_NONE, 0xd, 0x7);
-	/* offset 0x8016: CLK_25M Clock Select */
-	phy_write(phydev, MDIO_DEVAD_NONE, 0xe, 0x8016);
-	/* enable register write, no post increment, address 0x7 */
-	phy_write(phydev, MDIO_DEVAD_NONE, 0xd, 0x4007);
-	/* set to 125 MHz from local PLL source */
-	phy_write(phydev, MDIO_DEVAD_NONE, 0xe, 0x18);
-
-	/* rgmii tx clock delay enable */
-	/* set debug port address: SerDes Test and System Mode Control */
-	phy_write(phydev, MDIO_DEVAD_NONE, 0x1d, 0x05);
-	/* enable rgmii tx clock delay */
-	/* set the reserved bits to avoid board specific voltage peak issue*/
-	phy_write(phydev, MDIO_DEVAD_NONE, 0x1e, 0x3D47);
-
-	return 0;
-}
-
 int board_phy_config(struct phy_device *phydev)
 {
-	mx6_rgmii_rework(phydev);
+	/*
+	 * Set reserved bits to avoid board specific voltage peak issue. The
+	 * value is a magic number provided directly by Qualcomm. Note, that
+	 * PHY driver will take control of BIT(8) in this register to control
+	 * TX clock delay, so we do not initialize that bit here.
+	 */
+	phy_write(phydev, MDIO_DEVAD_NONE, AR8033_DBG_REG_ADDR, AR8033_SERDES_REG);
+	phy_write(phydev, MDIO_DEVAD_NONE, AR8033_DBG_REG_DATA, 0x3c47);
 
 	if (phydev->drv->config)
 		phydev->drv->config(phydev);
-- 
2.29.2

[PATCHv2 5/5] board: ge: bx50v3: cleanup phy config

[sbabic at denx.de]

> The current PHY rework does the following things:
> 1. Configure 125MHz clock
> 2. Setup the TX clock delay (RX is enabled by default),
> 3. Setup reserved bits to avoid voltage peak
> The clock delays are nowadays already configured by the
> PHY driver (in ar803x_delay_config). The code for that
> can simply be dropped. The clock speed can also be
> configured by the PHY driver by adding the device tree
> property "qca,clk-out-frequency".
> What is left is setting up the undocumented reserved bits
> to avoid the voltage peak problem. I slightly improved its
> documentation while updating the board's PHY rework code.
> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
Applied to u-boot-imx, master, thanks !

Best regards,
Stefano Babic

-- 
=====================================================================
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================