Page MenuHomeFreeBSD
Differential D46273

mlx5: Remove a less than helpful debug print
ClosedPublic
Actions

Authored by markj on Aug 12 2024, 4:26 PM.
Tags
None
Referenced Files
Unknown Object (File)
Mon, Apr 14, 3:11 PM
Unknown Object (File)
Mar 18 2025, 12:17 AM
Unknown Object (File)
Feb 13 2025, 6:03 AM
Unknown Object (File)
Feb 12 2025, 1:16 AM
Unknown Object (File)
Jan 12 2025, 7:49 AM
Unknown Object (File)
Dec 6 2024, 7:41 PM
Unknown Object (File)
Dec 6 2024, 3:35 PM
Unknown Object (File)
Nov 26 2024, 8:42 AM
View All 57 Files
Subscribers
ae
imp

Details

Reviewers
kib
khng
Commits
rG27211b7998d1: mlx5: Remove a less than helpful debug print
Summary

Fixes: e23731db48ef9 ("mlx5en: add IPSEC_OFFLOAD support")

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Revision Contents
Changeset List

PathSize
sys/
dev/
mlx5/
mlx5_core/
1 line

Diff 142026

View Options

sys/dev/mlx5/mlx5_core/mlx5_main.c

/*- /*-
* Copyright (c) 2016 Ruslan Bukin <br@bsdpad.com> * Copyright (c) 2016-2017 Ruslan Bukin <br@bsdpad.com>
* All rights reserved. * All rights reserved.
* *
* This software was developed by SRI International and the University of * This software was developed by SRI International and the University of
* Cambridge Computer Laboratory under DARPA/AFRL contract FA8750-10-C-0237 * Cambridge Computer Laboratory under DARPA/AFRL contract FA8750-10-C-0237
* ("CTSRD"), as part of the DARPA CRASH research programme. * ("CTSRD"), as part of the DARPA CRASH research programme.
* *
* Redistribution and use in source and binary forms, with or without * Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions * modification, are permitted provided that the following conditions
Show All 20 Lines
#include <sys/cdefs.h> #include <sys/cdefs.h>
__FBSDID("$FreeBSD$"); __FBSDID("$FreeBSD$");
#include "opt_platform.h" #include "opt_platform.h"
#include <sys/param.h> #include <sys/param.h>
#include <sys/conf.h> #include <sys/conf.h>
#include <sys/bus.h> #include <sys/bus.h>
#include <sys/kernel.h> #include <sys/kernel.h>
#include <sys/kthread.h>
#include <sys/queue.h> #include <sys/queue.h>
#include <sys/kobj.h> #include <sys/kobj.h>
#include <sys/malloc.h> #include <sys/malloc.h>
#include <sys/mutex.h> #include <sys/mutex.h>
#include <sys/mbuf.h>
#include <sys/limits.h> #include <sys/limits.h>
#include <sys/lock.h> #include <sys/lock.h>
#include <sys/sysctl.h> #include <sys/sysctl.h>
#include <sys/systm.h> #include <sys/systm.h>
#include <sys/sx.h> #include <sys/sx.h>
#include <sys/bus_dma.h> #include <sys/bus_dma.h>
#include <machine/bus.h> #include <machine/bus.h>
#ifdef FDT #ifdef FDT
#include <dev/fdt/fdt_common.h> #include <dev/fdt/fdt_common.h>
#include <dev/ofw/ofw_bus.h> #include <dev/ofw/ofw_bus.h>
#include <dev/ofw/ofw_bus_subr.h> #include <dev/ofw/ofw_bus_subr.h>
#endif #endif
#include <dev/xdma/xdma.h> #include <dev/xdma/xdma.h>
#include <xdma_if.h> #include <xdma_if.h>
MALLOC_DEFINE(M_XDMA, "xdma", "xDMA framework");
/* /*
* Multiple xDMA controllers may work with single DMA device, * Multiple xDMA controllers may work with single DMA device,
* so we have global lock for physical channel management. * so we have global lock for physical channel management.
*/ */
static struct mtx xdma_mtx; static struct mtx xdma_mtx;
#define XDMA_LOCK() mtx_lock(&xdma_mtx) #define XDMA_LOCK() mtx_lock(&xdma_mtx)
#define XDMA_UNLOCK() mtx_unlock(&xdma_mtx) #define XDMA_UNLOCK() mtx_unlock(&xdma_mtx)
#define XDMA_ASSERT_LOCKED() mtx_assert(&xdma_mtx, MA_OWNED) #define XDMA_ASSERT_LOCKED() mtx_assert(&xdma_mtx, MA_OWNED)
/* /*
* Per channel locks. * Per channel locks.
*/ */
#define XCHAN_LOCK(xchan) mtx_lock(&(xchan)->mtx_lock) #define XCHAN_LOCK(xchan) mtx_lock(&(xchan)->mtx_lock)
#define XCHAN_UNLOCK(xchan) mtx_unlock(&(xchan)->mtx_lock) #define XCHAN_UNLOCK(xchan) mtx_unlock(&(xchan)->mtx_lock)
#define XCHAN_ASSERT_LOCKED(xchan) mtx_assert(&(xchan)->mtx_lock, MA_OWNED) #define XCHAN_ASSERT_LOCKED(xchan) mtx_assert(&(xchan)->mtx_lock, MA_OWNED)
static int xchan_bufs_alloc(xdma_channel_t *xchan);
static int xchan_bufs_free(xdma_channel_t *xchan);
static void
xdma_task(void *arg)
{
xdma_controller_t *xdma;
xdma_channel_t *xchan_tmp;
xdma_channel_t *xchan;
xdma = arg;
for (;;) {
mtx_lock(&xdma->proc_mtx);
msleep(xdma, &xdma->proc_mtx, PRIBIO, "jobqueue", hz);
mtx_unlock(&xdma->proc_mtx);
if (TAILQ_EMPTY(&xdma->channels)) {
continue;
}
TAILQ_FOREACH_SAFE(xchan, &xdma->channels, xchan_next, xchan_tmp) {
if (xchan->flags & XCHAN_TYPE_SG) {
xdma_queue_submit(xchan);
}
}
}
}
/* /*
* Allocate virtual xDMA channel. * Allocate virtual xDMA channel.
*/ */
xdma_channel_t * xdma_channel_t *
xdma_channel_alloc(xdma_controller_t *xdma) xdma_channel_alloc(xdma_controller_t *xdma, uint32_t caps)
{ {
xdma_channel_t *xchan; xdma_channel_t *xchan;
int ret; int ret;
xchan = malloc(sizeof(xdma_channel_t), M_XDMA, M_WAITOK | M_ZERO); xchan = malloc(sizeof(xdma_channel_t), M_XDMA, M_WAITOK | M_ZERO);
if (xchan == NULL) { if (xchan == NULL) {
device_printf(xdma->dev, device_printf(xdma->dev,
"%s: Can't allocate memory for channel.\n", __func__); "%s: Can't allocate memory for channel.\n", __func__);
return (NULL); return (NULL);
} }
xchan->xdma = xdma; xchan->xdma = xdma;
xchan->caps = caps;
XDMA_LOCK(); XDMA_LOCK();
/* Request a real channel from hardware driver. */ /* Request a real channel from hardware driver. */
ret = XDMA_CHANNEL_ALLOC(xdma->dma_dev, xchan); ret = XDMA_CHANNEL_ALLOC(xdma->dma_dev, xchan);
if (ret != 0) { if (ret != 0) {
device_printf(xdma->dev, device_printf(xdma->dev,
"%s: Can't request hardware channel.\n", __func__); "%s: Can't request hardware channel.\n", __func__);
Show All 29 Lines if (err != 0) {
device_printf(xdma->dev, device_printf(xdma->dev,
"%s: Can't free real hw channel.\n", __func__); "%s: Can't free real hw channel.\n", __func__);
XDMA_UNLOCK(); XDMA_UNLOCK();
return (-1); return (-1);
} }
xdma_teardown_all_intr(xchan); xdma_teardown_all_intr(xchan);
/* Deallocate descriptors, if any. */ /* Deallocate bufs, if any. */
xdma_desc_free(xchan); xchan_bufs_free(xchan);
xchan_sglist_free(xchan);
if (xchan->flags & XCHAN_TYPE_SG) {
free(xchan->xr, M_XDMA);
}
mtx_destroy(&xchan->mtx_lock); mtx_destroy(&xchan->mtx_lock);
TAILQ_REMOVE(&xdma->channels, xchan, xchan_next); TAILQ_REMOVE(&xdma->channels, xchan, xchan_next);
free(xchan, M_XDMA); free(xchan, M_XDMA);
XDMA_UNLOCK(); XDMA_UNLOCK();
return (0); return (0);
} }
int int
xdma_setup_intr(xdma_channel_t *xchan, int (*cb)(void *), void *arg, xdma_setup_intr(xdma_channel_t *xchan,
void **ihandler) int (*cb)(void *, xdma_transfer_status_t *),
void *arg, void **ihandler)
{ {
struct xdma_intr_handler *ih; struct xdma_intr_handler *ih;
xdma_controller_t *xdma; xdma_controller_t *xdma;
xdma = xchan->xdma; xdma = xchan->xdma;
KASSERT(xdma != NULL, ("xdma is NULL")); KASSERT(xdma != NULL, ("xdma is NULL"));
/* Sanity check. */ /* Sanity check. */
▲ Show 20 Lines • Show All 61 Lines • ▼ Show 20 Linesxdma_teardown_all_intr(xdma_channel_t *xchan)
TAILQ_FOREACH_SAFE(ih, &xchan->ie_handlers, ih_next, ih_tmp) { TAILQ_FOREACH_SAFE(ih, &xchan->ie_handlers, ih_next, ih_tmp) {
TAILQ_REMOVE(&xchan->ie_handlers, ih, ih_next); TAILQ_REMOVE(&xchan->ie_handlers, ih, ih_next);
free(ih, M_XDMA); free(ih, M_XDMA);
} }
return (0); return (0);
} }
static void static int
xdma_dmamap_cb(void *arg, bus_dma_segment_t *segs, int nseg, int err) xdma_bufs_alloc_no_busdma(xdma_channel_t *xchan)
kanUnsubmitted
Done
Inline Actions

This is a xdma_desc_dmamap_cb in , the name is too short and nondescript. I do not think descriptor allocation belongs in xdma at all, but this is wrong nonetheless.

kan: This is a xdma_desc_dmamap_cb in , the name is too short and nondescript. I do not think…
brAuthorUnsubmitted
Not Done
Inline Actions

I moved descriptors allocation to DMA engine drivers. Thanks

br: I moved descriptors allocation to DMA engine drivers. Thanks
{ {
xdma_channel_t *xchan; xdma_controller_t *xdma;
int nsegments;
int i; int i;
xchan = (xdma_channel_t *)arg; xdma = xchan->xdma;
KASSERT(xchan != NULL, ("xchan is NULL"));
if (err) { nsegments = xchan->bufs_num;
xchan->map_err = 1;
return; xchan->bufs = malloc(nsegments * sizeof(struct xchan_buf),
M_XDMA, (M_WAITOK | M_ZERO));
if (xchan->bufs == NULL) {
device_printf(xdma->dev,
"%s: Can't allocate memory.\n", __func__);
return (-1);
} }
for (i = 0; i < nseg; i++) { for (i = 0; i < nsegments; i++) {
xchan->descs_phys[i].ds_addr = segs[i].ds_addr; xchan->bufs[i].cbuf = contigmalloc(xchan->maxsegsize,
xchan->descs_phys[i].ds_len = segs[i].ds_len; M_XDMA, 0, 0, ~0, PAGE_SIZE, 0);
} }
return (0);
} }
static int static int
xdma_desc_alloc_bus_dma(xdma_channel_t *xchan, uint32_t desc_size, xdma_bufs_alloc_busdma(xdma_channel_t *xchan)
uint32_t align)
{ {
xdma_controller_t *xdma; xdma_controller_t *xdma;
bus_size_t all_desc_sz;
xdma_config_t *conf;
int nsegments; int nsegments;
int err; int err;
int i;
xdma = xchan->xdma; xdma = xchan->xdma;
conf = &xchan->conf;
nsegments = conf->block_num; nsegments = xchan->bufs_num;
all_desc_sz = (nsegments * desc_size);
#if 0
printf("%s: nseg %d\n", __func__, nsegments);
#endif
xchan->bufs = malloc(nsegments * sizeof(struct xchan_buf),
M_XDMA, (M_WAITOK | M_ZERO));
if (xchan->bufs == NULL) {
device_printf(xdma->dev,
"%s: Can't allocate memory.\n", __func__);
return (-1);
}
err = bus_dma_tag_create( err = bus_dma_tag_create(
bus_get_dma_tag(xdma->dev), bus_get_dma_tag(xdma->dev), /* Parent tag. */
align, desc_size, /* alignment, boundary */ xchan->alignment, 0, /* alignment, boundary */
BUS_SPACE_MAXADDR_32BIT, /* lowaddr */ BUS_SPACE_MAXADDR_32BIT, /* lowaddr */
kanUnsubmitted
Done
Inline Actions

Why? How xchan knows boundaries handed by the underlying DMA device? How does it even know if bus_get_dma_tag(xdma->dev) is even a right tag to be used for descriptors? Descriptors often have much stricter requirements on alignment and locations of descriptors and just 'uint32_t desc_size, uint32_t align' cannot possibly describe them. The operation does not belong in generic xchan layer at all and is better left to an actual pdma implementation.

kan: Why? How xchan knows boundaries handed by the underlying DMA device? How does it even know if…
brAuthorUnsubmitted
Not Done
Inline Actions

Sounds right, thanks. I moved all the descriptors allocation/handling to DMA engine driver.

br: Sounds right, thanks. I moved all the descriptors allocation/handling to DMA engine driver.
BUS_SPACE_MAXADDR, /* highaddr */ BUS_SPACE_MAXADDR, /* highaddr */
NULL, NULL, /* filter, filterarg */ NULL, NULL, /* filter, filterarg */
all_desc_sz, nsegments, /* maxsize, nsegments*/ xchan->maxsegsize * xchan->maxnsegs, /* maxsize */
desc_size, 0, /* maxsegsize, flags */ xchan->maxnsegs, /* nsegments */
xchan->maxsegsize, /* maxsegsize */
0, /* flags */
NULL, NULL, /* lockfunc, lockarg */ NULL, NULL, /* lockfunc, lockarg */
&xchan->dma_tag); &xchan->dma_tag_bufs);
if (err) { if (err != 0) {
device_printf(xdma->dev, device_printf(xdma->dev,
"%s: Can't create bus_dma tag.\n", __func__); "%s: Can't create bus_dma tag.\n", __func__);
return (-1); return (-1);
} }
err = bus_dmamem_alloc(xchan->dma_tag, (void **)&xchan->descs, for (i = 0; i < nsegments; i++) {
BUS_DMA_WAITOK | BUS_DMA_COHERENT, &xchan->dma_map); err = bus_dmamap_create(xchan->dma_tag_bufs, 0,
if (err) { &xchan->bufs[i].map);
if (err != 0) {
device_printf(xdma->dev, device_printf(xdma->dev,
"%s: Can't allocate memory for descriptors.\n", __func__); "%s: Can't create buf DMA map.\n", __func__);
return (-1); return (-1);
} }
}
xchan->descs_phys = malloc(nsegments * sizeof(xdma_descriptor_t), M_XDMA, return (0);
(M_WAITOK | M_ZERO)); }
xchan->map_err = 0; static int
err = bus_dmamap_load(xchan->dma_tag, xchan->dma_map, xchan->descs, xchan_bufs_alloc(xdma_channel_t *xchan)
all_desc_sz, xdma_dmamap_cb, xchan, BUS_DMA_WAITOK); {
if (err) { xdma_controller_t *xdma;
int ret;
xdma = xchan->xdma;
if (xdma == NULL) {
device_printf(xdma->dev, device_printf(xdma->dev,
"%s: Can't load DMA map.\n", __func__); "%s: Channel was not allocated properly.\n", __func__);
return (-1); return (-1);
} }
if (xchan->map_err != 0) { if (xchan->caps & XCHAN_CAP_BUSDMA) {
ret = xdma_bufs_alloc_busdma(xchan);
} else {
ret = xdma_bufs_alloc_no_busdma(xchan);
}
kanUnsubmitted
Not Done
Inline Actions

Error handling, in particular, freeing already allocated memory and descriptors and unmapping mapped descriptors is considered futile?

kan: Error handling, in particular, freeing already allocated memory and descriptors and unmapping…
if (ret != 0) {
device_printf(xdma->dev, device_printf(xdma->dev,
"%s: Can't load DMA map.\n", __func__); "%s: Can't setup busdma.\n",
__func__);
return (-1); return (-1);
} }
xchan->flags |= XCHAN_BUFS_ALLOCATED;
return (0); return (0);
} }
/* static int
* This function called by DMA controller driver. xchan_bufs_free(xdma_channel_t *xchan)
*/ {
xchan_buf_t *b;
kanUnsubmitted
Done
Inline Actions

The name of the function and later use of MCLBYTES do correspond to each other rather weakly. It if an mbuf-specific functionality, name better should reflect that.

kan: The name of the function and later use of MCLBYTES do correspond to each other rather weakly.
int i;
kanUnsubmitted
Done
Inline Actions

Same thing. This seems to be very mbuf specific and ridden with arbitrary limitations.

kan: Same thing. This seems to be very mbuf specific and ridden with arbitrary limitations.
brAuthorUnsubmitted
Not Done
Inline Actions

tag parameters now specified by transfer preparation functions, e.g. xdma_prep_sg

br: tag parameters now specified by transfer preparation functions, e.g. xdma_prep_sg
if ((xchan->flags & XCHAN_BUFS_ALLOCATED) == 0) {
/* No bufs allocated. */
return (-1);
}
for (i = 0; i < xchan->bufs_num; i++) {
b = &xchan->bufs[i];
bus_dmamap_destroy(xchan->dma_tag_bufs, b->map);
}
bus_dma_tag_destroy(xchan->dma_tag_bufs);
free(xchan->bufs, M_XDMA);
xchan->flags &= ~XCHAN_BUFS_ALLOCATED;
return (0);
}
int int
xdma_desc_alloc(xdma_channel_t *xchan, uint32_t desc_size, uint32_t align) xdma_prep_memcpy(xdma_channel_t *xchan, uintptr_t src_addr,
uintptr_t dst_addr, size_t len)
{ {
kanUnsubmitted
Done
Inline Actions

Again, WHY???

kan: Again, WHY???
xdma_controller_t *xdma; xdma_controller_t *xdma;
xdma_config_t *conf; xdma_config_t *conf;
int ret; int ret;
XCHAN_ASSERT_LOCKED(xchan); xdma = xchan->xdma;
KASSERT(xdma != NULL, ("xdma is NULL"));
conf = &xchan->conf;
conf->direction = XDMA_MEM_TO_MEM;
conf->src_addr = src_addr;
conf->dst_addr = dst_addr;
conf->block_len = len;
conf->block_num = 1;
xchan->flags |= (XCHAN_CONFIGURED | XCHAN_TYPE_MEMCPY);
XCHAN_LOCK(xchan);
ret = XDMA_CHANNEL_PREP_MEMCPY(xdma->dma_dev, xchan);
if (ret != 0) {
device_printf(xdma->dev,
"%s: Can't prepare memcpy transfer.\n", __func__);
XCHAN_UNLOCK(xchan);
return (-1);
}
XCHAN_UNLOCK(xchan);
kanUnsubmitted
Not Done
Inline Actions

BUS_DMA_COHERENT???

kan: BUS_DMA_COHERENT???
return (0);
}
/*
* xr_num - xdma requests queue size,
* maxsegsize - maximum allowed scatter-gather list element size in bytes
*/
int
xdma_prep_sg(xdma_channel_t *xchan, uint32_t xr_num,
uint32_t maxsegsize, uint32_t alignment)
{
xdma_controller_t *xdma;
int ret;
xdma = xchan->xdma; xdma = xchan->xdma;
if (xdma == NULL) {
KASSERT(xdma != NULL, ("xdma is NULL"));
if (xchan->flags & XCHAN_CONFIGURED) {
device_printf(xdma->dev, device_printf(xdma->dev,
"%s: Channel was not allocated properly.\n", __func__); "%s: Channel is already configured.\n", __func__);
return (-1); return (-1);
} }
if (xchan->flags & XCHAN_DESC_ALLOCATED) { xchan->maxsegsize = maxsegsize;
xchan->alignment = alignment;
xchan->maxnsegs = 8;
xchan->bufs_num = xr_num;
xchan->xr_num = xr_num;
/* Allocate sglist. */
ret = xchan_sglist_init(xchan);
if (ret != 0) {
device_printf(xdma->dev, device_printf(xdma->dev,
"%s: Descriptors already allocated.\n", __func__); "%s: Can't allocate sglist.\n", __func__);
return (-1); return (-1);
} }
if ((xchan->flags & XCHAN_CONFIGURED) == 0) { /* Allocate request queue. */
xchan->xr = malloc(sizeof(struct xdma_request) * xr_num,
M_XDMA, M_WAITOK | M_ZERO);
if (xchan->xr == NULL) {
device_printf(xdma->dev, device_printf(xdma->dev,
"%s: Channel has no configuration.\n", __func__); "%s: Can't allocate request queue.\n", __func__);
return (-1); return (-1);
} }
conf = &xchan->conf; xchan->flags |= (XCHAN_CONFIGURED | XCHAN_TYPE_SG);
XCHAN_UNLOCK(xchan);
ret = xdma_desc_alloc_bus_dma(xchan, desc_size, align);
XCHAN_LOCK(xchan); XCHAN_LOCK(xchan);
/* Deallocate bufs, if any. */
xchan_bufs_free(xchan);
ret = XDMA_CHANNEL_PREP_SG(xdma->dma_dev, xchan);
if (ret != 0) { if (ret != 0) {
device_printf(xdma->dev, device_printf(xdma->dev,
"%s: Can't allocate memory for descriptors.\n", "%s: Can't prepare SG transfer.\n", __func__);
__func__); XCHAN_UNLOCK(xchan);
return (-1); return (-1);
} }
xchan->flags |= XCHAN_DESC_ALLOCATED; /* Allocate bufs */
ret = xchan_bufs_alloc(xchan);
if (ret != 0) {
device_printf(xdma->dev,
"%s: Can't allocate bufs.\n", __func__);
return (-1);
}
/* We are going to write to descriptors. */ XCHAN_UNLOCK(xchan);
bus_dmamap_sync(xchan->dma_tag, xchan->dma_map, BUS_DMASYNC_PREWRITE);
return (0); return (0);
} }
int int
xdma_desc_free(xdma_channel_t *xchan) xdma_dequeue(xdma_channel_t *xchan, void **user,
xdma_transfer_status_t *status)
{ {
struct xdma_request *xr;
if ((xchan->flags & XCHAN_DESC_ALLOCATED) == 0) { if (xchan->xr_tail == xchan->xr_processed) {
/* No descriptors allocated. */
return (-1); return (-1);
} }
bus_dmamap_unload(xchan->dma_tag, xchan->dma_map); xr = &xchan->xr[xchan->xr_tail];
bus_dmamem_free(xchan->dma_tag, xchan->descs, xchan->dma_map); if (xr->done == 0) {
bus_dma_tag_destroy(xchan->dma_tag); return (-1);
free(xchan->descs_phys, M_XDMA); }
xchan->flags &= ~(XCHAN_DESC_ALLOCATED); *user = xr->user;
status->error = xr->status.error;
status->transferred = xr->status.transferred;
xchan->xr_tail = xchan_next_req(xchan, xchan->xr_tail);
atomic_subtract_int(&xchan->xr_count, 1);
kanUnsubmitted
Done
Inline Actions

You did not protect XCHAN_DESC_ALLOCATED with ()'s above, why now?

kan: You did not protect XCHAN_DESC_ALLOCATED with ()'s above, why now?
return (0); return (0);
} }
int int
xdma_prep_memcpy(xdma_channel_t *xchan, uintptr_t src_addr, xdma_enqueue(xdma_channel_t *xchan, uintptr_t src, uintptr_t dst,
uintptr_t dst_addr, size_t len) bus_size_t len, enum xdma_direction dir, void *user)
{ {
struct xdma_request *xr;
xdma_controller_t *xdma; xdma_controller_t *xdma;
xdma_config_t *conf;
xdma = xchan->xdma;
if (xchan->xr_count >= (xchan->xr_num - 1)) {
/* No space is available yet. */
return (-1);
kanUnsubmitted
Done
Inline Actions

This is unconditional? You did not call map_create for !XCHAN_CAP_BUSDMA case.

kan: This is unconditional? You did not call map_create for !XCHAN_CAP_BUSDMA case.
brAuthorUnsubmitted
Not Done
Inline Actions

Fixed. thanks!

br: Fixed. thanks!
}
xr = &xchan->xr[xchan->xr_head];
xr->user = user;
xr->direction = dir;
kanUnsubmitted
Done
Inline Actions

You did not create tag either.

kan: You did not create tag either.
xr->m = NULL;
xr->len = len;
xr->type = 0;
kanUnsubmitted
Done
Inline Actions

Same here? Why ()'?

kan: Same here? Why ()'?
xr->src_addr = src;
xr->dst_addr = dst;
#if 0
if (dir == XDMA_MEM_TO_DEV) {
xr->dst_addr = addr;
xr->src_addr = 0;
} else {
xr->src_addr = addr;
xr->dst_addr = 0;
}
#endif
xr->done = 0;
xchan->xr_head = xchan_next_req(xchan, xchan->xr_head);
atomic_add_int(&xchan->xr_count, 1);
return (0);
}
struct seg_load_request {
struct bus_dma_segment *seg;
uint32_t nsegs;
uint32_t error;
};
static void
xdma_get1paddr(void *arg, bus_dma_segment_t *segs, int nsegs, int error)
{
struct seg_load_request *slr;
struct bus_dma_segment *seg;
int i;
slr = arg;
seg = slr->seg;
if (error != 0) {
slr->error = error;
return;
}
slr->nsegs = nsegs;
for (i = 0; i < nsegs; i++) {
seg[i].ds_addr = segs[i].ds_addr;
seg[i].ds_len = segs[i].ds_len;
}
}
static int
xdma_load_busdma(xdma_channel_t *xchan, struct xdma_request *xr,
struct bus_dma_segment *seg, uint32_t i)
{
xdma_controller_t *xdma;
struct seg_load_request slr;
uint32_t nsegs;
void *addr;
int error;
xdma = xchan->xdma;
error = 0;
nsegs = 0;
switch (xr->type) {
case XR_TYPE_MBUF:
error = bus_dmamap_load_mbuf_sg(xchan->dma_tag_bufs,
xchan->bufs[i].map, xr->m, seg, &nsegs, BUS_DMA_NOWAIT);
break;
case XR_TYPE_BIO:
slr.nsegs = 0;
slr.error = 0;
slr.seg = seg;
error = bus_dmamap_load_bio(xchan->dma_tag_bufs,
xchan->bufs[i].map, xr->bp, xdma_get1paddr, &slr, BUS_DMA_NOWAIT);
if (slr.error != 0) {
device_printf(xdma->dma_dev,
"%s: bus_dmamap_load failed, err %d\n",
__func__, slr.error);
return (0);
}
nsegs = slr.nsegs;
break;
case XR_TYPE_ADDR:
switch (xr->direction) {
case XDMA_MEM_TO_DEV:
addr = (void *)xr->src_addr;
break;
case XDMA_DEV_TO_MEM:
addr = (void *)xr->dst_addr;
break;
default:
device_printf(xdma->dma_dev,
"%s: Direction is not supported\n", __func__);
return (0);
}
slr.nsegs = 0;
slr.error = 0;
slr.seg = seg;
error = bus_dmamap_load(xchan->dma_tag_bufs, xchan->bufs[i].map,
addr, xr->len, xdma_get1paddr, &slr, BUS_DMA_NOWAIT);
if (slr.error != 0) {
device_printf(xdma->dma_dev,
"%s: bus_dmamap_load failed, err %d\n",
__func__, slr.error);
return (0);
}
nsegs = slr.nsegs;
break;
default:
break;
}
if (error != 0) {
if (error == ENOMEM) {
/*
* Out of memory. Try again later.
* TODO: count errors.
*/
} else {
device_printf(xdma->dma_dev,
"%s: bus_dmamap_load failed with err %d\n",
__func__, error);
}
return (0);
}
if (xr->direction == XDMA_MEM_TO_DEV) {
bus_dmamap_sync(xchan->dma_tag_bufs, xchan->bufs[i].map,
BUS_DMASYNC_PREWRITE);
} else {
bus_dmamap_sync(xchan->dma_tag_bufs, xchan->bufs[i].map,
BUS_DMASYNC_PREREAD);
}
return (nsegs);
}
static int
xdma_load_no_busdma(xdma_channel_t *xchan, struct xdma_request *xr,
struct bus_dma_segment *seg, uint32_t i)
{
xdma_controller_t *xdma;
struct mbuf *m;
uint32_t nsegs;
xdma = xchan->xdma;
m = xr->m;
nsegs = 1;
switch (xr->type) {
case XR_TYPE_MBUF:
if (xr->direction == XDMA_MEM_TO_DEV) {
m_copydata(m, 0, m->m_pkthdr.len, xchan->bufs[i].cbuf);
seg[0].ds_addr = (bus_addr_t)xchan->bufs[i].cbuf;
seg[0].ds_len = m->m_pkthdr.len;
} else {
seg[0].ds_addr = mtod(m, bus_addr_t);
seg[0].ds_len = m->m_pkthdr.len;
}
break;
case XR_TYPE_BIO:
case XR_TYPE_ADDR:
default:
panic("implement me\n");
}
return (nsegs);
}
static int
xdma_sglist_prepare_one(xdma_channel_t *xchan,
struct xdma_request *xr, struct bus_dma_segment *seg)
{
xdma_controller_t *xdma;
int error;
int nsegs;
int i;
xdma = xchan->xdma;
error = 0;
nsegs = 0;
i = xchan->buf_head;
if (xchan->caps & XCHAN_CAP_BUSDMA) {
nsegs = xdma_load_busdma(xchan, xr, seg, i);
} else {
nsegs = xdma_load_no_busdma(xchan, xr, seg, i);
}
if (nsegs == 0) {
/* Try again later. */
return (0);
}
xchan->bufs[i].xr = xr;
xchan->bufs[i].nsegs = nsegs;
xchan->bufs[i].nsegs_left = nsegs;
xchan->buf_head = xchan_next_buf(xchan, xchan->buf_head);
return (nsegs);
}
static int
xdma_sglist_prepare(xdma_channel_t *xchan,
struct xdma_sglist *sg)
{
struct bus_dma_segment seg[128];
struct xdma_request *xr;
xdma_controller_t *xdma;
uint32_t capacity;
uint32_t n;
uint32_t c;
int nsegs;
int ret; int ret;
xdma = xchan->xdma; xdma = xchan->xdma;
n = 0;
ret = XDMA_CHANNEL_CAPACITY(xdma->dma_dev, xchan, &capacity);
if (ret != 0) {
device_printf(xdma->dev,
"%s: Can't get DMA controller capacity.\n", __func__);
return (-1);
}
for (;;) {
if (xchan->xr_processed == xchan->xr_head) {
/* All the requests processed. */
break;
}
xr = &xchan->xr[xchan->xr_processed];
switch (xr->type) {
case XR_TYPE_MBUF:
c = xdma_mbuf_defrag(xchan, xr);
break;
case XR_TYPE_BIO:
case XR_TYPE_ADDR:
default:
c = 1;
}
if (capacity <= (c + n)) {
/*
* No space yet available for the entire
* request in the DMA engine.
*/
break;
}
nsegs = xdma_sglist_prepare_one(xchan, xr, seg);
if (nsegs == 0) {
break;
}
xdma_sglist_add(&sg[n], seg, nsegs, xr);
n += nsegs;
xchan->xr_processed = xchan_next_req(xchan, xchan->xr_processed);
}
return (n);
}
int
xdma_queue_submit(xdma_channel_t *xchan)
{
struct xdma_sglist *sg;
xdma_controller_t *xdma;
uint32_t sg_n;
int ret;
xdma = xchan->xdma;
KASSERT(xdma != NULL, ("xdma is NULL")); KASSERT(xdma != NULL, ("xdma is NULL"));
conf = &xchan->conf; sg = xchan->sg;
conf->direction = XDMA_MEM_TO_MEM;
conf->src_addr = src_addr;
conf->dst_addr = dst_addr;
conf->block_len = len;
conf->block_num = 1;
xchan->flags |= (XCHAN_CONFIGURED | XCHAN_TYPE_MEMCPY); if ((xchan->flags & XCHAN_BUFS_ALLOCATED) == 0) {
device_printf(xdma->dev,
"%s: Can't submit SG transfer: no bufs\n",
__func__);
return (-1);
}
XCHAN_LOCK(xchan); XCHAN_LOCK(xchan);
/* Deallocate old descriptors, if any. */ sg_n = xdma_sglist_prepare(xchan, sg);
xdma_desc_free(xchan); if (sg_n == 0) {
/* Nothing to submit */
XCHAN_UNLOCK(xchan);
return (0);
}
ret = XDMA_CHANNEL_PREP_MEMCPY(xdma->dma_dev, xchan); /* Now submit xdma_sglist to DMA engine driver. */
ret = XDMA_CHANNEL_SUBMIT_SG(xdma->dma_dev, xchan, sg, sg_n);
if (ret != 0) { if (ret != 0) {
device_printf(xdma->dev, device_printf(xdma->dev,
"%s: Can't prepare memcpy transfer.\n", __func__); "%s: Can't submit SG transfer.\n", __func__);
XCHAN_UNLOCK(xchan); XCHAN_UNLOCK(xchan);
return (-1); return (-1);
} }
if (xchan->flags & XCHAN_DESC_ALLOCATED) {
/* Driver created xDMA descriptors. */
bus_dmamap_sync(xchan->dma_tag, xchan->dma_map,
BUS_DMASYNC_POSTWRITE);
}
XCHAN_UNLOCK(xchan); XCHAN_UNLOCK(xchan);
return (0); return (0);
} }
int int
xdma_prep_cyclic(xdma_channel_t *xchan, enum xdma_direction dir, xdma_prep_cyclic(xdma_channel_t *xchan, enum xdma_direction dir,
uintptr_t src_addr, uintptr_t dst_addr, int block_len, uintptr_t src_addr, uintptr_t dst_addr, int block_len,
Show All 14 Linesxdma_prep_cyclic(xdma_channel_t *xchan, enum xdma_direction dir,
conf->block_num = block_num; conf->block_num = block_num;
conf->src_width = src_width; conf->src_width = src_width;
conf->dst_width = dst_width; conf->dst_width = dst_width;
xchan->flags |= (XCHAN_CONFIGURED | XCHAN_TYPE_CYCLIC); xchan->flags |= (XCHAN_CONFIGURED | XCHAN_TYPE_CYCLIC);
XCHAN_LOCK(xchan); XCHAN_LOCK(xchan);
/* Deallocate old descriptors, if any. */ /* Deallocate bufs, if any. */
xdma_desc_free(xchan); xchan_bufs_free(xchan);
ret = XDMA_CHANNEL_PREP_CYCLIC(xdma->dma_dev, xchan); ret = XDMA_CHANNEL_PREP_CYCLIC(xdma->dma_dev, xchan);
if (ret != 0) { if (ret != 0) {
kanUnsubmitted
Done
Inline Actions

By using circular buffer here you encode the requirement that dma requests should be completed by the controller in the order they were submitted, but that is not necessarily the case. I guess same applies by sglists.

kan: By using circular buffer here you encode the requirement that dma requests should be completed…
brAuthorUnsubmitted
Not Done
Inline Actions

I switched back to linked lists. Works a bit slower due to locking, but easier to improve /add features in future.
thanks!

br: I switched back to linked lists. Works a bit slower due to locking, but easier to improve /add…
device_printf(xdma->dev, device_printf(xdma->dev,
"%s: Can't prepare cyclic transfer.\n", __func__); "%s: Can't prepare cyclic transfer.\n", __func__);
XCHAN_UNLOCK(xchan); XCHAN_UNLOCK(xchan);
return (-1); return (-1);
} }
if (xchan->flags & XCHAN_DESC_ALLOCATED) {
/* Driver has created xDMA descriptors. */
bus_dmamap_sync(xchan->dma_tag, xchan->dma_map,
BUS_DMASYNC_POSTWRITE);
}
XCHAN_UNLOCK(xchan); XCHAN_UNLOCK(xchan);
return (0); return (0);
} }
int int
kanUnsubmitted
Done
Inline Actions

()'s again

kan: ()'s again
xdma_begin(xdma_channel_t *xchan) xdma_begin(xdma_channel_t *xchan)
{ {
xdma_controller_t *xdma; xdma_controller_t *xdma;
int ret; int ret;
xdma = xchan->xdma; xdma = xchan->xdma;
if (xchan->flags & XCHAN_TYPE_SG) {
/* Not valid. */
return (0);
};
ret = XDMA_CHANNEL_CONTROL(xdma->dma_dev, xchan, XDMA_CMD_BEGIN); ret = XDMA_CHANNEL_CONTROL(xdma->dma_dev, xchan, XDMA_CMD_BEGIN);
if (ret != 0) { if (ret != 0) {
device_printf(xdma->dev, device_printf(xdma->dev,
"%s: Can't begin the channel operation.\n", __func__); "%s: Can't begin the channel operation.\n", __func__);
return (-1); return (-1);
} }
return (0); return (0);
Show All 30 Lines if (ret != 0) {
device_printf(xdma->dev, device_printf(xdma->dev,
"%s: Can't pause the channel operation.\n", __func__); "%s: Can't pause the channel operation.\n", __func__);
return (-1); return (-1);
} }
return (ret); return (ret);
} }
int int
kanUnsubmitted
Not Done
Inline Actions

Is there a direct relationship of number of mbuf fragments and number of physical addresses it can result in?

kan: Is there a direct relationship of number of mbuf fragments and number of physical addresses it…
xdma_callback(xdma_channel_t *xchan) xchan_seg_done(xdma_channel_t *xchan,
struct xdma_transfer_status *st)
{ {
struct xdma_intr_handler *ih_tmp; struct xdma_request *xr;
struct xdma_intr_handler *ih; xdma_controller_t *xdma;
xchan_buf_t *b;
TAILQ_FOREACH_SAFE(ih, &xchan->ie_handlers, ih_next, ih_tmp) { xdma = xchan->xdma;
if (ih->cb != NULL) {
ih->cb(ih->cb_user); b = &xchan->bufs[xchan->buf_tail];
xr = b->xr;
atomic_subtract_int(&b->nsegs_left, 1);
if (b->nsegs_left == 0) {
kanUnsubmitted
Not Done
Inline Actions

What happens if segment in the middle completes with the error?
Also, what is st_trasferred and in what units it is specified? You invoke callback in per-desc (WHY? DMA driver is not allowed to do desc chaining?), but then only report status on the last desc up the consumer chain.

kan: What happens if segment in the middle completes with the error? Also, what is st_trasferred and…
if (xchan->caps & XCHAN_CAP_BUSDMA) {
if (xr->direction == XDMA_MEM_TO_DEV) {
bus_dmamap_sync(xchan->dma_tag_bufs, b->map,
BUS_DMASYNC_POSTWRITE);
} else {
bus_dmamap_sync(xchan->dma_tag_bufs, b->map,
BUS_DMASYNC_POSTREAD);
} }
bus_dmamap_unload(xchan->dma_tag_bufs, b->map);
} }
xr->status.error = st->error;
xr->status.transferred = st->transferred;
xr->done = 1;
xchan->buf_tail = xchan_next_buf(xchan, xchan->buf_tail);
}
return (0); return (0);
} }
void int
xdma_assert_locked(void) xdma_callback(xdma_channel_t *xchan, xdma_transfer_status_t *status)
{ {
struct xdma_intr_handler *ih_tmp;
struct xdma_intr_handler *ih;
xdma_controller_t *xdma;
XDMA_ASSERT_LOCKED(); xdma = xchan->xdma;
TAILQ_FOREACH_SAFE(ih, &xchan->ie_handlers, ih_next, ih_tmp) {
if (ih->cb != NULL) {
ih->cb(ih->cb_user, status);
} }
}
wakeup(xdma);
return (0);
}
#ifdef FDT #ifdef FDT
/* /*
* Notify the DMA driver we have machine-dependent data in FDT. * Notify the DMA driver we have machine-dependent data in FDT.
*/ */
static int static int
xdma_ofw_md_data(xdma_controller_t *xdma, pcell_t *cells, int ncells) xdma_ofw_md_data(xdma_controller_t *xdma, pcell_t *cells, int ncells)
{ {
uint32_t ret; uint32_t ret;
ret = XDMA_OFW_MD_DATA(xdma->dma_dev, cells, ncells, (void **)&xdma->data); ret = XDMA_OFW_MD_DATA(xdma->dma_dev,
cells, ncells, (void **)&xdma->data);
return (ret); return (ret);
} }
/* /*
* Allocate xdma controller. * Allocate xdma controller.
*/ */
xdma_controller_t * xdma_controller_t *
▲ Show 20 Lines • Show All 59 Lines • ▼ Show 20 Linesxdma_ofw_get(device_t dev, const char *prop)
xdma->dev = dev; xdma->dev = dev;
xdma->dma_dev = dma_dev; xdma->dma_dev = dma_dev;
TAILQ_INIT(&xdma->channels); TAILQ_INIT(&xdma->channels);
xdma_ofw_md_data(xdma, cells, ncells); xdma_ofw_md_data(xdma, cells, ncells);
free(cells, M_OFWPROP); free(cells, M_OFWPROP);
mtx_init(&xdma->proc_mtx, "xDMA ofw controller", NULL, MTX_DEF);
kproc_create(&xdma_task, xdma, &xdma->xdma_proc, 0, 0, "xdma drainer");
return (xdma); return (xdma);
} }
#endif #endif
/* /*
* Free xDMA controller object. * Free xDMA controller object.
*/ */
int int
xdma_put(xdma_controller_t *xdma) xdma_put(xdma_controller_t *xdma)
{ {
XDMA_LOCK(); XDMA_LOCK();
/* Ensure no channels allocated. */ /* Ensure no channels allocated. */
if (!TAILQ_EMPTY(&xdma->channels)) { if (!TAILQ_EMPTY(&xdma->channels)) {
device_printf(xdma->dev, "%s: Can't free xDMA\n", __func__); device_printf(xdma->dev, "%s: Can't free xDMA\n", __func__);
return (-1); return (-1);
} }
kproc_shutdown(&xdma->xdma_proc, 0);
mtx_destroy(&xdma->proc_mtx);
free(xdma->data, M_DEVBUF); free(xdma->data, M_DEVBUF);
free(xdma, M_XDMA); free(xdma, M_XDMA);
XDMA_UNLOCK(); XDMA_UNLOCK();
return (0); return (0);
} }
Show All 9 Lines