Hi,

First if all, thanks for the patches for this. I've a couple of comments on a few of them (including this one) I'll reply to the one I've comments on. no reply means I think it is fine :)

On 09/18/2015 08:06 AM, Maxime Ripard wrote:

Some devices don't have any MMC devices, so it doesn't really make sense to enable the MMC related functions and options for them.

Add an option to disable the MMC support entirely.

Signed-off-by: Maxime Ripard maxime.ripard@free-electrons.com

arch/arm/cpu/armv7/sunxi/board.c | 6 ++++++ include/configs/sunxi-common.h | 7 +++++-- 2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/arch/arm/cpu/armv7/sunxi/board.c b/arch/arm/cpu/armv7/sunxi/board.c index b40198b36ee5..e6721feb4a4c 100644 --- a/arch/arm/cpu/armv7/sunxi/board.c +++ b/arch/arm/cpu/armv7/sunxi/board.c @@ -135,7 +135,9 @@ DECLARE_GLOBAL_DATA_PTR; */ u32 spl_boot_device(void) { +#ifdef CONFIG_MMC struct mmc *mmc0, *mmc1; +#endif /* * When booting from the SD card or NAND memory, the "eGON.BT0" * signature is expected to be found in memory at the address 0x0004 @@ -156,15 +158,18 @@ u32 spl_boot_device(void) return BOOT_DEVICE_BOARD;

/* The BROM will try to boot from mmc0 first, so try that first. */ +#ifdef CONFIG_MMC mmc_initialize(gd->bd); mmc0 = find_mmc_device(0); if (sunxi_mmc_has_egon_boot_signature(mmc0)) return BOOT_DEVICE_MMC1; +#endif

/* Fallback to booting NAND if enabled. */ if (IS_ENABLED(CONFIG_SPL_NAND_SUPPORT)) return BOOT_DEVICE_NAND;

+#ifdef CONFIG_MMC if (CONFIG_MMC_SUNXI_SLOT_EXTRA == 2) { mmc1 = find_mmc_device(1); if (sunxi_mmc_has_egon_boot_signature(mmc1)) { @@ -178,6 +183,7 @@ u32 spl_boot_device(void) return BOOT_DEVICE_MMC2; } } +#endif

panic("Could not determine boot source\n"); return -1; /* Never reached */ diff --git a/include/configs/sunxi-common.h b/include/configs/sunxi-common.h index 48cc4ed6f629..4fde0d4371e4 100644 --- a/include/configs/sunxi-common.h +++ b/include/configs/sunxi-common.h @@ -142,7 +142,7 @@ #endif

/* mmc config */ -#if !defined(CONFIG_UART0_PORT_F) +#if !defined(CONFIG_UART0_PORT_F) && !defined(CONFIG_NO_MMC)

You're introducing a CONFIG_NO_MMC here, and setting that using EXTRA_OPTIONS. EXTRA_OPTIONS has been deprecated, and adding new options there is something which we do not want to do.

Instead I think it would be better to Kconfig-ify CONFIG_MMC, the problem with doing this is that CONFIG_MMC gets used by almost all SoCs supported by u-boot, and moving all of them to use Kconfig for this at once is not ideal.

What u-boot has been doing so far for this is using a construct like this:

config SYS_CLK_FREQ depends on ARC || ARCH_SUNXI int "CPU clock frequency" help TODO: Move CONFIG_SYS_CLK_FREQ for all the architecture

So for mmc we would get something like this in drivers/mmc/Kconfig:

config MMC depends on ARCH_SUNXI bool "Enable MMC support" help TODO: Move CONFIG_MMC for all the architecture

And in board/sunxi/Kconfig

config MMC default y if ARCH_SUNXI && !UART0_PORT_F

We need the if ARCH_SUNXI to make "make savedefconfig" not cry out on non SUNXI archs.

And then in sunxi-common.h we can just do:

#ifdef CONFIG_MMC #define CONFIG_GENERIC_MMC ...

#define CONFIG_MMC #define CONFIG_GENERIC_MMC #define CONFIG_CMD_MMC @@ -199,7 +199,7 @@

#define CONFIG_SPL_LIBDISK_SUPPORT

-#if !defined(CONFIG_UART0_PORT_F) +#if !defined(CONFIG_UART0_PORT_F) && !defined(CONFIG_NO_MMC) #define CONFIG_SPL_MMC_SUPPORT #endif

And this would become #ifdef CONFIG_MMC too

@@ -360,9 +360,12 @@ extern int soft_i2c_gpio_scl; #define CONFIG_FASTBOOT_BUF_SIZE 0x2000000

#define CONFIG_FASTBOOT_FLASH

+#ifdef CONFIG_MMC #define CONFIG_FASTBOOT_FLASH_MMC_DEV 0 #define CONFIG_EFI_PARTITION #endif +#endif

#ifdef CONFIG_USB_FUNCTION_MASS_STORAGE #define CONFIG_CMD_USB_MASS_STORAGE

Regards,

Hans

On Fri, Sep 18, 2015 at 02:06:15PM +0200, Maxime Ripard wrote:

Some devices don't have any MMC devices, so it doesn't really make sense to enable the MMC related functions and options for them.

Add an option to disable the MMC support entirely.

Signed-off-by: Maxime Ripard maxime.ripard@free-electrons.com

arch/arm/cpu/armv7/sunxi/board.c | 6 ++++++ include/configs/sunxi-common.h | 7 +++++-- 2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/arch/arm/cpu/armv7/sunxi/board.c b/arch/arm/cpu/armv7/sunxi/board.c index b40198b36ee5..e6721feb4a4c 100644 --- a/arch/arm/cpu/armv7/sunxi/board.c +++ b/arch/arm/cpu/armv7/sunxi/board.c @@ -135,7 +135,9 @@ DECLARE_GLOBAL_DATA_PTR; */ u32 spl_boot_device(void) { +#ifdef CONFIG_MMC struct mmc *mmc0, *mmc1; +#endif /* * When booting from the SD card or NAND memory, the "eGON.BT0" * signature is expected to be found in memory at the address 0x0004

Just __maybe_unused here, if that's fine with Hans.

On Fri, Sep 18, 2015 at 11:02:27AM -0400, Tom Rini wrote:

On Fri, Sep 18, 2015 at 02:06:15PM +0200, Maxime Ripard wrote:

...

Some devices don't have any MMC devices, so it doesn't really make sense to enable the MMC related functions and options for them.

Add an option to disable the MMC support entirely.

Signed-off-by: Maxime Ripard maxime.ripard@free-electrons.com

arch/arm/cpu/armv7/sunxi/board.c | 6 ++++++ include/configs/sunxi-common.h | 7 +++++-- 2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/arch/arm/cpu/armv7/sunxi/board.c b/arch/arm/cpu/armv7/sunxi/board.c index b40198b36ee5..e6721feb4a4c 100644 --- a/arch/arm/cpu/armv7/sunxi/board.c +++ b/arch/arm/cpu/armv7/sunxi/board.c @@ -135,7 +135,9 @@ DECLARE_GLOBAL_DATA_PTR; */ u32 spl_boot_device(void) { +#ifdef CONFIG_MMC struct mmc *mmc0, *mmc1; +#endif /* * When booting from the SD card or NAND memory, the "eGON.BT0" * signature is expected to be found in memory at the address 0x0004

Just __maybe_unused here, if that's fine with Hans.

Ack.

Thanks! Maxime

Hi,

On 09/18/2015 08:06 AM, Maxime Ripard wrote:

The current fastboot support assumes that CONFIG_FASTBOOT_FLASH implies that we have an MMC in our system, which might not be the case if we have some other storage device.

Change the configuration option protecting that call to FASTBOOT_FLASH_MMC_DEV, that makes much more sense.

Signed-off-by: Maxime Ripard maxime.ripard@free-electrons.com

Note I'm not going to take this one upstream through the sunxi tree, other then that it looks fine to me.

Regards,

Hans

---

drivers/usb/gadget/f_fastboot.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/f_fastboot.c b/drivers/usb/gadget/f_fastboot.c index ca01a018b5d1..ece48e668c96 100644 --- a/drivers/usb/gadget/f_fastboot.c +++ b/drivers/usb/gadget/f_fastboot.c @@ -554,7 +554,7 @@ static void cb_flash(struct usb_ep *ep, struct usb_request *req) static void cb_oem(struct usb_ep *ep, struct usb_request *req) { char *cmd = req->buf; -#ifdef CONFIG_FASTBOOT_FLASH +#ifdef CONFIG_FASTBOOT_FLASH_MMC_DEV if (strncmp("format", cmd + 4, 6) == 0) { char cmdbuf[32]; sprintf(cmdbuf, "gpt write mmc %x $partitions",

Hi,

On 09/18/2015 08:06 AM, Maxime Ripard wrote:

The C.H.I.P. is a small SBC with an Allwinner R8, 8GB of NAND, 512MB of RAM, USB host and OTG, a wifi / bluetooth combo chip, an audio/video jack and two connectors to plug additional boards on top of it.

Signed-off-by: Maxime Ripard maxime.ripard@free-electrons.com

arch/arm/dts/Makefile | 3 +- arch/arm/dts/sun5i-r8-chip.dts | 261 +++++++++++++++++++++++++++++++++++++++++ configs/CHIP_defconfig | 14 +++ 3 files changed, 277 insertions(+), 1 deletion(-) create mode 100644 arch/arm/dts/sun5i-r8-chip.dts create mode 100644 configs/CHIP_defconfig

diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile index 0a8c88de9e32..4d09e3de71f9 100644 --- a/arch/arm/dts/Makefile +++ b/arch/arm/dts/Makefile @@ -111,7 +111,8 @@ dtb-$(CONFIG_MACH_SUN5I) += \ sun5i-a13-olinuxino-micro.dtb \ sun5i-a13-q8-tablet.dtb \ sun5i-a13-tzx-q8-713b7.dtb \

• sun5i-a13-utoo-p66.dtb

• sun5i-a13-utoo-p66.dtb \
• sun5i-r8-chip.dtb dtb-$(CONFIG_MACH_SUN6I) += \ sun6i-a31-app4-evb1.dtb \ sun6i-a31-colombus.dtb \

diff --git a/arch/arm/dts/sun5i-r8-chip.dts b/arch/arm/dts/sun5i-r8-chip.dts new file mode 100644 index 000000000000..6cb3c4f1cd61 --- /dev/null +++ b/arch/arm/dts/sun5i-r8-chip.dts @@ -0,0 +1,261 @@ +/*

• • Copyright 2015 Free Electrons
• • Copyright 2015 NextThing Co
• • Maxime Ripard maxime.ripard@free-electrons.com
• • This file is dual-licensed: you can use it either under the terms
• • of the GPL or the X11 license, at your option. Note that this dual
• • licensing only applies to this file, and not this project as a
• • whole.
• • a) This file is free software; you can redistribute it and/or

• • modify it under the terms of the GNU General Public License as
    

• • published by the Free Software Foundation; either version 2 of the
    

• • License, or (at your option) any later version.
    

• • This file is distributed in the hope that it will be useful,
    

• • but WITHOUT ANY WARRANTY; without even the implied warranty of
    

• • MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
    

• • GNU General Public License for more details.
    

• • Or, alternatively,
• • b) Permission is hereby granted, free of charge, to any person

• • obtaining a copy of this software and associated documentation
    

• • files (the "Software"), to deal in the Software without
    

• • restriction, including without limitation the rights to use,
    

• • copy, modify, merge, publish, distribute, sublicense, and/or
    

• • sell copies of the Software, and to permit persons to whom the
    

• • Software is furnished to do so, subject to the following
    

• • conditions:
    

• • The above copyright notice and this permission notice shall be
    

• • included in all copies or substantial portions of the Software.
    

• • THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
    

• • EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
    

• • OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
    

• • NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
    

• • HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
    

• • WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
    

• • FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
    

• • OTHER DEALINGS IN THE SOFTWARE.
    

• */

+/dts-v1/; +#include "sun5i-a13.dtsi"

+#include <dt-bindings/gpio/gpio.h> +#include <dt-bindings/interrupt-controller/irq.h>

+/ {

• model = "NextThing C.H.I.P.";
• compatible = "nextthing,chip", "allwinner,sun5i-r8";
• aliases {

• i2c0 = &i2c0;
  

• i2c1 = &i2c1;
  

• i2c2 = &i2c2;
  

• serial0 = &uart1;
  

• serial1 = &uart3;
  

• };
• chosen {

• stdout-path = "serial0:115200n8";
  

• };
• dram_vcc: dram_vcc {

• compatible = "regulator-fixed";
  

• regulator-name = "dram-vcc";
  

• regulator-min-microvolt = <1600000>;
  

• regulator-max-microvolt = <1600000>;
  

• vin-supply = <&ipsout>;
  

• regulator-always-on;
  

• };
• ipsout: ipsout {

• compatible = "regulator-fixed";
  

• regulator-name = "ipsout";
  

• regulator-min-microvolt = <5000000>;
  

• regulator-max-microvolt = <5000000>;
  

• regulator-always-on;
  

• };
• usb0_vbus: usb0_vbus {

• compatible = "regulator-fixed";
  

• pinctrl-names = "default";
  

• pinctrl-0 = <&chip_vbus_pin>;
  

• regulator-name = "vbus-usb0";
  

• regulator-min-microvolt = <5000000>;
  

• regulator-max-microvolt = <5000000>;
  

• vin-supply = <&vcc_5v0>;
  

• gpio = <&pio 1 10 GPIO_ACTIVE_HIGH>; /* PB10 */
  

• enable-active-high;
  

• };
• vcc_3v3: vcc3v3 {

• compatible = "regulator-fixed";
  

• regulator-name = "vcc-3v3";
  

• regulator-min-microvolt = <3300000>;
  

• regulator-max-microvolt = <3300000>;
  

• vin-supply = <&ipsout>;
  

• regulator-always-on;
  

• };
• vcc_5v0: vcc5v0 {

• compatible = "regulator-fixed";
  

• regulator-name = "vcc-5v0";
  

• regulator-min-microvolt = <5000000>;
  

• regulator-max-microvolt = <5000000>;
  

• vin-supply = <&ipsout>;
  

• regulator-always-on;
  

• };

+};

+&ehci0 {

• status = "okay";

+};

+&i2c0 {

• pinctrl-names = "default";
• pinctrl-0 = <&i2c0_pins_a>;
• status = "okay";
• axp209: pmic@34 {

• reg = <0x34>;
  

• /*
  

•  * The interrupt is routed through the "External Fast
  

•             * Interrupt Request" pin (ball G13 of the module)
  

•             * directly to the main interrupt controller, without
  

•             * any other controller interfering.
  

•  */
  

• interrupts = <0>;
  

• };

+};

+#include "axp209.dtsi"

+/*

• • i2c1 is routed to the external pins and doesn't have any device
• • attached to it on the C.H.I.P itself.
• */

+&i2c1 {

• pinctrl-names = "default";
• pinctrl-0 = <&i2c1_pins_a>;
• status = "okay";

+};

+&i2c2 {

• pinctrl-names = "default";
• pinctrl-0 = <&i2c2_pins_a>;
• status = "okay";
• xio: gpio@38 {

• compatible = "nxp,pcf8574a";
  

• reg = <0x38>;
  

• gpio-controller;
  

• #gpio-cells = <2>;
  

• interrupt-parent = <&pio>;
  

• interrupts = <6 0 IRQ_TYPE_EDGE_FALLING>;
  

• interrupt-controller;
  

• #interrupt-cells = <2>;
  

• };

+};

+&mmc0 {

• pinctrl-names = "default";
• pinctrl-0 = <&mmc0_pins_a>;
• vmmc-supply = <&vcc_3v3>;
• bus-width = <4>;
• non-removable;
• status = "okay";

+};

+&ohci0 {

• status = "okay";

+};

+&otg_sram {

• status = "okay";

+};

+&pio {

• chip_vbus_pin: chip_vbus_pin@0 {

•        allwinner,pins = "PB10";
  

•        allwinner,function = "gpio_out";
  

•        allwinner,drive = <SUN4I_PINCTRL_10_MA>;
  

•        allwinner,pull = <SUN4I_PINCTRL_NO_PULL>;
  

• };
• chip_id_det_pin: chip_id_det_pin@0 {

•        allwinner,pins = "PG2";
  

•        allwinner,function = "gpio_in";
  

•        allwinner,drive = <SUN4I_PINCTRL_10_MA>;
  

•        allwinner,pull = <SUN4I_PINCTRL_NO_PULL>;
  

• };

+};

+&reg_dcdc2 {

• regulator-min-microvolt = <1000000>;
• regulator-max-microvolt = <1400000>;
• regulator-name = "cpuvdd";
• regulator-always-on;

+};

+&reg_dcdc3 {

• regulator-min-microvolt = <1000000>;
• regulator-max-microvolt = <1300000>;
• regulator-name = "corevdd";
• regulator-always-on;

+};

+&reg_ldo1 {

• regulator-name = "rtcvdd";

+};

+&reg_ldo2 {

• regulator-min-microvolt = <2700000>;
• regulator-max-microvolt = <3300000>;
• regulator-name = "avcc";
• regulator-always-on;

+};

+&reg_ldo5 {

• regulator-min-microvolt = <1800000>;
• regulator-max-microvolt = <1800000>;
• regulator-name = "vcc-1v8";

+};

+&uart1 {

• pinctrl-names = "default";
• pinctrl-0 = <&uart1_pins_b>;
• status = "okay";

+};

+&uart3 {

• pinctrl-names = "default";
• pinctrl-0 = <&uart3_pins_a>,

•     <&uart3_pins_cts_rts_a>;
  

• status = "okay";

+};

+&usb_otg {

• dr_mode = "otg";
• status = "okay";

+};

+&usbphy {

• pinctrl-names = "default";
• pinctrl-0 = <&chip_id_det_pin>;
• status = "okay";
• usb0_id_det-gpio = <&pio 6 2 GPIO_ACTIVE_HIGH>; /* PG2 */
• usb0_vbus-supply = <&usb0_vbus>;
• usb1_vbus-supply = <&vcc_5v0>;

+}; diff --git a/configs/CHIP_defconfig b/configs/CHIP_defconfig new file mode 100644 index 000000000000..feab46d26ddd --- /dev/null +++ b/configs/CHIP_defconfig @@ -0,0 +1,14 @@ +CONFIG_SPL=y +CONFIG_SYS_EXTRA_OPTIONS="CONS_INDEX=2,AXP209_POWER,NO_MMC"

The NO_MMC bit needs to be replaced with:

# CONFIG_MMC is not set

Other then that this one looks good to me.

Regards,

Hans

+CONFIG_DEFAULT_DEVICE_TREE="sun5i-r8-chip" +CONFIG_USB_MUSB_GADGET=y +CONFIG_USB_MUSB_SUNXI=y +CONFIG_USB0_VBUS_PIN="PB10" +CONFIG_ARM=y +CONFIG_ARCH_SUNXI=y +CONFIG_MACH_SUN5I=y +CONFIG_DRAM_CLK=360 +CONFIG_DRAM_ZQ=123 +CONFIG_DRAM_EMR1=4 +CONFIG_DRAM_TIMINGS_DDR3_800E_1066G_1333J=y +# CONFIG_CMD_IMLS is not set