[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <b8791155-c1c0-6dc8-13d1-7940cc34f689@cogentembedded.com>
Date: Fri, 23 Sep 2016 13:19:51 +0300
From: Sergei Shtylyov <sergei.shtylyov@...entembedded.com>
To: Sean Wang <sean.wang@...iatek.com>
Cc: john@...ozen.org, davem@...emloft.net, nbd@...nwrt.org,
netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-mediatek@...ts.infradead.org, keyhaede@...il.com,
objelf@...il.com
Subject: Re: [PATCH net-next v2 1/3] net: ethernet: mediatek: add extension of
phy-mode for TRGMII
Hello.
On 9/23/2016 6:32 AM, Sean Wang wrote:
>>> From: Sean Wang <sean.wang@...iatek.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
Powered by blists - more mailing lists