Hello Aidan,
I have started to look more closely at the various implementations
(Realtek provided, NetBSD --- indirectly from OpenBSD --- and Linux)
and your patch and I think we are on the same tracks.
Realtek uses the two masks for the macver: 0xFCF00000 to do what is
done (limitedly) with the macver on the current driver, and to set the
MACFG (MAC firmware conFiG?) type. The second mask 0x7C800000 is used
to refine with subconfiguration. I think we should do the same: the
general matching first; and handling the special cases, apart, after
without looping again.
One finds this for debugging in the Realtek driver:
sc->re_8169_MacVersion = (CSR_READ_4(sc, RE_TXCFG)&0x7c800000)>>25;
/* Get bit 26~30 */
sc->re_8169_MacVersion |= ((CSR_READ_4(sc, RE_TXCFG)&0x00800000)!=0 ? 1: 0);
/* Get bit 23 */
DBGPRINT1(sc->re_unit,"8169 Mac Version %d",sc->re_8169_MacVersion);
So all are considered variants of the 8169, and I don't think we
should bother to continue trying to spot a version with enumerations
like:
Macv01 = 0x00000000, /* RTL8169 */
Macv02 = 0x00800000, /* RTL8169S/8110S */
Macv03 = 0x04000000, /* RTL8169S/8110S */
...
but instead use, in a header, the MACFG_[0-9][0-9] used in the Realtek
driver and to let the hardcoded macver as is in a switch, simply
setting a type member in the Ctlr structure. This doesn't add any
complexity and this push the code nearer to what Realtek release, so
it will be more easy to read for people coming after us on the driver.
I think we agree that all the specific "black box" instructions for
the 8125 should be severed from the code. It is not binary firmware
(but in the spirit, it is), so we could put this in the header to keep the
C file humanly understandable (the Realtek if_re.c is almost 2MB in size!).
Since I'm on it, I will add also the 8126, even if I
don't have one, and add the variants:
enum { /* Variants */
Rtl8100e = (0x8136<<16)|0x10EC, /* RTL810[01]E: pci -e */
Rtl8169c = (0x0116<<16)|0x16EC, /* RTL8169C+ (USR997902) */
Rtl8125 = (0x8125<<16)|0x10EC, /* RTL8125 pci-e */
Rtl8126 = (0x8126<<16)|0x10EC, /* RTL8126 pci-e */
Rtl8129 = (0x8129<<16)|0x10EC, /* RTL8129 */
Rtl8139 = (0x8139<<16)|0x10EC, /* RTL8139 */
Rtl8111b = (0x8161<<16)|0x10EC, /* RTL8111/8168/8411: pci-e */
Rtl8168kb = (0x8162<<16)|0x10EC, /* RTL8168KB */
Rtl8169sc = (0x8167<<16)|0x10EC, /* RTL8169SC */
Rtl8168b = (0x8168<<16)|0x10EC, /* RTL8168B: pci-e */
Rtl8169 = (0x8169<<16)|0x10EC, /* RTL8169 */
};
What I'm unclear about at the moment is the Wifi support.
More generally, wouldn't it be more clear to use PCI_VENDOR_ID, PCI_DEVICE_ID,
etc. from a header instead of the hardcoded values? (that may be hard
to grep to find if support is there or not---I missed at first that
there is a U.S. Robotics card with a Realtek chip in the list since
the vendor id is 0x16EC, to be compared with Realtek 0x10EC.)
What do you, and what do other think about this?
I think I will have to "steal" a couple of hours this week every day
to come with something testable at the end of the week.
Best,
T. Laronde
On Fri, Mar 07, 2025 at 09:08:45PM -0800, Aidan K. Wiggins via 9fans wrote:
> Hi Thierry,
>
> > Thank you for the link. I will give it a look this week-end and will
> > go back to you during the next week. Hopefully, joining our efforts,
> > we should manage to get the cards wortking.
>
> Excellent! It will be good to have more work/hardware when it comes to
> these Realtek chips. Overtime, I will be updating the linked file of
> any bugs/cleanups I find, which I have also now moved to
> http://oneiri.one/9front/ether8169.patch for the time being. (One
> problem I have encountered is losing Rx after unplugging the cable for
> a couple of seconds, but this looks well evidenced in commits relating
> to the setting of something called "aldps", which I'll try to get
> going tonight.)
>
> > From a cursory look, there are two distinct uses: one mask to match a
> > macver, and another mask to select a revision, with both setting
> > divergent things. In our present drivers, it seems the two cases are
> > more or less mixed---once again: I'm just starting to discover the
> > whole thing, so I may be wrong.
>
> Yes this is right, and we do mix them. We (like Linux), are just
> making it so that the Mac version _and_ the revision ID identify the
> card (Macv |= RevID); thus having the mask of 0x[7F]CF000000 lets us
> differentiate between tested cards by switching on Macv[0-9]+, like
> the two RTL8125D versions (as opposed to sub-switching elsewhere in
> the code for every time we test for a 8125D).
>
> > Yes, the 8125 specially has a bunch of hard coded commands, given by
> > Realtek, and no explanations. This could have something to do with the
> > mixed support for 10Mb, 100Mb, 1Gb and 2.5Gb. But when it comes to
> > explanations, there are almost none...
>
> Yes :). This is going to be fun.
>
> > I will get back to you after doing some work on the stuff.
>
> I wish you good fortune.
>
> Thank you,
> Aidan
--
Thierry Laronde <tlaronde +AT+ kergis +dot+ com>
http://www.kergis.com/
http://kertex.kergis.com/
Key fingerprint = 0FF7 E906 FBAF FE95 FD89 250D 52B1 AE95 6006 F40C
------------------------------------------
9fans: 9fans
Permalink:
https://9fans.topicbox.com/groups/9fans/T832e366730c74bfa-M3e6548281ab075d38be62ef2
Delivery options: https://9fans.topicbox.com/groups/9fans/subscription
