[RFC] of: Allow for experimental device tree bindings

[RFC] of: Allow for experimental device tree bindings

[Thierry Reding]

Past and recent discussions have shown that there's some concensus that
device tree bindings should be considered an ABI and therefore need to
remain backwards-compatible once code to implement them is included in
the final release of a Linux kernel.

At the same time there is a desire to keep some manoeuvre while we're
trying to figure things out. The fact is that many of us lack the
experience to design good bindings from the start. At the same time
there is a shortage of people that can review bindings and help design
better ones.

Progress and the addition of new features (and restoration of features
that used to work before the advent of DT for that matter) are blocked
to a large degree because of that.

This patch attempts to restore some degree of freedom by introducing an
easy way to mark device tree bindings as experimental as well as a way
for users to disable the use of experimental bindings if they choose
functionality at the cost of potential device tree incompatibilities.

Bindings are marked experimental by prefixing the compatible value with
an exclamation mark (!). In order to make it clear that experimental
bindings are undesirable in the long run, a warning will be output when
an experimental binding is encountered.

Signed-off-by: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
 drivers/of/Kconfig |  7 +++++++
 drivers/of/base.c  | 32 +++++++++++++++++++++++++++++++-
 2 files changed, 38 insertions(+), 1 deletion(-)

diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig
index 78cc760..dc482f8 100644
--- a/drivers/of/Kconfig
+++ b/drivers/of/Kconfig
@@ -24,6 +24,13 @@ config OF_SELFTEST
 
 	  If unsure, say N here, but this option is safe to enable.
 
+config OF_EXPERIMENTAL
+	bool "Support experimental device tree bindings"
+	help
+	  This option allows experimental device tree bindings to be used.
+	  Note that experimental bindings are subject to change, possibly
+	  requiring the DTB to be updated.
+
 config OF_FLATTREE
 	bool
 	select DTC
diff --git a/drivers/of/base.c b/drivers/of/base.c
index a96f850..b0b8371 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -342,6 +342,36 @@ struct device_node *of_get_cpu_node(int cpu, unsigned int *thread)
 }
 EXPORT_SYMBOL(of_get_cpu_node);
 
+static int of_compat_match(const char *device, const char *driver,
+			   const struct device_node *np)
+{
+	const char *dev = device;
+	const char *drv = driver;
+
+	if (device[0] == '!')
+		device++;
+
+	if (driver[0] == '!')
+		driver++;
+
+	if (of_compat_cmp(device, driver, strlen(driver)) != 0)
+		return 0;
+
+	/* check if binding is experimental */
+	if (dev != device || drv != driver) {
+		pr_warn("of: device %s (%s) uses an experimental binding\n",
+			np->name, np->full_name);
+
+		/* don't match if we don't want experimental bindings */
+		if (!IS_ENABLED(CONFIG_OF_EXPERIMENTAL)) {
+			pr_err("of: refusing to use binding \"%s\"\n", device);
+			return 0;
+		}
+	}
+
+	return 1;
+}
+
 /** Checks if the given "compat" string matches one of the strings in
  * the device's "compatible" property
  */
