[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