[U-Boot] [PATCH v1 0/5] various fixes mainly for colibri_t20

[U-Boot] [PATCH v1 0/5] various fixes mainly for colibri_t20

[Marcel Ziswiler]

This series addresses various issues as seen on Colibri T20. Please
note that for successful Ethernet operation not only on Colibri T20
but also on Colibri T30 the following patch will still need applying
as well:

[PATCH] net: asix: Fix ASIX 88772B with driver model

https://www.mail-archive.com/u-boot at lists.denx.de/msg221455.html

Please further note that USB gadget operation on T20 currently still
gives me headaches and despite me comparing the U-Boot sources with
the Linux driver and trying varous things I am still getting the
following upon connecting a T20 target device to a host machine:

[70291.779549] usb 2-3.4.2.7: new high-speed USB device number 29 using
 xhci_hcd
[70291.779677] usb 2-3.4.2.7: Device not responding to setup address.
[70291.980783] usb 2-3.4.2.7: Device not responding to setup address.
[70292.181466] usb 2-3.4.2.7: device not accepting address 29, error
 -71
[70292.181507] usb 2-3.4.2-port7: unable to enumerate USB device

Any feedback or suggestions are much appreciated.


Marcel Ziswiler (5):
  tegra: usb gadget: fix ci udc operation if not hostpc capable
  simple panel: fix spelling of debug message
  colibri_t20: fix display configuration
  colibri_t20: fix usb operation and controller order
  colibri_t20: enable dfu also for nand

 arch/arm/dts/tegra20-colibri.dts          | 117 +++++++++++++++++++-----------
 drivers/video/simple_panel.c              |   2 +-
 include/configs/colibri_t20.h             |   6 ++
 include/configs/tegra-common-usb-gadget.h |   1 -
 include/configs/tegra114-common.h         |   3 +
 include/configs/tegra124-common.h         |   3 +
 include/configs/tegra186-common.h         |   3 +
 include/configs/tegra210-common.h         |   3 +
 include/configs/tegra30-common.h          |   3 +
 9 files changed, 98 insertions(+), 43 deletions(-)

-- 
2.5.5

[U-Boot] [PATCH v1 1/5] tegra: usb gadget: fix ci udc operation if not hostpc capable

[Marcel Ziswiler]

The Tegra 2 aka T20 is not host PC capable. Therefore move the define
CONFIG_CI_UDC_HAS_HOSTPC from the generic tegra-common-usb-gadget.h
header file into resp. SoC type specific ones.

Signed-off-by: Marcel Ziswiler <marcel.ziswiler@toradex.com>
---

 include/configs/tegra-common-usb-gadget.h | 1 -
 include/configs/tegra114-common.h         | 3 +++
 include/configs/tegra124-common.h         | 3 +++
 include/configs/tegra186-common.h         | 3 +++
 include/configs/tegra210-common.h         | 3 +++
 include/configs/tegra30-common.h          | 3 +++
 6 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/include/configs/tegra-common-usb-gadget.h b/include/configs/tegra-common-usb-gadget.h
index 3e3eeea..6c6a438 100644
--- a/include/configs/tegra-common-usb-gadget.h
+++ b/include/configs/tegra-common-usb-gadget.h
@@ -10,7 +10,6 @@
 
 #ifndef CONFIG_SPL_BUILD
 /* USB gadget mode support*/
-#define CONFIG_CI_UDC_HAS_HOSTPC
 /* USB mass storage protocol */
 #define CONFIG_USB_FUNCTION_MASS_STORAGE
 /* DFU protocol */
diff --git a/include/configs/tegra114-common.h b/include/configs/tegra114-common.h
index 107a0f8..7747e0a 100644
--- a/include/configs/tegra114-common.h
+++ b/include/configs/tegra114-common.h
@@ -60,6 +60,9 @@
 #define CONFIG_SYS_SPL_MALLOC_START	0x80090000
 #define CONFIG_SPL_STACK		0x800ffffc
 
+/* For USB gadget mode support */
+#define CONFIG_CI_UDC_HAS_HOSTPC
+
 /* For USB EHCI controller */
 #define CONFIG_EHCI_IS_TDI
 #define CONFIG_USB_EHCI_TXFIFO_THRESH	0x10
diff --git a/include/configs/tegra124-common.h b/include/configs/tegra124-common.h
index 8cf9bac..ad3dbbb 100644
--- a/include/configs/tegra124-common.h
+++ b/include/configs/tegra124-common.h
@@ -62,6 +62,9 @@
 #define CONFIG_SYS_SPL_MALLOC_START	0x80090000
 #define CONFIG_SPL_STACK		0x800ffffc
 
+/* For USB gadget mode support */
+#define CONFIG_CI_UDC_HAS_HOSTPC
+
 /* For USB EHCI controller */
 #define CONFIG_EHCI_IS_TDI
 #define CONFIG_USB_EHCI_TXFIFO_THRESH	0x10
diff --git a/include/configs/tegra186-common.h b/include/configs/tegra186-common.h
index 98e4fc2..68fcf94 100644
--- a/include/configs/tegra186-common.h
+++ b/include/configs/tegra186-common.h
@@ -65,4 +65,7 @@
 #define CONFIG_SYS_SPL_MALLOC_START	0x80090000
 #define CONFIG_SPL_STACK		0x800ffffc
 