@@ -355,7 +385,7 @@ static int __of_device_is_compatible(const struct device_node *device,
 	if (cp == NULL)
 		return 0;
 	while (cplen > 0) {
-		if (of_compat_cmp(cp, compat, strlen(compat)) == 0)
+		if (of_compat_match(cp, compat, device))
 			return 1;
 		l = strlen(cp) + 1;
 		cp += l;
-- 
1.8.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [Ksummit-2013-discuss] [RFC] of: Allow for experimental device tree bindings

[David Woodhouse]

[-- Attachment #1.1: Type: text/plain, Size: 747 bytes --]

On Wed, 2013-10-23 at 17:06 +0200, Thierry Reding wrote:
> +       /* check if binding is experimental */
> +       if (dev != device || drv != driver) {
> +               pr_warn("of: device %s (%s) uses an experimental binding\n",
> +                       np->name, np->full_name);
> +

In the discussions earlier I think we decided that this should set a
taint flag too. If you've built a kernel with CONFIG_OF_EXPERIMENTAL
(which I think we were calling CONFIG_UNSTABLE_DT) then you have no
expectation that it will boot tomorrow, although it might work with your
DTB today.

-- 
David Woodhouse                            Open Source Technology Centre
David.Woodhouse@intel.com                              Intel Corporation

[-- Attachment #1.2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 5745 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [Ksummit-2013-discuss] [RFC] of: Allow for experimental device tree bindings

[Guenter Roeck]

On Wed, Oct 23, 2013 at 05:05:32PM +0100, David Woodhouse wrote:
> On Wed, 2013-10-23 at 17:06 +0200, Thierry Reding wrote:
> > +       /* check if binding is experimental */
> > +       if (dev != device || drv != driver) {
> > +               pr_warn("of: device %s (%s) uses an experimental binding\n",
> > +                       np->name, np->full_name);
> > +
> 
> In the discussions earlier I think we decided that this should set a
> taint flag too. If you've built a kernel with CONFIG_OF_EXPERIMENTAL
> (which I think we were calling CONFIG_UNSTABLE_DT) then you have no
> expectation that it will boot tomorrow, although it might work with your
> DTB today.
> 
So to avoid tainting the kernel and clogging the kernel log I'll have
to remove all the "!" from the dt sources, or not use any "!" in the
dt bindings in the first place. Given that, not sure if anyone will
really use this mechanism. And CONFIG_OF_EXPERIMENTAL/CONFIG_UNSTABLE_DT
might have the same ultimate fate as CONFIG_EXPERIMENTAL.

Guenter
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [Ksummit-2013-discuss] [RFC] of: Allow for experimental device tree bindings

[David Woodhouse]

[-- Attachment #1: Type: text/plain, Size: 622 bytes --]

On Wed, 2013-10-23 at 09:55 -0700, Guenter Roeck wrote:
> 
> So to avoid tainting the kernel and clogging the kernel log I'll have
> to remove all the "!" from the dt sources, or not use any "!" in the
> dt bindings in the first place. Given that, not sure if anyone will
> really use this mechanism. And CONFIG_OF_EXPERIMENTAL/CONFIG_UNSTABLE_DT
> might have the same ultimate fate as CONFIG_EXPERIMENTAL.

It's not as if it's hard for you to patch out the check if you really
want to do that. This is about making it clear to people what they
should expect if they use staging code/bindings. 

-- 
dwmw2


[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 5745 bytes --]

Re: [Ksummit-2013-discuss] [RFC] of: Allow for experimental device tree bindings

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 953 bytes --]

On Wed, Oct 23, 2013 at 06:05:16PM +0100, David Woodhouse wrote:
> On Wed, 2013-10-23 at 09:55 -0700, Guenter Roeck wrote:
> > 
> > So to avoid tainting the kernel and clogging the kernel log I'll have
> > to remove all the "!" from the dt sources, or not use any "!" in the
> > dt bindings in the first place. Given that, not sure if anyone will
> > really use this mechanism. And CONFIG_OF_EXPERIMENTAL/CONFIG_UNSTABLE_DT
> > might have the same ultimate fate as CONFIG_EXPERIMENTAL.
> 
> It's not as if it's hard for you to patch out the check if you really
> want to do that. This is about making it clear to people what they
> should expect if they use staging code/bindings. 

I agree. The kernel is open source, so people are free to patch it in
any way they want. The best we can do is to come up with a good solution
in an upstream kernel. But there's no way we can control things beyond
that, and we don't have to.

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

Re: [Ksummit-2013-discuss] [RFC] of: Allow for experimental device tree bindings

[Thierry Reding]

On Wed, Oct 23, 2013 at 05:05:32PM +0100, David Woodhouse wrote:
> On Wed, 2013-10-23 at 17:06 +0200, Thierry Reding wrote:
> > +       /* check if binding is experimental */
> > +       if (dev != device || drv != driver) {
> > +               pr_warn("of: device %s (%s) uses an experimental binding\n",
> > +                       np->name, np->full_name);
> > +
> 
> In the discussions earlier I think we decided that this should set a
> taint flag too.

A taint flag seems somewhat drastic. It's not like using an experimental
binding should have an influence on the stability of the running kernel.
I always thought that taint flags were supposed to flag conditions where
code of unknown origin or code known to be broken was being executed
because they may destabilize the running kernel.

The worst that should happen if you run an experimental binding is that
some part of the system will just not come up.

> If you've built a kernel with CONFIG_OF_EXPERIMENTAL (which I think we
> were calling CONFIG_UNSTABLE_DT) then you have no expectation that it
> will boot tomorrow, although it might work with your DTB today.

Yes, something like that. Although I would hope that we can actually
come up with reasonably simple and stable bindings for essential things
required to boot. That way you could at least output some kind of
warning to the user that something went wrong. If the system just won't
boot at all then a taint flag won't help very much either.

Thierry
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [Ksummit-2013-discuss] [RFC] of: Allow for experimental device tree bindings

[Stephen Warren]

On 10/23/2013 07:51 PM, Thierry Reding wrote:
> On Wed, Oct 23, 2013 at 05:05:32PM +0100, David Woodhouse wrote:
>> On Wed, 2013-10-23 at 17:06 +0200, Thierry Reding wrote:
>>> +       /* check if binding is experimental */
>>> +       if (dev != device || drv != driver) {
>>> +               pr_warn("of: device %s (%s) uses an experimental binding\n",
>>> +                       np->name, np->full_name);
>>> +
>>
>> In the discussions earlier I think we decided that this should set a
>> taint flag too.
> 
> A taint flag seems somewhat drastic. It's not like using an experimental
> binding should have an influence on the stability of the running kernel.
> I always thought that taint flags were supposed to flag conditions where
> code of unknown origin or code known to be broken was being executed
> because they may destabilize the running kernel.
> 
> The worst that should happen if you run an experimental binding is that
> some part of the system will just not come up.

IIRC, the purpose of the taint flag was to make it clear that the kernel
or DT was not expected to function in the future, so don't be surpised
if you upgrade it, and it stops working, without you taking explicit
action, such as revising your DT to match the new kernel or vice-versa.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [Ksummit-2013-discuss] [RFC] of: Allow for experimental device tree bindings

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 2439 bytes --]

On Thu, Oct 24, 2013 at 11:26:19PM +0100, Stephen Warren wrote:
> On 10/23/2013 07:51 PM, Thierry Reding wrote:
> > On Wed, Oct 23, 2013 at 05:05:32PM +0100, David Woodhouse wrote:
> >> On Wed, 2013-10-23 at 17:06 +0200, Thierry Reding wrote:
> >>> +       /* check if binding is experimental */
> >>> +       if (dev != device || drv != driver) {
> >>> +               pr_warn("of: device %s (%s) uses an experimental binding\n",
> >>> +                       np->name, np->full_name);
> >>> +
> >>
> >> In the discussions earlier I think we decided that this should set a
> >> taint flag too.
> > 
> > A taint flag seems somewhat drastic. It's not like using an experimental
> > binding should have an influence on the stability of the running kernel.
> > I always thought that taint flags were supposed to flag conditions where
> > code of unknown origin or code known to be broken was being executed
> > because they may destabilize the running kernel.
> > 
> > The worst that should happen if you run an experimental binding is that
> > some part of the system will just not come up.
> 
> IIRC, the purpose of the taint flag was to make it clear that the kernel
> or DT was not expected to function in the future, so don't be surpised
> if you upgrade it, and it stops working, without you taking explicit
> action, such as revising your DT to match the new kernel or vice-versa.

I understand that, but I was arguing that it doesn't match existing uses
of taint flags. The various flags that are currently defined all seem to
be set whenever some event occurs that could cause instability of the
currently running system, such as loading a proprietary or out-of-tree
module, forcing a module to be loaded, overriding firmware parameters...

All those seem to have the goal of appearing in crash logs, so that
whoever looks at the bug report can point users somewhere else since the
problem is likely to be caused by their own (bad) decision. Or ask users
to reproduce crashes or bugs without doing whatever they did to cause
the taint flag(s) to be set.

Experimental bindings shouldn't cause any crashes in the first place, or
not cause memory corruption or similar for that matter. If we don't want
to support an experimental binding, then all we should do is not support
any functionality that relies on them. That doesn't mean that runtime
stability is in any way affected.

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

Re: [Ksummit-2013-discuss] [RFC] of: Allow for experimental device tree bindings

[Stephen Warren]

On 10/25/2013 09:22 AM, Thierry Reding wrote:
> On Thu, Oct 24, 2013 at 11:26:19PM +0100, Stephen Warren wrote:
>> On 10/23/2013 07:51 PM, Thierry Reding wrote:
>>> On Wed, Oct 23, 2013 at 05:05:32PM +0100, David Woodhouse
>>> wrote:
>>>> On Wed, 2013-10-23 at 17:06 +0200, Thierry Reding wrote:
>>>>> +       /* check if binding is experimental */ +       if
>>>>> (dev != device || drv != driver) { +
>>>>> pr_warn("of: device %s (%s) uses an experimental
>>>>> binding\n", +                       np->name,
>>>>> np->full_name); +
>>>> 
>>>> In the discussions earlier I think we decided that this
>>>> should set a taint flag too.
>>> 
>>> A taint flag seems somewhat drastic. It's not like using an
>>> experimental binding should have an influence on the stability
>>> of the running kernel. I always thought that taint flags were
>>> supposed to flag conditions where code of unknown origin or
>>> code known to be broken was being executed because they may
>>> destabilize the running kernel.
>>> 
>>> The worst that should happen if you run an experimental binding
>>> is that some part of the system will just not come up.
>> 
>> IIRC, the purpose of the taint flag was to make it clear that the
>> kernel or DT was not expected to function in the future, so don't
>> be surpised if you upgrade it, and it stops working, without you
>> taking explicit action, such as revising your DT to match the new
>> kernel or vice-versa.
> 
> I understand that, but I was arguing that it doesn't match existing
> uses of taint flags. The various flags that are currently defined
> all seem to be set whenever some event occurs that could cause
> instability of the currently running system, such as loading a
> proprietary or out-of-tree module, forcing a module to be loaded,
> overriding firmware parameters...

Executing a driver that supports an unstable binding does produce
instability in the system; the kernel configuration might not continue
to work if rebooted with an updated DT. Admittedly, this is a slightly
different concept than other taint flags, but seems like a logical
extension.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC] of: Allow for experimental device tree bindings

[Stephen Warren]

On 10/23/2013 04:06 PM, Thierry Reding wrote:
> Past and recent discussions have shown that there's some concensus that
> device tree bindings should be considered an ABI and therefore need to
> remain backwards-compatible once code to implement them is included in
> the final release of a Linux kernel.
> 
> At the same time there is a desire to keep some manoeuvre while we're
> trying to figure things out. The fact is that many of us lack the
> experience to design good bindings from the start. At the same time
> there is a shortage of people that can review bindings and help design
> better ones.
> 
> Progress and the addition of new features (and restoration of features
> that used to work before the advent of DT for that matter) are blocked
> to a large degree because of that.
> 
> This patch attempts to restore some degree of freedom by introducing an
> easy way to mark device tree bindings as experimental as well as a way
> for users to disable the use of experimental bindings if they choose
> functionality at the cost of potential device tree incompatibilities.
> 
> Bindings are marked experimental by prefixing the compatible value with
> an exclamation mark (!). In order to make it clear that experimental
> bindings are undesirable in the long run, a warning will be output when
> an experimental binding is encountered.

> diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig

> +static int of_compat_match(const char *device, const char *driver,
> +			   const struct device_node *np)
> +{
> +	const char *dev = device;
> +	const char *drv = driver;
> +
> +	if (device[0] == '!')
> +		device++;
> +
> +	if (driver[0] == '!')
> +		driver++;
> +
> +	if (of_compat_cmp(device, driver, strlen(driver)) != 0)
> +		return 0;

Do we really want to polute the drivers and DT files with a ! in the
compatible values? I thought we'd considered that, but chosen having the
drivers that use unstable bindings depend on a Kconfig option as an
alternative, not an additional step?

The one issue with doing this is that if a binding is thought to be
unstable, but becomes stable later without any changes, we'll have to do
busy-work to remove the ! in all the DT files, thus artificially
introducing an incompatibility. Perhaps that's fine though?
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [Ksummit-2013-discuss] [RFC] of: Allow for experimental device tree bindings

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 903 bytes --]


> Do we really want to polute the drivers and DT files with a ! in the
> compatible values? I thought we'd considered that, but chosen having the
> drivers that use unstable bindings depend on a Kconfig option as an
> alternative, not an additional step?

I'd even go further and use "unstable-" as the prefix instead of "!"
which is way more explicit.


> The one issue with doing this is that if a binding is thought to be
> unstable, but becomes stable later without any changes, we'll have to do
> busy-work to remove the ! in all the DT files, thus artificially
> introducing an incompatibility. Perhaps that's fine though?

I'd say yes. Going from unstable to stable is quite a step for a binding
and that should be visible and worth a patch IMO. Also, when looking at
a DTS file or some driver code, it will avoid
confusion/misinterpretation if one can see immediately the status of a
binding.


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [Ksummit-2013-discuss] [RFC] of: Allow for experimental device tree bindings

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 1379 bytes --]

On Wed, Oct 23, 2013 at 06:20:02PM +0100, Wolfram Sang wrote:
> 
> > Do we really want to polute the drivers and DT files with a ! in the
> > compatible values? I thought we'd considered that, but chosen having the
> > drivers that use unstable bindings depend on a Kconfig option as an
> > alternative, not an additional step?
> 
> I'd even go further and use "unstable-" as the prefix instead of "!"
> which is way more explicit.

I guess unstable- is as good as anything. I personally think that "!" is
disturbing enough to the eye to make it abundantly clear that something
is fishy.

> > The one issue with doing this is that if a binding is thought to be
> > unstable, but becomes stable later without any changes, we'll have to do
> > busy-work to remove the ! in all the DT files, thus artificially
> > introducing an incompatibility. Perhaps that's fine though?
> 
> I'd say yes. Going from unstable to stable is quite a step for a binding
> and that should be visible and worth a patch IMO. Also, when looking at
> a DTS file or some driver code, it will avoid
> confusion/misinterpretation if one can see immediately the status of a
> binding.

Yes, I fully agree. It might look like churn, but I think this could
actually be a part of the formal process to stabilize a binding. It
would be final step of that process, actually.

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

Re: [Ksummit-2013-discuss] [RFC] of: Allow for experimental device tree bindings

[Jason Gunthorpe]

On Wed, Oct 23, 2013 at 08:59:10PM +0200, Thierry Reding wrote:

> > I'd say yes. Going from unstable to stable is quite a step for a binding
> > and that should be visible and worth a patch IMO. Also, when looking at
> > a DTS file or some driver code, it will avoid
> > confusion/misinterpretation if one can see immediately the status of a
> > binding.
> 
> Yes, I fully agree. It might look like churn, but I think this could
> actually be a part of the formal process to stabilize a binding. It
> would be final step of that process, actually.

I actually think this makes things worse.

Ostensibly the purpose of stable DT is to allow the DT and kernel to
be separate, so you should minimize the churn in the DTs, and they
should trend to stable.

Having a flag day where someone goes and churns the DT to remove a !,
and then changes the kernel so all old DTs with a ! won't work at all
makes this whole thing seem kinda contrary to the basic motivating
premis.

Also, what happens during development? If you incompatibly change the
binding you should change the name, so maybe <version>!marvell,foo is
the way to go. 

v1 of the binding is 1!marvell,foo - version 2 is 2!marvell,foo, etc.

When stablized the last bang is kept and the non-bang version is
added. The boot warning is supressed once stable no matter the
compatible string used in the dt...

Jason
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [Ksummit-2013-discuss] [RFC] of: Allow for experimental device tree bindings

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 2748 bytes --]

On Wed, Oct 23, 2013 at 01:34:50PM -0600, Jason Gunthorpe wrote:
> On Wed, Oct 23, 2013 at 08:59:10PM +0200, Thierry Reding wrote:
> 
> > > I'd say yes. Going from unstable to stable is quite a step for a binding
> > > and that should be visible and worth a patch IMO. Also, when looking at
> > > a DTS file or some driver code, it will avoid
> > > confusion/misinterpretation if one can see immediately the status of a
> > > binding.
> > 
> > Yes, I fully agree. It might look like churn, but I think this could
> > actually be a part of the formal process to stabilize a binding. It
> > would be final step of that process, actually.
> 
> I actually think this makes things worse.
> 
> Ostensibly the purpose of stable DT is to allow the DT and kernel to
> be separate, so you should minimize the churn in the DTs, and they
> should trend to stable.

Well, I do think that stable DT has benefits. And quite frankly I think
the majority of bindings will eventually converge to some stable state
anyway, if only because active development stops. In an ideal world that
would be when a product ships.

So this proposal isn't so much about making a decision for stable or
experimental DT, but rather about giving users a choice. If they are
willing to live with the additional "burden" of updating the DTB every
once in a while, then they can enable the option and get additional
functionality. If they don't want any part of that, they can just leave
the option disabled and only get the parts that are stable.

> Having a flag day where someone goes and churns the DT to remove a !,
> and then changes the kernel so all old DTs with a ! won't work at all
> makes this whole thing seem kinda contrary to the basic motivating
> premis.

No. Matching doesn't include the ! marker. So if you remove it from DTS
files and/or drivers, the only thing that goes away is the warning at
runtime. Feature-wise there should be no difference.

> Also, what happens during development? If you incompatibly change the
> binding you should change the name, so maybe <version>!marvell,foo is
> the way to go. 
> 
> v1 of the binding is 1!marvell,foo - version 2 is 2!marvell,foo, etc.
> 
> When stablized the last bang is kept and the non-bang version is
> added. The boot warning is supressed once stable no matter the
> compatible string used in the dt...

Why would we want to go through all that trouble if we define up front
that the binding is experimental in the first place? Encoding a version
number in it somehow entails that earlier versions are still supported
in some way.

But the whole point of experimental bindings is so that we don't have to
worry about backwards compatibility.

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

Re: [Ksummit-2013-discuss] [RFC] of: Allow for experimental device tree bindings

[Jason Gunthorpe]

On Wed, Oct 23, 2013 at 09:58:50PM +0200, Thierry Reding wrote:

> > Having a flag day where someone goes and churns the DT to remove a !,
> > and then changes the kernel so all old DTs with a ! won't work at all
> > makes this whole thing seem kinda contrary to the basic motivating
> > premis.
> 
> No. Matching doesn't include the ! marker. So if you remove it from DTS
> files and/or drivers, the only thing that goes away is the warning at
> runtime. Feature-wise there should be no difference.

Sounds good
 
> > Also, what happens during development? If you incompatibly change the
> > binding you should change the name, so maybe <version>!marvell,foo is
> > the way to go. 
> > 
> > v1 of the binding is 1!marvell,foo - version 2 is 2!marvell,foo, etc.
> > 
> > When stablized the last bang is kept and the non-bang version is
> > added. The boot warning is supressed once stable no matter the
> > compatible string used in the dt...
> 
> Why would we want to go through all that trouble if we define up front
> that the binding is experimental in the first place? Encoding a version
> number in it somehow entails that earlier versions are still supported
> in some way.

No, certainly not. It just says the binding has changed and every DT
stanza that uses the old version needs to be reviewed and updated.

> But the whole point of experimental bindings is so that we don't have to
> worry about backwards compatibility.

The purpose is to not silently throw users/testers under the bus. A
driver that doesn't bind is much better than a driver that blows up in
some crazy, hard to determine way because the DT binding has been
silently incompatibly changed.

The rule for experimental bindings should be that incompatible changes
to the binding must bump the version number at the same time, clearly
signalling to everyone using that binding that they need to take some
action.

Jason
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [Ksummit-2013-discuss] [RFC] of: Allow for experimental device tree bindings

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 3093 bytes --]

On Wed, Oct 23, 2013 at 03:08:49PM -0600, Jason Gunthorpe wrote:
> On Wed, Oct 23, 2013 at 09:58:50PM +0200, Thierry Reding wrote:
[...]
> > > Also, what happens during development? If you incompatibly change the
> > > binding you should change the name, so maybe <version>!marvell,foo is
> > > the way to go. 
> > > 
> > > v1 of the binding is 1!marvell,foo - version 2 is 2!marvell,foo, etc.
> > > 
> > > When stablized the last bang is kept and the non-bang version is
> > > added. The boot warning is supressed once stable no matter the
> > > compatible string used in the dt...
> > 
> > Why would we want to go through all that trouble if we define up front
> > that the binding is experimental in the first place? Encoding a version
> > number in it somehow entails that earlier versions are still supported
> > in some way.
> 
> No, certainly not. It just says the binding has changed and every DT
> stanza that uses the old version needs to be reviewed and updated.
> 
> > But the whole point of experimental bindings is so that we don't have to
> > worry about backwards compatibility.
> 
> The purpose is to not silently throw users/testers under the bus. A
> driver that doesn't bind is much better than a driver that blows up in
> some crazy, hard to determine way because the DT binding has been
> silently incompatibly changed.
> 
> The rule for experimental bindings should be that incompatible changes
> to the binding must bump the version number at the same time, clearly
> signalling to everyone using that binding that they need to take some
> action.

I disagree. I think that we should apply the same rule to DT bindings
(at least experimental ones) that we apply to code within the Linux
kernel. If you change an experimental binding in an incompatible way
then it should be your responsibility to update all users of it so that
they don't break.

In reality I would hope that this isn't much of a problem really. Given
the nature of a compatible value there will typically only be a single
driver implementing it. Any users of it (DTS files) should be pretty
easy to fix up at the same time.

That's the same way that platform data used to work, except it had the
advantage of the compiler actually pointing out incompatible changes.
There has been some discussion about adding validation functionality to
DTC, which should make it easy to find DTS files that require updating
as well.

The above would also indicate that because of their nature, experimental
bindings might be better kept within the Linux kernel tree. That would
make it easy to keep a good overview of what needs to be updated when a
binding changes. It would also make the promotion to stable much more
explicit by physically moving the binding document to a different
repository. Obviously that comes with its own set of problem that we'll
need to find a way to extend the board DTS files that will presumably be
kept in the external stable binding repository with experimental content
only available in the Linux kernel tree.

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

Re: [Ksummit-2013-discuss] [RFC] of: Allow for experimental device tree bindings

[Jason Gunthorpe]

On Thu, Oct 24, 2013 at 10:04:19AM +0200, Thierry Reding wrote:

> > The rule for experimental bindings should be that incompatible changes
> > to the binding must bump the version number at the same time, clearly
> > signalling to everyone using that binding that they need to take some
> > action.
> 
> I disagree. I think that we should apply the same rule to DT bindings
> (at least experimental ones) that we apply to code within the Linux
> kernel. If you change an experimental binding in an incompatible way
> then it should be your responsibility to update all users of it so that
> they don't break.

Absolutely, but these things are going to get out of the kernel tree
and people are going to be using them in broad contexts (eg flashing
them into firmware) - especially if we imagine the stablization window
is multiple kernel releases.

Clearly designating which revision is supported lets people know what
is going on, very explicitly.

Also, it lets people that might have the need to support multiple
versions in their out-of-tree DT by having repeated nodes.

I just think it is good practice to get people into the habit that the
compatible string indicates a single exact schema, and if you change
the schema you have to change the compatible string.

Always.

Jason

Re: [Ksummit-2013-discuss] [RFC] of: Allow for experimental device tree bindings

[Andy Lutomirski]

On Wed, Oct 23, 2013 at 12:58 PM, Thierry Reding
<thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> On Wed, Oct 23, 2013 at 01:34:50PM -0600, Jason Gunthorpe wrote:
>> On Wed, Oct 23, 2013 at 08:59:10PM +0200, Thierry Reding wrote:
>>
>> > > I'd say yes. Going from unstable to stable is quite a step for a binding
>> > > and that should be visible and worth a patch IMO. Also, when looking at
>> > > a DTS file or some driver code, it will avoid
>> > > confusion/misinterpretation if one can see immediately the status of a
>> > > binding.
>> >
>> > Yes, I fully agree. It might look like churn, but I think this could
>> > actually be a part of the formal process to stabilize a binding. It
>> > would be final step of that process, actually.
>>
>> I actually think this makes things worse.
>>
>> Ostensibly the purpose of stable DT is to allow the DT and kernel to
>> be separate, so you should minimize the churn in the DTs, and they
>> should trend to stable.
>
> Well, I do think that stable DT has benefits. And quite frankly I think
> the majority of bindings will eventually converge to some stable state
> anyway, if only because active development stops. In an ideal world that
> would be when a product ships.
>
> So this proposal isn't so much about making a decision for stable or
> experimental DT, but rather about giving users a choice. If they are
> willing to live with the additional "burden" of updating the DTB every
> once in a while, then they can enable the option and get additional
> functionality. If they don't want any part of that, they can just leave
> the option disabled and only get the parts that are stable.
>
>> Having a flag day where someone goes and churns the DT to remove a !,
>> and then changes the kernel so all old DTs with a ! won't work at all
>> makes this whole thing seem kinda contrary to the basic motivating
>> premis.
>
> No. Matching doesn't include the ! marker. So if you remove it from DTS
> files and/or drivers, the only thing that goes away is the warning at
> runtime. Feature-wise there should be no difference.
>
>> Also, what happens during development? If you incompatibly change the
>> binding you should change the name, so maybe <version>!marvell,foo is
>> the way to go.
>>
>> v1 of the binding is 1!marvell,foo - version 2 is 2!marvell,foo, etc.
>>

[I'm a random bystander -- I don't really know anything about DT.]

This sounds awfully like the -moz, -webkit, etc. CSS property
selectors.  They are AFAICT nearly universally considered to have been
a mistake.

--Andy
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [Ksummit-2013-discuss] [RFC] of: Allow for experimental device tree bindings

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 496 bytes --]


> > I'd even go further and use "unstable-" as the prefix instead of "!"
> > which is way more explicit.
> 
> I guess unstable- is as good as anything. I personally think that "!" is
> disturbing enough to the eye to make it abundantly clear that something
> is fishy.

"!" marks the binding as "special" whatever that is. A busy person might
decide to not look that up as long as it works right now. "unstable-"
(or maybe "unstable!-" ;)) is explicit so people know what they get.


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [Ksummit-2013-discuss] [RFC] of: Allow for experimental device tree bindings

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 1112 bytes --]

On Wed, Oct 23, 2013 at 08:40:00PM +0100, Wolfram Sang wrote:
> 
> > > I'd even go further and use "unstable-" as the prefix instead of "!"
> > > which is way more explicit.
> > 
> > I guess unstable- is as good as anything. I personally think that "!" is
> > disturbing enough to the eye to make it abundantly clear that something
> > is fishy.
> 
> "!" marks the binding as "special" whatever that is. A busy person might
> decide to not look that up as long as it works right now. "unstable-"
> (or maybe "unstable!-" ;)) is explicit so people know what they get.

