Hi Alex,

On Thu, Jul 12, 2012 at 12:11 PM, Alex Courbot [off-list ref] wrote:

Hi Simon,


On Thu 12 Jul 2012 06:37:33 PM JST, Simon Glass wrote:

quoted

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.

quoted

quoted

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

quoted

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:

quoted

power_on_sequence {
    step@0 {

            type = "regulator";

quoted

       phandle = <&backlight_reg>;
       enable = <1>;
       post-delay = <10>;
    }
    step@1 {

            type = "pwm";

quoted

       phandle = <&pwm 2 5000000>;
    }
    step@2 {

            type = "gpio";

quoted

       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:

quoted

quoted

+               power-on-sequence = "REGULATOR", "power", <1>,
+                                   "DELAY", <10>,
+                                   "PWM", "backlight", <1>,
+                                   "GPIO", "enable", <1>;

About 106 bytes I think

quoted

    step@0 { 16

            type = "regulator"; 24

quoted

       phandle = <&backlight_reg>; 16
       enable = <1>; 16
       post-delay = <10>; 16
    }
    step@1 { 16

            type = "pwm"; 16

quoted

       phandle = <&pwm 2 5000000>; 24
    }
    step@2 { 16

            type = "gpio"; 20

quoted

       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