mv88e151x: fix potential attach and autonegotiation issues

[rlibby]

Summary

Reported by: GCC -Wmaybe-uninitialized, -Wtautological-compare

---

This is untested, I don't have this hardware. Please feel free to take this over.

Test Plan

env CROSS_TOOLCHAIN=amd64-gcc13 make buildkernel

[kp]

sys/dev/mii/mv88e151x.c

113

I don't like this all that much.
We only set mii_extcapabilities if sc->mii_capabilities & BMSR_EXTSTAT. Presumably the softc will be allocated with M_ZERO, but it's not ideal to rely on that.

Why don't we just uint32_t cop_cap = 0, cop_extcap = 0;?

227–228

This seems intuitively correct, and I believe the datasheet confirms.
See Table 99: Fiber Specific Status Register Page 1, Register 17 in https://www.mouser.com/datasheet/2/912/marvell-phys-transceivers-alaska-88e151x-datasheet-1360912.pdf

[rlibby]

sys/dev/mii/mv88e151x.c

113

That will solve the warning and I'm happy to do that and move on. I'll update the patch.

That said, if you are worried that sc->mii_extcapabilities might have some other value on entry to attach, shouldn't we then be initializing sc->mii_extcapabilities = 0? Else it seems we'd either leave the stale value set or if it's not stale then accidentally restore 0 over it. And maybe we should re-init it to 0 in the "Switch [to] the fiber PHY" block?

Here's what I think that would be. Let me know if you prefer this.
https://github.com/rlibby/freebsd/commit/122746a04f6f92fa354c9d7d56a46df07b30a1ca?diff=unified

227–228

Thanks for the pointer. Reading the table I notice the code does not check bit 11 "Speed and Duplex Resolved", but maybe that is already implied by the other checks (LINK, SYNC, and ENERGY)?

[kp]

sys/dev/mii/mv88e151x.c

113

That said, if you are worried that sc->mii_extcapabilities might have some other value on entry to attach, shouldn't we then be initializing sc->mii_extcapabilities = 0? Else it seems we'd either leave the stale value set or if it's not stale then accidentally restore 0 over it. And maybe we should re-init it to 0 in the "Switch [to] the fiber PHY" block?

I'm 85% sure that the code that allocates struct mii_softc does so with M_ZERO, so it should already be initialised to 0. The other PHY drivers also don't explicitly zero it out.

I've not found that code, but I'm on a high-latency link atm, so I didn't look very hard.

Here's what I think that would be. Let me know if you prefer this.
https://github.com/rlibby/freebsd/commit/122746a04f6f92fa354c9d7d56a46df07b30a1ca?diff=unified

I think I prefer just initialising cop_cap and cop_extcap to zero, but I don't have very strong feelings.

227–228

I'm not familiar enough with this hardware (or familiar with it at all, to be honest) to say.

This is pretty clearly an improvement already, so let's get this in and find someone with a test setup for this PHY and fiber links before we mess with it more.

[rlibby]

kp feedback: just initialize the copies to 0