[PATCH 1/2] net: macb: fix dma_alloc for rx_buffer
Steffen Trumtrar
s.trumtrar at pengutronix.de
Tue Nov 28 22:48:06 PST 2023
On 2023-11-28 at 17:56 +01, Lucas Stach <l.stach at pengutronix.de> wrote:
> Am Dienstag, dem 28.11.2023 um 17:29 +0100 schrieb Steffen Trumtrar:
>> rx_buffer gets dma_alloc'ed but is never dma_map'ed and therefor not
>> flushed before it is initially used.
>>
>> Map the rx_buffer when the macb is initialized and unmap it on ether_halt.
>>
>> While at it, cleanup the dma_alloc_coherent rx_ring/tx_ring, too.
>>
>> Signed-off-by: Steffen Trumtrar <s.trumtrar at pengutronix.de>
>> ---
>> drivers/net/macb.c | 37 ++++++++++++++++++++++++++++---------
>> 1 file changed, 28 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/net/macb.c b/drivers/net/macb.c
>> index 260c1e806a..92f78f7253 100644
>> --- a/drivers/net/macb.c
>> +++ b/drivers/net/macb.c
>> @@ -63,10 +63,13 @@ struct macb_device {
>> unsigned int tx_head;
>>
>> void *rx_buffer;
>> + dma_addr_t rx_buffer_phys;
>> void *tx_buffer;
>> void *rx_packet_buf;
>> struct macb_dma_desc *rx_ring;
>> + dma_addr_t rx_ring_phys;
>> struct macb_dma_desc *tx_ring;
>> + dma_addr_t tx_ring_phys;
>> struct macb_dma_desc *gem_q1_descs;
>>
>> int rx_buffer_size;
>> @@ -181,7 +184,7 @@ static int gem_recv(struct eth_device *edev)
>> barrier();
>> status = macb->rx_ring[macb->rx_tail].ctrl;
>> length = MACB_BFEXT(RX_FRMLEN, status);
>> - buffer = macb->rx_buffer + macb->rx_buffer_size * macb->rx_tail;
>> + buffer = (void *)macb->rx_buffer_phys + macb->rx_buffer_size * macb->rx_tail;
>> dma_sync_single_for_cpu(macb->dev, (unsigned long)buffer, length,
>> DMA_FROM_DEVICE);
>> net_receive(edev, buffer, length);
>> @@ -221,7 +224,7 @@ static int macb_recv(struct eth_device *edev)
>> }
>>
>> if (status & MACB_BIT(RX_EOF)) {
>> - buffer = macb->rx_buffer + macb->rx_buffer_size * macb->rx_tail;
>> + buffer = (void *)macb->rx_buffer_phys + macb->rx_buffer_size * macb->rx_tail;
>> length = MACB_BFEXT(RX_FRMLEN, status);
>> if (wrapped) {
>> unsigned int headlen, taillen;
>> @@ -232,12 +235,12 @@ static int macb_recv(struct eth_device *edev)
>> dma_sync_single_for_cpu(macb->dev, (unsigned long)buffer,
>> headlen, DMA_FROM_DEVICE);
>> memcpy(macb->rx_packet_buf, buffer, headlen);
>> - dma_sync_single_for_cpu(macb->dev, (unsigned long)macb->rx_buffer,
>> + dma_sync_single_for_cpu(macb->dev, (unsigned long)macb->rx_buffer_phys,
>
> You can drop all those (unsigned long) casts in calls to
> dma_sync_single, now that you are passing a argument of the proper
> dma_addr_t type.
>
Thanks, will drop.
>> taillen, DMA_FROM_DEVICE);
>> memcpy(macb->rx_packet_buf + headlen, macb->rx_buffer, taillen);
>> dma_sync_single_for_device(macb->dev, (unsigned long)buffer,
>> headlen, DMA_FROM_DEVICE);
>> - dma_sync_single_for_device(macb->dev, (unsigned long)macb->rx_buffer,
>> + dma_sync_single_for_device(macb->dev, (unsigned long)macb->rx_buffer_phys,
>> taillen, DMA_FROM_DEVICE);
>> net_receive(edev, macb->rx_packet_buf, length);
>> } else {
>> @@ -377,7 +380,7 @@ static int gmac_init_dummy_tx_queues(struct macb_device *macb)
>> return 0;
>> }
>>
>> -static void macb_init(struct macb_device *macb)
>> +static int macb_init(struct macb_device *macb)
>> {
>> unsigned long paddr, val = 0;
>> int i;
>> @@ -386,6 +389,11 @@ static void macb_init(struct macb_device *macb)
>> * macb_halt should have been called at some point before now,
>> * so we'll assume the controller is idle.
>> */
>> + macb->rx_buffer_phys = dma_map_single(macb->dev, macb->rx_buffer,
>> + macb->rx_buffer_size * macb->rx_ring_size,
>> + DMA_TO_DEVICE);
>
> The RX buffer is used to hold data written by the device, so it must be
> mapped with DMA_FROM_DEVICE.
>
Argh, of course :(
Thanks,
Steffen
--
Pengutronix e.K. | Dipl.-Inform. Steffen Trumtrar |
Steuerwalder Str. 21 | https://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686| Fax: +49-5121-206917-5555 |
More information about the barebox
mailing list