[PATCH 8/8] Initial Pine64 Pinephone support

[André Przywara]

On 27/07/2020 14:49, Peter Robinson wrote:

Hi,

thanks for piecing this together. As Maxime mentioned, many options are
not necessary (see below).

Some options are now set by ARCH_SUNXI, since 48313fe51008.

>> On Wed, Jul 22, 2020 at 03:18:40PM +0100, Peter Robinson wrote:
>>> The Pine64 Pinephone is a smartphone based on the AllWinner A64 SoC.
>>> It has the following features:
>>> * 2GB LPDDR3 SDRAM
>>> * 5.95 inch 1440x720 HD IPS capacitive touchscreen
>>> * 16GB eMMC, mSD slot
>>> * Quectel EG25 LTE Modem
>>> * Realtek RTL8723CS WiFi/BT
>>> * Front and read cameras
>>> * Accelerometer, gyro, proximity, ambient light, compass sensors
>>> * A USB Type-C, USB Host, DisplayPort alt mode output, 15W 5V 3A Quick Charge, follows USB PD specification
>>>
>>> Signed-off-by: Peter Robinson <pbrobinson at gmail.com>
>>> ---
>>>  arch/arm/Kconfig            |  2 +-
>>>  configs/pinephone_defconfig | 38 +++++++++++++++++++++++++++++++++++++
>>>  2 files changed, 39 insertions(+), 1 deletion(-)
>>>  create mode 100644 configs/pinephone_defconfig
>>>
>>> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
>>> index e16fe03887..636ba26938 100644
>>> --- a/arch/arm/Kconfig
>>> +++ b/arch/arm/Kconfig
>>> @@ -1004,7 +1004,7 @@ config ARCH_SUNXI
>>>       bool "Support sunxi (Allwinner) SoCs"
>>>       select BINMAN
>>>       select CMD_GPIO
>>> -     select CMD_MMC if MMC
>>> +select CMD_MMC if MMC
>>
>> That looks like a typo?
> 
> Yes, it is.
> 
>>>       select CMD_USB if DISTRO_DEFAULTS
>>>       select CLK
>>>       select DM
>>> diff --git a/configs/pinephone_defconfig b/configs/pinephone_defconfig
>>> new file mode 100644
>>> index 0000000000..d5750aa954
>>> --- /dev/null
>>> +++ b/configs/pinephone_defconfig
>>> @@ -0,0 +1,38 @@
>>> +CONFIG_ARM=y
>>> +CONFIG_ARCH_SUNXI=y
>>> +CONFIG_SPL=y
>>> +CONFIG_MACH_SUN50I=y
>>> +CONFIG_SUNXI_DRAM_LPDDR3_STOCK=y
>>> +CONFIG_DRAM_CLK=552
>>> +CONFIG_DRAM_ZQ=3881949
>>> +CONFIG_NR_DRAM_BANKS=1

defaults to 1 now for ARCH_SUNXI

>>> +CONFIG_MMC_SUNXI_SLOT_EXTRA=2
>>> +CONFIG_R_I2C_ENABLE=y
>>> +# CONFIG_SYS_MALLOC_CLEAR_ON_INIT is not set
>>> +# CONFIG_SPL_SPI_SUNXI is not set

defaults to n

>>> +# CONFIG_SPL_DOS_PARTITION is not set
>>> +# CONFIG_SPL_EFI_PARTITION is not set

those two default to "n" now for ARCH_SUNXI

>>> +CONFIG_DEFAULT_DEVICE_TREE="sun50i-a64-pinephone-1.1"
>>> +CONFIG_OF_LIST="sun50i-a64-pinephone-1.1 sun50i-a64-pinephone-1.0"
>>> +CONFIG_SYS_RELOC_GD_ENV_ADDR=y

defaults to y now for ARCH_SUNXI

>>> +CONFIG_DM_REGULATOR=y
>>> +CONFIG_DM_REGULATOR_FIXED=y
>>> +CONFIG_DM_PWM=y
>>> +CONFIG_PWM_SUNXI=y
>>> +CONFIG_CMD_GPIO=y
>>> +CONFIG_CMD_GPT=y

those two are already y be default

>>> +CONFIG_CMD_I2C=y
>>> +CONFIG_CMD_MMC=y

already y by default

>>> +# CONFIG_CMD_MII is not set
>>> +# CONFIG_CMD_NFS is not set
>>> +# CONFIG_DM_ETH is not set
>>> +# CONFIG_PHY is not set
>>> +# CONFIG_PHY_GIGE is not set
>>> +# CONFIG_SUN8I_EMAC is not set
>>> +# CONFIG_PHY_REALTEK is not set
>>> +# CONFIG_CMD_SF is not set
>>> +# CONFIG_SPI is not set
>>> +# CONFIG_DM_SPI is not set

Those two sounds like a good idea, but don't do anything, because
ARCH_SUNXI selects them. Sounds weird, and looks like not the right
thing, tbh.

>>> +# CONFIG_SPI_FLASH is not set
>>> +# CONFIG_SPI_MEM is not set
>>> +# CONFIG_DM_SPI_FLASH is not set
>>
>> I'm not entirely sure why we need to deviate from the default that much
>> here. Some options should definitely be disabled (like SUN8I_EMAC), but
>> I'm not really sure why it's enabled in the first place, and why we need
> 
> Because it's selected by default for the SoC,

Mmmh, really? I don't see that in Kconfig, and in fact removing that
"#CONFIG_SUN8I_EMAC is not set" line results in the exact same .config
created.

> I suspect that's because
> it was assumed that everything would be a dev board and have a wired
> ethernet port but there's more and more devices that don't so it
> probably makes sense to review that but I didn't want to do that as
> part of enabling a device.

Explicitly un-setting just CONFIG_NET should disable all network related
options. This disables even more commands and features than your
version, which is fine, unless we want USB Ethernet support. Doesn't
seem to be enabled with your config, and I don't know what's the USB
story on the Pinephone in general, and in U-Boot in particular.
What are the expectations in this regard?

Cheers,
Andre

>> to disable the other network related options (PHY, PHY_GIGE,
>> PHY_REALTEK). They shouldn't even be enabled in the first place.
> 
> That would be my guess but I suspect it's the same as the SUN8I_EMAC above.
> 
>> Similarly, CMD_GPIO, CMD_GPT should be enabled by default.
>>
>> (and I'm not sure why PWM_SUNXI is here in the first place?)
> 
> It's used by the backlight, it shouldn't have made it into this patch,
> as the DT bits aren't upstream yet and as I mentioned in my overview
> the screen side of things is still a WiP, I must have missed it as I
> sliced up my patch set between bits that worked with upstream and bits
> that still need some work before they go upstream.
> 
> Peter
>