Re: [PATCH net-next v4 1/7] dt-bindings: net: ftgmac100: support for AST2700

[Krzysztof Kozlowski]

On Thu, Dec 05, 2024 at 03:20:42PM +0800, Jacky Chou wrote:
> The AST2700 is the 7th generation SoC from Aspeed.
> Add compatible support and resets property for AST2700 in
> yaml.
> 
> Signed-off-by: Jacky Chou <jacky_chou@xxxxxxxxxxxxxx>
> Acked-by: Conor Dooley <conor.dooley@xxxxxxxxxxxxx>


Your changelog in cover letter does not mention received ack. When did
it happen?

> ---
>  .../bindings/net/faraday,ftgmac100.yaml         | 17 ++++++++++++++++-
>  1 file changed, 16 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/net/faraday,ftgmac100.yaml b/Documentation/devicetree/bindings/net/faraday,ftgmac100.yaml
> index 9bcbacb6640d..3bba8eee83d6 100644
> --- a/Documentation/devicetree/bindings/net/faraday,ftgmac100.yaml
> +++ b/Documentation/devicetree/bindings/net/faraday,ftgmac100.yaml
> @@ -21,6 +21,7 @@ properties:
>                - aspeed,ast2400-mac
>                - aspeed,ast2500-mac
>                - aspeed,ast2600-mac
> +              - aspeed,ast2700-mac
>            - const: faraday,ftgmac100
>  
>    reg:
> @@ -33,7 +34,7 @@ properties:
>      minItems: 1
>      items:
>        - description: MAC IP clock
> -      - description: RMII RCLK gate for AST2500/2600
> +      - description: RMII RCLK gate for AST2500/2600/2700
>  
>    clock-names:
>      minItems: 1
> @@ -73,6 +74,20 @@ required:
>  
>  unevaluatedProperties: false
>  
> +if:
> +  properties:
> +    compatible:
> +      contains:
> +        const: aspeed,ast2700-mac

1. That's a signigicant change. *Drop ack.*

2. Test your bindings.
3. Put if: block under allOf: and move entire allOf just above your
unevaluatedProperties... if this stays.
4. But you cannot define properties in if:then. They must be defined in
top level. You can disallow them for variants in if:then: with :false"

Even exmaple schema has exactly this case:
https://elixir.bootlin.com/linux/v5.19/source/Documentation/devicetree/bindings/example-schema.yaml#L212

Best regards,
Krzysztof