[U-Boot] [PATCH v2 3/4] odroid: make some macros common

[Lukasz Majewski <l.majewski@samsung.com>]

Hi Hyungwon,

> Some macros are used commonly for odroid series boards. This patch
> makes a common header file to congregate that kinds of macros. Even
> though there are more macros which can be common, they are not become
> common. Because they are a part of a register, the readability is
> better when they are defined at a place.
> 
> Signed-off-by: Hyungwon Hwang <human.hwang@samsung.com>
> ---
>  board/samsung/odroid/odroid.c | 1 +
>  board/samsung/odroid/setup.h  | 8 --------
>  2 files changed, 1 insertion(+), 8 deletions(-)
> 

I suspect that you have not added the new file to git repository - since
you only removed lines from board/samsung/odroid/setup.h

I also guess that odroid U3 will not build anymore. That is a very good
use case for buildman script.

> diff --git a/board/samsung/odroid/odroid.c
> b/board/samsung/odroid/odroid.c index 5edb250..ccbb3a0 100644
> --- a/board/samsung/odroid/odroid.c
> +++ b/board/samsung/odroid/odroid.c
> @@ -18,6 +18,7 @@
>  #include <usb.h>
>  #include <usb/s3c_udc.h>
>  #include <samsung/misc.h>
> +#include "../setup.h"

Relative path is not a good idea IMHO.

It would be better to place it at ./include/samsung/ with a self
explanatory name (like exynos4-pll.h or/and exynos4-{other excluded
defines for an IP blocks}).

In this way other boards could use those defines too.

>  #include "setup.h"
>  
>  DECLARE_GLOBAL_DATA_PTR;
> diff --git a/board/samsung/odroid/setup.h
> b/board/samsung/odroid/setup.h index 3e48dad..35f7af5 100644
> --- a/board/samsung/odroid/setup.h
> +++ b/board/samsung/odroid/setup.h
> @@ -8,14 +8,6 @@
>  #ifndef __ODROIDU3_SETUP__
>  #define __ODROIDU3_SETUP__
>  
> -/* A/M PLL_CON0 */
> -#define SDIV(x)                 ((x) & 0x7)
> -#define PDIV(x)                 (((x) & 0x3f) << 8)
> -#define MDIV(x)                 (((x) & 0x3ff) << 16)
> -#define FSEL(x)                 (((x) & 0x1) << 27)
> -#define PLL_LOCKED_BIT          (0x1 << 29)
> -#define PLL_ENABLE(x)           (((x) & 0x1) << 31)
> -

The above data is common for Exynos U3 and XU3, but is it the only one?
Aren't there any more defines to be extracted?

>  /* CLK_SRC_CPU */
>  #define MUX_APLL_SEL(x)         ((x) & 0x1)
>  #define MUX_CORE_SEL(x)         (((x) & 0x1) << 16)



-- 
Best regards,

Lukasz Majewski

Samsung R&D Institute Poland (SRPOL) | Linux Platform Group