[U-Boot] [RFC v3 03/15] dma: add bcm6348-iudma support

2018-02-21 Thread Álvaro Fernández Rojas
BCM6348 IUDMA controller is present on multiple BMIPS (BCM63xx) SoCs.

Signed-off-by: Álvaro Fernández Rojas 
---
 v3: no changes
 v2: Fix dma rx burst config and select DMA_CHANNELS.

 drivers/dma/Kconfig |   9 +
 drivers/dma/Makefile|   1 +
 drivers/dma/bcm6348-iudma.c | 505 
 3 files changed, 515 insertions(+)
 create mode 100644 drivers/dma/bcm6348-iudma.c

diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
index 21b2c0dcaa..9afa158b51 100644
--- a/drivers/dma/Kconfig
+++ b/drivers/dma/Kconfig
@@ -19,6 +19,15 @@ config DMA_CHANNELS
  Enable channels support for DMA. Some DMA controllers have multiple
  channels which can either transfer data to/from different devices.
 
+config BCM6348_IUDMA
+   bool "BCM6348 IUDMA driver"
+   depends on ARCH_BMIPS
+   select DMA_CHANNELS
+   help
+ Enable the BCM6348 IUDMA driver.
+ This driver support data transfer from devices to
+ memory and from memory to devices.
+
 config TI_EDMA3
bool "TI EDMA3 driver"
help
diff --git a/drivers/dma/Makefile b/drivers/dma/Makefile
index 39b78b2a3d..b2b4147349 100644
--- a/drivers/dma/Makefile
+++ b/drivers/dma/Makefile
@@ -9,6 +9,7 @@ obj-$(CONFIG_DMA) += dma-uclass.o
 
 obj-$(CONFIG_FSLDMAFEC) += MCD_tasksInit.o MCD_dmaApi.o MCD_tasks.o
 obj-$(CONFIG_APBH_DMA) += apbh_dma.o
+obj-$(CONFIG_BCM6348_IUDMA) += bcm6348-iudma.o
 obj-$(CONFIG_FSL_DMA) += fsl_dma.o
 obj-$(CONFIG_TI_KSNAV) += keystone_nav.o keystone_nav_cfg.o
 obj-$(CONFIG_TI_EDMA3) += ti-edma3.o
