On 07/20/2015 12:40 PM, Florian Fainelli wrote: > On 20/07/15 10:37, Dan Murphy wrote: >> Florian >> >> On 07/20/2015 12:28 PM, Florian Fainelli wrote: >>> Adding Dan, >>> >>> On 20/07/15 05:37, David Binderman wrote: >>>> Hello there, >>>> >>>> drivers/net/phy/dp83867.c:167:57: warning: logical ‘or’ of collectively >>>> exhaustive tests is always true [-Wlogical-op] >>>> >>>> Source code is >>>> >>>> if ((phydev->interface>= PHY_INTERFACE_MODE_RGMII_ID) || >>>> (phydev->interface <= PHY_INTERFACE_MODE_RGMII_RXID)) { >>>> >>>> Maybe >>>> >>>> if ((phydev->interface>= PHY_INTERFACE_MODE_RGMII_ID) && >>>> (phydev->interface <= PHY_INTERFACE_MODE_RGMII_RXID)) { >>> Sounds like the former is the intended comparison that will make sure >>> that phydev->interface is between MODE_RGMII_ID and MODE_RGMII_RXID, and >>> not below or after. >> That is correct. The internal delay only needs to be set if this is declared >> via the DT. There can be one of 3 interface internal delay (RGMII_ID), RX >> internal delay (RGMII_RX_ID) or TX >> internal delay (RGMII_TX_ID). > Sorry, I meant to write the latter instead of the former here, the > current code seems to be potentially too permissive, is not it? In that > case, it seems to me like David's proposed fix is relevant.
The proposed fix is logically correct. And it matches the code for phy_interface_is_rgmii with the '&&'. Did you want me to patch and test? Dan > >> This internal delay is only applicable for RGMII and can be made specific to >> the >> board. >> >> The driver only needs to set the delay per the declaration in the DT. >> >> Dan >> > -- ------------------ Dan Murphy -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html