linux-kernel - Re: [PATCH 5/9] dmaengine: pl330: provide ACPI dmaengine interface

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
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:	Sun, 13 Dec 2015 04:21:57 +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>,
	Joerg Roedel <joro@...tes.org>,
	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>,
	iommu@...ts.linux-foundation.org, Borislav Petkov <bp@...en8.de>,
	Huang Rui <ray.huang@....com>,
	Wan Zongshun <vincent.wan@....com>, Ken Xue <ken.xue@....com>,
	Tony Li <tony.li@....com>
Subject: Re: [PATCH 5/9] dmaengine: pl330: provide ACPI dmaengine interface

On Fri, Dec 4, 2015 at 5:24 AM, Wang Hongcheng <annie.wang@....com> wrote:
> register acpi_dma controller, so ACPI devices can request pl330 DMA
> channel.
> A filter is added in private data for Carrizo specific hardware
> design
>
> Signed-off-by: Wang Hongcheng <annie.wang@....com>
> ---
>  drivers/acpi/acpi_apd.c    | 12 ++++++++++++
>  drivers/dma/pl330.c        | 38 ++++++++++++++++++++++++++++++++++++++
>  include/linux/amba/pl330.h |  1 +
>  3 files changed, 51 insertions(+)
>
> diff --git a/drivers/acpi/acpi_apd.c b/drivers/acpi/acpi_apd.c
> index 7a582f5..906a20f 100644
> --- a/drivers/acpi/acpi_apd.c
> +++ b/drivers/acpi/acpi_apd.c
> @@ -38,12 +38,15 @@ struct apd_private_data;
>
>  static u8 peri_id[2] = { 0, 1 };
>
> +static int apd_acpi_xlate_filter(int slave_id, struct device *dev);
> +
>  static struct dma_pl330_platdata amd_pl330 = {
>         .nr_valid_peri = 2,
>         .peri_id = peri_id,
>         .has_no_cap_mask = true,
>         .mcbuf_sz = 0,
>         .flags = IRQF_SHARED,
> +       .acpi_xlate_filter = apd_acpi_xlate_filter,
>  };
>
>  /**
> @@ -68,6 +71,15 @@ struct apd_private_data {
>         const struct apd_device_desc *dev_desc;
>  };
>
> +int apd_acpi_xlate_filter(int slave_id, struct device *dev)
> +{
> +       if (((slave_id == 1) && (!strcmp(dev_name(dev), "AMD0020:00DMA")))
> +           || ((slave_id == 2) && (!strcmp(dev_name(dev), "AMD0020:01DMA"))))
> +               return 0;

First of all, too many parens. No need to have them here: (!strcmp()),
for example.

Second, looks like you actually compare slave_id with device _UID, am
I right? In that case maybe better to check device's _HID, _UID and
eventually slave_id.

> +
> +       return 1;
> +}
> +
>  #ifdef CONFIG_X86_AMD_PLATFORM_DEVICE
>  #define APD_ADDR(desc) ((unsigned long)&desc)
>
> diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c
> index 8300969..9d7af0d 100644
> --- a/drivers/dma/pl330.c
> +++ b/drivers/dma/pl330.c
> @@ -2079,6 +2079,35 @@ 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;
> +       int ret;
> +
> +       if (!dma_spec)
> +               return NULL;

AFAIR dma_spec is guaranteed not to be NULL.

> +
> +       if (!adma)
> +               return NULL;

Ditto.

> +
> +       pdat = dev_get_platdata(adma->dev);
> +
> +       chan_id = dma_spec->chan_id;
> +       if (chan_id >= pl330->num_peripherals)
> +               return NULL;
> +
> +       if (pdat->acpi_xlate_filter) {
> +               ret = pdat->acpi_xlate_filter(dma_spec->slave_id, adma->dev);
> +               if (ret)
> +                       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 +2947,15 @@ pl330_probe(struct amba_device *adev, const struct amba_id *id)
>                 }
>         }
>
> +       if (ACPI_HANDLE(&adev->dev)) {

I suspect where did you get this, but it's already updated. You better use
has_acpi_companion() instead.

> +               ret = acpi_dma_controller_register(&adev->dev,
> +                                                  acpi_dma_pl330_xlate, pl330);

So, someone has either to provide CSRT (which is mostly foreign to
ACPI tables), or to provide those resources somehow else (request line
base and number). In the latter case you would need to add the hook
into acpi_dma_controller_register().

> +               if (ret) {
> +                       dev_err(&adev->dev,
> +                               "unable to register DMA to the generic ACPI DMA helpers\n");
> +               }
> +       }
> +
>         adev->dev.dma_parms = &pl330->dma_parms;
>
>         /*

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