Re: alc(4): add support for 816x devices (and request for review)

[christos%zoulas.com@localhost (Christos Zoulas)]

On Jan 15,  4:18pm, iamleot%gmail.com@localhost (Leonardo Taccari) wrote:
-- Subject: Re: alc(4): add support for 816x devices (and request for review)

| Christos Zoulas writes:
| > Or we can just define IFM_UNKNOWN and use it.
| Done. I have added it to if_alcreg.h. According to <net/if_media.h> that
| the max subtype can be 31. I have chosen this value (that does not
| conflict with the ones defined in <net/if_media.h>... If someday there
| will be more subtype the "31" used for the local IFM_UNKNOWN may become
| problematic! :)).
| 
| So IFM_UNKNOWN is used were appropiate as an argument for alc_aspm().

Ok.

| > Yes, look for PCI_SUBSYS_ID_REG in /usr/src/sys/dev/pci on how to
| > get the subvendor/subdevice with pci_conf_read... Perhaps we should
| > grow convenience functions for those too.
| Thank you a lot for this information! I have added it too without the
| subvendor part that does not seems useful, please correct me if I am
| wrong (FreeBSD's VENDORID_ATHEROS or NetBSD's PCI_VENDOR_ATTANSIC is
| ATM the only vendor for all the ethernet cards supported by alc(4)).

I think that SUBSYS_ID_REG returns a 32 bit value that contains both
the vendor and the product id, so you need to mask the product id to compare
against just the id (look at the macros in pcireg.h).

| I will attach the new patches in this email. Apart the previous two
| parts discussed in this email I have also introduced various cosmetic
| fix in order to reduce the diff with the present if_alc.c (alc_attach()
| variables definitions and initializations).

LGTM...

| It would be nice if someone with an alc(4) ethernet card (especially a
| 813x) can test if I did not broke anything.

That would be great indeed.

| After I will receive an ok I will then open a PR attaching these patches
| with also various man pages improvements (I will do my best in trying to
| avoid wizd(8) invocation ;)).

Ok :-) I will take care of it from there...

christos