We'll need to document this somewhere to make people aware of it. And if
nobody bothers to read that documentation then they're not entitled to
complain.

That said, in my experience people are just as likely to ignore anything
with an "unstable" in it until it breaks. Also I don't think "unstable"
is the right term. "unstable" implies that it somehow influences the
system stability. But that's not the case. "experimental" is much more
accurate in that developers are experimenting with the representation.

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

Re: [Ksummit-2013-discuss] [RFC] of: Allow for experimental device tree bindings

[Grant Likely]

On Wed, 23 Oct 2013 18:20:02 +0100, Wolfram Sang <wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org> wrote:
> 
> > Do we really want to polute the drivers and DT files with a ! in the
> > compatible values? I thought we'd considered that, but chosen having the
> > drivers that use unstable bindings depend on a Kconfig option as an
> > alternative, not an additional step?
> 
> I'd even go further and use "unstable-" as the prefix instead of "!"
> which is way more explicit.
> 
> 
> > The one issue with doing this is that if a binding is thought to be
> > unstable, but becomes stable later without any changes, we'll have to do
> > busy-work to remove the ! in all the DT files, thus artificially
> > introducing an incompatibility. Perhaps that's fine though?
> 
> I'd say yes. Going from unstable to stable is quite a step for a binding
> and that should be visible and worth a patch IMO. Also, when looking at
> a DTS file or some driver code, it will avoid
> confusion/misinterpretation if one can see immediately the status of a
> binding.

