On February 9, 2022 1:40 AM, Ferruh Yigit wrote:
> On 2/8/2022 10:11 AM, Jiawen Wu wrote:
> > Reduce the probability of PHY init failure, And add its error return.
> >
>
> Patch is missing stable tag, is it intentional?
> Or do you want patch not to be backported?
>
Because PHY init failed never occurred in our local tests with the original
code.
But the problem of failure to link up recently appeared in the customer, which
is extremely unlikely, and we think it is related to PHY initialization.
At the driver level, this fix addresses some potential risks. However, due to
different hardware (customers use our chips to make their own network cards),
this fix cannot be fully proven to be effective.
So this is just an optimization that we think needs to be done.
> > Signed-off-by: Jiawen Wu <jiawe...@trustnetic.com>
>
> <...>
>
> > @@ -234,17 +242,7 @@ s32 ngbe_reset_phy_rtl(struct ngbe_hw *hw)
> > value |= RTL_BMCR_RESET;
> > status = hw->phy.write_reg(hw, RTL_BMCR, RTL_DEV_ZERO, value);
> >
> > - for (i = 0; i < RTL_PHY_RST_WAIT_PERIOD; i++) {
> > - status = hw->phy.read_reg(hw, RTL_BMCR, RTL_DEV_ZERO,
> &value);
> > - if (!(value & RTL_BMCR_RESET))
> > - break;
> > - msleep(1);
> > - }
> > -
> > - if (i == RTL_PHY_RST_WAIT_PERIOD) {
> > - DEBUGOUT("PHY reset polling failed to complete.\n");
> > - return NGBE_ERR_RESET_FAILED;
> > - }
> > + msec_delay(5);
> >
>
> There are hardcoded delays added in this patch and other ones in this set, I
> just
> want to remind that this can lead unexpected (and very hard to debug) errors.
Yes, this polling never succeeded. So we think hardcoded delay is just required.
