[PATCH net-next v2 0/3] add support for RGMII on GMAC0 through TRGMII hardware module

[PATCH net-next v2 0/3] add support for RGMII on GMAC0 through TRGMII hardware module

[sean.wang]

From: Sean Wang <sean.wang@mediatek.com>

By default, GMAC0 is connected to built-in switch called
MT7530 through the proprietary interface called Turbo RGMII
(TRGMII). TRGMII also supports well for RGMII as generic external
PHY uses but requires some slight changes to the setup of TRGMII 
and doesn't have well support on current driver.

So this patchset
1) provides the slight changes of the setup for RGMII can work
   through TRGMII
2) adds additional setting "trgmii" as PHY_INTERFACE_MODE_TRGMII 
   about phy-mode on device tree to make GMAC0 distinguish which
   mode it runs
3) changes dynamically source clock, TX/RX delay and interface
   mode on TRGMII for adapting various link

Changes since v1:
- fixed the style of comment which doesn't have a space at 
   the beginning and end of comment lines
- add support for phy-mode "trgmii" as PHY_INTERFACE_MODE_TRGMII 
   into linux/phy.h
- enhance the Documentation about device tree binding for trgmii
  which is applicable only for GMAC0 which uses fixed-link

Sean Wang (3):
  net: ethernet: mediatek: add extension of phy-mode for TRGMII
  net: ethernet: mediatek: add support for GMAC0 connecting with
    external PHY through TRGMII
  net: ethernet: mediatek: add the dts property to set if TRGMII
    supported on GMAC0

 .../devicetree/bindings/net/mediatek-net.txt       |  5 +++-
 drivers/net/ethernet/mediatek/mtk_eth_soc.c        | 34 +++++++++++++++++++++-
 drivers/net/ethernet/mediatek/mtk_eth_soc.h        | 34 +++++++++++++++++++++-
 include/linux/phy.h                                |  3 ++
 4 files changed, 73 insertions(+), 3 deletions(-)

-- 
1.9.1

[PATCH net-next v2 1/3] net: ethernet: mediatek: add extension of phy-mode for TRGMII

[sean.wang]

From: Sean Wang <sean.wang@mediatek.com>

adds PHY-mode "trgmii" as an extension for the operation
mode of the PHY interface for PHY_INTERFACE_MODE_TRGMII.
and adds a variable trgmii inside mtk_mac as the indication
to make the difference between the MAC connected to internal
switch or connected to external PHY by the given configuration
on the board and then to perform the corresponding setup on
TRGMII hardware module.

Signed-off-by: Sean Wang <sean.wang@mediatek.com>
Cc: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/net/ethernet/mediatek/mtk_eth_soc.c | 2 ++
 drivers/net/ethernet/mediatek/mtk_eth_soc.h | 3 +++
 include/linux/phy.h                         | 3 +++
 3 files changed, 8 insertions(+)

diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
index ca6b501..827f4bd 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
@@ -244,6 +244,8 @@ static int mtk_phy_connect(struct mtk_mac *mac)
 		return -ENODEV;
 
 	switch (of_get_phy_mode(np)) {
+	case PHY_INTERFACE_MODE_TRGMII:
+		mac->trgmii = true;
 	case PHY_INTERFACE_MODE_RGMII_TXID:
 	case PHY_INTERFACE_MODE_RGMII_RXID:
 	case PHY_INTERFACE_MODE_RGMII_ID:
diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.h b/drivers/net/ethernet/mediatek/mtk_eth_soc.h
index 7c5e534..e3b9525 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.h
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.h
@@ -529,6 +529,8 @@ struct mtk_eth {
  * @hw:			Backpointer to our main datastruture
  * @hw_stats:		Packet statistics counter
  * @phy_dev:		The attached PHY if available
+ * @trgmii		Indicate if the MAC uses TRGMII connected to internal
+			switch
  */
 struct mtk_mac {
 	int				id;
@@ -539,6 +541,7 @@ struct mtk_mac {
 	struct phy_device		*phy_dev;
 	__be32				hwlro_ip[MTK_MAX_LRO_IP_CNT];
 	int				hwlro_ip_cnt;
+	bool				trgmii;
 };
 
 /* the struct describing the SoC. these are declared in the soc_xyz.c files */
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 2d24b28..e25f183 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -80,6 +80,7 @@ typedef enum {
 	PHY_INTERFACE_MODE_XGMII,
 	PHY_INTERFACE_MODE_MOCA,
 	PHY_INTERFACE_MODE_QSGMII,
+	PHY_INTERFACE_MODE_TRGMII,
 	PHY_INTERFACE_MODE_MAX,
 } phy_interface_t;
 
@@ -123,6 +124,8 @@ static inline const char *phy_modes(phy_interface_t interface)
 		return "moca";
 	case PHY_INTERFACE_MODE_QSGMII:
 		return "qsgmii";
+	case PHY_INTERFACE_MODE_TRGMII:
+		return "trgmii";
 	default:
 		return "unknown";
 	}
-- 
1.9.1

[PATCH net-next v2 2/3] net: ethernet: mediatek: add support for GMAC0 connecting with external PHY through TRGMII

[sean.wang]

From: Sean Wang <sean.wang@mediatek.com>

Changing dynamically source clock, TX/RX delay and interface mode
used by TRGMII hardware module inside PHY capability polling routine
for adapting to the various speed of RGMII used by external PHY for
GMAC0.

Signed-off-by: Sean Wang <sean.wang@mediatek.com>
---
 drivers/net/ethernet/mediatek/mtk_eth_soc.c | 32 ++++++++++++++++++++++++++++-
 drivers/net/ethernet/mediatek/mtk_eth_soc.h | 31 +++++++++++++++++++++++++++-
 2 files changed, 61 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
index 827f4bd..73c7904 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
@@ -52,7 +52,7 @@ static const struct mtk_ethtool_stats {
 };
 
 static const char * const mtk_clks_source_name[] = {
-	"ethif", "esw", "gp1", "gp2"
+	"ethif", "esw", "gp1", "gp2", "trgpll"
 };
 
 void mtk_w32(struct mtk_eth *eth, u32 val, unsigned reg)
@@ -135,6 +135,33 @@ static int mtk_mdio_read(struct mii_bus *bus, int phy_addr, int phy_reg)
 	return _mtk_mdio_read(eth, phy_addr, phy_reg);
 }
 
+static void mtk_gmac0_rgmii_adjust(struct mtk_eth *eth, int speed)
+{
+	u32 val;
+	int ret;
+
+	val = (speed == SPEED_1000) ?
+		INTF_MODE_RGMII_1000 : INTF_MODE_RGMII_10_100;
+	mtk_w32(eth, val, INTF_MODE);
+
+	regmap_update_bits(eth->ethsys, ETHSYS_CLKCFG0,
+			   ETHSYS_TRGMII_CLK_SEL362_5,
+			   ETHSYS_TRGMII_CLK_SEL362_5);
+
+	val = (speed == SPEED_1000) ? 250000000 : 500000000;
+	ret = clk_set_rate(eth->clks[MTK_CLK_TRGPLL], val);
+	if (ret)
+		dev_err(eth->dev, "Failed to set trgmii pll: %d\n", ret);
+
+	val = (speed == SPEED_1000) ?
+		RCK_CTRL_RGMII_1000 : RCK_CTRL_RGMII_10_100;
+	mtk_w32(eth, val, TRGMII_RCK_CTRL);
+
+	val = (speed == SPEED_1000) ?
+		TCK_CTRL_RGMII_1000 : TCK_CTRL_RGMII_10_100;
+	mtk_w32(eth, val, TRGMII_TCK_CTRL);
+}
+
 static void mtk_phy_link_adjust(struct net_device *dev)
 {
 	struct mtk_mac *mac = netdev_priv(dev);
@@ -157,6 +184,9 @@ static void mtk_phy_link_adjust(struct net_device *dev)
 		break;
 	};
 
+	if (mac->id == 0 && !mac->trgmii)
+		mtk_gmac0_rgmii_adjust(mac->hw, mac->phy_dev->speed);
+
 	if (mac->phy_dev->link)
 		mcr |= MAC_MCR_FORCE_LINK;
 
diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.h b/drivers/net/ethernet/mediatek/mtk_eth_soc.h
index e3b9525..e521156 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.h
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.h
@@ -313,6 +313,30 @@
 				 MAC_MCR_FORCE_TX_FC | MAC_MCR_SPEED_1000 | \
 				 MAC_MCR_FORCE_DPX | MAC_MCR_FORCE_LINK)
 
