Hi Anup I have tested the MACB driver patch with the SiFive unleashed board and it's not working because of the GEM_DCFG6 register. The SiFive FU540 is supporting DMA address width is 64-bit while reading the GEM_DCFG6 register but it supports scatter-gather 32-bit wide bus mastering DMA. The SiFive unleashed board supports 32-bit DMA not 64-bit DMA.
We can't check dynamically the support of DMA. + if (GEM_BFEXT(DAW64, gem_readl(macb, DCFG6))) + macb->hw_dma_cap = HW_DMA_CAP_64B; + else + macb->hw_dma_cap = HW_DMA_CAP_32B; Can we use preprocessor macro(like below) to check DMA support? #ifdef CONFIG_DMA_ADDR_T_64BIT macb->hw_dma_cap = HW_DMA_CAP_64B; #else macb->hw_dma_cap = HW_DMA_CAP_32B; #endif By default CONFIG_DMA_ADDR_T_64BIT is not set, whichever the board supports 64-bit DMA then the board config can set the CONFIG_DMA_ADDR_T_64BIT. Regards Padmarao On Sun, Nov 15, 2020 at 5:40 PM Anup Patel <a...@brainfault.org> wrote: > On Tue, Nov 10, 2020 at 4:16 PM Padmarao Begari > <padmarao.beg...@microchip.com> wrote: > > > > Enable 32-bit or 64-bit DMA in the macb driver based on the design > > config debug6 register of MACB hardware which supports 32-bit or > > 64-bit DMA. > > > > Signed-off-by: Padmarao Begari <padmarao.beg...@microchip.com> > > --- > > drivers/net/macb.c | 121 +++++++++++++++++++++++++++++++++++++++------ > > drivers/net/macb.h | 6 +++ > > 2 files changed, 112 insertions(+), 15 deletions(-) > > > > diff --git a/drivers/net/macb.c b/drivers/net/macb.c > > index b80a259ff7..9ccbb7a166 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); > > + > > gem_writel(macb, DMACFG, dmacfg); > > } > > > > @@ -775,6 +848,12 @@ static int _macb_init(struct macb_device *macb, > const char *name) > > unsigned long paddr; > > int ret; > > int i; > > + int count; > > + > > + if (GEM_BFEXT(DAW64, gem_readl(macb, DCFG6))) > > + macb->hw_dma_cap = HW_DMA_CAP_64B; > > + else > > + macb->hw_dma_cap = HW_DMA_CAP_32B; > > > > /* > > * macb_halt should have been called at some point before now, > > @@ -786,20 +865,28 @@ static int _macb_init(struct macb_device *macb, > const char *name) > > for (i = 0; i < MACB_RX_RING_SIZE; i++) { > > if (i == (MACB_RX_RING_SIZE - 1)) > > paddr |= MACB_BIT(RX_WRAP); > > - macb->rx_ring[i].addr = paddr; > > - macb->rx_ring[i].ctrl = 0; > > + if (macb->hw_dma_cap & HW_DMA_CAP_64B) > > + count = i * 2; > > + else > > + count = i; > > + macb->rx_ring[count].ctrl = 0; > > + macb_set_addr(macb, &macb->rx_ring[count], paddr); > > paddr += macb->rx_buffer_size; > > } > > macb_flush_ring_desc(macb, RX); > > macb_flush_rx_buffer(macb); > > > > for (i = 0; i < MACB_TX_RING_SIZE; i++) { > > - macb->tx_ring[i].addr = 0; > > + if (macb->hw_dma_cap & HW_DMA_CAP_64B) > > + count = i * 2; > > + else > > + count = i; > > + macb_set_addr(macb, &macb->tx_ring[count], 0); > > if (i == (MACB_TX_RING_SIZE - 1)) > > - macb->tx_ring[i].ctrl = MACB_BIT(TX_USED) | > > + macb->tx_ring[count].ctrl = MACB_BIT(TX_USED) | > > MACB_BIT(TX_WRAP); > > else > > - macb->tx_ring[i].ctrl = MACB_BIT(TX_USED); > > + macb->tx_ring[count].ctrl = MACB_BIT(TX_USED); > > } > > macb_flush_ring_desc(macb, TX); > > > > @@ -812,8 +899,12 @@ static int _macb_init(struct macb_device *macb, > const char *name) > > gem_writel(macb, DMACFG, MACB_ZYNQ_GEM_DMACR_INIT); > > #endif > > > > - macb_writel(macb, RBQP, macb->rx_ring_dma); > > - macb_writel(macb, TBQP, macb->tx_ring_dma); > > + macb_writel(macb, RBQP, lower_32_bits(macb->rx_ring_dma)); > > + macb_writel(macb, TBQP, lower_32_bits(macb->tx_ring_dma)); > > + if (macb->hw_dma_cap & HW_DMA_CAP_64B) { > > + macb_writel(macb, RBQPH, > upper_32_bits(macb->rx_ring_dma)); > > + macb_writel(macb, TBQPH, > upper_32_bits(macb->tx_ring_dma)); > > + } > > > > if (macb_is_gem(macb)) { > > /* Initialize DMA properties */ > > diff --git a/drivers/net/macb.h b/drivers/net/macb.h > > index 9b16383eba..72b84ae96e 100644 > > --- a/drivers/net/macb.h > > +++ b/drivers/net/macb.h > > @@ -768,5 +768,11 @@ > > #define GEM_RX_CSUM_CHECKED_MASK 2 > > #define gem_writel_queue_TBQP(port, value, queue_num) \ > > writel((value), (port)->regs + GEM_TBQP(queue_num)) > > +#define gem_writel_queue_TBQPH(port, value, queue_num) \ > > + writel((value), (port)->regs + GEM_TBQPH(queue_num)) > > +#define gem_writel_queue_RBQP(port, value, queue_num) \ > > + writel((value), (port)->regs + GEM_RBQP(queue_num)) > > +#define gem_writel_queue_RBQPH(port, value, queue_num) \ > > + writel((value), (port)->regs + GEM_RBQPH(queue_num)) > > > > #endif /* __DRIVERS_MACB_H__ */ > > -- > > 2.17.1 > > > > Looks good to me. > > Reviewed-by: Anup Patel <anup.pa...@wdc.com> > > We need to test this patch on SiFive Unleashed as well because > it has same ethernet controller. > > Regards, > Anup >