Re: [PATCH v4 05/12] Documentation: net: phy: Add phy-is-internal binding

[David.Wu]

Hi Andrew, Florian

在 2017/8/10 8:20, Andrew Lunn 写道:

Hi Florian, David.

I'm happy with the property name. But i think the text needs more
description. We deal with Ethernet switches with integrated PHYs. Yet
for us, this property is unneeded.

Seeing this property means some bit of software needs to ensure the
internal PHY should be used, when given the choice between an internal
and external PHY. So i would say something like:

 If set, indicates that the PHY is integrated into the same
 physical package as the Ethernet MAC. If needed, muxers should be
 configured to ensure the (internal) PHY is used. The absence of this
 property indicates the muxers should be configured so that the
 external PHY is used.


Are we supposed to replace the words "internal" with "integrated" here?
So we have three kinds of PHY, they are intenal, external and integrated 
PHYs.




This last part is important. If the bootloader has set the internal
PHY to be used, you need to reset it. Otherwise we are going to get
into a mess sometime later and need to add a phy-is-external property.

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

[David.Wu]

Hi Chen-Yu,

在 2017/8/10 10:40, Chen-Yu Tsai 写道:

Hi David,

On Wed, Aug 9, 2017 at 5:38 PM, David.Wu <david...@rock-chips.com> wrote:

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.faine...@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...@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 <_pins> or <_pins>.
+ - pinctrl-0: pin-control mode. can be <_pins>, <_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: < 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.


So it looks like you have the clock for the internal/integrated PHY at the
MAC level. I think this lets you define/add the PHY at the board level more
easily without a lot of duplication?

Sunxi has the clock and reset in the PHY node, which is defined in the dtsi
file. (This part is already done.)


Okay, that is better difined in the PHY node, i'll move them later.



ChenYu

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

[David.Wu]

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  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 
---
  .../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 <_pins> or <_pins>.
+ - pinctrl-0: pin-control mode. can be <_pins>, <_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: < 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

Re: [PATCH v3 01/11] net: phy: Add rockchip phy driver support

[David.Wu]

Hi Andrew,

在 2017/8/2 21:21, Andrew Lunn 写道:

