[linux 6.6.y] ethernet: update motorcomm yt6801 to v1.0.31#1630
[linux 6.6.y] ethernet: update motorcomm yt6801 to v1.0.31#1630opsiff wants to merge 1 commit intodeepin-community:linux-6.6.yfrom
Conversation
Signed-off-by: Frank_Sae <Frank.Sae@motor-comm.com>
There was a problem hiding this comment.
Sorry @opsiff, your pull request is larger than the review limit of 150000 diff characters
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
There was a problem hiding this comment.
Pull request overview
Backports an updated Motorcomm YT6801 (fxgmac) Ethernet driver to the linux 6.6.y tree, bumping the in-driver version to v1.0.31 and adding new PHY/WOL/descriptor handling and maintenance refactors.
Changes:
- Update driver version/macros, expand register definitions, and adjust several driver feature toggles/defaults.
- Add/extend PHY link management (forced-mode helper + periodic link polling) and broaden ethtool support (WOL pattern programming, reset, ringparam behavior).
- Rework descriptor/ring/channel handling (optional static alloc, page-mapped RX buffers, per-channel desc init/reset), remove debugfs object from build and add ioctl implementation.
Reviewed changes
Copilot reviewed 12 out of 13 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| drivers/net/ethernet/motorcomm/yt6801/fuxi-os.h | Version bump, new feature macros, struct expansions, new prototypes/includes |
| drivers/net/ethernet/motorcomm/yt6801/fuxi-gmac.h | API/struct updates for rings, hw/desc ops signatures, new fields/features |
| drivers/net/ethernet/motorcomm/yt6801/fuxi-gmac-reg.h | Register/bitfield additions & naming cleanups |
| drivers/net/ethernet/motorcomm/yt6801/fuxi-gmac-phy.c | Forced PHY mode helper, link polling timer, link update flow |
| drivers/net/ethernet/motorcomm/yt6801/fuxi-gmac-pci.c | Suspend/resume/shutdown locking/LED hooks, metadata/license updates |
| drivers/net/ethernet/motorcomm/yt6801/fuxi-gmac-ioctl.c | Add SIOCDEVPRIVATE ioctl-backed diagnostics/register/eFuse access |
| drivers/net/ethernet/motorcomm/yt6801/fuxi-gmac-ethtool.c | WOL pattern setup/config, reset handler, ringparam changes |
| drivers/net/ethernet/motorcomm/yt6801/fuxi-gmac-desc.c | RX page-mapping path, desc init/reset refactor, alloc/free changes |
| drivers/net/ethernet/motorcomm/yt6801/fuxi-gmac-common.c | Init defaults/IRQ scheme tweaks, DMA mask fallback, mutex init, logging |
| drivers/net/ethernet/motorcomm/yt6801/fuxi-efuse.c | eFuse helpers refactor and bounds updates |
| drivers/net/ethernet/motorcomm/yt6801/Makefile | Replace debugfs object with ioctl object in build |
Comments suppressed due to low confidence (2)
drivers/net/ethernet/motorcomm/yt6801/fuxi-gmac-ioctl.c:208
- This ioctl handler provides raw register and eFuse read/write operations but does not enforce any privilege checks. Since SIOCDEVPRIVATE does not require CAP_NET_ADMIN at the core dev_ioctl layer, add an explicit capability check here (or in the driver ndo handler) and return -EPERM for unprivileged callers.
drivers/net/ethernet/motorcomm/yt6801/fuxi-gmac-ioctl.c:241 - After copy_from_user(&pcmd, ...), pcmd.cmd_buf.size_in/size_out are later used to compute in_total_size/in_data_size and drive kzalloc/copy_from_user. Add strict validation immediately after this copy (e.g., size_in >= sizeof(struct ext_ioctl_data), size_in <= FXGMAC_MAX_DBG_BUF_LEN, size_out <= limit) to prevent underflow and unbounded allocations/DoS.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| #ifndef FXGMAC_USE_STATIC_ALLOC | ||
| kfree(pdata->channel_head->tx_ring); | ||
| pdata->channel_head->tx_ring = NULL; | ||
|
|
||
| if (netif_msg_drv(pdata)) | ||
| DPRINTK("free_channels, rx_ring=%p\n", | ||
| pdata->channel_head->rx_ring); | ||
| kfree(pdata->channel_head->rx_ring); | ||
| pdata->channel_head->rx_ring = NULL; | ||
|
|
||
| if (netif_msg_drv(pdata)) | ||
| DPRINTK("free_channels, channel=%p\n", pdata->channel_head); | ||
| kfree(pdata->channel_head); | ||
|
|
||
| #endif | ||
| pdata->channel_head->tx_ring = NULL; | ||
| pdata->channel_head->rx_ring = NULL; | ||
| pdata->channel_head = NULL; |
There was a problem hiding this comment.
In the !FXGMAC_USE_STATIC_ALLOC path, this function kfree()s pdata->channel_head and then immediately dereferences it to NULL out tx_ring/rx_ring, which is a use-after-free. Set ring pointers to NULL before freeing the channel_head, or use a temporary pointer and clear pdata->channel_head first, then free the old allocations.
| pattern[i].mask_info[tip_offset / 8] |= 1 << type_offset % 8; | ||
| tip_offset++; | ||
| pattern[i].mask_info[tip_offset / 8] |= 1 << type_offset % 8; | ||
| tip_offset++; | ||
| pattern[i].mask_info[tip_offset / 8] |= 1 << type_offset % 8; |
There was a problem hiding this comment.
The ARP WOL mask setup uses type_offset in the bit shift for ar_tip bytes. This sets the wrong mask bits and will prevent the ARP pattern from matching (it should use tip_offset for each byte of ar_tip).
| pattern[i].mask_info[tip_offset / 8] |= 1 << type_offset % 8; | |
| tip_offset++; | |
| pattern[i].mask_info[tip_offset / 8] |= 1 << type_offset % 8; | |
| tip_offset++; | |
| pattern[i].mask_info[tip_offset / 8] |= 1 << type_offset % 8; | |
| pattern[i].mask_info[tip_offset / 8] |= 1 << tip_offset % 8; | |
| tip_offset++; | |
| pattern[i].mask_info[tip_offset / 8] |= 1 << tip_offset % 8; | |
| tip_offset++; | |
| pattern[i].mask_info[tip_offset / 8] |= 1 << tip_offset % 8; |
| /* arp type is 0x0800 (big endian) */ | ||
| packet.ar_pro = 0x0 << 8 | 0x08; | ||
| /* 1 is arp request,2 is arp replay, 3 is rarp request, | ||
| * 4 is rarp replay | ||
| */ | ||
| packet.ar_op = 0x1 << 8; |
There was a problem hiding this comment.
The local 'packet' struct is not initialized before copying into pattern_info, so unused bytes (e.g., ethernet header/reverse padding) contain stack garbage and can make the wake pattern unpredictable. Zero-initialize 'packet' before setting fields (and prefer cpu_to_be16() for __be16 fields).
| ip_addr = fxgmac_get_netdev_ip4addr(pdata); | ||
| packet.ar_tip[0] = ip_addr & 0xFF; | ||
| packet.ar_tip[1] = (ip_addr >> 8) & 0xFF; | ||
| packet.ar_tip[2] = (ip_addr >> 16) & 0xFF; | ||
| packet.ar_tip[3] = (ip_addr >> 24) & 0xFF; |
There was a problem hiding this comment.
fxgmac_get_netdev_ip4addr() returns an IPv4 address in network byte order (__be32). Extracting bytes via shifts on 'ip_addr' will be endian-dependent and likely reversed on little-endian hosts, so the ARP target IP pattern may never match. Also guard this call for the case where no IPv4 address is configured (netdev->ip_ptr can be NULL).
| PHY_CR_AUTOENG_LEN, (autoneg ? 1 : 0)); | ||
| regval = FXGMAC_SET_REG_BITS(regval, PHY_CR_DUPLEX_POS, | ||
| PHY_CR_DUPLEX_LEN, (duplex ? 1 : 0)); | ||
| hw_ops->write_ephy_reg(pdata, REG_MII_BMCR, regval); |
There was a problem hiding this comment.
fxgmac_phy_force_duplex() always returns 0 because it never assigns 'ret' from write_ephy_reg(). Capture and return the write result so failures propagate correctly.
| hw_ops->write_ephy_reg(pdata, REG_MII_BMCR, regval); | |
| ret = hw_ops->write_ephy_reg(pdata, REG_MII_BMCR, regval); |
Link: https://gitcode.com/deepin-community/kernel/pull/14