RE: [PATCH] intel_mid: Add Mrst & Mfld DMA Drivers

[Koul, Vinod]

> [..]
> > ++ *    midc_do_start           begin a transaction
> > ++ *    @midc: channel
> > ++ *    @first: first descriptor of series
> > ++ *
> > ++ *    Load a transaction into the engine. This must be called with dwc->lock
> > ++ *    held and bh disabled.
> > ++ */
> > +static void midc_dostart(struct intel_mid_dma_chan *midc, struct
> intel_mid_dma_desc
> 
> (nit) The function name does not match the kernel doc, and I don't
> know if those extra "+" characters will throw off the kernel-doc
> scripts.  They also show up on midc_put_desc, but not
> midc_scan_descriptors.
Okay, will remove extra "+" char

> > +       /*tx is complete*/
> > +       list_for_each_entry_safe(desc, _desc, &midc->active_list, desc_node) {
> > +               if (desc == NULL)
> > +                       continue;
> 
> How do we ever get desc == NULL at this point?
Yes we won't, will remove

> [..]
> > +#define _dma_printk(level, format, arg...)  \
> > +       printk(level "LNW_DMA: %s %d " format, __func__, __LINE__, ## arg)
> > +
> > +#ifdef CONFIG_LNW_DMA_DEBUG
> > +#define dma_dbg(format, arg...) _dma_printk(KERN_DEBUG, "DBG " format , ## arg)
> > +#else
> > +#define dma_dbg(format, arg...) do {} while (0);
> > +#endif
> 
> This makes us lose compile coverage of the debug statements so they
> bitrot until someone needs to debug a problem.
This was just for debug purposes, will be removed


 
> Any advantage to having a default callback in the slave configuration?
>  Why not let the client specify the callback on each operation, is it
> a callback that the client does not know about?
txd callback is enough, no need to keep here, will be removed

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