+static struct phy_driver rockchip_phy_driver[] = {
+{
+   .phy_id = 0x1234d400,
+   .phy_id_mask= 0xfff0,
+   .name   = "Rockchip internal EPHY",
+   .features   = (PHY_BASIC_FEATURES | SUPPORTED_Pause
+  | SUPPORTED_Asym_Pause),


Please take a look at Documentation/networking/phy.txt and
Fixes: 529ed1275263 ("net: phy: phy drivers should not set 
SUPPORTED_[Asym_]Pause")

Pause frames / flow control

  The PHY does not participate directly in flow control/pause frames except by
  making sure that the SUPPORTED_Pause and SUPPORTED_AsymPause bits are set in
  MII_ADVERTISE to indicate towards the link partner that the Ethernet MAC
  controller supports such a thing. Since flow control/pause frames generation
  involves the Ethernet MAC driver, it is recommended that this driver takes 
care
  of properly indicating advertisement and support for such features by setting
  the SUPPORTED_Pause and SUPPORTED_AsymPause bits accordingly. This can be done
  either before or after phy_connect() and/or as a result of implementing the
  ethtool::set_pauseparam feature.



Thanks for the reminder, I'll remove it.


Andrew

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

[David.Wu]

Hi Florian & ChenYu

在 2017/8/3 1:38, Florian Fainelli 写道:

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.


We may implement a read_bool_property after parsed phy phandle at 
stmmac_platform.c, which would make MAC driver know it is a internal phy.

Re: [PATCH v2 01/11] net: phy: Add rockchip phy driver support

[David.Wu]

Hi Florian,

在 2017/7/28 0:51, Florian Fainelli 写道:

+}
+
+static int rockchip_internal_phy_config_init(struct phy_device *phydev)
+{
+   int val;
+
+   /*
+* The auto MIDX has linked problem on some board,
+* workround to disable auto MDIX.
+*/

If this a board-specific problem you may consider register a PHY fixup
for the affected boards, or introduce a specific property to illustrate
that MDI-X is broken.


You mean is that if I need disable Auto-mdix, then I'm going to add a 
property to DTS, from here to decide whether or not to close Auto-mdix?

Re: [PATCH v2 01/11] net: phy: Add rockchip phy driver support

[David.Wu]

Hi Andrew,

在 2017/7/27 21:38, Andrew Lunn 写道:

+   /*
+* If mode switch happens from 10BT to 100BT, all DSP/AFE
+* registers are set to default values. So any AFE/DSP
+* registers have to be re-initialized in this case.
+*/


Hi David

Are they also lost on suspend and resume?


Yes, i will add the re-initialized on suspend and resume，and thank you！



 Andrew

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

[David.Wu]

Hi Florian,

在 2017/7/28 0:54, Florian Fainelli 写道:

- if you need knowledge about this PHY connection type prior to binding
the PHY device and its driver (that is, before of_phy_connect()) we
could add a boolean property e.g: "phy-is-internal" that allows us to
know that, or we can have a new phy-mode value, e.g: "internal-rmii"
which describes that, either way would probably be fine, but the former
scales better



Using "phy-is-internal" is very helpful, but it is easy to confuse with 
the real internal PHY, may we use the other words like phy is inlined.



Then again, using phy-mode = "internal" even though this is Reduced MII
is not big of a deal IMHO as long as there is no loss of information and
that internal de-facto means internal reduced MII for instance.
--

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

[David.Wu]

Hi Andrew,

在 2017/7/27 21:48, Andrew Lunn 写道:

I think we need to discuss this. This PHY appears to be on an MDIO
bus, it uses a standard PHY driver, and it appears to be using an RMII
interface. So it is just an ordinary PHY.

Internal is supposed to be something which is not ordinary, does not
use one of the standard phy modes, needs something special to make it
work.


Yes, it is a ordinary PHY in fact, using MDIO bus, but it is a internal 
phy inside Soc, so the "internal" is not the internal as Florain said.

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

[David.Wu]

Hi Andrew,

在 2017/6/24 10:29, Andrew Lunn 写道:

If this is the PHY clock, should it actually be specified in the PHY
binding? Can you read the PHY ID registers with this clock off?

If the phy clock is closed, we can not read the PHYID.

Re: [PATCH 01/11] net: phy: Add rockchip phy driver support

[David.Wu]

Hi Andrew,

在 2017/6/27 22:46, Andrew Lunn 写道:

it has been licensed from somebody.


And does that somebody already have a driver for it? There is no point
adding a driver, if all you need to do is add the ID to another
driver.



I didn't find it.
Maybe use the same, but the configuration is different.
But this may also be possible, upstream with a new driver.


Andrew

Re: [PATCH 01/11] net: phy: Add rockchip phy driver support

[David.Wu]

Hi Andrew,

在 2017/6/24 10:19, Andrew Lunn 写道:

On Fri, Jun 23, 2017 at 12:41:59PM +0800, David Wu wrote:

Support internal ephy currently.

Signed-off-by: David Wu 
---
  drivers/net/phy/Kconfig|  4 ++
  drivers/net/phy/Makefile   |  1 +
  drivers/net/phy/rockchip.c | 94 ++
  3 files changed, 99 insertions(+)
  create mode 100644 drivers/net/phy/rockchip.c

diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
index c360dd6..86010d4 100644
--- a/drivers/net/phy/Kconfig
+++ b/drivers/net/phy/Kconfig
@@ -350,6 +350,10 @@ config XILINX_GMII2RGMII
   the Reduced Gigabit Media Independent Interface(RGMII) between
   Ethernet physical media devices and the Gigabit Ethernet controller.
  
+config ROCKCHIP_MAC_PHY


This is a bit of an odd name, having both MAC and PHY in it. Are there
any other RockChip PHYs? Any external PHYS? Are they register
incompatible with the internal PHY?  Is it even RockChip IP? Or has it
been licensed from somebody else?



Maybe I just want to highlight it applies to the PHY with Mac 
connection, actually I was named Rockchip at the beginning, as Heiko 
said, PHY is a wide range, so add a modifier to restrict it.


Yes, we use other external phys, like realtek and etc...

If we have any other phy in our socs, it also could be expanded at 
rockchip_phy_tbl{} at rockchip.c


it has been licensed from somebody.


I would more likely just call it ROCKCHIP_PHY.

   Andrew

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

[David.Wu]

Hi Heiko,

在 2017/6/24 1:19, Heiko Stuebner 写道:

Hi David,

Am Freitag, 23. Juni 2017, 12:59:07 CEST schrieb David Wu:

To make internal phy worked, need to configure the phy_clock,
phy cru_reset and related registers.

Change-Id: I6971c0a769754b824b1b908b56080cbaf7867d13


please remove all Change-Ids from patches before sending upstream.
There were more affected patches in this series.


Signed-off-by: David Wu 
---
  .../devicetree/bindings/net/rockchip-dwmac.txt |  3 +
  drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c | 82 ++
  2 files changed, 85 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/rockchip-dwmac.txt 
b/Documentation/devicetree/bindings/net/rockchip-dwmac.txt
index 8f42755..0514f69 100644
--- a/Documentation/devicetree/bindings/net/rockchip-dwmac.txt
+++ b/Documentation/devicetree/bindings/net/rockchip-dwmac.txt
@@ -22,6 +22,7 @@ Required properties:
   < SCLK_MACREF_OUT> clock gate for RMII reference clock output
   < ACLK_GMAC>: AXI clock gate for GMAC
   < PCLK_GMAC>: APB clock gate for GMAC
+  < MAC_PHY>: clock for internal macphy


that clock should not be listed as always "Required" like it is here.
Make it some sort of extra paragraph marking it as required when using
an internal phy.



Okay, move it to the option.


   - 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.
@@ -35,6 +36,8 @@ Required properties:
   - assigned-clocks: main clock, should be < SCLK_MAC>;
   - assigned-clock-parents = parent of main clock.
 can be <_gmac> or < SCLK_MAC_PLL>.
+ - phy-type: For internal phy, it must be "internal"; For external phy, no need
+   to configure this.
  
  Optional properties:

   - tx_delay: Delay value for TXD timing. Range value is 0~0x7F, 0x30 as 
default.
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c 
b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
index a8e8fd5..c1a1413 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
@@ -41,6 +41,7 @@ struct rk_gmac_ops {
void (*set_to_rmii)(struct rk_priv_data *bsp_priv);
void (*set_rgmii_speed)(struct rk_priv_data *bsp_priv, int speed);
void (*set_rmii_speed)(struct rk_priv_data *bsp_priv, int speed);
+   void (*internal_phy_powerup)(struct rk_priv_data *bsp_priv);
  };
  
  struct rk_priv_data {

@@ -52,6 +53,7 @@ struct rk_priv_data {
  
  	bool clk_enabled;

bool clock_input;
+   bool internal_phy;
  
  	struct clk *clk_mac;

struct clk *gmac_clkin;
@@ -61,6 +63,9 @@ struct rk_priv_data {
struct clk *clk_mac_refout;
struct clk *aclk_mac;
struct clk *pclk_mac;
+   struct clk *clk_macphy;
+
+   struct reset_control *macphy_reset;
  
  	int tx_delay;

int rx_delay;
@@ -750,6 +755,48 @@ static void rk3399_set_rmii_speed(struct rk_priv_data 
*bsp_priv, int speed)
.set_rmii_speed = rk3399_set_rmii_speed,
  };
  
+#define RK_GRF_MACPHY_CON0		0xb00

+#define RK_GRF_MACPHY_CON1 0xb04
+#define RK_GRF_MACPHY_CON2 0xb08
+#define RK_GRF_MACPHY_CON3 0xb0c
+
+#define RK_MACPHY_ENABLE   GRF_BIT(0)
+#define RK_MACPHY_DISABLE  GRF_CLR_BIT(0)
+#define RK_MACPHY_CFG_CLK_50M  GRF_BIT(14)
+#define RK_GMAC2PHY_RMII_MODE  (GRF_BIT(6) | GRF_CLR_BIT(7))
+#define RK_GRF_CON2_MACPHY_ID  HIWORD_UPDATE(0x1234, 0x, 0)
+#define RK_GRF_CON3_MACPHY_ID  HIWORD_UPDATE(0x35, 0x3f, 0)


These are primarily registers for the rk3328 and come from the GRF which is
somehow prone to chip-designers moving bits around in registers and also
especially the register offsets (*_CONx) will probably not stay the same
on future socs.



I think they should try to keep the same. But what you said is very 
reasonable. So let's give rk3228 and rk3328 different 
internal_phy_powerup() in the rk_gmac_ops to set their own configuration?





+static void rk_gmac_internal_phy_powerup(struct rk_priv_data *priv)
+{
+   if (priv->ops->internal_phy_powerup)
+   priv->ops->internal_phy_powerup(priv);
+
+   regmap_write(priv->grf, RK_GRF_MACPHY_CON0, RK_MACPHY_CFG_CLK_50M);
+   regmap_write(priv->grf, RK_GRF_MACPHY_CON0, RK_GMAC2PHY_RMII_MODE);
+
+   regmap_write(priv->grf, RK_GRF_MACPHY_CON2, RK_GRF_CON2_MACPHY_ID);
+   regmap_write(priv->grf, RK_GRF_MACPHY_CON3, RK_GRF_CON3_MACPHY_ID);
+
+   /* disable macphy, the default value is enabled */


that comment is not providing useful information, maybe
/* macphy needs to be disabled before trying to reset it */



+   regmap_write(priv->grf, RK_GRF_MACPHY_CON0, RK_MACPHY_DISABLE);
+   if (priv->macphy_reset)
+   reset_control_assert(priv->macphy_reset);
+   

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

[David.Wu]

Hi Florian,

Sorry for reply late.

在 2017/6/24 0:22, Florian Fainelli 写道:

On 06/22/2017 09:59 PM, David Wu wrote:

To make internal phy worked, need to configure the phy_clock,
phy cru_reset and related registers.

Change-Id: I6971c0a769754b824b1b908b56080cbaf7867d13
Signed-off-by: David Wu 
---
  .../devicetree/bindings/net/rockchip-dwmac.txt |  3 +
  drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c | 82 ++
  2 files changed, 85 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/rockchip-dwmac.txt 
b/Documentation/devicetree/bindings/net/rockchip-dwmac.txt
index 8f42755..0514f69 100644
--- a/Documentation/devicetree/bindings/net/rockchip-dwmac.txt
+++ b/Documentation/devicetree/bindings/net/rockchip-dwmac.txt
@@ -22,6 +22,7 @@ Required properties:
   < SCLK_MACREF_OUT> clock gate for RMII reference clock output
   < ACLK_GMAC>: AXI clock gate for GMAC
   < PCLK_GMAC>: APB clock gate for GMAC
+  < MAC_PHY>: clock for internal macphy
   - 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.
@@ -35,6 +36,8 @@ Required properties:
   - assigned-clocks: main clock, should be < SCLK_MAC>;
   - assigned-clock-parents = parent of main clock.
 can be <_gmac> or < SCLK_MAC_PLL>.
+ - phy-type: For internal phy, it must be "internal"; For external phy, no need
+   to configure this.


Use the standard "phy-mode" property. You will see
drivers/net/ethernet/broadcom/genet/ actually define a phy-mode =
"internal" property specifically for that. This should probably be
generalized so it is useful to other drivers a well, I will do just that.



I'm a little confused for the property of phy-mode = "internal".
If the property of phy-mode is configured as "internal" from DT , i 
could not get the rmii or rgmii mode for the phy.

I use it to differentiate rmii or rgmii for different configuration.

  
  Optional properties:

   - tx_delay: Delay value for TXD timing. Range value is 0~0x7F, 0x30 as 
default.
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c 
b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
index a8e8fd5..c1a1413 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
@@ -41,6 +41,7 @@ struct rk_gmac_ops {
void (*set_to_rmii)(struct rk_priv_data *bsp_priv);
void (*set_rgmii_speed)(struct rk_priv_data *bsp_priv, int speed);
void (*set_rmii_speed)(struct rk_priv_data *bsp_priv, int speed);
+   void (*internal_phy_powerup)(struct rk_priv_data *bsp_priv);
  };
  
  struct rk_priv_data {

@@ -52,6 +53,7 @@ struct rk_priv_data {
  
  	bool clk_enabled;

bool clock_input;
+   bool internal_phy;
  
  	struct clk *clk_mac;

struct clk *gmac_clkin;
@@ -61,6 +63,9 @@ struct rk_priv_data {
struct clk *clk_mac_refout;
struct clk *aclk_mac;
struct clk *pclk_mac;
+   struct clk *clk_macphy;
+
+   struct reset_control *macphy_reset;
  
  	int tx_delay;

int rx_delay;
@@ -750,6 +755,48 @@ static void rk3399_set_rmii_speed(struct rk_priv_data 
*bsp_priv, int speed)
.set_rmii_speed = rk3399_set_rmii_speed,
  };
  
+#define RK_GRF_MACPHY_CON0		0xb00

+#define RK_GRF_MACPHY_CON1 0xb04
+#define RK_GRF_MACPHY_CON2 0xb08
+#define RK_GRF_MACPHY_CON3 0xb0c
+
+#define RK_MACPHY_ENABLE   GRF_BIT(0)
+#define RK_MACPHY_DISABLE  GRF_CLR_BIT(0)
+#define RK_MACPHY_CFG_CLK_50M  GRF_BIT(14)
+#define RK_GMAC2PHY_RMII_MODE  (GRF_BIT(6) | GRF_CLR_BIT(7))
+#define RK_GRF_CON2_MACPHY_ID  HIWORD_UPDATE(0x1234, 0x, 0)
+#define RK_GRF_CON3_MACPHY_ID  HIWORD_UPDATE(0x35, 0x3f, 0)
+
+static void rk_gmac_internal_phy_powerup(struct rk_priv_data *priv)
+{
+   if (priv->ops->internal_phy_powerup)
+   priv->ops->internal_phy_powerup(priv);
+
+   regmap_write(priv->grf, RK_GRF_MACPHY_CON0, RK_MACPHY_CFG_CLK_50M);
+   regmap_write(priv->grf, RK_GRF_MACPHY_CON0, RK_GMAC2PHY_RMII_MODE);
+
+   regmap_write(priv->grf, RK_GRF_MACPHY_CON2, RK_GRF_CON2_MACPHY_ID);
+   regmap_write(priv->grf, RK_GRF_MACPHY_CON3, RK_GRF_CON3_MACPHY_ID);
+
+   /* disable macphy, the default value is enabled */
+   regmap_write(priv->grf, RK_GRF_MACPHY_CON0, RK_MACPHY_DISABLE);
+   if (priv->macphy_reset)
+   reset_control_assert(priv->macphy_reset);
+   usleep_range(10, 20);
+   if (priv->macphy_reset)
+   reset_control_deassert(priv->macphy_reset);
+   usleep_range(10, 20);
+   regmap_write(priv->grf, RK_GRF_MACPHY_CON0, RK_MACPHY_ENABLE);
+   msleep(30);
+}
+
+static void rk_gmac_internal_phy_powerdown(struct rk_priv_data *priv)
+{
+   regmap_write(priv->grf, RK_GRF_MACPHY_CON0, RK_MACPHY_DISABLE);