In particular, don't check the value of the bus_dma map against NULL
to determine if either bus_dmamem_alloc() or bus_dmamap_load() succeeded.
Instead, assume that bus_dmamap_load() succeeeded (and thus that
bus_dmamap_unload() should be called) if the bus address for a resource
is non-zero, and assume that bus_dmamem_alloc() succeeded (and thus
that bus_dmamem_free() should be called) if the virtual address for a
resource is not NULL.
In many cases these bugs could result in leaks when a driver was detached.
Reviewed by: yongari
MFC after: 2 weeks
to this event, adding if_var.h to files that do need it. Also, include
all includes that now are included due to implicit pollution via if_var.h
Sponsored by: Netflix
Sponsored by: Nginx, Inc.
them, please let me know if not). Most of these are of the form:
static const struct bzzt_type {
[...list of members...]
} const bzzt_devs[] = {
[...list of initializers...]
};
The second const is unnecessary, as arrays cannot be modified anyway,
and if the elements are const, the whole thing is const automatically
(e.g. it is placed in .rodata).
I have verified this does not change the binary output of a full kernel
build (except for build timestamps embedded in the object files).
Reviewed by: yongari, marius
MFC after: 1 week
deterministically handled after the corresponding PHY drivers when
loaded as modules. Otherwise, when these MAC/PHY driver pairs are
compiled into a single module probing the PHY driver may fail. This
makes r151438 and r226154 actually work. [1]
Reported and tested by: yongari (fxp(4))
- Use DEVMETHOD_END.
- Use NULL instead of 0 for pointers.
Submitted by: jhb [1]
MFC after: 3 days
one. Interestingly, these are actually the default for quite some time
(bus_generic_driver_added(9) since r52045 and bus_generic_print_child(9)
since r52045) but even recently added device drivers do this unnecessarily.
Discussed with: jhb, marcel
- While at it, use DEVMETHOD_END.
Discussed with: jhb
- Also while at it, use __FBSDID.
dcphy(4) (CID 9283).
- In dc_detach(), check whether ifp is NULL as dc_attach() may call the
former without ifp being allocated (CID 4288).
Found with: Coverity Prevent(tm)
!DC_IS_ADMTEK in dc_miibus_statchg(). This change broke link
establishment of Intel 21143 with dcphy(4) where it stuck in
"ability detect" state without completing auto-negotiation.
Also nuke dc_if_media as it's not actually used.
Submitted by: marius
take advantage of it instead of duplicating it. This reduces the size of
the i386 GENERIC kernel by about 4k. The only potential in-tree user left
unconverted is xe(4), which generally should be changed to use miibus(4)
instead of implementing PHY handling on its own, as otherwise it makes not
much sense to add a dependency on miibus(4)/mii_bitbang(4) to xe(4) just
for the MII bitbang'ing code. The common MII bitbang'ing code also is
useful in the embedded space for using GPIO pins to implement MII access.
- Based on lessons learnt with dc(4) (see r185750), add bus barriers to the
MII bitbang read and write functions of the other drivers converted in
order to ensure the intended ordering. Given that register access via an
index register as well as register bank/window switching is subject to the
same problem, also add bus barriers to the respective functions of smc(4),
tl(4) and xl(4).
- Sprinkle some const.
Thanks to the following testers:
Andrew Bliznak (nge(4)), nwhitehorn@ (bm(4)), yongari@ (sis(4) and ste(4))
Thanks to Hans-Joerg Sirtl for supplying hardware to test stge(4).
Reviewed by: yongari (subset of drivers)
Obtained from: NetBSD (partially)
controller which is found on ULi M1563 South Bridge & M1689 Bridge.
These controllers look like a tulip clone.
M5263 controller does not support MII bitbang so use DC_ROM
register to access MII registers. Like other tulip variants, ULi
controller uses a setup frame to configure RX filter and uses new
setup frame format. It's not clear to me whether the controller
supports a hash based multicast filtering so this patch uses 14
perfect multicast filter to filter multicast frames. If number of
multicast addresses is greater than 14, controller is put into a
mode that receives all multicast frames.
Due to lack of access to M5261, this change was not tested with
M5261 but it probably works. Many thanks to Marco who provided
remote access to M5263.
Tested by: Marco Steinbach <coco <> executive-computing dot de>,
Martin MATO <martin.mato <> orange dot fr>
link such that calling dc_setcfg() right after media change would
be meaningless unless controller in question is not Davicom DM9102.
Ideally dc_setcfg() should be called when speed/duplex is resolved
otherwise it would reprogram controller with wrong speed/duplex
information. Because MII status change callback already calls
dc_setcfg() I think calling dc_setcfg() in dc_init_locked() is
wrong. For instance, it would take some time to establish a link
after mii_mediachg(), so blindly calling dc_setcfg() right after
mii_mediachg() will always yield wrong media configuration.
Extend dc_ifmedia_upd() to handle media change and still allow
21143 and Davidcom controllers program speed/duplex regardless of
current resolved speed/duplex of link. In theory 21143 may not need
to call dc_setcfg() right after media change, but leave it as it is
because there are too many variants to test that change. Probably
dc(4) shall need a PHY reset in dc_ifmedia_upd() but it's hard to
verify correctness of the change.
This change reliably makes ULi M5263 establish a link.
While I'm here correctly report media change result. Previously it
always reported a success.
present. Only call the bus to check if we actually do timeout so we
don't affect the normal case (since this case needn't be optimized and
this guards against all races).
of active DMA cycle. dc_setcfg() also has to wait until the DMA
engine is stopped so using a common function to handle the job is
better than duplicating the code.
No objection from: marius
With this change, driver may not notice updated descriptor status
change when bounce buffers are active. However, rxeof() in next run
will handle the synchronization.
Change dc_rxeof() a bit to return the number of processed frames in
RX descriptor ring. Previously it returned the number of frames
that were successfully passed to upper stack which in turn means it
ignored frames that were discarded due to errors. The number of
processed frames in RX descriptor ring is used to detect whether
driver is out of sync with controller's current descriptor pointer.
Returning number of processed frames reduces unnecessary (probably
wrong) re-synchronization.
Reviewed by: marius
value. Controllers that always require "store and forward" mode(
Davicom and PNIC 82C168) have no way to recover from TX underrun
except completely reinitializing hardware. Previously only Davicom
was reinitialized and the TX FIFO threshold was changed not to use
"store and forward" mode after reinitialization since the default
FIFO threshold value was 0. This effectively disabled Davicom
controller's "store and forward" mode once it encountered TX
underruns. In theory, this can cause watchodg timeouts.
Intel 21143 controller requires TX MAC should be idle before
changing TX FIFO threshold. So driver tried to disable TX MAC and
checked whether it saw the idle state of TX MAC. Driver should
perform full hardware reinitialization on failing to enter to idle
state and it should not touch TX MAC again once it performed full
reinitialization.
While I'm here remove resetting TX FIFO threshold to 0 when
interface is put into down state. If driver ever encountered TX
underrun, it's likely to trigger TX underrun again whenever
interface is brought to up again. Keeping old/learned TX FIFO
threshold value shall reduce the chance of seeing TX underrns in
next run.
link flips during alias address insertion or dhclient operation.
While I'm here remove dc_reset() in DC_ISR_BUS_ERR case. Device is
fully reinitialized again in dc_init_locked().
respectively and fix all bus_dma(9) issues seen when bounce buffers
are used.
o Setup frame handling had no bus_dmamap_sync(9) which prevented
driver from configuring RX filter. Add missing bus_dmamap_sync(9)
in both dc_setfilt_21143()/dc_setfilt_xircom() and dc_txeof().
o Use bus_addr_t for DMA segment instead of using u_int32_t.
o Introduce dc_dma_alloc()/dc_dma_free() functions to allocate/free
DMA'able memory.
o Create two DMA descriptor list for each TX/RX lists. This change
will minimize the size of bounce buffers that would be used in
each TX/RX path. Previously driver had to copy both TX/RX lists
when bounce buffer is active.
o 21143 data sheet says descriptor list requires 4 bytes alignment.
Remove PAGE_SIZE alignment restriction and use
sizeof(struct dc_dec).
o Setup frame requires 4 bytes alignment. Remove PAGE_SIZE
alignment restriction and use sizeof(struct dc_dec).
o Add missing DMA map unload for both setup frame and TX/RX
descriptor list.
o Overhaul RX handling logic such that make driver always allocate
new RX buffer with dc_newbuf(). Previously driver allowed to
copy received frame with m_devget(9) after passing the
descriptor ownership to controller. This can lead to passing
wrong frame to upper stack.
o Introduce dc_discard_rxbuf() which will discard received frame
and reuse loaded DMA map and RX mbuf.
o Correct several wrong bus_dmamap_sync(9) usage in dc_rxeof and
dc_txeof. The TX/RX descriptor lists are updated by both driver
and HW so READ/WRITE semantics should be used.
o If driver failed to allocate new RX buffer, update if_iqdrops
counter instead of if_ierrors since driver received the frame
without errors.
o Make sure to unload loaded setup frame DMA map in dc_txeof and
clear the mark of setup frame of the TX descriptor in dc_txeof().
o Add check for possible TX descriptor overruns in dc_encap() and
move check for free buffer to caller, dc_start_locked().
o Swap the loaded DMA map and the last DMA map for multi-segmented
frames. Since dc_txeof() assumes the last descriptor of the
frame has the DMA map, driver should swap the first and the last
DMA map in dc_encap(). Previously driver tried to unload
not-yet-loaded DMA map such that the loaded DMA map was not
unloaded at all for multi-segmented frames.
o Rewrite DC_RXDESC/DC_TXDESC macro to simpler one.
o Remove definition of ETHER_ALIGN, it's already defined in
ethernet.h.
With this changes, dc(4) works with bounce buffers and it shall
also fix issues which might have shown in PAE environments.
Tested by: marius
Previously dc(4) always checked whether there is pending interrupts
and this consumed a lot of CPU cycles in interrupt handler. Limit
the number of processing for TX/RX frames to 16. Also allow sending
frames in the loop not to starve TX under high RX load.
Reading DC_ISR register should be protected with driver lock,
otherwise interrupt handler could be run(e.g. link state change)
before the completion of dc_init_locked().
While I'm here remove unneeded code.
as well as controller has enough free TX descriptors.
Remove check for number of queued frames before attempting to
transmit. I guess it was added to allow draining queued frames
even if there is no link. I'm under the impression this type of
check should be done in upper layer. No other drivers in tree do
that.
ownership to controller before completion of access to the
descriptor. Driver is faking up status word so it should not give
ownership to controller until it completes RX processing.
request TX completion interrupt for every 8-th frames. Previously
dc(4) requested TX completion interrupt if number of queued TX
descriptors is greater than 64. This caused a lot of TX completion
interrupt under high TX load once driver queued more than 64 TX
descriptors. It's quite normal to see more than 64 queued TX
descriptors under high TX load.
This change reduces the number of TX completion interrupts to be
less than 17k under high TX load. Because this change does not
generate TX completion interrupt for each frame, add reclaiming
transmitted buffers in dc_tick not to generate false watchdog
timeouts.
While I'm here add check for queued descriptors in dc_txeof() since
there is no more work to do when there is no pending descriptors.
frame in DM910x controllers. In r67595(more than 10 years ago) it
was replaced to use "Store and Forward" mode and made controller
generate TX completion interrupt for every frame.
any other media configuration. Otherwise some 21143 controller
cannot establish a link. While I'm here remove the PHY
initialization code in dc_setcfg(). Since dc_setcfg() is called
whenever link state is changed, having the PHY initialization code
in dc_setcfg() resulted in continuous link flips.
After driver resets SIA, use default SIA transmit/receive
configuration instead of disabling autosense/autonegotiation.
Otherwise, controller fails to establish a link as well as losing
auto-negotiation capability. For manual media configuration, always
configure 21143 controller with specified media to ensure media
change. This change makes ANA-6922 establish link with/without
auto-negotiation.
While I'm here be more strict on link UP/DOWN detection logic.
Many thanks to marius who fixed several bugs in initial patch and
even tested the patch on a couple of dc(4) controllers.
PR: kern/79262
Reviewed by: marius
Tested by: marius
port, copy SROM information from base softc as well and run SROM
parser again. This change is necessary for some dual port
controllers to make dc(4) correctly detect PHY media based on first
port configuration table.
While I'm here add a check for validity of the base softc before
duplicating SROM contents from base softc. If driver failed to
attach to the first port it can access invalid area.
PR: kern/79262
Reviewed by: marius
the NIC drivers as well as the PHY drivers to take advantage of the
mii_attach() introduced in r213878 to get rid of certain hacks. For
the most part these were:
- Artificially limiting miibus_{read,write}reg methods to certain PHY
addresses; we now let mii_attach() only probe the PHY at the desired
address(es) instead.
- PHY drivers setting MIIF_* flags based on the NIC driver they hang
off from, partly even based on grabbing and using the softc of the
parent; we now pass these flags down from the NIC to the PHY drivers
via mii_attach(). This got us rid of all such hacks except those of
brgphy() in combination with bce(4) and bge(4), which is way beyond
what can be expressed with simple flags.
While at it, I took the opportunity to change the NIC drivers to pass
up the error returned by mii_attach() (previously by mii_phy_probe())
and unify the error message used in this case where and as appropriate
as mii_attach() actually can fail for a number of reasons, not just
because of no PHY(s) being present at the expected address(es).
Reviewed by: jhb, yongari
port such that reading station address from second port always
returned 0xFF:0xFF:0xFF:0xFF:0xFF:0xFF Unfortunately it seems there
is no easy way to know whether SROM is shared or not. Workaround
the issue by traversing dc(4) device list and see whether we're
using second port and use station address of controller 0 as base
station address of second port.
PR: kern/79262
MFC after: 2 weeks
IF_ADDR_UNLOCK() across network device drivers when accessing the
per-interface multicast address list, if_multiaddrs. This will
allow us to change the locking strategy without affecting our driver
programming interface or binary interface.
For two wireless drivers, remove unnecessary locking, since they
don't actually access the multicast address list.
Approved by: re (kib)
MFC after: 6 weeks
CPU for too long period than necessary. Additively, interfaces are kept
polled (in the tick) even if no more packets are available.
In order to avoid such situations a new generic mechanism can be
implemented in proactive way, keeping track of the time spent on any
packet and fragmenting the time for any tick, stopping the processing
as soon as possible.
In order to implement such mechanism, the polling handler needs to
change, returning the number of packets processed.
While the intended logic is not part of this patch, the polling KPI is
broken by this commit, adding an int return value and the new flag
IFCAP_POLLING_NOCOUNT (which will signal that the return value is
meaningless for the installed handler and checking should be skipped).
Bump __FreeBSD_version in order to signal such situation.
Reviewed by: emaste
Sponsored by: Sandvine Incorporated
drivers, there should be a 1us delay after every write when
bit-banging the MII. Also insert barriers in order to ensure
the intended ordering. These changes hopefully will solve the
bus wedging occasionally experienced with DM9102A since r182461.
- Deobfuscate dc_mii_readreg() a bit.
stopped nor the waiting state and also no other means to check
whether the receiver is idle (see also r163774), we have no choice
than to call mii_tick(9) unconditionally even in the case of the
DC_REDUCED_MII_POLL handling as far as the RX side is concerned.
This isn't necessarily worse than checking whether RX is idle
though because unlike as with TX we're racing with the hardware,
which might receive packets any time while we poll the MII, anyway.
Reported and tested by: Jacob Owens
Reviewed by: yongari
MFC after: 3 days
in a noticeable reduction in system time spent.
- If bus_dmamap_load_mbuf_sg(9) fails with EFBIG and we already have
defragmented the mbuf chain, don't bother to defragment and load it
a second time just yet as it's likely to fail again anyway.
MFC after: 3 days
is in little endian form. Likewise setting DC_AL_PAR0/DC_AL_PAR1
register expect the address to be in little endian form. For big
endian architectures the address should be swapped to get correct
one.
Change setting/getting ethernet hardware address to big endian
architecture frendly.
Reported by: Robert Murillo ( billypilgrim782001 at yahoo dot com )
Tested by: Robert Murillo ( billypilgrim782001 at yahoo dot com )
- Correct the maxsize parameter when creating the mbufs busdma tag to
reflect the actual requirement of dc(4).
- Move the KASSERT in dc_newbuf() to the right spot.
- Also convert the TX side to take advantage of bus_dmamap_load_mbuf_sg(9).
- Move the comment regarding dc_start_locked() to the right spot.
MFC after: 2 weeks
requiring DC_TX_ALIGN or DC_TX_COALESCE, which was previously done
in dc_start_locked(), into dc_encap().
o In dc_encap():
- If m_defrag() fails just drop the packet like other NIC drivers
do. This should only happen when there's a mbuf shortage, in which
case it was possible to end up with an IFQ full of packets which
couldn't be processed as they couldn't be defragmented as they
were taking up all the mbufs themselves. This includes adjusting
dc_start_locked() to not trying to prepend the mbuf (chain) if
dc_encap() has freed it.
- Likewise, if bus_dmamap_load_mbuf() fails as dc_dma_map_txbuf()
failed, free the mbuf possibly allocated by the above call to
m_defrag() and drop the packet.
o In dc_txeof():
- Don't clear IFF_DRV_OACTIVE unless there are at least 6 free TX
descriptors. Further down the road dc_encap() will bail if there
are only 5 or fewer free TX descriptors, causing dc_start_locked()
to abort and prepend the dequeued mbuf again so it makes no sense
to pretend we could process mbufs again when in fact we won't.
While at it replace this magic 5 with a macro DC_TX_LIST_RSVD.
- Just always assign idx to sc->dc_cdata.dc_tx_cons; it doesn't
make much sense to exclude the idx == sc->dc_cdata.dc_tx_cons
case.
o In dc_dma_map_txbuf() there's no need to set sc->dc_cdata.dc_tx_err
to error if the latter is != 0, bus_dmamap_load_mbuf() already
returns the same error value in that case anyway.
o For less overhead, convert to use bus_dmamap_load_mbuf_sg() for
loading RX buffers.
o Remove some banal and/or outdated comments.
Approved by: re (kensmith)
MFC after: 1 week
