Thread (23 messages) 23 messages, 6 authors, 2016-01-14

Re: [PATCH 5/6] dmaengine: pl330: provide ACPI dmaengine interface

From: Andy Shevchenko <hidden>
Date: 2016-01-04 14:47:01
Also in: linux-acpi, lkml

On Mon, Jan 4, 2016 at 7:31 AM, Wang Hongcheng [off-list ref] wrote:
quoted hunk ↗ jump to hunk
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 <redacted>
---
 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.
quoted hunk ↗ jump to hunk
+       }
 };
 /**
  * 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.
quoted hunk ↗ jump to hunk
+
        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
quoted hunk ↗ jump to hunk
+
+       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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help