On 12/06/2016 05:50 PM, Timur Tabi wrote: > Florian Fainelli wrote: >>> + if (phydrv->features & (SUPPORTED_Pause | SUPPORTED_Asym_Pause)) { >>> >+ phydev->supported &= ~(SUPPORTED_Pause | >>> SUPPORTED_Asym_Pause); >>> >+ phydev->supported |= phydrv->features & >>> >+ (SUPPORTED_Pause | SUPPORTED_Asym_Pause); >> Is not the & (SUPPORTED_Pause | SUPPORTED_Asym_Pause) redundant here >> anyway? > > I'm just trying to be safe. Can I be certain that those bits are > already zero?
The bits are most likely not zero, since we have all the PHY_GBIT_FEATURES bits defined in there as well, but I don't think that is a real problem though, because we did this before: /* Start out supporting everything. Eventually, * a controller will attach, and may modify one * or both of these values */ phydev->supported = phydrv->features; of_set_phy_supported(phydev); phydev->advertising = phydev->supported; which is why this made me think the & (SUPPORTED_Pause | SUPPPORTED_Asym_Pause) here is most likely redundant? Thanks! -- Florian