Bug 258473 - net/realtek-re-kmod: enable for other architectures and fix version number
Summary: net/realtek-re-kmod: enable for other architectures and fix version number
Status: In Progress
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Only Me
Assignee: Alex Dupre
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2021-09-13 08:55 UTC by Franco Fichtner
Modified: 2021-09-16 15:50 UTC (History)
2 users (show)

See Also:
bugzilla: maintainer-feedback? (ale)


Attachments
realtek updates (1.72 KB, patch)
2021-09-13 08:55 UTC, Franco Fichtner
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Franco Fichtner 2021-09-13 08:55:40 UTC
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
Comment 1 Daniel Engberg freebsd_committer 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 freebsd_committer 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 freebsd_committer 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.
Comment 4 Franco Fichtner 2021-09-14 14:49:36 UTC
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 freebsd_committer 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.
Comment 6 Franco Fichtner 2021-09-15 18:10:40 UTC
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 freebsd_committer 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.