Re: [PATCH v3 phy-next 14/16] dt-bindings: phy: lynx-10g: initial document

[Conor Dooley <conor@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 2916 bytes --]

On Wed, Jun 03, 2026 at 04:38:25PM +0300, Vladimir Oltean wrote:
> On Wed, Jun 03, 2026 at 01:33:50PM +0000, sashiko-bot@kernel.org wrote:
> > Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> > - [Medium] The `if` condition for enforcing the `big-endian` property incorrectly matches the `compatible` string array directly against an `enum`, which will always evaluate to false and cause the requirement to silently fail.
> > --
> > 
> > commit 8bd3982a89722b20a3686e42d4ab3b9f0711ae11
> > Author: Vladimir Oltean <vladimir.oltean@nxp.com>
> > 
> > dt-bindings: phy: lynx-10g: initial document
> > 
> > This commit adds the schema for the 10G Lynx SerDes used on most QorIQ and
> > Layerscape SoCs. It defines the valid compatible strings, PHY cell requirements,
> > and enforces SoC-specific constraints such as lane counts and endianness.
> > 
> > > diff --git a/Documentation/devicetree/bindings/phy/fsl,lynx-10g.yaml b/Documentation/devicetree/bindings/phy/fsl,lynx-10g.yaml
> > > new file mode 100644
> > > index 0000000000000..8cf6027a75b91
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/phy/fsl,lynx-10g.yaml
> > > +  - if:
> > > +      properties:
> > > +        compatible:
> > > +          enum:
> > > +            - fsl,ls1046a-serdes1
> > > +            - fsl,ls1046a-serdes2
> > 
> > [Severity: Medium]
> > Does this conditional correctly enforce the big-endian property? 
> > 
> > Since the compatible property is a string array in device trees, evaluating it
> > directly against an enum always returns false. Should this use "contains:" 
> > with the enum, similar to the preceding if block, so dt_binding_check 
> > correctly enforces the requirement?
> 
> The tests I did suggest that it does enforce the requirement.

Yeah, sashiko just doesn't understand how this works without "contains".
"contains" is only actually required for validation purposes if there's
fallback compatibles and you're trying to get a partial match.

> 
> - "make dt_binding_check" passes with the submitted example
> 
> - If I replace "fsl,ls1028a-serdes" from the example with
>   "fsl,ls1046a-serdes1" but leave everything else as is (implying:
>   little endian), I get:
> 
> $ make -j 8 DT_CHECKER_FLAGS=-m dt_binding_check DT_SCHEMA_FILES=phy/fsl,lynx-10g.yaml
>   SCHEMA  Documentation/devicetree/bindings/processed-schema.json
>   CHKDT   ../Documentation/devicetree/bindings
>   LINT    ../Documentation/devicetree/bindings
>   DTEX    Documentation/devicetree/bindings/phy/fsl,lynx-10g.example.dts
>   DTC [C] Documentation/devicetree/bindings/phy/fsl,lynx-10g.example.dtb
> Documentation/devicetree/bindings/phy/fsl,lynx-10g.example.dtb: serdes@1ea0000 (fsl,ls1046a-serdes1): 'big-endian' is a required property
>         from schema $id: http://devicetree.org/schemas/phy/fsl,lynx-10g.yaml

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]