Message-Id: <20121122133941.24B883E129E@localhost>
Date:	Thu, 22 Nov 2012 13:39:41 +0000
From:	Grant Likely <grant.likely@...retlab.ca>
To:	Alex Courbot <acourbot@...dia.com>
Cc:	Alexandre Courbot <gnurou@...il.com>,
	"linux-fbdev@...r.kernel.org" <linux-fbdev@...r.kernel.org>,
	Thierry Reding <thierry.reding@...onic-design.de>,
	Stephen Warren <swarren@...dia.com>,
	Arnd Bergmann <arnd@...db.de>,
	"linux-pm@...r.kernel.org" <linux-pm@...r.kernel.org>,
	"devicetree-discuss@...ts.ozlabs.org" 
	<devicetree-discuss@...ts.ozlabs.org>,
	Mark Brown <broonie@...nsource.wolfsonmicro.com>,
	Mark Zhang <markz@...dia.com>,
	Rob Herring <rob.herring@...xeda.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Anton Vorontsov <cbouatmailru@...il.com>,
	"linux-tegra@...r.kernel.org" <linux-tegra@...r.kernel.org>,
	David Woodhouse <dwmw2@...radead.org>,
	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCHv9 1/3] Runtime Interpreted Power Sequences

On Wed, 21 Nov 2012 13:23:06 +0900, Alex Courbot <acourbot@...dia.com> wrote:
> Hi Grant,
> 
> On Wednesday 21 November 2012 05:54:29 Grant Likely wrote:
> > > With the advent of the device tree and of ARM kernels that are not
> > > board-tied, we cannot rely on these board-specific hooks anymore but
> > 
> > This isn't strictly true. It is still perfectly fine to have board
> > specific code when necessary. However, there is strong encouragement to
> > enable that code in device drivers as much as possible and new board
> > files need to have very strong justification.
> 
> But doesn't introducing board-specific code into the kernel just defeats the 
> purpose of the DT? If we extend this logic, we are heading straight back to 
> board-definition files. To a lesser extent than before I agree, but the problem 
> is fundamentally the same.

There is *always* board specific code. There is always something fiddly
enough, complex enough, or just plain ugly enough that it is best
handled in C code. The trick is to make the board-specific stuff the
exception, not the rule.

When you can factor out common behavour (like you are doing here), then
it is a really good candidate for a common binding, but still please
always ask yourself the question is this going to make things better or
worse in the long run.

> > I'm thinking about the scripts as trivial-to-parse ascii strings that
> > have a very simple set of commands. The commands use resources already
> > defined in the node. ie. 'g0' refers to the first entry in the gpios
> > property. 'r0' for the regulator, 'p0' for the pwms, 'd' means delay. By
> > no means take this as the format to use, it is just an example off the
> > top of my head, but it is already way easier to work with than putting
> > each command into a node.
> 
> I'd argue that this opens the door wide open towards having a complete 
> interpreter in the device tree. At least DT nodes restrict what we can do 
> conveniently.

I disagree... see below.

> > The trick is still to define a syntax that doesn't fall apart when it
> > needs to be extended. I would also like to get opinions on whether or
> > not conditionals or loops should be supported (ie. loop waiting for a
> > status to change). If they should then we need to be a lot more careful
> > about the design (and due to my aforementioned nervousness, somebody may
> > need to get me therapy).
> 
> That's IMHO the main argument in favor of using DT nodes here (the syntax 
> extension, not your therapy).

hahaha :-)

> They can be extended simply by adding properties 
> to them, and I was relying on this to make power seqs extensible while keeping 
> things consistent (and backward-compatible). For instance, when we want to 
> support voltage setting in regulators we can just add a "voltage" property for 
> that.
> 
> So to sum up the pros of the current node-base representation:
> - give a "data like" representation of powering sequences (what they should 
> be, actually)
> - puts sanity barriers for not turning power seqs into a real code interpreter
> - easily extensible
> 
> There are probably a few cons, the numbering of steps being one, but at least 
> we don't need to design a new DSL and care too much about extensibility which 
> is, in the nodes case, built-in and free.

No matter how it is encoded, this *IS* a new DSL. Using nodes and
properties doesn't change that. Extensibility is no more built-in and
free with nodes that it is with another encoding. If there aren't clear
guidelines on how to extend it then we'll get weird stuff. For example,
even with the nodes case someone might do this:

	step3 {
		type = "loop";
		count = 10;
		step1 {
			type = "gpio";
			gpio = <&gpio 1>;
			value = 1;
		};
		step2 {
			type = "delay";
			value = 10000;
		};
		step3 {
			type = "gpio";
			gpio = <&gpio 1>;
			value = 0;
		};
		step4 {
			type = "delay";
			value = 10000;
		};
	};

And even while cringing as I type the above, I also have to consider
that looping may just be a valid use case for sequences.

And even here, a very simple sequence fragment required 22 lines of
code.

Next, I'm concerned about where these will show up. Say for instance
there needs to be a power sequence added to an spi bus node. Already spi
bus child nodes have a defined meaning; they are spi slaves. How then
should the sequence be attached to the spi bus?

> If that makes you feel better, maybe we can try and fix what is wrong with the 
> current bindings instead of introducing a new language that will be, by 
> nature, more complex to handle and difficult to extend without breaking things?

Okay, here are 3 concrete objections with the proposed binding:
- The syntax concerns of it being too verbose and it effectively uses
  line numbers for ordering (Do you remember fighting with BASIC?).
- There are many devices that won't be able to use the binding because
  they already have a meaning for child nodes
- I think resources should be declared separate from the sequence based
  on the assumption that multiple steps will be operating on the same
  resource.

I do think that each sequence should be contained within a single
property, but I'm open to other suggestions.

g.
--
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/

Powered by blists - more mailing lists