| Message ID | 6bb6289a1829bf4d03fc65994ad4887ca60afffa.1692795112.git.geert+renesas@glider.be |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | [RFC] of: unittest: overlay_pci_node: Fix overlay style | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | warning | total: 0 errors, 2 warnings, 38 lines checked |
| robh/patch-applied | fail | build log |
On Wed, Aug 23, 2023 at 7:52 AM Geert Uytterhoeven <geert+renesas@glider.be> wrote: > > Miscellaneous fixes and improvements to the overlay_pci_node: > - Add missing /plugin/, Is it really missing if it wasn't needed since no unresolved phandles? I guess the sugar syntax needs, too? > - Convert to sugar syntax, > - Add missing blank lines between properties and subnodes. > > As sugar syntax does not support empty target paths, the test device is > added to /testcase-data/overlay-node instead. There's a definite need for unspecified target paths. It's probably something of an oversight that a blank path was even allowed. I don't think putting in the wrong path is a good solution. There should be perhaps more discussion if a blank path is the right thing and then how to make the sugar syntax output a blank path. Rob
Hi Rob, On Wed, Aug 23, 2023 at 6:44 PM Rob Herring <robh+dt@kernel.org> wrote: > On Wed, Aug 23, 2023 at 7:52 AM Geert Uytterhoeven > <geert+renesas@glider.be> wrote: > > > > Miscellaneous fixes and improvements to the overlay_pci_node: > > - Add missing /plugin/, > > Is it really missing if it wasn't needed since no unresolved phandles? > I guess the sugar syntax needs, too? It's indeed needed for the &{...} reference using sugar syntax. > > - Convert to sugar syntax, > > - Add missing blank lines between properties and subnodes. > > > > As sugar syntax does not support empty target paths, the test device is > > added to /testcase-data/overlay-node instead. > > There's a definite need for unspecified target paths. It's probably > something of an oversight that a blank path was even allowed. I don't > think putting in the wrong path is a good solution. There should be > perhaps more discussion if a blank path is the right thing and then > how to make the sugar syntax output a blank path. The path will not be used anyway, as it will be overwritten by passing the new base parameter of of_overlay_fdt_apply(). So "/" could be used as well. I have more comments, but I'll give them as a reply to the original patch introducing the base parameter. Gr{oetje,eeting}s, Geert
diff --git a/drivers/of/unittest-data/overlay_pci_node.dtso b/drivers/of/unittest-data/overlay_pci_node.dtso index c05e52e9e44a9583..f18c9795e6efa5f3 100644 --- a/drivers/of/unittest-data/overlay_pci_node.dtso +++ b/drivers/of/unittest-data/overlay_pci_node.dtso @@ -1,22 +1,21 @@ // SPDX-License-Identifier: GPL-2.0 /dts-v1/; -/ { - fragment@0 { - target-path=""; - __overlay__ { - #address-cells = <3>; - #size-cells = <2>; - pci-ep-bus@0 { - compatible = "simple-bus"; - #address-cells = <1>; - #size-cells = <1>; - ranges = <0x0 0x0 0x0 0x0 0x1000>; - reg = <0 0 0 0 0>; - unittest-pci@100 { - compatible = "unittest-pci"; - reg = <0x100 0x200>; - }; - }; +/plugin/; + +&{/testcase-data/overlay-node} { + #address-cells = <3>; + #size-cells = <2>; + + pci-ep-bus@0 { + compatible = "simple-bus"; + #address-cells = <1>; + #size-cells = <1>; + ranges = <0x0 0x0 0x0 0x0 0x1000>; + reg = <0 0 0 0 0>; + + unittest-pci@100 { + compatible = "unittest-pci"; + reg = <0x100 0x200>; }; }; };
Miscellaneous fixes and improvements to the overlay_pci_node: - Add missing /plugin/, - Convert to sugar syntax, - Add missing blank lines between properties and subnodes. As sugar syntax does not support empty target paths, the test device is added to /testcase-data/overlay-node instead. Fixes: 26409dd045892904 ("of: unittest: Add pci_dt_testdrv pci driver") Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> --- RFC, as I don't have a QEMU setup to run the test. --- .../of/unittest-data/overlay_pci_node.dtso | 33 +++++++++---------- 1 file changed, 16 insertions(+), 17 deletions(-)