-----Original Message-----
From: Jakub Kicinski <kuba@kernel.org>
Sent: 2021年2月23日 3:46
To: Joakim Zhang <redacted>
Cc: peppe.cavallaro@st.com; alexandre.torgue@st.com;
joabreu@synopsys.com; davem@davemloft.net; netdev@vger.kernel.org;
dl-linux-imx [off-list ref]
Subject: Re: [PATCH V4 net 3/5] net: stmmac: fix dma physical address of
descriptor when display ring

On Sat, 20 Feb 2021 07:43:33 +0000 Joakim Zhang wrote:

quoted

quoted

quoted

 	pr_info("%s descriptor ring:\n", rx ? "RX" : "TX");

 	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),
+		pr_info("%03d [0x%llx]: 0x%x 0x%x 0x%x 0x%x\n",
+			i, (unsigned long long)(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++;

Why do you pass the desc_size in? The virt memory pointer is
incremented by
sizeof(*p) surely

	dma_addr + i * sizeof(*p)

I think we can't use sizeof(*p), as when display descriptor, only do "
struct dma_desc *p = (struct dma_desc *)head;", but driver can pass
"struct dma_desc", " struct dma_edesc" or " struct dma_extended_desc",

Looks like some of the functions you change already try to pick the right type.
Which one is problematic?

Yes, some functions have picked the right type:
drivers/net/ethernet/stmicro/stmmac/enh_desc.c -> enh_desc_display_ring()
drivers/net/ethernet/stmicro/stmmac/norm_desc.c -> ndesc_display_ring()

the problematic one is:
drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c -> dwmac4_display_ring()

Since the callback format is the same for them, and used from stmmac_main.c in the same way.
drivers/net/ethernet/stmicro/stmmac/hwif.h -> void (*display_ring)(void *head, unsigned int size, bool rx);

So I decide to modify them as a whole to avoid separate them as different format which would introduce more redundant code. Is it reasonable?

quoted

so it's necessary to pass desc_size to compatible all cases.

But you still increment the the VMA pointer ('p' in the quote above) but it's size,
so how is that correct if the DMA addr needs a special size increment?

Yes, you are right. It indeed a problem. Seems dwmac4_display_ring() function has not supported different desc format well.
DMA phy address is just one of its problem. I will fix it together. Thanks.

Best Regards,
Joakim Zhang