Re: [PATCH V2] dt: net: enhance DWC EQoS binding to support Tegra186

[Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>]

On 09/01/2016 01:50 AM, Lars Persson wrote:
>
>
> On 08/31/2016 11:48 PM, Stephen Warren wrote:
>> On 08/31/2016 03:15 AM, Lars Persson wrote:
>>> On 08/30/2016 10:50 PM, Stephen Warren wrote:
>>>> On 08/30/2016 01:01 PM, Rob Herring wrote:
>>>>> On Wed, Aug 24, 2016 at 03:20:46PM -0600, Stephen Warren wrote:
>>>>>> The Synopsys DWC EQoS is a configurable IP block which supports
>>>>>> multiple
>>>>>> options for bus type, clocking and reset structure, and feature list.
>>>>>> Extend the DT binding to define a "compatible value" for the
>>>>>> configuration
>>>>>> contained in NVIDIA's Tegra186 SoC, and define some new properties
>>>>>> and
>>>>>> list property entries required by that configuration.
>>
>>>>>> diff --git
>>>>>> a/Documentation/devicetree/bindings/net/snps,dwc-qos-ethernet.txt
>>>>>> b/Documentation/devicetree/bindings/net/snps,dwc-qos-ethernet.txt
>>
>>>>>> +- clock-names: May contain any/all of the following depending on
>>>>>> the IP
>>>>>> +  configuration, in any order:
>>>>>> +    The EQOS transmit path clock. The HW signal name is clk_tx_i.
>>>>>> +    In some configurations (e.g. GMII/RGMII), this clock also drives
>>>>>> the PHY TX
>>>>>> +    path. In other configurations, other clocks (such as tx_125,
>>>>>> rmii) may
>>>>>> +    drive the PHY TX path.
>>>>>> +  - "rx"
>>>>>> +    The EQOS receive path clock. The HW signal name is clk_rx_i.
>>>>>> +    In some configurations (e.g. GMII/RGMII), this clock also drives
>>>>>> the PHY RX
>>>>>> +    path. In other configurations, other clocks (such as rx_125,
>>>>>> pmarx_0,
>>>>>> +    pmarx_1, rmii) may drive the PHY RX path.
>>>
>>> It is not correct that clk_rx_i drives the PHY rx path for GMII/RGMII.
>>> The PHY is the source of the rx clock for these modes.
>>
>> I think both of our statements are true.
>>
>> There's a clock input to the EQOS module (clk_rx_i) that does drive the
>> RX path in the EQOS module.
>>
>> That clock also drives the PHY's RX path.
>>
>> Those statements make no comment regarding the /source/ of that clock;
>> either of the following might be true:
>>
>> 1) The PHY could generate the clock internally somehow, feed its own
>> internal logic with that clock, and send the clock out to feed the EQOS
>> RX path too.
>>
>> or,
>>
>> 2)  SoC integration could drive the same clock into both the EQOS and
>> PHY modules, so that both sets of logic are fed from the same external
>> clock.
>>
>> Perhaps the phrase "PHY RX path" is confusing; I was talking about the
>> EQOS modules' RX path from the PHY more than the PHY itself, although
>> given what I said above I believe either interpretation is valid and
>> correct.
>>
>>> Will the driver need to make any clock ops on the "rx" clock ?
>>
>> Yes. The EQOS driver needs to ensure that the clock is running before
>> attempting to receive data from the PHY, otherwise the EQOS's own RX
>> logic won't be clocked.
>>
>> Whether the phandle for this clock points at a SoC-level provider (it
>> will in Tegra) or a clock provider in the PHY (it might in other SoCs),
>> shouldn't matter as far as the DT binding goes, although it might affect
>> device probe ordering in some implementations.
>>
>
> I understand your point. The lines between PHY, MAC and SoC gets blurred
> with high integration.
>
> The thing is that when we talk about standard PHY interfaces the clock
> is a well defined part of the interface and it should not be mixed up
> with SoC-specific implementations that deviate from this. Please update
> the description of the rx clock to only cover cases when the clock is
> not implicitly sourced from the PHY.
>
> When the PHY is the source of the clock then it is managed by the phy
> library and the phy driver so it does not need to be also handled by the
> common clock framework.

Ah, I've found the source of confusion. We don't have an SoC-level clock 
that feeds both the MAC and the PHY RX logic, but simply a clock gate 
that sits between the RX clock output of the PHY and the RX clock input 
of the MAC (clk_rx_i). This gate is what's currently represented by the 
"rx" clock name the binding mentions.

tl;dr is that yes I agree to update the description of this rx clock 
along the lines you ask for.