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