[U-Boot] [PATCH v1 0/5] 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@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(-)
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
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.
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.
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;
On Fri, 9 Sep 2016 18:10:38 +0200 Marcel Ziswiler marcel.ziswiler@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
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@50000000 { - status = "okay"; dc@54200000 { - status = "okay"; rgb { status = "okay"; nvidia,panel = <&lcd_panel>; + display-timings { + timing@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@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 = <®_dummy>; + /* PWM<A> */ + pwms = <&pwm 0 5000000>; + }; + clocks { compatible = "simple-bus"; #address-cells = <1>; @@ -104,25 +133,28 @@ }; };
- pwm: pwm@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@0 { + compatible = "regulator-fixed"; + reg = <0>; + regulator-name = "Dummy"; + /* Dummy N/C */ + gpio = <&gpio TEGRA_GPIO(V, 7) GPIO_ACTIVE_HIGH>; + enable-active-high; + }; }; };
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@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.
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@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.
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.
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.
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@7000d000"; i2c1 = "/i2c@7000c000"; i2c2 = "/i2c@7000c400"; - usb0 = "/usb@c5008000"; - usb1 = "/usb@c5000000"; - usb2 = "/usb@c5004000"; sdhci0 = "/sdhci@c8000600"; + usb0 = "/usb@c5000000"; + usb1 = "/usb@c5004000"; /* on-module only, for ASIX */ + usb2 = "/usb@c5008000"; };
host1x@50000000 { @@ -43,24 +43,6 @@ }; };
- usb@c5000000 { - statuc = "okay"; - dr_mode = "otg"; - }; - - usb@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@c5008000 { - statuc = "okay"; - /* USBH_PEN */ - nvidia,vbus-gpio = <&gpio TEGRA_GPIO(W, 2) GPIO_ACTIVE_LOW>; - }; - nand-controller@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@c5000000 { + status = "okay"; + dr_mode = "otg"; + }; + + /* EHCI instance 1: ULPI -> USB3340 -> AX88772B */ + usb@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@c5008000 { + status = "okay"; + /* USBH_PEN */ + nvidia,vbus-gpio = <&gpio TEGRA_GPIO(W, 2) GPIO_ACTIVE_LOW>; + }; + sdhci@c8000600 { status = "okay"; bus-width = <4>;
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?
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.
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 */
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.
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?
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.
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.
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@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?
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@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.
participants (3)
-
Anatolij Gustschin -
Marcel Ziswiler -
Stephen Warren