diff --git a/drivers/dma/bcm6348-iudma.c b/drivers/dma/bcm6348-iudma.c
new file mode 100644
index 00..8016a58be2
--- /dev/null
+++ b/drivers/dma/bcm6348-iudma.c
@@ -0,0 +1,505 @@
+/*
+ * Copyright (C) 2018 Álvaro Fernández Rojas 
+ *
+ * Derived from linux/drivers/dma/bcm63xx-iudma.c:
+ * Copyright (C) 2015 Simon Arlott 
+ *
+ * Derived from linux/drivers/net/ethernet/broadcom/bcm63xx_enet.c:
+ * Copyright (C) 2008 Maxime Bizon 
+ *
+ * Derived from 
bcm963xx_4.12L.06B_consumer/shared/opensource/include/bcm963xx/63268_map_part.h:
+ * Copyright (C) 2000-2010 Broadcom Corporation
+ *
+ * Derived from 
bcm963xx_4.12L.06B_consumer/bcmdrivers/opensource/net/enet/impl4/bcmenet.c:
+ * Copyright (C) 2010 Broadcom Corporation
+ *
+ * SPDX-License-Identifier: GPL-2.0+
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+DECLARE_GLOBAL_DATA_PTR;
+
+#define ALIGN_END_ADDR(type, ptr, size)\
+   ((unsigned long)(ptr) + roundup((size) * sizeof(type), \
+ARCH_DMA_MINALIGN))
+
+/* DMA Channels */
+#define DMA_CHAN_FLOWC(x)  ((x) >> 1)
+#define DMA_CHAN_FLOWC_MAX 8
+#define DMA_CHAN_MAX   16
+#define DMA_CHAN_SIZE  0x10
+#define DMA_CHAN_TOUT  500
+
+/* DMA Global Configuration register */
+#define DMA_CFG_REG0x00
+#define DMA_CFG_ENABLE_SHIFT   0
+#define DMA_CFG_ENABLE_MASK(1 << DMA_CFG_ENABLE_SHIFT)
+#define DMA_CFG_FLOWC_ENABLE(x)BIT(DMA_CHAN_FLOWC(x) + 1)
+#define DMA_CFG_NCHANS_SHIFT   24
+#define DMA_CFG_NCHANS_MASK(0xf << DMA_CFG_NCHANS_SHIFT)
+
+/* DMA Global Flow Control Threshold registers */
+#define DMA_FLOWC_THR_LO_REG(x)(0x04 + DMA_CHAN_FLOWC(x) * 
0x0c)
+#define DMA_FLOWC_THR_LO_SHIFT 0
+#define DMA_FLOWC_THR_LO_MASK  (5 << DMA_FLOWC_THR_LO_SHIFT)
+
+#define DMA_FLOWC_THR_HI_REG(x)(0x08 + DMA_CHAN_FLOWC(x) * 
0x0c)
+#define DMA_FLOWC_THR_HI_SHIFT 0
+#define DMA_FLOWC_THR_HI_MASK  (10 << DMA_FLOWC_THR_HI_SHIFT)
+
+/* DMA Global Flow Control Buffer Allocation registers */
+#define DMA_FLOWC_ALLOC_REG(x) (0x0c + DMA_CHAN_FLOWC(x) * 0x0c)
+#define DMA_FLOWC_ALLOC_FORCE_SHIFT31
+#define DMA_FLOWC_ALLOC_FORCE_MASK (1 << DMA_FLOWC_ALLOC_FORCE_SHIFT)
+
+/* DMA Global Reset register */
+#define DMA_RST_REG0x34
+#define DMA_RST_CHAN_SHIFT 0
+#define DMA_RST_CHAN_MASK(x)   (1 << x)
+
+/* DMA Channel Configuration register */
+#define DMAC_CFG_REG(x)(DMA_CHAN_SIZE * (x) + 0x00)
+#define DMAC_CFG_ENABLE_SHIFT  0
+#define DMAC_CFG_ENABLE_MASK   (1 << DMAC_CFG_ENABLE_SHIFT)
+#define DMAC_CFG_PKT_HALT_SHIFT1
+#define DMAC_CFG_PKT_HALT_MASK (1 << DMAC_CFG_PKT_HALT_SHIFT)
+#define DMAC_CFG_BRST_HALT_SHIFT   2
+#define DMAC_CFG_BRST_HALT_MASK(1 << DMAC_CFG_BRST_HALT_SHIFT)
+
+/* DMA Channel Interrupts registers */
+#define DMAC_IR_ST_REG(x)  (DMA_CHAN_SIZE * (x) + 0x04)
+#define DMAC_IR_EN_REG(x)  (DMA_CHAN_SIZE * (x) + 0x08)
+
+#define DMAC_IR_DONE_SHIFT 2
+#define DMAC_IR_DONE_MASK  (1 << DMAC_IR_DONE_SHIFT)
+
+/* DMA Channel Max Burst Length register */
+#define DMAC_BURST_REG(x)

Re: [U-Boot] [RFC v3 03/15] dma: add bcm6348-iudma support

2018-02-22 Thread Grygorii Strashko
Hi 

I'd appreciated if you can clarify few points below.

On 02/21/2018 10:10 AM, Álvaro Fernández Rojas wrote:
> BCM6348 IUDMA controller is present on multiple BMIPS (BCM63xx) SoCs.
> 
> Signed-off-by: Álvaro Fernández Rojas 
> ---
>   v3: no changes
>   v2: Fix dma rx burst config and select DMA_CHANNELS.
> 
>   drivers/dma/Kconfig |   9 +
>   drivers/dma/Makefile|   1 +
>   drivers/dma/bcm6348-iudma.c | 505 
> 
>   3 files changed, 515 insertions(+)
>   create mode 100644 drivers/dma/bcm6348-iudma.c
>

[...]
 
