| Message ID | 20200722141840.143379-9-pbrobinson@gmail.com |
|---|---|
| State | Rejected |
| Delegated to: | Jagannadha Sutradharudu Teki |
| Headers | show |
| Series | [1/8] dt-bindings: clk: sync sun50i-a64-ccu.h to linux 5.8-rc1 | expand |
Hi, 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@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? > 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 > +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 > +# CONFIG_SPL_DOS_PARTITION is not set > +# CONFIG_SPL_EFI_PARTITION is not set > +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 > +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 > +CONFIG_CMD_I2C=y > +CONFIG_CMD_MMC=y > +# 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 > +# 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 to disable the other network related options (PHY, PHY_GIGE, PHY_REALTEK). They shouldn't even be enabled in the first place. Similarly, CMD_GPIO, CMD_GPT should be enabled by default. (and I'm not sure why PWM_SUNXI is here in the first place?) Maxime
> 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@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 > > +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 > > +# CONFIG_SPL_DOS_PARTITION is not set > > +# CONFIG_SPL_EFI_PARTITION is not set > > +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 > > +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 > > +CONFIG_CMD_I2C=y > > +CONFIG_CMD_MMC=y > > +# 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 > > +# 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, 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. > 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
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@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 >
On Mon, Jul 27, 2020 at 09:08:40PM +0100, André Przywara wrote: > 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@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 So really, one needs to run `make savedefconfig`, copy defconfig to configs/foo_defconfig and then see if there's some changes / settings there that are unexpected. Because if you put a change in there, and then run `make foo_defconfig` and don't see what you want in the new .config file, there's a missing dependency or similar. Thanks!
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 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 +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 +# CONFIG_SPL_DOS_PARTITION is not set +# CONFIG_SPL_EFI_PARTITION is not set +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 +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 +CONFIG_CMD_I2C=y +CONFIG_CMD_MMC=y +# 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 +# CONFIG_SPI_FLASH is not set +# CONFIG_SPI_MEM is not set +# CONFIG_DM_SPI_FLASH is not set
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@gmail.com> --- arch/arm/Kconfig | 2 +- configs/pinephone_defconfig | 38 +++++++++++++++++++++++++++++++++++++ 2 files changed, 39 insertions(+), 1 deletion(-) create mode 100644 configs/pinephone_defconfig