Re: [PATCH v1 1/3] dmaengine: append hsu DMA driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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 linux-serial" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux PPP]     [Linux FS]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Linmodem]     [Device Mapper]     [Linux Kernel for ARM]

  Powered by Linux