| 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: <20130329203543.GY10326@intel.com>
Date: Sat, 30 Mar 2013 02:05:43 +0530
From: Vinod Koul <vinod.koul@...el.com>
To: Andy Shevchenko <andriy.shevchenko@...ux.jf.intel.com>
Cc: "Rafael J . Wysocki" <rafael.j.wysocki@...el.com>,
Mika Westerberg <mika.westerberg@...ux.jf.intel.com>,
Viresh Kumar <viresh.kumar@...aro.org>,
linux-kernel@...r.kernel.org,
spear-devel <spear-devel@...t.st.com>, linux-acpi@...r.kernel.org
Subject: Re: [PATCH 1/6] dma: acpi-dma: introduce ACPI DMA helpers
On Wed, Mar 27, 2013 at 10:57:57AM +0200, Andy Shevchenko wrote:
> + * @dev: struct device to get DMA request from
> + * @index: index of FixedDMA descriptor for @dev
> + *
> + * Returns pointer to appropriate dma channel on success or NULL on error.
> + */
> +struct dma_chan *acpi_dma_request_slave_chan_by_index(struct device *dev,
> + size_t index)
> +{
So i think this will be called by client driver to request a channel right?
If so how does client find the device pointer for dma controller.
And index is a global one, right? What is it use ?
> + struct acpi_dma_parser_data pdata;
> + struct acpi_dma_spec *dma_spec = &pdata.dma_spec;
> + struct list_head resource_list;
> + struct acpi_device *adev;
> + struct acpi_dma *adma;
> + struct dma_chan *chan;
> +
> + /* Check if the device was enumerated by ACPI */
> + if (!dev || !ACPI_HANDLE(dev))
> + return NULL;
> +
> + if (acpi_bus_get_device(ACPI_HANDLE(dev), &adev))
> + return NULL;
> +
> + memset(&pdata, 0, sizeof(pdata));
> + pdata.index = index;
> +
> + /* Initial values for the request line and channel */
> + dma_spec->chan_id = -1;
> + dma_spec->slave_id = -1;
> +
> + INIT_LIST_HEAD(&resource_list);
> + acpi_dev_get_resources(adev, &resource_list,
> + acpi_dma_parse_fixed_dma, &pdata);
> + acpi_dev_free_resource_list(&resource_list);
> +
> + if (dma_spec->slave_id < 0 || dma_spec->chan_id < 0)
> + return NULL;
> +
> + mutex_lock(&acpi_dma_lock);
> +
> + list_for_each_entry(adma, &acpi_dma_list, dma_controllers) {
> + dma_spec->dev = adma->dev;
> + chan = adma->acpi_dma_xlate(dma_spec, adma);
> + if (chan) {
> + mutex_unlock(&acpi_dma_lock);
> + return chan;
> + }
> + }
> +
> + mutex_unlock(&acpi_dma_lock);
> + return NULL;
in this and error handling you are not doing anything different so why code
duplication?
> +}
> +EXPORT_SYMBOL_GPL(acpi_dma_request_slave_chan_by_index);
> +
> +/**
> + * acpi_dma_request_slave_chan_by_name - Get the DMA slave channel
> + * @dev: struct device to get DMA request from
> + * @name: represents corresponding FixedDMA descriptor for @dev
> + *
> + * In order to support both Device Tree and ACPI in a single driver we
> + * translate the names "tx" and "rx" here based on the most common case where
> + * the first FixedDMA descriptor is TX and second is RX.
> + *
> + * Returns pointer to appropriate dma channel on success or NULL on error.
> + */
> +struct dma_chan *acpi_dma_request_slave_chan_by_name(struct device *dev,
> + const char *name)
> +{
> + size_t index;
> +
> + if (!strcmp(name, "tx"))
> + index = 0;
> + else if (!strcmp(name, "rx"))
> + index = 1;
> + else
> + return NULL;
> +
> + return acpi_dma_request_slave_chan_by_index(dev, index);
are you going to a have a different descriptor for tx and rx? Is index only for
tx = 0 and rx =1 always. How would a controller be represented which has 8
channels, each channel can do DMA in either direction
> +}
> +EXPORT_SYMBOL_GPL(acpi_dma_request_slave_chan_by_name);
> +
> +/**
> + * acpi_dma_simple_xlate - Simple ACPI DMA engine translation helper
> + * @dma_spec: pointer to ACPI DMA specifier
> + * @adma: pointer to ACPI DMA controller data
> + *
> + * A simple translation function for ACPI based devices. Passes &struct
> + * dma_spec to the DMA controller driver provided filter function. Returns
> + * pointer to the channel if found or %NULL otherwise.
> + */
> +struct dma_chan *acpi_dma_simple_xlate(struct acpi_dma_spec *dma_spec,
> + struct acpi_dma *adma)
> +{
> + struct acpi_dma_filter_info *info = adma->data;
what is the purpose of filter here?
> +
> + if (!info || !info->filter_fn)
> + return NULL;
> +
> + return dma_request_channel(info->dma_cap, info->filter_fn, dma_spec);
> +}
> +EXPORT_SYMBOL_GPL(acpi_dma_simple_xlate);
> diff --git a/include/linux/acpi_dma.h b/include/linux/acpi_dma.h
> new file mode 100644
> index 0000000..d09deab
> --- /dev/null
> +++ b/include/linux/acpi_dma.h
> @@ -0,0 +1,116 @@
> +/*
> + * ACPI helpers for DMA request / controller
> + *
> + * Based on of_dma.h
> + *
> + * Copyright (C) 2013, Intel Corporation
> + * Author: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#ifndef __LINUX_ACPI_DMA_H
> +#define __LINUX_ACPI_DMA_H
> +
> +#include <linux/list.h>
> +#include <linux/device.h>
> +#include <linux/dmaengine.h>
> +
> +/**
> + * struct acpi_dma_spec - slave device DMA resources
> + * @chan_id: channel unique id
> + * @slave_id: request line unique id
> + * @dev: struct device of the DMA controller to be used in the filter
> + * function
> + */
> +struct acpi_dma_spec {
> + int chan_id;
> + int slave_id;
> + struct device *dev;
> +};
is this the representation of Fixed DMA, if so why have you omitted transfer widths?
I can see obvious benefits of using that
--
~Vinod
--
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