Hi Alex,
On Mon, Jul 9, 2012 at 8:08 AM, Alexandre Courbot <acourbot@xxxxxxxxxx> wrote:
> Signed-off-by: Alexandre Courbot <acourbot@xxxxxxxxxx>
> ---
> arch/arm/boot/dts/tegra20-ventana.dts | 31 +++++++++++++++++++++++++++++++
> arch/arm/boot/dts/tegra20.dtsi | 2 +-
> 2 files changed, 32 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/boot/dts/tegra20-ventana.dts b/arch/arm/boot/dts/tegra20-ventana.dts
> index be90544..c67d9e1 100644
> --- a/arch/arm/boot/dts/tegra20-ventana.dts
> +++ b/arch/arm/boot/dts/tegra20-ventana.dts
> @@ -317,6 +317,37 @@
> bus-width = <8>;
> };
>
I would like to do something similar in U-Boot for Tegra, although
perhaps not right away. For now I will go with something considerably
simpler! But if this is merged into the kernel we will move to it in
U-Boot. Anyway here are my comments:
> + backlight {
> + compatible = "pwm-backlight";
> + brightness-levels = <0 16 32 48 64 80 96 112 128 144 160 176 192 208 224 240 255>;
We seem to have a lot of these - should we move to a range and step size?
> + default-brightness-level = <12>;
> +
> + pwms = <&pwm 2 5000000>;
> + pwm-names = "backlight";
> + power-supply = <&backlight_reg>;
> + enable-gpios = <&gpio 28 0>;
> +
> + power-on-sequence = "REGULATOR", "power", <1>,
> + "DELAY", <10>,
> + "PWM", "backlight", <1>,
> + "GPIO", "enable", <1>;
So the names REGULATOR, DELAY, etc. here are looked up through some
existing mechanism? In general I am not a big fan of mixing strings
and numbers in a property. Maybe something like:
power_on_sequence {
step@0 {
phandle = <&backlight_reg>;
enable = <1>;
post-delay = <10>;
}
step@1 {
phandle = <&pwm 2 5000000>;
}
step@2 {
phandle = <&gpio 28 0>;
enable = <1>;
}
> + power-off-sequence = "GPIO", "enable", <0>,
> + "PWM", "backlight", <0>,
> + "DELAY", <10>,
> + "REGULATOR", "power", <0>;
> + };
> +
> + backlight_reg: fixedregulator@176 {
> + compatible = "regulator-fixed";
> + regulator-name = "backlight_regulator";
> + regulator-min-microvolt = <1800000>;
> + regulator-max-microvolt = <1800000>;
> + gpio = <&gpio 176 0>;
> + startup-delay-us = <0>;
> + enable-active-high;
> + regulator-boot-off;
> + };
> +
> sound {
> compatible = "nvidia,tegra-audio-wm8903-ventana",
> "nvidia,tegra-audio-wm8903";
> diff --git a/arch/arm/boot/dts/tegra20.dtsi b/arch/arm/boot/dts/tegra20.dtsi
> index 405d167..67a6cd9 100644
> --- a/arch/arm/boot/dts/tegra20.dtsi
> +++ b/arch/arm/boot/dts/tegra20.dtsi
> @@ -123,7 +123,7 @@
> status = "disabled";
> };
>
> - pwm {
> + pwm: pwm {
> compatible = "nvidia,tegra20-pwm";
> reg = <0x7000a000 0x100>;
> #pwm-cells = <2>;
> --
> 1.7.11.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
Regards,
Simon
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
