Re: [PATCH v10 4/6] dt-bindings: sun6i-a31-mipi-dphy: Add V3s SoC compatible entry

[Paul Kocialkowski <paulk@sys-base.io>]

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

Hi,

Le Sat 13 Jun 26, 15:35, sashiko-bot@kernel.org a écrit :
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] The newly added V3s compatible string is for an rx-only hardware variant, but the binding schema allows it to incorrectly default to `tx`.
> --
> 
> commit 2fd664ce10fb7edbf45c93a3bcbbd8adb0bad2ea
> Author: Paul Kocialkowski <paulk@sys-base.io>
> 
> dt-bindings: sun6i-a31-mipi-dphy: Add V3s SoC compatible entry
> 
> The V3s/V3/S3 comes with a rx-only D-PHY paired with the MIPI CSI-2
> controller. It is compatible with the D-PHY found on the A31.
> Add an entry with a new compatible and the A31 compatible as fallback.
> 
> > diff --git a/Documentation/devicetree/bindings/phy/allwinner,sun6i-a31-mipi-dphy.yaml b/Documentation/devicetree/bindings/phy/allwinner,sun6i-a31-mipi-dphy.yaml
> > index 6a4fd49299590..3ca1a1c47032f 100644
> > --- a/Documentation/devicetree/bindings/phy/allwinner,sun6i-a31-mipi-dphy.yaml
> > +++ b/Documentation/devicetree/bindings/phy/allwinner,sun6i-a31-mipi-dphy.yaml
> > @@ -21,6 +21,9 @@ properties:
> >        - items:
> >            - const: allwinner,sun50i-a64-mipi-dphy
> >            - const: allwinner,sun6i-a31-mipi-dphy
> > +      - items:
> > +          - const: allwinner,sun8i-v3s-mipi-dphy
> > +          - const: allwinner,sun6i-a31-mipi-dphy
> 
> [Severity: Medium]
> Since the commit message indicates the V3s D-PHY is rx-only, should this
> schema include an if-then block to enforce the rx direction?
> 
> The base binding defines allwinner,direction with a default of tx:
> 
>   allwinner,direction:
>     ...
>     enum:
>       - tx
>       - rx
>     default: tx
> 
> Without an allOf if-then block constraining this property for the
> allwinner,sun8i-v3s-mipi-dphy compatible string, a V3s device tree node
> that omits the property will implicitly default to tx.
> 
> Would this allow an incomplete device tree node to pass schema validation
> while misconfiguring the rx-only hardware?

That is correct, but maybe a bit over the top. If someone thinks this
should be done, feel free to let me know and I can respin.

All the best,

Paul

-- 
Paul Kocialkowski,

Independent contractor - sys-base - https://www.sys-base.io/
Free software developer - https://www.paulk.fr/

Expert in multimedia, graphics and embedded hardware support with Linux.

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