| Message ID | 20220927100347.176606-5-jean-philippe@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | hw/arm/virt: Fix dt-schema warnings | expand |
On Tue, 27 Sept 2022 at 11:12, Jean-Philippe Brucker <jean-philippe@linaro.org> wrote: > The node name of the gpio-key devicetree node should be "key-poweroff": > > gpio-keys: 'poweroff' does not match any of the regexes: '^(button|event|key|switch|(button|event|key|switch)-[a-z0-9-]+|[a-z0-9-]+-(button|event|key|switch))$', 'pinctrl-[0-9]+' > From schema: linux/Documentation/devicetree/bindings/input/gpio-keys.yaml > > Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org> This restriction only went into the DT documentation in July (kernel commit 5eb5652250). Please don't retrospectively make perfectly valid working DTs non-valid. I don't see any reason to change QEMU here. More generally, the set of things you might want the validator to warn about for a fresh new human-written DTB doesn't necessarily correspond to the set of things you want to enforce for a pre-existing code-generated DTB. For the former it makes much more sense to impose "coding style" and "naming convention" type rules. thanks -- PMM
On 9/27/22 12:03, Jean-Philippe Brucker wrote: > The node name of the gpio-key devicetree node should be "key-poweroff": > > gpio-keys: 'poweroff' does not match any of the regexes: '^(button|event|key|switch|(button|event|key|switch)-[a-z0-9-]+|[a-z0-9-]+-(button|event|key|switch))$', 'pinctrl-[0-9]+' > From schema: linux/Documentation/devicetree/bindings/input/gpio-keys.yaml > > Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org> Reviewed-by: Eric Auger <eric.auger@redhat.com> Eric > --- > hw/arm/virt.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/hw/arm/virt.c b/hw/arm/virt.c > index 8605f5058a..6805c57530 100644 > --- a/hw/arm/virt.c > +++ b/hw/arm/virt.c > @@ -932,12 +932,12 @@ static void create_gpio_keys(char *fdt, DeviceState *pl061_dev, > qemu_fdt_add_subnode(fdt, "/gpio-keys"); > qemu_fdt_setprop_string(fdt, "/gpio-keys", "compatible", "gpio-keys"); > > - qemu_fdt_add_subnode(fdt, "/gpio-keys/poweroff"); > - qemu_fdt_setprop_string(fdt, "/gpio-keys/poweroff", > + qemu_fdt_add_subnode(fdt, "/gpio-keys/key-poweroff"); > + qemu_fdt_setprop_string(fdt, "/gpio-keys/key-poweroff", > "label", "GPIO Key Poweroff"); > - qemu_fdt_setprop_cell(fdt, "/gpio-keys/poweroff", "linux,code", > + qemu_fdt_setprop_cell(fdt, "/gpio-keys/key-poweroff", "linux,code", > KEY_POWER); > - qemu_fdt_setprop_cells(fdt, "/gpio-keys/poweroff", > + qemu_fdt_setprop_cells(fdt, "/gpio-keys/key-poweroff", > "gpios", phandle, 3, 0); > } >
On Tue, Sep 27, 2022 at 6:56 AM Peter Maydell <peter.maydell@linaro.org> wrote: > > On Tue, 27 Sept 2022 at 11:12, Jean-Philippe Brucker > <jean-philippe@linaro.org> wrote: > > The node name of the gpio-key devicetree node should be "key-poweroff": > > > > gpio-keys: 'poweroff' does not match any of the regexes: '^(button|event|key|switch|(button|event|key|switch)-[a-z0-9-]+|[a-z0-9-]+-(button|event|key|switch))$', 'pinctrl-[0-9]+' > > From schema: linux/Documentation/devicetree/bindings/input/gpio-keys.yaml > > > > Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org> > > This restriction only went into the DT documentation in July > (kernel commit 5eb5652250). Fair enough. > Please don't retrospectively make perfectly valid working DTs > non-valid. I don't see any reason to change QEMU here. > > More generally, the set of things you might want the > validator to warn about for a fresh new human-written DTB > doesn't necessarily correspond to the set of things you want > to enforce for a pre-existing code-generated DTB. For the > former it makes much more sense to impose "coding style" > and "naming convention" type rules. I too would like to distinguish that, but haven't come up with a way to do that in json-schema yet. The way schemas are applied independently makes that a challenge. So far it's been low on the priority list as any platforms with few enough warnings to get to 0 haven't been a problem to fix (in a few cases we do end up relaxing the schemas). On the flip side, even existing things eventually get updated for coding style or evolving conventions. As long as we don't break ABIs, the same should apply to DT. Rob
On Thu, 13 Oct 2022 at 22:47, Rob Herring <robh+dt@kernel.org> wrote: > > On Tue, Sep 27, 2022 at 6:56 AM Peter Maydell <peter.maydell@linaro.org> wrote: > > Please don't retrospectively make perfectly valid working DTs > > non-valid. I don't see any reason to change QEMU here. > > > > More generally, the set of things you might want the > > validator to warn about for a fresh new human-written DTB > > doesn't necessarily correspond to the set of things you want > > to enforce for a pre-existing code-generated DTB. For the > > former it makes much more sense to impose "coding style" > > and "naming convention" type rules. > > I too would like to distinguish that, but haven't come up with a way > to do that in json-schema yet. The way schemas are applied > independently makes that a challenge. So far it's been low on the > priority list as any platforms with few enough warnings to get to 0 > haven't been a problem to fix (in a few cases we do end up relaxing > the schemas). > > On the flip side, even existing things eventually get updated for > coding style or evolving conventions. As long as we don't break ABIs, > the same should apply to DT. Yeah, but from QEMU's point of view pretty much the whole DT *is* ABI, because we have no idea what the guest will be doing with it, whether it is looking for things in the "correct" way or if it happens to have hard-coded a node name or similar: and the guest could be any of a wide variety of operating systems or custom code, and including pretty old versions of OSes as well as the latest-and-greatest. This is different from Linux's handwritten device trees, which only need to work with Linux and only with the associated Linux version, at that. So the set of things I'm happy changing tends to be more restricted than the set of things it's worth cleaning up in the dt sources that ship with the kernel. thanks -- PMM
diff --git a/hw/arm/virt.c b/hw/arm/virt.c index 8605f5058a..6805c57530 100644 --- a/hw/arm/virt.c +++ b/hw/arm/virt.c @@ -932,12 +932,12 @@ static void create_gpio_keys(char *fdt, DeviceState *pl061_dev, qemu_fdt_add_subnode(fdt, "/gpio-keys"); qemu_fdt_setprop_string(fdt, "/gpio-keys", "compatible", "gpio-keys"); - qemu_fdt_add_subnode(fdt, "/gpio-keys/poweroff"); - qemu_fdt_setprop_string(fdt, "/gpio-keys/poweroff", + qemu_fdt_add_subnode(fdt, "/gpio-keys/key-poweroff"); + qemu_fdt_setprop_string(fdt, "/gpio-keys/key-poweroff", "label", "GPIO Key Poweroff"); - qemu_fdt_setprop_cell(fdt, "/gpio-keys/poweroff", "linux,code", + qemu_fdt_setprop_cell(fdt, "/gpio-keys/key-poweroff", "linux,code", KEY_POWER); - qemu_fdt_setprop_cells(fdt, "/gpio-keys/poweroff", + qemu_fdt_setprop_cells(fdt, "/gpio-keys/key-poweroff", "gpios", phandle, 3, 0); }
The node name of the gpio-key devicetree node should be "key-poweroff": gpio-keys: 'poweroff' does not match any of the regexes: '^(button|event|key|switch|(button|event|key|switch)-[a-z0-9-]+|[a-z0-9-]+-(button|event|key|switch))$', 'pinctrl-[0-9]+' From schema: linux/Documentation/devicetree/bindings/input/gpio-keys.yaml Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org> --- hw/arm/virt.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)