+/* For USB gadget mode support */
+#define CONFIG_CI_UDC_HAS_HOSTPC
+
 #endif
diff --git a/include/configs/tegra210-common.h b/include/configs/tegra210-common.h
index 874fe34d..50be2bf 100644
--- a/include/configs/tegra210-common.h
+++ b/include/configs/tegra210-common.h
@@ -65,6 +65,9 @@
 #define CONFIG_SYS_SPL_MALLOC_START	0x80090000
 #define CONFIG_SPL_STACK		0x800ffffc
 
+/* For USB gadget mode support */
+#define CONFIG_CI_UDC_HAS_HOSTPC
+
 /* For USB EHCI controller */
 #define CONFIG_EHCI_IS_TDI
 #define CONFIG_USB_EHCI_TXFIFO_THRESH	0x10
diff --git a/include/configs/tegra30-common.h b/include/configs/tegra30-common.h
index baf3d00..0a3cd60 100644
--- a/include/configs/tegra30-common.h
+++ b/include/configs/tegra30-common.h
@@ -67,6 +67,9 @@
 #define CONFIG_SYS_SPL_MALLOC_START	0x80090000
 #define CONFIG_SPL_STACK		0x800ffffc
 
+/* For USB gadget mode support */
+#define CONFIG_CI_UDC_HAS_HOSTPC
+
 /* For USB EHCI controller */
 #define CONFIG_EHCI_IS_TDI
 #define CONFIG_USB_EHCI_TXFIFO_THRESH	0x10
-- 
2.5.5

[U-Boot] [PATCH v1 2/5] simple panel: fix spelling of debug message

[Marcel Ziswiler]

Fix spelling of debug message from cnnot to cannot.

Signed-off-by: Marcel Ziswiler <marcel.ziswiler@toradex.com>
---

 drivers/video/simple_panel.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/video/simple_panel.c b/drivers/video/simple_panel.c
index b2fe345..baa95f6 100644
--- a/drivers/video/simple_panel.c
+++ b/drivers/video/simple_panel.c
@@ -42,7 +42,7 @@ static int simple_panel_ofdata_to_platdata(struct udevice *dev)
 		ret = uclass_get_device_by_phandle(UCLASS_REGULATOR, dev,
 						   "power-supply", &priv->reg);
 		if (ret) {
-			debug("%s: Warning: cnnot get power supply: ret=%d\n",
+			debug("%s: Warning: cannot get power supply: ret=%d\n",
 			      __func__, ret);
 			if (ret != -ENOENT)
 				return ret;
-- 
2.5.5

[U-Boot] [PATCH v1 3/5] colibri_t20: fix display configuration

[Marcel Ziswiler]

Without this patch the following error will be shown:

stdio_add_devices: Video device failed (ret=-22)

As commit ec5507707a1d1e84056a6c864338f95f6118d3ca (video: tegra: Move
to using simple-panel and pwm-backlight) states the Colibri T20 needs
updating too which this patch finally attempts doing.

Please note that the current U-Boot implementation requires a dummy
GPIO e.g. for a fixed backlight regulator to be explicitly defined in
order to work unlike in the Linux kernel where this is taken care of
automatically.

Signed-off-by: Marcel Ziswiler <marcel.ziswiler@toradex.com>
---

 arch/arm/dts/tegra20-colibri.dts | 72 +++++++++++++++++++++++++++++-----------
 1 file changed, 52 insertions(+), 20 deletions(-)

diff --git a/arch/arm/dts/tegra20-colibri.dts b/arch/arm/dts/tegra20-colibri.dts
index 2cf24d3..7d4e64a 100644
--- a/arch/arm/dts/tegra20-colibri.dts
+++ b/arch/arm/dts/tegra20-colibri.dts
@@ -21,12 +21,24 @@
 	};
 
 	host1x at 50000000 {
-		status = "okay";
 		dc at 54200000 {
-			status = "okay";
 			rgb {
 				status = "okay";
 				nvidia,panel = <&lcd_panel>;
+				display-timings {
+					timing at 0 {
+						/* VESA VGA */
+						clock-frequency = <25175000>;
+						hactive = <640>;
+						vactive = <480>;
+						hback-porch = <48>;
+						hfront-porch = <16>;
+						hsync-len = <96>;
+						vback-porch = <31>;
+						vfront-porch = <11>;
+						vsync-len = <2>;
+					};
+				};
 			};
 		};
 	};
@@ -60,6 +72,10 @@
 		};
 	};
 