No, it shouldn't. Going from unstable to stable is not a large step, rather it is coming to the point of looking around and realizing that the binding is working quite well.

I don't think the solution is to put this into the kernel to be checked
at runtime. The better solution is to put it into DTC and make it
complain (either warn or error; depending on build config?) about usage
of compatible strings that are marked in the binding documentation as
unstable.

g.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [Ksummit-2013-discuss] [RFC] of: Allow for experimental device tree bindings

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 2519 bytes --]

On Thu, Oct 24, 2013 at 09:34:59AM +0100, Grant Likely wrote:
> On Wed, 23 Oct 2013 18:20:02 +0100, Wolfram Sang <wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org> wrote:
> > 
> > > Do we really want to polute the drivers and DT files with a ! in the
> > > compatible values? I thought we'd considered that, but chosen having the
> > > drivers that use unstable bindings depend on a Kconfig option as an
> > > alternative, not an additional step?
> > 
> > I'd even go further and use "unstable-" as the prefix instead of "!"
> > which is way more explicit.
> > 
> > 
> > > The one issue with doing this is that if a binding is thought to be
> > > unstable, but becomes stable later without any changes, we'll have to do
> > > busy-work to remove the ! in all the DT files, thus artificially
> > > introducing an incompatibility. Perhaps that's fine though?
> > 
> > I'd say yes. Going from unstable to stable is quite a step for a binding
> > and that should be visible and worth a patch IMO. Also, when looking at
> > a DTS file or some driver code, it will avoid
> > confusion/misinterpretation if one can see immediately the status of a
> > binding.
> 
> No, it shouldn't. Going from unstable to stable is not a large step,
> rather it is coming to the point of looking around and realizing that
> the binding is working quite well.