+/* TRGMII RXC control register */
+#define TRGMII_RCK_CTRL		0x10300
+#define DQSI0(x)		((x << 0) & GENMASK(6, 0))
+#define DQSI1(x)		((x << 8) & GENMASK(14, 8))
+#define RXCTL_DMWTLAT(x)	((x << 16) & GENMASK(18, 16))
+#define RXC_DQSISEL		BIT(30)
+#define RCK_CTRL_RGMII_1000	(RXC_DQSISEL | RXCTL_DMWTLAT(2) | DQSI1(16))
+#define RCK_CTRL_RGMII_10_100	RXCTL_DMWTLAT(2)
+
+/* TRGMII RXC control register */
+#define TRGMII_TCK_CTRL		0x10340
+#define TXCTL_DMWTLAT(x)	((x << 16) & GENMASK(18, 16))
+#define TXC_INV			BIT(30)
+#define TCK_CTRL_RGMII_1000	TXCTL_DMWTLAT(2)
+#define TCK_CTRL_RGMII_10_100	(TXC_INV | TXCTL_DMWTLAT(2))
+
+/* TRGMII Interface mode register */
+#define INTF_MODE		0x10390
+#define TRGMII_INTF_DIS		BIT(0)
+#define TRGMII_MODE		BIT(1)
+#define TRGMII_CENTRAL_ALIGNED	BIT(2)
+#define INTF_MODE_RGMII_1000    (TRGMII_MODE | TRGMII_CENTRAL_ALIGNED)
+#define INTF_MODE_RGMII_10_100  0
+
 /* GPIO port control registers for GMAC 2*/
 #define GPIO_OD33_CTRL8		0x4c0
 #define GPIO_BIAS_CTRL		0xed0
@@ -323,7 +347,11 @@
 #define SYSCFG0_GE_MASK		0x3
 #define SYSCFG0_GE_MODE(x, y)	(x << (12 + (y * 2)))
 
-/*ethernet reset control register*/
+/* ethernet subsystem clock register */
+#define ETHSYS_CLKCFG0		0x2c
+#define ETHSYS_TRGMII_CLK_SEL362_5	BIT(11)
+
+/* ethernet reset control register */
 #define ETHSYS_RSTCTRL		0x34
 #define RSTCTRL_FE		BIT(6)
 #define RSTCTRL_PPE		BIT(31)
@@ -389,6 +417,7 @@ enum mtk_clks_map {
 	MTK_CLK_ESW,
 	MTK_CLK_GP1,
 	MTK_CLK_GP2,
+	MTK_CLK_TRGPLL,
 	MTK_CLK_MAX
 };
 
-- 
1.9.1

[PATCH net-next v2 3/3] net: ethernet: mediatek: add the dts property to set if TRGMII supported on GMAC0

[sean.wang]

From: Sean Wang <sean.wang@mediatek.com>

Add the dts property for the capability if TRGMII supported on GAMC0

Signed-off-by: Sean Wang <sean.wang@mediatek.com>
---
 Documentation/devicetree/bindings/net/mediatek-net.txt | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/net/mediatek-net.txt b/Documentation/devicetree/bindings/net/mediatek-net.txt
index 6103e55..7111278 100644
--- a/Documentation/devicetree/bindings/net/mediatek-net.txt
+++ b/Documentation/devicetree/bindings/net/mediatek-net.txt
@@ -31,7 +31,10 @@ Optional properties:
 Required properties:
 - compatible: Should be "mediatek,eth-mac"
 - reg: The number of the MAC
-- phy-handle: see ethernet.txt file in the same directory.
+- phy-handle: see ethernet.txt file in the same directory and
+	the phy-mode "trgmii" required being provided when reg
+	is equal to 0 and the MAC uses fixed-link to connect
+	with inernal switch such as MT7530.
 
 Example:
 
-- 
1.9.1

Re: [PATCH net-next v2 0/3] add support for RGMII on GMAC0 through TRGMII hardware module

[Florian Fainelli]

Le 21/09/2016 à 19:33, sean.wang@mediatek.com a écrit :
> From: Sean Wang <sean.wang@mediatek.com>
> 
> By default, GMAC0 is connected to built-in switch called
> MT7530 through the proprietary interface called Turbo RGMII
> (TRGMII). TRGMII also supports well for RGMII as generic external
> PHY uses but requires some slight changes to the setup of TRGMII 
> and doesn't have well support on current driver.
> 
> So this patchset
> 1) provides the slight changes of the setup for RGMII can work
>    through TRGMII
> 2) adds additional setting "trgmii" as PHY_INTERFACE_MODE_TRGMII 
>    about phy-mode on device tree to make GMAC0 distinguish which
>    mode it runs
> 3) changes dynamically source clock, TX/RX delay and interface
>    mode on TRGMII for adapting various link
> 
> Changes since v1:
> - fixed the style of comment which doesn't have a space at 
>    the beginning and end of comment lines
> - add support for phy-mode "trgmii" as PHY_INTERFACE_MODE_TRGMII 
>    into linux/phy.h
> - enhance the Documentation about device tree binding for trgmii
>   which is applicable only for GMAC0 which uses fixed-link

