Re: [PATCH] dt-bindings: misc: add binding for Xilinx AXI4-Stream FIFO MM S

[sashiko-bot@kernel.org]

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 <danyboy28sep@gmail.com>

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.

> 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

[ ... ]

> +  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.

[ ... ]

> +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