Hi Thierry,
> 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.
I think I understand what you're getting at here: instead of having an
enum constant for every little card, we should be using the MACFG
values as indicated by the result of ANDing with 0xFCF000000, as seen
in re_check_mac_version() of Realtek's BSD source. I think there is
merit in this approach, but it would require changing more of the code
than I thought immediately necessary, which is partially why I just
decided to follow suit in borrowing values from Linux (esp. with it's
large userbase included). With looping in our vetmacv(), I was
attempting to keep some level of guaranteed backwards compatibility,
since I wasn't sure if a more specific (0xFCF) mask would tease out a
new Macv value from an older card that was otherwise previously
supported under the mask 0x7C800000.
> What I'm unclear about at the moment is the Wifi support.
Can you expand on this?
> 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.)
I agree with Ori's approach here, the device IDs are so specific that
they really only need inclusion in their respective drivers. We do,
however, have some vendor IDs in pci.h, so you could have:
#define PCIdev(vid, did) ((vid)<<16|(did))
enum{
Rtl8169 = PCIdev(Vrealtek, 0x8169),
Rtl8169c= PCIdev(0x16ec, 0x0116)
...
}
switch(PCIdev(p->vid, p->did)){
...
}
If it weren't for these non-realtek chips, I think we would just use
switch(did){}.
> 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.
Sounds good :)
Aidan
------------------------------------------
9fans: 9fans
Permalink:
https://9fans.topicbox.com/groups/9fans/T832e366730c74bfa-M1845e8001ee31b4a83f624ab
Delivery options: https://9fans.topicbox.com/groups/9fans/subscription
