Re: [PATCH 6/6] intel_mid_dma: change the slave interface

From: Dan Williams
Date: Fri Oct 08 2010 - 13:28:00 EST


On Mon, Oct 4, 2010 at 3:38 AM, Vinod Koul <vinod.koul@xxxxxxxxx> wrote:
> In 2.6.36 kernel, dma slave control command was introduced,
> this patch changes the intel-mid-dma driver to this
> new kernel slave interface
>
> Signed-off-by: Vinod Koul <vinod.koul@xxxxxxxxx>
> ---
[..]
> +static int dma_slave_control(struct dma_chan *chan, unsigned long arg)
> +{
> +       struct intel_mid_dma_chan       *midc = to_intel_mid_dma_chan(chan);
> +       struct dma_slave_config  *slave = (struct dma_slave_config *)arg;
> +       struct intel_mid_dma_slave *mid_slave;
> +
> +       BUG_ON(!midc);
> +       BUG_ON(!slave);
> +       pr_debug("MDMA: slave control called\n");
> +
> +       mid_slave = to_intel_mid_dma_slave(slave);

I know this was the initial recommendation to support custom
extensions to dma_slave_config, but this isn't safe. How does this
routine know that that 'slave' is embedded in an intel_mid_dma_slave
structure. I applied it as is, but I think we need a better approach.

Maybe a per device alloc_dma_slave_config() such that ->dma_control()
can assume the driver allocated the dma_slave_config. As it stands
nothing in the calling convention guarantees type safety.

--
Dan
--
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/