Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] dt-bindings: misc: tx-fifo-depth documented in bytes instead of words
- [Medium] dt-bindings: misc: additionalProperties false rejects valid properties
- [Low] dt-bindings: misc: incomplete migration leaves orphaned text binding
--
commit b044052b8807822a68aa39048ff207a8df629e04
Author: Daniel28972897 [off-list ref]
dt-bindings: misc: add binding for Xilinx AXI4-Stream FIFO MM S
This commit adds a YAML binding schema for the Xilinx AXI4-Stream FIFO MM S IP
core, which is supported by the axis-fifo driver. It documents the compatible
strings and six properties required by the driver.
The axis-fifo driver (drivers/staging/axis-fifo/axis-fifo.c) has
supported the "xlnx,axi-fifo-mm-s-4.1", "xlnx,axi-fifo-mm-s-4.2" and
"xlnx,axi-fifo-mm-s-4.3" compatible strings since it was added, but no
devicetree binding schema exists for them, which checkpatch.pl flags:
[Severity: Low]
Is the claim that no binding schema exists accurate?
The original text binding in drivers/staging/axis-fifo/axis-fifo.txt already
documents this hardware. This commit creates a new YAML binding but fails to
remove or update the existing comprehensive text binding, leaving split and
conflicting documentation in the tree.
quoted hunk ↗ jump to hunk
diff --git a/Documentation/devicetree/bindings/misc/xlnx,axi-fifo-mm-s.yaml b/Documentation/devicetree/bindings/misc/xlnx,axi-fifo-mm-s.yaml
new file mode 100644
index 0000000000000..56d58b37b71ed
--- /dev/null
+++ b/Documentation/devicetree/bindings/misc/xlnx,axi-fifo-mm-s.yaml
[ ... ]
quoted hunk ↗ jump to hunk
+ xlnx,rx-fifo-depth:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description:
+ Depth, in bytes, of the receive FIFO.
+
+ xlnx,tx-fifo-depth:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description:
+ Depth, in bytes, of the transmit FIFO.
[Severity: High]
Does the driver expect these depths in words rather than bytes?
If a user follows this YAML and specifies a depth in bytes (e.g. 2048 for a
512-word hardware FIFO), the driver will erroneously permit writes up to 2044
words.
The driver enforces maximum write sizes via
words_to_write > (fifo->tx_fifo_depth - 4) and calls
wait_event_interruptible() waiting for hardware FIFO vacancy to reach the
requested amount. Since the physical FIFO only has 512 words, this condition
will never be met, causing an infinite wait when a user writes data larger
than the actual hardware capacity.
[ ... ]
quoted hunk ↗ jump to hunk
+required:
+ - compatible
+ - reg
+ - interrupts
+ - xlnx,axi-str-rxd-tdata-width
+ - xlnx,axi-str-txd-tdata-width
+ - xlnx,rx-fifo-depth
+ - xlnx,tx-fifo-depth
+ - xlnx,use-rx-data
+ - xlnx,use-tx-data
+
+additionalProperties: false
[Severity: Medium]
Does setting this to false reject valid device trees?
Xilinx generation tools produce numerous hardware properties like
xlnx,axis-tdest-width and xlnx,axi-str-rxd-protocol which are explicitly
defined in the original axis-fifo.txt.
This new schema omits those valid properties, causing dtbs_check to wrongly
reject existing hardware configurations.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260919011417.156828-1-danyboy28sep@gmail.com?part=1