Re: [PATCH v4 01/12] ASoC: dt-bindings: renesas,rsnd: Split into generic and SoC-specific parts

[email protected]
Newsgroups org.kernel.vger.linux-devicetree
Message-ID <[email protected]>
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Medium] The extracted common schema silently drops strict validation constraints on `dmas` and `dma-names` for the `dvc`, `src`, `ssiu`, and `ssi` subnodes.
- [Medium] Validation string pattern constraints for `clock-names` and `reset-names` were entirely dropped for Gen1 SoCs.
- [Medium] The `reg` property strict length constraints for Gen2/Gen3 and Gen4 SoCs were unintentionally relaxed by replacing `minItems` with `maxItems`.
--

--- Patch [1]: [PATCH v4 01/12] ASoC: dt-bindings: renesas,rsnd: Split into generic and SoC-specific parts ---
commit 6e078a2691ebf1fb098584188a1f25bc334a8942
Author: John Madieu <[email protected]>

ASoC: dt-bindings: renesas,rsnd: Split into generic and SoC-specific parts

Refactors the renesas,rsnd.yaml binding by extracting common properties
shared across all SoCs into a dedicated renesas,rsnd-common.yaml schema
to simplify future additions.

> diff --git a/Documentation/devicetree/bindings/sound/renesas,rsnd-common.yaml b/Documentation/devicetree/bindings/sound/renesas,rsnd-common.yaml
> new file mode 100644
> index 0000000000000..ec6bf644d1a4e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/sound/renesas,rsnd-common.yaml

[ ... ]

> +  rcar_sound,dvc:
> +    description: DVC subnode.
> +    type: object
> +    patternProperties:
> +      "^dvc-[0-1]$":
> +        type: object
> +        additionalProperties: false
> +        properties:
> +          dmas: true
> +          dma-names: true

[Severity: Medium]
Does this unintentionally drop the validation rules for DMA channels and
names inside the dvc, src, ssiu, and ssi subnodes?

The original schema had explicit constraints for these properties, such as
array boundaries and specific enum lists for dma-names. Setting these
properties to true in the common schema might allow arrays of arbitrary
lengths or string typos to silently pass validation.

> diff --git a/Documentation/devicetree/bindings/sound/renesas,rsnd.yaml b/Documentation/devicetree/bindings/sound/renesas,rsnd.yaml
> index e8a2acb926460..0d989922a5b4b 100644
> --- a/Documentation/devicetree/bindings/sound/renesas,rsnd.yaml
> +++ b/Documentation/devicetree/bindings/sound/renesas,rsnd.yaml

[ ... ]

>  allOf:
> -  - $ref: dai-common.yaml#
> +  - $ref: renesas,rsnd-common.yaml#
>  
> -  # --------------------
> -  # reg/reg-names
> -  # --------------------
> -  # for Gen1
>    - if:
>        properties:
>          compatible:
> @@ -295,11 +134,10 @@ allOf:
>            maxItems: 3
>          reg-names:
>            items:
> -            enum:
> -              - sru
> -              - ssi
> -              - adg
> -  # for Gen2/Gen3
> +            - const: sru
> +            - const: ssi
> +            - const: adg
> +

[Severity: Medium]
Are we leaving clock-names and reset-names completely unrestricted for Gen1
SoCs here?

The global else block containing the string pattern validations was moved
to the Gen2/Gen3 conditional block, but it seems to have been omitted
entirely for Gen1.

>    - if:
>        properties:
>          compatible:
> @@ -310,16 +148,34 @@ allOf:
>      then:
>        properties:
>          reg:
> -          minItems: 5
> +          maxItems: 5
>          reg-names:
>            items:

[Severity: Medium]
Could this relax the register mapping length constraints by replacing
minItems with maxItems?

Gen2 and Gen3 previously required exactly 5 regions. A device tree
supplying fewer elements might now silently pass validation since the
schema evaluates successfully if the input array is shorter than the list.

This same minItems replacement appears to have occurred in the Gen4 block
as well.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.