| Message ID | d198d29119b37b2fdb700d8992b31963e98b6693.1560158667.git-series.maxime.ripard@bootlin.com |
|---|---|
| State | Changes Requested |
| Delegated to: | David Miller |
| Headers | show |
| Series | [v2,01/11] dt-bindings: net: Add YAML schemas for the generic Ethernet options | expand |
> +required: > + - compatible > + - reg > + - interrupts > + - clocks > + - phy > + - allwinner,sram Quoting ethernet.txt: - phy: the same as "phy-handle" property, not recommended for new bindings. - phy-handle: phandle, specifies a reference to a node representing a PHY device; this property is described in the Devicetree Specification and so preferred; Can this be expressed in Yaml? Accept phy, but give a warning. Accept phy-handle without a warning? Enforce that one or the other is present? Thanks Andrew
Hi Andrew, On Mon, Jun 10, 2019 at 04:31:39PM +0200, Andrew Lunn wrote: > > +required: > > + - compatible > > + - reg > > + - interrupts > > + - clocks > > + - phy > > + - allwinner,sram > > Quoting ethernet.txt: > > - phy: the same as "phy-handle" property, not recommended for new bindings. > - phy-handle: phandle, specifies a reference to a node representing a PHY > device; this property is described in the Devicetree Specification and so > preferred; > > Can this be expressed in Yaml? Accept phy, but give a warning. Accept > phy-handle without a warning? Enforce that one or the other is > present? This is what we should be aiming for, yes, but right now we don't really have a way to express that for properties. The next specification of the schema spec seems to address that, and it should be released pretty soon, so it's always something that we can address later on, when it will be out. For that particular case, we can also work around it by requiring phy-handle instead of phy. That way, if phy-handle is missing we will have a warning. phy will not be validated though, which is kind of a shame, but still much better than what we currently have. Maxime -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
On Mon, Jun 10, 2019 at 8:31 AM Andrew Lunn <andrew@lunn.ch> wrote: > > > +required: > > + - compatible > > + - reg > > + - interrupts > > + - clocks > > + - phy > > + - allwinner,sram > > Quoting ethernet.txt: > > - phy: the same as "phy-handle" property, not recommended for new bindings. > > - phy-handle: phandle, specifies a reference to a node representing a PHY > device; this property is described in the Devicetree Specification and so > preferred; > > Can this be expressed in Yaml? Accept phy, but give a warning. Accept > phy-handle without a warning? Enforce that one or the other is > present? The common schema could have 'phy: false'. This works as long as we've updated (or plan to) all the dts files to use phy-handle. The issue is how far back do you need kernels to work with newer dtbs. Rob
Hi Rob, On Mon, Jun 10, 2019 at 12:59:29PM -0600, Rob Herring wrote: > On Mon, Jun 10, 2019 at 8:31 AM Andrew Lunn <andrew@lunn.ch> wrote: > > > > > +required: > > > + - compatible > > > + - reg > > > + - interrupts > > > + - clocks > > > + - phy > > > + - allwinner,sram > > > > Quoting ethernet.txt: > > > > - phy: the same as "phy-handle" property, not recommended for new bindings. > > > > - phy-handle: phandle, specifies a reference to a node representing a PHY > > device; this property is described in the Devicetree Specification and so > > preferred; > > > > Can this be expressed in Yaml? Accept phy, but give a warning. Accept > > phy-handle without a warning? Enforce that one or the other is > > present? > > The common schema could have 'phy: false'. This works as long as we've > updated (or plan to) all the dts files to use phy-handle. The issue is > how far back do you need kernels to work with newer dtbs. I guess another question being raised by this is how hard do we want to be a deprecating things, and should the DT validation be a tool to enforce that validation. For example, you've used in you GPIO meta-schema false for anything ending with -gpio, since it's deprecated. This means that we can't convert any binding using a deprecated property without introducing a build error in the schemas, which in turn means that you'll have a lot of friction to support schemas, since you would have to convert your driver to support the new way of doing things, before being able to have a schema for your binding. And then, we need to agree on how to express the deprecation. I guess we could allow the deprecated keyword that will be there in the draft-8, instead of ad-hoc solutions? Maxime -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
On Thu, Jun 13, 2019 at 7:25 AM Maxime Ripard <maxime.ripard@bootlin.com> wrote: > > Hi Rob, > > On Mon, Jun 10, 2019 at 12:59:29PM -0600, Rob Herring wrote: > > On Mon, Jun 10, 2019 at 8:31 AM Andrew Lunn <andrew@lunn.ch> wrote: > > > > > > > +required: > > > > + - compatible > > > > + - reg > > > > + - interrupts > > > > + - clocks > > > > + - phy > > > > + - allwinner,sram > > > > > > Quoting ethernet.txt: > > > > > > - phy: the same as "phy-handle" property, not recommended for new bindings. > > > > > > - phy-handle: phandle, specifies a reference to a node representing a PHY > > > device; this property is described in the Devicetree Specification and so > > > preferred; > > > > > > Can this be expressed in Yaml? Accept phy, but give a warning. Accept > > > phy-handle without a warning? Enforce that one or the other is > > > present? > > > > The common schema could have 'phy: false'. This works as long as we've > > updated (or plan to) all the dts files to use phy-handle. The issue is > > how far back do you need kernels to work with newer dtbs. > > I guess another question being raised by this is how hard do we want > to be a deprecating things, and should the DT validation be a tool to > enforce that validation. > > For example, you've used in you GPIO meta-schema false for anything > ending with -gpio, since it's deprecated. This means that we can't > convert any binding using a deprecated property without introducing a > build error in the schemas, which in turn means that you'll have a lot > of friction to support schemas, since you would have to convert your > driver to support the new way of doing things, before being able to > have a schema for your binding. I've err'ed on the stricter side. We may need to back off on some things to get to warning free builds. Really, I'd like to have levels to separate checks for existing bindings, new bindings, and pedantic checks. For '-gpio', we may be okay because the suffix is handled in the GPIO core. It should be safe to update the binding to use the preferred form. > And then, we need to agree on how to express the deprecation. I guess > we could allow the deprecated keyword that will be there in the > draft-8, instead of ad-hoc solutions? Oh, nice! I hadn't seen that. Seems like we should use that. We can start even without draft-8 support because unknown keywords are ignored (though we probably have to add it to our meta-schema). Then at some point we can add a 'disallow deprecated' flag to the tool. Rob
Hi Rob, On Thu, Jun 13, 2019 at 11:32:30AM -0600, Rob Herring wrote: > On Thu, Jun 13, 2019 at 7:25 AM Maxime Ripard <maxime.ripard@bootlin.com> wrote: > > On Mon, Jun 10, 2019 at 12:59:29PM -0600, Rob Herring wrote: > > > On Mon, Jun 10, 2019 at 8:31 AM Andrew Lunn <andrew@lunn.ch> wrote: > > > > > > > > > +required: > > > > > + - compatible > > > > > + - reg > > > > > + - interrupts > > > > > + - clocks > > > > > + - phy > > > > > + - allwinner,sram > > > > > > > > Quoting ethernet.txt: > > > > > > > > - phy: the same as "phy-handle" property, not recommended for new bindings. > > > > > > > > - phy-handle: phandle, specifies a reference to a node representing a PHY > > > > device; this property is described in the Devicetree Specification and so > > > > preferred; > > > > > > > > Can this be expressed in Yaml? Accept phy, but give a warning. Accept > > > > phy-handle without a warning? Enforce that one or the other is > > > > present? > > > > > > The common schema could have 'phy: false'. This works as long as we've > > > updated (or plan to) all the dts files to use phy-handle. The issue is > > > how far back do you need kernels to work with newer dtbs. > > > > I guess another question being raised by this is how hard do we want > > to be a deprecating things, and should the DT validation be a tool to > > enforce that validation. > > > > For example, you've used in you GPIO meta-schema false for anything > > ending with -gpio, since it's deprecated. This means that we can't > > convert any binding using a deprecated property without introducing a > > build error in the schemas, which in turn means that you'll have a lot > > of friction to support schemas, since you would have to convert your > > driver to support the new way of doing things, before being able to > > have a schema for your binding. > > I've err'ed on the stricter side. We may need to back off on some > things to get to warning free builds. Really, I'd like to have levels > to separate checks for existing bindings, new bindings, and pedantic > checks. That would be awesome. Do you have a plan for that already though? I can't really think of a way to implement it at the moment. > For '-gpio', we may be okay because the suffix is handled in the GPIO > core. It should be safe to update the binding to use the preferred > form. It might require a bit of work though in drivers, since the fallback is only handled if you're using the gpiod API, and not the legacy one. > > And then, we need to agree on how to express the deprecation. I guess > > we could allow the deprecated keyword that will be there in the > > draft-8, instead of ad-hoc solutions? > > Oh, nice! I hadn't seen that. Seems like we should use that. We can > start even without draft-8 support because unknown keywords are > ignored (though we probably have to add it to our meta-schema). Then > at some point we can add a 'disallow deprecated' flag to the tool. So, in the generic ethernet binding, we would have: properties: phy-handle: $ref: /schemas/types.yaml#definitions/phandle description: Specifies a reference to a node representing a PHY device. phy: $ref: "#/properties/phy-handle" deprecated: true phy-device: $ref: "#/properties/phy-handle" deprecated: true Does that sound good? Now, how do we handle the case above, in the device specific binding? We just require the non-deprecated one, or the three? Thanks! Maxime -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
On Fri, Jun 14, 2019 at 3:50 AM Maxime Ripard <maxime.ripard@bootlin.com> wrote: > > Hi Rob, > > On Thu, Jun 13, 2019 at 11:32:30AM -0600, Rob Herring wrote: > > On Thu, Jun 13, 2019 at 7:25 AM Maxime Ripard <maxime.ripard@bootlin.com> wrote: > > > On Mon, Jun 10, 2019 at 12:59:29PM -0600, Rob Herring wrote: > > > > On Mon, Jun 10, 2019 at 8:31 AM Andrew Lunn <andrew@lunn.ch> wrote: > > > > > > > > > > > +required: > > > > > > + - compatible > > > > > > + - reg > > > > > > + - interrupts > > > > > > + - clocks > > > > > > + - phy > > > > > > + - allwinner,sram > > > > > > > > > > Quoting ethernet.txt: > > > > > > > > > > - phy: the same as "phy-handle" property, not recommended for new bindings. > > > > > > > > > > - phy-handle: phandle, specifies a reference to a node representing a PHY > > > > > device; this property is described in the Devicetree Specification and so > > > > > preferred; > > > > > > > > > > Can this be expressed in Yaml? Accept phy, but give a warning. Accept > > > > > phy-handle without a warning? Enforce that one or the other is > > > > > present? > > > > > > > > The common schema could have 'phy: false'. This works as long as we've > > > > updated (or plan to) all the dts files to use phy-handle. The issue is > > > > how far back do you need kernels to work with newer dtbs. > > > > > > I guess another question being raised by this is how hard do we want > > > to be a deprecating things, and should the DT validation be a tool to > > > enforce that validation. > > > > > > For example, you've used in you GPIO meta-schema false for anything > > > ending with -gpio, since it's deprecated. This means that we can't > > > convert any binding using a deprecated property without introducing a > > > build error in the schemas, which in turn means that you'll have a lot > > > of friction to support schemas, since you would have to convert your > > > driver to support the new way of doing things, before being able to > > > have a schema for your binding. > > > > I've err'ed on the stricter side. We may need to back off on some > > things to get to warning free builds. Really, I'd like to have levels > > to separate checks for existing bindings, new bindings, and pedantic > > checks. > > That would be awesome. Do you have a plan for that already though? I > can't really think of a way to implement it at the moment. The only idea I have so far is some sort of 'level' property and then we filter schema based on what level we run validation at. I'm not too sure if that would take some restructuring of schema though because it's all a mixture ATM. The other aspect is how to set the 'level' per platform so new platforms have to pass a higher level. We already have that problem just with dtc warnings. Ideally, we should build new platforms with 'W=1' or 'W=12'. Maybe the soc/board schema's can specify the level. > > For '-gpio', we may be okay because the suffix is handled in the GPIO > > core. It should be safe to update the binding to use the preferred > > form. > > It might require a bit of work though in drivers, since the fallback > is only handled if you're using the gpiod API, and not the legacy one. > > > > And then, we need to agree on how to express the deprecation. I guess > > > we could allow the deprecated keyword that will be there in the > > > draft-8, instead of ad-hoc solutions? > > > > Oh, nice! I hadn't seen that. Seems like we should use that. We can > > start even without draft-8 support because unknown keywords are > > ignored (though we probably have to add it to our meta-schema). Then > > at some point we can add a 'disallow deprecated' flag to the tool. > > So, in the generic ethernet binding, we would have: > > properties: > phy-handle: > $ref: /schemas/types.yaml#definitions/phandle > description: > Specifies a reference to a node representing a PHY device. > > phy: > $ref: "#/properties/phy-handle" > deprecated: true > > phy-device: > $ref: "#/properties/phy-handle" > deprecated: true > > Does that sound good? Yes. > Now, how do we handle the case above, in the device specific binding? > We just require the non-deprecated one, or the three? Wouldn't that just depend if all the instances of the device specific binding have been updated? Rob
Hi, On Fri, Jun 14, 2019 at 07:37:49AM -0600, Rob Herring wrote: > > > For '-gpio', we may be okay because the suffix is handled in the GPIO > > > core. It should be safe to update the binding to use the preferred > > > form. > > > > It might require a bit of work though in drivers, since the fallback > > is only handled if you're using the gpiod API, and not the legacy one. > > > > > > And then, we need to agree on how to express the deprecation. I guess > > > > we could allow the deprecated keyword that will be there in the > > > > draft-8, instead of ad-hoc solutions? > > > > > > Oh, nice! I hadn't seen that. Seems like we should use that. We can > > > start even without draft-8 support because unknown keywords are > > > ignored (though we probably have to add it to our meta-schema). Then > > > at some point we can add a 'disallow deprecated' flag to the tool. > > > > So, in the generic ethernet binding, we would have: > > > > properties: > > phy-handle: > > $ref: /schemas/types.yaml#definitions/phandle > > description: > > Specifies a reference to a node representing a PHY device. > > > > phy: > > $ref: "#/properties/phy-handle" > > deprecated: true > > > > phy-device: > > $ref: "#/properties/phy-handle" > > deprecated: true > > > > Does that sound good? > > Yes. Great, I'll post that. > > Now, how do we handle the case above, in the device specific binding? > > We just require the non-deprecated one, or the three? > > Wouldn't that just depend if all the instances of the device specific > binding have been updated? You mean in the DTS? It shouldn't matter, we'll want to have a warning anyway. But yeah, I'll update them too. Maxme -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
diff --git a/Documentation/devicetree/bindings/net/allwinner,sun4i-a10-emac.yaml b/Documentation/devicetree/bindings/net/allwinner,sun4i-a10-emac.yaml new file mode 100644 index 000000000000..b5d82d0a59d8 --- /dev/null +++ b/Documentation/devicetree/bindings/net/allwinner,sun4i-a10-emac.yaml @@ -0,0 +1,55 @@ +# SPDX-License-Identifier: GPL-2.0 +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/net/allwinner,sun4i-a10-emac.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Allwinner A10 EMAC Ethernet Controller Device Tree Bindings + +allOf: + - $ref: "ethernet-controller.yaml#" + +maintainers: + - Chen-Yu Tsai <wens@csie.org> + - Maxime Ripard <maxime.ripard@bootlin.com> + +properties: + compatible: + const: allwinner,sun4i-a10-emac + + reg: + maxItems: 1 + + interrupts: + maxItems: 1 + + clocks: + maxItems: 1 + + allwinner,sram: + description: Phandle to the device SRAM + $ref: /schemas/types.yaml#/definitions/phandle-array + +required: + - compatible + - reg + - interrupts + - clocks + - phy + - allwinner,sram + +examples: + - | + emac: ethernet@1c0b000 { + compatible = "allwinner,sun4i-a10-emac"; + reg = <0x01c0b000 0x1000>; + interrupts = <55>; + clocks = <&ahb_gates 17>; + phy = <&phy0>; + }; + +# FIXME: We should set it, but it would report all the generic +# properties as additional properties. +# additionalProperties: false + +... diff --git a/Documentation/devicetree/bindings/net/allwinner,sun4i-emac.txt b/Documentation/devicetree/bindings/net/allwinner,sun4i-emac.txt deleted file mode 100644 index e98118aef5f6..000000000000 --- a/Documentation/devicetree/bindings/net/allwinner,sun4i-emac.txt +++ /dev/null @@ -1,19 +0,0 @@ -* Allwinner EMAC ethernet controller - -Required properties: -- compatible: should be "allwinner,sun4i-a10-emac" (Deprecated: - "allwinner,sun4i-emac") -- reg: address and length of the register set for the device. -- interrupts: interrupt for the device -- phy: see ethernet.txt file in the same directory. -- clocks: A phandle to the reference clock for this device - -Example: - -emac: ethernet@1c0b000 { - compatible = "allwinner,sun4i-a10-emac"; - reg = <0x01c0b000 0x1000>; - interrupts = <55>; - clocks = <&ahb_gates 17>; - phy = <&phy0>; -};
Switch our Allwinner A10 EMAC controller binding to a YAML schema to enable the DT validation. Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com> --- Documentation/devicetree/bindings/net/allwinner,sun4i-a10-emac.yaml | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ Documentation/devicetree/bindings/net/allwinner,sun4i-emac.txt | 19 ------------------- 2 files changed, 55 insertions(+), 19 deletions(-) create mode 100644 Documentation/devicetree/bindings/net/allwinner,sun4i-a10-emac.yaml delete mode 100644 Documentation/devicetree/bindings/net/allwinner,sun4i-emac.txt