Yes, the difference between the unstable binding before it is declared
stable and the stable one shouldn't be big. In fact it should be no
different at all.

However the decision is still a conscious one. And it is a big step,
because when you declare it stable you assert that it will never change
in an incompatible way.

> I don't think the solution is to put this into the kernel to be checked
> at runtime. The better solution is to put it into DTC and make it
> complain (either warn or error; depending on build config?) about usage
> of compatible strings that are marked in the binding documentation as
> unstable.

Perhaps. Doing it in the kernel seemed easier. Furthermore not every
user might generate their own DTB and whoever generates the DTB may not
make the same choice as every user might have made. Granted, that might
be a little far fetched.

I personally don't mind where exactly it is checked for, as long as we
can settle on something. What I'm primarily concerned about is that the
current situation hinders progress and early adoption, which I consider
both essential for upstream Linux development.

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

Re: [Ksummit-2013-discuss] [RFC] of: Allow for experimental device tree bindings

[Matt Sealey]

On Thu, Oct 24, 2013 at 3:34 AM, Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org> wrote:
> On Wed, 23 Oct 2013 18:20:02 +0100, Wolfram Sang <wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org> wrote:
>>
>> I'd say yes. Going from unstable to stable is quite a step for a binding
>> and that should be visible and worth a patch IMO.

