| Message ID | 1473437441-938-4-git-send-email-marcel.ziswiler@toradex.com |
|---|---|
| State | Superseded |
| Delegated to: | Tom Warren |
| Headers | show |
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.
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; + }; }; };
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(-)