Hi Padmarao, On Fri, Dec 11, 2020 at 8:07 PM Padmarao Begari <padmara...@gmail.com> wrote: > > Hi Bin, > > On Fri, Dec 11, 2020 at 2:59 PM Bin Meng <bmeng...@gmail.com> wrote: >> >> Hi Padmarao, >> >> On Fri, Dec 11, 2020 at 4:49 PM Padmarao Begari <padmara...@gmail.com> wrote: >> > >> > Hi Bin, >> > >> > On Thu, Dec 10, 2020 at 4:08 PM Bin Meng <bmeng...@gmail.com> wrote: >> >> >> >> Hi Padmarao, >> >> >> >> On Thu, Dec 10, 2020 at 6:33 PM Bin Meng <bmeng...@gmail.com> wrote: >> >> > >> >> > Hi Padmarao, >> >> > >> >> > On Thu, Dec 3, 2020 at 4:44 AM Padmarao Begari >> >> > <padmarao.beg...@microchip.com> wrote: >> >> > > >> >> > > Enable 32-bit or 64-bit DMA in the macb driver based on the macb >> >> > > compatible string of the device tree node. >> >> > > >> >> > > Signed-off-by: Padmarao Begari <padmarao.beg...@microchip.com> >> >> > > Reviewed-by: Anup Patel <anup.pa...@wdc.com> >> >> > > --- >> >> > > drivers/net/macb.c | 131 >> >> > > +++++++++++++++++++++++++++++++++++++++------ >> >> > > drivers/net/macb.h | 6 +++ >> >> > > 2 files changed, 120 insertions(+), 17 deletions(-) >> >> > > >> >> > > diff --git a/drivers/net/macb.c b/drivers/net/macb.c >> >> > > index b80a259ff7..e7c93d4747 100644 >> >> > > --- a/drivers/net/macb.c >> >> > > +++ b/drivers/net/macb.c >> >> > > @@ -83,7 +83,16 @@ struct macb_dma_desc { >> >> > > u32 ctrl; >> >> > > }; >> >> > > >> >> > > -#define DMA_DESC_BYTES(n) (n * sizeof(struct macb_dma_desc)) >> >> > > +struct macb_dma_desc_64 { >> >> > > + u32 addrh; >> >> > > + u32 unused; >> >> > > +}; >> >> > > + >> >> > > +#define HW_DMA_CAP_32B 0 >> >> > > +#define HW_DMA_CAP_64B 1 >> >> > > + >> >> > > +#define DMA_DESC_SIZE 16 >> >> > > +#define DMA_DESC_BYTES(n) ((n) * DMA_DESC_SIZE) >> >> > > #define MACB_TX_DMA_DESC_SIZE (DMA_DESC_BYTES(MACB_TX_RING_SIZE)) >> >> > > #define MACB_RX_DMA_DESC_SIZE (DMA_DESC_BYTES(MACB_RX_RING_SIZE)) >> >> > > #define MACB_TX_DUMMY_DMA_DESC_SIZE (DMA_DESC_BYTES(1)) >> >> > > @@ -133,6 +142,7 @@ struct macb_device { >> >> > > #endif >> >> > > phy_interface_t phy_interface; >> >> > > #endif >> >> > > + unsigned short hw_dma_cap; >> >> > > }; >> >> > > >> >> > > struct macb_config { >> >> > > @@ -307,6 +317,24 @@ static inline void >> >> > > macb_invalidate_rx_buffer(struct macb_device *macb) >> >> > > >> >> > > #if defined(CONFIG_CMD_NET) >> >> > > >> >> > > +static struct macb_dma_desc_64 *macb_64b_desc(struct macb_dma_desc >> >> > > *desc) >> >> > > +{ >> >> > > + return (struct macb_dma_desc_64 *)((void *)desc >> >> > > + + sizeof(struct macb_dma_desc)); >> >> > > +} >> >> > > + >> >> > > +static void macb_set_addr(struct macb_device *macb, struct >> >> > > macb_dma_desc *desc, >> >> > > + ulong addr) >> >> > > +{ >> >> > > + struct macb_dma_desc_64 *desc_64; >> >> > > + >> >> > > + if (macb->hw_dma_cap & HW_DMA_CAP_64B) { >> >> > > + desc_64 = macb_64b_desc(desc); >> >> > > + desc_64->addrh = upper_32_bits(addr); >> >> > > + } >> >> > > + desc->addr = lower_32_bits(addr); >> >> > > +} >> >> > > + >> >> > > static int _macb_send(struct macb_device *macb, const char *name, >> >> > > void *packet, >> >> > > int length) >> >> > > { >> >> > > @@ -325,8 +353,12 @@ static int _macb_send(struct macb_device *macb, >> >> > > const char *name, void *packet, >> >> > > macb->tx_head++; >> >> > > } >> >> > > >> >> > > + if (macb->hw_dma_cap & HW_DMA_CAP_64B) >> >> > > + tx_head = tx_head * 2; >> >> > > + >> >> > > macb->tx_ring[tx_head].ctrl = ctrl; >> >> > > - macb->tx_ring[tx_head].addr = paddr; >> >> > > + macb_set_addr(macb, &macb->tx_ring[tx_head], paddr); >> >> > > + >> >> > > barrier(); >> >> > > macb_flush_ring_desc(macb, TX); >> >> > > macb_writel(macb, NCR, MACB_BIT(TE) | MACB_BIT(RE) | >> >> > > MACB_BIT(TSTART)); >> >> > > @@ -363,19 +395,28 @@ static void reclaim_rx_buffers(struct >> >> > > macb_device *macb, >> >> > > unsigned int new_tail) >> >> > > { >> >> > > unsigned int i; >> >> > > + unsigned int count; >> >> > > >> >> > > i = macb->rx_tail; >> >> > > >> >> > > macb_invalidate_ring_desc(macb, RX); >> >> > > while (i > new_tail) { >> >> > > - macb->rx_ring[i].addr &= ~MACB_BIT(RX_USED); >> >> > > + if (macb->hw_dma_cap & HW_DMA_CAP_64B) >> >> > > + count = i * 2; >> >> > > + else >> >> > > + count = i; >> >> > > + macb->rx_ring[count].addr &= ~MACB_BIT(RX_USED); >> >> > > i++; >> >> > > if (i > MACB_RX_RING_SIZE) >> >> > > i = 0; >> >> > > } >> >> > > >> >> > > while (i < new_tail) { >> >> > > - macb->rx_ring[i].addr &= ~MACB_BIT(RX_USED); >> >> > > + if (macb->hw_dma_cap & HW_DMA_CAP_64B) >> >> > > + count = i * 2; >> >> > > + else >> >> > > + count = i; >> >> > > + macb->rx_ring[count].addr &= ~MACB_BIT(RX_USED); >> >> > > i++; >> >> > > } >> >> > > >> >> > > @@ -390,16 +431,25 @@ static int _macb_recv(struct macb_device *macb, >> >> > > uchar **packetp) >> >> > > void *buffer; >> >> > > int length; >> >> > > u32 status; >> >> > > + u8 flag = false; >> >> > > >> >> > > macb->wrapped = false; >> >> > > for (;;) { >> >> > > macb_invalidate_ring_desc(macb, RX); >> >> > > >> >> > > + if (macb->hw_dma_cap & HW_DMA_CAP_64B) >> >> > > + next_rx_tail = next_rx_tail * 2; >> >> > > + >> >> > > if (!(macb->rx_ring[next_rx_tail].addr & >> >> > > MACB_BIT(RX_USED))) >> >> > > return -EAGAIN; >> >> > > >> >> > > status = macb->rx_ring[next_rx_tail].ctrl; >> >> > > if (status & MACB_BIT(RX_SOF)) { >> >> > > + if (macb->hw_dma_cap & HW_DMA_CAP_64B) { >> >> > > + next_rx_tail = next_rx_tail / 2; >> >> > > + flag = true; >> >> > > + } >> >> > > + >> >> > > if (next_rx_tail != macb->rx_tail) >> >> > > reclaim_rx_buffers(macb, >> >> > > next_rx_tail); >> >> > > macb->wrapped = false; >> >> > > @@ -426,11 +476,22 @@ static int _macb_recv(struct macb_device *macb, >> >> > > uchar **packetp) >> >> > > *packetp = buffer; >> >> > > } >> >> > > >> >> > > + if (macb->hw_dma_cap & HW_DMA_CAP_64B) { >> >> > > + if (!flag) >> >> > > + next_rx_tail = next_rx_tail / >> >> > > 2; >> >> > > + } >> >> > > + >> >> > > if (++next_rx_tail >= MACB_RX_RING_SIZE) >> >> > > next_rx_tail = 0; >> >> > > macb->next_rx_tail = next_rx_tail; >> >> > > return length; >> >> > > } else { >> >> > > + if (macb->hw_dma_cap & HW_DMA_CAP_64B) { >> >> > > + if (!flag) >> >> > > + next_rx_tail = next_rx_tail / >> >> > > 2; >> >> > > + flag = false; >> >> > > + } >> >> > > + >> >> > > if (++next_rx_tail >= MACB_RX_RING_SIZE) { >> >> > > macb->wrapped = true; >> >> > > next_rx_tail = 0; >> >> > > @@ -718,6 +779,7 @@ static int gmac_init_multi_queues(struct >> >> > > macb_device *macb) >> >> > > { >> >> > > int i, num_queues = 1; >> >> > > u32 queue_mask; >> >> > > + unsigned long paddr; >> >> > > >> >> > > /* bit 0 is never set but queue 0 always exists */ >> >> > > queue_mask = gem_readl(macb, DCFG6) & 0xff; >> >> > > @@ -731,10 +793,18 @@ static int gmac_init_multi_queues(struct >> >> > > macb_device *macb) >> >> > > macb->dummy_desc->addr = 0; >> >> > > flush_dcache_range(macb->dummy_desc_dma, macb->dummy_desc_dma >> >> > > + >> >> > > ALIGN(MACB_TX_DUMMY_DMA_DESC_SIZE, PKTALIGN)); >> >> > > - >> >> > > - for (i = 1; i < num_queues; i++) >> >> > > - gem_writel_queue_TBQP(macb, macb->dummy_desc_dma, i - >> >> > > 1); >> >> > > - >> >> > > + paddr = macb->dummy_desc_dma; >> >> > > + >> >> > > + for (i = 1; i < num_queues; i++) { >> >> > > + gem_writel_queue_TBQP(macb, lower_32_bits(paddr), i - >> >> > > 1); >> >> > > + gem_writel_queue_RBQP(macb, lower_32_bits(paddr), i - >> >> > > 1); >> >> > > + if (macb->hw_dma_cap & HW_DMA_CAP_64B) { >> >> > > + gem_writel_queue_TBQPH(macb, >> >> > > upper_32_bits(paddr), >> >> > > + i - 1); >> >> > > + gem_writel_queue_RBQPH(macb, >> >> > > upper_32_bits(paddr), >> >> > > + i - 1); >> >> > > + } >> >> > > + } >> >> > > return 0; >> >> > > } >> >> > > >> >> > > @@ -760,6 +830,9 @@ static void gmac_configure_dma(struct macb_device >> >> > > *macb) >> >> > > dmacfg &= ~GEM_BIT(ENDIA_DESC); >> >> > > >> >> > > dmacfg &= ~GEM_BIT(ADDR64); >> >> > > + if (macb->hw_dma_cap & HW_DMA_CAP_64B) >> >> > > + dmacfg |= GEM_BIT(ADDR64); >> >> >> >> Does GEM on PolarFire not support 32-bit DMA address? I believe in >> >> U-Boot we can only get 32-bit address. >> >> >> > >> > Yes, it's not work when I tested with 32-bit DMA. >> >> Okay, then this explains why 64-bit DMA patch is needed. Is this a bug >> of the GEM IP integrated in the PolarFire? >> Could you please write something in the commit message to clarify? >> That would be helpful. Thanks! > > > No, it's not a bug, The PolarFire SoC Memory Protection Unit(MPU) gives the > 64-bit DMA access with GEM, the MPU transactions on the AXI bus is 64-bit not > 32-bit.
This sounds like a GEM IP integration configuration issue on PolarFire. Regards, Bin