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

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@sholland.org

With the "else" down below removed:

Reviewed-by: Andre Przywara andre.przywara@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