[PATCH 0/7] e1000 and rtl8169 drivers fixes

[PATCH 0/7] e1000 and rtl8169 drivers fixes

[Denis Orlov]

Denis Orlov (7):
  net: e1000: remove superfluous allocation check
  net: e1000: properly read/write MDI registers on 8254x cards
  net: e1000: do not actually acquire/put swfw_sync on 8254x cards
  net: e1000: configure tx/rx and rctl in open() instead of init()
  net: rtl8169: properly map rx/tx buffers for dma
  net: rtl8169: enable pci device bus mastering on ifup
  net: rtl8169: free allocated memory on halt()

 drivers/net/e1000/e1000.h |  3 +++
 drivers/net/e1000/main.c  | 44 +++++++++++++++++++++++++++------
 drivers/net/rtl8169.c     | 51 ++++++++++++++++++++++++++-------------
 3 files changed, 73 insertions(+), 25 deletions(-)

-- 
2.20.1

[PATCH 1/7] net: e1000: remove superfluous allocation check

[Denis Orlov]

dma_alloc() calls xmemalign which panics if unable to allocate memory,
so there is no need to check the returned pointer for validity.

Signed-off-by: Denis Orlov <denorl2009@gmail.com>
---
 drivers/net/e1000/main.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/net/e1000/main.c b/drivers/net/e1000/main.c
index f67c5d867b..bfb8d8ff2f 100644
--- a/drivers/net/e1000/main.c
+++ b/drivers/net/e1000/main.c
@@ -3575,9 +3575,6 @@ static int e1000_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 	edev->priv = hw;
 
 	hw->packet = dma_alloc(PAGE_SIZE);
-	if (!hw->packet)
-		return -ENOMEM;
-
 	hw->packet_dma = dma_map_single(hw->dev, hw->packet, PAGE_SIZE,
 					DMA_FROM_DEVICE);
 	if (dma_mapping_error(hw->dev, hw->packet_dma))
-- 
2.20.1

[PATCH 2/7] net: e1000: properly read/write MDI registers on 8254x cards

[Denis Orlov]

Taken from Linux e1000 driver.

Signed-off-by: Denis Orlov <denorl2009@gmail.com>
---
 drivers/net/e1000/e1000.h |  3 +++
 drivers/net/e1000/main.c  | 22 ++++++++++++++++++++++
 2 files changed, 25 insertions(+)