A patch to remove the ! in front of !marvell,dma-controller is still
going to be as visible as a patch to remove
"unstable-!-version2-!-!-!!-omg!marvel,dma-controller" isn't it?

My worry is some bindings will never go stable, and the kernel will
keep printing warnings. At this point, marking it unstable is like
marking a driver as depends on EXPERIMENTAL in Kconfig - everyone
enables EXPERIMENTAL so there was no reason for it to exist. So, a few
warnings pop out on a console with a ! in front, people ignore those
and carry on. Who are you really trying to affect by implementing an
unstable binding warning?

>> a DTS file or some driver code, it will avoid
>> confusion/misinterpretation if one can see immediately the status of a
>> binding.
>
> No, it shouldn't. Going from unstable to stable is not a large step, rather it is coming to the point of looking around and realizing that the binding is working quite well.

Who decides that?

> I don't think the solution is to put this into the kernel to be checked
> at runtime.

I think the warning needs to be selectable at runtime; distributions
should mandate it turned on (therefore they can tell if weird crap
happens in their bug trackers same as the staging taint flag) but
normal people can turn it off. In the meantime, everything keeps
working as it was; except that new device trees will no longer work on
older systems since they will not be ignoring the bang and devices
will disappear.

It also implies that every current binding has been implicitly made
stable, and again the worry that new bindings will stay unstable
forever (and forever knock out that warning until it gets someone
patching away the bang upstream whether it is really 'stable' or not.)