Looks good to me:

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>

Thanks Sean!
-- 
Florian

Re: [PATCH net-next v2 3/3] net: ethernet: mediatek: add the dts property to set if TRGMII supported on GMAC0

[Sergei Shtylyov]

Hello.

On 9/22/2016 5:33 AM, sean.wang@mediatek.com wrote:

> From: Sean Wang <sean.wang@mediatek.com>
>
> Add the dts property for the capability if TRGMII supported on GAMC0
>
> Signed-off-by: Sean Wang <sean.wang@mediatek.com>
> ---
>  Documentation/devicetree/bindings/net/mediatek-net.txt | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/net/mediatek-net.txt b/Documentation/devicetree/bindings/net/mediatek-net.txt
> index 6103e55..7111278 100644
> --- a/Documentation/devicetree/bindings/net/mediatek-net.txt
> +++ b/Documentation/devicetree/bindings/net/mediatek-net.txt
> @@ -31,7 +31,10 @@ Optional properties:
>  Required properties:
>  - compatible: Should be "mediatek,eth-mac"
>  - reg: The number of the MAC
> -- phy-handle: see ethernet.txt file in the same directory.
> +- phy-handle: see ethernet.txt file in the same directory and
> +	the phy-mode "trgmii" required being provided when reg

   Since you've modified the generic parser of the "phy-mode" to add your 
"trgmii", you also need to update ethernet.txt...

> +	is equal to 0 and the MAC uses fixed-link to connect
> +	with inernal switch such as MT7530.

   Internal.

[...]

MBR, Sergei

Re: [PATCH net-next v2 1/3] net: ethernet: mediatek: add extension of phy-mode for TRGMII

[Sergei Shtylyov]

Hello.

On 9/22/2016 5:33 AM, sean.wang@mediatek.com wrote:

> From: Sean Wang <sean.wang@mediatek.com>
>
> adds PHY-mode "trgmii" as an extension for the operation
> mode of the PHY interface for PHY_INTERFACE_MODE_TRGMII.
> and adds a variable trgmii inside mtk_mac as the indication
> to make the difference between the MAC connected to internal
> switch or connected to external PHY by the given configuration
> on the board and then to perform the corresponding setup on
> TRGMII hardware module.
>
> Signed-off-by: Sean Wang <sean.wang@mediatek.com>
> Cc: Florian Fainelli <f.fainelli@gmail.com>
> ---
>  drivers/net/ethernet/mediatek/mtk_eth_soc.c | 2 ++
>  drivers/net/ethernet/mediatek/mtk_eth_soc.h | 3 +++
>  include/linux/phy.h                         | 3 +++
>  3 files changed, 8 insertions(+)
>
> diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> index ca6b501..827f4bd 100644
> --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> @@ -244,6 +244,8 @@ static int mtk_phy_connect(struct mtk_mac *mac)
>  		return -ENODEV;
>
>  	switch (of_get_phy_mode(np)) {
> +	case PHY_INTERFACE_MODE_TRGMII:
> +		mac->trgmii = true;
>  	case PHY_INTERFACE_MODE_RGMII_TXID:
>  	case PHY_INTERFACE_MODE_RGMII_RXID:
>  	case PHY_INTERFACE_MODE_RGMII_ID:
> diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.h b/drivers/net/ethernet/mediatek/mtk_eth_soc.h
> index 7c5e534..e3b9525 100644
> --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.h
> +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.h
> @@ -529,6 +529,8 @@ struct mtk_eth {
>   * @hw:			Backpointer to our main datastruture
>   * @hw_stats:		Packet statistics counter
>   * @phy_dev:		The attached PHY if available
> + * @trgmii		Indicate if the MAC uses TRGMII connected to internal
> +			switch
>   */
>  struct mtk_mac {
>  	int				id;
> @@ -539,6 +541,7 @@ struct mtk_mac {
>  	struct phy_device		*phy_dev;
>  	__be32				hwlro_ip[MTK_MAX_LRO_IP_CNT];
>  	int				hwlro_ip_cnt;
> +	bool				trgmii;

     I don't see where this is used.

[...]
> diff --git a/include/linux/phy.h b/include/linux/phy.h
> index 2d24b28..e25f183 100644
> --- a/include/linux/phy.h
> +++ b/include/linux/phy.h
> @@ -80,6 +80,7 @@ typedef enum {
>  	PHY_INTERFACE_MODE_XGMII,
>  	PHY_INTERFACE_MODE_MOCA,
>  	PHY_INTERFACE_MODE_QSGMII,
> +	PHY_INTERFACE_MODE_TRGMII,
>  	PHY_INTERFACE_MODE_MAX,
>  } phy_interface_t;
>
> @@ -123,6 +124,8 @@ static inline const char *phy_modes(phy_interface_t interface)
>  		return "moca";
>  	case PHY_INTERFACE_MODE_QSGMII:
>  		return "qsgmii";
> +	case PHY_INTERFACE_MODE_TRGMII:
> +		return "trgmii";
>  	default:
>  		return "unknown";
>  	}

    I think this should be done in a separate phylib patch.

MBR, Sergei

Re: [PATCH net-next v2 0/3] add support for RGMII on GMAC0 through TRGMII hardware module

[David Miller]

From: <sean.wang@mediatek.com>
Date: Thu, 22 Sep 2016 10:33:53 +0800

> By default, GMAC0 is connected to built-in switch called
> MT7530 through the proprietary interface called Turbo RGMII
> (TRGMII). TRGMII also supports well for RGMII as generic external
> PHY uses but requires some slight changes to the setup of TRGMII 
> and doesn't have well support on current driver.
> 
> So this patchset
> 1) provides the slight changes of the setup for RGMII can work
>    through TRGMII
> 2) adds additional setting "trgmii" as PHY_INTERFACE_MODE_TRGMII 
>    about phy-mode on device tree to make GMAC0 distinguish which
>    mode it runs
> 3) changes dynamically source clock, TX/RX delay and interface
>    mode on TRGMII for adapting various link
> 
> Changes since v1:
> - fixed the style of comment which doesn't have a space at 
>    the beginning and end of comment lines
> - add support for phy-mode "trgmii" as PHY_INTERFACE_MODE_TRGMII 
>    into linux/phy.h
> - enhance the Documentation about device tree binding for trgmii
>   which is applicable only for GMAC0 which uses fixed-link

Series applied.

Re: [PATCH net-next v2 3/3] net: ethernet: mediatek: add the dts property to set if TRGMII supported on GMAC0

[Sean Wang]

Date: Thu, 22 Sep 2016 14:28:36 +0300, Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> wrote:
>Hello.
>
>On 9/22/2016 5:33 AM, sean.wang@mediatek.com wrote:
>
>> From: Sean Wang <sean.wang@mediatek.com>
>>
>> Add the dts property for the capability if TRGMII supported on GAMC0
>>
>> Signed-off-by: Sean Wang <sean.wang@mediatek.com>
>> ---
>>  Documentation/devicetree/bindings/net/mediatek-net.txt | 5 ++++-
>>  1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/devicetree/bindings/net/mediatek-net.txt b/Documentation/devicetree/bindings/net/mediatek-net.txt
>> index 6103e55..7111278 100644
>> --- a/Documentation/devicetree/bindings/net/mediatek-net.txt
>> +++ b/Documentation/devicetree/bindings/net/mediatek-net.txt
>> @@ -31,7 +31,10 @@ Optional properties:
>>  Required properties:
>>  - compatible: Should be "mediatek,eth-mac"
>>  - reg: The number of the MAC
>> -- phy-handle: see ethernet.txt file in the same directory.
>> +- phy-handle: see ethernet.txt file in the same directory and
>> +	the phy-mode "trgmii" required being provided when reg
>
>   Since you've modified the generic parser of the "phy-mode" to add your 
>"trgmii", you also need to update ethernet.txt...
>

>> +	is equal to 0 and the MAC uses fixed-link to connect
>> +	with inernal switch such as MT7530.
>
>   Internal.
>

thank you for taking time and effort to review this
I will fix it up

Re: [PATCH net-next v2 0/3] add support for RGMII on GMAC0 through TRGMII hardware module

[Sergei Shtylyov]

Hello.

On 09/22/2016 03:22 PM, David Miller wrote:

>> By default, GMAC0 is connected to built-in switch called
>> MT7530 through the proprietary interface called Turbo RGMII
>> (TRGMII). TRGMII also supports well for RGMII as generic external
>> PHY uses but requires some slight changes to the setup of TRGMII
>> and doesn't have well support on current driver.
>>
>> So this patchset
>> 1) provides the slight changes of the setup for RGMII can work
>>    through TRGMII
>> 2) adds additional setting "trgmii" as PHY_INTERFACE_MODE_TRGMII
>>    about phy-mode on device tree to make GMAC0 distinguish which
>>    mode it runs
>> 3) changes dynamically source clock, TX/RX delay and interface
>>    mode on TRGMII for adapting various link
>>
>> Changes since v1:
>> - fixed the style of comment which doesn't have a space at
>>    the beginning and end of comment lines
>> - add support for phy-mode "trgmii" as PHY_INTERFACE_MODE_TRGMII
>>    into linux/phy.h
>> - enhance the Documentation about device tree binding for trgmii
>>   which is applicable only for GMAC0 which uses fixed-link
>
> Series applied.

    Despite my comments? Sigh...

MBR, Sergei

Re: [PATCH net-next v2 0/3] add support for RGMII on GMAC0 through TRGMII hardware module

[David Miller]

From: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
Date: Thu, 22 Sep 2016 20:08:47 +0300

>    Despite my comments? Sigh...

Sorry, I thought he had addressed your feedback in v2.

I'll wait longer next time.

Re: [PATCH net-next v2 1/3] net: ethernet: mediatek: add extension of phy-mode for TRGMII

[Sean Wang]

Date: Thu, 22 Sep 2016 14:30:53 +0300, Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> wrote:
>>Hello.
>
>On 9/22/2016 5:33 AM, sean.wang@mediatek.com wrote:
>
>> From: Sean Wang <sean.wang@mediatek.com>
>>
>> adds PHY-mode "trgmii" as an extension for the operation mode of the 
>> PHY interface for PHY_INTERFACE_MODE_TRGMII.

.. deleted

>>  	switch (of_get_phy_mode(np)) {
>> +	case PHY_INTERFACE_MODE_TRGMII:
>> +		mac->trgmii = true;
>>  	case PHY_INTERFACE_MODE_RGMII_TXID:
>>  	case PHY_INTERFACE_MODE_RGMII_RXID:
>>  	case PHY_INTERFACE_MODE_RGMII_ID:
>> diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.h 
>> b/drivers/net/ethernet/mediatek/mtk_eth_soc.h
>> index 7c5e534..e3b9525 100644
>> --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.h
>> +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.h
>> @@ -529,6 +529,8 @@ struct mtk_eth {
>>   * @hw:			Backpointer to our main datastruture
>>   * @hw_stats:		Packet statistics counter
>>   * @phy_dev:		The attached PHY if available
>> + * @trgmii		Indicate if the MAC uses TRGMII connected to internal
>> +			switch
>>   */
>>  struct mtk_mac {
>>  	int				id;
>> @@ -539,6 +541,7 @@ struct mtk_mac {
>>  	struct phy_device		*phy_dev;
>>  	__be32				hwlro_ip[MTK_MAX_LRO_IP_CNT];
>>  	int				hwlro_ip_cnt;
>> +	bool				trgmii;
>
>     I don't see where this is used.

I set trgmii as below
  	switch (of_get_phy_mode(np)) {
	case PHY_INTERFACE_MODE_TRGMII:
		mac->trgmii = true;
  	case PHY_INTERFACE_MODE_RGMII_TXID:

>[...]
>> diff --git a/include/linux/phy.h b/include/linux/phy.h index 
>> 2d24b28..e25f183 100644
>> --- a/include/linux/phy.h
>> +++ b/include/linux/phy.h
>> @@ -80,6 +80,7 @@ typedef enum {
>>  	PHY_INTERFACE_MODE_XGMII,
>>  	PHY_INTERFACE_MODE_MOCA,
>>  	PHY_INTERFACE_MODE_QSGMII,
>> +	PHY_INTERFACE_MODE_TRGMII,
>>  	PHY_INTERFACE_MODE_MAX,
>>  } phy_interface_t;
>>
>> @@ -123,6 +124,8 @@ static inline const char *phy_modes(phy_interface_t interface)
>>  		return "moca";
>>  	case PHY_INTERFACE_MODE_QSGMII:
>>  		return "qsgmii";
>> +	case PHY_INTERFACE_MODE_TRGMII:
>> +		return "trgmii";
>>  	default:
>>  		return "unknown";
>>  	}
>
>    I think this should be done in a separate phylib patch.

this patch is applied, so I am so little confused how to do this.
next time I will note placing modification for generic layer
into separate patch.

>
>MBR, Sergei
>

Re: [PATCH net-next v2 1/3] net: ethernet: mediatek: add extension of phy-mode for TRGMII

[Sergei Shtylyov]

Hello.

On 9/23/2016 6:32 AM, Sean Wang wrote:

>>> From: Sean Wang <sean.wang@mediatek.com>
>>>
>>> adds PHY-mode "trgmii" as an extension for the operation mode of the
>>> PHY interface for PHY_INTERFACE_MODE_TRGMII.
>
> .. deleted

    What? Why?

>>>  	switch (of_get_phy_mode(np)) {
>>> +	case PHY_INTERFACE_MODE_TRGMII:
>>> +		mac->trgmii = true;
>>>  	case PHY_INTERFACE_MODE_RGMII_TXID:
>>>  	case PHY_INTERFACE_MODE_RGMII_RXID:
>>>  	case PHY_INTERFACE_MODE_RGMII_ID:
>>> diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.h
>>> b/drivers/net/ethernet/mediatek/mtk_eth_soc.h
>>> index 7c5e534..e3b9525 100644
>>> --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.h
>>> +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.h
>>> @@ -529,6 +529,8 @@ struct mtk_eth {
>>>   * @hw:			Backpointer to our main datastruture
>>>   * @hw_stats:		Packet statistics counter
>>>   * @phy_dev:		The attached PHY if available
>>> + * @trgmii		Indicate if the MAC uses TRGMII connected to internal
>>> +			switch
>>>   */
>>>  struct mtk_mac {
>>>  	int				id;
>>> @@ -539,6 +541,7 @@ struct mtk_mac {
>>>  	struct phy_device		*phy_dev;
>>>  	__be32				hwlro_ip[MTK_MAX_LRO_IP_CNT];
>>>  	int				hwlro_ip_cnt;
>>> +	bool				trgmii;
>>
>>     I don't see where this is used.
>
> I set trgmii as below
>   	switch (of_get_phy_mode(np)) {
> 	case PHY_INTERFACE_MODE_TRGMII:
> 		mac->trgmii = true;
>   	case PHY_INTERFACE_MODE_RGMII_TXID:

    You only set it in this patch but don't check it, so this is essentially a 
NOP. It only gets checked in another patch...

>> [...]
>>> diff --git a/include/linux/phy.h b/include/linux/phy.h index
>>> 2d24b28..e25f183 100644
>>> --- a/include/linux/phy.h
>>> +++ b/include/linux/phy.h
>>> @@ -80,6 +80,7 @@ typedef enum {
>>>  	PHY_INTERFACE_MODE_XGMII,
>>>  	PHY_INTERFACE_MODE_MOCA,
>>>  	PHY_INTERFACE_MODE_QSGMII,
>>> +	PHY_INTERFACE_MODE_TRGMII,
>>>  	PHY_INTERFACE_MODE_MAX,
>>>  } phy_interface_t;
>>>
>>> @@ -123,6 +124,8 @@ static inline const char *phy_modes(phy_interface_t interface)
>>>  		return "moca";
>>>  	case PHY_INTERFACE_MODE_QSGMII:
>>>  		return "qsgmii";
>>> +	case PHY_INTERFACE_MODE_TRGMII:
>>> +		return "trgmii";
>>>  	default:
>>>  		return "unknown";
>>>  	}
>>
>>    I think this should be done in a separate phylib patch.
>
> this patch is applied, so I am so little confused how to do this.

    Right, it's too late now.

MBR, Sergei

Re: [PATCH net-next v2 0/3] add support for RGMII on GMAC0 through TRGMII hardware module

[Sergei Shtylyov]

Hello.

On 9/23/2016 3:13 AM, David Miller wrote:

>>    Despite my comments? Sigh...
>
> Sorry, I thought he had addressed your feedback in v2.

    IIRC, I had only commented to v2, not v1.

> I'll wait longer next time.

    Thank you. :-)

MBR, Sergei