diff --git a/drivers/net/e1000/e1000.h b/drivers/net/e1000/e1000.h
index 52ad3d4cdb..3a5ee9988e 100644
--- a/drivers/net/e1000/e1000.h
+++ b/drivers/net/e1000/e1000.h
@@ -1388,6 +1388,9 @@ struct e1000_eeprom_info {
 #define PHY_1000T_STATUS		0x0A	/* 1000Base-T Status Reg */
 #define PHY_EXT_STATUS			0x0F	/* Extended Status Reg */
 
+#define MAX_PHY_REG_ADDRESS		0x1F	/* 5 bit address bus (0-0x1F) */
+#define MAX_PHY_MULTI_PAGE_REG		0xF	/* Registers equal on all pages */
+
 /* M88E1000 Specific Registers */
 #define M88E1000_PHY_SPEC_CTRL		0x10	/* PHY Specific Control Register */
 #define M88E1000_PHY_SPEC_STATUS	0x11	/* PHY Specific Status Register */
diff --git a/drivers/net/e1000/main.c b/drivers/net/e1000/main.c
index bfb8d8ff2f..3b45fb3b51 100644
--- a/drivers/net/e1000/main.c
+++ b/drivers/net/e1000/main.c
@@ -54,6 +54,8 @@ static int e1000_get_speed_and_duplex(struct e1000_hw *hw, uint16_t *speed,
 				       uint16_t *duplex);
 static int e1000_read_phy_reg(struct e1000_hw *hw, uint32_t reg_addr,
 			      uint16_t *phy_data);
+static int e1000_phy_write(struct mii_bus *bus, int phy_addr, int reg_addr,
+			   u16 phy_data);
 static int e1000_write_phy_reg(struct e1000_hw *hw, uint32_t reg_addr,
 			       uint16_t phy_data);
 static int32_t e1000_phy_hw_reset(struct e1000_hw *hw);
@@ -2627,6 +2629,15 @@ static int e1000_read_phy_reg(struct e1000_hw *hw, uint32_t reg_addr,
 {
 	int ret;
 
+	if ((hw->phy_type == e1000_phy_igp) && (reg_addr > MAX_PHY_MULTI_PAGE_REG)) {
+		ret = e1000_phy_write(&hw->miibus, 1, IGP01E1000_PHY_PAGE_SELECT,
+				      (u16)reg_addr);
+		if (ret)
+			return ret;
+
+		reg_addr &= MAX_PHY_REG_ADDRESS;
+	}
+
 	ret = e1000_phy_read(&hw->miibus, 1, reg_addr);
 	if (ret < 0)
 		return ret;
@@ -2702,6 +2713,17 @@ static int e1000_phy_write(struct mii_bus *bus, int phy_addr,
  ******************************************************************************/
 static int e1000_write_phy_reg(struct e1000_hw *hw, uint32_t reg_addr, uint16_t phy_data)
 {
+	int ret;
+
+	if ((hw->phy_type == e1000_phy_igp) && (reg_addr > MAX_PHY_MULTI_PAGE_REG)) {
+		ret = e1000_phy_write(&hw->miibus, 1, IGP01E1000_PHY_PAGE_SELECT,
+				      (u16)reg_addr);
+		if (ret)
+			return ret;
+
+		reg_addr &= MAX_PHY_REG_ADDRESS;
+	}
+
 	return e1000_phy_write(&hw->miibus, 1, reg_addr, phy_data);
 }
 
-- 
2.20.1

[PATCH 3/7] net: e1000: do not actually acquire/put swfw_sync on 8254x cards

[Denis Orlov]

There is no Software-Firmware Synchronization register on these
controllers.

Signed-off-by: Denis Orlov <denorl2009@gmail.com>
---
 drivers/net/e1000/main.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/net/e1000/main.c b/drivers/net/e1000/main.c
index 3b45fb3b51..4ca598eb2f 100644
--- a/drivers/net/e1000/main.c
+++ b/drivers/net/e1000/main.c
@@ -245,6 +245,10 @@ int32_t e1000_swfw_sync_acquire(struct e1000_hw *hw, uint16_t mask)
 	int32_t timeout = 200;
 
 	DEBUGFUNC();
+
+	if (hw->mac_type <= e1000_82547_rev_2)
+		return E1000_SUCCESS;
+
 	while (timeout) {
 		if (e1000_get_hw_eeprom_semaphore(hw))
 			return -E1000_ERR_SWFW_SYNC;
@@ -276,6 +280,9 @@ int32_t e1000_swfw_sync_release(struct e1000_hw *hw, uint16_t mask)
 {
 	uint32_t swfw_sync;
 
+	if (hw->mac_type <= e1000_82547_rev_2)
+		return E1000_SUCCESS;
+
 	if (e1000_get_hw_eeprom_semaphore(hw))
 		return -E1000_ERR_SWFW_SYNC;
 
-- 
2.20.1

[PATCH 4/7] net: e1000: configure tx/rx and rctl in open() instead of init()

[Denis Orlov]

Otherwise, ifdowning the interface leads to a state in which trying to
ifup it again fails.

Signed-off-by: Denis Orlov <denorl2009@gmail.com>
---
 drivers/net/e1000/main.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/net/e1000/main.c b/drivers/net/e1000/main.c
index 4ca598eb2f..363730de0a 100644
--- a/drivers/net/e1000/main.c
+++ b/drivers/net/e1000/main.c
@@ -62,7 +62,9 @@ static int32_t e1000_phy_hw_reset(struct e1000_hw *hw);
 static int e1000_phy_reset(struct e1000_hw *hw);
 static int e1000_detect_gig_phy(struct e1000_hw *hw);
 static void e1000_set_media_type(struct e1000_hw *hw);
-
+static void e1000_configure_tx(struct e1000_hw *hw);
+static void e1000_configure_rx(struct e1000_hw *hw);
+static void e1000_setup_rctl(struct e1000_hw *hw);
 
 static int32_t e1000_check_phy_reset_block(struct e1000_hw *hw);
 
@@ -811,6 +813,10 @@ static int e1000_open(struct eth_device *edev)
 		e1000_write_reg(hw, E1000_CTRL_EXT, ctrl_ext);
 	}
 
+	e1000_configure_tx(hw);
+	e1000_configure_rx(hw);
+	e1000_setup_rctl(hw);
+
 	return 0;
 }
 
@@ -3575,10 +3581,6 @@ static int e1000_init(struct eth_device *edev)
 	if (hw->mac_type == e1000_igb)
 		mdelay(15);
 
-	e1000_configure_tx(hw);
-	e1000_configure_rx(hw);
-	e1000_setup_rctl(hw);
-
 	return 0;
 }
 
-- 
2.20.1

[PATCH 5/7] net: rtl8169: properly map rx/tx buffers for dma

[Denis Orlov]

Signed-off-by: Denis Orlov <denorl2009@gmail.com>
---
 drivers/net/rtl8169.c | 37 ++++++++++++++++++++-----------------
 1 file changed, 20 insertions(+), 17 deletions(-)

diff --git a/drivers/net/rtl8169.c b/drivers/net/rtl8169.c
index 341f5f240e..e78cc40ffe 100644
--- a/drivers/net/rtl8169.c
+++ b/drivers/net/rtl8169.c
@@ -42,11 +42,13 @@ struct rtl8169_priv {
 	volatile struct bufdesc	*tx_desc;
 	dma_addr_t		tx_desc_phys;
 	void			*tx_buf;
+	dma_addr_t		tx_buf_phys;
 	unsigned int		cur_tx;
 
 	volatile struct bufdesc	*rx_desc;
 	dma_addr_t		rx_desc_phys;
 	void			*rx_buf;
+	dma_addr_t		rx_buf_phys;
 	unsigned int		cur_rx;
 
 	struct mii_bus miibus;
@@ -218,14 +220,17 @@ static void rtl8169_init_ring(struct rtl8169_priv *priv)
 
 	priv->cur_rx = priv->cur_tx = 0;
 
-	priv->tx_desc = dma_alloc_coherent(NUM_TX_DESC *
-				sizeof(struct bufdesc), &priv->tx_desc_phys);
+	priv->tx_desc = dma_alloc_coherent(NUM_TX_DESC * sizeof(struct bufdesc),
+					   &priv->tx_desc_phys);
 	priv->tx_buf = malloc(NUM_TX_DESC * PKT_BUF_SIZE);
-	priv->rx_desc = dma_alloc_coherent(NUM_RX_DESC *
-				sizeof(struct bufdesc), &priv->rx_desc_phys);
+	priv->tx_buf_phys = dma_map_single(&priv->edev.dev, priv->tx_buf,
+					   NUM_TX_DESC * PKT_BUF_SIZE, DMA_TO_DEVICE);
+
+	priv->rx_desc = dma_alloc_coherent(NUM_RX_DESC * sizeof(struct bufdesc),
+					   &priv->rx_desc_phys);
 	priv->rx_buf = malloc(NUM_RX_DESC * PKT_BUF_SIZE);
-	dma_sync_single_for_device((unsigned long)priv->rx_buf,
-				   NUM_RX_DESC * PKT_BUF_SIZE, DMA_FROM_DEVICE);
+	priv->rx_buf_phys = dma_map_single(&priv->edev.dev, priv->rx_buf,
+					   NUM_RX_DESC * PKT_BUF_SIZE, DMA_FROM_DEVICE);
 
 	for (i = 0; i < NUM_RX_DESC; i++) {
 		if (i == (NUM_RX_DESC - 1))
@@ -236,7 +241,7 @@ static void rtl8169_init_ring(struct rtl8169_priv *priv)
 				cpu_to_le32(BD_STAT_OWN | PKT_BUF_SIZE);
 
 		priv->rx_desc[i].buf_addr =
-				cpu_to_le32(virt_to_phys(priv->rx_buf + i * PKT_BUF_SIZE));
+				cpu_to_le32(priv->rx_buf_phys + i * PKT_BUF_SIZE);
 	}
 }
 
@@ -353,12 +358,12 @@ static int rtl8169_eth_send(struct eth_device *edev, void *packet,
 	if (packet_length < ETH_ZLEN)
 		memset(priv->tx_buf + entry * PKT_BUF_SIZE, 0, ETH_ZLEN);
 	memcpy(priv->tx_buf + entry * PKT_BUF_SIZE, packet, packet_length);
-	dma_sync_single_for_device((unsigned long)priv->tx_buf + entry *
+	dma_sync_single_for_device(priv->tx_buf_phys + entry *
 				   PKT_BUF_SIZE, PKT_BUF_SIZE, DMA_TO_DEVICE);
 
 	priv->tx_desc[entry].buf_Haddr = 0;
 	priv->tx_desc[entry].buf_addr =
-		cpu_to_le32(virt_to_phys(priv->tx_buf + entry * PKT_BUF_SIZE));
+		cpu_to_le32(priv->tx_buf_phys + entry * PKT_BUF_SIZE);
 
 	if (entry != (NUM_TX_DESC - 1)) {
 		priv->tx_desc[entry].status =
@@ -375,8 +380,8 @@ static int rtl8169_eth_send(struct eth_device *edev, void *packet,
 	while (le32_to_cpu(priv->tx_desc[entry].status) & BD_STAT_OWN)
 		;
 
-	dma_sync_single_for_cpu((unsigned long)priv->tx_buf + entry *
-				PKT_BUF_SIZE, PKT_BUF_SIZE, DMA_TO_DEVICE);
+	dma_sync_single_for_cpu(priv->tx_buf_phys + entry * PKT_BUF_SIZE,
+				PKT_BUF_SIZE, DMA_TO_DEVICE);
 
 	priv->cur_tx++;
 
@@ -395,15 +400,13 @@ static int rtl8169_eth_rx(struct eth_device *edev)
 		if (!(le32_to_cpu(priv->rx_desc[entry].status) & BD_STAT_RX_RES)) {
 			pkt_size = (le32_to_cpu(priv->rx_desc[entry].status) & 0x1fff) - 4;
 
-			dma_sync_single_for_cpu((unsigned long)priv->rx_buf
-						+ entry * PKT_BUF_SIZE,
+			dma_sync_single_for_cpu(priv->rx_buf_phys + entry * PKT_BUF_SIZE,
 						pkt_size, DMA_FROM_DEVICE);
 
 			net_receive(edev, priv->rx_buf + entry * PKT_BUF_SIZE,
 			            pkt_size);
 
-			dma_sync_single_for_device((unsigned long)priv->rx_buf
-						   + entry * PKT_BUF_SIZE,
+			dma_sync_single_for_device(priv->rx_buf_phys + entry * PKT_BUF_SIZE,
 						   pkt_size, DMA_FROM_DEVICE);
 
 			if (entry == NUM_RX_DESC - 1)
@@ -413,8 +416,8 @@ static int rtl8169_eth_rx(struct eth_device *edev)
 				priv->rx_desc[entry].status =
 					cpu_to_le32(BD_STAT_OWN | PKT_BUF_SIZE);
 			priv->rx_desc[entry].buf_addr =
-				cpu_to_le32(virt_to_phys(priv->rx_buf +
-							 entry * PKT_BUF_SIZE));
+				cpu_to_le32(priv->rx_buf_phys +
+					    entry * PKT_BUF_SIZE);
 		} else {
 			dev_err(&edev->dev, "rx error\n");
 		}
-- 
2.20.1

[PATCH 6/7] net: rtl8169: enable pci device bus mastering on ifup

[Denis Orlov]

Otherwise, the board hangs on doing ifup after ifdown as the latter
disables bus mastering for the device in rtl8169_eth_halt().

Signed-off-by: Denis Orlov <denorl2009@gmail.com>
---
 drivers/net/rtl8169.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/rtl8169.c b/drivers/net/rtl8169.c
index e78cc40ffe..4b40c539dd 100644
--- a/drivers/net/rtl8169.c
+++ b/drivers/net/rtl8169.c
@@ -293,6 +293,8 @@ static int rtl8169_eth_open(struct eth_device *edev)
 	struct rtl8169_priv *priv = edev->priv;
 	int ret;
 
+	pci_set_master(priv->pci_dev);
+
 	rtl8169_init_ring(priv);
 	rtl8169_hw_start(priv);
 
-- 
2.20.1

[PATCH 7/7] net: rtl8169: free allocated memory on halt()

[Denis Orlov]

Otherwise it just leaks.

Signed-off-by: Denis Orlov <denorl2009@gmail.com>
---
 drivers/net/rtl8169.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/net/rtl8169.c b/drivers/net/rtl8169.c
index 4b40c539dd..e923e179bf 100644
--- a/drivers/net/rtl8169.c
+++ b/drivers/net/rtl8169.c
@@ -478,6 +478,18 @@ static void rtl8169_eth_halt(struct eth_device *edev)
 	RTL_W32(priv, RxMissed, 0);
 
 	pci_clear_master(priv->pci_dev);
+
+	dma_unmap_single(&edev->dev, priv->tx_buf_phys, NUM_TX_DESC * PKT_BUF_SIZE,
+			 DMA_TO_DEVICE);
+	free(priv->tx_buf);
+	dma_free_coherent((void *)priv->tx_desc, priv->tx_desc_phys,
+			  NUM_TX_DESC * sizeof(struct bufdesc));
+
+	dma_unmap_single(&edev->dev, priv->rx_buf_phys, NUM_RX_DESC * PKT_BUF_SIZE,
+			 DMA_FROM_DEVICE);
+	free(priv->rx_buf);
+	dma_free_coherent((void *)priv->rx_desc, priv->rx_desc_phys,
+			  NUM_RX_DESC * sizeof(struct bufdesc));
 }
 
 static int rtl8169_probe(struct pci_dev *pdev, const struct pci_device_id *id)
-- 
2.20.1

Re: [PATCH 0/7] e1000 and rtl8169 drivers fixes

[Sascha Hauer]

On Wed, Jul 20, 2022 at 04:30:53PM +0300, Denis Orlov wrote:
> Denis Orlov (7):
>   net: e1000: remove superfluous allocation check
>   net: e1000: properly read/write MDI registers on 8254x cards
>   net: e1000: do not actually acquire/put swfw_sync on 8254x cards
>   net: e1000: configure tx/rx and rctl in open() instead of init()
>   net: rtl8169: properly map rx/tx buffers for dma
>   net: rtl8169: enable pci device bus mastering on ifup
>   net: rtl8169: free allocated memory on halt()
> 
>  drivers/net/e1000/e1000.h |  3 +++
>  drivers/net/e1000/main.c  | 44 +++++++++++++++++++++++++++------
>  drivers/net/rtl8169.c     | 51 ++++++++++++++++++++++++++-------------
>  3 files changed, 73 insertions(+), 25 deletions(-)

Applied, thanks

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |