Re: [PATCH net-next v4 2/2] net: phy: add DAPU Telecom DAP8211R(I) Gigabit Ethernet PHY driver

On Tue, Jul 28, 2026 at 10:53 AM <[email protected]> wrote:

> > +config DAP8211R_PHY
> > +     tristate "DAPU Telecom DAP8211R(I) Gigabit Ethernet PHY"
> > +     depends on OF
> > +     help
> > +       Support for the DAPU Telecom DAP8211R(I) Gigabit Ethernet PHY.
> > +       This PHY is designed for enterprise and industrial networking
> > +       applications, supporting 10/100/1000 Mbps operation.
> > +       RGMII with: configurable TX/RX clock delays, optional flag to enable
> > +       180-degree TX clock phase shift and internal packet generator.
>
> [Severity: Medium]
> Are the 180-degree TX clock phase shift and internal packet generator features
> actually implemented in this driver?
>
> The help text advertises these features for users configuring the kernel, but
> looking at the driver code, there doesn't seem to be any implementation for
> them. Could this mislead users who expect these features to be available based
> on the documentation?
Yes, I'll fix it.

> > +     switch (phydev->interface) {
> > +     case PHY_INTERFACE_MODE_RGMII:
> > +             if (rx_internal_delay < 0)
> > +                     rx_internal_delay = 0;
> > +
> > +             if (tx_internal_delay < 0)
> > +                     tx_internal_delay = 0;
> > +             break;
> > +     case PHY_INTERFACE_MODE_RGMII_RXID:
> > +             if (rx_internal_delay < 0)
> > +                     rx_internal_delay = DAP8211R_DEFAULT_DEL_SEL;
> > +
> > +             tx_internal_delay = 0;
>
> [Severity: Medium]
> Is it intentional to silently drop an explicitly provided tx-internal-delay-ps
> property when using PHY_INTERFACE_MODE_RGMII_RXID?
>
> For PHY_INTERFACE_MODE_RGMII, explicit delay properties are kept and applied.
> However, in dap8211r_config_init(), tx_internal_delay is hardcoded to 0 for
> the rgmii-rxid mode.
>
> This might lead to unexpected configuration drops if a device tree specifies
> phy-mode = "rgmii-rxid" along with a tx-internal-delay-ps property, as the
> requested delay will be ignored without warning.

Yes, this is intentional.

The RGMII interface modes are defined as:
RGMII_RXID: MAC provides TX delay, PHY provides RX delay
RGMII_TXID: MAC provides RX delay, PHY provides TX delay
RGMII_ID: Both delays are handled by the PHY

For RGMII_RXID, the TX delay is not expected to be provided via dts,
so any TX delay value specified in DT for the PHY would be incorrect and
is therefore ignored. The same applies to RGMII_TXID for RX delays.

I did this to keep the switch statement simpler. I thought it would be better
than adding these features to its cases.
--
Best regards,
Artem