Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1707194

Re: [PATCH v3 05/11] net: stmmac: dwmac-rk: Add internal phy support

Show all headers | View raw

Hello Corentin, Chen-Yu

在 2017/8/9 16:45, Corentin Labbe 写道:
> On Thu, Aug 03, 2017 at 07:06:33PM +0800, Chen-Yu Tsai wrote:
>> On Thu, Aug 3, 2017 at 1:38 AM, Florian Fainelli <f.fainelli@gmail.com> wrote:
>>> On 08/01/2017 11:21 PM, David Wu wrote:
>>>> To make internal phy work, need to configure the phy_clock,
>>>> phy cru_reset and related registers.
>>>>
>>>> Signed-off-by: David Wu <david.wu@rock-chips.com>
>>>> ---
>>>>   .../devicetree/bindings/net/rockchip-dwmac.txt     |  6 +-
>>>>   drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c     | 81 ++++++++++++++++++++++
>>>>   2 files changed, 86 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/net/rockchip-dwmac.txt b/Documentation/devicetree/bindings/net/rockchip-dwmac.txt
>>>> index 8f42755..ec39b31 100644
>>>> --- a/Documentation/devicetree/bindings/net/rockchip-dwmac.txt
>>>> +++ b/Documentation/devicetree/bindings/net/rockchip-dwmac.txt
>>>> @@ -25,7 +25,8 @@ Required properties:
>>>>    - clock-names: One name for each entry in the clocks property.
>>>>    - phy-mode: See ethernet.txt file in the same directory.
>>>>    - pinctrl-names: Names corresponding to the numbered pinctrl states.
>>>> - - pinctrl-0: pin-control mode. can be <&rgmii_pins> or <&rmii_pins>.
>>>> + - pinctrl-0: pin-control mode. can be <&rgmii_pins>, <&rmii_pins> or led pins
>>>> +   for internal phy mode.
>>>>    - clock_in_out: For RGMII, it must be "input", means main clock(125MHz)
>>>>      is not sourced from SoC's PLL, but input from PHY; For RMII, "input" means
>>>>      PHY provides the reference clock(50MHz), "output" means GMAC provides the
>>>> @@ -40,6 +41,9 @@ Optional properties:
>>>>    - tx_delay: Delay value for TXD timing. Range value is 0~0x7F, 0x30 as default.
>>>>    - rx_delay: Delay value for RXD timing. Range value is 0~0x7F, 0x10 as default.
>>>>    - phy-supply: phandle to a regulator if the PHY needs one
>>>> + - clocks: <&cru MAC_PHY>: Clock selector for internal macphy
>>>> + - phy-is-internal: A boolean property allows us to know that MAC will connect to
>>>> +   internal phy.
>>>
>>> This is incorrect in two ways:
>>>
>>> - this is a property of the PHY, so it should be documented as such in
>>> Documentation/devicetree/bindings/net/phy.txt so other bindings can
>>> re-use it
>>>
>>> - if it was specific to your MAC you would expect a vendor prefix to
>>> this property, which is not there
>>>
>>> An alternative way to implement the external/internal logic selection
>>> would be mandate that your Ethernet PHY node have a compatible string
>>> like this:
>>>
>>> phy@0 {
>>>          compatible = "ethernet-phy-id1234.d400", "ethernet-phy-802.3-c22";
>>> };
>>>
>>> Then you don't need this phy-is-internal property, because you can
>>> locate the PHY node by the phy-handle (see more about that in a reply to
>>> patch 10) and you can determine ahead of time whether this PHY is
>>> internal or not based on its compatible string.
>>
>> This doesn't really work for us (sunxi). The "internal" PHY of the H3
>> is also available in the X-Powers AC200 combo chip, which would be
>> external. Same ID. So if someone were to be stupid and put the two
>> together, you wouldn't know which one it actually is. Generically
>> it doesn't make sense to match against the ID either. The PHY is
>> only integrated or inlined into the SoC, meaning it could be moved
>> elsewhere or even be a discreet part.
>>
>> The way I see it is more like a reversed pinmux. The system should
>> select either the internal set or external set of xMII pins to use.
>>
>> A phy-is-internal property in the PHY node would work for us. We
>> already need a PHY node to describe its clocks and resets.
>>
>> A side note to this solution is that, since the internal PHY is defined
>> at the .dtsi level, any external PHYs at the same address would need to
>> make sure to delete the property, which is kind of counterintuitive, but
>> it is how device tree works. One would want to put the internal PHY's
>> address, assuming it is configurable, on something that is rarely used.
>>
> 
> Hello David, Florian, Andrew
> 
> Could someone ack this ? or nack if you think that the chance for having two PHY id both internal and external is too low.
> Anyway, we need a choice.
> 

