Hi Alex,
On Thu, Jul 12, 2012 at 12:11 PM, Alex Courbot <acourbot@xxxxxxxxxx> wrote:
> 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.
That might be the 100th time that I have heard this. I have brought a
patch from Stephen Warren into our tree to solve this locally, but it
is not merged upstream.
>
>
>> 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.
Well it's similar to giving them names - you would need to look up the
phandle and see its compatible string or which driver type owns it.
Maybe too complicated, and no such infrastructure exists, so:
>> power_on_sequence {
>> step@0 {
type = "regulator";
>> phandle = <&backlight_reg>;
>> enable = <1>;
>> post-delay = <10>;
>> }
>> step@1 {
type = "pwm";
>> phandle = <&pwm 2 5000000>;
>> }
>> step@2 {
type = "gpio";
>> phandle = <&gpio 28 0>;
>> enable = <1>;
>> }
>
> 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.
The property names are only stored one, in the string table. I am not
sure about parsing complexity, but it might actually be easier.
>
> 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.
At a guess:
>>> + power-on-sequence = "REGULATOR", "power", <1>,
>>> + "DELAY", <10>,
>>> + "PWM", "backlight", <1>,
>>> + "GPIO", "enable", <1>;
About 106 bytes I think
>> step@0 { 16
type = "regulator"; 24
>> phandle = <&backlight_reg>; 16
>> enable = <1>; 16
>> post-delay = <10>; 16
>> }
>> step@1 { 16
type = "pwm"; 16
>> phandle = <&pwm 2 5000000>; 24
>> }
>> step@2 { 16
type = "gpio"; 20
>> phandle = <&gpio 28 0>; 24
>> enable = <1>; 16
>> }
220?
>From my understanding mixing strings and numbers in a property is
frowned on though.
>
> Alex.
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
