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

From: Sergei Shtylyov
Date: Fri Sep 23 2016 - 06:20:03 EST


Hello.

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

From: Sean Wang <sean.wang@xxxxxxxxxxxx>

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