>
> +static int bcm6348_iudma_enable(struct dma *dma)
> +{
> + struct bcm6348_iudma_priv *priv = dev_get_priv(dma->dev);
> + struct bcm6348_chan_priv *ch_priv = priv->ch_priv[dma->id];
> + struct bcm6348_dma_desc *dma_desc;
> + uint8_t i;
> +
> + /* init dma rings */
> + dma_desc = ch_priv->dma_ring;
> + for (i = 0; i < ch_priv->dma_ring_size; i++) {
> + if (bcm6348_iudma_chan_is_rx(dma->id)) {
> + dma_desc->status = DMAD_ST_OWN_MASK;
> + dma_desc->length = PKTSIZE_ALIGN;
> + dma_desc->address = virt_to_phys(net_rx_packets[i]);

You are filling RX queue/ring with buffers defined in Net core.
Does it mean that this DMA driver will not be usable for other purposes, as
Net can be compiled out?

Wouldn't it be reasonable to have some sort of .receive_prepare() callback in
DMA dma_ops, so DMA user can control which buffers to push in RX DMA channel?
And it also can be used in eth_ops.free_pkt() callback (see below).

> + } else {
> + dma_desc->status = 0;
> + dma_desc->length = 0;
> + dma_desc->address = 0;
> + }
> +
> + if (i == ch_priv->dma_ring_size - 1)
> + dma_desc->status |= DMAD_ST_WRAP_MASK;
> +
> + if (bcm6348_iudma_chan_is_rx(dma->id))
> + writel_be(1,
> +   priv->base + DMA_FLOWC_ALLOC_REG(dma->id));
> +
> + dma_desc++;
> + }
> +
> + /* init to first descriptor */
> + ch_priv->desc_id = 0;
> +
> + /* force cache writeback */
> + flush_dcache_range((ulong)ch_priv->dma_ring,
> + ALIGN_END_ADDR(struct bcm6348_dma_desc, ch_priv->dma_ring,
> +ch_priv->dma_ring_size));
> +
> + /* clear sram */
> + writel_be(0, priv->sram + DMAS_STATE_DATA_REG(dma->id));
> + writel_be(0, priv->sram + DMAS_DESC_LEN_STATUS_REG(dma->id));
> + writel_be(0, priv->sram + DMAS_DESC_BASE_BUFPTR_REG(dma->id));
> +
> + /* set dma ring start */
> + writel_be(virt_to_phys(ch_priv->dma_ring),
> +   priv->sram + DMAS_RSTART_REG(dma->id));
> +
> + /* set flow control */
> + if (bcm6348_iudma_chan_is_rx(dma->id)) {
> + u32 val;
> +
> + setbits_be32(priv->base + DMA_CFG_REG,
> +  DMA_CFG_FLOWC_ENABLE(dma->id));
> +
> + val = ch_priv->dma_ring_size / 3;
> + writel_be(val, priv->base + DMA_FLOWC_THR_LO_REG(dma->id));
> +
> + val = (ch_priv->dma_ring_size * 2) / 3;
> + writel_be(val, priv->base + DMA_FLOWC_THR_HI_REG(dma->id));
> + }
> +
> + /* set dma max burst */
> + writel_be(ch_priv->dma_ring_size,
> +   priv->chan + DMAC_BURST_REG(dma->id));
> +
> + /* clear interrupts */
> + writel_be(DMAC_IR_DONE_MASK, priv->chan + DMAC_IR_ST_REG(dma->id));
> + writel_be(0, priv->chan + DMAC_IR_EN_REG(dma->id));
> +
> + setbits_be32(priv->chan + DMAC_CFG_REG(dma->id), DMAC_CFG_ENABLE_MASK);
> +
> + return 0;
> +}

[...]

