| 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
| ||
|
Date: Mon, 4 Jan 2016 16:46:51 +0200
From: Andy Shevchenko <andy.shevchenko@...il.com>
To: Wang Hongcheng <annie.wang@....com>
Cc: Vinod Koul <vinod.koul@...el.com>,
Mika Westerberg <mika.westerberg@...ux.intel.com>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
"Rafael J. Wysocki" <rjw@...ysocki.net>,
"linux-acpi@...r.kernel.org" <linux-acpi@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"linux-serial@...r.kernel.org" <linux-serial@...r.kernel.org>,
dmaengine <dmaengine@...r.kernel.org>,
Borislav Petkov <bp@...en8.de>, Huang Rui <ray.huang@....com>,
Wan Zongshun <vincent.wan@....com>, Ken Xue <ken.xue@....com>,
Robin Murphy <robin.murphy@....com>,
Graeme Gregory <gg@...mlogic.co.uk>, Tony Li <tony.li@....com>,
Xiangliang Yu <Xiangliang.Yu@....com>
Subject: Re: [PATCH 5/6] dmaengine: pl330: provide ACPI dmaengine interface
On Mon, Jan 4, 2016 at 7:31 AM, Wang Hongcheng <annie.wang@....com> wrote:
> register acpi_dma controller, so ACPI devices can request pl330 DMA
> channel. Request line info is get from private data.
>
> Signed-off-by: Wang Hongcheng <annie.wang@....com>
> ---
> drivers/acpi/acpi_apd.c | 34 +++++++++++++++++++++++++---------
> drivers/dma/pl330.c | 27 +++++++++++++++++++++++++++
> include/linux/amba/pl330.h | 4 ++++
> 3 files changed, 56 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/acpi/acpi_apd.c b/drivers/acpi/acpi_apd.c
> index 4d71a65..1f16cca 100644
> --- a/drivers/acpi/acpi_apd.c
> +++ b/drivers/acpi/acpi_apd.c
> @@ -38,11 +38,23 @@ struct apd_private_data;
>
> static u8 peri_id[2] = { 0, 1 };
>
> -static struct dma_pl330_platdata amd_pl330 = {
> - .nr_valid_peri = 2,
> - .peri_id = peri_id,
> - .mcbuf_sz = 0,
> - .flags = IRQF_SHARED,
Agreed with bot, those two looks like WIP. Please, unite them in one
clean patch.
> +static struct dma_pl330_platdata amd_pl330[] = {
> + {
> + .nr_valid_peri = 2,
> + .peri_id = peri_id,
> + .mcbuf_sz = 0,
> + .flags = IRQF_SHARED,
> + .base_request_line = 1,
> + .num = 0,
(1)
> + },
> + {
> + .nr_valid_peri = 2,
> + .peri_id = peri_id,
> + .mcbuf_sz = 0,
> + .flags = IRQF_SHARED,
> + .base_request_line = 2,
> + .num = 0,
(2)
(1) and (2) seem use wrong interpretation of the parameters. num is an
amount of request lines in the range:
base_request_line..base_request_line + num - 1.
> + }
> };
> /**
> * struct apd_device_desc - a descriptor for apd device
> @@ -101,6 +113,7 @@ static int acpi_apd_setup_quirks(struct apd_private_data *pdata)
> unsigned int irq[AMBA_NR_IRQS];
> struct clk *clk = ERR_PTR(-ENODEV);
> char amba_devname[100];
> + int devnum;
>
> resource = kzalloc(sizeof(*resource), GFP_KERNEL);
> if (!resource)
> @@ -131,8 +144,11 @@ static int acpi_apd_setup_quirks(struct apd_private_data *pdata)
> if (!amba_dev)
> goto amba_alloc_err;
>
> + devnum = amba_devname[strlen(dev_name(&pdev->dev)) - 1] - '0';
This looks error prone.
> +
> amba_dev->dev.coherent_dma_mask
> = acpi_dma_supported(ACPI_COMPANION(&pdev->dev)) ? DMA_BIT_MASK(64) : 0;
> + amba_dev->dev.platform_data = &amd_pl330[devnum];
> amba_dev->dev.fwnode = acpi_fwnode_handle(ACPI_COMPANION(&pdev->dev));
>
> amba_dev->dev.parent = &pdev->dev;
> @@ -162,10 +178,10 @@ static int acpi_apd_setup_quirks(struct apd_private_data *pdata)
>
> kfree(resource);
>
> - dma_cap_set(DMA_MEMCPY, amd_pl330.cap_mask);
> - dma_cap_set(DMA_SLAVE, amd_pl330.cap_mask);
> - dma_cap_set(DMA_CYCLIC, amd_pl330.cap_mask);
> - dma_cap_set(DMA_PRIVATE, amd_pl330.cap_mask);
> + dma_cap_set(DMA_MEMCPY, amd_pl330[devnum].cap_mask);
> + dma_cap_set(DMA_SLAVE, amd_pl330[devnum].cap_mask);
> + dma_cap_set(DMA_CYCLIC, amd_pl330[devnum].cap_mask);
> + dma_cap_set(DMA_PRIVATE, amd_pl330[devnum].cap_mask);
>
> return 0;
>
> diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c
> index 5e5fb46..e0e0b6e 100644
> --- a/drivers/dma/pl330.c
> +++ b/drivers/dma/pl330.c
> @@ -2079,6 +2079,22 @@ static struct dma_chan *of_dma_pl330_xlate(struct of_phandle_args *dma_spec,
> return dma_get_slave_channel(&pl330->peripherals[chan_id].chan);
> }
>
> +static struct dma_chan *acpi_dma_pl330_xlate(struct acpi_dma_spec *dma_spec,
> + struct acpi_dma *adma)
> +{
> + struct pl330_dmac *pl330 = adma->data;
> + struct dma_pl330_platdata *pdat;
> + unsigned int chan_id;
> +
> + pdat = dev_get_platdata(adma->dev);
This could be part of the definition block above
> +
> + chan_id = dma_spec->chan_id;
> + if (chan_id >= pl330->num_peripherals)
> + return NULL;
> +
> + return dma_get_slave_channel(&pl330->peripherals[chan_id].chan);
> +}
> +
> static int pl330_alloc_chan_resources(struct dma_chan *chan)
> {
> struct dma_pl330_chan *pch = to_pchan(chan);
> @@ -2918,6 +2934,17 @@ pl330_probe(struct amba_device *adev, const struct amba_id *id)
> }
> }
>
> + if (has_acpi_companion(&adev->dev)) {
> + ret = acpi_dma_controller_register(&adev->dev,
> + acpi_dma_pl330_xlate,
> + pdat->base_request_line,
> + pdat->num, pl330);
> + if (ret) {
> + dev_err(&adev->dev,
> + "unable to register DMA to the generic ACPI DMA helpers\n");
> + }
> + }
> +
> adev->dev.dma_parms = &pl330->dma_parms;
>
> /*
> diff --git a/include/linux/amba/pl330.h b/include/linux/amba/pl330.h
> index cdc80f0..1190f69 100644
> --- a/include/linux/amba/pl330.h
> +++ b/include/linux/amba/pl330.h
> @@ -31,6 +31,10 @@ struct dma_pl330_platdata {
> unsigned mcbuf_sz;
> /*flags for irq sharing, default is non-shared*/
> unsigned flags;
> + /*device base request line*/
Spaces and capital letters. Keep one style.
> + unsigned short base_request_line;
> + /*device request line range*/
Ditto.
> + unsigned short num;
> };
>
> extern bool pl330_filter(struct dma_chan *chan, void *param);
> --
> 1.9.1
>
--
With Best Regards,
Andy Shevchenko
--
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