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 05/11] net: stmmac: dwmac-rk: Add internal phy support

[Heiko Stuebner]

Hi David,

Am Dienstag, 27. Juni 2017, 22:33:20 CEST schrieb David.Wu:
> 在 2017/6/24 1:19, Heiko Stuebner 写道:
> > 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_CON00xb00
> >> +#define RK_GRF_MACPHY_CON10xb04
> >> +#define RK_GRF_MACPHY_CON20xb08
> >> +#define RK_GRF_MACPHY_CON30xb0c
> >> +
> >> +#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?

I just looked at both the rk3228 and rk3328 GRFs and really this seems
to be the first time I see GRF-parts that are similar :-) .

There is no need to duplicate code unnecessarily, if the registers really
are the same for both. So I guess, just prefix everything with a rk3228_*
and add a comment that the rk3328 uses the same GRF layout.

That way future socs, can then add their (likely) changed 

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

[Andrew Lunn]

> 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.

phy-mode is about the bus between the MAC and the PHY. Internal means
there is not a standard bus between the MAC and the PHY, something
proprietary is being used to embed the PHY in the MAC.

If you are using RMII or RGMII, then it is not internal, in that as
standard bus is being used. It does not matter if that bus is not
available external to the SoC, it still exists.

  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);

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

[Andrew Lunn]

On Fri, Jun 23, 2017 at 12:59:07PM +0800, 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

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?

 Andrew

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

[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.

>   - 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_DISABLEGRF_CLR_BIT(0)
> +#define RK_MACPHY_CFG_CLK_50MGRF_BIT(14)
> +#define RK_GMAC2PHY_RMII_MODE(GRF_BIT(6) | GRF_CLR_BIT(7))
> +#define RK_GRF_CON2_MACPHY_IDHIWORD_UPDATE(0x1234, 0x, 0)
> +#define RK_GRF_CON3_MACPHY_IDHIWORD_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.


> +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);
> + usleep_range(10, 20);
> + if (priv->macphy_reset)
> + reset_control_deassert(priv->macphy_reset);
> + usleep_range(10, 20);
> + 

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

[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.

>  
>  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_DISABLEGRF_CLR_BIT(0)
> +#define RK_MACPHY_CFG_CLK_50MGRF_BIT(14)
> +#define RK_GMAC2PHY_RMII_MODE(GRF_BIT(6) | GRF_CLR_BIT(7))
> +#define RK_GRF_CON2_MACPHY_IDHIWORD_UPDATE(0x1234, 0x, 0)
> +#define RK_GRF_CON3_MACPHY_IDHIWORD_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);
> + if (priv->macphy_reset)
> + reset_control_assert(priv->macphy_reset);
> +}
> +
>  static int gmac_clk_init(struct rk_priv_data *bsp_priv)
>  {
>   struct 

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

[David Wu]

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.
 
 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);
+   if (priv->macphy_reset)
+   reset_control_assert(priv->macphy_reset);
+}
+
 static int gmac_clk_init(struct rk_priv_data *bsp_priv)
 {
struct device *dev = _priv->pdev->dev;
@@ -803,6 +850,14 @@ static int gmac_clk_init(struct rk_priv_data *bsp_priv)
clk_set_rate(bsp_priv->clk_mac, 5000);
}
 
+   if (bsp_priv->internal_phy) {
+   bsp_priv->clk_macphy = devm_clk_get(dev, "clk_macphy");
+   if (IS_ERR(bsp_priv->clk_macphy))
+   dev_err(dev, "cannot get %s clock\n", "clk_macphy");
+   else
+