> The better solution is to put it into DTC and make it
> complain (either warn or error; depending on build config?) about usage
> of compatible strings that are marked in the binding documentation as
> unstable.

I would agree with this, except less people care about the kernel
build output than console output. People using precompiled distros
won't even notice..

Again, DTC would have to strip the bang before handing it to the
kernel, or every current (as of today) binding is stable. But it is
less intrusive.

The state of the binding - being experimental or not - should probably
just be denoted by the binding being in an "experimental" directory in
the source, or marked with a X-DT-Binding-Status: TOTALLY_WHACKED_OUT
tag at the top of the binding itself. If you build a kernel and stuff
acts weird, check the binding for the device that acts weird. If it's
marked as such, go harangue the developers...

-- 
Matt Sealey <neko-HhXTZounMxbZATc7fWT8Dg@public.gmane.org>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [Ksummit-2013-discuss] [RFC] of: Allow for experimental device tree bindings

[Stephen Warren]

On 10/24/2013 09:34 AM, Grant Likely wrote:
> On Wed, 23 Oct 2013 18:20:02 +0100, Wolfram Sang <wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org> wrote:
>>
>>> Do we really want to polute the drivers and DT files with a ! in the
>>> compatible values? I thought we'd considered that, but chosen having the
>>> drivers that use unstable bindings depend on a Kconfig option as an
>>> alternative, not an additional step?
>>
>> I'd even go further and use "unstable-" as the prefix instead of "!"
>> which is way more explicit.
>>
>>
>>> The one issue with doing this is that if a binding is thought to be
>>> unstable, but becomes stable later without any changes, we'll have to do
>>> busy-work to remove the ! in all the DT files, thus artificially
>>> introducing an incompatibility. Perhaps that's fine though?
>>
>> I'd say yes. Going from unstable to stable is quite a step for a binding
>> and that should be visible and worth a patch IMO. Also, when looking at
>> a DTS file or some driver code, it will avoid
>> confusion/misinterpretation if one can see immediately the status of a
>> binding.
> 
> No, it shouldn't. Going from unstable to stable is not a large step, rather it is coming to the point of looking around and realizing that the binding is working quite well.
> 
> I don't think the solution is to put this into the kernel to be checked
> at runtime. The better solution is to put it into DTC and make it
> complain (either warn or error; depending on build config?) about usage
> of compatible strings that are marked in the binding documentation as
> unstable.

