diff mbox series

[4/6] ethernet: stmmac: fix dma physical address of descriptor when display ring

Message ID 20210111113538.12077-5-qiangqing.zhang@nxp.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series ethernet: fixes for stmmac driver | expand

Checks

Context Check Description
netdev/cover_letter success Link
netdev/fixes_present success Link
netdev/patch_count success Link
netdev/tree_selection success Guessed tree name to be net-next
netdev/subject_prefix warning Target tree name not specified in the subject
netdev/cc_maintainers warning 3 maintainers not CCed: linux-stm32@st-md-mailman.stormreply.com mcoquelin.stm32@gmail.com linux-arm-kernel@lists.infradead.org
netdev/source_inline success Was 0 now: 0
netdev/verify_signedoff success Link
netdev/module_param success Was 0 now: 0
netdev/build_32bit success Errors and warnings before: 22 this patch: 22
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/verify_fixes success Link
netdev/checkpatch warning WARNING: line length of 82 exceeds 80 columns WARNING: line length of 83 exceeds 80 columns WARNING: line length of 84 exceeds 80 columns WARNING: line length of 88 exceeds 80 columns
netdev/build_allmodconfig_warn success Errors and warnings before: 22 this patch: 22
netdev/header_inline success Link
netdev/stable success Stable not CCed

Commit Message

Joakim Zhang Jan. 11, 2021, 11:35 a.m. UTC
Driver uses dma_alloc_coherent to allocate dma memory for descriptors,
dma_alloc_coherent will return both the virtual address and physical
address. AFAIK, virt_to_phys could not convert virtual address to
physical address, for which memory is allocated by dma_alloc_coherent.

Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com>
---
 .../ethernet/stmicro/stmmac/dwmac4_descs.c    |  5 +-
 .../net/ethernet/stmicro/stmmac/enh_desc.c    |  5 +-
 drivers/net/ethernet/stmicro/stmmac/hwif.h    |  3 +-
 .../net/ethernet/stmicro/stmmac/norm_desc.c   |  5 +-
 .../net/ethernet/stmicro/stmmac/stmmac_main.c | 50 ++++++++++++-------
 5 files changed, 44 insertions(+), 24 deletions(-)

Comments

