Re: [PATCH net-next v2 3/4] dt-bindings: net: Add RGMII internal delay for DP83869

From: Dan Murphy
Date: Wed May 20 2020 - 11:28:13 EST


Andrew

On 5/20/20 8:56 AM, Andrew Lunn wrote:
On Wed, May 20, 2020 at 07:18:34AM -0500, Dan Murphy wrote:
Add the internal delay values into the header and update the binding
with the internal delay properties.

Signed-off-by: Dan Murphy <dmurphy@xxxxxx>
---
.../devicetree/bindings/net/ti,dp83869.yaml | 16 ++++++++++++++++
include/dt-bindings/net/ti-dp83869.h | 18 ++++++++++++++++++
2 files changed, 34 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/ti,dp83869.yaml b/Documentation/devicetree/bindings/net/ti,dp83869.yaml
index 5b69ef03bbf7..344015ab9081 100644
--- a/Documentation/devicetree/bindings/net/ti,dp83869.yaml
+++ b/Documentation/devicetree/bindings/net/ti,dp83869.yaml
@@ -64,6 +64,20 @@ properties:
Operational mode for the PHY. If this is not set then the operational
mode is set by the straps. see dt-bindings/net/ti-dp83869.h for values
+ ti,rx-internal-delay:
+ $ref: /schemas/types.yaml#definitions/uint32
+ description: |
+ RGMII Receive Clock Delay - see dt-bindings/net/ti-dp83869.h
+ for applicable values. Required only if interface type is
+ PHY_INTERFACE_MODE_RGMII_ID or PHY_INTERFACE_MODE_RGMII_RXID.
Hi Dan

Having it required with PHY_INTERFACE_MODE_RGMII_ID or
PHY_INTERFACE_MODE_RGMII_RXID is pretty unusual. Normally these
properties are used to fine tune the delay, if the default of 2ns does
not work.

Also if the MAC phy-mode is configured with RGMII-ID and no internal delay values defined wouldn't that be counter intuitive?

The driver will error out if the RGMII-ID is used and there was no internal delay defined for either rx or tx making either one required.

The MAC node needs to indicate to use the internal delay for RGMII other wise the driver should ignore the internal delay programming as these internal delays are not applicable to SGMII or MII modes. The RGMII mode can be used if the default 2ns delay is acceptable.

Thus why we are documenting in the binding when the internal delay is required as putting these under "required" is not correct.

Dan


Andrew