I just wonder if the panic is a feature that the driver is broken? That is, does the driver still work ok if the ordering is wrong and this patch is applied?

sys/net/if_media.c
294	Style wants a blank line before comments.

bz added a subscriber: bz.Sat, May 3, 5:56 PM

bz added inline comments.

sys/net/if_media.c
303	That's likely going to be an unhelpful error. ENXIO is likely also misleading. But it should really be a panic("Fix the driver") or given you want it to be graceful, I'd add a uprintf saying something along the lines of "This driver needs fixing to work" so you do not get and endless amount of PRs (which you open yourself).

In D50136#1143592, @jhb wrote:

I just wonder if the panic is a feature that the driver is broken? That is, does the driver still work ok if the ordering is wrong and this patch is applied?

Both if_vr and if_re work fine with the diff applied. miibus eventually does come up, phys get probed, IP address assignment => working traffic, tested with iperf on both NICs.

adrian added inline comments.Sun, May 4, 12:34 AM

sys/net/if_media.c
303	ENXIO is likely .. less misleading? I don't mind adding a printf that's less terrible than mine in local debugging (ifmedia_ioctl: re0: ifm_status = NULL; erroring). It's happening when netlink wants to query the media state before announcing the new link. I'm sure /something/ will break that's subscribing to the notification.

address jhb/bz feedback

Harbormaster completed remote builds in B63886: Diff 154824.Mon, May 5, 3:51 AM

EDOOFUS was invented exactly for that. How many drivers do need a fix? Why this change instead of fixing the drivers?

In D50136#1144522, @glebius wrote:

EDOOFUS was invented exactly for that. How many drivers do need a fix? Why this change instead of fixing the drivers?

Shall I return EDOOFUS? :-)

I just want it to not panic in -HEAD whilst I go and do some cleanup and if_re work.

adrian marked an inline comment as done.Tue, May 6, 5:33 AM

I think EDOOFUS is fine.

This revision is now accepted and ready to land.Tue, May 6, 9:16 PM