zephyrproject-rtos / zephyr

Primary Git Repository for the Zephyr Project. Zephyr is a new generation, scalable, optimized, secure RTOS for multiple hardware architectures.
https://docs.zephyrproject.org
Apache License 2.0
10.44k stars 6.4k forks source link

Document stability levels for devicetree bindings #32225

Open galak opened 3 years ago

galak commented 3 years ago

Add details w/regards to devicetree bindings to https://github.com/zephyrproject-rtos/zephyr/blob/master/doc/development_process/api_lifecycle.rst

mbolivar-nordic commented 3 years ago

I'm curious if we want to mark some bindings as stable, and others unstable, just like APIs, or if all bindings are stable by default. It seems like Linux contemplates unstable bindings that get more stable over time. cc @pabigot

galak commented 3 years ago

It seems like Linux contemplates unstable bindings that get more stable over time. cc @pabigot

How does linux denote stable v unstable binding?

pabigot commented 3 years ago

IMO any binding that's present in a release should be considered stable, as that's the point people might start using it out-of tree. I think we could have an exception where deprecation isn't necessary if the binding was introduced in the same development cycle as the breaking change. I don't know whether we need to record that anywhere; if we do perhaps a stable key in the yaml would work.

We should also clarify what it means to deprecate a property. Rephrasing from https://github.com/zephyrproject-rtos/zephyr/pull/32168#discussion_r574448722 I propose that deprecated properties should:

(I also would like anything deprecated to include in a comment the release in which it was deprecated, preferably in a standard format like "Deprecated in 2.5.0" so we can find these things and remove them promptly.)

It's unclear whether the functional documentation for a deprecated property should remain, or be removed. I prefer it be there (so people who have to replace it understand what it was meant to do), but there's some precedent in deprecating C functions to remove the documentation to make it more clear it shouldn't be used.

Whichever way we go that should also be documented and consistent for code, Kconfig, and devicetree.

mbolivar-nordic commented 3 years ago

IMO any binding that's present in a release should be considered stable, as that's the point people might start using it out-of tree.

I'm not so sure about this one. We have a process for making APIs increasingly stable, and it doesn't start with "as soon as an API is part of a release, it's stable" -- it starts with "experimental", then it goes to "unstable", before becoming "stable".

I see no reason why bindings should be held to a higher standard.

pabigot commented 3 years ago

I see no reason why bindings should be held to a higher standard.

I'm fine with that. A stability key in the YAML that defaults to "experimental" could handle this.

mbolivar-nordic commented 3 years ago

I'm fine with that. A stability key in the YAML that defaults to "experimental" could handle this.

I've changed the issue title to match. @galak @nashif is the discussion above enough of a plan forward for DTS stability for LTS?

galak commented 1 year ago

@mbolivar-nordic I feel like stability would have the states of unstable, stable and possibly deprecated. Not sure if I see the additional experimental state making sense.

mbolivar-nordic commented 1 year ago

@galak my thinking there is to match https://docs.zephyrproject.org/latest/develop/api/api_lifecycle.html. In particular, 'experimental' is a signal 'this can change or be removed later', while 'unstable' means 'this can change later'.

Been thinking about how we would want to flag changes to any bindings marked stable to add a label or something for failing CI if we change a stable binding in an incompatible way.

What we can do in the devicetree python package is parse the bindings before and after the PR, and flag changes that break users of stable bindings.

Looking at our API here:

https://github.com/zephyrproject-rtos/zephyr/blob/2d8857f27864ee6a40316d6784b73f12330ff770/scripts/dts/python-devicetree/src/devicetree/edtlib.py#L1682

I think we'd want to flag any of these as "incompatible" changes to bindings:

Other changes all look 'compatible' to me. For example, changing 'required' from true to false doesn't break anybody, even though it's a change.