| Message ID | 20200903050716.48488-5-samuel@sholland.org |
|---|---|
| State | Changes Requested |
| Delegated to: | Jagannadha Sutradharudu Teki |
| Headers | show |
| Series | PinePhone automatic device tree selection | expand |
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 >
On Thu, Sep 3, 2020 at 10:37 AM Samuel Holland <samuel@sholland.org> wrote: > > 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. > > Signed-off-by: Samuel Holland <samuel@sholland.org> > --- > 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 Look like these board detection CONFIG items are keep on increasing. Better to have something like CONFIG_SUNXI_DT_SELECTION for all dt selection code.
On 10/21/20 1:56 PM, Jagan Teki wrote: > On Thu, Sep 3, 2020 at 10:37 AM Samuel Holland <samuel@sholland.org> wrote: >> >> 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. >> >> Signed-off-by: Samuel Holland <samuel@sholland.org> >> --- >> 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 > > Look like these board detection CONFIG items are keep on increasing. > Better to have something like CONFIG_SUNXI_DT_SELECTION for all dt > selection code. Are you sure? This is in SPL, where we are always running out of space. And a single SPL binary cannot work on both Pine A64 and PinePhone anyway, since they have different DRAM types. I think the space savings is worth the cost of the extra config symbol (especially if more boards need special handling in the future). Cheers, Samuel
On Thu, Oct 22, 2020 at 7:08 AM Samuel Holland <samuel@sholland.org> wrote: > > On 10/21/20 1:56 PM, Jagan Teki wrote: > > On Thu, Sep 3, 2020 at 10:37 AM Samuel Holland <samuel@sholland.org> wrote: > >> > >> 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. > >> > >> Signed-off-by: Samuel Holland <samuel@sholland.org> > >> --- > >> 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 > > > > Look like these board detection CONFIG items are keep on increasing. > > Better to have something like CONFIG_SUNXI_DT_SELECTION for all dt > > selection code. > > Are you sure? This is in SPL, where we are always running out of space. And a > single SPL binary cannot work on both Pine A64 and PinePhone anyway, since they > have different DRAM types. I think the space savings is worth the cost of the > extra config symbol (especially if more boards need special handling in the future). Does marking 'dt selection only' code as a single config symbol is increasing the SPL size? Jagan.
On Wed, Oct 21, 2020 at 08:38:21PM -0500, Samuel Holland wrote: > On 10/21/20 1:56 PM, Jagan Teki wrote: > > On Thu, Sep 3, 2020 at 10:37 AM Samuel Holland <samuel@sholland.org> wrote: > >> > >> 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. > >> > >> Signed-off-by: Samuel Holland <samuel@sholland.org> > >> --- > >> 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 > > > > Look like these board detection CONFIG items are keep on increasing. > > Better to have something like CONFIG_SUNXI_DT_SELECTION for all dt > > selection code. > > Are you sure? This is in SPL, where we are always running out of space. And a > single SPL binary cannot work on both Pine A64 and PinePhone anyway, since they > have different DRAM types. I think the space savings is worth the cost of the > extra config symbol (especially if more boards need special handling in the future). I agree, it will save some space and it's not like it's unmaintainable at the moment. Maxime
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")) { + /* 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); + } +#endif + return strcmp(name, best_dt_name); } #endif
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. Signed-off-by: Samuel Holland <samuel@sholland.org> --- arch/arm/mach-sunxi/Kconfig | 7 +++++++ board/sunxi/board.c | 21 +++++++++++++++++++++ 2 files changed, 28 insertions(+)