linux-kernel - Re: [PATCH 1/2] dma: Add Freescale qDMA engine driver support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20151005143713.GC13501@vkoul-mobl.iind.intel.com>
Date:	Mon, 5 Oct 2015 15:37:14 +0100
From:	Vinod Koul <vinod.koul@...el.com>
To:	Yuan Yao <yao.yuan@...escale.com>
Cc:	shawn.guo@...aro.org, dan.j.williams@...el.com,
	dmaengine@...r.kernel.org, linux-kernel@...r.kernel.org,
	linux-arm-kernel@...ts.infradead.org, devicetree@...r.kernel.org
Subject: Re: [PATCH 1/2] dma: Add Freescale qDMA engine driver support

On Fri, Sep 11, 2015 at 01:53:52PM +0800, Yuan Yao wrote:

> +Examples:
> +
> +	qdma: qdma@...0000 {
> +		compatible = "fsl,ls-qdma";
> +		reg = <0x0 0x8380000 0x0 0x20000>;
> +		interrupts = <GIC_SPI 170 IRQ_TYPE_LEVEL_HIGH>,
> +				<GIC_SPI 185 IRQ_TYPE_LEVEL_HIGH>;
> +		interrupt-names = "qdma-tx", "qdma-err";
> +		big-endian;
> +		channels = <1>;
> +	};

Binding should be a separate patch

> +FREESCALE qDMA DRIVER
> +M:     Yuan Yao <yao.yuan@...escale.com>
> +L:     linux-arm-kernel@...ts.infradead.org

not dmaengine ML ?


> +config FSL_QDMA
> +	tristate "Freescale qDMA engine support"
> +	select DMA_ENGINE
> +	select DMA_VIRTUAL_CHANNELS

No depends on arch, can it work on x86?

> +static int fsl_qdma_alloc_chan_resources(struct dma_chan *chan)
> +{
> +	struct fsl_qdma_chan *fsl_chan = to_fsl_qdma_chan(chan);
> +
> +	fsl_chan->desc = NULL;
> +	return 0;
> +}

why do you need this it seems to do nothing

> +static struct fsl_qdma_desc *fsl_qdma_alloc_desc(struct fsl_qdma_chan *fsl_chan)
> +{
> +	struct fsl_qdma_desc *fsl_desc;
> +
> +	fsl_desc = kzalloc(sizeof(*fsl_desc), GFP_NOWAIT);
> +

empty line here is not required

> +	if (!fsl_desc)
> +		return NULL;
> +
> +	fsl_desc->qchan = fsl_chan;
> +
> +	return fsl_desc;

why not return fsl_desc->qchan ;


> +	dma_cap_set(DMA_PRIVATE, fsl_qdma->dma_dev.cap_mask);
> +	dma_cap_set(DMA_SLAVE, fsl_qdma->dma_dev.cap_mask);
> +	dma_cap_set(DMA_MEMCPY, fsl_qdma->dma_dev.cap_mask);
> +
> +	fsl_qdma->dma_dev.dev = &pdev->dev;
> +	fsl_qdma->dma_dev.device_alloc_chan_resources
> +		= fsl_qdma_alloc_chan_resources;
> +	fsl_qdma->dma_dev.device_free_chan_resources
> +		= fsl_qdma_free_chan_resources;
> +	fsl_qdma->dma_dev.device_tx_status = fsl_qdma_tx_status;
> +	fsl_qdma->dma_dev.device_prep_dma_memcpy = fsl_qdma_prep_memcpy;
> +	fsl_qdma->dma_dev.device_issue_pending = fsl_qdma_issue_pending;

You claim DMA_SLAVE but no prep_ for that?

> +
> +static int __init fsl_qdma_init(void)
> +{
> +	return platform_driver_register(&fsl_qdma_driver);
> +}
> +subsys_initcall(fsl_qdma_init);
why subsys_init?

-- 
~Vinod
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/