> +
> +static int bcm6348_iudma_receive(struct dma *dma, void **dst)
> +{
> + struct bcm6348_iudma_priv *priv = dev_get_priv(dma->dev);
> + struct bcm6348_chan_priv *ch_priv = priv->ch_priv[dma->id];
> + struct bcm6348_dma_desc *dma_desc;
> + void __iomem *dma_buff;
> + uint16_t status;
> + int ret;
> +
> + /* get dma ring descriptor address */
> + dma_desc = ch_priv->dma_ring;
> + dma_desc += ch_priv->desc_id;
> +
> + /* invalidate cache data */
> + invalidate_dcache_range((ulong)dma_desc,
> + ALIGN_END_ADDR(struct bcm6348_dma_desc, dma_desc, 1));
> +
> + /* check dma own */
> + if (dma_desc->status & DMAD_ST_OWN_MASK)
> + return 0;
> +
> + /* check dma end */
> + if (!(dma_desc->status & DMAD_ST_EOP_MASK))
> + return -EINVAL;
> +
> + /* get dma buff descriptor address */
> + dma_buff = phys_to_virt(dma_desc->address);
> +
> + /* invalidate cache data */
> + invalidate_dcache_range((ulong)dma_buff,
> + (ulong)(dma_buff + PKTSIZE_ALIGN));
> +
> + /* get dma data */
> + *dst = dma_buff;
> + ret = dma_desc->length;
> +
> + /* reinit dma descriptor */
> + status = dma_desc->status & DMAD_ST_

Re: [U-Boot] [RFC v3 03/15] dma: add bcm6348-iudma support

2018-02-22 Thread Álvaro Fernández Rojas

Hi Grygori,

El 22/02/2018 a las 20:50, Grygorii Strashko escribió:

Hi

I'd appreciated if you can clarify few points below.

On 02/21/2018 10:10 AM, Álvaro Fernández Rojas wrote:

BCM6348 IUDMA controller is present on multiple BMIPS (BCM63xx) SoCs.

Signed-off-by: Álvaro Fernández Rojas 
---
   v3: no changes
   v2: Fix dma rx burst config and select DMA_CHANNELS.

   drivers/dma/Kconfig |   9 +
   drivers/dma/Makefile|   1 +
   drivers/dma/bcm6348-iudma.c | 505 

   3 files changed, 515 insertions(+)
   create mode 100644 drivers/dma/bcm6348-iudma.c


[...]
  

+static int bcm6348_iudma_enable(struct dma *dma)
+{
+   struct bcm6348_iudma_priv *priv = dev_get_priv(dma->dev);
+   struct bcm6348_chan_priv *ch_priv = priv->ch_priv[dma->id];
+   struct bcm6348_dma_desc *dma_desc;
+   uint8_t i;
+
+   /* init dma rings */
+   dma_desc = ch_priv->dma_ring;
+   for (i = 0; i < ch_priv->dma_ring_size; i++) {
+   if (bcm6348_iudma_chan_is_rx(dma->id)) {
+   dma_desc->status = DMAD_ST_OWN_MASK;
+   dma_desc->length = PKTSIZE_ALIGN;
+   dma_desc->address = virt_to_phys(net_rx_packets[i]);

You are filling RX queue/ring with buffers defined in Net core.
Does it mean that this DMA driver will not be usable for other purposes, as
Net can be compiled out?
As far as I know, and depending on the specific SoC, BCM63xx IUDMA is 
used for Ethernet, USB (device only) and xDSL.

So yes, in u-boot it will be used for ethernet only.
BTW, my first attempt didn't use net_rx_packets, but I saw that in 
pic32_eth implementation and dropped the dma specific buffers. I will 
add them again ;).


Wouldn't it be reasonable to have some sort of .receive_prepare() callback in
DMA dma_ops, so DMA user can control which buffers to push in RX DMA channel?
And it also can be used in eth_ops.free_pkt() callback (see below).

Yes, probably, but maybe we can achieve that without adding another call.



+   } else {
+   dma_desc->status = 0;
+   dma_desc->length = 0;
+   dma_desc->address = 0;
+   }
+
+   if (i == ch_priv->dma_ring_size - 1)
+   dma_desc->status |= DMAD_ST_WRAP_MASK;
+
+   if (bcm6348_iudma_chan_is_rx(dma->id))
+   writel_be(1,
+ priv->base + DMA_FLOWC_ALLOC_REG(dma->id));
+
+   dma_desc++;
+   }
+
+   /* init to first descriptor */
+   ch_priv->desc_id = 0;
+
+   /* force cache writeback */
+   flush_dcache_range((ulong)ch_priv->dma_ring,
+   ALIGN_END_ADDR(struct bcm6348_dma_desc, ch_priv->dma_ring,
+  ch_priv->dma_ring_size));
+
+   /* clear sram */
+   writel_be(0, priv->sram + DMAS_STATE_DATA_REG(dma->id));
+   writel_be(0, priv->sram + DMAS_DESC_LEN_STATUS_REG(dma->id));
+   writel_be(0, priv->sram + DMAS_DESC_BASE_BUFPTR_REG(dma->id));
+
+   /* set dma ring start */
+   writel_be(virt_to_phys(ch_priv->dma_ring),
+ priv->sram + DMAS_RSTART_REG(dma->id));
+
+   /* set flow control */
+   if (bcm6348_iudma_chan_is_rx(dma->id)) {
+   u32 val;
+
+   setbits_be32(priv->base + DMA_CFG_REG,
+DMA_CFG_FLOWC_ENABLE(dma->id));
+
+   val = ch_priv->dma_ring_size / 3;
+   writel_be(val, priv->base + DMA_FLOWC_THR_LO_REG(dma->id));
+
+   val = (ch_priv->dma_ring_size * 2) / 3;
+   writel_be(val, priv->base + DMA_FLOWC_THR_HI_REG(dma->id));
+   }
+
+   /* set dma max burst */
+   writel_be(ch_priv->dma_ring_size,
+ priv->chan + DMAC_BURST_REG(dma->id));
+
+   /* clear interrupts */
+   writel_be(DMAC_IR_DONE_MASK, priv->chan + DMAC_IR_ST_REG(dma->id));
+   writel_be(0, priv->chan + DMAC_IR_EN_REG(dma->id));
+
+   setbits_be32(priv->chan + DMAC_CFG_REG(dma->id), DMAC_CFG_ENABLE_MASK);
+
+   return 0;
+}

