Messages in this thread | | | Date | Wed, 12 Oct 2011 18:34:55 +0200 | From | Nicolas Ferre <> | Subject | Re: [PATCH 1/4] dmaengine: at_hdmac: platform data move to use .id_table |
| |
On 10/11/2011 04:16 PM, Vinod Koul : > On Mon, 2011-10-10 at 18:37 +0200, Nicolas Ferre wrote: >> We remove platform data from DMA controller and move >> to the use of .id_table to distinguish between compatible >> types. The two implementations allow to determine the >> number of channels and the capabilities of the controller. >> >> Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com> >> --- >> drivers/dma/at_hdmac.c | 48 ++++++++++++++++++++++++++++++++++--------- >> drivers/dma/at_hdmac_regs.h | 8 +++++++ >> 2 files changed, 46 insertions(+), 10 deletions(-) >> >> diff --git a/drivers/dma/at_hdmac.c b/drivers/dma/at_hdmac.c >> index fcfa0a8..fef57bc 100644 >> --- a/drivers/dma/at_hdmac.c >> +++ b/drivers/dma/at_hdmac.c >> @@ -1175,6 +1175,18 @@ static void atc_free_chan_resources(struct dma_chan *chan) >> >> /*-- Module Management -----------------------------------------------*/ >> >> +static struct platform_device_id atdma_devtypes[] = { >> + { >> + .name = "at91sam9rl_dma", >> + .driver_data = ATDMA_DEVTYPE_SAM9RL, >> + }, { >> + .name = "at91sam9g45_dma", >> + .driver_data = ATDMA_DEVTYPE_SAM9G45, >> + }, { >> + /* sentinel */ >> + } >> +}; >> + >> /** >> * at_dma_off - disable DMA controller >> * @atdma: the Atmel HDAMC device >> @@ -1193,18 +1205,32 @@ static void at_dma_off(struct at_dma *atdma) >> >> static int __init at_dma_probe(struct platform_device *pdev) >> { >> - struct at_dma_platform_data *pdata; >> struct resource *io; >> struct at_dma *atdma; >> size_t size; >> int irq; >> int err; >> int i; >> + u32 nr_channels; >> + dma_cap_mask_t cap_mask = {}; >> + enum atdma_devtype atdmatype; >> + >> + dma_cap_set(DMA_MEMCPY, cap_mask); >> + >> + /* get DMA parameters from controller type */ >> + atdmatype = platform_get_device_id(pdev)->driver_data; >> >> - /* get DMA Controller parameters from platform */ >> - pdata = pdev->dev.platform_data; >> - if (!pdata || pdata->nr_channels > AT_DMA_MAX_NR_CHANNELS) >> + switch (atdmatype) { >> + case ATDMA_DEVTYPE_SAM9RL: >> + nr_channels = 2; >> + break; >> + case ATDMA_DEVTYPE_SAM9G45: >> + nr_channels = 8; >> + dma_cap_set(DMA_SLAVE, cap_mask); >> + break; >> + default: >> return -EINVAL; >> + } >> >> io = platform_get_resource(pdev, IORESOURCE_MEM, 0); >> if (!io) >> @@ -1215,14 +1241,15 @@ static int __init at_dma_probe(struct platform_device *pdev) >> return irq; >> >> size = sizeof(struct at_dma); >> - size += pdata->nr_channels * sizeof(struct at_dma_chan); >> + size += nr_channels * sizeof(struct at_dma_chan); >> atdma = kzalloc(size, GFP_KERNEL); >> if (!atdma) >> return -ENOMEM; >> >> - /* discover transaction capabilites from the platform data */ >> - atdma->dma_common.cap_mask = pdata->cap_mask; >> - atdma->all_chan_mask = (1 << pdata->nr_channels) - 1; >> + /* discover transaction capabilites */ > capabilities
Yes.
>> + atdma->dma_common.cap_mask = cap_mask; >> + atdma->all_chan_mask = (1 << nr_channels) - 1; >> + atdma->devtype = atdmatype; >> >> size = resource_size(io); >> if (!request_mem_region(io->start, size, pdev->dev.driver->name)) { >> @@ -1268,7 +1295,7 @@ static int __init at_dma_probe(struct platform_device *pdev) >> >> /* initialize channels related values */ >> INIT_LIST_HEAD(&atdma->dma_common.channels); >> - for (i = 0; i < pdata->nr_channels; i++) { >> + for (i = 0; i < nr_channels; i++) { >> struct at_dma_chan *atchan = &atdma->chan[i]; >> >> atchan->chan_common.device = &atdma->dma_common; >> @@ -1313,7 +1340,7 @@ static int __init at_dma_probe(struct platform_device *pdev) >> dev_info(&pdev->dev, "Atmel AHB DMA Controller ( %s%s), %d channels\n", >> dma_has_cap(DMA_MEMCPY, atdma->dma_common.cap_mask) ? "cpy " : "", >> dma_has_cap(DMA_SLAVE, atdma->dma_common.cap_mask) ? "slave " : "", >> - pdata->nr_channels); >> + nr_channels); > indent?
Well, this dev_info statement seems pretty well indented: if I use a tab, then I cross the 80 column rule, and I would like to keep the "?:" operator on one line... So I would like to keep it like this.
>> dma_async_device_register(&atdma->dma_common); >> >> @@ -1495,6 +1522,7 @@ static const struct dev_pm_ops at_dma_dev_pm_ops = { >> static struct platform_driver at_dma_driver = { >> .remove = __exit_p(at_dma_remove), >> .shutdown = at_dma_shutdown, >> + .id_table = atdma_devtypes, >> .driver = { >> .name = "at_hdmac", >> .pm = &at_dma_dev_pm_ops, >> diff --git a/drivers/dma/at_hdmac_regs.h b/drivers/dma/at_hdmac_regs.h >> index aa4c9ae..d7d6737 100644 >> --- a/drivers/dma/at_hdmac_regs.h >> +++ b/drivers/dma/at_hdmac_regs.h >> @@ -248,9 +248,16 @@ static inline struct at_dma_chan *to_at_dma_chan(struct dma_chan *dchan) >> >> /*-- Controller ------------------------------------------------------*/ >> >> +enum atdma_devtype { >> + ATDMA_DEVTYPE_UNDEFINED = 0, >> + ATDMA_DEVTYPE_SAM9RL, /* compatible with SAM9RL DMA controller */ >> + ATDMA_DEVTYPE_SAM9G45, /* compatible with SAM9G45 DMA controller */ >> +}; >> + >> /** >> * struct at_dma - internal representation of an Atmel HDMA Controller >> * @chan_common: common dmaengine dma_device object members >> + * @atdma_devtype: identifier of DMA controller compatibility >> * @ch_regs: memory mapped register base >> * @clk: dma controller clock >> * @save_imr: interrupt mask register that is saved on suspend/resume cycle >> @@ -260,6 +267,7 @@ static inline struct at_dma_chan *to_at_dma_chan(struct dma_chan *dchan) >> */ >> struct at_dma { >> struct dma_device dma_common; >> + enum atdma_devtype devtype; >> void __iomem *regs; >> struct clk *clk; >> u32 save_imr; > Little confused at this, where is the remove of existing platfrom data > per the changelog?
Well, yes, it is the use of platform data that is removed, not the structure it self. I change the log message in my v2 patch series.
thanks, bye, -- Nicolas Ferre
| |