| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20111024093414.GF8708@ponder.secretlab.ca>
Date: Mon, 24 Oct 2011 11:34:14 +0200
From: Grant Likely <grant.likely@...retlab.ca>
To: Nicolas Ferre <nicolas.ferre@...el.com>
Cc: vinod.koul@...el.com, linux-arm-kernel@...ts.infradead.org,
linux-kernel@...r.kernel.org, robherring2@...il.com,
devicetree-discuss@...ts.ozlabs.org
Subject: Re: [PATCH v4 1/2] dmaengine: at_hdmac: platform data move to use
.id_table
On Mon, Oct 17, 2011 at 02:56:40PM +0200, Nicolas Ferre wrote:
> We remove the use of platform data from DMA controller driver.
> We now 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@...el.com>
> Acked-by: Grant Likely <grant.likely@...retlab.ca>
> ---
> 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..d1869c5 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;
Instead of this song and dance, why not make a configuration structure
and embed that into the platform_device_id table? Like this:
struct at_dma_platform_data at91sam9rl_config {
.nr_channels = 2;
.cap_mask = 0;
};
struct at_dma_platform_data at91samg45_config {
.nr_channels = 8;
.cap_mask = DMA_SLAVE;
};
static struct platform_device_id atdma_devtypes[] = {
{
.name = "at91sam9rl_dma",
.driver_data = (unsigned long) at91sam9rl_config,
/*
* Yes, I know, ugly cast; but one case will be needed
* regardless when the of_device_id table is added.
* It's due to the platform_device_id not using a
* void*
*/
}, {
.name = "at91sam9g45_dma",
.driver_data = (unsigned long) at91sam9g45_config,
}, {
/* sentinel */
}
};
And then the enum can be eliminated entirely.
> + }
>
> 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 capabilities */
> + 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);
>
> 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;
> --
> 1.7.5.4
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists