Skip to contentSkip to conversation

various fixes mainly for colibri_t20

PATCH New Marcel Ziswiler 3 participants 21 messages Sep 9, 2016 – Sep 14, 2016

Subsystems (3)
Arm (32-bit) Framebuffer layer Tegra architecture support
lore
Share & tools
lore

Marcel Ziswiler

author
#0 lore raw

cover various fixes mainly for colibri_t20

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(-)
Signature · 1 line
2.5.5
author
#1 lore raw

PATCH 1/5 tegra: usb gadget: fix ci udc operation if not hostpc capable

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 <[email protected]>

---

 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(-)
include/configs/tegra-common-usb-gadget.h +0 -1
index 3e3eeea..6c6a438 100644
@@ -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 */
include/configs/tegra114-common.h +3 -0
index 107a0f8..7747e0a 100644
@@ -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
include/configs/tegra124-common.h +3 -0
index 8cf9bac..ad3dbbb 100644
@@ -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
include/configs/tegra186-common.h +3 -0
index 98e4fc2..68fcf94 100644
@@ -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
include/configs/tegra210-common.h +3 -0
index 874fe34d..50be2bf 100644
@@ -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
include/configs/tegra30-common.h +3 -0
index baf3d00..0a3cd60 100644
@@ -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
Signature · 1 line
2.5.5
#2 lore raw

PATCH 1/5 tegra: usb gadget: fix ci udc operation if not hostpc capable

Quoted 3 lines · 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.

This is OK, but ...

include/configs/tegra-common-usb-gadget.h line 13

Quoted 5 lines
include/configs/tegra-common-usb-gadget.h +0 -1
 
 #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.

author
#3 lore raw

PATCH 1/5 tegra: usb gadget: fix ci udc operation if not hostpc capable

Quoted 18 lines · Stephen Warren
Quoted 5 lines · 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.

This is OK, but ...

Quoted 7 lines
include/configs/tegra-common-usb-gadget.h +0 -0

?#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.

Quoted 3 lines

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.

author
#4 lore raw

PATCH 2/5 simple panel: fix spelling of debug message

In reply to Marcel Ziswiler

Fix spelling of debug message from cnnot to cannot.

Signed-off-by Marcel Ziswiler <[email protected]>

---

 drivers/video/simple_panel.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/video/simple_panel.c +1 -1
index b2fe345..baa95f6 100644
@@ -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;
Signature · 1 line
2.5.5
author
#6 lore raw

PATCH 3/5 colibri_t20: fix display configuration

In reply to 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 <[email protected]>

---

 arch/arm/dts/tegra20-colibri.dts | 72 +++++++++++++++++++++++++++++-----------
 1 file changed, 52 insertions(+), 20 deletions(-)
arch/arm/dts/tegra20-colibri.dts +52 -20
index 2cf24d3..7d4e64a 100644
@@ -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;
+		};
 	};
 };
Signature · 1 line
2.5.5
#7 lore raw

PATCH 3/5 colibri_t20: fix display configuration

Quoted 4 lines · Marcel Ziswiler

Without this patch the following error will be shown:

stdio_add_devices: Video device failed (ret=-22)

Quoted 8 lines

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.

arch/arm/dts/tegra20-colibri.dts line 151

Quoted 8 lines
arch/arm/dts/tegra20-colibri.dts +6 -0
 
+	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.

arch/arm/dts/tegra20-colibri.dts line 156

Quoted 5 lines
arch/arm/dts/tegra20-colibri.dts +5 -0
+			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.

author
#8 lore raw

PATCH 3/5 colibri_t20: fix display configuration

Quoted 12 lines · Stephen Warren
Quoted 11 lines · 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.

Quoted 8 lines
Quoted 6 lines

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.

Quoted 16 lines

arch/arm/dts/tegra20-colibri.dts line 151

Quoted 9 lines
arch/arm/dts/tegra20-colibri.dts +6 -0
 
+	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.

Quoted 11 lines
Quoted 6 lines
arch/arm/dts/tegra20-colibri.dts +5 -0
+			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.

#9 lore raw

PATCH 3/5 colibri_t20: fix display configuration

Quoted 27 lines · Marcel Ziswiler
Quoted 15 lines · Stephen Warren
Quoted 13 lines · 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.

Quoted 10 lines
Quoted 6 lines
arch/arm/dts/tegra20-colibri.dts +5 -0
+			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.

Quoted 1 line

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.

Quoted 5 lines

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.

author
#10 lore raw

PATCH 3/5 colibri_t20: fix display configuration

Quoted 41 lines · Stephen Warren
Quoted 26 lines · Marcel Ziswiler
Quoted 24 lines · Stephen Warren
Quoted 22 lines · 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.

Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
arch/arm/dts/tegra20-colibri.dts +5 -0
+			compatible = "regulator-fixed";
+			reg = <0>;
+			regulator-name = "Dummy";
+			/* Dummy N/C */
+			gpio = <&gpio TEGRA_GPIO(V, 7)

GPIO_ACTIVE_HIGH>;

?>>

Quoted 7 lines
Quoted 7 lines

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.

Quoted 8 lines
Quoted 1 line

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.

Quoted 8 lines

?> If I leave it away I get the following:

Quoted 5 lines

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.

author
#11 lore raw

PATCH 4/5 colibri_t20: fix usb operation and controller order

In reply to 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 <[email protected]>

---

 arch/arm/dts/tegra20-colibri.dts | 45 +++++++++++++++++++++-------------------
 1 file changed, 24 insertions(+), 21 deletions(-)
arch/arm/dts/tegra20-colibri.dts +24 -21
index 7d4e64a..7df4e1b 100644
@@ -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>;
Signature · 1 line
2.5.5
#12 lore raw

PATCH 4/5 colibri_t20: fix usb operation and controller order

Quoted 5 lines · Marcel Ziswiler

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?

author
#13 lore raw

PATCH 4/5 colibri_t20: fix usb operation and controller order

Quoted 4 lines · Stephen Warren
Quoted 3 lines · Marcel Ziswiler

Without this patch the following error will be shown:

Quoted 8 lines
Quoted 3 lines

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.

author
#14 lore raw

PATCH 5/5 colibri_t20: enable dfu also for nand

In reply to Marcel Ziswiler

Enable USB gadget DFU functionality for NAND as well.

Signed-off-by Marcel Ziswiler <[email protected]>

---

 include/configs/colibri_t20.h | 6 ++++++
 1 file changed, 6 insertions(+)
include/configs/colibri_t20.h +6 -0
index c15f0cb..33e1ef5 100644
@@ -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 */
Signature · 1 line
2.5.5
#15 lore raw

PATCH 5/5 colibri_t20: enable dfu also for nand

include/configs/colibri_t20.h line 35

Quoted 6 lines · Marcel Ziswiler

Enable USB gadget DFU functionality for NAND as well.

include/configs/colibri_t20.h +2 -0
 
+/* 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?

include/configs/colibri_t20.h line 95

Quoted 4 lines
include/configs/colibri_t20.h +3 -0
+#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.

author
#16 lore raw

PATCH 5/5 colibri_t20: enable dfu also for nand

Quoted 7 lines · Stephen Warren
Quoted 6 lines · Marcel Ziswiler

Enable USB gadget DFU functionality for NAND as well.

include/configs/colibri_t20.h +0 -0
Quoted 6 lines

include/configs/colibri_t20.h line 35

Quoted 2 lines
include/configs/colibri_t20.h +2 -0
+/* 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.

Quoted 7 lines

include/configs/colibri_t20.h line 95

Quoted 4 lines
include/configs/colibri_t20.h +2 -0
+#define DFU_ALT_NAND_INFO	"u-boot part 0,1;ubi part 0,4"
+

?#define BOARD_EXTRA_ENV_SETTINGS \

include/configs/colibri_t20.h +1 -0
+	"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?

#17 lore raw

PATCH 5/5 colibri_t20: enable dfu also for nand

Quoted 8 lines · Marcel Ziswiler
Quoted 7 lines · Stephen Warren
Quoted 6 lines · Marcel Ziswiler

Enable USB gadget DFU functionality for NAND as well.

include/configs/colibri_t20.h +0 -0
Quoted 8 lines
Quoted 6 lines

include/configs/colibri_t20.h line 35

Quoted 2 lines
include/configs/colibri_t20.h +2 -0
+/* 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.

Quoted 5 lines

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).

Quoted 8 lines
Quoted 7 lines

include/configs/colibri_t20.h line 95

Quoted 4 lines
include/configs/colibri_t20.h +3 -0
+#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.

Quoted 4 lines

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.

author
#18 lore raw

PATCH 5/5 colibri_t20: enable dfu also for nand

Quoted 20 lines · Stephen Warren
Quoted 19 lines · Marcel Ziswiler
Quoted 17 lines · Stephen Warren

include/configs/colibri_t20.h line 35

Quoted 9 lines · Marcel Ziswiler

Enable USB gadget DFU functionality for NAND as well.

include/configs/colibri_t20.h +2 -0
 
+/* 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?

Quoted 4 lines
Quoted 1 line

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.

Quoted 11 lines
Quoted 8 lines

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.

Quoted 7 lines

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.

Quoted 14 lines
Quoted 8 lines
Quoted 8 lines

include/configs/colibri_t20.h line 95

Quoted 5 lines

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

include/configs/colibri_t20.h +1 -0
+	"dfu_alt_info=" DFU_ALT_NAND_INFO "\0" \

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

Quoted 6 lines

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?

Quoted 10 lines

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.

#19 lore raw

cover various fixes mainly for colibri_t20

In reply to Marcel Ziswiler
Quoted 22 lines · 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
Quoted 1 line

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?

author
#20 lore raw

cover various fixes mainly for colibri_t20

Quoted 30 lines · Stephen Warren
Quoted 29 lines · 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
Quoted 3 lines
Quoted 1 line

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.