[RFC PATCH 0/4] r8169: support dash

[RFC PATCH 0/4] r8169: support dash

[Hayes Wang]

These patches are used to support dash for RTL8111EP and
RTL8111FP(RTL81117).

Hayes Wang (4):
  r8169: remove the relative code about dash
  r8169: add type2 access functions
  r8169: support CMAC
  r8169: add sysfs for dash

 drivers/net/ethernet/realtek/Makefile     |   2 +-
 drivers/net/ethernet/realtek/r8169.h      |   5 +
 drivers/net/ethernet/realtek/r8169_dash.c | 887 ++++++++++++++++++++++
 drivers/net/ethernet/realtek/r8169_dash.h |  30 +
 drivers/net/ethernet/realtek/r8169_main.c | 349 +++++++--
 5 files changed, 1196 insertions(+), 77 deletions(-)
 create mode 100644 drivers/net/ethernet/realtek/r8169_dash.c
 create mode 100644 drivers/net/ethernet/realtek/r8169_dash.h

-- 
2.31.1

[RFC PATCH 1/4] r8169: remove the relative code about dash

[Hayes Wang]

Remove the relative code of dash, except for RTL8111DP.

The flow of initialization is different when considering supporting the
dash, so remove the relative code first.

The RTL8111DP has the different design and is stopped to maintain, so
keep the current behavior is enough.

Signed-off-by: Hayes Wang <hayeswang@realtek.com>
---
 drivers/net/ethernet/realtek/r8169_main.c | 70 ++---------------------
 1 file changed, 5 insertions(+), 65 deletions(-)

diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
index 3d6843332c77..fd295bcd125c 100644
--- a/drivers/net/ethernet/realtek/r8169_main.c
+++ b/drivers/net/ethernet/realtek/r8169_main.c
@@ -245,10 +245,6 @@ enum rtl_registers {
 	FuncEvent	= 0xf0,
 	FuncEventMask	= 0xf4,
 	FuncPresetState	= 0xf8,
-	IBCR0           = 0xf8,
-	IBCR2           = 0xf9,
-	IBIMR0          = 0xfa,
-	IBISR0          = 0xfb,
 	FuncForceEvent	= 0xfc,
 };
 
@@ -1127,67 +1123,18 @@ DECLARE_RTL_COND(rtl_dp_ocp_read_cond)
 	return r8168dp_ocp_read(tp, reg) & 0x00000800;
 }
 
-DECLARE_RTL_COND(rtl_ep_ocp_read_cond)
-{
-	return r8168ep_ocp_read(tp, 0x124) & 0x00000001;
-}
-
-DECLARE_RTL_COND(rtl_ocp_tx_cond)
-{
-	return RTL_R8(tp, IBISR0) & 0x20;
-}
-
-static void rtl8168ep_stop_cmac(struct rtl8169_private *tp)
-{
-	RTL_W8(tp, IBCR2, RTL_R8(tp, IBCR2) & ~0x01);
-	rtl_loop_wait_high(tp, &rtl_ocp_tx_cond, 50000, 2000);
-	RTL_W8(tp, IBISR0, RTL_R8(tp, IBISR0) | 0x20);
-	RTL_W8(tp, IBCR0, RTL_R8(tp, IBCR0) & ~0x01);
-}
-
 static void rtl8168dp_driver_start(struct rtl8169_private *tp)
 {
 	r8168dp_oob_notify(tp, OOB_CMD_DRIVER_START);
 	rtl_loop_wait_high(tp, &rtl_dp_ocp_read_cond, 10000, 10);
 }
 
-static void rtl8168ep_driver_start(struct rtl8169_private *tp)
-{
-	r8168ep_ocp_write(tp, 0x01, 0x180, OOB_CMD_DRIVER_START);
-	r8168ep_ocp_write(tp, 0x01, 0x30, r8168ep_ocp_read(tp, 0x30) | 0x01);
-	rtl_loop_wait_high(tp, &rtl_ep_ocp_read_cond, 10000, 10);
-}
-
-static void rtl8168_driver_start(struct rtl8169_private *tp)
-{
-	if (tp->dash_type == RTL_DASH_DP)
-		rtl8168dp_driver_start(tp);
-	else
-		rtl8168ep_driver_start(tp);
-}
-
 static void rtl8168dp_driver_stop(struct rtl8169_private *tp)
 {
 	r8168dp_oob_notify(tp, OOB_CMD_DRIVER_STOP);
 	rtl_loop_wait_low(tp, &rtl_dp_ocp_read_cond, 10000, 10);
 }
 
