[PATCH v3 03/10] arm: mvebu: clearfog: initial ClearFog Base variant

[Baruch Siach]

Hi Joel,

On Tue, Jan 21, 2020 at 10:32:17AM -0700, Joel Johnson wrote:
> Add a unique entry for ClearFog Base variant, reflected in the board
> name and adjusted SerDes topology.
> 
> Signed-off-by: Joel Johnson <mrjoel at lixil.net>
> 
> ---
> 
> v2 changes:
>   - reworked based on Baruch's run-time TLV EEPROM detection series
> v3 changes:
>   - rebased on mvebu merged run-time TLV EEPROM detection series
>   - minor update to help test regarding runtime detection failures
> 
> ---
>  arch/arm/mach-mvebu/Kconfig        |  2 ++
>  board/solidrun/clearfog/Kconfig    | 18 ++++++++++++++++++
>  board/solidrun/clearfog/clearfog.c | 12 ++++++++++++
>  3 files changed, 32 insertions(+)
>  create mode 100644 board/solidrun/clearfog/Kconfig
> 
> diff --git a/arch/arm/mach-mvebu/Kconfig b/arch/arm/mach-mvebu/Kconfig
> index bc5eaa5a76..161dee937f 100644
> --- a/arch/arm/mach-mvebu/Kconfig
> +++ b/arch/arm/mach-mvebu/Kconfig
> @@ -280,4 +280,6 @@ config SECURED_MODE_CSK_INDEX
>  	default 0
>  	depends on SECURED_MODE_IMAGE
>  
> +source "board/solidrun/clearfog/Kconfig"
> +
>  endif
> diff --git a/board/solidrun/clearfog/Kconfig b/board/solidrun/clearfog/Kconfig
> new file mode 100644
> index 0000000000..936d5918f8
> --- /dev/null
> +++ b/board/solidrun/clearfog/Kconfig
> @@ -0,0 +1,18 @@
> +menu "ClearFog configuration"
> +	depends on TARGET_CLEARFOG
> +
> +config TARGET_CLEARFOG_BASE
> +	bool "Use ClearFog Base static configuration"
> +	help
> +	  Use the ClearFog Base as the static configuration instead of the
> +	  default which uses the ClearFog Pro.
> +
> +	  Runtime board detection is always attempted and used if available. The
> +	  static configuration is used as a fallback in cases where runtime
> +	  detection is disabled, is not available in hardware, or otherwise fails.
> +
> +	  Only newer revisions of the ClearFog product line support runtime
> +	  detection via additional EEPROM hardware. This option enables selecting
> +	  the Base variant for older hardware revisions.
> +
> +endmenu
> diff --git a/board/solidrun/clearfog/clearfog.c b/board/solidrun/clearfog/clearfog.c
> index e268ef55a2..e77b9465d4 100644
> --- a/board/solidrun/clearfog/clearfog.c
> +++ b/board/solidrun/clearfog/clearfog.c
> @@ -47,7 +47,11 @@ static struct serdes_map board_serdes_map[] = {
>  	{SGMII1, SERDES_SPEED_1_25_GBPS, SERDES_DEFAULT_MODE, 0, 0},
>  	{PEX1, SERDES_SPEED_5_GBPS, PEX_ROOT_COMPLEX_X1, 0, 0},
>  	{USB3_HOST1, SERDES_SPEED_5_GBPS, SERDES_DEFAULT_MODE, 0, 0},
> +#if defined (CONFIG_TARGET_CLEARFOG_BASE)
> +	{USB3_HOST0, SERDES_SPEED_5_GBPS, SERDES_DEFAULT_MODE, 0, 0},
> +#else
>  	{PEX2, SERDES_SPEED_5_GBPS, PEX_ROOT_COMPLEX_X1, 0, 0},
> +#endif

I'd prefer run-time test instead of '#ifdefs' that IMO make the code harder to 
read. Something like this (build tested only):

diff --git a/board/solidrun/clearfog/clearfog.c b/board/solidrun/clearfog/clearfog.c
index e268ef55a2a0..202c60cb7841 100644
--- a/board/solidrun/clearfog/clearfog.c
+++ b/board/solidrun/clearfog/clearfog.c
@@ -55,7 +55,8 @@ int hws_board_topology_load(struct serdes_map **serdes_map_array, u8 *count)
 {
 	cf_read_tlv_data();
 
-	if (sr_product_is(&cf_tlv_data, "Clearfog GTR")) {
+	if (sr_product_is(&cf_tlv_data, "Clearfog GTR") ||
+			CONFIG_IS_ENABLED(TARGET_CLEARFOG_BASE)) {
 		board_serdes_map[0].serdes_type = PEX0;
 		board_serdes_map[0].serdes_speed = SERDES_SPEED_5_GBPS;
 		board_serdes_map[0].serdes_mode = PEX_ROOT_COMPLEX_X1;
@@ -172,6 +173,9 @@ int checkboard(void)
 {
 	char *board = "ClearFog";
 
+	if (CONFIG_IS_ENABLED(TARGET_CLEARFOG_BASE))
+		board = "ClearFog Base";
+
 	cf_read_tlv_data();
 	if (strlen(cf_tlv_data.tlv_product_name[0]) > 0)
 		board = cf_tlv_data.tlv_product_name[0];
@@ -194,7 +198,8 @@ int board_late_init(void)
 {
 	cf_read_tlv_data();
 
-	if (sr_product_is(&cf_tlv_data, "Clearfog Base"))
+	if (sr_product_is(&cf_tlv_data, "Clearfog Base") ||
+			CONFIG_IS_ENABLED(TARGET_CLEARFOG_BASE))
 		env_set("fdtfile", "armada-388-clearfog-base.dtb");
 	else if (sr_product_is(&cf_tlv_data, "Clearfog GTR S4"))
 		env_set("fdtfile", "armada-385-clearfog-gtr-s4.dtb");

What do you think?

baruch

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