Re: drivers/net/phy/dp83867.c:167: possible bad if ?

On 20/07/15 10:37, Dan Murphy wrote:

Florian

On 07/20/2015 12:28 PM, Florian Fainelli wrote:

quoted

Adding Dan,

On 20/07/15 05:37, David Binderman wrote:

quoted

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.

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

-- 
Florian