Florian Fainelli Jan. 11, 2021, 5:30 p.m. UTC | #1
On 1/11/21 3:35 AM, Joakim Zhang wrote:
> Driver uses dma_alloc_coherent to allocate dma memory for descriptors,
> dma_alloc_coherent will return both the virtual address and physical
> address. AFAIK, virt_to_phys could not convert virtual address to
> physical address, for which memory is allocated by dma_alloc_coherent.
> 
> Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com>
> ---
>  .../ethernet/stmicro/stmmac/dwmac4_descs.c    |  5 +-
>  .../net/ethernet/stmicro/stmmac/enh_desc.c    |  5 +-
>  drivers/net/ethernet/stmicro/stmmac/hwif.h    |  3 +-
>  .../net/ethernet/stmicro/stmmac/norm_desc.c   |  5 +-
>  .../net/ethernet/stmicro/stmmac/stmmac_main.c | 50 ++++++++++++-------
>  5 files changed, 44 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c b/drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c
> index c6540b003b43..8e1ee33ba1e6 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c
> @@ -402,7 +402,8 @@ static void dwmac4_rd_set_tx_ic(struct dma_desc *p)
>  	p->des2 |= cpu_to_le32(TDES2_INTERRUPT_ON_COMPLETION);
>  }
>  
> -static void dwmac4_display_ring(void *head, unsigned int size, bool rx)
> +static void dwmac4_display_ring(void *head, unsigned int size, bool rx,
> +				unsigned int dma_rx_phy, unsigned int desc_size)
>  {
>  	struct dma_desc *p = (struct dma_desc *)head;
>  	int i;
> @@ -411,7 +412,7 @@ static void dwmac4_display_ring(void *head, unsigned int size, bool rx)
>  
>  	for (i = 0; i < size; i++) {
>  		pr_info("%03d [0x%x]: 0x%x 0x%x 0x%x 0x%x\n",
> -			i, (unsigned int)virt_to_phys(p),
> +			i, (unsigned int)(dma_rx_phy + i * desc_size),

This code will probably not work correctly on a machine with physical
addresses greater than 32-bit anyway and the address bits would be
truncated then, cannot you use a phy_addr_t or dma_addr_t and use %pa as
far as the printk format goes?
Joakim Zhang Jan. 12, 2021, 1:41 a.m. UTC | #2
> -----Original Message-----
> From: Florian Fainelli <f.fainelli@gmail.com>
> Sent: 2021年1月12日 1:30
> To: Joakim Zhang <qiangqing.zhang@nxp.com>; peppe.cavallaro@st.com;
> alexandre.torgue@st.com; joabreu@synopsys.com; davem@davemloft.net;
> kuba@kernel.org
> Cc: netdev@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>
> Subject: Re: [PATCH 4/6] ethernet: stmmac: fix dma physical address of
> descriptor when display ring
> 
> On 1/11/21 3:35 AM, Joakim Zhang wrote:
> > Driver uses dma_alloc_coherent to allocate dma memory for descriptors,
> > dma_alloc_coherent will return both the virtual address and physical
> > address. AFAIK, virt_to_phys could not convert virtual address to
> > physical address, for which memory is allocated by dma_alloc_coherent.
> >
> > Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com>
> > ---
> >  .../ethernet/stmicro/stmmac/dwmac4_descs.c    |  5 +-
> >  .../net/ethernet/stmicro/stmmac/enh_desc.c    |  5 +-
> >  drivers/net/ethernet/stmicro/stmmac/hwif.h    |  3 +-
> >  .../net/ethernet/stmicro/stmmac/norm_desc.c   |  5 +-
> >  .../net/ethernet/stmicro/stmmac/stmmac_main.c | 50
> > ++++++++++++-------
> >  5 files changed, 44 insertions(+), 24 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c
> > b/drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c
> > index c6540b003b43..8e1ee33ba1e6 100644
> > --- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c
> > +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c
> > @@ -402,7 +402,8 @@ static void dwmac4_rd_set_tx_ic(struct dma_desc
> *p)
> >  	p->des2 |= cpu_to_le32(TDES2_INTERRUPT_ON_COMPLETION);
> >  }
> >
> > -static void dwmac4_display_ring(void *head, unsigned int size, bool
> > rx)
> > +static void dwmac4_display_ring(void *head, unsigned int size, bool rx,
> > +				unsigned int dma_rx_phy, unsigned int desc_size)
> >  {
> >  	struct dma_desc *p = (struct dma_desc *)head;
> >  	int i;
> > @@ -411,7 +412,7 @@ static void dwmac4_display_ring(void *head,
> > unsigned int size, bool rx)
> >
> >  	for (i = 0; i < size; i++) {
> >  		pr_info("%03d [0x%x]: 0x%x 0x%x 0x%x 0x%x\n",
> > -			i, (unsigned int)virt_to_phys(p),
> > +			i, (unsigned int)(dma_rx_phy + i * desc_size),
> 
> This code will probably not work correctly on a machine with physical addresses
> greater than 32-bit anyway and the address bits would be truncated then,
> cannot you use a phy_addr_t or dma_addr_t and use %pa as far as the printk
> format goes?

Thanks, I will fix it.

Best Regards,
Joakim Zhang
> --
> Florian
diff mbox series

Patch

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c b/drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c
index c6540b003b43..8e1ee33ba1e6 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c
@@ -402,7 +402,8 @@  static void dwmac4_rd_set_tx_ic(struct dma_desc *p)
 	p->des2 |= cpu_to_le32(TDES2_INTERRUPT_ON_COMPLETION);
 }
 
-static void dwmac4_display_ring(void *head, unsigned int size, bool rx)
+static void dwmac4_display_ring(void *head, unsigned int size, bool rx,
+				unsigned int dma_rx_phy, unsigned int desc_size)
 {
 	struct dma_desc *p = (struct dma_desc *)head;
 	int i;
@@ -411,7 +412,7 @@  static void dwmac4_display_ring(void *head, unsigned int size, bool rx)
 
 	for (i = 0; i < size; i++) {
 		pr_info("%03d [0x%x]: 0x%x 0x%x 0x%x 0x%x\n",
-			i, (unsigned int)virt_to_phys(p),
+			i, (unsigned int)(dma_rx_phy + i * desc_size),
 			le32_to_cpu(p->des0), le32_to_cpu(p->des1),
 			le32_to_cpu(p->des2), le32_to_cpu(p->des3));
 		p++;
diff --git a/drivers/net/ethernet/stmicro/stmmac/enh_desc.c b/drivers/net/ethernet/stmicro/stmmac/enh_desc.c
index d02cec296f51..a7324b9c1a02 100644
--- a/drivers/net/ethernet/stmicro/stmmac/enh_desc.c
+++ b/drivers/net/ethernet/stmicro/stmmac/enh_desc.c
@@ -417,7 +417,8 @@  static int enh_desc_get_rx_timestamp_status(void *desc, void *next_desc,
 	}
 }
 
-static void enh_desc_display_ring(void *head, unsigned int size, bool rx)
+static void enh_desc_display_ring(void *head, unsigned int size, bool rx,
+				  unsigned int dma_rx_phy, unsigned int desc_size)
 {
 	struct dma_extended_desc *ep = (struct dma_extended_desc *)head;
 	int i;
@@ -429,7 +430,7 @@  static void enh_desc_display_ring(void *head, unsigned int size, bool rx)
 
 		x = *(u64 *)ep;
 		pr_info("%03d [0x%x]: 0x%x 0x%x 0x%x 0x%x\n",
-			i, (unsigned int)virt_to_phys(ep),
+			i, (unsigned int)(dma_rx_phy + i * desc_size),
 			(unsigned int)x, (unsigned int)(x >> 32),
 			ep->basic.des2, ep->basic.des3);
 		ep++;
diff --git a/drivers/net/ethernet/stmicro/stmmac/hwif.h b/drivers/net/ethernet/stmicro/stmmac/hwif.h
index b40b2e0667bb..fc5260cf27ea 100644
--- a/drivers/net/ethernet/stmicro/stmmac/hwif.h
+++ b/drivers/net/ethernet/stmicro/stmmac/hwif.h
@@ -78,7 +78,8 @@  struct stmmac_desc_ops {
 	/* get rx timestamp status */
 	int (*get_rx_timestamp_status)(void *desc, void *next_desc, u32 ats);
 	/* Display ring */
-	void (*display_ring)(void *head, unsigned int size, bool rx);
+	void (*display_ring)(void *head, unsigned int size, bool rx,
+			     unsigned int dma_rx_phy, unsigned int desc_size);
 	/* set MSS via context descriptor */
 	void (*set_mss)(struct dma_desc *p, unsigned int mss);
 	/* get descriptor skbuff address */
diff --git a/drivers/net/ethernet/stmicro/stmmac/norm_desc.c b/drivers/net/ethernet/stmicro/stmmac/norm_desc.c
index f083360e4ba6..c26544de0766 100644
--- a/drivers/net/ethernet/stmicro/stmmac/norm_desc.c
+++ b/drivers/net/ethernet/stmicro/stmmac/norm_desc.c
@@ -269,7 +269,8 @@  static int ndesc_get_rx_timestamp_status(void *desc, void *next_desc, u32 ats)
 		return 1;
 }
 
-static void ndesc_display_ring(void *head, unsigned int size, bool rx)
+static void ndesc_display_ring(void *head, unsigned int size, bool rx,
+			       unsigned int dma_rx_phy, unsigned int desc_size)
 {
 	struct dma_desc *p = (struct dma_desc *)head;
 	int i;
@@ -281,7 +282,7 @@  static void ndesc_display_ring(void *head, unsigned int size, bool rx)
 
 		x = *(u64 *)p;
 		pr_info("%03d [0x%x]: 0x%x 0x%x 0x%x 0x%x",
-			i, (unsigned int)virt_to_phys(p),
+			i, (unsigned int)(dma_rx_phy + i * desc_size),
 			(unsigned int)x, (unsigned int)(x >> 32),
 			p->des2, p->des3);
 		p++;
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 41d9a5a3cc9a..ca24e268f49a 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -1133,6 +1133,7 @@  static int stmmac_phy_setup(struct stmmac_priv *priv)
 static void stmmac_display_rx_rings(struct stmmac_priv *priv)
 {
 	u32 rx_cnt = priv->plat->rx_queues_to_use;
+	unsigned int desc_size;
 	void *head_rx;
 	u32 queue;
 
@@ -1142,19 +1143,24 @@  static void stmmac_display_rx_rings(struct stmmac_priv *priv)
 
 		pr_info("\tRX Queue %u rings\n", queue);
 
-		if (priv->extend_desc)
+		if (priv->extend_desc) {
 			head_rx = (void *)rx_q->dma_erx;
-		else
+			desc_size = sizeof(struct dma_extended_desc);
+		} else {
 			head_rx = (void *)rx_q->dma_rx;
+			desc_size = sizeof(struct dma_desc);
+		}
 
 		/* Display RX ring */
-		stmmac_display_ring(priv, head_rx, priv->dma_rx_size, true);
+		stmmac_display_ring(priv, head_rx, priv->dma_rx_size, true,
+				    rx_q->dma_rx_phy, desc_size);
 	}
 }
 
 static void stmmac_display_tx_rings(struct stmmac_priv *priv)
 {
 	u32 tx_cnt = priv->plat->tx_queues_to_use;
+	unsigned int desc_size;
 	void *head_tx;
 	u32 queue;
 
@@ -1164,14 +1170,19 @@  static void stmmac_display_tx_rings(struct stmmac_priv *priv)
 
 		pr_info("\tTX Queue %d rings\n", queue);
 
-		if (priv->extend_desc)
+		if (priv->extend_desc) {
 			head_tx = (void *)tx_q->dma_etx;
-		else if (tx_q->tbs & STMMAC_TBS_AVAIL)
+			desc_size = sizeof(struct dma_extended_desc);
+		} else if (tx_q->tbs & STMMAC_TBS_AVAIL) {
 			head_tx = (void *)tx_q->dma_entx;
-		else
+			desc_size = sizeof(struct dma_edesc);
+		} else {
 			head_tx = (void *)tx_q->dma_tx;
+			desc_size = sizeof(struct dma_desc);
+		}
 
-		stmmac_display_ring(priv, head_tx, priv->dma_tx_size, false);
+		stmmac_display_ring(priv, head_tx, priv->dma_tx_size, false,
+				    tx_q->dma_tx_phy, desc_size);
 	}
 }
 
@@ -3736,18 +3747,23 @@  static int stmmac_rx(struct stmmac_priv *priv, int limit, u32 queue)
 	unsigned int count = 0, error = 0, len = 0;
 	int status = 0, coe = priv->hw->rx_csum;
 	unsigned int next_entry = rx_q->cur_rx;
+	unsigned int desc_size;
 	struct sk_buff *skb = NULL;
 
 	if (netif_msg_rx_status(priv)) {
 		void *rx_head;
 
 		netdev_dbg(priv->dev, "%s: descriptor ring:\n", __func__);
-		if (priv->extend_desc)
+		if (priv->extend_desc) {
 			rx_head = (void *)rx_q->dma_erx;
-		else
+			desc_size = sizeof(struct dma_extended_desc);
+		} else {
 			rx_head = (void *)rx_q->dma_rx;
+			desc_size = sizeof(struct dma_desc);
+		}
 
-		stmmac_display_ring(priv, rx_head, priv->dma_rx_size, true);
+		stmmac_display_ring(priv, rx_head, priv->dma_rx_size, true,
+				    rx_q->dma_rx_phy, desc_size);
 	}
 	while (count < limit) {
 		unsigned int buf1_len = 0, buf2_len = 0;
@@ -4314,7 +4330,7 @@  static int stmmac_set_mac_address(struct net_device *ndev, void *addr)
 static struct dentry *stmmac_fs_dir;
 
 static void sysfs_display_ring(void *head, int size, int extend_desc,
-			       struct seq_file *seq)
+			       struct seq_file *seq, unsigned int dma_phy_addr)
 {
 	int i;
 	struct dma_extended_desc *ep = (struct dma_extended_desc *)head;
@@ -4323,7 +4339,7 @@  static void sysfs_display_ring(void *head, int size, int extend_desc,
 	for (i = 0; i < size; i++) {
 		if (extend_desc) {
 			seq_printf(seq, "%d [0x%x]: 0x%x 0x%x 0x%x 0x%x\n",
-				   i, (unsigned int)virt_to_phys(ep),
+				   i, (unsigned int)(dma_phy_addr + i * sizeof(ep)),
 				   le32_to_cpu(ep->basic.des0),
 				   le32_to_cpu(ep->basic.des1),
 				   le32_to_cpu(ep->basic.des2),
@@ -4331,7 +4347,7 @@  static void sysfs_display_ring(void *head, int size, int extend_desc,
 			ep++;
 		} else {
 			seq_printf(seq, "%d [0x%x]: 0x%x 0x%x 0x%x 0x%x\n",
-				   i, (unsigned int)virt_to_phys(p),
+				   i, (unsigned int)(dma_phy_addr + i * sizeof(p)),
 				   le32_to_cpu(p->des0), le32_to_cpu(p->des1),
 				   le32_to_cpu(p->des2), le32_to_cpu(p->des3));
 			p++;
@@ -4359,11 +4375,11 @@  static int stmmac_rings_status_show(struct seq_file *seq, void *v)
 		if (priv->extend_desc) {
 			seq_printf(seq, "Extended descriptor ring:\n");
 			sysfs_display_ring((void *)rx_q->dma_erx,
-					   priv->dma_rx_size, 1, seq);
+					   priv->dma_rx_size, 1, seq, rx_q->dma_rx_phy);
 		} else {
 			seq_printf(seq, "Descriptor ring:\n");
 			sysfs_display_ring((void *)rx_q->dma_rx,
-					   priv->dma_rx_size, 0, seq);
+					   priv->dma_rx_size, 0, seq, rx_q->dma_rx_phy);
 		}
 	}
 
@@ -4375,11 +4391,11 @@  static int stmmac_rings_status_show(struct seq_file *seq, void *v)
 		if (priv->extend_desc) {
 			seq_printf(seq, "Extended descriptor ring:\n");
 			sysfs_display_ring((void *)tx_q->dma_etx,
-					   priv->dma_tx_size, 1, seq);
+					   priv->dma_tx_size, 1, seq, tx_q->dma_tx_phy);
 		} else if (!(tx_q->tbs & STMMAC_TBS_AVAIL)) {
 			seq_printf(seq, "Descriptor ring:\n");
 			sysfs_display_ring((void *)tx_q->dma_tx,
-					   priv->dma_tx_size, 0, seq);
+					   priv->dma_tx_size, 0, seq, tx_q->dma_tx_phy);
 		}
 	}