On Fri, Sep 11, 2015 at 03:14:26PM +0100, Peter Griffin wrote: > +static char *fdma_clk_name[CLK_MAX_NUM] = { > + [CLK_SLIM] = "fdma_slim", > + [CLK_HI] = "fdma_hi", > + [CLK_LOW] = "fdma_low", > + [CLK_IC] = "fdma_ic", > +}; why do we want to have this sort of data in kernel, we should get the clocks from DT here > +static int config_reqctrl(struct st_fdma_chan *fchan) > +{ > + u32 maxburst = 0, addr = 0; > + enum dma_slave_buswidth width; > + int ch_id = fchan->vchan.chan.chan_id; > + struct st_fdma_dev *fdev = fchan->fdev; > + > + if (fchan->scfg.direction == DMA_DEV_TO_MEM) { > + fchan->cfg.req_ctrl &= ~REQ_CTRL_WNR; > + maxburst = fchan->scfg.src_maxburst; > + width = fchan->scfg.src_addr_width; > + addr = fchan->scfg.src_addr; > + } else if (fchan->scfg.direction == DMA_MEM_TO_DEV) { > + fchan->cfg.req_ctrl |= REQ_CTRL_WNR; > + maxburst = fchan->scfg.dst_maxburst; > + width = fchan->scfg.dst_addr_width; > + addr = fchan->scfg.dst_addr; This raises red flags, why do we have direction in channel, and where is this set. Direction belong to a descriptor and you are supposed to use direction from prep_ calls! > + if (!is_slave_direction(direction)) { > + dev_err(fchan->fdev->dev, "bad direction?\n"); > + return NULL; > + } > + > + if (!config_reqctrl(fchan)) { > + dev_err(fchan->fdev->dev, "bad width or direction\n"); would make sense to print proper error in called fn > +static enum dma_status st_fdma_tx_status(struct dma_chan *chan, > + dma_cookie_t cookie, > + struct dma_tx_state *txstate) > +{ > + struct st_fdma_chan *fchan = to_st_fdma_chan(chan); > + struct virt_dma_desc *vd; > + enum dma_status ret; > + unsigned long flags; > + > + ret = dma_cookie_status(chan, cookie, txstate); > + if (ret == DMA_COMPLETE) > + return ret; > + > + if (!txstate) > + return fchan->status; why channel status and not cookie status ? > +static const struct st_fdma_ram fdma_mpe31_mem[] = { > + { .name = "dmem", .offset = 0x10000, .size = 0x3000 }, > + { .name = "imem", .offset = 0x18000, .size = 0x8000 }, > +}; why aren't these coming from DT ? > +MODULE_LICENSE("GPL v2"); > +MODULE_DESCRIPTION("STMicroelectronics FDMA engine driver"); > +MODULE_AUTHOR("Ludovic.barre <Ludovic.barre@xxxxxx>"); No MODULE_ALIAS ? -- ~Vinod -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html