+	pwm at 7000a000 {
+		status = "okay";
+	};
+
 	/*
 	 * GEN1_I2C: I2C_SDA/SCL on SODIMM pin 194/196 (e.g. RTC on carrier
 	 * board)
@@ -91,6 +107,19 @@
 		cd-gpios = <&gpio TEGRA_GPIO(C, 7) GPIO_ACTIVE_LOW>;
 	};
 
+	backlight: backlight {
+		compatible = "pwm-backlight";
+
+		brightness-levels = <255 128 64 32 16 8 4 0>;
+		default-brightness-level = <6>;
+		/* BL_ON */
+		enable-gpios = <&gpio TEGRA_GPIO(T, 4) GPIO_ACTIVE_HIGH>;
+		/* Dummy */
+		power-supply = <&reg_dummy>;
+		/* PWM<A> */
+		pwms = <&pwm 0 5000000>;
+	};
+
 	clocks {
 		compatible = "simple-bus";
 		#address-cells = <1>;
@@ -104,25 +133,28 @@
 		};
 	};
 
-	pwm: pwm at 7000a000 {
-		status = "okay";
+	lcd_panel: panel {
+		/*
+		 * edt,et057090dhu: EDT 5.7" LCD TFT
+		 * edt,et070080dh6: EDT 7.0" LCD TFT
+		 */
+		compatible = "edt,et057090dhu", "simple-panel";
+
+		backlight = <&backlight>;
 	};
 
-	lcd_panel: panel {
-		clock = <25175000>;
-		xres = <640>;
-		yres = <480>;
-		left-margin = <48>;	/* horizontal back porch */
-		right-margin = <16>;	/* horizontal front porch */
-		hsync-len = <96>;
-		lower-margin = <11>;	/* vertical front porch */
-		upper-margin = <31>;	/* vertical back porch */
-		vsync-len = <2>;
-		hsync-active-high;
-		vsync-active-high;
-		nvidia,bits-per-pixel = <16>;
-		nvidia,pwm = <&pwm 0 0>;
-		nvidia,backlight-enable-gpios = <&gpio TEGRA_GPIO(T, 4) GPIO_ACTIVE_HIGH>;
-		nvidia,panel-timings = <0 0 0 0>;
+	regulators {
+		compatible = "simple-bus";
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		reg_dummy: regulator at 0 {
+			compatible = "regulator-fixed";
+			reg = <0>;
+			regulator-name = "Dummy";
+			/* Dummy N/C */
+			gpio = <&gpio TEGRA_GPIO(V, 7) GPIO_ACTIVE_HIGH>;
+			enable-active-high;
+		};
 	};
 };
-- 
2.5.5

[U-Boot] [PATCH v1 4/5] colibri_t20: fix usb operation and controller order

[Marcel Ziswiler]

Without this patch the following error will be shown:

Colibri T20 # usb start
starting USB...
No controllers found

This patch fixes USB operation and also the controller order as the
CI UDC driver may only be instantiated on the first aka OTG port.

Signed-off-by: Marcel Ziswiler <marcel.ziswiler@toradex.com>
---

 arch/arm/dts/tegra20-colibri.dts | 45 +++++++++++++++++++++-------------------
 1 file changed, 24 insertions(+), 21 deletions(-)

diff --git a/arch/arm/dts/tegra20-colibri.dts b/arch/arm/dts/tegra20-colibri.dts
index 7d4e64a..7df4e1b 100644
--- a/arch/arm/dts/tegra20-colibri.dts
+++ b/arch/arm/dts/tegra20-colibri.dts
@@ -14,10 +14,10 @@
 		i2c0 = "/i2c at 7000d000";
 		i2c1 = "/i2c at 7000c000";
 		i2c2 = "/i2c at 7000c400";
-		usb0 = "/usb at c5008000";
-		usb1 = "/usb at c5000000";
-		usb2 = "/usb at c5004000";
 		sdhci0 = "/sdhci at c8000600";
+		usb0 = "/usb at c5000000";
+		usb1 = "/usb at c5004000"; /* on-module only, for ASIX */
+		usb2 = "/usb at c5008000";
 	};
 
 	host1x at 50000000 {
@@ -43,24 +43,6 @@
 		};
 	};
 
-	usb at c5000000 {
-		statuc = "okay";
-		dr_mode = "otg";
-	};
-
-	usb at c5004000 {
-		statuc = "okay";
-		/* VBUS_LAN */
-		nvidia,phy-reset-gpio = <&gpio TEGRA_GPIO(V, 1) GPIO_ACTIVE_HIGH>;
-		nvidia,vbus-gpio = <&gpio TEGRA_GPIO(BB, 1) GPIO_ACTIVE_HIGH>;
-	};
-
-	usb at c5008000 {
-		statuc = "okay";
-		/* USBH_PEN */
-		nvidia,vbus-gpio = <&gpio TEGRA_GPIO(W, 2) GPIO_ACTIVE_LOW>;
-	};
-
 	nand-controller at 70008000 {
 		nvidia,wp-gpios = <&gpio TEGRA_GPIO(S, 0) GPIO_ACTIVE_HIGH>;
 		nvidia,width = <8>;
@@ -101,6 +83,27 @@
 		clock-frequency = <100000>;
 	};
 
