| Message ID | 079279c3713042bdcc76ea27907d5f75bcb483fc.1554216856.git-series.maxime.ripard@bootlin.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2,1/2] dt-bindings: mtd: Add YAML schemas for the generic NAND options | expand |
On Tue, Apr 2, 2019 at 9:54 AM Maxime Ripard <maxime.ripard@bootlin.com> wrote: > > Switch the DT binding to a YAML schema to enable the DT validation. > > Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com> > > --- > > Changes from v1 > - Added controller constraints to the generic options > --- > Documentation/devicetree/bindings/mtd/allwinner,sun4i-a10-nand.yaml | 96 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++- > Documentation/devicetree/bindings/mtd/sunxi-nand.txt | 48 +------------------------------------ > 2 files changed, 96 insertions(+), 48 deletions(-) > create mode 100644 Documentation/devicetree/bindings/mtd/allwinner,sun4i-a10-nand.yaml > delete mode 100644 Documentation/devicetree/bindings/mtd/sunxi-nand.txt Same 'a-f' comment here, but otherwise, Reviewed-by: Rob Herring <robh@kernel.org> And thanks for being an early adopter. Let me know if you have any feedback on the schema or pain points. Rob
On Tue, Apr 02, 2019 at 08:25:59PM -0500, Rob Herring wrote: > On Tue, Apr 2, 2019 at 9:54 AM Maxime Ripard <maxime.ripard@bootlin.com> wrote: > > > > Switch the DT binding to a YAML schema to enable the DT validation. > > > > Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com> > > > > --- > > > > Changes from v1 > > - Added controller constraints to the generic options > > --- > > Documentation/devicetree/bindings/mtd/allwinner,sun4i-a10-nand.yaml | 96 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++- > > Documentation/devicetree/bindings/mtd/sunxi-nand.txt | 48 +------------------------------------ > > 2 files changed, 96 insertions(+), 48 deletions(-) > > create mode 100644 Documentation/devicetree/bindings/mtd/allwinner,sun4i-a10-nand.yaml > > delete mode 100644 Documentation/devicetree/bindings/mtd/sunxi-nand.txt > > Same 'a-f' comment here, but otherwise, > > Reviewed-by: Rob Herring <robh@kernel.org> > > And thanks for being an early adopter. Let me know if you have any > feedback on the schema or pain points. My main feedback is that it's awesome :) We (sunxi) have an awful lot of DT in the tree, and I made schemas for most of the bindings we have now. It allowed us to find a huge (or at least way more than what I was expecting) number of issues in our DTs, like inconsistent node naming, typos, etc., and also that our bindings were not updated as they should have. The following patches are a direct result from that, and I expect to find more. http://lists.infradead.org/pipermail/linux-arm-kernel/2019-March/640978.html On the pain point side, I guess the main one is that most of the time it's not really clear to me how I should express a particular set of constraints. You've been really helpful to deal with that one, and I guess it also stems from the fact that there's not a lot of examples in the tree right now. I expect it to go away the more schemas we have. The other one that might be more problematic is that it also tries to validate nodes that are not enabled. For in-SoC components that don't rely on anything external, it's fine, however, for components that would require something that is connected on the board (like a regulator, a phy, a GPIO, whatever), then we can't have all the required resources in the DTSI, and boards that don't use that component (and keep it disabled) will emit warning that this particular property is missing. I've tried to look into it but couldn't find an easy fix for that in the tooling, so I've opened a github issue for this. Let me know if it's not appropriate. Maxime -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
On Wed, Apr 3, 2019 at 2:44 AM Maxime Ripard <maxime.ripard@bootlin.com> wrote: > > On Tue, Apr 02, 2019 at 08:25:59PM -0500, Rob Herring wrote: > > On Tue, Apr 2, 2019 at 9:54 AM Maxime Ripard <maxime.ripard@bootlin.com> wrote: > > > > > > Switch the DT binding to a YAML schema to enable the DT validation. > > > > > > Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com> > > > > > > --- > > > > > > Changes from v1 > > > - Added controller constraints to the generic options > > > --- > > > Documentation/devicetree/bindings/mtd/allwinner,sun4i-a10-nand.yaml | 96 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++- > > > Documentation/devicetree/bindings/mtd/sunxi-nand.txt | 48 +------------------------------------ > > > 2 files changed, 96 insertions(+), 48 deletions(-) > > > create mode 100644 Documentation/devicetree/bindings/mtd/allwinner,sun4i-a10-nand.yaml > > > delete mode 100644 Documentation/devicetree/bindings/mtd/sunxi-nand.txt > > > > Same 'a-f' comment here, but otherwise, > > > > Reviewed-by: Rob Herring <robh@kernel.org> > > > > And thanks for being an early adopter. Let me know if you have any > > feedback on the schema or pain points. > > My main feedback is that it's awesome :) Thanks. > We (sunxi) have an awful lot of DT in the tree, and I made schemas for > most of the bindings we have now. It allowed us to find a huge (or at > least way more than what I was expecting) number of issues in our DTs, > like inconsistent node naming, typos, etc., and also that our bindings > were not updated as they should have. > > The following patches are a direct result from that, and I expect to > find more. > http://lists.infradead.org/pipermail/linux-arm-kernel/2019-March/640978.html > > On the pain point side, I guess the main one is that most of the time > it's not really clear to me how I should express a particular set of > constraints. You've been really helpful to deal with that one, and I > guess it also stems from the fact that there's not a lot of examples > in the tree right now. I expect it to go away the more schemas we > have. Yes, there is a bit of a learning curve to json-schema if you have to do anything out of the ordinary. The meta-schema is supposed to help constrain things, but it's only as good as what we define there. That also mainly works for already known property names and patterns we've thought of. > The other one that might be more problematic is that it also tries to > validate nodes that are not enabled. For in-SoC components that don't > rely on anything external, it's fine, however, for components that > would require something that is connected on the board (like a > regulator, a phy, a GPIO, whatever), then we can't have all the > required resources in the DTSI, and boards that don't use that > component (and keep it disabled) will emit warning that this > particular property is missing. > > I've tried to look into it but couldn't find an easy fix for that in > the tooling, so I've opened a github issue for this. Let me know if > it's not appropriate. Yeah, I need to go look at that. Skipping disabled nodes shouldn't be too hard to do within the tool. This came up before I think and I've resisted because I don't want to skip validating at least what is there. Maybe the better solution would be to drop 'required' from schema when validating disabled nodes. (At least, I think just 'required' is the problem?) The complication would be we'd need 2 sets of schemas and select the right one for each node. Another way to implement it would be just to filter out the warning messages. Rob
On Wed, Apr 03, 2019 at 03:33:42AM -0500, Rob Herring wrote: > > The other one that might be more problematic is that it also tries to > > validate nodes that are not enabled. For in-SoC components that don't > > rely on anything external, it's fine, however, for components that > > would require something that is connected on the board (like a > > regulator, a phy, a GPIO, whatever), then we can't have all the > > required resources in the DTSI, and boards that don't use that > > component (and keep it disabled) will emit warning that this > > particular property is missing. > > > > I've tried to look into it but couldn't find an easy fix for that in > > the tooling, so I've opened a github issue for this. Let me know if > > it's not appropriate. > > Yeah, I need to go look at that. Skipping disabled nodes shouldn't be > too hard to do within the tool. This came up before I think and I've > resisted because I don't want to skip validating at least what is > there. Maybe the better solution would be to drop 'required' from > schema when validating disabled nodes. (At least, I think just > 'required' is the problem?) I thought about something along those lines too, since indeed the only thing that causes troube is required. > The complication would be we'd need 2 sets of schemas and select the > right one for each node. You mean two instances of the schema in the tool, right? Not two schema files? Do you expect any drawback to that (like performance-wise maybe, if we have more schemas with more complicated select patterns)? > Another way to implement it would be just to filter out the warning > messages. That would work too I guess :) Maxime -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
diff --git a/Documentation/devicetree/bindings/mtd/allwinner,sun4i-a10-nand.yaml b/Documentation/devicetree/bindings/mtd/allwinner,sun4i-a10-nand.yaml new file mode 100644 index 000000000000..fcd2faec9af5 --- /dev/null +++ b/Documentation/devicetree/bindings/mtd/allwinner,sun4i-a10-nand.yaml @@ -0,0 +1,96 @@ +# SPDX-License-Identifier: GPL-2.0 +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/mtd/allwinner,sun4i-a10-nand.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Allwinner A10 NAND Controller Device Tree Bindings + +allOf: + - $ref: "nand-controller.yaml" + +maintainers: + - Chen-Yu Tsai <wens@csie.org> + - Maxime Ripard <maxime.ripard@bootlin.com> + +properties: + "#address-cells": true + "#size-cells": true + + compatible: + const: allwinner,sun4i-a10-nand + + reg: + maxItems: 1 + + interrupts: + maxItems: 1 + + clocks: + items: + - description: Bus Clock + - description: Module Clock + + clock-names: + items: + - const: ahb + - const: mod + + resets: + maxItems: 1 + + reset-names: + const: ahb + + dmas: + maxItems: 1 + + dma-names: + const: rxtx + + pinctrl-names: true + +patternProperties: + "^pinctrl-[0-9]+$": true + + "^nand@[a-z0-9]+$": + properties: + reg: + maxItems: 1 + minimum: 0 + maximum: 7 + + nand-ecc-mode: true + + nand-ecc-algo: + const: bch + + nand-ecc-step-size: + enum: [ 512, 1024 ] + + nand-ecc-strength: + maximum: 80 + + allwinner,rb: + description: + Contains the native Ready/Busy IDs. + allOf: + - $ref: /schemas/types.yaml#/definitions/uint32-array + - minItems: 1 + maxItems: 2 + items: + minimum: 0 + maximum: 1 + + additionalProperties: false + +required: + - compatible + - reg + - interrupts + - clocks + - clock-names + +additionalProperties: false + +... diff --git a/Documentation/devicetree/bindings/mtd/sunxi-nand.txt b/Documentation/devicetree/bindings/mtd/sunxi-nand.txt deleted file mode 100644 index dcd5a5d80dc0..000000000000 --- a/Documentation/devicetree/bindings/mtd/sunxi-nand.txt +++ /dev/null @@ -1,48 +0,0 @@ -Allwinner NAND Flash Controller (NFC) - -Required properties: -- compatible : "allwinner,sun4i-a10-nand". -- reg : shall contain registers location and length for data and reg. -- interrupts : shall define the nand controller interrupt. -- #address-cells: shall be set to 1. Encode the nand CS. -- #size-cells : shall be set to 0. -- clocks : shall reference nand controller clocks. -- clock-names : nand controller internal clock names. Shall contain : - * "ahb" : AHB gating clock - * "mod" : nand controller clock - -Optional properties: -- dmas : shall reference DMA channel associated to the NAND controller. -- dma-names : shall be "rxtx". - -Optional children nodes: -Children nodes represent the available nand chips. - -Optional properties: -- reset : phandle + reset specifier pair -- reset-names : must contain "ahb" -- allwinner,rb : shall contain the native Ready/Busy ids. -- nand-ecc-mode : one of the supported ECC modes ("hw", "soft", "soft_bch" or - "none") - -see Documentation/devicetree/bindings/mtd/nand.txt for generic bindings. - - -Examples: -nfc: nand@1c03000 { - compatible = "allwinner,sun4i-a10-nand"; - reg = <0x01c03000 0x1000>; - interrupts = <0 37 1>; - clocks = <&ahb_gates 13>, <&nand_clk>; - clock-names = "ahb", "mod"; - #address-cells = <1>; - #size-cells = <0>; - pinctrl-names = "default"; - pinctrl-0 = <&nand_pins_a &nand_cs0_pins_a &nand_rb0_pins_a>; - - nand@0 { - reg = <0>; - allwinner,rb = <0>; - nand-ecc-mode = "soft_bch"; - }; -};
Switch the DT binding to a YAML schema to enable the DT validation. Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com> --- Changes from v1 - Added controller constraints to the generic options --- Documentation/devicetree/bindings/mtd/allwinner,sun4i-a10-nand.yaml | 96 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++- Documentation/devicetree/bindings/mtd/sunxi-nand.txt | 48 +------------------------------------ 2 files changed, 96 insertions(+), 48 deletions(-) create mode 100644 Documentation/devicetree/bindings/mtd/allwinner,sun4i-a10-nand.yaml delete mode 100644 Documentation/devicetree/bindings/mtd/sunxi-nand.txt