| 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 linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <ef07016f-fe3b-99d5-1f93-fc8e34baf18c@gmail.com>
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