Re: [PATCH v2 2/3] dt-bindings: dp83867: add binding for output-impedance

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The property `ti,output-impedance-ohms` expects a register value (0-31) instead of an actual resistance in ohms, violating standard devicetree unit suffix rules.
--

commit baf7480d85f53a0dc882f36f231a4b891961b193
Author: Steffen Trumtrar <s.trumtrar@pengutronix.de>

dt-bindings: dp83867: add binding for output-impedance

This commit adds a new devicetree binding, ti,output-impedance-ohms, to allow
specifying the output impedance for the DP83867 PHY via a fixed value. The
existing boolean values for minimum and maximum still take precedence over the
new value.

> diff --git a/Documentation/devicetree/bindings/net/ti,dp83867.yaml b/Documentation/devicetree/bindings/net/ti,dp83867.yaml
> --- a/Documentation/devicetree/bindings/net/ti,dp83867.yaml
> +++ b/Documentation/devicetree/bindings/net/ti,dp83867.yaml
> @@ -38,6 +38,16 @@ properties:
>      items:
>        - const: io_impedance_ctrl
>  
> +  ti,output-impedance-ohms:
> +    description: |
> +      MAC Interface Impedance control to set the programmable output impedance.
> +      Valid range is 0x00 to 0x1f (32 steps).
> +      - 0x1f = 35 ohms (minimum impedance)
> +      - 0x00 = 70 ohms (maximum impedance)
> +      - Step size varies with process.
> +    minimum: 0
> +    maximum: 31

[Severity: Medium]
Does the property name ti,output-impedance-ohms violate devicetree unit suffix
conventions by accepting raw register values (0-31) instead of actual ohms?

According to standard devicetree property rules, the -ohms suffix dictates
that the value must be specified in Ohms. However, the schema restricts this
property to raw register step values (0 to 31, where 31 configures 35 ohms
and 0 configures 70 ohms).

This mismatch could actively mislead devicetree authors. A developer might
provide a value like 35 which will be rejected by the schema as it exceeds
the maximum of 31, or dangerously configure 31 believing it sets 31 ohms,
but actually configuring 35 ohms.

Could this property be renamed to omit the -ohms suffix, or could the driver
and binding be updated to accept the actual resistance in ohms?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260826-v7-2-topic-dp83867-v2-0-a97b4cec6e93@pengutronix.de?part=2