Re: [RFC PATCH v2 3/7] dt-bindings: net: amlogic,meson-dwmac: add amlogic,t7-dwmac

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] net: stmmac: silent bitfield truncation of tx-internal-delay-ps
--

commit df8d2ca7732ce1f0cf81bbfee0085a28dfb7f50f
Author: Lucas Tanure <tanure@linux.com>

dt-bindings: net: amlogic,meson-dwmac: add amlogic,t7-dwmac

The T7 has the same ethernet glue as the G12A, but the controller
reaches memory through a pipeline stage that this SoC inserts in the
bus path. That stage has a gate of its own, which no other member of
the family has, and nothing else in Linux claims it. It is therefore
switched off as unused, and the port can then no longer finish a
transfer.

Describing it needs a fourth clock, and the fourth clock of the
existing list drives the timing adjustment logic, which the T7 never
uses. So the T7 gets a list of its own with the gate in that place,
under the name the SD and eMMC controllers already use.

Assisted-by: LLM
Signed-off-by: Lucas Tanure <tanure@linux.com>

> diff --git a/Documentation/devicetree/bindings/net/amlogic,meson-dwmac.yaml b/Documentation/devicetree/bindings/net/amlogic,meson-dwmac.yaml
> index 90ef79161ab1a..96118b4d15609 100644
> --- a/Documentation/devicetree/bindings/net/amlogic,meson-dwmac.yaml
> +++ b/Documentation/devicetree/bindings/net/amlogic,meson-dwmac.yaml
[ ... ]
> @@ -57,6 +58,47 @@ allOf:
>              - const: clkin1
>              - const: timing-adjustment
>  
> +  # The T7 applies the RGMII RX delay through PRG_ETH1 and never enables the
> +  # timing adjustment logic, so it has no timing-adjustment clock. What it does
> +  # need is the gate of the pipeline stage the controller reaches DRAM through.
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: amlogic,t7-dwmac
> +    then:
> +      properties:
> +        clocks:
> +          items:
> +            - description: GMAC main clock
> +            - description: First parent clock of the internal mux
> +            - description: Second parent clock of the internal mux
> +            - description:
> +                clock of the pipeline stage inserted in the bus path between
> +                the controller and the DRAM. Without it, the controller cannot
> +                complete DMA transfers.
> +
> +        clock-names:
> +          items:
> +            - const: stmmaceth
> +            - const: clkin0
> +            - const: clkin1
> +            - const: pipeline
> +
> +  # Delay definitions for Meson8b and newer (including T7)
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - amlogic,meson8b-dwmac
> +              - amlogic,meson8m2-dwmac
> +              - amlogic,meson-gxbb-dwmac
> +              - amlogic,meson-axg-dwmac
> +              - amlogic,meson-g12a-dwmac
> +              - amlogic,t7-dwmac
> +    then:
> +      properties:
>          amlogic,tx-delay-ns:
>            enum: [0, 2, 4, 6]
>            default: 2

[Severity: Medium]
Does the schema lack constraints for tx-internal-delay-ps?

The driver reads tx-internal-delay-ps from the device tree and assigns it
without any bounds checking:

drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c:meson8b_dwmac_probe() {
    ...
	if (!of_property_read_u32(pdev->dev.of_node, "tx-internal-delay-ps",
				  &tx_delay_ps))
		dwmac->tx_delay_ns = tx_delay_ps / 1000;
    ...
}

Because it is not validated, a value >= 8000 (8ns) will be silently truncated
by the 2-bit PRG_ETH0_TXDLY_MASK:

drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c:meson8b_init_rgmii_delays() {
    ...
	tx_dly_config = FIELD_PREP(PRG_ETH0_TXDLY_MASK,
				   dwmac->tx_delay_ns >> 1);
    ...
}

Because the schema does not define constraints for tx-internal-delay-ps
and the driver does not validate the bounds, a misconfigured DTS will silently
fail and misconfigure the hardware.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261007082627.63807-1-tanure@linux.com?part=3

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic