On Mon, Feb 23, 2015 at 04:24:42PM +0200, Andy Shevchenko wrote:
> + * DMA channel allocation:
> + * 1. Even number chans are used for DMA Read (UART TX), odd chans for DMA
> + * Write (UART RX).
> + * 2. 0/1 channel are assigned to port 0, 2/3 chan to port 1, 4/5 chan to
> + * port 3, and so on.
> + */
is this hard coded in the hardware? Also which platforms does this HSU
support?
> +static void hsu_dma_chan_start(struct hsu_dma_chan *hsuc)
> +{
> + struct dma_slave_config *config = &hsuc->config;
> + struct hsu_dma_desc *desc = hsuc->desc;
> + u32 bsr, mtsr;
> + u32 dcr = HSU_CH_DCR_CHSOE | HSU_CH_DCR_CHEI;
> + unsigned int i, count;
> +
> + if (hsuc->direction == DMA_MEM_TO_DEV) {
> + bsr = config->dst_maxburst;
> + mtsr = config->dst_addr_width;
> + } else if (hsuc->direction == DMA_DEV_TO_MEM) {
> + bsr = config->src_maxburst;
> + mtsr = config->src_addr_width;
> + } else {
> + /* Not supported direction */
> + return;
> + }
shouldn't this check be performed when dma_slave_config is programmed?
> +irqreturn_t hsu_dma_irq(struct hsu_dma_chip *chip, unsigned short nr)
> +{
> + struct hsu_dma_chan *hsuc;
> + struct hsu_dma_desc *desc;
> + unsigned long flags;
> + u32 sr;
> +
> + /* Sanity check */
> + if (nr >= chip->pdata->nr_channels)
> + return IRQ_NONE;
> +
> + hsuc = &chip->hsu->chan[nr];
> +
> + /*
> + * No matter what situation, need read clear the IRQ status
> + * There is a bug, see Errata 5, HSD 2900918
> + */
> + sr = hsu_dma_chan_get_sr(hsuc);
> + if (!sr)
> + return IRQ_NONE;
> +
> + /* Timeout IRQ, need wait some time, see Errata 2 */
> + if (hsuc->direction == DMA_DEV_TO_MEM && (sr & HSU_CH_SR_DESCTO_ANY))
> + udelay(2);
these errata in hsu core part, isn't this applicable for implementation of HSU
rather than everywhere?
> +
> + sr &= ~HSU_CH_SR_DESCTO_ANY;
> + if (!sr)
> + return IRQ_HANDLED;
> +
> + spin_lock_irqsave(&hsuc->vchan.lock, flags);
> + desc = hsuc->desc;
> + if (desc) {
> + if (sr & HSU_CH_SR_CHE) {
> + desc->status = DMA_ERROR;
> + } else if (desc->active < desc->nents) {
> + hsu_dma_start_channel(hsuc);
> + } else {
> + vchan_cookie_complete(&desc->vdesc);
> + desc->status = DMA_COMPLETE;
> + hsu_dma_start_transfer(hsuc);
> + }
> + }
> + spin_unlock_irqrestore(&hsuc->vchan.lock, flags);
> +
> + return IRQ_HANDLED;
> +}
> +EXPORT_SYMBOL_GPL(hsu_dma_irq);
> +
> +static struct hsu_dma_desc *hsu_dma_alloc_desc(unsigned int nents)
> +{
> + struct hsu_dma_desc *desc;
> +
> + desc = kzalloc(sizeof(*desc), GFP_ATOMIC);
GFP_NOWAIT pls
> + if (!desc)
> + return NULL;
> +
> + desc->sg = kcalloc(nents, sizeof(*desc->sg), GFP_ATOMIC);
here too
> + if (!desc->sg) {
> + kfree(desc);
> + return NULL;
> + }
> +
> + return desc;
> +}
> +
> +static void hsu_dma_desc_free(struct virt_dma_desc *vdesc)
> +{
> + struct hsu_dma_desc *desc = to_hsu_dma_desc(vdesc);
> +
> + kfree(desc->sg);
> + kfree(desc);
> +}
> +
> +static struct dma_async_tx_descriptor *hsu_dma_prep_slave_sg(
> + struct dma_chan *chan, struct scatterlist *sgl,
> + unsigned int sg_len, enum dma_transfer_direction direction,
> + unsigned long flags, void *context)
> +{
> + struct hsu_dma_chan *hsuc = to_hsu_dma_chan(chan);
> + struct hsu_dma_desc *desc;
> + struct scatterlist *sg;
> + unsigned int i;
> +
> + desc = hsu_dma_alloc_desc(sg_len);
> + if (!desc)
> + return NULL;
> +
> + for_each_sg(sgl, sg, sg_len, i) {
> + desc->sg[i].addr = sg_dma_address(sg);
> + desc->sg[i].len = sg_dma_len(sg);
> + }
> +
> + desc->nents = sg_len;
> + desc->direction = direction;
> + desc->active = 0;
why is this set here
> +
> +static int hsu_dma_alloc_chan_resources(struct dma_chan *chan)
> +{
> + return 0;
> +}
this is not required
> +#endif /* __DMA_HSU_H__ */
> diff --git a/drivers/dma/hsu/pci.c b/drivers/dma/hsu/pci.c
> new file mode 100644
> index 0000000..563b468
> --- /dev/null
> +++ b/drivers/dma/hsu/pci.c
> @@ -0,0 +1,123 @@
> +/*
> + * PCI driver for the High Speed UART DMA
ideally this should have been a patch on its own
> +struct hsu_dma_chip {
> + struct device *dev;
> + int irq;
> + void __iomem *regs;
> + unsigned int length;
> + unsigned int offset;
> + struct hsu_dma *hsu;
> + struct hsu_dma_platform_data *pdata;
> +};
> +
> +/* Export to the internal users */
> +irqreturn_t hsu_dma_irq(struct hsu_dma_chip *chip, unsigned short nr);
> +
> +/* Export to the platform drivers */
> +int hsu_dma_probe(struct hsu_dma_chip *chip);
> +int hsu_dma_remove(struct hsu_dma_chip *chip);
and those users are pci driver and some other dma driver in future right, so
why should this be exposed to other susbsytems? Why cant this be in
driver/dma ?
> diff --git a/include/linux/platform_data/dma-hsu.h b/include/linux/platform_data/dma-hsu.h
> new file mode 100644
> index 0000000..8a1f6a4
> --- /dev/null
> +++ b/include/linux/platform_data/dma-hsu.h
> @@ -0,0 +1,25 @@
> +/*
> + * Driver for the High Speed UART DMA
> + *
> + * Copyright (C) 2015 Intel Corporation
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#ifndef _PLATFORM_DATA_DMA_HSU_H
> +#define _PLATFORM_DATA_DMA_HSU_H
> +
> +#include <linux/device.h>
> +
> +struct hsu_dma_slave {
> + struct device *dma_dev;
> + int chan_id;
> +};
> +
> +struct hsu_dma_platform_data {
> + unsigned short nr_channels;
> +};
> +
> +#endif /* _PLATFORM_DATA_DMA_HSU_H */
> --
> 2.1.4
>
I didnt see a platform device so why do we need this here?
Thanks
--
~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
