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@gmail.com>
---
(no changes since v2)
Changes in v2:
- Move happy_meal_begin_auto_negotiation earlier and remove forward declaration
drivers/net/ethernet/sun/sunhme.c | 245 +++++++++++++++---------------
1 file changed, 119 insertions(+), 126 deletions(-)
diff --git a/drivers/net/ethernet/sun/sunhme.c b/drivers/net/ethernet/sun/sunhme.c
index b0c7ab74a82e..65733ee5ddd9 100644
--- a/drivers/net/ethernet/sun/sunhme.c
+++ b/drivers/net/ethernet/sun/sunhme.c
@@ -589,8 +589,6 @@ static int set_happy_link_modes(struct happy_meal *hp, void __iomem *tregs)
return 1;
}
-static int happy_meal_init(struct happy_meal *hp);
-
static int is_lucent_phy(struct happy_meal *hp)
{
void __iomem *tregs = hp->tcvregs;
@@ -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);
+ }
+ 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. */
+ add_timer(&hp->happy_timer);
+}
+
static void happy_meal_timer(struct timer_list *t)
{
struct happy_meal *hp = from_timer(hp, t, happy_timer);
@@ -743,12 +859,7 @@ static void happy_meal_timer(struct timer_list *t)
netdev_notice(hp->dev,
"Link down, cable problem?\n");
- ret = happy_meal_init(hp);
- if (ret) {
- /* ho hum... */
- netdev_err(hp->dev,
- "Error, cannot re-init the Happy Meal.\n");
- }
+ happy_meal_begin_auto_negotiation(hp, tregs, NULL);
goto out;
}
if (!is_lucent_phy(hp)) {
@@ -1201,124 +1312,6 @@ static void happy_meal_init_rings(struct happy_meal *hp)
HMD("done\n");
}
-/* 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);
- }
- 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. */
- add_timer(&hp->happy_timer);
-}
-
/* hp->happy_lock must be held */
static int happy_meal_init(struct happy_meal *hp)
{
--
2.37.1On 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@gmail.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.
> + }
> + 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);
> + add_timer(&hp->happy_timer);
> +}
> +
...
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@gmail.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
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@gmail.com> Reviewed-by: Simon Horman <simon.horman@corigine.com>
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@gmail.com> Hi Sean, This patch looks fine to me, as do patches 3 - 4, which is as far as I have got with my review. I do, however, have a general question regarding most of the patches in this series: to what extent have they been tested on HW? And my follow-up question is: to what extent should we consider removing support for hardware that isn't being tested and therefore has/will likely have become broken break at some point? Quattro, the subject of a latter patch in this series, seems to be a case in point.
On 3/15/23 04:20, 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@gmail.com> > > Hi Sean, > > This patch looks fine to me, as do patches 3 - 4, which is as far as I have > got with my review. > > I do, however, have a general question regarding most of the patches in this > series: to what extent have they been tested on HW? I have tested them with some PCI cards, mostly with the other end autonegotiating 100M. This series doesn't really touch the phy state machines, so I think it is fine to just make sure the link comes up (and things work after bringing the interface down and up). > And my follow-up question is: to what extent should we consider removing > support for hardware that isn't being tested and therefore has/will likely > have become broken break at some point? Quattro, the subject of a latter > patch in this series, seems to be a case in point. Well, I ordered a quattro card (this hardware is quite cheap on ebay) so hopefully I can test that. The real question is whether there's anyone using this on sparc. I tried CCing some sparc users mailing lists in the cover letter, but no luck so far. --Sean
On Wed, Mar 15, 2023 at 11:35:19AM -0400, Sean Anderson wrote: > On 3/15/23 04:20, 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@gmail.com> > > > > Hi Sean, > > > > This patch looks fine to me, as do patches 3 - 4, which is as far as I have > > got with my review. > > > > I do, however, have a general question regarding most of the patches in this > > series: to what extent have they been tested on HW? > > I have tested them with some PCI cards, mostly with the other end > autonegotiating 100M. This series doesn't really touch the phy state > machines, so I think it is fine to just make sure the link comes up (and > things work after bringing the interface down and up). Understood, I'll proceed with my review on that basis. > > And my follow-up question is: to what extent should we consider removing > > support for hardware that isn't being tested and therefore has/will likely > > have become broken break at some point? Quattro, the subject of a latter > > patch in this series, seems to be a case in point. > > Well, I ordered a quattro card (this hardware is quite cheap on ebay) so > hopefully I can test that. Yes, for some reason I was searching on eBay too :) > The real question is whether there's anyone using this on sparc. I tried > CCing some sparc users mailing lists in the cover letter, but no luck so > far. Yes, that is the question.
© 2016 - 2026 Red Hat, Inc.