lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Sat, 18 Mar 2023 10:31:48 -0400 From: Sean Anderson <seanga2@...il.com> To: Simon Horman <simon.horman@...igine.com> Cc: "David S . Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, netdev@...r.kernel.org, linux-kernel@...r.kernel.org Subject: Re: [PATCH net-next v3 1/9] net: sunhme: Just restart autonegotiation if we can't bring the link up On 3/18/23 04:37, Simon Horman wrote: > On Mon, Mar 13, 2023 at 08:36:05PM -0400, Sean Anderson wrote: >> If we've tried regular autonegotiation and forcing the link mode, just >> restart autonegotiation instead of reinitializing the whole NIC. >> >> Signed-off-by: Sean Anderson <seanga2@...il.com> > > ... > >> @@ -606,6 +604,124 @@ static int is_lucent_phy(struct happy_meal *hp) >> return ret; >> } >> >> +/* hp->happy_lock must be held */ >> +static void >> +happy_meal_begin_auto_negotiation(struct happy_meal *hp, >> + void __iomem *tregs, >> + const struct ethtool_link_ksettings *ep) >> +{ >> + int timeout; >> + >> + /* Read all of the registers we are interested in now. */ >> + hp->sw_bmsr = happy_meal_tcvr_read(hp, tregs, MII_BMSR); >> + hp->sw_bmcr = happy_meal_tcvr_read(hp, tregs, MII_BMCR); >> + hp->sw_physid1 = happy_meal_tcvr_read(hp, tregs, MII_PHYSID1); >> + hp->sw_physid2 = happy_meal_tcvr_read(hp, tregs, MII_PHYSID2); >> + >> + /* XXX Check BMSR_ANEGCAPABLE, should not be necessary though. */ >> + >> + hp->sw_advertise = happy_meal_tcvr_read(hp, tregs, MII_ADVERTISE); >> + if (!ep || ep->base.autoneg == AUTONEG_ENABLE) { >> + /* Advertise everything we can support. */ >> + if (hp->sw_bmsr & BMSR_10HALF) >> + hp->sw_advertise |= (ADVERTISE_10HALF); >> + else >> + hp->sw_advertise &= ~(ADVERTISE_10HALF); >> + >> + if (hp->sw_bmsr & BMSR_10FULL) >> + hp->sw_advertise |= (ADVERTISE_10FULL); >> + else >> + hp->sw_advertise &= ~(ADVERTISE_10FULL); >> + if (hp->sw_bmsr & BMSR_100HALF) >> + hp->sw_advertise |= (ADVERTISE_100HALF); >> + else >> + hp->sw_advertise &= ~(ADVERTISE_100HALF); >> + if (hp->sw_bmsr & BMSR_100FULL) >> + hp->sw_advertise |= (ADVERTISE_100FULL); >> + else >> + hp->sw_advertise &= ~(ADVERTISE_100FULL); >> + happy_meal_tcvr_write(hp, tregs, MII_ADVERTISE, hp->sw_advertise); >> + >> + /* XXX Currently no Happy Meal cards I know off support 100BaseT4, >> + * XXX and this is because the DP83840 does not support it, changes >> + * XXX would need to be made to the tx/rx logic in the driver as well >> + * XXX so I completely skip checking for it in the BMSR for now. >> + */ >> + >> + ASD("Advertising [ %s%s%s%s]\n", >> + hp->sw_advertise & ADVERTISE_10HALF ? "10H " : "", >> + hp->sw_advertise & ADVERTISE_10FULL ? "10F " : "", >> + hp->sw_advertise & ADVERTISE_100HALF ? "100H " : "", >> + hp->sw_advertise & ADVERTISE_100FULL ? "100F " : ""); >> + >> + /* Enable Auto-Negotiation, this is usually on already... */ >> + hp->sw_bmcr |= BMCR_ANENABLE; >> + happy_meal_tcvr_write(hp, tregs, MII_BMCR, hp->sw_bmcr); >> + >> + /* Restart it to make sure it is going. */ >> + hp->sw_bmcr |= BMCR_ANRESTART; >> + happy_meal_tcvr_write(hp, tregs, MII_BMCR, hp->sw_bmcr); >> + >> + /* BMCR_ANRESTART self clears when the process has begun. */ >> + >> + timeout = 64; /* More than enough. */ >> + while (--timeout) { >> + hp->sw_bmcr = happy_meal_tcvr_read(hp, tregs, MII_BMCR); >> + if (!(hp->sw_bmcr & BMCR_ANRESTART)) >> + break; /* got it. */ >> + udelay(10); > > nit: Checkpatch tells me that usleep_range() is preferred over udelay(). > Perhaps it would be worth looking into that for a follow-up patch. This will be fixed in another series. >> + } >> + if (!timeout) { >> + netdev_err(hp->dev, >> + "Happy Meal would not start auto negotiation BMCR=0x%04x\n", >> + hp->sw_bmcr); >> + netdev_notice(hp->dev, >> + "Performing force link detection.\n"); >> + goto force_link; >> + } else { >> + hp->timer_state = arbwait; >> + } >> + } else { >> +force_link: >> + /* Force the link up, trying first a particular mode. >> + * Either we are here at the request of ethtool or >> + * because the Happy Meal would not start to autoneg. >> + */ >> + >> + /* Disable auto-negotiation in BMCR, enable the duplex and >> + * speed setting, init the timer state machine, and fire it off. >> + */ >> + if (!ep || ep->base.autoneg == AUTONEG_ENABLE) { >> + hp->sw_bmcr = BMCR_SPEED100; >> + } else { >> + if (ep->base.speed == SPEED_100) >> + hp->sw_bmcr = BMCR_SPEED100; >> + else >> + hp->sw_bmcr = 0; >> + if (ep->base.duplex == DUPLEX_FULL) >> + hp->sw_bmcr |= BMCR_FULLDPLX; >> + } >> + happy_meal_tcvr_write(hp, tregs, MII_BMCR, hp->sw_bmcr); >> + >> + if (!is_lucent_phy(hp)) { >> + /* OK, seems we need do disable the transceiver for the first >> + * tick to make sure we get an accurate link state at the >> + * second tick. >> + */ >> + hp->sw_csconfig = happy_meal_tcvr_read(hp, tregs, >> + DP83840_CSCONFIG); >> + hp->sw_csconfig &= ~(CSCONFIG_TCVDISAB); >> + happy_meal_tcvr_write(hp, tregs, DP83840_CSCONFIG, >> + hp->sw_csconfig); >> + } >> + hp->timer_state = ltrywait; >> + } >> + >> + hp->timer_ticks = 0; >> + hp->happy_timer.expires = jiffies + (12 * HZ)/10; /* 1.2 sec. */ > > nit: as a follow-up perhaps you could consider something like this. > (* completely untested! * ) > > hp->happy_timer.expires = jiffies + msecs_to_jiffies(1200); ditto. >> + add_timer(&hp->happy_timer); >> +} >> + > > ... --Sean
Powered by blists - more mailing lists