On Sat, Apr 26, 2014 at 02:03:44PM +0200, Guennadi Liakhovetski wrote:
> This patch adds a driver for NBPF DMAC IP cores from Renesas, designed for
> the AMBA AXI bus.
>
> diff --git a/Documentation/devicetree/bindings/dma/nbpfaxi.txt b/Documentation/devicetree/bindings/dma/nbpfaxi.txt
> new file mode 100644
> index 0000000..d5e2522
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/dma/nbpfaxi.txt
> @@ -0,0 +1,61 @@
> +* Renesas "Type-AXI" NBPFAXI* DMA controllers
> +
> +* DMA controller
> +
> +Required properties
> +
> +- compatible: must be one of
> + "renesas,nbpfaxi64dmac1b4"
> + "renesas,nbpfaxi64dmac1b8"
> + "renesas,nbpfaxi64dmac1b16"
> + "renesas,nbpfaxi64dmac4b4"
> + "renesas,nbpfaxi64dmac4b8"
> + "renesas,nbpfaxi64dmac4b16"
> + "renesas,nbpfaxi64dmac8b4"
> + "renesas,nbpfaxi64dmac8b8"
> + "renesas,nbpfaxi64dmac8b16"
> +- #dma-cells: must be 2: the first integer is a terminal number, to which this
> + slave is connected, the second one is flags. Flags is a bitmask
> + with the following bits defined:
> +
> +#define NBPF_SLAVE_RQ_HIGH 1
> +#define NBPF_SLAVE_RQ_LOW 2
> +#define NBPF_SLAVE_RQ_LEVEL 4
> +
> +Optional properties:
> +
> +You can use dma-channels and dma-requests as described in dma.txt, although they
> +won't be used, this information is derived from the compatibility string.
> +
> +Example:
> +
> + dma: dma-controller@48000000 {
> + compatible = "renesas,nbpfaxi64dmac8b4";
> + reg = <0x48000000 0x400>;
> + interrupts = <0 12 0x4
> + 0 13 0x4
> + 0 14 0x4
> + 0 15 0x4
> + 0 16 0x4
> + 0 17 0x4
> + 0 18 0x4
> + 0 19 0x4>;
> + #dma-cells = <2>;
> + dma-channels = <8>;
> + dma-requests = <8>;
> + };
> +
> +* DMA client
> +
> +Required properties:
> +
> +dmas and dma-names are required, as described in dma.txt.
> +
> +Example:
> +
> +#include <dt-bindings/dma/nbpfaxi.h>
> +
> +...
> + dmas = <&dma 0 (NBPF_SLAVE_RQ_HIGH | NBPF_SLAVE_RQ_LEVEL)
> + &dma 1 (NBPF_SLAVE_RQ_HIGH | NBPF_SLAVE_RQ_LEVEL)>;
> + dma-names = "rx", "tx";
Pls split the DT bindings to seprate patch. This part needs ack from DT folks
> +static size_t nbpf_xfer_size(struct nbpf_device *nbpf,
> + enum dma_slave_buswidth width, u32 burst)
> +{
> + size_t size;
> +
> + if (!burst)
> + burst = 1;
> +
> + switch (width) {
> + case DMA_SLAVE_BUSWIDTH_8_BYTES:
> + size = 8 * burst;
> + break;
> + case DMA_SLAVE_BUSWIDTH_4_BYTES:
> + size = 4 * burst;
> + break;
> + case DMA_SLAVE_BUSWIDTH_2_BYTES:
> + size = 2 * burst;
why not size = width * burst;
for all these three cases?
> + break;
> + default:
> + pr_warn("%s(): invalid bus width %u\n", __func__, width);
> + case DMA_SLAVE_BUSWIDTH_1_BYTE:
> + size = burst;
this would fit too in above :)
> +static void nbpf_cookie_update(struct nbpf_channel *chan, dma_cookie_t cookie)
> +{
> + dma_cookie_t forgotten = cookie - NBPF_STATUS_KEEP;
> +
> + if (cookie < NBPF_STATUS_KEEP && !chan->cookie_forgotten)
> + /* Not forgetting yet */
> + return;
> +
> + if (forgotten < DMA_MIN_COOKIE)
> + /* Wrapped */
> + forgotten = (DMA_MAX_COOKIE & ~NBPF_STATUS_MASK) + cookie;
> + chan->cookie_forgotten = forgotten;
> +}
why dont use core handler for this? Why do you need to track forgotten?
> +
> +static bool nbpf_cookie_in_cache(struct nbpf_channel *chan,
> + dma_cookie_t cookie, struct dma_tx_state *state)
> +{
> + if (cookie > chan->cookie_forgotten)
> + return true;
> +
> + /*
> + * We still remember the cookie, if it has wrapped beyond INT_MAX:
> + * (1) forgotten is almost INT_MAX
> + * cookie_forgotten >> NBPF_STATUS_SHIFT == DMA_MAX_COOKIE >> NBPF_STATUS_SHIFT
> + * and enquired is small
> + * (2)
> + * cookie & ~NBPF_STATUS_MASK == 0
Again this logic is handled so driver shoudl worry about this
> + */
> +
> + return chan->cookie_forgotten >> NBPF_STATUS_SHIFT ==
> + DMA_MAX_COOKIE >> NBPF_STATUS_SHIFT &&
> + !(cookie < ~NBPF_STATUS_MASK);
> +}
> +
> +static enum dma_status nbpf_tx_status(struct dma_chan *dchan,
> + dma_cookie_t cookie, struct dma_tx_state *state)
> +{
> + struct nbpf_channel *chan = nbpf_to_chan(dchan);
> + enum dma_status status = dma_cookie_status(dchan, cookie, state);
> + dma_cookie_t running = chan->running ? chan->running->async_tx.cookie : -EINVAL;
> +
> + if (chan->paused)
> + status = DMA_PAUSED;
> +
> + /* Note: we cannot return residues for old cookies */
??? If cookie is completed then reside is 0. So how is this comment valid?
> + if (state && cookie == running) {
> + state->residue = nbpf_bytes_left(chan);
> + dev_dbg(dchan->device->dev, "%s(): residue %u\n", __func__,
> + state->residue);
> + }
> +
> + if (status == DMA_IN_PROGRESS || status == DMA_PAUSED)
> + return status;
no residue here?
> +
> + if (nbpf_cookie_in_cache(chan, cookie, state))
> + return nbpf_cookie_cached(chan, cookie, state);
> +
> + return status;
> +}
> +
> +static int nbpf_control(struct dma_chan *dchan, enum dma_ctrl_cmd cmd,
> + unsigned long arg)
> +{
> + struct nbpf_channel *chan = nbpf_to_chan(dchan);
> + struct dma_slave_config *config;
> +
> + dev_dbg(dchan->device->dev, "Entry %s(%d)\n", __func__, cmd);
> +
> + switch (cmd) {
> + case DMA_TERMINATE_ALL:
> + dev_dbg(dchan->device->dev, "Terminating\n");
> + nbpf_chan_halt(chan);
> + nbpf_chan_idle(chan);
> + break;
> + case DMA_SLAVE_CONFIG:
> + if (!arg)
> + return -EINVAL;
> + config = (struct dma_slave_config *)arg;
> +
> + /*
> + * We could check config->slave_id to match chan->terminal here,
> + * but with DT they would be coming from the same source, so
> + * such a check would be superflous
> + */
> +
> + switch (config->direction) {
> + case DMA_MEM_TO_DEV:
> + chan->slave_addr = config->dst_addr;
> + chan->slave_width = nbpf_xfer_size(chan->nbpf,
> + config->dst_addr_width, 1);
> + chan->slave_burst = nbpf_xfer_size(chan->nbpf,
> + config->dst_addr_width,
> + config->dst_maxburst);
> + break;
> + case DMA_DEV_TO_MEM:
> + chan->slave_addr = config->src_addr;
> + chan->slave_width = nbpf_xfer_size(chan->nbpf,
> + config->src_addr_width, 1);
> + chan->slave_burst = nbpf_xfer_size(chan->nbpf,
> + config->src_addr_width,
> + config->src_maxburst);
pls store all as direction makes no sense here and will be removed.
Now based on descriptor direction you need to use one set of above.
> + break;
> + default:
> + return -EINVAL;
> + }
> + break;
> + case DMA_PAUSE:
> + chan->paused = true;
> + nbpf_pause(chan);
> + break;
> + case DMA_RESUME:
> + /* Leave unimplemented until we get a use case. */
why not remove?
> + default:
> + return -ENXIO;
> + }
> +
> + return 0;
> +}
> +
> +static struct dma_async_tx_descriptor *nbpf_prep_sg(struct nbpf_channel *chan,
> + struct scatterlist *src_sg, struct scatterlist *dst_sg,
> + size_t len, enum dma_transfer_direction direction,
> + unsigned long flags)
> +{
> + struct nbpf_link_desc *ldesc;
> + struct scatterlist *mem_sg;
> + struct nbpf_desc *desc;
> + bool inc_src, inc_dst;
> + int i = 0;
> +
> + switch (direction) {
> + case DMA_DEV_TO_MEM:
> + mem_sg = dst_sg;
> + inc_src = false;
> + inc_dst = true;
> + break;
empty line here and similar instance pls
> + case DMA_MEM_TO_DEV:
> + mem_sg = src_sg;
> + inc_src = true;
> + inc_dst = false;
> + break;
> + default:
> + case DMA_MEM_TO_MEM:
> + mem_sg = src_sg;
> + inc_src = true;
> + inc_dst = true;
> + }
> +
> + desc = nbpf_desc_get(chan, len);
> + if (!desc)
> + return NULL;
> +
> + desc->async_tx.flags = flags;
> + desc->async_tx.cookie = -EBUSY;
> + desc->user_wait = false;
> +
> +static int nbpf_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + const struct of_device_id *of_id = of_match_device(nbpf_match, dev);
> + struct device_node *np = dev->of_node;
> + struct nbpf_device *nbpf;
> + struct dma_device *dma_dev;
> + struct resource *iomem, *irq_res;
> + const struct nbpf_config *cfg;
> + int num_channels;
> + int ret, irq, eirq, i;
> + int irqbuf[9] /* maximum 8 channels + error IRQ */;
> + unsigned int irqs = 0;
> +
> + BUILD_BUG_ON(sizeof(struct nbpf_desc_page) > PAGE_SIZE);
> +
> + /* DT only */
> + if (!np || !of_id || !of_id->data)
> + return -ENODEV;
> +
> + cfg = of_id->data;
> + num_channels = cfg->num_channels;
> +
> + nbpf = devm_kzalloc(dev, sizeof(*nbpf) + num_channels *
> + sizeof(nbpf->chan[0]), GFP_KERNEL);
I think we have devm_kcalloc for these things?
--
~Vinod
--
To unsubscribe from this list: send the line "unsubscribe dmaengine" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
