[PATCH v5 2/7] net: macb: Add DMA 64-bit address support for macb

Padmarao Begari padmarao.b at gmail.com
Fri Dec 11 09:35:08 CET 2020


Hi Bin,

On Thu, Dec 10, 2020 at 4:08 PM Bin Meng <bmeng.cn at gmail.com> wrote:

> Hi Padmarao,
>
> On Thu, Dec 10, 2020 at 6:33 PM Bin Meng <bmeng.cn at gmail.com> wrote:
> >
> > Hi Padmarao,
> >
> > On Thu, Dec 3, 2020 at 4:44 AM Padmarao Begari
> > <padmarao.begari at 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.begari at microchip.com>
> > > Reviewed-by: Anup Patel <anup.patel at 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.
>
> > > +
> > >         gem_writel(macb, DMACFG, dmacfg);
> > >  }
> > >
> > > @@ -775,6 +848,7 @@ static int _macb_init(struct macb_device *macb,
> const char *name)
> > >         unsigned long paddr;
> > >         int ret;
> > >         int i;
> > > +       int count;
> > >
> > >         /*
> > >          * macb_halt should have been called at some point before now,
> > > @@ -786,20 +860,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 +894,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 */
> > > @@ -1225,10 +1311,11 @@ static int macb_eth_probe(struct udevice *dev)
> > >         struct eth_pdata *pdata = dev_get_platdata(dev);
> > >         struct macb_device *macb = dev_get_priv(dev);
> > >         const char *phy_mode;
> > > +       const char *compatible;
> > >         int ret;
> > >
> > > -       phy_mode = fdt_getprop(gd->fdt_blob, dev_of_offset(dev),
> "phy-mode",
> > > -                              NULL);
> > > +       phy_mode = dev_read_prop(dev, "phy-mode", NULL);
> > > +
> > >         if (phy_mode)
> > >                 macb->phy_interface =
> phy_get_interface_by_name(phy_mode);
> > >         if (macb->phy_interface == -1) {
> > > @@ -1236,6 +1323,15 @@ static int macb_eth_probe(struct udevice *dev)
> > >                 return -EINVAL;
> > >         }
> > >
> > > +       compatible = dev_read_prop(dev, "compatible", NULL);
> > > +
> > > +       if (compatible) {
> > > +               if (!strcmp(compatible, "microchip,mpfs-mss-gem"))
> > > +                       macb->hw_dma_cap = HW_DMA_CAP_64B;
> > > +               else
> > > +                       macb->hw_dma_cap = HW_DMA_CAP_32B;
> > > +       }
> > > +
> > >         macb->regs = (void *)pdata->iobase;
> > >
> > >         macb->is_big_endian = (cpu_to_be32(0x12345678) == 0x12345678);
> > > @@ -1324,6 +1420,7 @@ static const struct udevice_id macb_eth_ids[] = {
> > >         { .compatible = "cdns,zynq-gem" },
> > >         { .compatible = "sifive,fu540-c000-gem",
> > >           .data = (ulong)&sifive_config },
> > > +       { .compatible = "microchip,mpfs-mss-gem" },
> >
> > Can we make the hw_dma_cap part of the .data (struct macb_config), like:
> >
> > static const struct macb_config default_gem_config = {
> >     .dma_burst_length = 16,
> >     .hw_dma_cap = HW_DMA_CAP_32B,
> >     .clk_init = NULL,
> > };
> >
>
>
Yes, we can use hw_dma_cap in struct macb_config

#define HW_DMA_CAP_32B   0
#define HW_DMA_CAP_64B  1

Can we check 64-bit DMA condition like:

if (macb->config->hw_dma_cap)
or
if (macb->config->hw_dma_cap & HW_DMA_CAP_64B)

Which one will be used in the driver?

Regards
Padmarao

Regards,
> Bin
>


More information about the U-Boot mailing list