Bug 258473

Comment 1 Daniel Engberg 2021-09-13 16:03:12 UTC

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

Comment 2 Alex Dupre 2021-09-14 14:23:46 UTC

I'm fine with the port changes, but I'm not 100% sure about the driver patch.

Konstantin, can you have a look please?

Comment 3 Konstantin Belousov 2021-09-14 14:39:59 UTC

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.

Comment 5 Konstantin Belousov 2021-09-15 14:48:42 UTC

(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.  :)

Comment 7 Alex Dupre 2021-09-16 15:50:55 UTC

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

Comment 10 commit-hook 2022-01-17 16:15:33 UTC

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(-)

Comment 11 Alex Dupre 2022-01-17 16:17:05 UTC

Committed as is, since it cannot be worse than not supporting it at all, and the patch seems quite safe.

Super, thanks a lot!