Message-ID: <6568619.UkU5qISONv@vostro.rjw.lan>
Date:	Sat, 04 Oct 2014 02:13:23 +0200
From:	"Rafael J. Wysocki" <rjw@...ysocki.net>
To:	Mark Rutland <mark.rutland@....com>
Cc:	Mika Westerberg <mika.westerberg@...ux.intel.com>,
	Arnd Bergmann <arnd@...db.de>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-acpi@...r.kernel.org" <linux-acpi@...r.kernel.org>,
	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Linus Walleij <linus.walleij@...aro.org>,
	Alexandre Courbot <gnurou@...il.com>,
	Dmitry Torokhov <dmitry.torokhov@...il.com>,
	Bryan Wu <cooloney@...il.com>,
	Lee Jones <lee.jones@...aro.org>,
	"grant.likely@...aro.org" <grant.likely@...aro.org>,
	Aaron Lu <aaron.lu@...el.com>,
	Darren Hart <dvhart@...ux.intel.com>
Subject: Re: [PATCH v3 04/15] ACPI: Document ACPI device specific properties

On Friday, October 03, 2014 03:35:47 PM Mark Rutland wrote:
> On Fri, Oct 03, 2014 at 03:38:49PM +0100, Rafael J. Wysocki wrote:
> > On Friday, October 03, 2014 02:58:26 PM Mark Rutland wrote:
> > > On Fri, Oct 03, 2014 at 03:03:51AM +0100, Rafael J. Wysocki wrote:
> > > > On Thursday, October 02, 2014 04:36:54 PM Mika Westerberg wrote:
> > > > > On Thu, Oct 02, 2014 at 02:46:30PM +0200, Arnd Bergmann wrote:
> > > > > > On Thursday 02 October 2014 15:15:08 Mika Westerberg wrote:
> > > > 
> > > > [cut]
> > > > 
> > > > > 
> > > > > Putting everything to a single package results this:
> > > > > 
> > > > > 		Package () { "pwms", Package () {"led-red", ^PWM0, 0, 10, "led-green", ^PWM0, 1, 10 }}
> > > > > 
> > > > > But I think the below looks better:
> > > > 
> > > > I agree.
> > > > 
> > > > > 		Package () { "pwms", Package () {^PWM0, 0, 10, ^PWM0, 1, 10 }}
> > > > > 		Package () { "pwm-names", Package () {"led-red", "led-green"}}
> > > > > 
> > > > > and it is trivial to match with the corresponding DT fragment.
> > > > > 
> > > > > > 	}
> > > > > > 
> > > > > > vs.
> > > > > > 
> > > > > > 	pwm-slave {
> > > > > > 		pwms = <&pwm0 0 10>, <&pwm1 1 20>;
> > > > > > 		pwm-names = "led-red", "led-green";
> > > > > > 	};
> > > > > > 
> > > > > 
> > > > > I don't have strong feelings which way it should be. The current
> > > > > implementation limits references so that you can have only integer
> > > > > arguments, like {ref0, int, int, ref1, int} but if people think it is
> > > > > better to allow strings there as well, it can be changed.
> > > > > 
> > > > > I would like to get comments from Darren and Rafael about this, though.
> > > > 
> > > > In my opinion there needs to be a "canonical" representation of the
> > > > binding that people always can expect to work.  It seems reasonable to
> > > > use the one exactly matching the DT representation for that.
> > > 
> > > I don't follow. The two forms would share the same high-level accessors,
> > > but the binary representation is already different. Why should we choose
> > > the inferior layout given they are already different binary formats?
> > 
> > Well, why is it inferior in the first place?  It represents the same information
> > and I'm not sure why the binary formats matter here?
> 
> Because people get the format wrong regardless of documentation. The
> format:
> 
> Package () {
> 	Package () { ^ref1, data, data },
> 	Package () { ^ref2, data },
> 	Package () { ^ref3, data, data, data },
> }
> 
> Is superior to the format:
> 
> Package () { ^ref1, data, data, ^ref2, data, ^ref3, data, data, data }
> 
> Because in the former you have delimiters that can be used to verify
> each tuple. Imagine someone misses a data element for one of these
> tuples. In the former layout you can detect this easily while in the
> latter you cannot.

I agree with this particular thing (although other people seem to have
problems with too many package nesting levels) but I'm not sure what that
has to do with the example given above (let me quote again):

> Putting everything to a single package results this:
> 
> 	Package () { "pwms", Package () {"led-red", ^PWM0, 0, 10, "led-green", ^PWM0, 1, 10 }}
> 
> But I think the below looks better:
> 
> 	Package () { "pwms", Package () {^PWM0, 0, 10, ^PWM0, 1, 10 }}
> 	Package () { "pwm-names", Package () {"led-red", "led-green"}}
> 
> and it is trivial to match with the corresponding DT fragment.

that I was commenting.  Both cases contains the

	Package () { ^ref1, data, data, ^ref2, data, ^ref3, data, data, data }

format that you don't like, don't they?

> Additionally, the former can represent variadic phandle/reference + args
> formats, which the latter cannot. The DT pinctrl bindings look the way
> they do because we can't represent variadic args in DT due to a lack of
> delimiters.
> 
> You have the ability to embed structure in the binary format. Throwing
> this away because it doesn't quite match DT does not to me sounds like
> the right tradeoff.
> 
> > If I'm to create a _DSD with that information and have a DT template, it
> > surely is easier to copy it exactly than to figure out how to resolve it
> > to represent something I can actually put in there. 
> 
> Sure, you can put something together fast. However, that doesn't make it
> necessarily better.
> 
> If you want to make reusing DT templates easier, why not just embed a
> DTB and be done with it?

Because that sucks?

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
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