-static void rtl8168ep_driver_stop(struct rtl8169_private *tp)
-{
-	rtl8168ep_stop_cmac(tp);
-	r8168ep_ocp_write(tp, 0x01, 0x180, OOB_CMD_DRIVER_STOP);
-	r8168ep_ocp_write(tp, 0x01, 0x30, r8168ep_ocp_read(tp, 0x30) | 0x01);
-	rtl_loop_wait_low(tp, &rtl_ep_ocp_read_cond, 10000, 10);
-}
-
-static void rtl8168_driver_stop(struct rtl8169_private *tp)
-{
-	if (tp->dash_type == RTL_DASH_DP)
-		rtl8168dp_driver_stop(tp);
-	else
-		rtl8168ep_driver_stop(tp);
-}
-
 static bool r8168dp_check_dash(struct rtl8169_private *tp)
 {
 	u16 reg = rtl8168_get_ocp_reg(tp);
@@ -3227,8 +3174,6 @@ static void rtl_hw_start_8168h_1(struct rtl8169_private *tp)
 
 static void rtl_hw_start_8168ep(struct rtl8169_private *tp)
 {
-	rtl8168ep_stop_cmac(tp);
-
 	rtl_set_fifo_size(tp, 0x08, 0x10, 0x02, 0x06);
 	rtl8168g_set_pause_thresholds(tp, 0x2f, 0x5f);
 
@@ -3324,8 +3269,6 @@ static void rtl_hw_start_8117(struct rtl8169_private *tp)
 	};
 	int rg_saw_cnt;
 
-	rtl8168ep_stop_cmac(tp);
-
 	/* disable aspm and clock request before access ephy */
 	rtl_hw_aspm_clkreq_enable(tp, false);
 	rtl_ephy_init(tp, e_info_8117);
@@ -4968,8 +4911,8 @@ static void rtl_remove_one(struct pci_dev *pdev)
 
 	unregister_netdev(tp->dev);
 
-	if (tp->dash_type != RTL_DASH_NONE)
-		rtl8168_driver_stop(tp);
+	if (tp->dash_type == RTL_DASH_DP)
+		rtl8168dp_driver_stop(tp);
 
 	rtl_release_firmware(tp);
 
@@ -5161,10 +5104,7 @@ static void rtl_hw_init_8125(struct rtl8169_private *tp)
 static void rtl_hw_initialize(struct rtl8169_private *tp)
 {
 	switch (tp->mac_version) {
-	case RTL_GIGA_MAC_VER_49 ... RTL_GIGA_MAC_VER_53:
-		rtl8168ep_stop_cmac(tp);
-		fallthrough;
-	case RTL_GIGA_MAC_VER_40 ... RTL_GIGA_MAC_VER_48:
+	case RTL_GIGA_MAC_VER_40 ... RTL_GIGA_MAC_VER_53:
 		rtl_hw_init_8168g(tp);
 		break;
 	case RTL_GIGA_MAC_VER_60 ... RTL_GIGA_MAC_VER_63:
@@ -5443,9 +5383,9 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 			    jumbo_max, tp->mac_version <= RTL_GIGA_MAC_VER_06 ?
 			    "ok" : "ko");
 
-	if (tp->dash_type != RTL_DASH_NONE) {
+	if (tp->dash_type == RTL_DASH_DP) {
 		netdev_info(dev, "DASH enabled\n");
-		rtl8168_driver_start(tp);
+		rtl8168dp_driver_start(tp);
 	}
 
 	if (pci_dev_run_wake(pdev))
-- 
2.31.1

[RFC PATCH 2/4] r8169: add type2 access functions

[Hayes Wang]

The type2 functions are used to access the OOB registers of RTL8111EP
and RTL8111FP(RTL8117). The OOB registers are used to control the
settings about dash.

Signed-off-by: Hayes Wang <hayeswang@realtek.com>
---
 drivers/net/ethernet/realtek/r8169.h      |  3 ++
 drivers/net/ethernet/realtek/r8169_main.c | 39 +++++++++++++++++++++++
 2 files changed, 42 insertions(+)

diff --git a/drivers/net/ethernet/realtek/r8169.h b/drivers/net/ethernet/realtek/r8169.h
index 8da4b66b71b5..7db647b4796f 100644
--- a/drivers/net/ethernet/realtek/r8169.h
+++ b/drivers/net/ethernet/realtek/r8169.h
@@ -77,3 +77,6 @@ u16 rtl8168h_2_get_adc_bias_ioffset(struct rtl8169_private *tp);
 u8 rtl8168d_efuse_read(struct rtl8169_private *tp, int reg_addr);
 void r8169_hw_phy_config(struct rtl8169_private *tp, struct phy_device *phydev,
 			 enum mac_version ver);
+
+u32 r8168_type2_read(struct rtl8169_private *tp, u32 addr);
+void r8168_type2_write(struct rtl8169_private *tp, u8 mask, u32 addr, u32 val);
diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
index fd295bcd125c..eb56b91fe41b 100644
--- a/drivers/net/ethernet/realtek/r8169_main.c
+++ b/drivers/net/ethernet/realtek/r8169_main.c
@@ -1071,6 +1071,45 @@ static u16 rtl_ephy_read(struct rtl8169_private *tp, int reg_addr)
 		RTL_R32(tp, EPHYAR) & EPHYAR_DATA_MASK : ~0;
 }
 
+static u32 r8168_dash_adjust_addr(struct rtl8169_private *tp, u32 addr)
+{
+	switch (tp->mac_version) {
+	case RTL_GIGA_MAC_VER_49 ... RTL_GIGA_MAC_VER_51:
+		return ((addr & 0xf000) << 8) | (addr & 0xfff);
+	case RTL_GIGA_MAC_VER_52 ... RTL_GIGA_MAC_VER_53:
+		return ((addr & 0xfff000) << 6) | (addr & 0xfff);
+	default:
+		WARN_ON_ONCE(1);
+		return (addr & 0xfff);
+	}
+}
+
+u32 r8168_type2_read(struct rtl8169_private *tp, u32 addr)
+{
+	u32 cmd = ERIAR_READ_CMD | ERIAR_OOB | ERIAR_MASK_1111;
+
+	cmd |= r8168_dash_adjust_addr(tp, addr);
+	RTL_W32(tp, ERIAR, cmd);
+
+	return rtl_loop_wait_high(tp, &rtl_eriar_cond, 100, 100) ?
+		RTL_R32(tp, ERIDR) : ~0;
+}
+
+void r8168_type2_write(struct rtl8169_private *tp, u8 mask, u32 addr, u32 val)
+{
+	u32 cmd = ERIAR_WRITE_CMD | ERIAR_OOB |
+		  ((u32)mask & 0x0f) << ERIAR_MASK_SHIFT;
+
+	if (WARN(addr & 3 || !mask, "addr: 0x%x, mask: 0x%08x\n", addr, mask))
+		return;
+
+	RTL_W32(tp, ERIDR, val);
+	cmd |= r8168_dash_adjust_addr(tp, addr);
+	RTL_W32(tp, ERIAR, cmd);
+
+	rtl_loop_wait_low(tp, &rtl_eriar_cond, 100, 100);
+}
+
 static u32 r8168dp_ocp_read(struct rtl8169_private *tp, u16 reg)
 {
 	RTL_W32(tp, OCPAR, 0x0fu << 12 | (reg & 0x0fff));
-- 
2.31.1

[RFC PATCH 3/4] r8169: support CMAC

[Hayes Wang]

Support CMAC for RTL8111EP and RTL8111FP.

CMAC is the major interface to configure the firmware when dash is
enabled.

Signed-off-by: Hayes Wang <hayeswang@realtek.com>
---
 drivers/net/ethernet/realtek/Makefile     |   2 +-
 drivers/net/ethernet/realtek/r8169.h      |   2 +
 drivers/net/ethernet/realtek/r8169_dash.c | 756 ++++++++++++++++++++++
 drivers/net/ethernet/realtek/r8169_dash.h |  22 +
 drivers/net/ethernet/realtek/r8169_main.c |  54 +-
 5 files changed, 824 insertions(+), 12 deletions(-)
 create mode 100644 drivers/net/ethernet/realtek/r8169_dash.c
 create mode 100644 drivers/net/ethernet/realtek/r8169_dash.h

diff --git a/drivers/net/ethernet/realtek/Makefile b/drivers/net/ethernet/realtek/Makefile
index 2e1d78b106b0..8f3372ee92ff 100644
--- a/drivers/net/ethernet/realtek/Makefile
+++ b/drivers/net/ethernet/realtek/Makefile
@@ -6,5 +6,5 @@
 obj-$(CONFIG_8139CP) += 8139cp.o
 obj-$(CONFIG_8139TOO) += 8139too.o
 obj-$(CONFIG_ATP) += atp.o
-r8169-objs += r8169_main.o r8169_firmware.o r8169_phy_config.o
+r8169-objs += r8169_main.o r8169_firmware.o r8169_phy_config.o r8169_dash.o
 obj-$(CONFIG_R8169) += r8169.o
diff --git a/drivers/net/ethernet/realtek/r8169.h b/drivers/net/ethernet/realtek/r8169.h
index 7db647b4796f..b75484a2a580 100644
--- a/drivers/net/ethernet/realtek/r8169.h
+++ b/drivers/net/ethernet/realtek/r8169.h
@@ -78,5 +78,7 @@ u8 rtl8168d_efuse_read(struct rtl8169_private *tp, int reg_addr);
 void r8169_hw_phy_config(struct rtl8169_private *tp, struct phy_device *phydev,
 			 enum mac_version ver);
 
+u32 r8168ep_ocp_read(struct rtl8169_private *tp, u16 reg);
 u32 r8168_type2_read(struct rtl8169_private *tp, u32 addr);
+void r8168ep_ocp_write(struct rtl8169_private *tp, u8 mask, u16 reg, u32 data);
 void r8168_type2_write(struct rtl8169_private *tp, u8 mask, u32 addr, u32 val);
diff --git a/drivers/net/ethernet/realtek/r8169_dash.c b/drivers/net/ethernet/realtek/r8169_dash.c
new file mode 100644
index 000000000000..acee7519e9f1
--- /dev/null
+++ b/drivers/net/ethernet/realtek/r8169_dash.c
@@ -0,0 +1,756 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+#include <linux/pci.h>
+#include <linux/workqueue.h>
+#include <linux/rtnetlink.h>
+#include <linux/string.h>
+#include "r8169.h"
+#include "r8169_dash.h"
+
+#define IBCR0				0xf8
+#define IBCR2				0xf9
+#define IBIMR0				0xfa
+#define IBISR0				0xfb
+
+#define RTXS_LS				BIT(12)
+#define RTXS_FS				BIT(13)
+#define RTXS_EOR			BIT(14)
+#define RTXS_OWN			BIT(15)
+
+#define DASH_ISR_ROK			BIT(0)
+#define DASH_ISR_RDU			BIT(1)
+#define DASH_ISR_TOK			BIT(2)
+#define DASH_ISR_TDU			BIT(3)
+#define DASH_ISR_TX_DISABLE_IDLE	BIT(5)
+#define DASH_ISR_RX_DISABLE_IDLE	BIT(6)
+
+#define CMAC_DESC_NUM		4
+#define CMAC_DESC_SIZE		(CMAC_DESC_NUM * sizeof(struct cmac_desc))
+#define CMAC_TIMEOUT		(HZ * 5)
+
+#define OOB_CMD_DRIVER_START		0x05
+#define OOB_CMD_DRIVER_STOP		0x06
+#define OOB_CMD_CMAC_STOP		0x25
+#define OOB_CMD_CMAC_INIT		0x26
+#define OOB_CMD_CMAC_RESET		0x2a
+
+enum dash_cmac_state {
+	CMAC_STATE_STOP = 0,
+	CMAC_STATE_READY,
+	CMAC_STATE_RUNNING,
+};
+
+enum dash_flag {
+	DASH_FLAG_CHECK_CMAC = 0,
+	DASH_FLAG_MAX
+};
+
+#pragma pack(push)
+#pragma pack(1)
+
+struct cmac_desc {
+	__le16 length;
+	__le16 status;
+	__le32 resv;
+	__le64 dma_addr;
+};
+
+struct oob_hdr {
+	__le32 len;
+	u8 type;
+	u8 flag;
+	u8 host_req;
+	u8 res;
+};
+
+#pragma pack(pop)
+
+struct dash_tx_info {
+	u8 *buf;
+	u32 len;
+	bool ack;
+};
+
+struct rtl_dash {
+	struct rtl8169_private *tp;
+	struct pci_dev *pdev_cmac;
+	void __iomem *cmac_ioaddr;
+	struct cmac_desc *tx_desc, *rx_desc;
+	struct page *tx_buf, *rx_buf;
+	struct dash_tx_info tx_info[CMAC_DESC_NUM];
+	struct tasklet_struct tl;
+	struct completion cmac_tx, cmac_rx, fw_ack;
+	spinlock_t cmac_lock; /* spin lock for CMAC */
+	dma_addr_t tx_desc_dma, rx_desc_dma;
+	enum rtl_dash_type hw_dash_ver;
+	enum dash_cmac_state cmac_state;
+
+	struct {
+		DECLARE_BITMAP(flags, DASH_FLAG_MAX);
+		struct work_struct work;
+	} wk;
+
+	/* u64 */
+
+	/* u32 */
+	u32 tx_free, tx_used;
+	u32 rx_cur;
+
+	/* u16 */
+
+	/* u8 */
+};
+
+/* cmac write/read MMIO register */
+#define RTL_CMAC_W8(d, reg, v8)		writeb((v8), (d)->cmac_ioaddr + (reg))
+#define RTL_CMAC_W16(d, reg, v16)	writew((v16), (d)->cmac_ioaddr + (reg))
+#define RTL_CMAC_W32(d, reg, v32)	writel((v32), (d)->cmac_ioaddr + (reg))
+#define RTL_CMAC_R8(d, reg)		readb((d)->cmac_ioaddr + (reg))
+#define RTL_CMAC_R16(d, reg)		readw((d)->cmac_ioaddr + (reg))
+#define RTL_CMAC_R32(d, reg)		readl((d)->cmac_ioaddr + (reg))
+
+struct rtl_dash_cond {
+	bool (*check)(struct rtl_dash *dash);
+	const char *msg;
+};
+
+static bool rtl_dash_loop_wait(struct rtl_dash *dash,
+			       const struct rtl_dash_cond *c,
+			       unsigned long usecs, int n, bool high)
+{
+	int i;
+
+	for (i = 0; i < n; i++) {
+		if (c->check(dash) == high)
+			return true;
+		fsleep(usecs);
+	}
+
+	if (net_ratelimit())
+		dev_err(&dash->pdev_cmac->dev,
+			"%s == %d (loop: %d, delay: %lu).\n",
+			c->msg, !high, n, usecs);
+	return false;
+}
+
+static bool rtl_dash_loop_wait_high(struct rtl_dash *dash,
+				    const struct rtl_dash_cond *c,
+				    unsigned long d, int n)
+{
+	return rtl_dash_loop_wait(dash, c, d, n, true);
+}
+
+static bool rtl_dash_loop_wait_low(struct rtl_dash *dash,
+				   const struct rtl_dash_cond *c,
+				   unsigned long d, int n)
+{
+	return rtl_dash_loop_wait(dash, c, d, n, false);
+}
+
+#define DECLARE_RTL_DASH_COND(name)			\
+static bool name ## _check(struct rtl_dash *dash);	\
+							\
+static const struct rtl_dash_cond name = {		\
+	.check	= name ## _check,			\
+	.msg	= #name					\
+};							\
+							\
+static bool name ## _check(struct rtl_dash *dash)
+
+static inline void rtl_dash_intr_en(struct rtl_dash *dash)
+{
+	RTL_CMAC_W8(dash, IBIMR0, DASH_ISR_ROK | DASH_ISR_RDU | DASH_ISR_TOK |
+				  DASH_ISR_TDU | DASH_ISR_RX_DISABLE_IDLE);
+}
+
+static void dash_tx_bottom(struct rtl_dash *dash)
+{
+	u32 tx_used = dash->tx_used;
+
+	while (tx_used != dash->tx_free) {
+		struct device *d = &dash->pdev_cmac->dev;
+		u32 index = tx_used % CMAC_DESC_NUM;
+		struct cmac_desc *tx_desc = dash->tx_desc + index;
+		struct dash_tx_info *info = dash->tx_info + index;
+
+		if (le16_to_cpu(tx_desc->status) & RTXS_OWN)
+			break;
+
+		dma_unmap_single(d, le64_to_cpu(tx_desc->dma_addr), info->len,
+				 DMA_TO_DEVICE);
+
+		if (!info->ack) {
+			complete(&dash->cmac_tx);
+			dev_dbg(d, "CMAC send TX OK\n");
+		}
+
+		info->len = 0;
+		tx_used++;
+	}
+
+	if (dash->tx_used != tx_used)
+		dash->tx_used = tx_used;
+}
+
+static int cmac_start_xmit(struct rtl_dash *dash, u8 *data, u32 size, bool ack)
+{
+	struct device *d = &dash->pdev_cmac->dev;
+	u32 index = dash->tx_free % CMAC_DESC_NUM;
+	struct cmac_desc *desc = dash->tx_desc + index;
+	struct dash_tx_info *info = dash->tx_info + index;
+	dma_addr_t mapping;
+	int ret;
+
+	if (dash->cmac_state != CMAC_STATE_RUNNING)
+		return -ENETDOWN;
+
+	if ((dash->tx_free - dash->tx_used) >= CMAC_DESC_NUM)
+		return -EBUSY;
+
+	if (IS_ERR_OR_NULL(data) || size > CMAC_BUF_SIZE)
+		return -EINVAL;
+
+	if (unlikely(le16_to_cpu(desc->status) & RTXS_OWN))
+		return -EFAULT;
+
+	memcpy(info->buf, data, size);
+	if (ack) {
+		struct oob_hdr *hdr = (struct oob_hdr *)info->buf;
+
+		hdr->len = 0;
+	}
+
+	mapping = dma_map_single(d, info->buf, size, DMA_TO_DEVICE);
+	ret = dma_mapping_error(d, mapping);
+	if (unlikely(ret)) {
+		dev_err(d, "Failed to map TX DMA!\n");
+		goto err;
+	}
+
+	info->len = size;
+	info->ack = ack;
+
+	desc->dma_addr = cpu_to_le64(mapping);
+	desc->length = cpu_to_le16(size);
+	dma_wmb();
+	desc->status |= cpu_to_le16(RTXS_OWN);
+	dma_wmb();
+	RTL_CMAC_W8(dash, IBCR2, RTL_CMAC_R8(dash, IBCR2) | BIT(1));
+
+	dash->tx_free++;
+	ret = size;
+
+err:
+	return ret;
+}
+
+static int dash_rx_data(struct rtl_dash *dash, u8 *target, u32 size)
+{
+	u32 cur = dash->rx_cur;
+	int i, ret = 0;
+
+	for (i = 0; i < CMAC_DESC_NUM; i++) {
+		u32 index = cur % CMAC_DESC_NUM;
+		struct cmac_desc *desc = dash->rx_desc + index;
+		struct device *d = &dash->pdev_cmac->dev;
+		struct oob_hdr *hdr;
+		dma_addr_t mapping;
+		u8 *addr;
+		u16 pkt_size;
+
+		if (le16_to_cpu(desc->status) & RTXS_OWN)
+			break;
+
+		addr = page_address(dash->rx_buf) + index * CMAC_BUF_SIZE;
+		pkt_size = le16_to_cpu(desc->length);
+
+		mapping = le64_to_cpu(desc->dma_addr);
+		if (mapping) {
+			dma_unmap_single(d, mapping, pkt_size, DMA_FROM_DEVICE);
+			desc->dma_addr = 0;
+		}
+
+		if (pkt_size < sizeof(*hdr))
+			goto re_map;
+
+		hdr = (struct oob_hdr *)addr;
+		switch (hdr->host_req) {
+		case 0x91:
+			dev_dbg(d, "CMAC RX DATA\n");
+			if (mapping &&
+			    cmac_start_xmit(dash, addr, sizeof(struct oob_hdr),
+					    true) < 0)
+				dev_err(d, "send ACK fail\n");
+
+			complete(&dash->cmac_rx);
+
+			if (!target) {
+				ret = -EDESTADDRREQ;
+				break;
+			} else if (size < pkt_size) {
+				ret = -EMSGSIZE;
+				break;
+			}
+			memcpy(target, addr, pkt_size);
+			ret = pkt_size;
+			break;
+		case 0x92:
+			dev_dbg(d, "CMAC RX ACK\n");
+			complete(&dash->fw_ack);
+			break;
+		default:
+			break;
+		}
+
+		if (ret < 0)
+			break;
+
+re_map:
+		mapping = dma_map_single(d, addr, CMAC_BUF_SIZE,
+					 DMA_FROM_DEVICE);
+		if (unlikely(dma_mapping_error(d, mapping))) {
+			dev_err(d, "Failed to map RX DMA!\n");
+			desc->length = 0;
+			break;
+		}
+
+		desc->dma_addr = cpu_to_le64(mapping);
+		desc->length = cpu_to_le16(CMAC_BUF_SIZE);
+		dma_wmb();
+		desc->status |= cpu_to_le16(RTXS_OWN);
+
+		cur++;
+
+		if (ret == pkt_size)
+			break;
+	}
+
+	if (dash->rx_cur != cur)
+		dash->rx_cur = cur;
+
+	return ret;
+}
+
+static void dash_half(struct tasklet_struct *t)
+{
+	struct rtl_dash *dash = from_tasklet(dash, t, tl);
+
+	dash_tx_bottom(dash);
+
+	spin_lock(&dash->cmac_lock);
+	dash_rx_data(dash, NULL, 0);
+	spin_unlock(&dash->cmac_lock);
+
+	rtl_dash_intr_en(dash);
+}
+
+static void rtl_dash_change_cmac_state(struct rtl_dash *dash, u8 state);
+
+DECLARE_RTL_DASH_COND(rtl_cmac_tx_cond)
+{
+	return RTL_CMAC_R8(dash, IBISR0) & DASH_ISR_TX_DISABLE_IDLE;
+}
+
+static void rtl_cmac_disable(struct rtl_dash *dash)
+{
+	u8 status;
+	int i;
+
+	if (dash->cmac_state == CMAC_STATE_RUNNING)
+		tasklet_disable(&dash->tl);
+
+	dash->cmac_state = CMAC_STATE_STOP;
+
+	status = RTL_CMAC_R8(dash, IBCR2);
+	if (status & 0x01) {
+		RTL_CMAC_W8(dash, IBCR2, status & ~0x01);
+		rtl_dash_loop_wait_high(dash, &rtl_cmac_tx_cond, 5000, 2000);
+	}
+
+	status = RTL_CMAC_R8(dash, IBCR0);
+	if (status & 0x01)
+		RTL_CMAC_W8(dash, IBCR0, status & ~0x01);
+
+	for (i = 0; i < CMAC_DESC_NUM; i++) {
+		struct device *d = &dash->pdev_cmac->dev;
+		struct cmac_desc *tx_desc = dash->tx_desc + i;
+		struct cmac_desc *rx_desc = dash->rx_desc + i;
+
+		if (dash->tx_info[i].len)
+			dma_unmap_single(d, le64_to_cpu(tx_desc->dma_addr),
+					 dash->tx_info[i].len, DMA_TO_DEVICE);
+
+		if (rx_desc->dma_addr)
+			dma_unmap_single(d, le64_to_cpu(rx_desc->dma_addr),
+					 CMAC_BUF_SIZE, DMA_TO_DEVICE);
+	}
+
+	memset(dash->tx_desc, 0, CMAC_DESC_SIZE);
+	memset(dash->rx_desc, 0, CMAC_DESC_SIZE);
+	memset(dash->tx_info, 0, sizeof(dash->tx_info));
+
+	RTL_CMAC_W8(dash, IBIMR0, 0);
+	RTL_CMAC_W8(dash, IBISR0, RTL_CMAC_R8(dash, IBISR0));
+}
+
+static int rtl_cmac_enable(struct rtl_dash *dash)
+{
+	u32 desc_addr;
+	int i;
+
+	for (i = 0; i < CMAC_DESC_NUM; i++) {
+		struct dash_tx_info *info = dash->tx_info + i;
+		struct cmac_desc *rx_desc = dash->rx_desc + i;
+		struct cmac_desc *tx_desc = dash->tx_desc + i;
+		struct device *d = &dash->pdev_cmac->dev;
+		dma_addr_t mapping;
+		u8 *addr;
+		u16 ops_rx, ops_tx;
+
+		info->buf = page_address(dash->tx_buf) + i * CMAC_BUF_SIZE;
+		info->len = 0;
+		info->ack = false;
+
+		addr = page_address(dash->rx_buf) + i * CMAC_BUF_SIZE;
+		mapping = dma_map_single(d, addr, CMAC_BUF_SIZE,
+					 DMA_FROM_DEVICE);
+		if (unlikely(dma_mapping_error(d, mapping))) {
+			dev_err(d, "Failed to map RX DMA!\n");
+			rtl_dash_change_cmac_state(dash, OOB_CMD_CMAC_STOP);
+			return -ENOMEM;
+		}
+
+		rx_desc->dma_addr = cpu_to_le64(mapping);
+		rx_desc->length = CMAC_BUF_SIZE;
+		rx_desc->resv = 0;
+
+		if (i == (CMAC_DESC_NUM - 1)) {
+			ops_rx = RTXS_OWN | RTXS_EOR;
+			ops_tx = RTXS_FS | RTXS_LS | RTXS_EOR;
+		} else {
+			ops_rx = RTXS_OWN;
+			ops_tx = RTXS_FS | RTXS_LS;
+		}
+
+		rx_desc->status = cpu_to_le16(ops_rx);
+		tx_desc->status = cpu_to_le16(ops_tx);
+	}
+
+	dash->tx_free = 0;
+	dash->tx_used = 0;
+	dash->rx_cur = 0;
+
+	switch (dash->hw_dash_ver) {
+	case RTL_DASH_EP:
+		desc_addr = 0x890;
+		break;
+	case RTL_DASH_FP:
+		desc_addr = 0xf20090;
+		break;
+	default:
+		desc_addr = 0xf20090;
+		WARN_ON_ONCE(1);
+		break;
+	}
+
+	r8168_type2_write(dash->tp, 0xf, desc_addr,
+			  dash->rx_desc_dma & DMA_BIT_MASK(32));
+	r8168_type2_write(dash->tp, 0xf, desc_addr + 4,
+			  dash->rx_desc_dma >> 32);
+	r8168_type2_write(dash->tp, 0xf, desc_addr + 8,
+			  dash->tx_desc_dma & DMA_BIT_MASK(32));
+	r8168_type2_write(dash->tp, 0xf, desc_addr + 12,
+			  dash->tx_desc_dma >> 32);
+
+	RTL_CMAC_W8(dash, IBCR2, RTL_CMAC_R8(dash, IBCR2) | 0x01);
+	RTL_CMAC_W8(dash, IBCR0, RTL_CMAC_R8(dash, IBCR0) | 0x01);
+
+	tasklet_enable(&dash->tl);
+	dash->cmac_state = CMAC_STATE_RUNNING;
+
+	rtl_dash_intr_en(dash);
+
+	return 0;
+}
+
+static void rtl_dash_oob_notify(struct rtl_dash *dash, u8 cmd)
+{
+	struct rtl8169_private *tp = dash->tp;
+
+	r8168ep_ocp_write(tp, 0x01, 0x180, cmd);
+	r8168ep_ocp_write(tp, 0x01, 0x30, r8168ep_ocp_read(tp, 0x30) | 0x01);
+}
+
+static void rtl_cmac_hw_reset(struct rtl_dash *dash)
+{
+	struct rtl8169_private *tp = dash->tp;
+	u32 TmpUlong;
+
+	TmpUlong = r8168ep_ocp_read(tp, 0x150);
+	r8168ep_ocp_write(tp, 0xf, 0x150, TmpUlong | BIT(5));
+
+	switch (dash->hw_dash_ver) {
+	case RTL_DASH_EP:
+		r8168ep_ocp_write(tp, 0xf, 0x150, TmpUlong & ~BIT(5));
+		RTL_CMAC_W8(dash, IBISR0,
+			    RTL_CMAC_R8(dash, IBISR0) | DASH_ISR_ROK);
+
+		TmpUlong = r8168ep_ocp_read(tp, 0x80c);
+		r8168ep_ocp_write(tp, 0xf, 0x80c, TmpUlong | BIT(24));
+		break;
+	case RTL_DASH_FP:
+		fsleep(1);
+		RTL_CMAC_W8(dash, IBISR0,
+			    RTL_CMAC_R8(dash, IBISR0) | DASH_ISR_ROK);
+		break;
+	default:
+		break;
+	}
+
+	dash->cmac_state = CMAC_STATE_READY;
+}
+
+static void rtl_dash_change_cmac_state(struct rtl_dash *dash, u8 state)
+{
+	switch (state) {
+	case OOB_CMD_CMAC_INIT:
+		if (likely(dash->cmac_state != CMAC_STATE_RUNNING)) {
+			rtl_cmac_hw_reset(dash);
+			break;
+		}
+		state = OOB_CMD_CMAC_STOP;
+		WARN_ON_ONCE(1);
+		fallthrough;
+	case OOB_CMD_CMAC_STOP:
+		rtl_cmac_disable(dash);
+		break;
+
+	case OOB_CMD_CMAC_RESET:
+	default:
+		WARN_ON_ONCE(1);
+		return;
+	}
+
+	rtl_dash_oob_notify(dash, state);
+}
+
+static void rtl_dash_cmac_reset_routine(struct rtl_dash *dash)
+{
+	u32 reg, state;
+
+	switch (dash->hw_dash_ver) {
+	case RTL_DASH_EP:
+		reg = 0x2c20;
+		break;
+	case RTL_DASH_FP:
+		reg = 0xf80420;
+		break;
+	default:
+		WARN_ON_ONCE(1);
+		reg = 0xf80420;
+		break;
+	}
+
+	state = r8168_type2_read(dash->tp, reg);
+	r8168_type2_write(dash->tp, 0xf, reg, 0);
+
+	switch (state) {
+	case OOB_CMD_CMAC_RESET:
+		dev_dbg(&dash->pdev_cmac->dev, "OOB_CMD_CMAC_RESET\n");
+		rtl_dash_change_cmac_state(dash, OOB_CMD_CMAC_STOP);
+		break;
+	case OOB_CMD_CMAC_STOP:
+		dev_dbg(&dash->pdev_cmac->dev, "OOB_CMD_CMAC_STOP\n");
+		rtl_dash_change_cmac_state(dash, OOB_CMD_CMAC_INIT);
+		break;
+	case OOB_CMD_CMAC_INIT:
+		dev_dbg(&dash->pdev_cmac->dev, "OOB_CMD_CMAC_INIT\n");
+		rtl_cmac_enable(dash);
+		break;
+	default:
+		WARN_ON_ONCE(1);
+		return;
+	}
+}
+
+static void rtl_dash_schedule_work(struct rtl_dash *dash, enum dash_flag flag)
+{
+	set_bit(flag, dash->wk.flags);
+	queue_work(system_long_wq, &dash->wk.work);
+}
+
+static void rtl_dash_task(struct work_struct *work)
+{
+	struct rtl_dash *dash = container_of(work, struct rtl_dash, wk.work);
+
+	rtnl_lock();
+
+	if (test_and_clear_bit(DASH_FLAG_CHECK_CMAC, dash->wk.flags))
+		rtl_dash_cmac_reset_routine(dash);
+
+	rtnl_unlock();
+}
+
+DECLARE_RTL_DASH_COND(rtl_dash_state_cond)
+{
+	return r8168ep_ocp_read(dash->tp, 0x124) & 0x00000001;
+}
+
+static void rtl_driver_start(struct rtl_dash *dash)
+{
+	rtl_dash_oob_notify(dash, OOB_CMD_DRIVER_START);
+	rtl_dash_loop_wait_high(dash, &rtl_dash_state_cond, 10000, 10);
+}
+
+static void rtl_driver_stop(struct rtl_dash *dash)
+{
+	rtl_dash_oob_notify(dash, OOB_CMD_DRIVER_STOP);
+	rtl_dash_loop_wait_low(dash, &rtl_dash_state_cond, 10000, 10);
+}
+
+static int rtl_get_cmac_resource(struct rtl_dash **pdash, struct pci_dev *pdev)
+{
+	struct pci_dev *pdev_cmac;
+	void __iomem *cmac_ioaddr;
+
+	pdev_cmac = pci_get_slot(pdev->bus,
+				 PCI_DEVFN(PCI_SLOT(pdev->devfn), 0));
+	cmac_ioaddr = ioremap(pci_resource_start((*pdash)->pdev_cmac, 2), 256);
+	if (cmac_ioaddr) {
+		(*pdash)->pdev_cmac = pdev_cmac;
+		(*pdash)->cmac_ioaddr = cmac_ioaddr;
+		return 0;
+	} else {
+		return -ENOMEM;
+	}
+}
+
+struct rtl_dash *rtl_request_dash(struct rtl8169_private *tp,
+				  struct pci_dev *pci_dev, enum mac_version ver,
+				  void __iomem *mmio_addr)
+{
+	unsigned int order = get_order(CMAC_DESC_NUM * CMAC_BUF_SIZE);
+	struct rtl_dash *dash;
+	int node, ret = -ENOMEM;
+
+	dash = kzalloc(sizeof(*dash), GFP_KERNEL);
+	if (!dash)
+		goto err1;
+
+	dash->tp = tp;
+
+	switch (ver) {
+	case RTL_GIGA_MAC_VER_49 ... RTL_GIGA_MAC_VER_51:
+		dash->pdev_cmac = pci_dev;
+		dash->cmac_ioaddr = mmio_addr;
+		dash->hw_dash_ver = RTL_DASH_EP;
+		break;
+	case RTL_GIGA_MAC_VER_52 ... RTL_GIGA_MAC_VER_53:
+		if (rtl_get_cmac_resource(&dash, pci_dev) < 0)
+			goto err2;
+		dash->hw_dash_ver = RTL_DASH_FP;
+		break;
+	default:
+		ret = -ENODEV;
+		goto err2;
+	}
+
+	dash->tx_desc = dma_alloc_coherent(&dash->pdev_cmac->dev,
+					   CMAC_DESC_SIZE, &dash->tx_desc_dma,
+					   GFP_KERNEL);
+	if (!dash->tx_desc)
+		goto err2;
+
+	dash->rx_desc = dma_alloc_coherent(&dash->pdev_cmac->dev,
+					   CMAC_DESC_SIZE, &dash->rx_desc_dma,
+					   GFP_KERNEL);
+	if (!dash->rx_desc)
+		goto free_tx_desc;
+
+	node = dev_to_node(&dash->pdev_cmac->dev);
+
+	dash->tx_buf = alloc_pages_node(node, GFP_KERNEL, order);
+	if (!dash->tx_buf)
+		goto free_rx_desc;
+
+	dash->rx_buf = alloc_pages_node(node, GFP_KERNEL, order);
+	if (!dash->rx_buf)
+		goto free_tx_fuf;
+
+	memset(dash->tx_desc, 0, CMAC_DESC_SIZE);
+	memset(dash->rx_desc, 0, CMAC_DESC_SIZE);
+	memset(dash->tx_info, 0, sizeof(dash->tx_info));
+
+	INIT_WORK(&dash->wk.work, rtl_dash_task);
+	tasklet_setup(&dash->tl, dash_half);
+	tasklet_disable(&dash->tl);
+	init_completion(&dash->cmac_tx);
+	init_completion(&dash->cmac_rx);
+	init_completion(&dash->fw_ack);
+	spin_lock_init(&dash->cmac_lock);
+
+	return dash;
+
+free_tx_fuf:
+	__free_pages(dash->tx_buf, order);
+free_rx_desc:
+	dma_free_coherent(&dash->pdev_cmac->dev, CMAC_DESC_SIZE, dash->rx_desc,
+			  dash->rx_desc_dma);
+free_tx_desc:
+	dma_free_coherent(&dash->pdev_cmac->dev, CMAC_DESC_SIZE, dash->tx_desc,
+			  dash->tx_desc_dma);
+err2:
+	kfree(dash);
+err1:
+	return ERR_PTR(ret);
+}
+
+void rtl_release_dash(struct rtl_dash *dash)
+{
+	unsigned int order = get_order(CMAC_DESC_NUM * CMAC_BUF_SIZE);
+
+	if (IS_ERR_OR_NULL(dash))
+		return;
+
+	tasklet_kill(&dash->tl);
+
+	__free_pages(dash->rx_buf, order);
+	__free_pages(dash->tx_buf, order);
+	dma_free_coherent(&dash->pdev_cmac->dev, CMAC_DESC_SIZE, dash->rx_desc,
+			  dash->rx_desc_dma);
+	dma_free_coherent(&dash->pdev_cmac->dev, CMAC_DESC_SIZE, dash->tx_desc,
+			  dash->tx_desc_dma);
+
+	if (dash->hw_dash_ver != RTL_DASH_EP)
+		iounmap(dash->cmac_ioaddr);
+
+	kfree(dash);
+}
+
+void rtl_dash_up(struct rtl_dash *dash)
+{
+	rtl_driver_start(dash);
+	rtl_dash_change_cmac_state(dash, OOB_CMD_CMAC_STOP);
+}
+
+void rtl_dash_down(struct rtl_dash *dash)
+{
+	bitmap_zero(dash->wk.flags, DASH_FLAG_MAX);
+	cancel_work_sync(&dash->wk.work);
+
+	rtl_cmac_disable(dash);
+	rtl_driver_stop(dash);
+}
+
+void rtl_dash_cmac_reset_indicate(struct rtl_dash *dash)
+{
+	rtl_dash_schedule_work(dash, DASH_FLAG_CHECK_CMAC);
+}
+
+void rtl_dash_interrupt(struct rtl_dash *dash)
+{
+	RTL_CMAC_W8(dash, IBIMR0, 0);
+	RTL_CMAC_W8(dash, IBISR0, RTL_CMAC_R8(dash, IBISR0));
+	tasklet_schedule(&dash->tl);
+}
+
diff --git a/drivers/net/ethernet/realtek/r8169_dash.h b/drivers/net/ethernet/realtek/r8169_dash.h
new file mode 100644
index 000000000000..1e9a54a3df1b
--- /dev/null
+++ b/drivers/net/ethernet/realtek/r8169_dash.h
@@ -0,0 +1,22 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+#define CMAC_BUF_SIZE		2048
+
+enum rtl_dash_type {
+	RTL_DASH_NONE,
+	RTL_DASH_DP,
+	RTL_DASH_EP,
+	RTL_DASH_FP,
+};
+
+struct rtl_dash;
+
+void rtl_release_dash(struct rtl_dash *dash);
+void rtl_dash_up(struct rtl_dash *dash);
+void rtl_dash_down(struct rtl_dash *dash);
+void rtl_dash_cmac_reset_indicate(struct rtl_dash *dash);
+void rtl_dash_interrupt(struct rtl_dash *dash);
+
+struct rtl_dash *rtl_request_dash(struct rtl8169_private *tp,
+				  struct pci_dev *pci_dev, enum mac_version ver,
+				  void __iomem *mmio_addr);
diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
index eb56b91fe41b..83da05e5769e 100644
--- a/drivers/net/ethernet/realtek/r8169_main.c
+++ b/drivers/net/ethernet/realtek/r8169_main.c
@@ -33,6 +33,7 @@
 
 #include "r8169.h"
 #include "r8169_firmware.h"
+#include "r8169_dash.h"
 
 #define FIRMWARE_8168D_1	"rtl_nic/rtl8168d-1.fw"
 #define FIRMWARE_8168D_2	"rtl_nic/rtl8168d-2.fw"
@@ -348,8 +349,10 @@ enum rtl8125_registers {
 
 enum rtl_register_content {
 	/* InterruptStatusBits */
+	DashCMAC	= 0x8000,
 	SYSErr		= 0x8000,
 	PCSTimeout	= 0x4000,
+	DashIntr	= 0x1000,
 	SWInt		= 0x0100,
 	TxDescUnavail	= 0x0080,
 	RxFIFOOver	= 0x0040,
@@ -586,12 +589,6 @@ enum rtl_flag {
 	RTL_FLAG_MAX
 };
 
-enum rtl_dash_type {
-	RTL_DASH_NONE,
-	RTL_DASH_DP,
-	RTL_DASH_EP,
-};
-
 struct rtl8169_private {
 	void __iomem *mmio_addr;	/* memory map physical address */
 	struct pci_dev *pci_dev;
@@ -628,6 +625,7 @@ struct rtl8169_private {
 
 	const char *fw_name;
 	struct rtl_fw *rtl_fw;
+	struct rtl_dash *rtl_dash;
 
 	u32 ocp_base;
 };
@@ -1117,7 +1115,7 @@ static u32 r8168dp_ocp_read(struct rtl8169_private *tp, u16 reg)
 		RTL_R32(tp, OCPDR) : ~0;
 }
 
-static u32 r8168ep_ocp_read(struct rtl8169_private *tp, u16 reg)
+u32 r8168ep_ocp_read(struct rtl8169_private *tp, u16 reg)
 {
 	return _rtl_eri_read(tp, reg, ERIAR_OOB);
 }
@@ -1130,8 +1128,7 @@ static void r8168dp_ocp_write(struct rtl8169_private *tp, u8 mask, u16 reg,
 	rtl_loop_wait_low(tp, &rtl_ocpar_cond, 100, 20);
 }
 
-static void r8168ep_ocp_write(struct rtl8169_private *tp, u8 mask, u16 reg,
-			      u32 data)
+void r8168ep_ocp_write(struct rtl8169_private *tp, u8 mask, u16 reg, u32 data)
 {
 	_rtl_eri_write(tp, reg, ((u32)mask & 0x0f) << ERIAR_MASK_SHIFT,
 		       data, ERIAR_OOB);
@@ -4555,7 +4552,13 @@ static irqreturn_t rtl8169_interrupt(int irq, void *dev_instance)
 	if ((status & 0xffff) == 0xffff || !(status & tp->irq_mask))
 		return IRQ_NONE;
 
-	if (unlikely(status & SYSErr)) {
+	if (tp->rtl_dash) {
+		if (status & DashCMAC)
+			rtl_dash_cmac_reset_indicate(tp->rtl_dash);
+
+		if (status & DashIntr)
+			rtl_dash_interrupt(tp->rtl_dash);
+	} else if (unlikely(status & SYSErr)) {
 		rtl8169_pcierr_interrupt(tp->dev);
 		goto out;
 	}
@@ -4651,6 +4654,15 @@ static int r8169_phy_connect(struct rtl8169_private *tp)
 	return 0;
 }
 
+static void rtl8169_dash_release(struct rtl8169_private *tp)
+{
+	if (tp->dash_type > RTL_DASH_DP) {
+		tp->irq_mask &= ~(DashCMAC | DashIntr);
+		rtl_release_dash(tp->rtl_dash);
+		tp->rtl_dash = NULL;
+	}
+}
+
 static void rtl8169_down(struct rtl8169_private *tp)
 {
 	/* Clear all task flags */
@@ -4665,6 +4677,9 @@ static void rtl8169_down(struct rtl8169_private *tp)
 
 	rtl8169_cleanup(tp, true);
 
+	if (tp->rtl_dash)
+		rtl_dash_down(tp->rtl_dash);
+
 	rtl_prepare_power_down(tp);
 }
 
@@ -4678,6 +4693,9 @@ static void rtl8169_up(struct rtl8169_private *tp)
 	set_bit(RTL_FLAG_TASK_ENABLED, tp->wk.flags);
 	rtl_reset_work(tp);
 
+	if (tp->rtl_dash)
+		rtl_dash_up(tp->rtl_dash);
+
 	phy_start(tp->phydev);
 }
 
@@ -4693,6 +4711,7 @@ static int rtl8169_close(struct net_device *dev)
 	rtl8169_rx_clear(tp);
 
 	cancel_work_sync(&tp->wk.work);
+	rtl8169_dash_release(tp);
 
 	free_irq(pci_irq_vector(pdev, 0), tp);
 
@@ -4748,11 +4767,22 @@ static int rtl_open(struct net_device *dev)
 
 	rtl_request_firmware(tp);
 
+	if (tp->dash_type > RTL_DASH_DP) {
+		netdev_info(dev, "DASH enabled\n");
+		tp->rtl_dash = rtl_request_dash(tp, pdev, tp->mac_version,
+						tp->mmio_addr);
+		retval = PTR_ERR_OR_ZERO(tp->rtl_dash);
+		if (retval)
+			goto err_release_fw_2;
+
+		tp->irq_mask |= DashCMAC | DashIntr;
+	}
+
 	irqflags = pci_dev_msi_enabled(pdev) ? IRQF_NO_THREAD : IRQF_SHARED;
 	retval = request_irq(pci_irq_vector(pdev, 0), rtl8169_interrupt,
 			     irqflags, dev->name, tp);
 	if (retval < 0)
-		goto err_release_fw_2;
+		goto err_release_dash;
 
 	retval = r8169_phy_connect(tp);
 	if (retval)
@@ -4768,6 +4798,8 @@ static int rtl_open(struct net_device *dev)
 
 err_free_irq:
 	free_irq(pci_irq_vector(pdev, 0), tp);
+err_release_dash:
+	rtl8169_dash_release(tp);
 err_release_fw_2:
 	rtl_release_firmware(tp);
 	rtl8169_rx_clear(tp);
-- 
2.31.1

Re: [RFC PATCH 3/4] r8169: support CMAC

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 4944 bytes --]

Hi Hayes,

[FYI, it's a private test report for your RFC patch.]
[auto build test WARNING on horms-ipvs/master]
[also build test WARNING on linus/master v5.16-rc3 next-20211129]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Hayes-Wang/r8169-support-dash/20211129-181721
base:   https://git.kernel.org/pub/scm/linux/kernel/git/horms/ipvs.git master
config: powerpc-allyesconfig (https://download.01.org/0day-ci/archive/20211129/202111292307.PquauVvv-lkp(a)intel.com/config)
compiler: powerpc-linux-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/813fd268861808bfcf2eb07e84aca80e45c35d2f
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Hayes-Wang/r8169-support-dash/20211129-181721
        git checkout 813fd268861808bfcf2eb07e84aca80e45c35d2f
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=powerpc SHELL=/bin/bash drivers/net/ethernet/realtek/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   drivers/net/ethernet/realtek/r8169_dash.c: In function 'rtl_cmac_enable':
>> drivers/net/ethernet/realtek/r8169_dash.c:459:45: warning: right shift count >= width of type [-Wshift-count-overflow]
     459 |                           dash->rx_desc_dma >> 32);
         |                                             ^~
   drivers/net/ethernet/realtek/r8169_dash.c:463:45: warning: right shift count >= width of type [-Wshift-count-overflow]
     463 |                           dash->tx_desc_dma >> 32);
         |                                             ^~


vim +459 drivers/net/ethernet/realtek/r8169_dash.c

   395	
   396	static int rtl_cmac_enable(struct rtl_dash *dash)
   397	{
   398		u32 desc_addr;
   399		int i;
   400	
   401		for (i = 0; i < CMAC_DESC_NUM; i++) {
   402			struct dash_tx_info *info = dash->tx_info + i;
   403			struct cmac_desc *rx_desc = dash->rx_desc + i;
   404			struct cmac_desc *tx_desc = dash->tx_desc + i;
   405			struct device *d = &dash->pdev_cmac->dev;
   406			dma_addr_t mapping;
   407			u8 *addr;
   408			u16 ops_rx, ops_tx;
   409	
   410			info->buf = page_address(dash->tx_buf) + i * CMAC_BUF_SIZE;
   411			info->len = 0;
   412			info->ack = false;
   413	
   414			addr = page_address(dash->rx_buf) + i * CMAC_BUF_SIZE;
   415			mapping = dma_map_single(d, addr, CMAC_BUF_SIZE,
   416						 DMA_FROM_DEVICE);
   417			if (unlikely(dma_mapping_error(d, mapping))) {
   418				dev_err(d, "Failed to map RX DMA!\n");
   419				rtl_dash_change_cmac_state(dash, OOB_CMD_CMAC_STOP);
   420				return -ENOMEM;
   421			}
   422	
   423			rx_desc->dma_addr = cpu_to_le64(mapping);
   424			rx_desc->length = CMAC_BUF_SIZE;
   425			rx_desc->resv = 0;
   426	
   427			if (i == (CMAC_DESC_NUM - 1)) {
   428				ops_rx = RTXS_OWN | RTXS_EOR;
   429				ops_tx = RTXS_FS | RTXS_LS | RTXS_EOR;
   430			} else {
   431				ops_rx = RTXS_OWN;
   432				ops_tx = RTXS_FS | RTXS_LS;
   433			}
   434	
   435			rx_desc->status = cpu_to_le16(ops_rx);
   436			tx_desc->status = cpu_to_le16(ops_tx);
   437		}
   438	
   439		dash->tx_free = 0;
   440		dash->tx_used = 0;
   441		dash->rx_cur = 0;
   442	
   443		switch (dash->hw_dash_ver) {
   444		case RTL_DASH_EP:
   445			desc_addr = 0x890;
   446			break;
   447		case RTL_DASH_FP:
   448			desc_addr = 0xf20090;
   449			break;
   450		default:
   451			desc_addr = 0xf20090;
   452			WARN_ON_ONCE(1);
   453			break;
   454		}
   455	
   456		r8168_type2_write(dash->tp, 0xf, desc_addr,
   457				  dash->rx_desc_dma & DMA_BIT_MASK(32));
   458		r8168_type2_write(dash->tp, 0xf, desc_addr + 4,
 > 459				  dash->rx_desc_dma >> 32);
   460		r8168_type2_write(dash->tp, 0xf, desc_addr + 8,
   461				  dash->tx_desc_dma & DMA_BIT_MASK(32));
   462		r8168_type2_write(dash->tp, 0xf, desc_addr + 12,
   463				  dash->tx_desc_dma >> 32);
   464	
   465		RTL_CMAC_W8(dash, IBCR2, RTL_CMAC_R8(dash, IBCR2) | 0x01);
   466		RTL_CMAC_W8(dash, IBCR0, RTL_CMAC_R8(dash, IBCR0) | 0x01);
   467	
   468		tasklet_enable(&dash->tl);
   469		dash->cmac_state = CMAC_STATE_RUNNING;
   470	
   471		rtl_dash_intr_en(dash);
   472	
   473		return 0;
   474	}
   475	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

Re: [RFC PATCH 3/4] r8169: support CMAC

[Heiner Kallweit]

On 29.11.2021 11:13, Hayes Wang wrote:
> Support CMAC for RTL8111EP and RTL8111FP.
> 
> CMAC is the major interface to configure the firmware when dash is
> enabled.
> 
> Signed-off-by: Hayes Wang <hayeswang@realtek.com>
> ---
>  drivers/net/ethernet/realtek/Makefile     |   2 +-
>  drivers/net/ethernet/realtek/r8169.h      |   2 +
>  drivers/net/ethernet/realtek/r8169_dash.c | 756 ++++++++++++++++++++++
>  drivers/net/ethernet/realtek/r8169_dash.h |  22 +
>  drivers/net/ethernet/realtek/r8169_main.c |  54 +-
>  5 files changed, 824 insertions(+), 12 deletions(-)
>  create mode 100644 drivers/net/ethernet/realtek/r8169_dash.c
>  create mode 100644 drivers/net/ethernet/realtek/r8169_dash.h
> 
> diff --git a/drivers/net/ethernet/realtek/Makefile b/drivers/net/ethernet/realtek/Makefile
> index 2e1d78b106b0..8f3372ee92ff 100644
> --- a/drivers/net/ethernet/realtek/Makefile
> +++ b/drivers/net/ethernet/realtek/Makefile
> @@ -6,5 +6,5 @@
>  obj-$(CONFIG_8139CP) += 8139cp.o
>  obj-$(CONFIG_8139TOO) += 8139too.o
>  obj-$(CONFIG_ATP) += atp.o
> -r8169-objs += r8169_main.o r8169_firmware.o r8169_phy_config.o
> +r8169-objs += r8169_main.o r8169_firmware.o r8169_phy_config.o r8169_dash.o
>  obj-$(CONFIG_R8169) += r8169.o
> diff --git a/drivers/net/ethernet/realtek/r8169.h b/drivers/net/ethernet/realtek/r8169.h
> index 7db647b4796f..b75484a2a580 100644
> --- a/drivers/net/ethernet/realtek/r8169.h
> +++ b/drivers/net/ethernet/realtek/r8169.h
> @@ -78,5 +78,7 @@ u8 rtl8168d_efuse_read(struct rtl8169_private *tp, int reg_addr);
>  void r8169_hw_phy_config(struct rtl8169_private *tp, struct phy_device *phydev,
>  			 enum mac_version ver);
>  
> +u32 r8168ep_ocp_read(struct rtl8169_private *tp, u16 reg);
>  u32 r8168_type2_read(struct rtl8169_private *tp, u32 addr);
> +void r8168ep_ocp_write(struct rtl8169_private *tp, u8 mask, u16 reg, u32 data);
>  void r8168_type2_write(struct rtl8169_private *tp, u8 mask, u32 addr, u32 val);
> diff --git a/drivers/net/ethernet/realtek/r8169_dash.c b/drivers/net/ethernet/realtek/r8169_dash.c
> new file mode 100644
> index 000000000000..acee7519e9f1
> --- /dev/null
> +++ b/drivers/net/ethernet/realtek/r8169_dash.c
> @@ -0,0 +1,756 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +
> +#include <linux/pci.h>
> +#include <linux/workqueue.h>
> +#include <linux/rtnetlink.h>
> +#include <linux/string.h>
> +#include "r8169.h"
> +#include "r8169_dash.h"
> +
> +#define IBCR0				0xf8
> +#define IBCR2				0xf9
> +#define IBIMR0				0xfa
> +#define IBISR0				0xfb
> +
> +#define RTXS_LS				BIT(12)
> +#define RTXS_FS				BIT(13)
> +#define RTXS_EOR			BIT(14)
> +#define RTXS_OWN			BIT(15)
> +
> +#define DASH_ISR_ROK			BIT(0)
> +#define DASH_ISR_RDU			BIT(1)
> +#define DASH_ISR_TOK			BIT(2)
> +#define DASH_ISR_TDU			BIT(3)
> +#define DASH_ISR_TX_DISABLE_IDLE	BIT(5)
> +#define DASH_ISR_RX_DISABLE_IDLE	BIT(6)
> +
> +#define CMAC_DESC_NUM		4
> +#define CMAC_DESC_SIZE		(CMAC_DESC_NUM * sizeof(struct cmac_desc))
> +#define CMAC_TIMEOUT		(HZ * 5)
> +
> +#define OOB_CMD_DRIVER_START		0x05
> +#define OOB_CMD_DRIVER_STOP		0x06
> +#define OOB_CMD_CMAC_STOP		0x25
> +#define OOB_CMD_CMAC_INIT		0x26
> +#define OOB_CMD_CMAC_RESET		0x2a
> +
> +enum dash_cmac_state {
> +	CMAC_STATE_STOP = 0,
> +	CMAC_STATE_READY,
> +	CMAC_STATE_RUNNING,
> +};
> +
> +enum dash_flag {
> +	DASH_FLAG_CHECK_CMAC = 0,
> +	DASH_FLAG_MAX
> +};
> +
> +#pragma pack(push)
> +#pragma pack(1)
> +
> +struct cmac_desc {
> +	__le16 length;
> +	__le16 status;
> +	__le32 resv;
> +	__le64 dma_addr;
> +};
> +
> +struct oob_hdr {
> +	__le32 len;
> +	u8 type;
> +	u8 flag;
> +	u8 host_req;
> +	u8 res;
> +};
> +
> +#pragma pack(pop)
> +
> +struct dash_tx_info {
> +	u8 *buf;
> +	u32 len;
> +	bool ack;
> +};
> +
> +struct rtl_dash {
> +	struct rtl8169_private *tp;
> +	struct pci_dev *pdev_cmac;
> +	void __iomem *cmac_ioaddr;
> +	struct cmac_desc *tx_desc, *rx_desc;
> +	struct page *tx_buf, *rx_buf;
> +	struct dash_tx_info tx_info[CMAC_DESC_NUM];
> +	struct tasklet_struct tl;

Please see the following in include/linux/interrupt.h:

/* Tasklets --- multithreaded analogue of BHs.

   This API is deprecated. Please consider using threaded IRQs instead:
   https://lore.kernel.org/lkml/20200716081538.2sivhkj4hcyrusem@linutronix.de

RE: [RFC PATCH 3/4] r8169: support CMAC

[Hayes Wang]

Heiner Kallweit <hkallweit1@gmail.com>
> Sent: Tuesday, November 30, 2021 4:47 AM
> To: Hayes Wang <hayeswang@realtek.com>; Jakub Kicinski <kuba@kernel.org>;
[...]
> > +struct rtl_dash {
> > +	struct rtl8169_private *tp;
> > +	struct pci_dev *pdev_cmac;
> > +	void __iomem *cmac_ioaddr;
> > +	struct cmac_desc *tx_desc, *rx_desc;
> > +	struct page *tx_buf, *rx_buf;
> > +	struct dash_tx_info tx_info[CMAC_DESC_NUM];
> > +	struct tasklet_struct tl;
> 
> Please see the following in include/linux/interrupt.h:
> 
> /* Tasklets --- multithreaded analogue of BHs.
> 
>    This API is deprecated. Please consider using threaded IRQs instead:

How about replacing the tasklet with work?
It seems that the bottom half of threaded IRQ
is the work, too.

Best Regards,
Hayes

Re: [RFC PATCH 3/4] r8169: support CMAC

[Heiner Kallweit]

On 03.12.2021 08:57, Hayes Wang wrote:
> Heiner Kallweit <hkallweit1@gmail.com>
>> Sent: Tuesday, November 30, 2021 4:47 AM
>> To: Hayes Wang <hayeswang@realtek.com>; Jakub Kicinski <kuba@kernel.org>;
> [...]
>>> +struct rtl_dash {
>>> +	struct rtl8169_private *tp;
>>> +	struct pci_dev *pdev_cmac;
>>> +	void __iomem *cmac_ioaddr;
>>> +	struct cmac_desc *tx_desc, *rx_desc;
>>> +	struct page *tx_buf, *rx_buf;
>>> +	struct dash_tx_info tx_info[CMAC_DESC_NUM];
>>> +	struct tasklet_struct tl;
>>
>> Please see the following in include/linux/interrupt.h:
>>
>> /* Tasklets --- multithreaded analogue of BHs.
>>
>>    This API is deprecated. Please consider using threaded IRQs instead:
> 
> How about replacing the tasklet with work?
> It seems that the bottom half of threaded IRQ
> is the work, too.
> 
Right, it's running in process context. It's to a certain extent
comparable with the efforts to move NAPI processing from softirg
context to threads.

> Best Regards,
> Hayes
> 
> 

[RFC PATCH 4/4] r8169: add sysfs for dash

[Hayes Wang]

Add the sysfs for dash. Then the application could configure the
firmware through them.

Signed-off-by: Hayes Wang <hayeswang@realtek.com>
---
 drivers/net/ethernet/realtek/r8169_dash.c | 131 +++++++++++++++
 drivers/net/ethernet/realtek/r8169_dash.h |   8 +
 drivers/net/ethernet/realtek/r8169_main.c | 188 +++++++++++++++++++++-
 3 files changed, 326 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/realtek/r8169_dash.c b/drivers/net/ethernet/realtek/r8169_dash.c
index acee7519e9f1..197feb2c4a23 100644
--- a/drivers/net/ethernet/realtek/r8169_dash.c
+++ b/drivers/net/ethernet/realtek/r8169_dash.c
@@ -754,3 +754,134 @@ void rtl_dash_interrupt(struct rtl_dash *dash)
 	tasklet_schedule(&dash->tl);
 }
 
+int rtl_dash_to_fw(struct rtl_dash *dash, u8 *src, int size)
+{
+	int ret;
+	long t;
+
+	if (dash->cmac_state != CMAC_STATE_RUNNING)
+		return -ENETDOWN;
+
+	spin_lock_bh(&dash->cmac_lock);
+	ret = dash_rx_data(dash, NULL, 0);
+	spin_unlock_bh(&dash->cmac_lock);
+	if (ret < 0)
+		return -EUCLEAN;
+
+	reinit_completion(&dash->cmac_tx);
+	reinit_completion(&dash->fw_ack);
+
+	spin_lock_bh(&dash->cmac_lock);
+	ret = cmac_start_xmit(dash, src, size, false);
+	spin_unlock_bh(&dash->cmac_lock);
+
+	if (ret < 0)
+		goto err;
+
+	t = wait_for_completion_interruptible_timeout(&dash->cmac_tx,
+						      CMAC_TIMEOUT);
+	if (!t) {
+		ret = -ETIMEDOUT;
+		dev_err(&dash->pdev_cmac->dev, "CMAC tx timeout\n");
+		goto err;
+	} else if (t < 0) {
+		ret = t;
+		dev_err(&dash->pdev_cmac->dev, "CMAC tx fail %ld\n", t);
+		goto err;
+	}
+
+	t = wait_for_completion_interruptible_timeout(&dash->fw_ack,
+						      CMAC_TIMEOUT);
+	if (!t) {
+		ret = -ETIMEDOUT;
+		dev_err(&dash->pdev_cmac->dev, "FW ACK timeout\n");
+	} else if (t < 0) {
+		ret = t;
+		dev_err(&dash->pdev_cmac->dev, "FW ACK fail %ld\n", t);
+	}
+
+err:
+	return ret;
+}
+
+int rtl_dash_from_fw(struct rtl_dash *dash, u8 *src, int size)
+{
+	int ret;
+	long t;
+
+	if (dash->cmac_state != CMAC_STATE_RUNNING)
+		return -ENETDOWN;
+
+	reinit_completion(&dash->cmac_rx);
+
+	spin_lock_bh(&dash->cmac_lock);
+	ret = dash_rx_data(dash, src, size);
+	spin_unlock_bh(&dash->cmac_lock);
+
+	if (ret)
+		goto out;
+
+	t = wait_for_completion_interruptible_timeout(&dash->cmac_rx,
+						      CMAC_TIMEOUT);
+	if (!t) {
+		dev_warn(&dash->pdev_cmac->dev, "CMAC data timeout\n");
+	} else if (t < 0) {
+		ret = t;
+		dev_err(&dash->pdev_cmac->dev, "Wait CMAC data fail %ld\n", t);
+		goto out;
+	}
+
+	spin_lock_bh(&dash->cmac_lock);
+	ret = dash_rx_data(dash, src, size);
+	spin_unlock_bh(&dash->cmac_lock);
+
+out:
+	return ret;
+}
+
+void rtl_dash_set_ap_ready(struct rtl_dash *dash, bool enable)
+{
+	struct rtl8169_private *tp = dash->tp;
+	u32 data;
+
+	data = r8168ep_ocp_read(tp, 0x124);
+	if (enable)
+		data |= BIT(1);
+	else
+		data &= ~BIT(1);
+	r8168ep_ocp_write(tp, 0x1, 0x124, data);
+}
+
+bool rtl_dash_get_ap_ready(struct rtl_dash *dash)
+{
+	struct rtl8169_private *tp = dash->tp;
+
+	if (r8168ep_ocp_read(tp, 0x124) & BIT(1))
+		return true;
+	else
+		return false;
+}
+
+ssize_t rtl_dash_info(struct rtl_dash *dash, char *buf)
+{
+	struct rtl8169_private *tp = dash->tp;
+	char *dest = buf;
+	int size;
+	u32 data;
+
+	data = r8168ep_ocp_read(tp, 0x120);
+	size = sprintf(dest, "FW_VERSION=0x%08X\n", data);
+	if (size > 0)
+		dest += size;
+	else
+		return size;
+
+	data = r8168ep_ocp_read(tp, 0x174);
+	size = sprintf(dest, "FW_BUILD=0x%08X\n", data);
+	if (size > 0)
+		dest += size;
+	else
+		return size;
+
+	return strlen(buf) + 1;
+}
diff --git a/drivers/net/ethernet/realtek/r8169_dash.h b/drivers/net/ethernet/realtek/r8169_dash.h
index 1e9a54a3df1b..b33f3adeef13 100644
--- a/drivers/net/ethernet/realtek/r8169_dash.h
+++ b/drivers/net/ethernet/realtek/r8169_dash.h
@@ -16,6 +16,14 @@ void rtl_dash_up(struct rtl_dash *dash);
 void rtl_dash_down(struct rtl_dash *dash);
 void rtl_dash_cmac_reset_indicate(struct rtl_dash *dash);
 void rtl_dash_interrupt(struct rtl_dash *dash);
+void rtl_dash_set_ap_ready(struct rtl_dash *dash, bool enable);
+
+int rtl_dash_to_fw(struct rtl_dash *dash, u8 *src, int size);
+int rtl_dash_from_fw(struct rtl_dash *dash, u8 *src, int size);
+
+bool rtl_dash_get_ap_ready(struct rtl_dash *dash);
+
+ssize_t rtl_dash_info(struct rtl_dash *dash, char *buf);
 
 struct rtl_dash *rtl_request_dash(struct rtl8169_private *tp,
 				  struct pci_dev *pci_dev, enum mac_version ver,
diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
index 83da05e5769e..4c8439d3ae4d 100644
--- a/drivers/net/ethernet/realtek/r8169_main.c
+++ b/drivers/net/ethernet/realtek/r8169_main.c
@@ -4654,6 +4654,184 @@ static int r8169_phy_connect(struct rtl8169_private *tp)
 	return 0;
 }
 
+static ssize_t
+information_show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+	struct net_device *net = to_net_dev(dev);
+	struct rtl8169_private *tp = netdev_priv(net);
+	ssize_t ret;
+
+	if (rtnl_lock_killable())
+		return -EINTR;
+
+	ret = rtl_dash_info(tp->rtl_dash, buf);
+
+	rtnl_unlock();
+
+	return ret;
+}
+
+static DEVICE_ATTR_RO(information);
+
+static ssize_t
+ap_ready_show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+	struct net_device *net = to_net_dev(dev);
+	struct rtl8169_private *tp = netdev_priv(net);
+	bool enable;
+
+	if (rtnl_lock_killable())
+		return -EINTR;
+
+	enable = rtl_dash_get_ap_ready(tp->rtl_dash);
+
+	rtnl_unlock();
+
+	if (enable)
+		strcat(buf, "enable\n");
+	else
+		strcat(buf, "disable\n");
+
+	return (strlen(buf) + 1);
+}
+
+static ssize_t ap_ready_store(struct device *dev, struct device_attribute *attr,
+			      const char *buf, size_t count)
+{
+	struct net_device *net = to_net_dev(dev);
+	struct rtl8169_private *tp = netdev_priv(net);
+	ssize_t len = strlen(buf);
+	bool enable;
+
+	if (buf[len - 1] <= ' ')
+		len--;
+
+	/* strlen("enable") = 6, and strlen("disable") = 7 */
+	if (len != 6 && len != 7)
+		return -EINVAL;
+
+	if (len == 6 && !strncmp(buf, "enable", 6))
+		enable = true;
+	else if (len == 7 && !strncmp(buf, "disable", 7))
+		enable = false;
+	else
+		return -EINVAL;
+
+	if (rtnl_lock_killable())
+		return -EINTR;
+
+	rtl_dash_set_ap_ready(tp->rtl_dash, enable);
+
+	rtnl_unlock();
+
+	return count;
+}
+
+static DEVICE_ATTR_RW(ap_ready);
+
+static struct attribute *rtl_dash_attrs[] = {
+	&dev_attr_ap_ready.attr,
+	&dev_attr_information.attr,
+	NULL
+};
+
+static ssize_t cmac_data_write(struct file *fp, struct kobject *kobj,
+			       struct bin_attribute *attr, char *buf,
+			       loff_t offset, size_t size)
+{
+	struct device *dev = kobj_to_dev(kobj);
+	struct net_device *net = to_net_dev(dev);
+	struct rtl8169_private *tp = netdev_priv(net);
+	int ret;
+
+	if (IS_ERR_OR_NULL(tp->rtl_dash))
+		return -ENODEV;
+
+	if (size > CMAC_BUF_SIZE)
+		return -EFBIG;
+
+	if (offset)
+		return -EINVAL;
+
+	if (rtnl_lock_killable())
+		return -EINTR;
+
+	ret = rtl_dash_to_fw(tp->rtl_dash, buf, size);
+
+	rtnl_unlock();
+
+	return ret;
+}
+
+static ssize_t cmac_data_read(struct file *fp, struct kobject *kobj,
+			      struct bin_attribute *attr, char *buf,
+			      loff_t offset, size_t size)
+{
+	struct device *dev = kobj_to_dev(kobj);
+	struct net_device *net = to_net_dev(dev);
+	struct rtl8169_private *tp = netdev_priv(net);
+	int ret;
+
+	if (IS_ERR_OR_NULL(tp->rtl_dash))
+		return -ENODEV;
+
+	if (offset)
+		return -EINVAL;
+
+	if (rtnl_lock_killable())
+		return -EINTR;
+
+	ret = rtl_dash_from_fw(tp->rtl_dash, buf, size);
+
+	rtnl_unlock();
+
+	return ret;
+}
+
+static BIN_ATTR_RW(cmac_data, CMAC_BUF_SIZE);
+
+static struct bin_attribute *rtl_dash_bin_attrs[] = {
+	&bin_attr_cmac_data,
+	NULL
+};
+
+static umode_t is_dash_visible(struct kobject *kobj, struct attribute *attr,
+			       int n)
+{
+	struct device *dev = kobj_to_dev(kobj);
+	struct net_device *net = to_net_dev(dev);
+	struct rtl8169_private *tp = netdev_priv(net);
+
+	if (IS_ERR_OR_NULL(tp->rtl_dash))
+		return 0;
+
+	if (attr == &dev_attr_information.attr)
+		return 0440;
+
+	return 0660;
+}
+
+static umode_t is_dash_bin_visible(struct kobject *kobj,
+				   struct bin_attribute *attr, int n)
+{
+	struct device *dev = kobj_to_dev(kobj);
+	struct net_device *net = to_net_dev(dev);
+	struct rtl8169_private *tp = netdev_priv(net);
+
+	if (IS_ERR_OR_NULL(tp->rtl_dash))
+		return 0;
+
+	return 0660;
+}
+
+static struct attribute_group rtl_dash_grp = {
+	.name = "dash",
+	.is_visible	= is_dash_visible,
+	.attrs		= rtl_dash_attrs,
+	.is_bin_visible = is_dash_bin_visible,
+	.bin_attrs = rtl_dash_bin_attrs,
+};
+
 static void rtl8169_dash_release(struct rtl8169_private *tp)
 {
 	if (tp->dash_type > RTL_DASH_DP) {
@@ -4717,6 +4895,8 @@ static int rtl8169_close(struct net_device *dev)
 
 	phy_disconnect(tp->phydev);
 
+	sysfs_remove_group(&dev->dev.kobj, &rtl_dash_grp);
+
 	dma_free_coherent(&pdev->dev, R8169_RX_RING_BYTES, tp->RxDescArray,
 			  tp->RxPhyAddr);
 	dma_free_coherent(&pdev->dev, R8169_TX_RING_BYTES, tp->TxDescArray,
@@ -4775,6 +4955,10 @@ static int rtl_open(struct net_device *dev)
 		if (retval)
 			goto err_release_fw_2;
 
+		retval = sysfs_create_group(&dev->dev.kobj, &rtl_dash_grp);
+		if (retval < 0)
+			goto err_release_dash;
+
 		tp->irq_mask |= DashCMAC | DashIntr;
 	}
 
@@ -4782,7 +4966,7 @@ static int rtl_open(struct net_device *dev)
 	retval = request_irq(pci_irq_vector(pdev, 0), rtl8169_interrupt,
 			     irqflags, dev->name, tp);
 	if (retval < 0)
-		goto err_release_dash;
+		goto err_remove_group;
 
 	retval = r8169_phy_connect(tp);
 	if (retval)
@@ -4798,6 +4982,8 @@ static int rtl_open(struct net_device *dev)
 
 err_free_irq:
 	free_irq(pci_irq_vector(pdev, 0), tp);
+err_remove_group:
+	sysfs_remove_group(&dev->dev.kobj, &rtl_dash_grp);
 err_release_dash:
 	rtl8169_dash_release(tp);
 err_release_fw_2:
-- 
2.31.1

Re: [RFC PATCH 4/4] r8169: add sysfs for dash

[Heiner Kallweit]

On 29.11.2021 11:13, Hayes Wang wrote:
> Add the sysfs for dash. Then the application could configure the
> firmware through them.
> 
> Signed-off-by: Hayes Wang <hayeswang@realtek.com>
> ---
>  drivers/net/ethernet/realtek/r8169_dash.c | 131 +++++++++++++++
>  drivers/net/ethernet/realtek/r8169_dash.h |   8 +
>  drivers/net/ethernet/realtek/r8169_main.c | 188 +++++++++++++++++++++-
>  3 files changed, 326 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/realtek/r8169_dash.c b/drivers/net/ethernet/realtek/r8169_dash.c
> index acee7519e9f1..197feb2c4a23 100644
> --- a/drivers/net/ethernet/realtek/r8169_dash.c
> +++ b/drivers/net/ethernet/realtek/r8169_dash.c
> @@ -754,3 +754,134 @@ void rtl_dash_interrupt(struct rtl_dash *dash)
>  	tasklet_schedule(&dash->tl);
>  }
>  
> +int rtl_dash_to_fw(struct rtl_dash *dash, u8 *src, int size)
> +{
> +	int ret;
> +	long t;
> +
> +	if (dash->cmac_state != CMAC_STATE_RUNNING)
> +		return -ENETDOWN;
> +
> +	spin_lock_bh(&dash->cmac_lock);
> +	ret = dash_rx_data(dash, NULL, 0);
> +	spin_unlock_bh(&dash->cmac_lock);
> +	if (ret < 0)
> +		return -EUCLEAN;
> +
> +	reinit_completion(&dash->cmac_tx);
> +	reinit_completion(&dash->fw_ack);
> +
> +	spin_lock_bh(&dash->cmac_lock);
> +	ret = cmac_start_xmit(dash, src, size, false);
> +	spin_unlock_bh(&dash->cmac_lock);
> +
> +	if (ret < 0)
> +		goto err;
> +
> +	t = wait_for_completion_interruptible_timeout(&dash->cmac_tx,
> +						      CMAC_TIMEOUT);
> +	if (!t) {
> +		ret = -ETIMEDOUT;
> +		dev_err(&dash->pdev_cmac->dev, "CMAC tx timeout\n");
> +		goto err;
> +	} else if (t < 0) {
> +		ret = t;
> +		dev_err(&dash->pdev_cmac->dev, "CMAC tx fail %ld\n", t);
> +		goto err;
> +	}
> +
> +	t = wait_for_completion_interruptible_timeout(&dash->fw_ack,
> +						      CMAC_TIMEOUT);
> +	if (!t) {
> +		ret = -ETIMEDOUT;
> +		dev_err(&dash->pdev_cmac->dev, "FW ACK timeout\n");
> +	} else if (t < 0) {
> +		ret = t;
> +		dev_err(&dash->pdev_cmac->dev, "FW ACK fail %ld\n", t);
> +	}
> +
> +err:
> +	return ret;
> +}
> +
> +int rtl_dash_from_fw(struct rtl_dash *dash, u8 *src, int size)
> +{
> +	int ret;
> +	long t;
> +
> +	if (dash->cmac_state != CMAC_STATE_RUNNING)
> +		return -ENETDOWN;
> +
> +	reinit_completion(&dash->cmac_rx);
> +
> +	spin_lock_bh(&dash->cmac_lock);
> +	ret = dash_rx_data(dash, src, size);
> +	spin_unlock_bh(&dash->cmac_lock);
> +
> +	if (ret)
> +		goto out;
> +
> +	t = wait_for_completion_interruptible_timeout(&dash->cmac_rx,
> +						      CMAC_TIMEOUT);
> +	if (!t) {
> +		dev_warn(&dash->pdev_cmac->dev, "CMAC data timeout\n");
> +	} else if (t < 0) {
> +		ret = t;
> +		dev_err(&dash->pdev_cmac->dev, "Wait CMAC data fail %ld\n", t);
> +		goto out;
> +	}
> +
> +	spin_lock_bh(&dash->cmac_lock);
> +	ret = dash_rx_data(dash, src, size);
> +	spin_unlock_bh(&dash->cmac_lock);
> +
> +out:
> +	return ret;
> +}
> +
> +void rtl_dash_set_ap_ready(struct rtl_dash *dash, bool enable)
> +{
> +	struct rtl8169_private *tp = dash->tp;
> +	u32 data;
> +
> +	data = r8168ep_ocp_read(tp, 0x124);
> +	if (enable)
> +		data |= BIT(1);
> +	else
> +		data &= ~BIT(1);
> +	r8168ep_ocp_write(tp, 0x1, 0x124, data);
> +}
> +
> +bool rtl_dash_get_ap_ready(struct rtl_dash *dash)
> +{
> +	struct rtl8169_private *tp = dash->tp;
> +
> +	if (r8168ep_ocp_read(tp, 0x124) & BIT(1))
> +		return true;
> +	else
> +		return false;
> +}
> +
> +ssize_t rtl_dash_info(struct rtl_dash *dash, char *buf)
> +{
> +	struct rtl8169_private *tp = dash->tp;
> +	char *dest = buf;
> +	int size;
> +	u32 data;
> +
> +	data = r8168ep_ocp_read(tp, 0x120);
> +	size = sprintf(dest, "FW_VERSION=0x%08X\n", data);
> +	if (size > 0)
> +		dest += size;
> +	else
> +		return size;
> +
> +	data = r8168ep_ocp_read(tp, 0x174);
> +	size = sprintf(dest, "FW_BUILD=0x%08X\n", data);
> +	if (size > 0)
> +		dest += size;
> +	else
> +		return size;
> +
> +	return strlen(buf) + 1;
> +}
> diff --git a/drivers/net/ethernet/realtek/r8169_dash.h b/drivers/net/ethernet/realtek/r8169_dash.h
> index 1e9a54a3df1b..b33f3adeef13 100644
> --- a/drivers/net/ethernet/realtek/r8169_dash.h
> +++ b/drivers/net/ethernet/realtek/r8169_dash.h
> @@ -16,6 +16,14 @@ void rtl_dash_up(struct rtl_dash *dash);
>  void rtl_dash_down(struct rtl_dash *dash);
>  void rtl_dash_cmac_reset_indicate(struct rtl_dash *dash);
>  void rtl_dash_interrupt(struct rtl_dash *dash);
> +void rtl_dash_set_ap_ready(struct rtl_dash *dash, bool enable);
> +
> +int rtl_dash_to_fw(struct rtl_dash *dash, u8 *src, int size);
> +int rtl_dash_from_fw(struct rtl_dash *dash, u8 *src, int size);
> +
> +bool rtl_dash_get_ap_ready(struct rtl_dash *dash);
> +
> +ssize_t rtl_dash_info(struct rtl_dash *dash, char *buf);
>  
>  struct rtl_dash *rtl_request_dash(struct rtl8169_private *tp,
>  				  struct pci_dev *pci_dev, enum mac_version ver,
> diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
> index 83da05e5769e..4c8439d3ae4d 100644
> --- a/drivers/net/ethernet/realtek/r8169_main.c
> +++ b/drivers/net/ethernet/realtek/r8169_main.c
> @@ -4654,6 +4654,184 @@ static int r8169_phy_connect(struct rtl8169_private *tp)
>  	return 0;
>  }
>  
> +static ssize_t
> +information_show(struct device *dev, struct device_attribute *attr, char *buf)
> +{
> +	struct net_device *net = to_net_dev(dev);
> +	struct rtl8169_private *tp = netdev_priv(net);
> +	ssize_t ret;
> +
> +	if (rtnl_lock_killable())
> +		return -EINTR;
> +
> +	ret = rtl_dash_info(tp->rtl_dash, buf);
> +
> +	rtnl_unlock();
> +
> +	return ret;
> +}
> +
> +static DEVICE_ATTR_RO(information);
> +
> +static ssize_t
> +ap_ready_show(struct device *dev, struct device_attribute *attr, char *buf)
> +{
> +	struct net_device *net = to_net_dev(dev);
> +	struct rtl8169_private *tp = netdev_priv(net);
> +	bool enable;
> +
> +	if (rtnl_lock_killable())
> +		return -EINTR;
> +
> +	enable = rtl_dash_get_ap_ready(tp->rtl_dash);
> +
> +	rtnl_unlock();
> +
> +	if (enable)
> +		strcat(buf, "enable\n");
> +	else
> +		strcat(buf, "disable\n");
> +
> +	return (strlen(buf) + 1);
> +}
> +
> +static ssize_t ap_ready_store(struct device *dev, struct device_attribute *attr,
> +			      const char *buf, size_t count)
> +{
> +	struct net_device *net = to_net_dev(dev);
> +	struct rtl8169_private *tp = netdev_priv(net);
> +	ssize_t len = strlen(buf);
> +	bool enable;
> +
> +	if (buf[len - 1] <= ' ')
> +		len--;
> +
> +	/* strlen("enable") = 6, and strlen("disable") = 7 */
> +	if (len != 6 && len != 7)
> +		return -EINVAL;
> +
> +	if (len == 6 && !strncmp(buf, "enable", 6))
> +		enable = true;
> +	else if (len == 7 && !strncmp(buf, "disable", 7))
> +		enable = false;
> +	else
> +		return -EINVAL;
> +
> +	if (rtnl_lock_killable())
> +		return -EINTR;
> +
> +	rtl_dash_set_ap_ready(tp->rtl_dash, enable);
> +
> +	rtnl_unlock();
> +
> +	return count;
> +}
> +
> +static DEVICE_ATTR_RW(ap_ready);
> +
> +static struct attribute *rtl_dash_attrs[] = {
> +	&dev_attr_ap_ready.attr,
> +	&dev_attr_information.attr,
> +	NULL
> +};
> +
> +static ssize_t cmac_data_write(struct file *fp, struct kobject *kobj,
> +			       struct bin_attribute *attr, char *buf,
> +			       loff_t offset, size_t size)
> +{
> +	struct device *dev = kobj_to_dev(kobj);
> +	struct net_device *net = to_net_dev(dev);
> +	struct rtl8169_private *tp = netdev_priv(net);
> +	int ret;
> +
> +	if (IS_ERR_OR_NULL(tp->rtl_dash))
> +		return -ENODEV;
> +
> +	if (size > CMAC_BUF_SIZE)
> +		return -EFBIG;
> +
> +	if (offset)
> +		return -EINVAL;
> +
> +	if (rtnl_lock_killable())
> +		return -EINTR;
> +
> +	ret = rtl_dash_to_fw(tp->rtl_dash, buf, size);
> +
> +	rtnl_unlock();
> +
> +	return ret;
> +}
> +
> +static ssize_t cmac_data_read(struct file *fp, struct kobject *kobj,
> +			      struct bin_attribute *attr, char *buf,
> +			      loff_t offset, size_t size)
> +{
> +	struct device *dev = kobj_to_dev(kobj);
> +	struct net_device *net = to_net_dev(dev);
> +	struct rtl8169_private *tp = netdev_priv(net);
> +	int ret;
> +
> +	if (IS_ERR_OR_NULL(tp->rtl_dash))
> +		return -ENODEV;
> +
> +	if (offset)
> +		return -EINVAL;
> +
> +	if (rtnl_lock_killable())
> +		return -EINTR;
> +
> +	ret = rtl_dash_from_fw(tp->rtl_dash, buf, size);
> +
> +	rtnl_unlock();
> +
> +	return ret;
> +}
> +
> +static BIN_ATTR_RW(cmac_data, CMAC_BUF_SIZE);
> +
> +static struct bin_attribute *rtl_dash_bin_attrs[] = {
> +	&bin_attr_cmac_data,
> +	NULL
> +};
> +
> +static umode_t is_dash_visible(struct kobject *kobj, struct attribute *attr,
> +			       int n)
> +{
> +	struct device *dev = kobj_to_dev(kobj);
> +	struct net_device *net = to_net_dev(dev);
> +	struct rtl8169_private *tp = netdev_priv(net);
> +
> +	if (IS_ERR_OR_NULL(tp->rtl_dash))
> +		return 0;
> +
> +	if (attr == &dev_attr_information.attr)
> +		return 0440;
> +
> +	return 0660;
> +}
> +
> +static umode_t is_dash_bin_visible(struct kobject *kobj,
> +				   struct bin_attribute *attr, int n)
> +{
> +	struct device *dev = kobj_to_dev(kobj);
> +	struct net_device *net = to_net_dev(dev);
> +	struct rtl8169_private *tp = netdev_priv(net);
> +
> +	if (IS_ERR_OR_NULL(tp->rtl_dash))
> +		return 0;
> +
> +	return 0660;
> +}
> +
> +static struct attribute_group rtl_dash_grp = {
> +	.name = "dash",
> +	.is_visible	= is_dash_visible,
> +	.attrs		= rtl_dash_attrs,
> +	.is_bin_visible = is_dash_bin_visible,
> +	.bin_attrs = rtl_dash_bin_attrs,
> +};
> +
>  static void rtl8169_dash_release(struct rtl8169_private *tp)
>  {
>  	if (tp->dash_type > RTL_DASH_DP) {
> @@ -4717,6 +4895,8 @@ static int rtl8169_close(struct net_device *dev)
>  
>  	phy_disconnect(tp->phydev);
>  
> +	sysfs_remove_group(&dev->dev.kobj, &rtl_dash_grp);
> +
>  	dma_free_coherent(&pdev->dev, R8169_RX_RING_BYTES, tp->RxDescArray,
>  			  tp->RxPhyAddr);
>  	dma_free_coherent(&pdev->dev, R8169_TX_RING_BYTES, tp->TxDescArray,
> @@ -4775,6 +4955,10 @@ static int rtl_open(struct net_device *dev)
>  		if (retval)
>  			goto err_release_fw_2;
>  
> +		retval = sysfs_create_group(&dev->dev.kobj, &rtl_dash_grp);
> +		if (retval < 0)
> +			goto err_release_dash;
> +
>  		tp->irq_mask |= DashCMAC | DashIntr;
>  	}
>  
> @@ -4782,7 +4966,7 @@ static int rtl_open(struct net_device *dev)
>  	retval = request_irq(pci_irq_vector(pdev, 0), rtl8169_interrupt,
>  			     irqflags, dev->name, tp);
>  	if (retval < 0)
> -		goto err_release_dash;
> +		goto err_remove_group;
>  
>  	retval = r8169_phy_connect(tp);
>  	if (retval)
> @@ -4798,6 +4982,8 @@ static int rtl_open(struct net_device *dev)
>  
>  err_free_irq:
>  	free_irq(pci_irq_vector(pdev, 0), tp);
> +err_remove_group:
> +	sysfs_remove_group(&dev->dev.kobj, &rtl_dash_grp);
>  err_release_dash:
>  	rtl8169_dash_release(tp);
>  err_release_fw_2:
> 

With regard to sysfs usage:
- attributes should be documented under /Documentation/ABI/testing
- attributes should be defined statically (driver.dev_groups instead
  of sysfs_create_group)
- for printing info there's sysfs_emit()
- is really RTNL needed? Or would a lighter mutex do?

RE: [RFC PATCH 4/4] r8169: add sysfs for dash

[Hayes Wang]

Heiner Kallweit <hkallweit1@gmail.com>
> Sent: Friday, December 3, 2021 11:15 PM
[...]
> With regard to sysfs usage:
> - attributes should be documented under /Documentation/ABI/testing
> - attributes should be defined statically (driver.dev_groups instead
>   of sysfs_create_group)
> - for printing info there's sysfs_emit()
> - is really RTNL needed? Or would a lighter mutex do?

In addition to protect the critical section, RTNL is used to avoid
calling close() before CMAC is finished. The transfer of CMAC
may contain several steps. And close() would disable CMAC.
I don't wish the CMAC stays at strange state. It may influence
the firmware or hardware. Besides, I find the original driver only
use RTNL to protect critical section. Is there a better way for it?

Best Regards,
Hayes

Re: [RFC PATCH 4/4] r8169: add sysfs for dash

[Heiner Kallweit]

On 07.12.2021 07:53, Hayes Wang wrote:
> Heiner Kallweit <hkallweit1@gmail.com>
>> Sent: Friday, December 3, 2021 11:15 PM
> [...]
>> With regard to sysfs usage:
>> - attributes should be documented under /Documentation/ABI/testing
>> - attributes should be defined statically (driver.dev_groups instead
>>   of sysfs_create_group)
>> - for printing info there's sysfs_emit()
>> - is really RTNL needed? Or would a lighter mutex do?
> 
> In addition to protect the critical section, RTNL is used to avoid
> calling close() before CMAC is finished. The transfer of CMAC
> may contain several steps. And close() would disable CMAC.
> I don't wish the CMAC stays at strange state. It may influence
> the firmware or hardware. Besides, I find the original driver only
> use RTNL to protect critical section. Is there a better way for it?
> 

The main issue I see is that you call rtl_dash_from_fw() under RTNL,
and this function may sleep up to 5s. Holding a system-wide mutex
for that long isn't too nice.

In rtl_dash_info() you just print FW version and build number.
Wouldn't it be sufficient to print this info once to syslog
instead of exporting it via sysfs?

> Best Regards,
> Hayes
> 

RE: [RFC PATCH 4/4] r8169: add sysfs for dash

[Hayes Wang]

Heiner Kallweit <hkallweit1@gmail.com>
> Sent: Tuesday, December 7, 2021 3:38 PM
[...]
> > In addition to protect the critical section, RTNL is used to avoid
> > calling close() before CMAC is finished. The transfer of CMAC
> > may contain several steps. And close() would disable CMAC.
> > I don't wish the CMAC stays at strange state. It may influence
> > the firmware or hardware. Besides, I find the original driver only
> > use RTNL to protect critical section. Is there a better way for it?
> >
> 
> The main issue I see is that you call rtl_dash_from_fw() under RTNL,
> and this function may sleep up to 5s. Holding a system-wide mutex
> for that long isn't too nice.

I would think if there is a better way to replace current one.
Thanks.

> In rtl_dash_info() you just print FW version and build number.
> Wouldn't it be sufficient to print this info once to syslog
> instead of exporting it via sysfs?

It is the information which our user space tool need.
I think it would be replaced with devlink param.

Best Regards,
Hayes

Re: [RFC PATCH 0/4] r8169: support dash

[Jakub Kicinski]

On Mon, 29 Nov 2021 18:13:11 +0800 Hayes Wang wrote:
> These patches are used to support dash for RTL8111EP and
> RTL8111FP(RTL81117).

If I understand correctly DASH is a DMTF standard for remote control.

Since it's a standard I think we should have a common way of
configuring it across drivers. Is enable/disable the only configuration
that we will need?

We don't use sysfs too much these days, can we move the knob to
devlink, please? (If we only need an on/off switch generic devlink param
should be fine).

[Intel-wired-lan] [RFC PATCH 0/4] r8169: support dash

[Jakub Kicinski]

On Mon, 29 Nov 2021 18:13:11 +0800 Hayes Wang wrote:
> These patches are used to support dash for RTL8111EP and
> RTL8111FP(RTL81117).

If I understand correctly DASH is a DMTF standard for remote control.

Since it's a standard I think we should have a common way of
configuring it across drivers. Is enable/disable the only configuration
that we will need?

We don't use sysfs too much these days, can we move the knob to
devlink, please? (If we only need an on/off switch generic devlink param
should be fine).

RE: [RFC PATCH 0/4] r8169: support dash

[Hayes Wang]

Jakub Kicinski <kuba@kernel.org>
> Sent: Tuesday, November 30, 2021 2:00 AM
> Subject: Re: [RFC PATCH 0/4] r8169: support dash
> 
> On Mon, 29 Nov 2021 18:13:11 +0800 Hayes Wang wrote:
> > These patches are used to support dash for RTL8111EP and
> > RTL8111FP(RTL81117).
> 
> If I understand correctly DASH is a DMTF standard for remote control.
> 
> Since it's a standard I think we should have a common way of
> configuring it across drivers.

Excuse me. I am not familiar with it.
What document or sample code could I start?

> Is enable/disable the only configuration
> that we will need?

I don't think I could answer it before I understand the above way
you mentioned.

> We don't use sysfs too much these days, can we move the knob to
> devlink, please? (If we only need an on/off switch generic devlink param
> should be fine).

Thanks. I would study devlink.

Best Regards,
Hayes

[Intel-wired-lan] [RFC PATCH 0/4] r8169: support dash

[Hayes Wang]

Jakub Kicinski <kuba@kernel.org>
> Sent: Tuesday, November 30, 2021 2:00 AM
> Subject: Re: [RFC PATCH 0/4] r8169: support dash
> 
> On Mon, 29 Nov 2021 18:13:11 +0800 Hayes Wang wrote:
> > These patches are used to support dash for RTL8111EP and
> > RTL8111FP(RTL81117).
> 
> If I understand correctly DASH is a DMTF standard for remote control.
> 
> Since it's a standard I think we should have a common way of
> configuring it across drivers.

Excuse me. I am not familiar with it.
What document or sample code could I start?

> Is enable/disable the only configuration
> that we will need?

I don't think I could answer it before I understand the above way
you mentioned.

> We don't use sysfs too much these days, can we move the knob to
> devlink, please? (If we only need an on/off switch generic devlink param
> should be fine).

Thanks. I would study devlink.

Best Regards,
Hayes

Re: [RFC PATCH 0/4] r8169: support dash

[Jakub Kicinski]

On Wed, 1 Dec 2021 02:57:00 +0000 Hayes Wang wrote:
> Jakub Kicinski <kuba@kernel.org>
> > Sent: Tuesday, November 30, 2021 2:00 AM
> > Subject: Re: [RFC PATCH 0/4] r8169: support dash
> > 
> > On Mon, 29 Nov 2021 18:13:11 +0800 Hayes Wang wrote:  
> > > These patches are used to support dash for RTL8111EP and
> > > RTL8111FP(RTL81117).  
> > 
> > If I understand correctly DASH is a DMTF standard for remote control.
> > 
> > Since it's a standard I think we should have a common way of
> > configuring it across drivers.  
> 
> Excuse me. I am not familiar with it.
> What document or sample code could I start?
> 
> > Is enable/disable the only configuration
> > that we will need?  
> 
> I don't think I could answer it before I understand the above way
> you mentioned.
> 
> > We don't use sysfs too much these days, can we move the knob to
> > devlink, please? (If we only need an on/off switch generic devlink param
> > should be fine).  
> 
> Thanks. I would study devlink.

I'm not sure how relevant it will be to you but this is the
documentation we have:

https://www.kernel.org/doc/html/latest/networking/devlink/index.html
https://www.kernel.org/doc/html/latest/networking/devlink/devlink-params.html

You'll need to add a generic parameter (define + a short description)
like 325e0d0aa683 ("devlink: Add 'enable_iwarp' generic device param")

In terms of driver changes I think the most relevant example to you
will be:

drivers/net/ethernet/ti/cpsw_new.c

You need to call devlink_alloc(), devlink_register and
devlink_params_register() (and the inverse functions).

[Intel-wired-lan] [RFC PATCH 0/4] r8169: support dash

[Jakub Kicinski]

On Wed, 1 Dec 2021 02:57:00 +0000 Hayes Wang wrote:
> Jakub Kicinski <kuba@kernel.org>
> > Sent: Tuesday, November 30, 2021 2:00 AM
> > Subject: Re: [RFC PATCH 0/4] r8169: support dash
> > 
> > On Mon, 29 Nov 2021 18:13:11 +0800 Hayes Wang wrote:  
> > > These patches are used to support dash for RTL8111EP and
> > > RTL8111FP(RTL81117).  
> > 
> > If I understand correctly DASH is a DMTF standard for remote control.
> > 
> > Since it's a standard I think we should have a common way of
> > configuring it across drivers.  
> 
> Excuse me. I am not familiar with it.
> What document or sample code could I start?
> 
> > Is enable/disable the only configuration
> > that we will need?  
> 
> I don't think I could answer it before I understand the above way
> you mentioned.
> 
> > We don't use sysfs too much these days, can we move the knob to
> > devlink, please? (If we only need an on/off switch generic devlink param
> > should be fine).  
> 
> Thanks. I would study devlink.

I'm not sure how relevant it will be to you but this is the
documentation we have:

https://www.kernel.org/doc/html/latest/networking/devlink/index.html
https://www.kernel.org/doc/html/latest/networking/devlink/devlink-params.html

You'll need to add a generic parameter (define + a short description)
like 325e0d0aa683 ("devlink: Add 'enable_iwarp' generic device param")

In terms of driver changes I think the most relevant example to you
will be:

drivers/net/ethernet/ti/cpsw_new.c

You need to call devlink_alloc(), devlink_register and
devlink_params_register() (and the inverse functions).

RE: [RFC PATCH 0/4] r8169: support dash

[Hayes Wang]

Jakub Kicinski <kuba@kernel.org>
> Sent: Wednesday, December 1, 2021 11:09 AM
[...]
> I'm not sure how relevant it will be to you but this is the
> documentation we have:
> 
> https://www.kernel.org/doc/html/latest/networking/devlink/index.html
> https://www.kernel.org/doc/html/latest/networking/devlink/devlink-params.ht
> ml
> 
> You'll need to add a generic parameter (define + a short description)
> like 325e0d0aa683 ("devlink: Add 'enable_iwarp' generic device param")
> 
> In terms of driver changes I think the most relevant example to you
> will be:
> 
> drivers/net/ethernet/ti/cpsw_new.c
> 
> You need to call devlink_alloc(), devlink_register and
> devlink_params_register() (and the inverse functions).

I have studied the devlink briefly.

However, I find some problems. First, our
settings are dependent on the design of
both the hardware and firmware. That is,
I don't think the others need to do the
settings as the same as us. The devlink
seems to let everyone could use the same
command to do the same setting. However,
most of our settings are useless for the
other devices.

Second, according to the design of our
CMAC, the application has to read and
write data with variable length from/to
the firmware. Each custom has his own
requests. Therefore, our customs would
get different firmware with different
behavior. Only the application and the
firmware know how to communicate with
each other. The driver only passes the
data between them. Like the Ethernet
driver, it doesn't need to know the
contend of the packet. I could implement
the CMAC through sysfs, but I don't
know how to do by devlink.

In brief, CMAC is our major method to
configure the firmware and get response
from the firmware. Except for certain information,
the other settings are not standard and useless
for the other vendors.

Is the devlink the only method I could use?
Actually, we use IOCTL now. We wish to
convert it to sysfs for upstream driver.

Best Regards,
Hayes

[Intel-wired-lan] [RFC PATCH 0/4] r8169: support dash

[Hayes Wang]

Jakub Kicinski <kuba@kernel.org>
> Sent: Wednesday, December 1, 2021 11:09 AM
[...]
> I'm not sure how relevant it will be to you but this is the
> documentation we have:
> 
> https://www.kernel.org/doc/html/latest/networking/devlink/index.html
> https://www.kernel.org/doc/html/latest/networking/devlink/devlink-params.ht
> ml
> 
> You'll need to add a generic parameter (define + a short description)
> like 325e0d0aa683 ("devlink: Add 'enable_iwarp' generic device param")
> 
> In terms of driver changes I think the most relevant example to you
> will be:
> 
> drivers/net/ethernet/ti/cpsw_new.c
> 
> You need to call devlink_alloc(), devlink_register and
> devlink_params_register() (and the inverse functions).

I have studied the devlink briefly.

However, I find some problems. First, our
settings are dependent on the design of
both the hardware and firmware. That is,
I don't think the others need to do the
settings as the same as us. The devlink
seems to let everyone could use the same
command to do the same setting. However,
most of our settings are useless for the
other devices.

Second, according to the design of our
CMAC, the application has to read and
write data with variable length from/to
the firmware. Each custom has his own
requests. Therefore, our customs would
get different firmware with different
behavior. Only the application and the
firmware know how to communicate with
each other. The driver only passes the
data between them. Like the Ethernet
driver, it doesn't need to know the
contend of the packet. I could implement
the CMAC through sysfs, but I don't
know how to do by devlink.

In brief, CMAC is our major method to
configure the firmware and get response
from the firmware. Except for certain information,
the other settings are not standard and useless
for the other vendors.

Is the devlink the only method I could use?
Actually, we use IOCTL now. We wish to
convert it to sysfs for upstream driver.

Best Regards,
Hayes

Re: [RFC PATCH 0/4] r8169: support dash

[Jakub Kicinski]

On Fri, 3 Dec 2021 07:57:08 +0000 Hayes Wang wrote:
> Jakub Kicinski <kuba@kernel.org>
> > I'm not sure how relevant it will be to you but this is the
> > documentation we have:
> > 
> > https://www.kernel.org/doc/html/latest/networking/devlink/index.html
> > https://www.kernel.org/doc/html/latest/networking/devlink/devlink-params.ht
> > ml
> > 
> > You'll need to add a generic parameter (define + a short description)
> > like 325e0d0aa683 ("devlink: Add 'enable_iwarp' generic device param")
> > 
> > In terms of driver changes I think the most relevant example to you
> > will be:
> > 
> > drivers/net/ethernet/ti/cpsw_new.c
> > 
> > You need to call devlink_alloc(), devlink_register and
> > devlink_params_register() (and the inverse functions).  
> 
> I have studied the devlink briefly.
> 
> However, I find some problems. First, our
> settings are dependent on the design of
> both the hardware and firmware. That is,
> I don't think the others need to do the
> settings as the same as us. The devlink
> seems to let everyone could use the same
> command to do the same setting. However,
> most of our settings are useless for the
> other devices.
> 
> Second, according to the design of our
> CMAC, the application has to read and
> write data with variable length from/to
> the firmware. Each custom has his own
> requests. Therefore, our customs would
> get different firmware with different
> behavior. Only the application and the
> firmware know how to communicate with
> each other. The driver only passes the
> data between them. Like the Ethernet
> driver, it doesn't need to know the
> contend of the packet. I could implement
> the CMAC through sysfs, but I don't
> know how to do by devlink.
> 
> In brief, CMAC is our major method to
> configure the firmware and get response
> from the firmware. Except for certain information,
> the other settings are not standard and useless
> for the other vendors.
> 
> Is the devlink the only method I could use?
> Actually, we use IOCTL now. We wish to
> convert it to sysfs for upstream driver.

Ah, I've only spotted the enable/disable knob in the patch. 
If you're exchanging arbitrary binary data with the FW we 
can't help you. It's not going to fly upstream.

[Intel-wired-lan] [RFC PATCH 0/4] r8169: support dash

[Jakub Kicinski]

On Fri, 3 Dec 2021 07:57:08 +0000 Hayes Wang wrote:
> Jakub Kicinski <kuba@kernel.org>
> > I'm not sure how relevant it will be to you but this is the
> > documentation we have:
> > 
> > https://www.kernel.org/doc/html/latest/networking/devlink/index.html
> > https://www.kernel.org/doc/html/latest/networking/devlink/devlink-params.ht
> > ml
> > 
> > You'll need to add a generic parameter (define + a short description)
> > like 325e0d0aa683 ("devlink: Add 'enable_iwarp' generic device param")
> > 
> > In terms of driver changes I think the most relevant example to you
> > will be:
> > 
> > drivers/net/ethernet/ti/cpsw_new.c
> > 
> > You need to call devlink_alloc(), devlink_register and
> > devlink_params_register() (and the inverse functions).  
> 
> I have studied the devlink briefly.
> 
> However, I find some problems. First, our
> settings are dependent on the design of
> both the hardware and firmware. That is,
> I don't think the others need to do the
> settings as the same as us. The devlink
> seems to let everyone could use the same
> command to do the same setting. However,
> most of our settings are useless for the
> other devices.
> 
> Second, according to the design of our
> CMAC, the application has to read and
> write data with variable length from/to
> the firmware. Each custom has his own
> requests. Therefore, our customs would
> get different firmware with different
> behavior. Only the application and the
> firmware know how to communicate with
> each other. The driver only passes the
> data between them. Like the Ethernet
> driver, it doesn't need to know the
> contend of the packet. I could implement
> the CMAC through sysfs, but I don't
> know how to do by devlink.
> 
> In brief, CMAC is our major method to
> configure the firmware and get response
> from the firmware. Except for certain information,
> the other settings are not standard and useless
> for the other vendors.
> 
> Is the devlink the only method I could use?
> Actually, we use IOCTL now. We wish to
> convert it to sysfs for upstream driver.

Ah, I've only spotted the enable/disable knob in the patch. 
If you're exchanging arbitrary binary data with the FW we 
can't help you. It's not going to fly upstream.

Re: [RFC PATCH 0/4] r8169: support dash

[Alexander Lobakin]

From: Jakub Kicinski <kuba@kernel.org>
Date: Fri, 3 Dec 2021 07:04:10 -0800

> On Fri, 3 Dec 2021 07:57:08 +0000 Hayes Wang wrote:
> > Jakub Kicinski <kuba@kernel.org>
> > > I'm not sure how relevant it will be to you but this is the
> > > documentation we have:
> > > 
> > > https://www.kernel.org/doc/html/latest/networking/devlink/index.html
> > > https://www.kernel.org/doc/html/latest/networking/devlink/devlink-params.ht
> > > ml
> > > 
> > > You'll need to add a generic parameter (define + a short description)
> > > like 325e0d0aa683 ("devlink: Add 'enable_iwarp' generic device param")
> > > 
> > > In terms of driver changes I think the most relevant example to you
> > > will be:
> > > 
> > > drivers/net/ethernet/ti/cpsw_new.c
> > > 
> > > You need to call devlink_alloc(), devlink_register and
> > > devlink_params_register() (and the inverse functions).  
> > 
> > I have studied the devlink briefly.
> > 
> > However, I find some problems. First, our
> > settings are dependent on the design of
> > both the hardware and firmware. That is,
> > I don't think the others need to do the
> > settings as the same as us. The devlink
> > seems to let everyone could use the same
> > command to do the same setting. However,
> > most of our settings are useless for the
> > other devices.
> > 
> > Second, according to the design of our
> > CMAC, the application has to read and
> > write data with variable length from/to
> > the firmware. Each custom has his own
> > requests. Therefore, our customs would
> > get different firmware with different
> > behavior. Only the application and the
> > firmware know how to communicate with
> > each other. The driver only passes the
> > data between them. Like the Ethernet
> > driver, it doesn't need to know the
> > contend of the packet. I could implement
> > the CMAC through sysfs, but I don't
> > know how to do by devlink.
> > 
> > In brief, CMAC is our major method to
> > configure the firmware and get response
> > from the firmware. Except for certain information,
> > the other settings are not standard and useless
> > for the other vendors.
> > 
> > Is the devlink the only method I could use?
> > Actually, we use IOCTL now. We wish to
> > convert it to sysfs for upstream driver.
> 
> Ah, I've only spotted the enable/disable knob in the patch. 
> If you're exchanging arbitrary binary data with the FW we 
> can't help you. It's not going to fly upstream.

Uhm. I'm not saying sysfs is a proper way to do that, not at all,
buuut...
We have a ton of different subsystems providing a communication
channel between userspace and HW/FW. Chardevices all over the
tree, highly used rpmsg for remoteproc, uio. We have register
dump in Ethtool, as well as get/set for EEPROM, I'd count them
as well.
So it probably isn't a bad idea to provide some standard API for
network drivers to talk to HW/FW from userspace, like get/set or
rx/tx (when having enough caps for sure)? It could be Devlink ops
or Ethtool ops, the latter fits more to me.

Al

[Intel-wired-lan] [RFC PATCH 0/4] r8169: support dash

[Alexander Lobakin]

From: Jakub Kicinski <kuba@kernel.org>
Date: Fri, 3 Dec 2021 07:04:10 -0800

> On Fri, 3 Dec 2021 07:57:08 +0000 Hayes Wang wrote:
> > Jakub Kicinski <kuba@kernel.org>
> > > I'm not sure how relevant it will be to you but this is the
> > > documentation we have:
> > > 
> > > https://www.kernel.org/doc/html/latest/networking/devlink/index.html
> > > https://www.kernel.org/doc/html/latest/networking/devlink/devlink-params.ht
> > > ml
> > > 
> > > You'll need to add a generic parameter (define + a short description)
> > > like 325e0d0aa683 ("devlink: Add 'enable_iwarp' generic device param")
> > > 
> > > In terms of driver changes I think the most relevant example to you
> > > will be:
> > > 
> > > drivers/net/ethernet/ti/cpsw_new.c
> > > 
> > > You need to call devlink_alloc(), devlink_register and
> > > devlink_params_register() (and the inverse functions).  
> > 
> > I have studied the devlink briefly.
> > 
> > However, I find some problems. First, our
> > settings are dependent on the design of
> > both the hardware and firmware. That is,
> > I don't think the others need to do the
> > settings as the same as us. The devlink
> > seems to let everyone could use the same
> > command to do the same setting. However,
> > most of our settings are useless for the
> > other devices.
> > 
> > Second, according to the design of our
> > CMAC, the application has to read and
> > write data with variable length from/to
> > the firmware. Each custom has his own
> > requests. Therefore, our customs would
> > get different firmware with different
> > behavior. Only the application and the
> > firmware know how to communicate with
> > each other. The driver only passes the
> > data between them. Like the Ethernet
> > driver, it doesn't need to know the
> > contend of the packet. I could implement
> > the CMAC through sysfs, but I don't
> > know how to do by devlink.
> > 
> > In brief, CMAC is our major method to
> > configure the firmware and get response
> > from the firmware. Except for certain information,
> > the other settings are not standard and useless
> > for the other vendors.
> > 
> > Is the devlink the only method I could use?
> > Actually, we use IOCTL now. We wish to
> > convert it to sysfs for upstream driver.
> 
> Ah, I've only spotted the enable/disable knob in the patch. 
> If you're exchanging arbitrary binary data with the FW we 
> can't help you. It's not going to fly upstream.

Uhm. I'm not saying sysfs is a proper way to do that, not at all,
buuut...
We have a ton of different subsystems providing a communication
channel between userspace and HW/FW. Chardevices all over the
tree, highly used rpmsg for remoteproc, uio. We have register
dump in Ethtool, as well as get/set for EEPROM, I'd count them
as well.
So it probably isn't a bad idea to provide some standard API for
network drivers to talk to HW/FW from userspace, like get/set or
rx/tx (when having enough caps for sure)? It could be Devlink ops
or Ethtool ops, the latter fits more to me.

Al

Re: [RFC PATCH 0/4] r8169: support dash

[Jakub Kicinski]

On Sat,  4 Dec 2021 02:08:29 +0100 Alexander Lobakin wrote:
> > Ah, I've only spotted the enable/disable knob in the patch. 
> > If you're exchanging arbitrary binary data with the FW we 
> > can't help you. It's not going to fly upstream.  
> 
> Uhm. I'm not saying sysfs is a proper way to do that, not at all,
> buuut...
> We have a ton of different subsystems providing a communication
> channel between userspace and HW/FW. Chardevices all over the
> tree, highly used rpmsg for remoteproc, uio. 

Not in Ethernet.

> We have register dump in Ethtool,

Read only.

> as well as get/set for EEPROM, I'd count them as well.

EEPROM writes are supposed to update FW images, not send random
messages.

> So it probably isn't a bad idea to provide some standard API for
> network drivers to talk to HW/FW from userspace, like get/set or
> rx/tx (when having enough caps for sure)? It could be Devlink ops
> or Ethtool ops, the latter fits more to me.

I'm not saying it's wrong to merge shim drivers into the kernel and
let the user space talk to device FW. I'm saying it's counter to what
netdev's policy has always been and counter to my personal interests.

What is a standard API for custom, proprietary FW message interface?
We want standards at a functional level. Once you open up a raw FW
write interface there is no policing of what goes thru it.

I CCed Intel since you also have the (infamous) ME, but I never heard
of the need to communicate from the OS to the ME via the netdev
driver... Not sure why things are different for Realtek.

[Intel-wired-lan] [RFC PATCH 0/4] r8169: support dash

[Jakub Kicinski]

On Sat,  4 Dec 2021 02:08:29 +0100 Alexander Lobakin wrote:
> > Ah, I've only spotted the enable/disable knob in the patch. 
> > If you're exchanging arbitrary binary data with the FW we 
> > can't help you. It's not going to fly upstream.  
> 
> Uhm. I'm not saying sysfs is a proper way to do that, not at all,
> buuut...
> We have a ton of different subsystems providing a communication
> channel between userspace and HW/FW. Chardevices all over the
> tree, highly used rpmsg for remoteproc, uio. 

Not in Ethernet.

> We have register dump in Ethtool,

Read only.

> as well as get/set for EEPROM, I'd count them as well.

EEPROM writes are supposed to update FW images, not send random
messages.

> So it probably isn't a bad idea to provide some standard API for
> network drivers to talk to HW/FW from userspace, like get/set or
> rx/tx (when having enough caps for sure)? It could be Devlink ops
> or Ethtool ops, the latter fits more to me.

I'm not saying it's wrong to merge shim drivers into the kernel and
let the user space talk to device FW. I'm saying it's counter to what
netdev's policy has always been and counter to my personal interests.

What is a standard API for custom, proprietary FW message interface?
We want standards at a functional level. Once you open up a raw FW
write interface there is no policing of what goes thru it.

I CCed Intel since you also have the (infamous) ME, but I never heard
of the need to communicate from the OS to the ME via the netdev
driver... Not sure why things are different for Realtek.

RE: [RFC PATCH 0/4] r8169: support dash

[Hayes Wang]

Jakub Kicinski <kuba@kernel.org>
> Sent: Friday, December 3, 2021 11:04 PM
[...]
> Ah, I've only spotted the enable/disable knob in the patch.
> If you're exchanging arbitrary binary data with the FW we
> can't help you. It's not going to fly upstream. 

How is it that we only provide certain basic settings,
such as IPv4 address, IPv6 address, and so on? Then,
they are not the arbitrary binary data.

Could devlink param be used for more than 4 bytes settings?
At least the IPv6 address is longer.

Besides, we need the information of SMBIOS which could
be 4K ~ 8K bytes data. Is there any way we could transmit
it to firmware?

Best Regards,
Hayes

[Intel-wired-lan] [RFC PATCH 0/4] r8169: support dash

[Hayes Wang]

Jakub Kicinski <kuba@kernel.org>
> Sent: Friday, December 3, 2021 11:04 PM
[...]
> Ah, I've only spotted the enable/disable knob in the patch.
> If you're exchanging arbitrary binary data with the FW we
> can't help you. It's not going to fly upstream. 

How is it that we only provide certain basic settings,
such as IPv4 address, IPv6 address, and so on? Then,
they are not the arbitrary binary data.

Could devlink param be used for more than 4 bytes settings?
At least the IPv6 address is longer.

Besides, we need the information of SMBIOS which could
be 4K ~ 8K bytes data. Is there any way we could transmit
it to firmware?

Best Regards,
Hayes

Re: [RFC PATCH 0/4] r8169: support dash

[Jakub Kicinski]

On Tue, 7 Dec 2021 07:28:02 +0000 Hayes Wang wrote:
> Jakub Kicinski <kuba@kernel.org>
> > Ah, I've only spotted the enable/disable knob in the patch.
> > If you're exchanging arbitrary binary data with the FW we
> > can't help you. It's not going to fly upstream.   
> 
> How is it that we only provide certain basic settings,
> such as IPv4 address, IPv6 address, and so on? Then,
> they are not the arbitrary binary data.
> 
> Could devlink param be used for more than 4 bytes settings?
> At least the IPv6 address is longer.

We can add a new devlink sub-command and driver callback in that case.

> Besides, we need the information of SMBIOS which could
> be 4K ~ 8K bytes data. Is there any way we could transmit
> it to firmware?

Is structure of that data defined by some DMTF standard?

[Intel-wired-lan] [RFC PATCH 0/4] r8169: support dash

[Jakub Kicinski]

On Tue, 7 Dec 2021 07:28:02 +0000 Hayes Wang wrote:
> Jakub Kicinski <kuba@kernel.org>
> > Ah, I've only spotted the enable/disable knob in the patch.
> > If you're exchanging arbitrary binary data with the FW we
> > can't help you. It's not going to fly upstream.   
> 
> How is it that we only provide certain basic settings,
> such as IPv4 address, IPv6 address, and so on? Then,
> they are not the arbitrary binary data.
> 
> Could devlink param be used for more than 4 bytes settings?
> At least the IPv6 address is longer.

We can add a new devlink sub-command and driver callback in that case.

> Besides, we need the information of SMBIOS which could
> be 4K ~ 8K bytes data. Is there any way we could transmit
> it to firmware?

Is structure of that data defined by some DMTF standard?

RE: [RFC PATCH 0/4] r8169: support dash

[Hayes Wang]

Jakub Kicinski <kuba@kernel.org>
> Sent: Wednesday, December 8, 2021 12:21 PM
[...]
> > Could devlink param be used for more than 4 bytes settings?
> > At least the IPv6 address is longer.
> 
> We can add a new devlink sub-command and driver callback in that case.

Excuse me. Do you mean someone will add it? Then, I could use it.
Or, I have to add it.

> > Besides, we need the information of SMBIOS which could
> > be 4K ~ 8K bytes data. Is there any way we could transmit
> > it to firmware?
> 
> Is structure of that data defined by some DMTF standard?
Yes.

Best Regards,
Hayes

[Intel-wired-lan] [RFC PATCH 0/4] r8169: support dash

[Hayes Wang]

Jakub Kicinski <kuba@kernel.org>
> Sent: Wednesday, December 8, 2021 12:21 PM
[...]
> > Could devlink param be used for more than 4 bytes settings?
> > At least the IPv6 address is longer.
> 
> We can add a new devlink sub-command and driver callback in that case.

Excuse me. Do you mean someone will add it? Then, I could use it.
Or, I have to add it.

> > Besides, we need the information of SMBIOS which could
> > be 4K ~ 8K bytes data. Is there any way we could transmit
> > it to firmware?
> 
> Is structure of that data defined by some DMTF standard?
Yes.

Best Regards,
Hayes

Re: [RFC PATCH 0/4] r8169: support dash

[Jakub Kicinski]

On Wed, 8 Dec 2021 07:53:28 +0000 Hayes Wang wrote:
> Jakub Kicinski <kuba@kernel.org>
> > Sent: Wednesday, December 8, 2021 12:21 PM  
> > > Could devlink param be used for more than 4 bytes settings?
> > > At least the IPv6 address is longer.  
> > 
> > We can add a new devlink sub-command and driver callback in that case.  
> 
> Excuse me. Do you mean someone will add it? Then, I could use it.
> Or, I have to add it.

You'd need to write all the code.

> > > Besides, we need the information of SMBIOS which could
> > > be 4K ~ 8K bytes data. Is there any way we could transmit
> > > it to firmware?  
> > 
> > Is structure of that data defined by some DMTF standard?  
> Yes.

That's good, as long as the kernel parses and validates the messages
the exact interface to user space does not matter that much.

[Intel-wired-lan] [RFC PATCH 0/4] r8169: support dash

[Jakub Kicinski]

On Wed, 8 Dec 2021 07:53:28 +0000 Hayes Wang wrote:
> Jakub Kicinski <kuba@kernel.org>
> > Sent: Wednesday, December 8, 2021 12:21 PM  
> > > Could devlink param be used for more than 4 bytes settings?
> > > At least the IPv6 address is longer.  
> > 
> > We can add a new devlink sub-command and driver callback in that case.  
> 
> Excuse me. Do you mean someone will add it? Then, I could use it.
> Or, I have to add it.

You'd need to write all the code.

> > > Besides, we need the information of SMBIOS which could
> > > be 4K ~ 8K bytes data. Is there any way we could transmit
> > > it to firmware?  
> > 
> > Is structure of that data defined by some DMTF standard?  
> Yes.

That's good, as long as the kernel parses and validates the messages
the exact interface to user space does not matter that much.

RE: [RFC PATCH 0/4] r8169: support dash

[Hayes Wang]

Jakub Kicinski <kuba@kernel.org>
> Sent: Thursday, December 9, 2021 5:38 AM
[...]
> > Excuse me. Do you mean someone will add it? Then, I could use it.
> > Or, I have to add it.
> 
> You'd need to write all the code.
I need more time to think how to do.
Thanks.

Best Regards,
Hayes

[Intel-wired-lan] [RFC PATCH 0/4] r8169: support dash

[Hayes Wang]

Jakub Kicinski <kuba@kernel.org>
> Sent: Thursday, December 9, 2021 5:38 AM
[...]
> > Excuse me. Do you mean someone will add it? Then, I could use it.
> > Or, I have to add it.
> 
> You'd need to write all the code.
I need more time to think how to do.
Thanks.

Best Regards,
Hayes

Re: [RFC PATCH 4/4] r8169: add sysfs for dash

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 18592 bytes --]

CC: llvm(a)lists.linux.dev
CC: kbuild-all(a)lists.01.org
In-Reply-To: <20211129101315.16372-385-nic_swsd@realtek.com>
References: <20211129101315.16372-385-nic_swsd@realtek.com>
TO: Hayes Wang <hayeswang@realtek.com>

Hi Hayes,

[FYI, it's a private test report for your RFC patch.]
[auto build test WARNING on horms-ipvs/master]
[also build test WARNING on linus/master v5.16-rc3 next-20211203]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Hayes-Wang/r8169-support-dash/20211129-181721
base:   https://git.kernel.org/pub/scm/linux/kernel/git/horms/ipvs.git master
:::::: branch date: 5 days ago
:::::: commit date: 5 days ago
config: i386-randconfig-c001-20211130 (https://download.01.org/0day-ci/archive/20211205/202112050307.xxJGRfym-lkp(a)intel.com/config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project 25eb7fa01d7ebbe67648ea03841cda55b4239ab2)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/e0b1115782b23f3f7cc28430c69ed35655dbf7eb
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Hayes-Wang/r8169-support-dash/20211129-181721
        git checkout e0b1115782b23f3f7cc28430c69ed35655dbf7eb
        # save the config file to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=i386 clang-analyzer 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>


clang-analyzer warnings: (new ones prefixed by >>)
   #define smp_store_release(p, v) __smp_store_release(p, v)
                                   ^
   arch/x86/include/asm/barrier.h:67:2: note: expanded from macro '__smp_store_release'
           WRITE_ONCE(*p, v);                                              \
           ^
   note: (skipping 2 expansions in backtrace; use -fmacro-backtrace-limit=0 to see all)
   include/linux/compiler_types.h:335:2: note: expanded from macro 'compiletime_assert'
           _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
           ^
   include/linux/compiler_types.h:323:2: note: expanded from macro '_compiletime_assert'
           __compiletime_assert(condition, msg, prefix, suffix)
           ^
   include/linux/compiler_types.h:315:3: note: expanded from macro '__compiletime_assert'
                   if (!(condition))                                       \
                   ^
   drivers/target/target_core_alua.c:1809:2: note: Loop condition is false.  Exiting loop
           rcu_assign_pointer(lun->lun_tg_pt_gp, tg_pt_gp);
           ^
   include/linux/rcupdate.h:453:3: note: expanded from macro 'rcu_assign_pointer'
                   smp_store_release(&p, RCU_INITIALIZER((typeof(p))_r_a_p__v)); \
                   ^
   include/asm-generic/barrier.h:138:33: note: expanded from macro 'smp_store_release'
   #define smp_store_release(p, v) __smp_store_release(p, v)
                                   ^
   arch/x86/include/asm/barrier.h:67:2: note: expanded from macro '__smp_store_release'
           WRITE_ONCE(*p, v);                                              \
           ^
   note: (skipping 2 expansions in backtrace; use -fmacro-backtrace-limit=0 to see all)
   include/linux/compiler_types.h:335:2: note: expanded from macro 'compiletime_assert'
           _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
           ^
   include/linux/compiler_types.h:323:2: note: expanded from macro '_compiletime_assert'
           __compiletime_assert(condition, msg, prefix, suffix)
           ^
   include/linux/compiler_types.h:307:2: note: expanded from macro '__compiletime_assert'
           do {                                                            \
           ^
   drivers/target/target_core_alua.c:1809:2: note: Loop condition is false.  Exiting loop
           rcu_assign_pointer(lun->lun_tg_pt_gp, tg_pt_gp);
           ^
   include/linux/rcupdate.h:453:3: note: expanded from macro 'rcu_assign_pointer'
                   smp_store_release(&p, RCU_INITIALIZER((typeof(p))_r_a_p__v)); \
                   ^
   include/asm-generic/barrier.h:138:33: note: expanded from macro 'smp_store_release'
   #define smp_store_release(p, v) __smp_store_release(p, v)
                                   ^
   arch/x86/include/asm/barrier.h:67:2: note: expanded from macro '__smp_store_release'
           WRITE_ONCE(*p, v);                                              \
           ^
   include/asm-generic/rwonce.h:61:2: note: expanded from macro 'WRITE_ONCE'
           __WRITE_ONCE(x, val);                                           \
           ^
   include/asm-generic/rwonce.h:53:35: note: expanded from macro '__WRITE_ONCE'
   #define __WRITE_ONCE(x, val)                                            \
                                                                           ^
   drivers/target/target_core_alua.c:1809:2: note: Loop condition is false.  Exiting loop
           rcu_assign_pointer(lun->lun_tg_pt_gp, tg_pt_gp);
           ^
   include/linux/rcupdate.h:453:3: note: expanded from macro 'rcu_assign_pointer'
                   smp_store_release(&p, RCU_INITIALIZER((typeof(p))_r_a_p__v)); \
                   ^
   include/asm-generic/barrier.h:138:33: note: expanded from macro 'smp_store_release'
   #define smp_store_release(p, v) __smp_store_release(p, v)
                                   ^
   arch/x86/include/asm/barrier.h:67:2: note: expanded from macro '__smp_store_release'
           WRITE_ONCE(*p, v);                                              \
           ^
   include/asm-generic/rwonce.h:58:33: note: expanded from macro 'WRITE_ONCE'
   #define WRITE_ONCE(x, val)                                              \
                                                                           ^
   drivers/target/target_core_alua.c:1809:2: note: Loop condition is false.  Exiting loop
           rcu_assign_pointer(lun->lun_tg_pt_gp, tg_pt_gp);
           ^
   include/linux/rcupdate.h:453:3: note: expanded from macro 'rcu_assign_pointer'
                   smp_store_release(&p, RCU_INITIALIZER((typeof(p))_r_a_p__v)); \
                   ^
   include/asm-generic/barrier.h:138:33: note: expanded from macro 'smp_store_release'
   #define smp_store_release(p, v) __smp_store_release(p, v)
                                   ^
   arch/x86/include/asm/barrier.h:63:39: note: expanded from macro '__smp_store_release'
   #define __smp_store_release(p, v)                                       \
                                                                           ^
   drivers/target/target_core_alua.c:1809:2: note: Loop condition is false.  Exiting loop
           rcu_assign_pointer(lun->lun_tg_pt_gp, tg_pt_gp);
           ^
   include/linux/rcupdate.h:445:44: note: expanded from macro 'rcu_assign_pointer'
   #define rcu_assign_pointer(p, v)                                              \
                                                                                 ^
   drivers/target/target_core_alua.c:1810:41: note: Passing null pointer value via 2nd parameter 'head'
           list_add_tail(&lun->lun_tg_pt_gp_link, &tg_pt_gp->tg_pt_gp_lun_list);
                                                  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/target/target_core_alua.c:1810:2: note: Calling 'list_add_tail'
           list_add_tail(&lun->lun_tg_pt_gp_link, &tg_pt_gp->tg_pt_gp_lun_list);
           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/list.h:102:18: note: Access to field 'prev' results in a dereference of a null pointer (loaded from variable 'head')
           __list_add(new, head->prev, head);
                           ^~~~
   Suppressed 5 warnings (5 in non-user code).
   Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
   13 warnings generated.
>> drivers/net/ethernet/realtek/r8169_main.c:4677:3: warning: Call to function 'strcat' is insecure as it does not provide bounding of the memory buffer. Replace unbounded copy functions with analogous functions that support length arguments such as 'strlcat'. CWE-119 [clang-analyzer-security.insecureAPI.strcpy]
                   strcat(buf, "enable\n");
                   ^~~~~~
   drivers/net/ethernet/realtek/r8169_main.c:4677:3: note: Call to function 'strcat' is insecure as it does not provide bounding of the memory buffer. Replace unbounded copy functions with analogous functions that support length arguments such as 'strlcat'. CWE-119
                   strcat(buf, "enable\n");
                   ^~~~~~
   drivers/net/ethernet/realtek/r8169_main.c:4679:3: warning: Call to function 'strcat' is insecure as it does not provide bounding of the memory buffer. Replace unbounded copy functions with analogous functions that support length arguments such as 'strlcat'. CWE-119 [clang-analyzer-security.insecureAPI.strcpy]
                   strcat(buf, "disable\n");
                   ^~~~~~
   drivers/net/ethernet/realtek/r8169_main.c:4679:3: note: Call to function 'strcat' is insecure as it does not provide bounding of the memory buffer. Replace unbounded copy functions with analogous functions that support length arguments such as 'strlcat'. CWE-119
                   strcat(buf, "disable\n");
                   ^~~~~~
   Suppressed 11 warnings (11 in non-user code).
   Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
   3 warnings generated.
   Suppressed 3 warnings (3 in non-user code).
   Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
   6 warnings generated.
   Suppressed 6 warnings (6 in non-user code).
   Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
   14 warnings generated.
>> drivers/net/ethernet/realtek/r8169_dash.c:882:3: warning: Value stored to 'dest' is never read [clang-analyzer-deadcode.DeadStores]
                   dest += size;
                   ^       ~~~~
   drivers/net/ethernet/realtek/r8169_dash.c:882:3: note: Value stored to 'dest' is never read
                   dest += size;
                   ^       ~~~~
   Suppressed 13 warnings (11 in non-user code, 2 with check filters).
   Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
   11 warnings generated.
   Suppressed 11 warnings (11 in non-user code).
   Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
   17 warnings generated.
   drivers/net/ethernet/sis/sis900.c:532:2: warning: Value stored to 'ret' is never read [clang-analyzer-deadcode.DeadStores]
           ret = 0;
           ^     ~
   drivers/net/ethernet/sis/sis900.c:532:2: note: Value stored to 'ret' is never read
           ret = 0;
           ^     ~
   drivers/net/ethernet/sis/sis900.c:740:3: warning: Value stored to 'status' is never read [clang-analyzer-deadcode.DeadStores]
                   status = mdio_read(net_dev, phy->phy_addr, MII_STATUS);
                   ^        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/net/ethernet/sis/sis900.c:740:3: note: Value stored to 'status' is never read
                   status = mdio_read(net_dev, phy->phy_addr, MII_STATUS);
                   ^        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/net/ethernet/sis/sis900.c:767:23: warning: Access to field 'phy_addr' results in a dereference of a null pointer (loaded from variable 'default_phy') [clang-analyzer-core.NullDereference]
                   sis_priv->cur_phy = default_phy->phy_addr;
                                       ^
   drivers/net/ethernet/sis/sis900.c:1325:11: note: Value assigned to field 'first_mii'
           status = mdio_read(net_dev, sis_priv->cur_phy, MII_STATUS);
                    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/net/ethernet/sis/sis900.c:1328:6: note: Assuming the condition is true
           if (!netif_carrier_ok(net_dev)) {
               ^~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/net/ethernet/sis/sis900.c:1328:2: note: Taking true branch
           if (!netif_carrier_ok(net_dev)) {
           ^
   drivers/net/ethernet/sis/sis900.c:1331:12: note: Calling 'sis900_default_phy'
                   status = sis900_default_phy(net_dev);
                            ^~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/net/ethernet/sis/sis900.c:739:39: note: Assuming pointer value is null
           for (phy=sis_priv->first_mii; phy; phy=phy->next) {
                                         ^~~
   drivers/net/ethernet/sis/sis900.c:739:9: note: Loop condition is false. Execution continues on line 758
           for (phy=sis_priv->first_mii; phy; phy=phy->next) {
           ^
   drivers/net/ethernet/sis/sis900.c:758:7: note: 'default_phy' is null
           if (!default_phy && phy_home)
                ^~~~~~~~~~~
   drivers/net/ethernet/sis/sis900.c:758:6: note: Left side of '&&' is true
           if (!default_phy && phy_home)
               ^
   drivers/net/ethernet/sis/sis900.c:758:22: note: 'phy_home' is null
           if (!default_phy && phy_home)
                               ^~~~~~~~
   drivers/net/ethernet/sis/sis900.c:758:2: note: Taking false branch
           if (!default_phy && phy_home)
           ^
   drivers/net/ethernet/sis/sis900.c:760:12: note: 'default_phy' is null
           else if (!default_phy && phy_lan)
                     ^~~~~~~~~~~
   drivers/net/ethernet/sis/sis900.c:760:11: note: Left side of '&&' is true
           else if (!default_phy && phy_lan)
                    ^
   drivers/net/ethernet/sis/sis900.c:760:27: note: 'phy_lan' is null
           else if (!default_phy && phy_lan)
                                    ^~~~~~~
   drivers/net/ethernet/sis/sis900.c:760:7: note: Taking false branch
           else if (!default_phy && phy_lan)
                ^
   drivers/net/ethernet/sis/sis900.c:762:12: note: 'default_phy' is null
           else if (!default_phy)
                     ^~~~~~~~~~~
   drivers/net/ethernet/sis/sis900.c:762:7: note: Taking true branch
           else if (!default_phy)
                ^
   drivers/net/ethernet/sis/sis900.c:763:3: note: Null pointer value stored to 'default_phy'
                   default_phy = sis_priv->first_mii;
                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/net/ethernet/sis/sis900.c:765:6: note: Assuming 'default_phy' is not equal to field 'mii'
           if (sis_priv->mii != default_phy) {
               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/net/ethernet/sis/sis900.c:765:2: note: Taking true branch
           if (sis_priv->mii != default_phy) {
           ^
   drivers/net/ethernet/sis/sis900.c:767:23: note: Access to field 'phy_addr' results in a dereference of a null pointer (loaded from variable 'default_phy')
                   sis_priv->cur_phy = default_phy->phy_addr;
                                       ^~~~~~~~~~~
   drivers/net/ethernet/sis/sis900.c:778:2: warning: Value stored to 'status' is never read [clang-analyzer-deadcode.DeadStores]
           status = mdio_read(net_dev, sis_priv->cur_phy, MII_STATUS);
           ^        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/net/ethernet/sis/sis900.c:778:2: note: Value stored to 'status' is never read
           status = mdio_read(net_dev, sis_priv->cur_phy, MII_STATUS);
           ^        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/net/ethernet/sis/sis900.c:1324:2: warning: Value stored to 'status' is never read [clang-analyzer-deadcode.DeadStores]
           status = mdio_read(net_dev, sis_priv->cur_phy, MII_STATUS);
           ^        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/net/ethernet/sis/sis900.c:1324:2: note: Value stored to 'status' is never read
           status = mdio_read(net_dev, sis_priv->cur_phy, MII_STATUS);
           ^        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/net/ethernet/sis/sis900.c:1332:3: warning: Value stored to 'mii_phy' is never read [clang-analyzer-deadcode.DeadStores]
                   mii_phy = sis_priv->mii;

vim +4677 drivers/net/ethernet/realtek/r8169_main.c

e0b1115782b23f Hayes Wang 2021-11-29  4661  
e0b1115782b23f Hayes Wang 2021-11-29  4662  static ssize_t
e0b1115782b23f Hayes Wang 2021-11-29  4663  ap_ready_show(struct device *dev, struct device_attribute *attr, char *buf)
e0b1115782b23f Hayes Wang 2021-11-29  4664  {
e0b1115782b23f Hayes Wang 2021-11-29  4665  	struct net_device *net = to_net_dev(dev);
e0b1115782b23f Hayes Wang 2021-11-29  4666  	struct rtl8169_private *tp = netdev_priv(net);
e0b1115782b23f Hayes Wang 2021-11-29  4667  	bool enable;
e0b1115782b23f Hayes Wang 2021-11-29  4668  
e0b1115782b23f Hayes Wang 2021-11-29  4669  	if (rtnl_lock_killable())
e0b1115782b23f Hayes Wang 2021-11-29  4670  		return -EINTR;
e0b1115782b23f Hayes Wang 2021-11-29  4671  
e0b1115782b23f Hayes Wang 2021-11-29  4672  	enable = rtl_dash_get_ap_ready(tp->rtl_dash);
e0b1115782b23f Hayes Wang 2021-11-29  4673  
e0b1115782b23f Hayes Wang 2021-11-29  4674  	rtnl_unlock();
e0b1115782b23f Hayes Wang 2021-11-29  4675  
e0b1115782b23f Hayes Wang 2021-11-29  4676  	if (enable)
e0b1115782b23f Hayes Wang 2021-11-29 @4677  		strcat(buf, "enable\n");
e0b1115782b23f Hayes Wang 2021-11-29  4678  	else
e0b1115782b23f Hayes Wang 2021-11-29  4679  		strcat(buf, "disable\n");
e0b1115782b23f Hayes Wang 2021-11-29  4680  
e0b1115782b23f Hayes Wang 2021-11-29  4681  	return (strlen(buf) + 1);
e0b1115782b23f Hayes Wang 2021-11-29  4682  }
e0b1115782b23f Hayes Wang 2021-11-29  4683  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org