I don't think that's what we talked about on Wednesday though. At a
quick glance, this didn't make it into the meeting notes though[1], but
is in the presentation we created for the kernel summit readout. Is
sharing a link to that before it's presented OK?

[1] http://etherpad.osuosl.org/arm-ksummit-2013-day-2
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC] of: Allow for experimental device tree bindings

[jonsmirl-Re5JQEeQqe8AvxtiuMwx3w]

On Wed, Oct 23, 2013 at 11:06 AM, Thierry Reding
<thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> Past and recent discussions have shown that there's some concensus that
> device tree bindings should be considered an ABI and therefore need to
> remain backwards-compatible once code to implement them is included in
> the final release of a Linux kernel.

Doing it this way clutters up the DTS source files.

Schemas are a way to do this without clutter. There should be a master
schema that would validate all accepted device tree bindings.   So
experimental bindings naturally drop right of of this - they'll
generate errors when validated against the master schema.  When the
binding is finally sorted out it gets added to the master schema and
the validation errors go away.


>
> At the same time there is a desire to keep some manoeuvre while we're
> trying to figure things out. The fact is that many of us lack the
> experience to design good bindings from the start. At the same time
> there is a shortage of people that can review bindings and help design
> better ones.
>
> Progress and the addition of new features (and restoration of features
> that used to work before the advent of DT for that matter) are blocked
> to a large degree because of that.
>
> This patch attempts to restore some degree of freedom by introducing an
> easy way to mark device tree bindings as experimental as well as a way
> for users to disable the use of experimental bindings if they choose
> functionality at the cost of potential device tree incompatibilities.
>
> Bindings are marked experimental by prefixing the compatible value with
> an exclamation mark (!). In order to make it clear that experimental
> bindings are undesirable in the long run, a warning will be output when
> an experimental binding is encountered.
>
> Signed-off-by: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> ---
>  drivers/of/Kconfig |  7 +++++++
>  drivers/of/base.c  | 32 +++++++++++++++++++++++++++++++-
>  2 files changed, 38 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig
> index 78cc760..dc482f8 100644
> --- a/drivers/of/Kconfig
> +++ b/drivers/of/Kconfig
> @@ -24,6 +24,13 @@ config OF_SELFTEST
>
>           If unsure, say N here, but this option is safe to enable.
>
> +config OF_EXPERIMENTAL
> +       bool "Support experimental device tree bindings"
> +       help
> +         This option allows experimental device tree bindings to be used.
> +         Note that experimental bindings are subject to change, possibly
> +         requiring the DTB to be updated.
> +
>  config OF_FLATTREE
>         bool
>         select DTC
> diff --git a/drivers/of/base.c b/drivers/of/base.c
> index a96f850..b0b8371 100644
> --- a/drivers/of/base.c
> +++ b/drivers/of/base.c
> @@ -342,6 +342,36 @@ struct device_node *of_get_cpu_node(int cpu, unsigned int *thread)
>  }
>  EXPORT_SYMBOL(of_get_cpu_node);
>
> +static int of_compat_match(const char *device, const char *driver,
> +                          const struct device_node *np)
> +{
> +       const char *dev = device;
> +       const char *drv = driver;
> +
> +       if (device[0] == '!')
> +               device++;
> +
> +       if (driver[0] == '!')
> +               driver++;
> +
> +       if (of_compat_cmp(device, driver, strlen(driver)) != 0)
> +               return 0;
> +
> +       /* check if binding is experimental */
> +       if (dev != device || drv != driver) {
> +               pr_warn("of: device %s (%s) uses an experimental binding\n",
> +                       np->name, np->full_name);
> +
> +               /* don't match if we don't want experimental bindings */
> +               if (!IS_ENABLED(CONFIG_OF_EXPERIMENTAL)) {
> +                       pr_err("of: refusing to use binding \"%s\"\n", device);
> +                       return 0;
> +               }
> +       }
> +
> +       return 1;
> +}
> +
>  /** Checks if the given "compat" string matches one of the strings in
>   * the device's "compatible" property
>   */
> @@ -355,7 +385,7 @@ static int __of_device_is_compatible(const struct device_node *device,
>         if (cp == NULL)
>                 return 0;
>         while (cplen > 0) {
> -               if (of_compat_cmp(cp, compat, strlen(compat)) == 0)
> +               if (of_compat_match(cp, compat, device))
>                         return 1;
>                 l = strlen(cp) + 1;
>                 cp += l;
> --
> 1.8.4
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel



-- 
Jon Smirl
jonsmirl-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html