Re: [PATCH net-next] drivers: net: xgene: Check all RGMII phy mode variants
> Hi Andrew, > > Our purpose is to handle our internal pdata->phy_mode, so > phy_interface_is_rgmii(phydev) seems not to fit. > Instead, we're working on the below: > > +bool is_xgene_enet_phy_mode_rgmii(struct net_device *ndev) > +{ > + struct xgene_enet_pdata *pdata = netdev_priv(ndev); > + > + return pdata->phy_mode >= PHY_INTERFACE_MODE_RGMII && > + pdata->phy_mode <= PHY_INTERFACE_MODE_RGMII_TXID; > +} > + This is very generic, can could be used by other drivers. I prefer what Florian suggested, have a generic helper which takes phy_mode as a parameters. And then modify phy_interface_is_rgmii() to use this helper. Andrew
Re: [PATCH net-next] drivers: net: xgene: Check all RGMII phy mode variants
On Thu, May 18, 2017 at 3:26 AM, Andrew Lunn wrote: >> +bool is_xgene_enet_phy_mode_rgmii(struct net_device *ndev) >> +{ >> + struct xgene_enet_pdata *pdata = netdev_priv(ndev); >> + int phy_mode = pdata->phy_mode; >> + bool ret; >> + >> + ret = phy_mode == PHY_INTERFACE_MODE_RGMII || >> + phy_mode == PHY_INTERFACE_MODE_RGMII_ID || >> + phy_mode == PHY_INTERFACE_MODE_RGMII_RXID || >> + phy_mode == PHY_INTERFACE_MODE_RGMII_TXID; >> + >> + return ret; >> +} > > include/linux/phy.h: > > /** > * phy_interface_is_rgmii - Convenience function for testing if a PHY > interface > * is RGMII (all variants) > * @phydev: the phy_device struct > */ > static inline bool phy_interface_is_rgmii(struct phy_device *phydev) > { > return phydev->interface >= PHY_INTERFACE_MODE_RGMII && > phydev->interface <= PHY_INTERFACE_MODE_RGMII_TXID; > }; > Hi Andrew, Our purpose is to handle our internal pdata->phy_mode, so phy_interface_is_rgmii(phydev) seems not to fit. Instead, we're working on the below: +bool is_xgene_enet_phy_mode_rgmii(struct net_device *ndev) +{ + struct xgene_enet_pdata *pdata = netdev_priv(ndev); + + return pdata->phy_mode >= PHY_INTERFACE_MODE_RGMII && + pdata->phy_mode <= PHY_INTERFACE_MODE_RGMII_TXID; +} +
Re: [PATCH net-next] drivers: net: xgene: Check all RGMII phy mode variants
On 05/17/2017 02:29 PM, Iyappan Subramanian wrote: > On Wed, May 17, 2017 at 1:26 PM, Andrew Lunn wrote: >>> +bool is_xgene_enet_phy_mode_rgmii(struct net_device *ndev) >>> +{ >>> + struct xgene_enet_pdata *pdata = netdev_priv(ndev); >>> + int phy_mode = pdata->phy_mode; >>> + bool ret; >>> + >>> + ret = phy_mode == PHY_INTERFACE_MODE_RGMII || >>> + phy_mode == PHY_INTERFACE_MODE_RGMII_ID || >>> + phy_mode == PHY_INTERFACE_MODE_RGMII_RXID || >>> + phy_mode == PHY_INTERFACE_MODE_RGMII_TXID; >>> + >>> + return ret; >>> +} >> >> include/linux/phy.h: >> >> /** >> * phy_interface_is_rgmii - Convenience function for testing if a PHY >> interface >> * is RGMII (all variants) >> * @phydev: the phy_device struct >> */ >> static inline bool phy_interface_is_rgmii(struct phy_device *phydev) >> { >> return phydev->interface >= PHY_INTERFACE_MODE_RGMII && >> phydev->interface <= PHY_INTERFACE_MODE_RGMII_TXID; >> }; > > Thanks. I'll use this helper function. If you can use it, that's great, the reason why I did not recommend using it before was because it takes a phydev reference and your code clearly could have run before connecting to the PHY device. Alternatively, we could introduce a helper function that just checks a phy_interface_t type, something like: static inline bool phy_interface_is_rgmii(phy_interface_t mode) { ... } and introduce: static inline bool phydev_interface_is_rgmii(struct phy_device *phydev) { return phy_interface_is_rgmii(phydev->interface); } which would use this helper function internally. Or just drop the second helper, and always pass phydev->interface where needed. -- Florian
Re: [PATCH net-next] drivers: net: xgene: Check all RGMII phy mode variants
On Wed, May 17, 2017 at 1:26 PM, Andrew Lunn wrote: >> +bool is_xgene_enet_phy_mode_rgmii(struct net_device *ndev) >> +{ >> + struct xgene_enet_pdata *pdata = netdev_priv(ndev); >> + int phy_mode = pdata->phy_mode; >> + bool ret; >> + >> + ret = phy_mode == PHY_INTERFACE_MODE_RGMII || >> + phy_mode == PHY_INTERFACE_MODE_RGMII_ID || >> + phy_mode == PHY_INTERFACE_MODE_RGMII_RXID || >> + phy_mode == PHY_INTERFACE_MODE_RGMII_TXID; >> + >> + return ret; >> +} > > include/linux/phy.h: > > /** > * phy_interface_is_rgmii - Convenience function for testing if a PHY > interface > * is RGMII (all variants) > * @phydev: the phy_device struct > */ > static inline bool phy_interface_is_rgmii(struct phy_device *phydev) > { > return phydev->interface >= PHY_INTERFACE_MODE_RGMII && > phydev->interface <= PHY_INTERFACE_MODE_RGMII_TXID; > }; Thanks. I'll use this helper function. > > Andrew
Re: [PATCH net-next] drivers: net: xgene: Check all RGMII phy mode variants
> +bool is_xgene_enet_phy_mode_rgmii(struct net_device *ndev) > +{ > + struct xgene_enet_pdata *pdata = netdev_priv(ndev); > + int phy_mode = pdata->phy_mode; > + bool ret; > + > + ret = phy_mode == PHY_INTERFACE_MODE_RGMII || > + phy_mode == PHY_INTERFACE_MODE_RGMII_ID || > + phy_mode == PHY_INTERFACE_MODE_RGMII_RXID || > + phy_mode == PHY_INTERFACE_MODE_RGMII_TXID; > + > + return ret; > +} include/linux/phy.h: /** * phy_interface_is_rgmii - Convenience function for testing if a PHY interface * is RGMII (all variants) * @phydev: the phy_device struct */ static inline bool phy_interface_is_rgmii(struct phy_device *phydev) { return phydev->interface >= PHY_INTERFACE_MODE_RGMII && phydev->interface <= PHY_INTERFACE_MODE_RGMII_TXID; }; Andrew