Created attachment 227869 [details] realtek updates Hi, Version number shouldn't start with a "v" character. Luckily pkg considers this an upgrade. Bumped PORTREVISION anyway to avoid confusion, but can be omitted. Also disables arch-specific code to make it run on e.g. ARM. Cheers, Franco
I know I'm picky now but DISTVERSIONPREFIX should be defined before DISTVERSION (not PORTVERSION). https://docs.freebsd.org/en/books/porters-handbook/book/#makefile-versions-ex2
I'm fine with the port changes, but I'm not 100% sure about the driver patch. Konstantin, can you have a look please?
I am not sure about the driver patch either. It creates memory mapping for some registers. Is it safe to just not do that, I do not know. I suspect a port to non-x86 would consist in selecting the right BUS_MAP_SPACE constants, like for arm64 it is probably BUS_SPACE_MAP_LINEAR.
Error case in guarded code also sets re_dash = 0; which disables use of the feature or is set when the NIC doesn't support it in the first place. As far as I understand the dash feature is not in the base FreeBSD driver either.
(In reply to Franco Fichtner from comment #4) But why it is safe to ignore it when the chip supports it? I have no idea what is the 'dash' feature. Also I do not see how the reference to the base driver is relevant. All this discussion is pointless, it should be trivial to change the type if the bus space and test for somebody who has access to an arm machine with the card.
I think you just discarded the probable answer to your question as irrelevant. I don't particularly mind and your implied options to proceed are not what I'm willing to chase because they are irrelevant to the proposed patch. :)
As reference, this is a good article about the Realtek DASH feature: https://www.asrockind.com/en-gb/index.php?route=newsblog/faq&faq_id=79 And this is the equivalent code of the Linux driver: if (tp->DASH) { if (HW_DASH_SUPPORT_TYPE_3(tp)) { u64 CmacMemPhysAddress; void __iomem *cmac_ioaddr = NULL; //map CMAC IO space CmacMemPhysAddress = rtl8125_csi_other_fun_read(tp, 0, 0x18); if (!(CmacMemPhysAddress & BIT_0)) { if (CmacMemPhysAddress & BIT_2) CmacMemPhysAddress |= (u64)rtl8125_csi_other_fun_read(tp, 0, 0x1C) << 32; CmacMemPhysAddress &= 0xFFFFFFF0; /* ioremap MMIO region */ cmac_ioaddr = ioremap(CmacMemPhysAddress, R8125_REGS_SIZE); } if (cmac_ioaddr == NULL) { #if LINUX_VERSION_CODE > KERNEL_VERSION(2,6,0) if (netif_msg_probe(tp)) dev_err(&pdev->dev, "cannot remap CMAC MMIO, aborting\n"); #endif //LINUX_VERSION_CODE > KERNEL_VERSION(2,6,0) } if (cmac_ioaddr == NULL) { tp->DASH = 0; } else { tp->mapped_cmac_ioaddr = cmac_ioaddr; } } eee_enable = 0; } It's true that if the ioremap/bus_space_map fails, then the feature is disabled, so it seems safe to do that. On the ther hand, if we know how to change it, probably it'd be better to do that.
After some cooling down I'd still like to see the version fixed so it matches the pattern ${PORTNAME}-[0-9]*. As for supporting more architectures the proposal really isn't any more bad than the base driver and better than doing nothing. ;) Cheers, Franco
Maintainer timeout? I'm happy to change the patch in any way that doesn't require conjuring up ARM hardware and spending time on "selecting the right BUS_MAP_SPACE constants". Cheers, Franco
A commit in branch main references this bug: URL: https://cgit.FreeBSD.org/ports/commit/?id=6a4e552e40762e99e2b7b15e9542d46fbfd3c6d9 commit 6a4e552e40762e99e2b7b15e9542d46fbfd3c6d9 Author: Alex Dupre <ale@FreeBSD.org> AuthorDate: 2022-01-17 16:11:24 +0000 Commit: Alex Dupre <ale@FreeBSD.org> CommitDate: 2022-01-17 16:14:06 +0000 net/realtek-re-kmod: allow building for other architectures Disable dash support on such archs and fix version number. PR: 258473 Submitted by: Franco Fichtner <franco@opnsense.org> net/realtek-re-kmod/Makefile | 9 +++------ net/realtek-re-kmod/files/patch-optional-dash (new) | 20 ++++++++++++++++++++ 2 files changed, 23 insertions(+), 6 deletions(-)
Committed as is, since it cannot be worse than not supporting it at all, and the patch seems quite safe.
Super, thanks a lot!