various fixes mainly for colibri_t20
Subsystems (3)
v3 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 be required already waiting in Marek's usb tree:
[PATCH v2] net: asix: Fix ASIX 88772B with driver model
https://www.mail-archive.com/u-boot at lists.denx.de/msg224349.html
Changes in v3:
- Add Stephen's ack.
- No change.
- Introduce new patch to honour optionality of fixed regulator enable GPIO.
- Get rid of dummy N/C GPIO work around now as the fixed regulator properly honours optionality of enable GPIO.
- Add Stephen's ack.
Changes in v2:
- As suggested by Stephen gating the CONFIG_CI_UDC_HAS_HOSTPC define with CONFIG_TEGRA20 rather than duplicating the same into all other SoC type specific header files.
- Add Anatolij's ack.
- Rename dummy regulator to reg_3v3 as suggested by Stephen.
- Keep dummy N/C GPIO to work around bug in U-Boot regulator driver requiring such node despite its binding claiming it being optional.
- No change.
- As suggested by Stephen remove last patch 5/5 colibri_t20: enable dfu also for nand.
Marcel Ziswiler (5):
tegra: usb gadget: fix ci udc operation if not hostpc capable simple panel: fix spelling of debug message
regulator: fixed: honour optionality of enable gpio
colibri_t20: fix display configuration
colibri_t20: fix usb operation and controller order
arch/arm/dts/tegra20-colibri.dts | 116 +++++++++++++++++++----------- drivers/power/regulator/fixed.c | 11 +-- drivers/video/simple_panel.c | 2 +- include/configs/tegra-common-usb-gadget.h | 2 + 4 files changed, 85 insertions(+), 46 deletions(-)
Signature · 1 line
2.5.5
PATCH v3 1/5 tegra: usb gadget: fix ci udc operation if not hostpc capable
The Tegra 2 aka T20 is not host PC capable. Therefore gate the define CONFIG_CI_UDC_HAS_HOSTPC in tegra-common-usb-gadget.h in case of CONFIG_TEGRA20.
---
Changes in v3:
- Add Stephen's ack.
Changes in v2:
- As suggested by Stephen gating the CONFIG_CI_UDC_HAS_HOSTPC define with CONFIG_TEGRA20 rather than duplicating the same into all other SoC type specific header files.
include/configs/tegra-common-usb-gadget.h | 2 ++ 1 file changed, 2 insertions(+)
include/configs/tegra-common-usb-gadget.h +2 -0
@@ -10,7 +10,9 @@#ifndef CONFIG_SPL_BUILD /* USB gadget mode support*/ +#ifndef CONFIG_TEGRA20 #define CONFIG_CI_UDC_HAS_HOSTPC +#endif /* USB mass storage protocol */ #define CONFIG_USB_FUNCTION_MASS_STORAGE /* DFU protocol */
Signature · 1 line
2.5.5
PATCH v3 2/5 simple panel: fix spelling of debug message
In reply to Marcel ZiswilerFix spelling of debug message from cnnot to cannot.
---
Changes in v3:
- No change.
Changes in v2:
- Add Anatolij's ack.
drivers/video/simple_panel.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/video/simple_panel.c +1 -1
@@ -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
PATCH v3 3/5 regulator: fixed: honour optionality of enable gpio
In reply to Marcel ZiswilerAccording to the binding documentation the fixed regulator enable GPIO is optional. However so far registration thereof failed if no enable GPIO was specified. Fix this by making it entirely optional whether an enable GPIO is used.
---
Changes in v3:
- Introduce new patch to honour optionality of fixed regulator enable GPIO.
Changes in v2: None
drivers/power/regulator/fixed.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-)
drivers/power/regulator/fixed.c +7 -4
@@ -37,11 +37,12 @@ static int fixed_regulator_ofdata_to_platdata(struct udevice *dev)/* Set type to fixed */ uc_pdata->type =REGULATOR_TYPE_FIXED ; -/* Get fixed regulator gpio desc */ +/* Get fixed regulator optional enable GPIO desc */ gpio = &dev_pdata->gpio; ret =gpio_request_by_name (dev,"gpio" ,0 , gpio,GPIOD_IS_OUT );if (ret) -debug ("Fixed regulator gpio - not found! Error: %d" , ret); +debug ("Fixed reg optional enable GPIO - not found! Error: %d" , + ret);/* Get optional ramp up delay */ dev_pdata->startup_delay_us = fdtdec_get_uint(gd->fdt_blob,
@@ -87,8 +88,9 @@ static bool fixed_regulator_get_enable(struct udevice *dev) {struct fixed_regulator_platdata *dev_pdata =dev_get_platdata (dev); +/* Enable GPIO is optional */ if (!dev_pdata->gpio.dev) -return false; +return true;return dm_gpio_get_value (&dev_pdata->gpio); }
@@ -98,8 +100,9 @@ static int fixed_regulator_set_enable(struct udevice *dev, bool enable)struct fixed_regulator_platdata *dev_pdata =dev_get_platdata (dev);int ret; +/* Enable GPIO is optional */ if (!dev_pdata->gpio.dev) -return -ENOSYS ; +return 0 ; ret =dm_gpio_set_value (&dev_pdata->gpio, enable);if (ret) {
Signature · 1 line
2.5.5
PATCH v3 3/5 regulator: fixed: honour optionality of enable gpio
Hello Marcel,
Quoted 24 lines · Marcel Ziswiler
According to the binding documentation the fixed regulator enable GPIO is optional. However so far registration thereof failed if no enable GPIO was specified. Fix this by making it entirely optional whether an enable GPIO is used.
Signed-off-by Marcel Ziswiler <[email protected]>---
Changes in v3:
- Introduce new patch to honour optionality of fixed regulator enable GPIO.Changes in v2: None
drivers/power/regulator/fixed.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-)drivers/power/regulator/fixed.c +0
-0@@ -37,11 +37,12 @@ static int fixed_regulator_ofdata_to_platdata(struct udevice *dev)/* Set type to fixed */
drivers/power/regulator/fixed.c static int fixed_regulator_ofdata_to_platdata(struct udevice *dev) · line 45
Quoted 10 lines
drivers/power/regulator/fixed.c +3
-2uc_pdata->type =REGULATOR_TYPE_FIXED ; -/* Get fixed regulator gpio desc */ +/* Get fixed regulator optional enable GPIO desc */ gpio = &dev_pdata->gpio; ret =gpio_request_by_name (dev,"gpio" ,0 , gpio,GPIOD_IS_OUT );if (ret) -debug ("Fixed regulator gpio - not found! Error: %d" , ret); +debug ("Fixed reg optional enable GPIO - not found! Error: %d" , + ret);
The "reg" is usually threated as a "register", so full word is better here.
drivers/power/regulator/fixed.c static bool fixed_regulator_get_enable(struct udevice *dev) · line 93
Quoted 11 lines
drivers/power/regulator/fixed.c +2
-1/* Get optional ramp up delay */ dev_pdata->startup_delay_us = fdtdec_get_uint(gd->fdt_blob,@@ -87,8 +88,9 @@ static bool fixed_regulator_get_enable(struct udevice *dev) {struct fixed_regulator_platdata *dev_pdata =dev_get_platdata (dev); +/* Enable GPIO is optional */ if (!dev_pdata->gpio.dev) -return false; +return true;
That is good fix :)
Quoted 4 lines
drivers/power/regulator/fixed.c +0
-0return dm_gpio_get_value (&dev_pdata->gpio); }
drivers/power/regulator/fixed.c static int fixed_regulator_set_enable(struct udevice *dev, bool enable) · line 108
Quoted 10 lines
drivers/power/regulator/fixed.c +2
-1@@ -98,8 +100,9 @@ static int fixed_regulator_set_enable(struct udevice *dev, bool enable)struct fixed_regulator_platdata *dev_pdata =dev_get_platdata (dev);int ret; +/* Enable GPIO is optional */ if (!dev_pdata->gpio.dev) -return -ENOSYS ; +return 0 ; ret =dm_gpio_set_value (&dev_pdata->gpio, enable);if (ret) {
I don't think, that returning the 0 (success) is a good idea.
We should return some error value, because this is checked by the regulator's command code.
User can be informed about what was happened - instead of getting command succeed - but without any results behind the command.
Maybe the -ENOSYS is not the best here. What about -EPERM?
We could assume, that such operation is not permitted for this device.
Best regards,
Signature · 4 lines
Przemyslaw Marczak Samsung R&D Institute Poland Samsung Electronics p.marczak at samsung.com
PATCH v3 3/5 regulator: fixed: honour optionality of enable gpio
In reply to Marcel ZiswilerQuoted 24 lines · Marcel Ziswiler
According to the binding documentation the fixed regulator enable GPIO is optional. However so far registration thereof failed if no enable GPIO was specified. Fix this by making it entirely optional whether an enable GPIO is used.
Signed-off-by Marcel Ziswiler <[email protected]>---
Changes in v3:
- Introduce new patch to honour optionality of fixed regulator enable GPIO.Changes in v2: None
drivers/power/regulator/fixed.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-)drivers/power/regulator/fixed.c +0
-0@@ -37,11 +37,12 @@ static int fixed_regulator_ofdata_to_platdata(struct udevice *dev)/* Set type to fixed */
drivers/power/regulator/fixed.c static int fixed_regulator_ofdata_to_platdata(struct udevice *dev) · line 45
Quoted 10 lines
drivers/power/regulator/fixed.c +3
-2uc_pdata->type =REGULATOR_TYPE_FIXED ; -/* Get fixed regulator gpio desc */ +/* Get fixed regulator optional enable GPIO desc */ gpio = &dev_pdata->gpio; ret =gpio_request_by_name (dev,"gpio" ,0 , gpio,GPIOD_IS_OUT );if (ret) -debug ("Fixed regulator gpio - not found! Error: %d" , ret); +debug ("Fixed reg optional enable GPIO - not found! Error: %d" , + ret);
If we're changing this, should we tighten up the error handling? It's only ENOENT that we want to ignore here, other errors should probably make fixed_regulator_ofdata_to_platdata() fail.
drivers/power/regulator/fixed.c static bool fixed_regulator_get_enable(struct udevice *dev) · line 93
Quoted 10 lines
drivers/power/regulator/fixed.c +2
-1/* Get optional ramp up delay */ dev_pdata->startup_delay_us = fdtdec_get_uint(gd->fdt_blob,@@ -87,8 +88,9 @@ static bool fixed_regulator_get_enable(struct udevice *dev) {struct fixed_regulator_platdata *dev_pdata =dev_get_platdata (dev); +/* Enable GPIO is optional */ if (!dev_pdata->gpio.dev) -return false; +return true;
This looks much better to me (I thought I had a similar change locally but I can't find it now).
drivers/power/regulator/fixed.c static int fixed_regulator_set_enable(struct udevice *dev, bool enable) · line 105
Quoted 10 lines
drivers/power/regulator/fixed.c +2
-1return dm_gpio_get_value (&dev_pdata->gpio); }@@ -98,8 +100,9 @@ static int fixed_regulator_set_enable(struct udevice *dev, bool enable)struct fixed_regulator_platdata *dev_pdata =dev_get_platdata (dev);int ret; +/* Enable GPIO is optional */ if (!dev_pdata->gpio.dev) -return -ENOSYS ; +return 0 ;
I'm not sure about this change, the current behaviour seems correct to me. After this we're pretending that fixed_set_enable(dev, false) has succeeded, when it has not.
Quoted 2 lines
drivers/power/regulator/fixed.c +0
-0ret =dm_gpio_set_value (&dev_pdata->gpio, enable);if (ret) {
PATCH v3 3/5 regulator: fixed: honour optionality of enable gpio
Quoted 11 lines · John Keeping
Quoted 9 lines · Marcel Ziswiler
According to the binding documentation the fixed regulator enable GPIO is optional. However so far registration thereof failed if no enable GPIO was specified. Fix this by making it entirely optional whether an enable GPIO is used.
drivers/power/regulator/fixed.c +0
-0@@ -98,8 +100,9 @@ static int fixed_regulator_set_enable(struct udevice *dev, bool enable)struct fixed_regulator_platdata *dev_pdata =dev_get_platdata (dev);int ret;
Quoted 8 lines
drivers/power/regulator/fixed.c static int fixed_regulator_set_enable(struct udevice *dev, bool enable) · line 105
Quoted 4 lines
drivers/power/regulator/fixed.c +2
-1+/* Enable GPIO is optional */ if (!dev_pdata->gpio.dev) -return -ENOSYS ; +return 0 ;I'm not sure about this change, the current behaviour seems correct to me. After this we're pretending that
fixed_set_enable(dev, false)has succeeded, when it has not.
That should probably be:
if (!dev_pdata->gpio.dev) {
if (!enable)
return -ENOSYS
return 0;
}PATCH v3 4/5 colibri_t20: fix display configuration
In reply to Marcel ZiswilerWithout 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.
---
Changes in v3:
- Get rid of dummy N/C GPIO work around now as the fixed regulator properly honours optionality of enable GPIO.
Changes in v2:
- Rename dummy regulator to reg_3v3 as suggested by Stephen.
- Keep dummy N/C GPIO to work around bug in U-Boot regulator driver requiring such node despite its binding claiming it being optional.
arch/arm/dts/tegra20-colibri.dts | 71 +++++++++++++++++++++++++++++----------- 1 file changed, 51 insertions(+), 20 deletions(-)
arch/arm/dts/tegra20-colibri.dts +51 -20
@@ -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,18 @@ 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>; + power-supply = <®_3v3>; + /* PWM<A> */ + pwms = <&pwm 0 5000000>; + }; + clocks { compatible = "simple-bus"; #address-cells = <1>;
@@ -104,25 +132,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_3v3: regulator at 0 { + compatible = "regulator-fixed"; + reg = <0>; + regulator-name = "+V3.3"; + regulator-min-microvolt = <3300000>; + regulator-max-microvolt = <3300000>; + regulator-always-on; + }; }; };
Signature · 1 line
2.5.5
PATCH v3 4/5 colibri_t20: fix display configuration
Quoted 7 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.
I haven't looked at this in any detail at all, but the one thing that stood out to me before is fixed, so,
PATCH v3 5/5 colibri_t20: fix usb operation and controller order
In reply to Marcel ZiswilerWithout 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.
---
Changes in v3:
- Add Stephen's ack.
Changes in v2:
- No change.
- As suggested by Stephen remove last patch 5/5 colibri_t20: enable dfu also for nand.
arch/arm/dts/tegra20-colibri.dts | 45 +++++++++++++++++++++------------------- 1 file changed, 24 insertions(+), 21 deletions(-)
arch/arm/dts/tegra20-colibri.dts +24 -21
@@ -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