[new driver] Microchips LAN743X driver for FreeBSD

[emaste]

Summary

This driver adds support for LAN7430/LAN7431.

What's left to do:

• Multi-buffer packets (RX and TX)
• Multiple RX queues
• Checksum offloading
• Other H/W Supported functions
  • WoL
  • LSO
  • RSS
  • MAC Stats and debugging info
  • VLAN
  • Recieve Packet Filtering

Depends on D19946

Test Plan

Tested by pinging device.

[aryeeteygerald_rogers.com]

Link to Git repository

[emaste]

• rebase on updated D19946 and now-committed D20142
• pass newly-added IFLIB_DRIVER_MEDIA flag from D19946
• apply some style(9)
• add man page

[emaste]

sys/dev/mgb/if_mgb.c

1418–1423

no K&R in new code

[emaste]

some more style(9)

[jhb]

sys/dev/mgb/if_mgb.c

73

sys/param.h is always first and in this case should replace sys/types.h. Also, style is to sort headers and not really break them up into blocks like this.

202

Not your fault, but device_register is horrible. It is completely iflib specific and shouldn't have the generic name it has. Also, it seems there should be a template iflib driver you can inherit from as a template that has the various device_foo methods mapped to iflib_device_foo so you don't have to duplicate all that.

230

So why not use this always?

284

Why are there two driver_t's and two devmethod arrays? Can't there just be one?

399

Normally attach isn't really a hot path such that we sprinkle __builtin_expect(). In FreeBSD we only use that in places where it actually matters (and ideally with some kind of measurement to show that it matters)

639

Much better to export these as sysctl nodes.

1218

Having a constant for this register would be nice along with constants for the subfields, or at least a comment in the header explaining what the subfields are.

1236

Having this be part of a 'alloc_regs' function is somewhat confusing. (pci_enable_busmaster means "I want permission to initiate DMA"). Most drivers just inline this into attach anyway without a subroutine. You could at least move pci_enable_busmaster directly into attach.

1254

Most drivers don't bother doing this on detach.

sys/dev/mgb/if_mgb.h

259

Is this even used? The C file uses a 'pci_vendor_info_t'?

263

Did you mean to define a global variable in all the files that include this?

290

Debugging leftover?

300

These macros do seem to be a bit of an obscure way of dealing with a 'struct mgb_ring_info' defined in the #if 0'd comment.

[emaste]

sys/dev/mgb/if_mgb.h

259

Appears completely unused.

263

Almost certainly not :) That said only if_mgb.c will include if_mgb.h.

Aside, I noticed Linux drivers typically do not prefix register definitions etc. in their driver .h file, presumably because it will only be included by the driver itself.

[emaste]

update some feedback from @jhb

[aryeeteygerald_rogers.com]

sys/dev/mgb/if_mgb.c

284

This is how all the other iflib drivers do it but I think they could be combined.

sys/dev/mgb/if_mgb.h

300

Yes, all of these are obsolete.

They were defined prior to having multiple dma queues. If we revert back to a single dma'd block (which is preferable) then they might come in handy. I defined the macros because I don't think the struct would work quite the way we would want since it requires allocating extra space for the pointer.

[emaste]

D21240 moved ETHER_IS_ZERO to ethernet.h, need to update this. Most likely I will commit the WIP (connected to the build or not) so that changes in the future can update this driver too.