+	/* EHCI instance 0: USB1_DP/N -> USBC_P/N */
+	usb at c5000000 {
+		status = "okay";
+		dr_mode = "otg";
+	};
+
+	/* EHCI instance 1: ULPI -> USB3340 -> AX88772B */
+	usb at c5004000 {
+		status = "okay";
+		/* VBUS_LAN */
+		nvidia,phy-reset-gpio = <&gpio TEGRA_GPIO(V, 1) GPIO_ACTIVE_HIGH>;
+		nvidia,vbus-gpio = <&gpio TEGRA_GPIO(BB, 1) GPIO_ACTIVE_HIGH>;
+	};
+
+	/* EHCI instance 2: USB3_DP/N -> USBH_P/N */
+	usb at c5008000 {
+		status = "okay";
+		/* USBH_PEN */
+		nvidia,vbus-gpio = <&gpio TEGRA_GPIO(W, 2) GPIO_ACTIVE_LOW>;
+	};
+
 	sdhci at c8000600 {
 		status = "okay";
 		bus-width = <4>;
-- 
2.5.5

[U-Boot] [PATCH v1 5/5] colibri_t20: enable dfu also for nand

[Marcel Ziswiler]

Enable USB gadget DFU functionality for NAND as well.

Signed-off-by: Marcel Ziswiler <marcel.ziswiler@toradex.com>

---

 include/configs/colibri_t20.h | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/include/configs/colibri_t20.h b/include/configs/colibri_t20.h
index c15f0cb..33e1ef5 100644
--- a/include/configs/colibri_t20.h
+++ b/include/configs/colibri_t20.h
@@ -31,6 +31,9 @@
 #define CONFIG_GENERIC_MMC
 #define CONFIG_TEGRA_MMC
 
+/* USB DFU */
+#define CONFIG_DFU_NAND
+
 /* USB host support */
 #define CONFIG_USB_EHCI
 #define CONFIG_USB_EHCI_TEGRA
@@ -86,7 +89,10 @@
 /* Miscellaneous commands */
 #define CONFIG_FAT_WRITE
 
+#define DFU_ALT_NAND_INFO	"u-boot part 0,1;ubi part 0,4"
+
 #define BOARD_EXTRA_ENV_SETTINGS \
+	"dfu_alt_info=" DFU_ALT_NAND_INFO "\0" \
 	"mtdparts=" MTDPARTS_DEFAULT "\0"
 
 /* Increase console I/O buffer size */
-- 
2.5.5

[U-Boot] [PATCH v1 2/5] simple panel: fix spelling of debug message

[Anatolij Gustschin]

On Fri,  9 Sep 2016 18:10:38 +0200
Marcel Ziswiler marcel.ziswiler at toradex.com wrote:

> Fix spelling of debug message from cnnot to cannot.
> 
> Signed-off-by: Marcel Ziswiler <marcel.ziswiler@toradex.com>

Acked-by: Anatolij Gustschin <agust@denx.de>

[U-Boot] [PATCH v1 0/5] various fixes mainly for colibri_t20

[Stephen Warren]

On 09/09/2016 10:10 AM, Marcel Ziswiler wrote:
> This series addresses various issues as seen on Colibri T20. Please
> note that for successful Ethernet operation not only on Colibri T20
> but also on Colibri T30 the following patch will still need applying
> as well:
>
> [PATCH] net: asix: Fix ASIX 88772B with driver model
>
> https://www.mail-archive.com/u-boot at lists.denx.de/msg221455.html
>
> Please further note that USB gadget operation on T20 currently still
> gives me headaches and despite me comparing the U-Boot sources with
> the Linux driver and trying varous things I am still getting the
> following upon connecting a T20 target device to a host machine:
>
> [70291.779549] usb 2-3.4.2.7: new high-speed USB device number 29 using
>  xhci_hcd
> [70291.779677] usb 2-3.4.2.7: Device not responding to setup address.
> [70291.980783] usb 2-3.4.2.7: Device not responding to setup address.
> [70292.181466] usb 2-3.4.2.7: device not accepting address 29, error
>  -71
> [70292.181507] usb 2-3.4.2-port7: unable to enumerate USB device
>
> Any feedback or suggestions are much appreciated.

I'm not convinced it's a good idea to enable the feature until it's 
reasonably robust?

[U-Boot] [PATCH v1 1/5] tegra: usb gadget: fix ci udc operation if not hostpc capable

[Stephen Warren]

On 09/09/2016 10:10 AM, Marcel Ziswiler wrote:
> The Tegra 2 aka T20 is not host PC capable. Therefore move the define
> CONFIG_CI_UDC_HAS_HOSTPC from the generic tegra-common-usb-gadget.h
> header file into resp. SoC type specific ones.

This is OK, but ...

> diff --git a/include/configs/tegra-common-usb-gadget.h b/include/configs/tegra-common-usb-gadget.h

>  #ifndef CONFIG_SPL_BUILD
>  /* USB gadget mode support*/
> -#define CONFIG_CI_UDC_HAS_HOSTPC

... it seems a bit simpler to just wrap that in #ifndef CONFIG_TEGRA20.

That would avoid having to duplicate the define in all tegra*-common.h 
forever. Still, it's not a huge deal.

[U-Boot] [PATCH v1 3/5] colibri_t20: fix display configuration

[Stephen Warren]

On 09/09/2016 10:10 AM, Marcel Ziswiler wrote:
> Without this patch the following error will be shown:
>
> stdio_add_devices: Video device failed (ret=-22)
>
> As commit ec5507707a1d1e84056a6c864338f95f6118d3ca (video: tegra: Move
> to using simple-panel and pwm-backlight) states the Colibri T20 needs
> updating too which this patch finally attempts doing.
>
> Please note that the current U-Boot implementation requires a dummy
> GPIO e.g. for a fixed backlight regulator to be explicitly defined in
> order to work unlike in the Linux kernel where this is taken care of
> automatically.

The binding documentation does state that the power supply is mandatory.

> diff --git a/arch/arm/dts/tegra20-colibri.dts b/arch/arm/dts/tegra20-colibri.dts

> +	regulators {
> +		compatible = "simple-bus";
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		reg_dummy: regulator at 0 {

Why call this a dummy? This is a real regulator that describes the power 
supply to the backlight. Even if there's no SW control over the power to 
the backlight, there is still a (fixed) power source, and this DT node 
represents that power source.

> +			compatible = "regulator-fixed";
> +			reg = <0>;
> +			regulator-name = "Dummy";
> +			/* Dummy N/C */
> +			gpio = <&gpio TEGRA_GPIO(V, 7) GPIO_ACTIVE_HIGH>;

This is wrong. If that GPIO isn't actually part of the backlight, the DT 
should not say that it is. The gpio property is optional according to 
the DT binding documentation, so this shouldn't be needed.

[U-Boot] [PATCH v1 4/5] colibri_t20: fix usb operation and controller order

[Stephen Warren]

On 09/09/2016 10:10 AM, Marcel Ziswiler wrote:
> Without this patch the following error will be shown:
>
> Colibri T20 # usb start
> starting USB...
> No controllers found

This change seems fine, but I'm concerned that the alias order causes 
failures; arbitrary orders should be allowed. Have you determined what 
the problem is?

[U-Boot] [PATCH v1 5/5] colibri_t20: enable dfu also for nand

[Stephen Warren]

On 09/09/2016 10:10 AM, Marcel Ziswiler wrote:
> Enable USB gadget DFU functionality for NAND as well.

> diff --git a/include/configs/colibri_t20.h b/include/configs/colibri_t20.h

> +/* USB DFU */
> +#define CONFIG_DFU_NAND

Oh, I see this file already includes tegra-common-usb-gadget.h, so USB 
device-mode is already enabled for this board. Does that make sense 
given that it doesn't actually work?

> +#define DFU_ALT_NAND_INFO	"u-boot part 0,1;ubi part 0,4"
> +
>  #define BOARD_EXTRA_ENV_SETTINGS \
> +	"dfu_alt_info=" DFU_ALT_NAND_INFO "\0" \

I would defer this to the user, since people may choose different flash 
layouts.

[U-Boot] [PATCH v1 1/5] tegra: usb gadget: fix ci udc operation if not hostpc capable

[Marcel Ziswiler]

On Mon, 2016-09-12 at 12:13 -0600, Stephen Warren wrote:
> On 09/09/2016 10:10 AM, Marcel Ziswiler wrote:
> > 
> > The Tegra 2 aka T20 is not host PC capable. Therefore move the
> > define
> > CONFIG_CI_UDC_HAS_HOSTPC from the generic tegra-common-usb-gadget.h
> > header file into resp. SoC type specific ones.
> This is OK, but ...
> 
> > 
> > diff --git a/include/configs/tegra-common-usb-gadget.h
> > b/include/configs/tegra-common-usb-gadget.h
> > 
> > ?#ifndef CONFIG_SPL_BUILD
> > ?/* USB gadget mode support*/
> > -#define CONFIG_CI_UDC_HAS_HOSTPC
> ... it seems a bit simpler to just wrap that in #ifndef
> CONFIG_TEGRA20.
> 
> That would avoid having to duplicate the define in all tegra*-
> common.h?
> forever. Still, it's not a huge deal.


Agreed, I will change this in a v2.

[U-Boot] [PATCH v1 3/5] colibri_t20: fix display configuration

[Marcel Ziswiler]

On Mon, 2016-09-12 at 12:18 -0600, Stephen Warren wrote:
> On 09/09/2016 10:10 AM, Marcel Ziswiler wrote:
> > 
> > Without this patch the following error will be shown:
> > 
> > stdio_add_devices: Video device failed (ret=-22)
> > 
> > As commit ec5507707a1d1e84056a6c864338f95f6118d3ca (video: tegra:
> > Move
> > to using simple-panel and pwm-backlight) states the Colibri T20
> > needs
> > updating too which this patch finally attempts doing.
> > 
> > Please note that the current U-Boot implementation requires a dummy
> > GPIO e.g. for a fixed backlight regulator to be explicitly defined
> > in
> > order to work unlike in the Linux kernel where this is taken care
> > of
> > automatically.
> The binding documentation does state that the power supply is
> mandatory.

Yes, of course.

> > diff --git a/arch/arm/dts/tegra20-colibri.dts
> > b/arch/arm/dts/tegra20-colibri.dts
> > 
> > +	regulators {
> > +		compatible = "simple-bus";
> > +		#address-cells = <1>;
> > +		#size-cells = <0>;
> > +
> > +		reg_dummy: regulator at 0 {
> Why call this a dummy? This is a real regulator that describes the
> power?
> supply to the backlight. Even if there's no SW control over the power
> to?
> the backlight, there is still a (fixed) power source, and this DT
> node?
> represents that power source.

OK.

> > +			compatible = "regulator-fixed";
> > +			reg = <0>;
> > +			regulator-name = "Dummy";
> > +			/* Dummy N/C */
> > +			gpio = <&gpio TEGRA_GPIO(V, 7)
> > GPIO_ACTIVE_HIGH>;
> This is wrong. If that GPIO isn't actually part of the backlight, the
> DT?
> should not say that it is. The gpio property is optional according
> to?
> the DT binding documentation, so this shouldn't be needed.


Well, I guess then it's lying. If I leave it away I get the following:

stdio_add_devices: Video device failed (ret=-38)

And it won't quite work.

[U-Boot] [PATCH v1 4/5] colibri_t20: fix usb operation and controller order

[Marcel Ziswiler]

On Mon, 2016-09-12 at 12:20 -0600, Stephen Warren wrote:
> On 09/09/2016 10:10 AM, Marcel Ziswiler wrote:
> > 
> > Without this patch the following error will be shown:
> > 
> > Colibri T20 # usb start
> > starting USB...
> > No controllers found
> This change seems fine, but I'm concerned that the alias order
> causes?
> failures; arbitrary orders should be allowed. Have you determined
> what?
> the problem is?


Not really, no. Unfortunately with U-Boot there is so much broken- resp.
strangeness everywhere if I would start looking into every detail I
would never get any real work done. As for this particular case I guess
we are now just doing what all other Tegra boards do really.

[U-Boot] [PATCH v1 5/5] colibri_t20: enable dfu also for nand

[Marcel Ziswiler]

On Mon, 2016-09-12 at 12:24 -0600, Stephen Warren wrote:
> On 09/09/2016 10:10 AM, Marcel Ziswiler wrote:
> > 
> > Enable USB gadget DFU functionality for NAND as well.
> > 
> > diff --git a/include/configs/colibri_t20.h
> > b/include/configs/colibri_t20.h
> > 
> > +/* USB DFU */
> > +#define CONFIG_DFU_NAND
> Oh, I see this file already includes tegra-common-usb-gadget.h, so
> USB?
> device-mode is already enabled for this board. Does that make sense?
> given that it doesn't actually work?

Well, it's not like it hurts anything else really isn't it?

My hopes were that somebody may actually help me looking into it which
this would ease. However I understand that you NVIDIA people long since
stopped even having any of them older Tegra 2 and 3 hardware any
longer. At least Harmony and Ventana currently looks rather broken in
many aspects which I left for another days exercise.

> > +#define DFU_ALT_NAND_INFO	"u-boot part 0,1;ubi part 0,4"
> > +
> > ?#define BOARD_EXTRA_ENV_SETTINGS \
> > +	"dfu_alt_info=" DFU_ALT_NAND_INFO "\0" \
> I would defer this to the user, since people may choose different
> flash?
> layouts.

Given the DFU NAND syntax being rather delicate at least Google
returning rather some wrong stuff with respect to now starting with
zero or one I thought that would at least make it clear. It's not that
a user could not overwrite it any time if he wishes to do so isn't it?

[U-Boot] [PATCH v1 0/5] various fixes mainly for colibri_t20

[Marcel Ziswiler]

On Mon, 2016-09-12 at 12:13 -0600, Stephen Warren wrote:
> On 09/09/2016 10:10 AM, Marcel Ziswiler wrote:
> > 
> > This series addresses various issues as seen on Colibri T20. Please
> > note that for successful Ethernet operation not only on Colibri T20
> > but also on Colibri T30 the following patch will still need
> > applying
> > as well:
> > 
> > [PATCH] net: asix: Fix ASIX 88772B with driver model
> > 
> > https://www.mail-archive.com/u-boot at lists.denx.de/msg221455.html
> > 
> > Please further note that USB gadget operation on T20 currently
> > still
> > gives me headaches and despite me comparing the U-Boot sources with
> > the Linux driver and trying varous things I am still getting the
> > following upon connecting a T20 target device to a host machine:
> > 
> > [70291.779549] usb 2-3.4.2.7: new high-speed USB device number 29
> > using
> > ?xhci_hcd
> > [70291.779677] usb 2-3.4.2.7: Device not responding to setup
> > address.
> > [70291.980783] usb 2-3.4.2.7: Device not responding to setup
> > address.
> > [70292.181466] usb 2-3.4.2.7: device not accepting address 29,
> > error
> > ?-71
> > [70292.181507] usb 2-3.4.2-port7: unable to enumerate USB device
> > 
> > Any feedback or suggestions are much appreciated.
> I'm not convinced it's a good idea to enable the feature until it's?
> reasonably robust?

Well, as mentioned before it's not like it hurts anything else really
isn't it?

My hopes were that somebody may actually help me looking into it which
this would ease. I'm not that much familiar with the?nitty-gritty
details of low-level USB driver interworking plus am currently missing
any tool like a USB analyser or the like to help me understand what may
be going wrong.

[U-Boot] [PATCH v1 3/5] colibri_t20: fix display configuration

[Stephen Warren]

On 09/14/2016 09:20 AM, Marcel Ziswiler wrote:
> On Mon, 2016-09-12 at 12:18 -0600, Stephen Warren wrote:
>> On 09/09/2016 10:10 AM, Marcel Ziswiler wrote:
>>>
>>> Without this patch the following error will be shown:
>>>
>>> stdio_add_devices: Video device failed (ret=-22)
>>>
>>> As commit ec5507707a1d1e84056a6c864338f95f6118d3ca (video: tegra: Move
>>> to using simple-panel and pwm-backlight) states the Colibri T20 needs
>>> updating too which this patch finally attempts doing.
>>>
>>> Please note that the current U-Boot implementation requires a dummy
>>> GPIO e.g. for a fixed backlight regulator to be explicitly defined in
>>> order to work unlike in the Linux kernel where this is taken care of
>>> automatically.

>>> +			compatible = "regulator-fixed";
>>> +			reg = <0>;
>>> +			regulator-name = "Dummy";
>>> +			/* Dummy N/C */
>>> +			gpio = <&gpio TEGRA_GPIO(V, 7)
>>> GPIO_ACTIVE_HIGH>;
 >>
>> This is wrong. If that GPIO isn't actually part of the backlight, the DT
>> should not say that it is. The gpio property is optional according to
>> the DT binding documentation, so this shouldn't be needed.
>
> Well, I guess then it's lying.

Does "it" mean the binding? Please note that the binding defines how the 
DT should be structured and how code interpreting the DT should operate. 
The binding isn't derived from the code, but rather the code is derived 
from the binding.

 > If I leave it away I get the following:
>
> stdio_add_devices: Video device failed (ret=-38)
>
> And it won't quite work.

That sounds like a bug in the U-Boot regulator driver. I believe you 
should fix that, rather than working around the bug in DT.

[U-Boot] [PATCH v1 5/5] colibri_t20: enable dfu also for nand

[Stephen Warren]

On 09/14/2016 09:41 AM, Marcel Ziswiler wrote:
> On Mon, 2016-09-12 at 12:24 -0600, Stephen Warren wrote:
>> On 09/09/2016 10:10 AM, Marcel Ziswiler wrote:
>>>
>>> Enable USB gadget DFU functionality for NAND as well.
>>>
>>> diff --git a/include/configs/colibri_t20.h
>>> b/include/configs/colibri_t20.h
>>>
>>> +/* USB DFU */
>>> +#define CONFIG_DFU_NAND
>> Oh, I see this file already includes tegra-common-usb-gadget.h, so
>> USB
>> device-mode is already enabled for this board. Does that make sense
>> given that it doesn't actually work?
>
> Well, it's not like it hurts anything else really isn't it?

Having the feature enabled implies that it works in my opinion. If it 
doesn't, I think this will only confuse users.

> My hopes were that somebody may actually help me looking into it which
> this would ease. However I understand that you NVIDIA people long since
> stopped even having any of them older Tegra 2 and 3 hardware any
> longer. At least Harmony and Ventana currently looks rather broken in
> many aspects which I left for another days exercise.

If someone wants to fix USB device mode on Tegra20, I don't imagine it 
would be hard for them to enable it while working on it.

What's broken on Harmony and Ventana? They both worked when I tested all 
Tegra boards within the last few months. We have a Tegra30 board (but 
admittedly not Tegra20 board) in our automated upstream U-Boot test farm 
(running test/py).

>>> +#define DFU_ALT_NAND_INFO	"u-boot part 0,1;ubi part 0,4"
>>> +
>>>  #define BOARD_EXTRA_ENV_SETTINGS \
>>> +	"dfu_alt_info=" DFU_ALT_NAND_INFO "\0" \
>> I would defer this to the user, since people may choose different
>> flash
>> layouts.
>
> Given the DFU NAND syntax being rather delicate at least Google
> returning rather some wrong stuff with respect to now starting with
> zero or one I thought that would at least make it clear. It's not that
> a user could not overwrite it any time if he wishes to do so isn't it?

Certainly a user could over-write it. However, I'm not convinced it's a 
good idea to provide an arbitrary default value that may or may not be 
remotely relevant to the user's actual configuration. Again, this may 
lead users down the wrong path of wondering why they can't get this 
default configuration to work, rather than researching what the correct 
configuration is.

[U-Boot] [PATCH v1 5/5] colibri_t20: enable dfu also for nand

[Marcel Ziswiler]

On Wed, 2016-09-14 at 17:23 +0000, Stephen Warren wrote:
> On 09/14/2016 09:41 AM, Marcel Ziswiler wrote:
> > 
> > On Mon, 2016-09-12 at 12:24 -0600, Stephen Warren wrote:
> > > 
> > > On 09/09/2016 10:10 AM, Marcel Ziswiler wrote:
> > > > 
> > > > 
> > > > Enable USB gadget DFU functionality for NAND as well.
> > > > 
> > > > diff --git a/include/configs/colibri_t20.h
> > > > b/include/configs/colibri_t20.h
> > > > 
> > > > +/* USB DFU */
> > > > +#define CONFIG_DFU_NAND
> > > Oh, I see this file already includes tegra-common-usb-gadget.h,
> > > so
> > > USB
> > > device-mode is already enabled for this board. Does that make
> > > sense
> > > given that it doesn't actually work?
> > Well, it's not like it hurts anything else really isn't it?
> Having the feature enabled implies that it works in my opinion. If
> it?
> doesn't, I think this will only confuse users.

Well, as you correctly noticed it is and always was already enabled
anyway.

> > My hopes were that somebody may actually help me looking into it
> > which
> > this would ease. However I understand that you NVIDIA people long
> > since
> > stopped even having any of them older Tegra 2 and 3 hardware any
> > longer. At least Harmony and Ventana currently looks rather broken
> > in
> > many aspects which I left for another days exercise.
> If someone wants to fix USB device mode on Tegra20, I don't imagine
> it?
> would be hard for them to enable it while working on it.

Sure, as it long since was already enabled.

> What's broken on Harmony and Ventana? They both worked when I tested
> all?
> Tegra boards within the last few months. We have a Tegra30 board
> (but?
> admittedly not Tegra20 board) in our automated upstream U-Boot test
> farm?
> (running test/py).

Last I checked a few days ago USB failed at least on Ventana.

> > > > +#define DFU_ALT_NAND_INFO	"u-boot part 0,1;ubi part
> > > > 0,4"
> > > > +
> > > > ?#define BOARD_EXTRA_ENV_SETTINGS \
> > > > +	"dfu_alt_info=" DFU_ALT_NAND_INFO "\0" \
> > > I would defer this to the user, since people may choose different
> > > flash
> > > layouts.
> > Given the DFU NAND syntax being rather delicate at least Google
> > returning rather some wrong stuff with respect to now starting with
> > zero or one I thought that would at least make it clear. It's not
> > that
> > a user could not overwrite it any time if he wishes to do so isn't
> > it?
> Certainly a user could over-write it. However, I'm not convinced it's
> a?
> good idea to provide an arbitrary default value that may or may not
> be?
> remotely relevant to the user's actual configuration. Again, this
> may?
> lead users down the wrong path of wondering why they can't get this?
> default configuration to work, rather than researching what the
> correct?
> configuration is.

Sure, I will just drop this patch. Don't worry.

[U-Boot] [PATCH v1 3/5] colibri_t20: fix display configuration

[Marcel Ziswiler]

On Wed, 2016-09-14 at 17:19 +0000, Stephen Warren wrote:
> On 09/14/2016 09:20 AM, Marcel Ziswiler wrote:
> > 
> > On Mon, 2016-09-12 at 12:18 -0600, Stephen Warren wrote:
> > > 
> > > On 09/09/2016 10:10 AM, Marcel Ziswiler wrote:
> > > > 
> > > > 
> > > > Without this patch the following error will be shown:
> > > > 
> > > > stdio_add_devices: Video device failed (ret=-22)
> > > > 
> > > > As commit ec5507707a1d1e84056a6c864338f95f6118d3ca (video:
> > > > tegra: Move
> > > > to using simple-panel and pwm-backlight) states the Colibri T20
> > > > needs
> > > > updating too which this patch finally attempts doing.
> > > > 
> > > > Please note that the current U-Boot implementation requires a
> > > > dummy
> > > > GPIO e.g. for a fixed backlight regulator to be explicitly
> > > > defined in
> > > > order to work unlike in the Linux kernel where this is taken
> > > > care of
> > > > automatically.
> > 
> > > 
> > > > 
> > > > +			compatible = "regulator-fixed";
> > > > +			reg = <0>;
> > > > +			regulator-name = "Dummy";
> > > > +			/* Dummy N/C */
> > > > +			gpio = <&gpio TEGRA_GPIO(V, 7)
> > > > GPIO_ACTIVE_HIGH>;
> ?>>
> > 
> > > 
> > > This is wrong. If that GPIO isn't actually part of the backlight,
> > > the DT
> > > should not say that it is. The gpio property is optional
> > > according to
> > > the DT binding documentation, so this shouldn't be needed.
> > Well, I guess then it's lying.
> Does "it" mean the binding? Please note that the binding defines how
> the?
> DT should be structured and how code interpreting the DT should
> operate.?
> The binding isn't derived from the code, but rather the code is
> derived?
> from the binding.

In theory I agree but in practical speak this is wishful thinking as we
just first handedly saw now.

> ?> If I leave it away I get the following:
> > 
> > 
> > stdio_add_devices: Video device failed (ret=-38)
> > 
> > And it won't quite work.
> That sounds like a bug in the U-Boot regulator driver. I believe you?
> should fix that, rather than working around the bug in DT.


Yes, I am pretty sure it won't be the last bug I uncover.
Unfortunately right now I do not feel like fixing all of U-Boot. This
series just tries to fix a very few select things (e.g. display and
USB) which used to work just fine before some bigger agendas came along
and broke them.