I think we should be specific to the situation, for us we have the 
possibility that the Mac only picks up internal PHY, so this can be 
fixed at the. DTSi level, also possible INTERNL PHY's Mac can also be 
used to connect external PHY, while cutting off the connection with the 
internal PHY, so we should define the internal PHY at the DTS level, so 
I think Florian's approach is acceptable.

> Regards
> 
> 
> 

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[PATCH v3 00/11] Add the internal phy support David Wu <david.wu@rock-chips.com> - 2017-08-02 08:20 +0200
  [PATCH v3 02/11] multi_v7_defconfig: Make rockchip phy built-in David Wu <david.wu@rock-chips.com> - 2017-08-02 08:20 +0200
  [PATCH v3 03/11] arm64: defconfig: Enable CONFIG_ROCKCHIP_PHY David Wu <david.wu@rock-chips.com> - 2017-08-02 08:20 +0200
  [PATCH v3 01/11] net: phy: Add rockchip phy driver support David Wu <david.wu@rock-chips.com> - 2017-08-02 08:20 +0200
    Re: [PATCH v3 01/11] net: phy: Add rockchip phy driver support Corentin Labbe <clabbe.montjoie@gmail.com> - 2017-08-02 08:30 +0200
    Re: [PATCH v3 01/11] net: phy: Add rockchip phy driver support Andrew Lunn <andrew@lunn.ch> - 2017-08-02 15:30 +0200
      Re: [PATCH v3 01/11] net: phy: Add rockchip phy driver support "David.Wu" <david.wu@rock-chips.com> - 2017-08-09 09:10 +0200
  [PATCH v3 05/11] net: stmmac: dwmac-rk: Add internal phy support David Wu <david.wu@rock-chips.com> - 2017-08-02 08:20 +0200
    Re: [PATCH v3 05/11] net: stmmac: dwmac-rk: Add internal phy support Florian Fainelli <f.fainelli@gmail.com> - 2017-08-02 19:40 +0200
      Re: [PATCH v3 05/11] net: stmmac: dwmac-rk: Add internal phy support Chen-Yu Tsai <wens@csie.org> - 2017-08-03 13:10 +0200
        Re: [PATCH v3 05/11] net: stmmac: dwmac-rk: Add internal phy support Corentin Labbe <clabbe.montjoie@gmail.com> - 2017-08-09 10:50 +0200
          Re: [PATCH v3 05/11] net: stmmac: dwmac-rk: Add internal phy support "David.Wu" <david.wu@rock-chips.com> - 2017-08-09 11:40 +0200
            Re: [PATCH v3 05/11] net: stmmac: dwmac-rk: Add internal phy support Chen-Yu Tsai <wens@csie.org> - 2017-08-10 04:50 +0200
              Re: [PATCH v3 05/11] net: stmmac: dwmac-rk: Add internal phy support "David.Wu" <david.wu@rock-chips.com> - 2017-08-10 09:50 +0200
          Re: [PATCH v3 05/11] net: stmmac: dwmac-rk: Add internal phy support Florian Fainelli <f.fainelli@gmail.com> - 2017-08-10 00:50 +0200
      Re: [PATCH v3 05/11] net: stmmac: dwmac-rk: Add internal phy support "David.Wu" <david.wu@rock-chips.com> - 2017-08-03 16:10 +0200
  [PATCH v3 08/11] ARM: dts: rk322x: Add support internal phy for gmac David Wu <david.wu@rock-chips.com> - 2017-08-02 08:20 +0200
  [PATCH v3 04/11] net: stmmac: dwmac-rk: Remove unwanted code for rk3328_set_to_rmii() David Wu <david.wu@rock-chips.com> - 2017-08-02 08:20 +0200
  [PATCH v3 07/11] net: stmmac: dwmac-rk: Add internal phy supprot for rk3328 David Wu <david.wu@rock-chips.com> - 2017-08-02 08:20 +0200
  [PATCH v3 09/11] ARM: dts: rk3228-evb: Enable the internal phy for gmac David Wu <david.wu@rock-chips.com> - 2017-08-02 08:20 +0200
  [PATCH v3 06/11] net: stmmac: dwmac-rk: Add internal phy support for rk3228 David Wu <david.wu@rock-chips.com> - 2017-08-02 08:20 +0200
  [PATCH v3 10/11] ARM64: dts: rockchip: Add gmac2phy node support for rk3328 David Wu <david.wu@rock-chips.com> - 2017-08-02 08:30 +0200
    Re: [PATCH v3 10/11] ARM64: dts: rockchip: Add gmac2phy node support  for rk3328 Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> - 2017-08-02 13:50 +0200
    Re: [PATCH v3 10/11] ARM64: dts: rockchip: Add gmac2phy node support  for rk3328 Florian Fainelli <f.fainelli@gmail.com> - 2017-08-02 19:50 +0200
  [PATCH v3 11/11] ARM64: dts: rockchip: Enable gmac2phy for rk3328-evb David Wu <david.wu@rock-chips.com> - 2017-08-02 08:30 +0200

csiph-web