[...]


+
+static int bcm6348_iudma_receive(struct dma *dma, void **dst)
+{
+   struct bcm6348_iudma_priv *priv = dev_get_priv(dma->dev);
+   struct bcm6348_chan_priv *ch_priv = priv->ch_priv[dma->id];
+   struct bcm6348_dma_desc *dma_desc;
+   void __iomem *dma_buff;
+   uint16_t status;
+   int ret;
+
+   /* get dma ring descriptor address */
+   dma_desc = ch_priv->dma_ring;
+   dma_desc += ch_priv->desc_id;
+
+   /* invalidate cache data */
+   invalidate_dcache_range((ulong)dma_desc,
+   ALIGN_END_ADDR(struct bcm6348_dma_desc, dma_desc, 1));
+
+   /* check dma own */
+   if (dma_desc->status & DMAD_ST_OWN_MASK)
+   return 0;
+
+   /* check dma end */
+   if (!(dma_desc->status & DMAD_ST_EOP_MASK))
+   return -EINVAL;
+
+   /* get dma buf

Re: [U-Boot] [RFC v3 03/15] dma: add bcm6348-iudma support

2018-02-23 Thread Grygorii Strashko
Hi

thanks for your comments.

On 02/22/2018 02:48 PM, Álvaro Fernández Rojas wrote:
> El 22/02/2018 a las 20:50, Grygorii Strashko escribió:
>> On 02/21/2018 10:10 AM, Álvaro Fernández Rojas wrote:
>>> BCM6348 IUDMA controller is present on multiple BMIPS (BCM63xx) SoCs.
>>>
>>> Signed-off-by: Álvaro Fernández Rojas 
>>> ---
>>>    v3: no changes
>>>    v2: Fix dma rx burst config and select DMA_CHANNELS.
>>>
>>>    drivers/dma/Kconfig |   9 +
>>>    drivers/dma/Makefile    |   1 +
>>>    drivers/dma/bcm6348-iudma.c | 505 
>>> 
>>>    3 files changed, 515 insertions(+)
>>>    create mode 100644 drivers/dma/bcm6348-iudma.c
>>>
>> [...]
>>> +static int bcm6348_iudma_enable(struct dma *dma)
>>> +{
>>> +    struct bcm6348_iudma_priv *priv = dev_get_priv(dma->dev);
>>> +    struct bcm6348_chan_priv *ch_priv = priv->ch_priv[dma->id];
>>> +    struct bcm6348_dma_desc *dma_desc;
>>> +    uint8_t i;
>>> +
>>> +    /* init dma rings */
>>> +    dma_desc = ch_priv->dma_ring;
>>> +    for (i = 0; i < ch_priv->dma_ring_size; i++) {
>>> +    if (bcm6348_iudma_chan_is_rx(dma->id)) {
>>> +    dma_desc->status = DMAD_ST_OWN_MASK;
>>> +    dma_desc->length = PKTSIZE_ALIGN;
>>> +    dma_desc->address = virt_to_phys(net_rx_packets[i]);
>> You are filling RX queue/ring with buffers defined in Net core.
>> Does it mean that this DMA driver will not be usable for other 
>> purposes, as
>> Net can be compiled out?
> As far as I know, and depending on the specific SoC, BCM63xx IUDMA is 
> used for Ethernet, USB (device only) and xDSL.
> So yes, in u-boot it will be used for ethernet only.
> BTW, my first attempt didn't use net_rx_packets, but I saw that in 
> pic32_eth implementation and dropped the dma specific buffers. I will 
> add them again ;).

it is really net specific :)

>>
>> Wouldn't it be reasonable to have some sort of .receive_prepare() 
>> callback in
>> DMA dma_ops, so DMA user can control which buffers to push in RX DMA 
>> channel?
>> And it also can be used in eth_ops.free_pkt() callback (see below).
> Yes, probably, but maybe we can achieve that without adding another call.
>>

I'm looking at this patch set from our HW point of view. In my case,
DMA channel can be used with different IPs (not only networking), so
it would be really great if DMA user can pass RX buffers in DMA driver -
network driver can use net_rx_packets, other drivers might use own buffers.
So hard-codding RX buffers in DMA driver looks like not a good choice.

>>> +
>>> +    /* flush cache */
>>> +    flush_dcache_range((ulong)dma_desc,
>>> +    ALIGN_END_ADDR(struct bcm6348_dma_desc, dma_desc, 1));
>> Could you clarify pls, if you do return dma_desc to RX ring here or not?
> Yes.
>>
>> if yes, wouldn't it cause potential problem on Net RX path
>>     ret = eth_get_ops(current)->recv(current, flags, &packet);
>> ^^ (1) here buffer will be received from DMA ( and pushed back to RX 
>> ring ?? )
>>
>>     flags = 0;
>>     if (ret > 0)
>>     net_process_received_packet(packet, ret);
>> ^^ (2) here it will be passed in Net stack
>>
>>     if (ret >= 0 && eth_get_ops(current)->free_pkt)
>>     eth_get_ops(current)->free_pkt(current, packet, ret);
>> ^^ at this point it should be safe to return buffer in DMA RX ring.
>>
>>     if (ret <= 0)
>>     break;
>>
>> Can DMA overwrite packet after point (1) while packet is still 
>> processed (2)?
> I don't think so, because as far as I know u-boot is not processing more 
> than one packet at once, is it?

u-boot can't process more than one packet, but dma does. if buffer returned
to DMA and there are some traffic on the line - DMA can potentially refill 
all buffers in its RX ring while u-boot still processing one packet.


> But yeah, I see your point and if it does process more than one packet 
> at once this is not the proper way to do that.
> I will use free_pkt in next version and lock the packet until it's 
> processed.


-- 
regards,
-grygorii
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [RFC v3 03/15] dma: add bcm6348-iudma support

2018-03-03 Thread Álvaro Fernández Rojas

Hi Grygorii,

El 23/02/2018 a las 17:57, Grygorii Strashko escribió:

Hi

thanks for your comments.

On 02/22/2018 02:48 PM, Álvaro Fernández Rojas wrote:

El 22/02/2018 a las 20:50, Grygorii Strashko escribió:

On 02/21/2018 10:10 AM, Álvaro Fernández Rojas wrote:

BCM6348 IUDMA controller is present on multiple BMIPS (BCM63xx) SoCs.

Signed-off-by: Álvaro Fernández Rojas 
---
    v3: no changes
    v2: Fix dma rx burst config and select DMA_CHANNELS.

    drivers/dma/Kconfig |   9 +
    drivers/dma/Makefile    |   1 +
    drivers/dma/bcm6348-iudma.c | 505

    3 files changed, 515 insertions(+)
    create mode 100644 drivers/dma/bcm6348-iudma.c


[...]

+static int bcm6348_iudma_enable(struct dma *dma)
+{
+    struct bcm6348_iudma_priv *priv = dev_get_priv(dma->dev);
+    struct bcm6348_chan_priv *ch_priv = priv->ch_priv[dma->id];
+    struct bcm6348_dma_desc *dma_desc;
+    uint8_t i;
+
+    /* init dma rings */
+    dma_desc = ch_priv->dma_ring;
+    for (i = 0; i < ch_priv->dma_ring_size; i++) {
+    if (bcm6348_iudma_chan_is_rx(dma->id)) {
+    dma_desc->status = DMAD_ST_OWN_MASK;
+    dma_desc->length = PKTSIZE_ALIGN;
+    dma_desc->address = virt_to_phys(net_rx_packets[i]);

You are filling RX queue/ring with buffers defined in Net core.
Does it mean that this DMA driver will not be usable for other
purposes, as
Net can be compiled out?

As far as I know, and depending on the specific SoC, BCM63xx IUDMA is
used for Ethernet, USB (device only) and xDSL.
So yes, in u-boot it will be used for ethernet only.
BTW, my first attempt didn't use net_rx_packets, but I saw that in
pic32_eth implementation and dropped the dma specific buffers. I will
add them again ;).

it is really net specific :)


Wouldn't it be reasonable to have some sort of .receive_prepare()
callback in
DMA dma_ops, so DMA user can control which buffers to push in RX DMA
channel?
And it also can be used in eth_ops.free_pkt() callback (see below).

Yes, probably, but maybe we can achieve that without adding another call.

I'm looking at this patch set from our HW point of view. In my case,
DMA channel can be used with different IPs (not only networking), so
it would be really great if DMA user can pass RX buffers in DMA driver -
network driver can use net_rx_packets, other drivers might use own buffers.
So hard-codding RX buffers in DMA driver looks like not a good choice.


+
+    /* flush cache */
+    flush_dcache_range((ulong)dma_desc,
+    ALIGN_END_ADDR(struct bcm6348_dma_desc, dma_desc, 1));

Could you clarify pls, if you do return dma_desc to RX ring here or not?

Yes.

if yes, wouldn't it cause potential problem on Net RX path
     ret = eth_get_ops(current)->recv(current, flags, &packet);
^^ (1) here buffer will be received from DMA ( and pushed back to RX
ring ?? )

     flags = 0;
     if (ret > 0)
     net_process_received_packet(packet, ret);
^^ (2) here it will be passed in Net stack

     if (ret >= 0 && eth_get_ops(current)->free_pkt)
     eth_get_ops(current)->free_pkt(current, packet, ret);
^^ at this point it should be safe to return buffer in DMA RX ring.

     if (ret <= 0)
     break;

Can DMA overwrite packet after point (1) while packet is still
processed (2)?

I don't think so, because as far as I know u-boot is not processing more
than one packet at once, is it?

u-boot can't process more than one packet, but dma does. if buffer returned
to DMA and there are some traffic on the line - DMA can potentially refill
all buffers in its RX ring while u-boot still processing one packet.
I just sent a new version of the patches which doesn't use 
net_rx_packets in bcm6348-iudma.
However, after several tests it turns out that dma received buffers must 
be processed as soon as possible in order to avoid flow control issues.
Considering that, releasing the buffer by using free_pkt after the 
packet is processed isn't a valid option, since it makes ethernet unusable.
That's why I decided to allocate a dynamic buffer on bcm6348-iudma, 
saving the received packets on net_rx_packets and immediately returning 
the dma descriptor to the RX ring.


However, you can still add dma_receive_prepare to dma ops if you need it 
for your HW.




But yeah, I see your point and if it does process more than one packet
at once this is not the proper way to do that.
I will use free_pkt in next version and lock the packet until it's
processed.



Regards,
Álvaro.

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot