On Mon, Dec 04, 2023 at 11:16:04AM +1000, David Gwynne wrote:
> On Sun, Dec 03, 2023 at 06:02:03PM +0100, Jan Stary wrote:
> > (please keep replies on the list)
> >
> > On Dec 03 12:08:08, kolip...@exoticsilicon.com wrote:
> > > On Sun, Dec 03, 2023 at 02:35:11PM +0100, Jan Stary wrote:
> > > > This is current/amd64 on a HP 260 G2 mini PC (dmesg below).
> > > > Everything works, except the wifi seems to be unsupported:
> > > >
> > > > "Realtek 8723BE" rev 0x00 at pci2 dev 0 function 0 not configured
> > >
> > > What does pcidump -v show?
> >
> > First of all, pcidump -v (but not pcidump) fucks up re(4):
> >
> > rgephy0 detached
> > re0 detached
> > re0 at pci1 dev 0 function 0 "Realtek 8168" rev 0x10: RTL8168GU/8111GU
> > (0x5080), msi, address 7c:d3:0a:21:eb:f5
> > rgephy0 at re0 phy 7: RTL8251 PHY, rev. 0
> > re0: cannot create re-stats kstat
> > rgephy0 detached
> > re0 detached
> > re0 at pci1 dev 0 function 0 "Realtek 8168" rev 0x10: RTL8168GU/8111GU
> > (0x5080), msi, address 7c:d3:0a:21:eb:f5
> > rgephy0 at re0 phy 7: RTL8251 PHY, rev. 0
> > re0: cannot create re-stats kstat
> >
> > Is anyone seeing that, i.e. devices detaching
> > when they are being probed by pcidump?
> >
> > After doing the pcidump -v localy and rebooting to upload, I get this.
> > Note that the Realtek 8168 entry seems mangled (related to the above?).
>
> pcidump causing a device to detach is a problem, but the kstat bit is a
> separate problem too.
>
> the diff below consolidates the detach code in re(4) and adds the code
> to tear the kstat down when the device goes away.
Makes sense. One question below.
OK claudio@
> Index: ic/re.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/ic/re.c,v
> retrieving revision 1.216
> diff -u -p -r1.216 re.c
> --- ic/re.c 10 Nov 2023 15:51:20 -0000 1.216
> +++ ic/re.c 4 Dec 2023 01:03:30 -0000
> @@ -2608,6 +2630,27 @@ freedma:
> destroy:
> bus_dmamap_destroy(sc->sc_dmat, re_ks_sc->re_ks_sc_map);
> free:
> + free(re_ks_sc, M_DEVBUF, sizeof(*re_ks_sc));
> +}
> +
> +void
> +re_kstat_detach(struct rl_softc *sc)
> +{
> + struct kstat *ks = sc->rl_kstat;
> + struct re_kstat_softc *re_ks_sc;
> +
> + if (ks == NULL)
> + return;
> +
> + kstat_remove(ks);
> + re_ks_sc = ks->ks_ptr;
> + kstat_destroy(ks);
> +
> + bus_dmamap_unload(sc->sc_dmat, re_ks_sc->re_ks_sc_map);
> + bus_dmamem_unmap(sc->sc_dmat,
> + (caddr_t)re_ks_sc->re_ks_sc_stats, sizeof(struct re_stats));
> + bus_dmamem_free(sc->sc_dmat, &re_ks_sc->re_ks_sc_seg, 1);
Shouldn't this use re_ks_sc->re_ks_sc_nsegs?
Or actually why save re_ks_sc_nsegs when it is known to be 1?
It is just confusing to see a difference with re_kstat_attach() in this
regard.
> + bus_dmamap_destroy(sc->sc_dmat, re_ks_sc->re_ks_sc_map);
> free(re_ks_sc, M_DEVBUF, sizeof(*re_ks_sc));
> }
> #endif /* NKSTAT > 0 */
--
:wq Claudio
