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

[Dan Murphy <dmurphy@ti.com>]

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@ti.com>
>> ---
>>   .../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