[U-Boot] [RFC v3 03/15] dma: add bcm6348-iudma support
Álvaro Fernández Rojas
noltari at gmail.com
Thu Feb 22 20:48:22 UTC 2018
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 <noltari at gmail.com>
>> ---
>> 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 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_WRAP_MASK;
>> + status |= DMAD_ST_OWN_MASK;
>> +
>> + dma_desc->length = PKTSIZE_ALIGN;
>> + dma_desc->status = status;
>> +
>> + /* 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?
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.
>
>> +
>> + /* set flow control buffer alloc */
>> + writel_be(1, priv->base + DMA_FLOWC_ALLOC_REG(dma->id));
>> +
>> + /* enable dma */
>> + setbits_be32(priv->chan + DMAC_CFG_REG(dma->id), DMAC_CFG_ENABLE_MASK);
>> +
>> + /* set interrupt */
>> + writel_be(DMAC_IR_DONE_MASK, priv->chan + DMAC_IR_EN_REG(dma->id));
>> +
>> + /* increment dma descriptor */
>> + ch_priv->desc_id = (ch_priv->desc_id + 1) % ch_priv->dma_ring_size;
>> +
>> + return ret - 4;
>> +}
>> +
>
>
Regards,
Álvaro.
More information about the U-Boot
mailing list