NetBSD-Bugs archive
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]
Re: kern/42314: IC Plus IP100x PHY support
The following reply was made to PR kern/42314; it has been noted by GNATS.
From: Andrius V <vezhlys%gmail.com@localhost>
To: Masanobu SAITOH <msaitoh%execsw.org@localhost>
Cc: gnats-bugs%netbsd.org@localhost, msaitoh%netbsd.org@localhost, gnats-admin%netbsd.org@localhost,
netbsd-bugs%netbsd.org@localhost, tharada%oucrc.org@localhost
Subject: Re: kern/42314: IC Plus IP100x PHY support
Date: Wed, 20 Nov 2019 22:46:40 +0200
--000000000000809c8c0597cd473a
Content-Type: text/plain; charset="UTF-8"
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)
>
--000000000000809c8c0597cd473a
Content-Type: text/html; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
<div dir=3D"auto">Btw, ipgphy.c and ipgphyreg.h doesn't have NetBSD hea=
der comment. Probably should be added too?</div><br><div class=3D"gmail_quo=
te"><div dir=3D"ltr" class=3D"gmail_attr">On Wed, Nov 20, 2019, 21:45 Andri=
us V <<a href=3D"mailto:vezhlys%gmail.com@localhost";>vezhlys%gmail.com@localhost</a>> wro=
te:<br></div><blockquote class=3D"gmail_quote" style=3D"margin:0 0 0 .8ex;b=
order-left:1px #ccc solid;padding-left:1ex">Tested, this patch works well. =
Changing media type works in all cases<br>
now. Thanks.<br>
<br>
On Wed, Nov 20, 2019 at 5:14 PM SAITOH Masanobu <<a href=3D"mailto:msait=
oh%execsw.org@localhost" target=3D"_blank" rel=3D"noreferrer">msaitoh%execsw.org@localhost</a>&=
gt; wrote:<br>
><br>
> On 2019/11/20 15:25, Andrius V wrote:<br>
> > Hi,<br>
> ><br>
> > With the latest code ifconfig selects media correctly and I don&#=
39;t see<br>
> > watchdog timeout messages but dhcpcd still fails to get IP addres=
s<br>
> > properly though (assigns <a href=3D"http://169.254.161.13/16"; rel=
=3D"noreferrer noreferrer" target=3D"_blank">169.254.161.13/16</a>) and net=
work is not working<br>
> > because of that. Same happens with 1000baseT-FDX and 1000baseT no=
w<br>
> > (for 1000baseT it is a regression since previous commit). Reselec=
ting<br>
> > auto or lower media types (100baseT, 10baseT) works correctly and=
dhcp<br>
> > reassigns proper the IP address.<br>
><br>
> Could you test the following diff?<br>
> - Set duplex correctly when user setting is not IFM_AUTO.<br>
> - When the link is up, set VGE_DIAGCTL not from user media setting but=
from<br>
>=C2=A0 =C2=A0the current active link status.<br>
><br>
> Index: if_vge.c<br>
> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D<br>
> RCS file: /cvsroot/src/sys/dev/pci/if_vge.c,v<br>
> retrieving revision 1.76<br>
> diff -u -p -r1.76 if_vge.c<br>
> --- if_vge.c=C2=A0 =C2=A0 19 Nov 2019 09:54:07 -0000=C2=A0 =C2=A0 =C2=
=A0 1.76<br>
> +++ if_vge.c=C2=A0 =C2=A0 20 Nov 2019 15:08:28 -0000<br>
> @@ -1928,33 +1928,34 @@ vge_miibus_statchg(struct ifnet *ifp)<br>
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 * always implied, so we turn on the =
forced mode bit but leave<br>
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 * the FDX bit cleared.<br>
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 */<br>
> -<br>
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0dctl =3D CSR_READ_1(sc, VGE_DIAGCTL);=
<br>
><br>
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0switch (IFM_SUBTYPE(ife->ifm_media)) {<=
br>
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0case IFM_AUTO:<br>
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0if (IFM_SUBTYPE(ife->ifm_media) =3D=3D =
IFM_AUTO) {<br>
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0dctl &=
;=3D ~VGE_DIAGCTL_MACFORCE;<br>
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0dctl &=
;=3D ~VGE_DIAGCTL_FDXFORCE;<br>
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0break;<br>
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0case IFM_1000_T:<br>
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0dctl |=3D VGE_=
DIAGCTL_MACFORCE;<br>
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0dctl &=3D =
~VGE_DIAGCTL_FDXFORCE;<br>
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0dctl |=3D VGE_=
DIAGCTL_GMII;<br>
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0break;<br>
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0case IFM_100_TX:<br>
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0case IFM_10_T:<br>
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0} else {<br>
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0u_int ifmword;=
<br>
> +<br>
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0/* If the link=
is up, use the current active media. */<br>
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0if ((mii->m=
ii_media_status & IFM_ACTIVE) !=3D 0)<br>
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0=
=C2=A0 =C2=A0ifmword =3D mii->mii_media_active;<br>
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0else<br>
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0=
=C2=A0 =C2=A0ifmword =3D ife->ifm_media;<br>
> +<br>
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0dctl |=3D=
VGE_DIAGCTL_MACFORCE;<br>
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0dctl &=3D =
~VGE_DIAGCTL_GMII;<br>
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0if ((ife->i=
fm_media & IFM_FDX) !=3D 0)<br>
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0if ((ifmword &=
amp; IFM_FDX) !=3D 0)<br>
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0 =C2=A0 =C2=A0dctl |=3D VGE_DIAGCTL_FDXFORCE;<br>
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0else<br>
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0 =C2=A0 =C2=A0dctl &=3D ~VGE_DIAGCTL_FDXFORCE;<br>
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0break;<br>
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0default:<br>
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0printf("%=
s: unknown media type: %x\n",<br>
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0=
device_xname(sc->sc_dev),<br>
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0=
IFM_SUBTYPE(ife->ifm_media));<br>
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0break;<br>
> +<br>
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0if (IFM_SUBTYP=
E(ifmword) =3D=3D IFM_1000_T) {<br>
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0=
=C2=A0 =C2=A0/*<br>
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0=
=C2=A0 =C2=A0 * It means the user setting is not auto and it's<br>
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0=
=C2=A0 =C2=A0 * 1000baseT-FDX or 1000baseT.<br>
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0=
=C2=A0 =C2=A0 */<br>
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0=
=C2=A0 =C2=A0dctl |=3D VGE_DIAGCTL_GMII;<br>
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0} else<br>
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0=
=C2=A0 =C2=A0dctl &=3D ~VGE_DIAGCTL_GMII;<br>
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0}<br>
><br>
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0CSR_WRITE_1(sc, VGE_DIAGCTL, dctl);<b=
r>
> -----------<br>
><br>
><br>
><br>
> The same diff is at:<br>
><br>
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0<a href=3D"http://www.netbsd.org/~msa=
itoh/vge-20191120-0.dif" rel=3D"noreferrer noreferrer" target=3D"_blank">ht=
tp://www.netbsd.org/~msaitoh/vge-20191120-0.dif</a><br>
><br>
> Thanks in advance.<br>
><br>
> --<br>
> -----------------------------------------------<br>
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0SAITOH Ma=
sanobu (<a href=3D"mailto:msaitoh%execsw.org@localhost"; target=3D"_blank" rel=3D"nore=
ferrer">msaitoh%execsw.org@localhost</a><br>
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 <a href=3D"mailto:msaitoh@=
netbsd.org" target=3D"_blank" rel=3D"noreferrer">msaitoh%netbsd.org@localhost</a>)<br=
>
</blockquote></div>
--000000000000809c8c0597cd473a--
Home |
Main Index |
Thread Index |
Old Index