Re: kern/42314: IC Plus IP100x PHY support

[Andrius V <vezhlys%gmail.com@localhost>]

Btw, ipgphy.c and ipgphyreg.h doesn't have NetBSD header comment. Probably should be added too?

On Wed, Nov 20, 2019, 21:45 Andrius V <vezhlys%gmail.com@localhost> wrote:

Tested, this patch works well. Changing media type works in all cases
now. Thanks.

On Wed, Nov 20, 2019 at 5:14 PM SAITOH Masanobu <msaitoh%execsw.org@localhost> wrote:
>
> On 2019/11/20 15:25, Andrius V wrote:
> > Hi,
> >
> > With the latest code ifconfig selects media correctly and I don't see
> > watchdog timeout messages but dhcpcd still fails to get IP address
> > properly though (assigns 169.254.161.13/16) and network is not working
> > because of that. Same happens with 1000baseT-FDX and 1000baseT now
> > (for 1000baseT it is a regression since previous commit). Reselecting
> > auto or lower media types (100baseT, 10baseT) works correctly and dhcp
> > reassigns proper the IP address.
>
> Could you test the following diff?
> - Set duplex correctly when user setting is not IFM_AUTO.
> - When the link is up, set VGE_DIAGCTL not from user media setting but from
>   the current active link status.
>
> Index: if_vge.c
> ===================================================================
> RCS file: /cvsroot/src/sys/dev/pci/if_vge.c,v
> retrieving revision 1.76
> diff -u -p -r1.76 if_vge.c
> --- if_vge.c    19 Nov 2019 09:54:07 -0000      1.76
> +++ if_vge.c    20 Nov 2019 15:08:28 -0000
> @@ -1928,33 +1928,34 @@ vge_miibus_statchg(struct ifnet *ifp)
>          * always implied, so we turn on the forced mode bit but leave
>          * the FDX bit cleared.
>          */
> -
>         dctl = CSR_READ_1(sc, VGE_DIAGCTL);
>
> -       switch (IFM_SUBTYPE(ife->ifm_media)) {
> -       case IFM_AUTO:
> +       if (IFM_SUBTYPE(ife->ifm_media) == IFM_AUTO) {
>                 dctl &= ~VGE_DIAGCTL_MACFORCE;
>                 dctl &= ~VGE_DIAGCTL_FDXFORCE;
> -               break;
> -       case IFM_1000_T:
> -               dctl |= VGE_DIAGCTL_MACFORCE;
> -               dctl &= ~~VGE_DIAGCTL_FDXFORCE;
> -               dctl |= VGE_DIAGCTL_GMII;
> -               break;
> -       case IFM_100_TX:
> -       case IFM_10_T:
> +       } else {
> +               u_int ifmword;
> +
> +               /* If the link is up, use the current active media. */
> +               if ((mii->mii_media_status & IFM_ACTIVE) != 0)
> +                       ifmword = mii->mii_media_active;
> +               else
> +                       ifmword = ife->ifm_media;
> +
>                 dctl |= VGE_DIAGCTL_MACFORCE;
> -               dctl &= ~~VGE_DIAGCTL_GMII;
> -               if ((ife->ifm_media & IFM_FDX) != 0)
> +               if ((ifmword & IFM_FDX) != 0)
>                         dctl |= VGE_DIAGCTL_FDXFORCE;
>                 else
>                         dctl &= ~VGE_DIAGCTL_FDXFORCE;
> -               break;
> -       default:
> -               printf("%s: unknown media type: %x\n",
> -                   device_xname(sc->sc_dev),
> -                   IFM_SUBTYPE(ife->ifm_media));
> -               break;
> +
> +               if (IFM_SUBTYPE(ifmword) == IFM_1000_T) {
> +                       /*
> +                        * It means the user setting is not auto and it's
> +                        * 1000baseT-FDX or 1000baseT.
> +                        */
> +                       dctl |= VGE_DIAGCTL_GMII;
> +               } else
> +                       dctl &= ~VGE_DIAGCTL_GMII;
>         }
>
>         CSR_WRITE_1(sc, VGE_DIAGCTL, dctl);
> -----------
>
>
>
> The same diff is at:
>
>         http://www.netbsd.org/~msaitoh/vge-20191120-0.dif
>
> Thanks in advance.
>
> --
> -----------------------------------------------
>                 SAITOH Masanobu (msaitoh%execsw.org@localhost
>                                  msaitoh%netbsd.org@localhost)