From: Hrishabh Rajput <hrishabh.rajput@oss.qualcomm.com>
The Gunyah Hypervisor applies a devicetree overlay providing the
pretimeout interrupt for the Gunyah Watchdog that it will be using to
notify watchdog's pretimeout event. Add the DT bindings that Gunyah
adheres to for the hypervisor and watchdog.
Signed-off-by: Hrishabh Rajput <hrishabh.rajput@oss.qualcomm.com>
---
.../bindings/watchdog/qcom,gh-watchdog.yaml | 76 ++++++++++++++++++++++
MAINTAINERS | 1 +
2 files changed, 77 insertions(+)
diff --git a/Documentation/devicetree/bindings/watchdog/qcom,gh-watchdog.yaml b/Documentation/devicetree/bindings/watchdog/qcom,gh-watchdog.yaml
new file mode 100644
index 000000000000..bde8438c6242
--- /dev/null
+++ b/Documentation/devicetree/bindings/watchdog/qcom,gh-watchdog.yaml
@@ -0,0 +1,76 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/watchdog/qcom,gh-watchdog.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm Gunyah Virtual Watchdog
+
+maintainers:
+ - Hrishabh Rajput <hrishabh.rajput@oss.qualcomm.com>
+
+description: |+
+ The Gunyah Hypervisor provides an SMC-based watchdog interface for its virtual
+ machines. The virtual machines use this information to determine the
+ pretimeout IRQ which the hypervisor will be using to communicate pretimeout
+ event.
+ See also: [1]
+
+ [1]: https://github.com/quic/gunyah-resource-manager/blob/1b23ceb0dfa010b3b6b5a5f7a4ec1e95b93ab99d/src/vm_creation/dto_construct.c#L519
+
+properties:
+ compatible:
+ allOf:
+ - const: gunyah-hypervisor
+ - const: simple-bus
+
+ "#address-cells":
+ description: Number of cells needed to represent 64-bit capability IDs.
+ const: 2
+
+ "#size-cells":
+ description: must be 0, because capability IDs are not memory address
+ ranges and do not have a size.
+ const: 0
+
+patternProperties:
+ "^gh-watchdog":
+ type: object
+ description:
+ Watchdog node which provides information about the pretimeout IRQ which
+ will be used to communicate the pretimeout event.
+
+ properties:
+ compatible:
+ const: qcom,gh-watchdog
+
+ interrupts:
+ items:
+ description: Interrupt for the pretimeout.
+
+ additionalProperties: false
+
+ required:
+ - compatible
+ - interrupts
+
+additionalProperties: false
+
+required:
+ - compatible
+ - "#address-cells"
+ - "#size-cells"
+
+examples:
+ - |
+ #include <dt-bindings/interrupt-controller/arm-gic.h>
+
+ hypervisor {
+ compatible = "gunyah-hypervisor", "simple-bus";
+ #address-cells = <2>;
+ #size-cells = <0>;
+
+ gh-watchdog {
+ compatible = "qcom,gh-watchdog";
+ interrupts = <GIC_SPI 0 IRQ_TYPE_EDGE_RISING>; /* Pretimeout IRQ */
+ };
diff --git a/MAINTAINERS b/MAINTAINERS
index c0b444e5fd5a..03b74513e4ac 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3076,6 +3076,7 @@ F: Documentation/devicetree/bindings/cache/qcom,llcc.yaml
F: Documentation/devicetree/bindings/firmware/qcom,scm.yaml
F: Documentation/devicetree/bindings/reserved-memory/qcom*
F: Documentation/devicetree/bindings/soc/qcom/
+F: Documentation/devicetree/bindings/watchdog/qcom,gh-watchdog.yaml
F: arch/arm/boot/dts/qcom/
F: arch/arm/configs/qcom_defconfig
F: arch/arm/mach-qcom/
--
2.43.0On 03/09/2025 21:33, Hrishabh Rajput via B4 Relay wrote: > From: Hrishabh Rajput <hrishabh.rajput@oss.qualcomm.com> > > The Gunyah Hypervisor applies a devicetree overlay providing the > pretimeout interrupt for the Gunyah Watchdog that it will be using to > notify watchdog's pretimeout event. Add the DT bindings that Gunyah > adheres to for the hypervisor and watchdog. Wasn't tested, so limited review. Please use subject prefixes matching the subsystem. You can get them for example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory your patch is touching. For bindings, the preferred subjects are explained here: https://www.kernel.org/doc/html/latest/devicetree/bindings/submitting-patches.html#i-for-patch-submitters A nit, subject: drop second/last, redundant "bindings". The "dt-bindings" prefix is already stating that these are bindings. See also: https://elixir.bootlin.com/linux/v6.17-rc3/source/Documentation/devicetree/bindings/submitting-patches.rst#L18 > > Signed-off-by: Hrishabh Rajput <hrishabh.rajput@oss.qualcomm.com> > --- > .../bindings/watchdog/qcom,gh-watchdog.yaml | 76 ++++++++++++++++++++++ > MAINTAINERS | 1 + > 2 files changed, 77 insertions(+) > > diff --git a/Documentation/devicetree/bindings/watchdog/qcom,gh-watchdog.yaml b/Documentation/devicetree/bindings/watchdog/qcom,gh-watchdog.yaml > new file mode 100644 > index 000000000000..bde8438c6242 > --- /dev/null > +++ b/Documentation/devicetree/bindings/watchdog/qcom,gh-watchdog.yaml > @@ -0,0 +1,76 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/watchdog/qcom,gh-watchdog.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Qualcomm Gunyah Virtual Watchdog > + > +maintainers: > + - Hrishabh Rajput <hrishabh.rajput@oss.qualcomm.com> > + > +description: |+ > + The Gunyah Hypervisor provides an SMC-based watchdog interface for its virtual > + machines. The virtual machines use this information to determine the > + pretimeout IRQ which the hypervisor will be using to communicate pretimeout > + event. > + See also: [1] > + > + [1]: https://github.com/quic/gunyah-resource-manager/blob/1b23ceb0dfa010b3b6b5a5f7a4ec1e95b93ab99d/src/vm_creation/dto_construct.c#L519 > + > +properties: > + compatible: > + allOf: > + - const: gunyah-hypervisor > + - const: simple-bus What? No. Don't create patches with AI. > + > + "#address-cells": > + description: Number of cells needed to represent 64-bit capability IDs. > + const: 2 > + > + "#size-cells": > + description: must be 0, because capability IDs are not memory address > + ranges and do not have a size. > + const: 0 > + > +patternProperties: > + "^gh-watchdog": I could not express more: NAK. Does not match any DT style. Please do some internal reviews first. This patch does not meet minimum quality criteria for public posting. Best regards, Krzysztof
On 9/4/2025 3:22 PM, Krzysztof Kozlowski wrote: > On 03/09/2025 21:33, Hrishabh Rajput via B4 Relay wrote: >> From: Hrishabh Rajput <hrishabh.rajput@oss.qualcomm.com> >> >> The Gunyah Hypervisor applies a devicetree overlay providing the >> pretimeout interrupt for the Gunyah Watchdog that it will be using to >> notify watchdog's pretimeout event. Add the DT bindings that Gunyah >> adheres to for the hypervisor and watchdog. > Wasn't tested, so limited review. > > Please use subject prefixes matching the subsystem. You can get them for > example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory > your patch is touching. For bindings, the preferred subjects are > explained here: > https://www.kernel.org/doc/html/latest/devicetree/bindings/submitting-patches.html#i-for-patch-submitters > > A nit, subject: drop second/last, redundant "bindings". The > "dt-bindings" prefix is already stating that these are bindings. > See also: > https://elixir.bootlin.com/linux/v6.17-rc3/source/Documentation/devicetree/bindings/submitting-patches.rst#L18 > Noted. Will go through the referenced links and update accordingly. >> Signed-off-by: Hrishabh Rajput <hrishabh.rajput@oss.qualcomm.com> >> --- >> .../bindings/watchdog/qcom,gh-watchdog.yaml | 76 ++++++++++++++++++++++ >> MAINTAINERS | 1 + >> 2 files changed, 77 insertions(+) >> >> diff --git a/Documentation/devicetree/bindings/watchdog/qcom,gh-watchdog.yaml b/Documentation/devicetree/bindings/watchdog/qcom,gh-watchdog.yaml >> new file mode 100644 >> index 000000000000..bde8438c6242 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/watchdog/qcom,gh-watchdog.yaml >> @@ -0,0 +1,76 @@ >> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >> +%YAML 1.2 >> +--- >> +$id: http://devicetree.org/schemas/watchdog/qcom,gh-watchdog.yaml# >> +$schema: http://devicetree.org/meta-schemas/core.yaml# >> + >> +title: Qualcomm Gunyah Virtual Watchdog >> + >> +maintainers: >> + - Hrishabh Rajput <hrishabh.rajput@oss.qualcomm.com> >> + >> +description: |+ >> + The Gunyah Hypervisor provides an SMC-based watchdog interface for its virtual >> + machines. The virtual machines use this information to determine the >> + pretimeout IRQ which the hypervisor will be using to communicate pretimeout >> + event. >> + See also: [1] >> + >> + [1]: https://github.com/quic/gunyah-resource-manager/blob/1b23ceb0dfa010b3b6b5a5f7a4ec1e95b93ab99d/src/vm_creation/dto_construct.c#L519 >> + >> +properties: >> + compatible: >> + allOf: >> + - const: gunyah-hypervisor >> + - const: simple-bus > What? No. > > Don't create patches with AI. This patch was not created with AI. Reference was taken from the patch [1]. That being said, I see your point about the mistakes which were made while adding the compatible "simple-bus". I apologize for the same. I will make sure `make dt_binding_check` passes with latest versions of dtschema and yamllint as pointed out by Rob and as should have been done with this patch as well. [1] https://lore.kernel.org/all/20240222-gunyah-v17-2-1e9da6763d38@quicinc.com/ Thanks, Hrishabh >> + >> + "#address-cells": >> + description: Number of cells needed to represent 64-bit capability IDs. >> + const: 2 >> + >> + "#size-cells": >> + description: must be 0, because capability IDs are not memory address >> + ranges and do not have a size. >> + const: 0 >> + >> +patternProperties: >> + "^gh-watchdog": > > I could not express more: NAK. Does not match any DT style. Please do > some internal reviews first. This patch does not meet minimum quality > criteria for public posting. > > > Best regards, > Krzysztof
On 04/09/2025 15:07, Hrishabh Rajput wrote: >>> +properties: >>> + compatible: >>> + allOf: >>> + - const: gunyah-hypervisor >>> + - const: simple-bus >> What? No. >> >> Don't create patches with AI. > > This patch was not created with AI. Reference was taken from the patch [1]. There is no such syntax like allOf in [1]. Nowhere in Linux kernel, btw, that's some total invention, thus my gut told me - it must be made with poor AI tools. > > That being said, I see your point about the mistakes which were made > while adding the compatible "simple-bus". > I apologize for the same. > > I will make sure `make dt_binding_check` passes with latest versions of > dtschema and yamllint as pointed out by Rob and as should have been done > with this patch as well. No, that's not enough. You should ask for internal review. I did an extra effort, I checked that and: 1. You did post it for internal review, BUT: 2. Your internal testing system pointed out errors (schema failure) or failed itself, 3. You did not ask your internal testing system to RETEST the patch, in case this was a system failure. That's your mistake. If this was true failure of schema, then you obviously should not send it, but investigate why schema fails on your patch. 4. You did not receive review (at least no track of it) but decided to post it on mailing list. That's also your mistake, because lack of internal review does not mean you can post it to the mailing lists. Talk with your managers or colleagues about missing review, for example. Best regards, Krzysztof
On 9/4/2025 6:47 PM, Krzysztof Kozlowski wrote: > On 04/09/2025 15:07, Hrishabh Rajput wrote: >>>> +properties: >>>> + compatible: >>>> + allOf: >>>> + - const: gunyah-hypervisor >>>> + - const: simple-bus >>> What? No. >>> >>> Don't create patches with AI. >> This patch was not created with AI. Reference was taken from the patch [1]. > There is no such syntax like allOf in [1]. Nowhere in Linux kernel, btw, > that's some total invention, thus my gut told me - it must be made with > poor AI tools. > >> That being said, I see your point about the mistakes which were made >> while adding the compatible "simple-bus". >> I apologize for the same. >> >> I will make sure `make dt_binding_check` passes with latest versions of >> dtschema and yamllint as pointed out by Rob and as should have been done >> with this patch as well. > No, that's not enough. > > You should ask for internal review. I did an extra effort, I checked > that and: > > 1. You did post it for internal review, BUT: > > 2. Your internal testing system pointed out errors (schema failure) or > failed itself, > > 3. You did not ask your internal testing system to RETEST the patch, in > case this was a system failure. That's your mistake. If this was true > failure of schema, then you obviously should not send it, but > investigate why schema fails on your patch. > > 4. You did not receive review (at least no track of it) but decided to > post it on mailing list. That's also your mistake, because lack of > internal review does not mean you can post it to the mailing lists. Talk > with your managers or colleagues about missing review, for example. > > Best regards, > Krzysztof I agree with your points. I will do my best so that situations like these don't arise and save everyone's time. Apologies. Thanks, Hrishabh
On Thu, Sep 04, 2025 at 11:52:32AM +0200, Krzysztof Kozlowski wrote: > On 03/09/2025 21:33, Hrishabh Rajput via B4 Relay wrote: > > From: Hrishabh Rajput <hrishabh.rajput@oss.qualcomm.com> > > > > The Gunyah Hypervisor applies a devicetree overlay providing the > > pretimeout interrupt for the Gunyah Watchdog that it will be using to > > notify watchdog's pretimeout event. Add the DT bindings that Gunyah > > adheres to for the hypervisor and watchdog. > > Wasn't tested, so limited review. > > Please use subject prefixes matching the subsystem. You can get them for > example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory > your patch is touching. For bindings, the preferred subjects are > explained here: > https://www.kernel.org/doc/html/latest/devicetree/bindings/submitting-patches.html#i-for-patch-submitters > > A nit, subject: drop second/last, redundant "bindings". The > "dt-bindings" prefix is already stating that these are bindings. > See also: > https://elixir.bootlin.com/linux/v6.17-rc3/source/Documentation/devicetree/bindings/submitting-patches.rst#L18 > > > > > > Signed-off-by: Hrishabh Rajput <hrishabh.rajput@oss.qualcomm.com> > > --- > > .../bindings/watchdog/qcom,gh-watchdog.yaml | 76 ++++++++++++++++++++++ > > MAINTAINERS | 1 + > > 2 files changed, 77 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/watchdog/qcom,gh-watchdog.yaml b/Documentation/devicetree/bindings/watchdog/qcom,gh-watchdog.yaml > > new file mode 100644 > > index 000000000000..bde8438c6242 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/watchdog/qcom,gh-watchdog.yaml > > @@ -0,0 +1,76 @@ > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > +%YAML 1.2 > > +--- > > +$id: http://devicetree.org/schemas/watchdog/qcom,gh-watchdog.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: Qualcomm Gunyah Virtual Watchdog > > + > > +maintainers: > > + - Hrishabh Rajput <hrishabh.rajput@oss.qualcomm.com> > > + > > +description: |+ > > + The Gunyah Hypervisor provides an SMC-based watchdog interface for its virtual > > + machines. The virtual machines use this information to determine the > > + pretimeout IRQ which the hypervisor will be using to communicate pretimeout > > + event. > > + See also: [1] > > + > > + [1]: https://github.com/quic/gunyah-resource-manager/blob/1b23ceb0dfa010b3b6b5a5f7a4ec1e95b93ab99d/src/vm_creation/dto_construct.c#L519 > > + > > +properties: > > + compatible: > > + allOf: > > + - const: gunyah-hypervisor > > + - const: simple-bus > > What? No. > > Don't create patches with AI. > I am next to Hrishabh when he is writing this patch. I can confirm he did not use AI :-) not sure what tool Krzysztof is using to catch patches being written with AI, that tool needs some improvement for sure. I will let Hrishabh share why he put simple-bus here. Thanks, Pavan
On 04/09/2025 12:16, Pavan Kondeti wrote: >>> + compatible: >>> + allOf: >>> + - const: gunyah-hypervisor >>> + - const: simple-bus >> >> What? No. >> >> Don't create patches with AI. >> > I am next to Hrishabh when he is writing this patch. I can confirm he > did not use AI :-) not sure what tool Krzysztof is using to catch My brain? > patches being written with AI, that tool needs some improvement for > sure. Heh? Seriously, instead replying something like this think from how is it possible to come with such syntax? It does not exist. NOWHERE. It had to be completely hallucinated by AI because I cannot imagine coming with code which is completely different then EVERYTHING else. There is no single code looking like that. > > I will let Hrishabh share why he put simple-bus here. It is not about simple-bus! Best regards, Krzysztof
On 04/09/2025 12:49, Krzysztof Kozlowski wrote: > On 04/09/2025 12:16, Pavan Kondeti wrote: >>>> + compatible: >>>> + allOf: >>>> + - const: gunyah-hypervisor >>>> + - const: simple-bus >>> >>> What? No. >>> >>> Don't create patches with AI. >>> >> I am next to Hrishabh when he is writing this patch. I can confirm he >> did not use AI :-) not sure what tool Krzysztof is using to catch > > My brain? > >> patches being written with AI, that tool needs some improvement for >> sure. > > Heh? Seriously, instead replying something like this think from how is > it possible to come with such syntax? > > It does not exist. NOWHERE. > > It had to be completely hallucinated by AI because I cannot imagine > coming with code which is completely different then EVERYTHING else. > There is no single code looking like that. > > >> >> I will let Hrishabh share why he put simple-bus here. > > > It is not about simple-bus! > And to clarify: it's not only about this part of the binding. Entire binding is terrible, does not meet any basic standards, does not follow basic principles of writing DTS. I cannot imagine this code passing internal review, so hallucinated AI is the most reasonable explanation. Sorry, if you send extremely poor code using patterns which do not exist, that;s either huge waste of community time or AI-based waste of community time. Best regards, Krzysztof
On Thu, Sep 04, 2025 at 12:59:14PM +0200, Krzysztof Kozlowski wrote: > On 04/09/2025 12:49, Krzysztof Kozlowski wrote: > > On 04/09/2025 12:16, Pavan Kondeti wrote: > >>>> + compatible: > >>>> + allOf: > >>>> + - const: gunyah-hypervisor > >>>> + - const: simple-bus > >>> > >>> What? No. > >>> > >>> Don't create patches with AI. > >>> > >> I am next to Hrishabh when he is writing this patch. I can confirm he > >> did not use AI :-) not sure what tool Krzysztof is using to catch > > > > My brain? > > > >> patches being written with AI, that tool needs some improvement for > >> sure. > > > > Heh? Seriously, instead replying something like this think from how is > > it possible to come with such syntax? > > > > It does not exist. NOWHERE. > > > > It had to be completely hallucinated by AI because I cannot imagine > > coming with code which is completely different then EVERYTHING else. > > There is no single code looking like that. > > > > > >> > >> I will let Hrishabh share why he put simple-bus here. > > > > > > It is not about simple-bus! > > Ok, I see what we messed up there around compatible property when adding simple-bus. Sorry about this. > > And to clarify: it's not only about this part of the binding. Entire > binding is terrible, does not meet any basic standards, does not follow > basic principles of writing DTS. I cannot imagine this code passing > internal review, so hallucinated AI is the most reasonable explanation. > Sorry, if you send extremely poor code using patterns which do not > exist, that;s either huge waste of community time or AI-based waste of > community time. > Point taken. The intention is absolutely not to waste the time of reviewers or maintainers. We will make sure to apply the lesson learned here. Thanks again for your time. Thanks, Pavan
On Wed, 03 Sep 2025 19:33:59 +0000, Hrishabh Rajput wrote: > The Gunyah Hypervisor applies a devicetree overlay providing the > pretimeout interrupt for the Gunyah Watchdog that it will be using to > notify watchdog's pretimeout event. Add the DT bindings that Gunyah > adheres to for the hypervisor and watchdog. > > Signed-off-by: Hrishabh Rajput <hrishabh.rajput@oss.qualcomm.com> > --- > .../bindings/watchdog/qcom,gh-watchdog.yaml | 76 ++++++++++++++++++++++ > MAINTAINERS | 1 + > 2 files changed, 77 insertions(+) > My bot found errors running 'make dt_binding_check' on your patch: yamllint warnings/errors: dtschema/dtc warnings/errors: Error: Documentation/devicetree/bindings/watchdog/qcom,gh-watchdog.example.dts:37.3-38.1 syntax error FATAL ERROR: Unable to parse input tree make[2]: *** [scripts/Makefile.dtbs:131: Documentation/devicetree/bindings/watchdog/qcom,gh-watchdog.example.dtb] Error 1 make[2]: *** Waiting for unfinished jobs.... make[1]: *** [/builds/robherring/dt-review-ci/linux/Makefile:1519: dt_binding_check] Error 2 make: *** [Makefile:248: __sub-make] Error 2 doc reference errors (make refcheckdocs): See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20250903-gunyah_watchdog-v1-1-3ae690530e4b@oss.qualcomm.com The base for the series is generally the latest rc1. A different dependency should be noted in *this* patch. If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date: pip3 install dtschema --upgrade Please check and re-submit after running the above command yourself. Note that DT_SCHEMA_FILES can be set to your schema file to speed up checking your schema. However, it must be unset to test all examples with your schema.
© 2016 - 2025 Red Hat, Inc.