[PATCH 4/8] sunxi: board: Add PinePhone DT selection logic

[André Przywara]

On 03/09/2020 06:07, Samuel Holland wrote:

Hi,

> There are two different publicly-released revisions of the PinePhone
> hardware, versions 1.1 and 1.2; and they need different device trees.
> Since some GPIO pins were rerouted, we can use that to distinguish
> between them.

Nice one. I once had a similar solution to differentiate between the
(otherwise very similar) Pinebook and Pine64-LTS.

> 
> Signed-off-by: Samuel Holland <samuel at sholland.org>

With the "else" down below removed:

Reviewed-by: Andre Przywara <andre.przywara at arm.com>

> ---
>  arch/arm/mach-sunxi/Kconfig |  7 +++++++
>  board/sunxi/board.c         | 21 +++++++++++++++++++++
>  2 files changed, 28 insertions(+)
> 
> diff --git a/arch/arm/mach-sunxi/Kconfig b/arch/arm/mach-sunxi/Kconfig
> index be0822bfb7d..8421f3b6854 100644
> --- a/arch/arm/mach-sunxi/Kconfig
> +++ b/arch/arm/mach-sunxi/Kconfig
> @@ -1010,4 +1010,11 @@ config PINE64_DT_SELECTION
>  	  option, the device tree selection code specific to Pine64 which
>  	  utilizes the DRAM size will be enabled.
>  
> +config PINEPHONE_DT_SELECTION
> +	bool "Enable PinePhone device tree selection code"
> +	depends on MACH_SUN50I
> +	help
> +	  Enable this option to automatically select the device tree for the
> +	  correct PinePhone hardware revision during boot.
> +
>  endif
> diff --git a/board/sunxi/board.c b/board/sunxi/board.c
> index fb0d5bf4743..3d64ed18664 100644
> --- a/board/sunxi/board.c
> +++ b/board/sunxi/board.c
> @@ -27,6 +27,7 @@
>  #include <asm/arch/dram.h>
>  #include <asm/arch/gpio.h>
>  #include <asm/arch/mmc.h>
> +#include <asm/arch/prcm.h>
>  #include <asm/arch/spl.h>
>  #include <linux/delay.h>
>  #include <u-boot/crc.h>
> @@ -920,6 +921,26 @@ int board_fit_config_name_match(const char *name)
>  			best_dt_name = "sun50i-a64-pine64";
>  	}
>  #endif
> +#ifdef CONFIG_PINEPHONE_DT_SELECTION
> +	else if (strstr(best_dt_name, "-pinephone")) {

I think to improve readability and increase robustness against future
changes you can lose the "else" here. Even if both selection methods
should be selected, only one will realistically match the strstr()
comparison.

> +		/* Differentiate the PinePhone revisions by GPIO inputs. */
> +		prcm_apb0_enable(PRCM_APB0_GATE_PIO);
> +		sunxi_gpio_set_pull(SUNXI_GPL(6), SUNXI_GPIO_PULL_UP);
> +		sunxi_gpio_set_cfgpin(SUNXI_GPL(6), SUNXI_GPIO_INPUT);
> +		udelay(100);
> +
> +		/* PL6 is pulled low by the modem on v1.2. */
> +		if (gpio_get_value(SUNXI_GPL(6)) == 0)
> +			best_dt_name = "sun50i-a64-pinephone-1.2";
> +		else
> +			best_dt_name = "sun50i-a64-pinephone-1.1";
> +
> +		sunxi_gpio_set_cfgpin(SUNXI_GPL(6), SUNXI_GPIO_DISABLE);
> +		sunxi_gpio_set_pull(SUNXI_GPL(6), SUNXI_GPIO_PULL_DISABLE);
> +		prcm_apb0_disable(PRCM_APB0_GATE_PIO);

Looking forward, this should probably restore the former state, in case
some code elsewhere had enabled the PIO gate already. But for now, with
the current code state, this is fine.

Cheers,
Andre

> +	}
> +#endif
> +
>  	return strcmp(name, best_dt_name);
>  }
>  #endif
>