linux-kernel - Re: [RFC][PATCH V2 3/3] tegra: add pwm backlight device tree nodes

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Thu, 12 Jul 2012 19:11:32 +0900
From:	Alex Courbot <acourbot@...dia.com>
To:	Simon Glass <sjg@...omium.org>
CC:	Thierry Reding <thierry.reding@...onic-design.de>,
	"linux-tegra@...r.kernel.org" <linux-tegra@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-fbdev@...r.kernel.org" <linux-fbdev@...r.kernel.org>,
	"devicetree-discuss@...ts.ozlabs.org" 
	<devicetree-discuss@...ts.ozlabs.org>
Subject: Re: [RFC][PATCH V2 3/3] tegra: add pwm backlight device tree nodes

Hi Simon,

On Thu 12 Jul 2012 06:37:33 PM JST, Simon Glass wrote:
> 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.

Cool, I'd love to see that used in U-Boot as well.

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

Yes, these are strings we are looking up to know the type of the next 
element to power on/off in the sequence. I don't like these, honestly, 
and would rather have them replaced by constants - however there is no 
way to define constants in the DT AFAIK (but I have heard some other 
persons are interested in having them too), and this is the only way I 
have found to keep the sequence readable.

> 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>;
>     }

I see a few problems with this: first, how do you know the types of your 
phandles? At step 0, we should get a regulator, step 1 is a PWM and step 
2 is a GPIO. This is why I have used strings to identify the type of the 
phandle and the number (and type) of additional arguments to parse for a 
step.

Second, I am afraid the representation in memory would be significantly 
bigger since the properties names would have to be stored as well (I am 
no DT expert, please correct me if I am wrong). Lastly, the additional 
freedom of having extra properties might make the parser more complicated.

I agree the type strings are a problem in the current form - if we could 
get constants in the device tree, that would be much better. Your way of 
representing the sequences is interesting though, if we can solve the 
type issue (and also evaluate its cost in terms of memory footprint), it 
would be interesting to consider it as well.

Alex.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/