Re: [PATCHv9 1/3] Runtime Interpreted Power Sequences

On Wed, 21 Nov 2012 10:31:34 +0900, Mark Brown [off-list ref] wrote:

On Tue, Nov 20, 2012 at 09:54:29PM +0000, Grant Likely wrote:

quoted

On Sat, 17 Nov 2012 19:55:45 +0900, Alexandre Courbot [off-list ref] wrote:

quoted

quoted

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

quoted

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.

This isn't the message that's gone over, and even for device drivers
everyone seems to be taking the whole device tree thing as a move to
pull all data out of the kernel.  In some cases there are some real
practical advantages to doing this but a lot of the people making these
changes seem to view having things in DT as a goal in itself.

I know, and I do apologize for my part in not communicating and
shepherding enough. For anyone reading; please be pragmatic on putting
things into the DT. If a binding starts to look really complex, ask
whether or not it helps things.

quoted

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.

It does appear to have some legibility challenges, especially with using
the indexes into the arrays of resources.  On the other hand the arrays
should be fairly small.

This is merely an example off the top of my head. I'm in no way wed to
the syntax.

quoted

quoted

+Platform Data Format
+--------------------
+All relevant data structures for declaring power sequences are located in
+include/linux/power_seq.h.

quoted

Hmm... If sequences are switched to a string instead, then platform_data
should also use it. The power sequence data structures can be created at
runtime by parsing the string.

Seems like a step backwards to me - why not let people store the more
parsed version of the data?  It's going to be less convenient for users
on non-DT systems.

As I said in my earlier reviews I think this is a useful thing to have
as a utility library for drivers independantly of the DT bindings, it
would allow drivers to become more data driven.  Perhaps we can rework
the series so that the DT bindings are added as a final patch?  All the
rest of the code seems OK.

I'm fine with that if the bindings don't get merged yet.

g.