On Mon, Mar 13, 2017 at 04:06:39PM +0100, M'boumba Cedric Madianga wrote:
> This patch adds the driver for the STM32 MDMA controller.
Again pls do describe the controller
> +#include <linux/clk.h>
> +#include <linux/delay.h>
> +#include <linux/dmaengine.h>
> +#include <linux/dma-mapping.h>
> +#include <linux/dmapool.h>
> +#include <linux/err.h>
> +#include <linux/init.h>
> +#include <linux/iopoll.h>
> +#include <linux/jiffies.h>
> +#include <linux/list.h>
> +#include <linux/log2.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <linux/of_dma.h>
> +#include <linux/platform_device.h>
> +#include <linux/reset.h>
> +#include <linux/sched.h>
why do you need sched.h, i am sure many of these may not be required, pls
check
> +static int stm32_mdma_get_width(struct stm32_mdma_chan *chan,
> + enum dma_slave_buswidth width)
> +{
> + switch (width) {
> + case DMA_SLAVE_BUSWIDTH_1_BYTE:
> + return STM32_MDMA_BYTE;
> + case DMA_SLAVE_BUSWIDTH_2_BYTES:
> + return STM32_MDMA_HALF_WORD;
> + case DMA_SLAVE_BUSWIDTH_4_BYTES:
> + return STM32_MDMA_WORD;
> + case DMA_SLAVE_BUSWIDTH_8_BYTES:
> + return STM32_MDMA_DOUBLE_WORD;
IIUC we can do this with ffs()
> + default:
> + dev_err(chan2dev(chan), "Dma bus width not supported\n");
> + return -EINVAL;
> + }
> +}
> +
> +static enum dma_slave_buswidth stm32_mdma_get_max_width(u32 buf_len, u32
> tlen)
> +{
> + enum dma_slave_buswidth max_width = DMA_SLAVE_BUSWIDTH_8_BYTES;
> +
> + while ((buf_len <= max_width || buf_len % max_width ||
> + tlen < max_width) && max_width > DMA_SLAVE_BUSWIDTH_1_BYTE)
> + max_width = max_width >> 1;
1. this is hard to read
2. sound like this can be optimized :)
> +
> + return max_width;
> +}
> +
> +static u32 stm32_mdma_get_best_burst(u32 buf_len, u32 tlen, u32 max_burst,
> + enum dma_slave_buswidth width)
> +{
> + u32 best_burst = max_burst;
> + u32 burst_len = best_burst * width;
> +
> + if (buf_len % tlen)
> + return 0;
> +
> + while ((tlen < burst_len && best_burst > 1) ||
> + (burst_len > 0 && tlen % burst_len)) {
> + best_burst = best_burst >> 1;
> + burst_len = best_burst * width;
same thing here too
> +
> + return (best_burst > 1) ? best_burst : 0;
> +}
> +
> +static int stm32_mdma_disable_chan(struct stm32_mdma_chan *chan)
> +{
> + struct stm32_mdma_device *dmadev = stm32_mdma_get_dev(chan);
> + u32 ccr, cisr, id, reg;
> + int ret;
> +
> + id = chan->id;
> + reg = STM32_MDMA_CCR(id);
> +
> + /* Disable interrupts */
> + stm32_mdma_clr_bits(dmadev, reg, STM32_MDMA_CCR_IRQ_MASK);
> +
> + ccr = stm32_mdma_read(dmadev, reg);
> + if (ccr & STM32_MDMA_CCR_EN) {
> + stm32_mdma_clr_bits(dmadev, reg, STM32_MDMA_CCR_EN);
> +
> + /* Ensure that any ongoing transfer has been completed */
> + ret = readl_relaxed_poll_timeout_atomic(
why not simple readl
> +static void stm32_mdma_set_dst_bus(struct stm32_mdma_device *dmadev, u32
> *ctbr,
> + u32 dst_addr)
> +{
> + u32 mask;
> + int i;
> +
> + /* Check if memory device is on AHB or AXI */
> + *ctbr &= ~STM32_MDMA_CTBR_DBUS;
> + mask = dst_addr & 0xF0000000;
> + for (i = 0; i < dmadev->nr_ahb_addr_masks; i++) {
> + if (mask == dmadev->ahb_addr_masks[i]) {
> + *ctbr |= STM32_MDMA_CTBR_DBUS;
> + break;
> + }
> + }
> +}
> +
> +static void stm32_mdma_set_src_bus(struct stm32_mdma_device *dmadev, u32
> *ctbr,
> + u32 src_addr)
> +{
> + u32 mask;
> + int i;
> +
> + /* Check if memory device is on AHB or AXI */
> + *ctbr &= ~STM32_MDMA_CTBR_SBUS;
> + mask = src_addr & 0xF0000000;
> + for (i = 0; i < dmadev->nr_ahb_addr_masks; i++) {
> + if (mask == dmadev->ahb_addr_masks[i]) {
> + *ctbr |= STM32_MDMA_CTBR_SBUS;
> + break;
> + }
> + }
these too look awfully same..
> +static int __init stm32_mdma_init(void)
> +{
> + return platform_driver_probe(&stm32_mdma_driver, stm32_mdma_probe);
> +}
> +
> +subsys_initcall(stm32_mdma_init);
why subsys?
> --
> 1.9.1
>
--
~Vinod
