RE: [RFC PATCH 03/13] ARM: edma: add DT and runtime PM support forAM335x

From: Hebbar, Gururaja
Date: Fri Sep 21 2012 - 04:54:32 EST

Next message: Wei Ni: "[PATCH v2 2/5] ARM: dt: t20 seaboard: turn on the power for wlan"
Previous message: Hebbar, Gururaja: "RE: [RFC PATCH 05/13] dma: Add TI EDMA device tree binding"
In reply to: Matt Porter: "[RFC PATCH 03/13] ARM: edma: add DT and runtime PM support for AM335x"
Next in thread: Matt Porter: "[RFC PATCH 10/13] spi: omap2-mcspi: dma_request_slave_channel() support for DT platforms"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

On Thu, Sep 20, 2012 at 20:13:36, Porter, Matt wrote:
> Adds support for parsing the TI EDMA DT data into the required
> EDMA private API platform data.
>
> Calls runtime PM API only in the DT case in order to unidle the
> associated hwmods on AM335x.
>
> Signed-off-by: Matt Porter <mporter@xxxxxx>
> ---
> arch/arm/common/edma.c | 252 ++++++++++++++++++++++++++++++++++++--
> arch/arm/include/asm/mach/edma.h | 8 +-
> 2 files changed, 244 insertions(+), 16 deletions(-)

The binding documentation should be updated along with the driver
change that does introduce the binding. You could just merged patch #4
and #5.

>
> diff --git a/arch/arm/common/edma.c b/arch/arm/common/edma.c
> index 001d268..f337f81 100644
> --- a/arch/arm/common/edma.c
> +++ b/arch/arm/common/edma.c
> @@ -24,6 +24,13 @@
> #include <linux/platform_device.h>
> #include <linux/io.h>
> #include <linux/slab.h>
> +#include <linux/edma.h>
> +#include <linux/err.h>
> +#include <linux/of_address.h>
> +#include <linux/of_device.h>
> +#include <linux/of_dma.h>
> +#include <linux/of_irq.h>
> +#include <linux/pm_runtime.h>
>
> #include <asm/mach/edma.h>
>
> @@ -1366,30 +1373,236 @@ void edma_clear_event(unsigned channel)
> EXPORT_SYMBOL(edma_clear_event);
>
> /*-----------------------------------------------------------------------*/
> +static int edma_of_read_u32_to_s8_array(const struct device_node *np,
> + const char *propname, s8 *out_values,
> + size_t sz)
> +{
> + struct property *prop = of_find_property(np, propname, NULL);
> + const __be32 *val;
> +
> + if (!prop)
> + return -EINVAL;
> + if (!prop->value)
> + return -ENODATA;
> + if ((sz * sizeof(u32)) > prop->length)
> + return -EOVERFLOW;
> +
> + val = prop->value;
> +
> + while (sz--)
> + *out_values++ = (s8)(be32_to_cpup(val++) & 0xff);
> +
> + /* Terminate it */
> + *out_values++ = -1;
> + *out_values++ = -1;
> +
> + return 0;
> +}
> +
> +static int edma_of_read_u32_to_s16_array(const struct device_node *np,
> + const char *propname, s16 *out_values,
> + size_t sz)
> +{
> + struct property *prop = of_find_property(np, propname, NULL);
> + const __be32 *val;
> +
> + if (!prop)
> + return -EINVAL;
> + if (!prop->value)
> + return -ENODATA;
> + if ((sz * sizeof(u32)) > prop->length)
> + return -EOVERFLOW;
> +
> + val = prop->value;
> +
> + while (sz--)
> + *out_values++ = (s16)(be32_to_cpup(val++) & 0xffff);
> +
> + /* Terminate it */
> + *out_values++ = -1;
> + *out_values++ = -1;
> +
> + return 0;
> +}
> +
> +static int edma_of_parse_dt(struct device *dev,
> + struct device_node *node,
> + struct edma_soc_info *pdata)
> +{
> + int ret = 0;
> + u32 value;
> + struct property *prop;
> + size_t sz;
> + struct edma_rsv_info *rsv_info;
> + s16 (*rsv_chans)[2], (*rsv_slots)[2];
> + s8 (*queue_tc_map)[2], (*queue_priority_map)[2];
> +
> + ret = of_property_read_u32(node, "dma-channels", &value);
> + if (ret < 0)
> + return ret;
> + pdata->n_channel = value;
> +
> + ret = of_property_read_u32(node, "ti,edma-regions", &value);
> + if (ret < 0)
> + return ret;
> + pdata->n_region = value;
> +
> + ret = of_property_read_u32(node, "ti,edma-slots", &value);
> + if (ret < 0)
> + return ret;
> + pdata->n_slot = value;
> +
> + pdata->n_cc = 1;
> + /* This is unused */
> + pdata->n_tc = 3;
> +
> + rsv_info =
> + devm_kzalloc(dev, sizeof(struct edma_rsv_info), GFP_KERNEL);
> + if (!rsv_info)
> + return -ENOMEM;
> + pdata->rsv = rsv_info;
> +
> + /* Build the reserved channel/slots arrays */
> + prop = of_find_property(node, "ti,edma-reserved-channels", &sz);
> + if (!prop)
> + return -EINVAL;
> +
> + rsv_chans =
> + devm_kzalloc(dev, sz/sizeof(s16) + 2*sizeof(s16), GFP_KERNEL);
> + if (!rsv_chans)
> + return -ENOMEM;
> + pdata->rsv->rsv_chans = rsv_chans;
> +
> + ret = edma_of_read_u32_to_s16_array(node, "ti,edma-reserved-channels",
> + (s16 *)rsv_chans, sz/sizeof(u32));
> + if (ret < 0)
> + return ret;
> +
> + prop = of_find_property(node, "ti,edma-reserved-slots", &sz);
> + if (!prop)
> + return -EINVAL;
> +

Binding Documentation mentions edma-reserved-[channels/slots] as optional.
But here the code returns as error if they are not found. Kindly reconfirm

>From patch-set 5/13

+Optional properties:
+- ti,edma-reserved-channels: List of reserved channel regions
+- ti,edma-reserved-slots: List of reserved slot regions

> + rsv_slots = devm_kzalloc(dev,
> + sz/sizeof(s16) + 2*sizeof(s16),
> + GFP_KERNEL);
> + if (!rsv_slots)
> + return -ENOMEM;
> + pdata->rsv->rsv_slots = rsv_slots;
> +
> + ret = edma_of_read_u32_to_s16_array(node,
> + "ti,edma-reserved-slots",
> + (s16 *)rsv_slots,
> + sz/sizeof(u32));
> + if (ret < 0)
> + return ret;
> +
> + prop = of_find_property(node, "ti,edma-queue-tc-map", &sz);
> + if (!prop)
> + return -EINVAL;
> +
> + queue_tc_map = devm_kzalloc(dev,
> + sz/sizeof(s8) + 2*sizeof(s8),
> + GFP_KERNEL);
> + if (!rsv_slots)
> + return -ENOMEM;
> + pdata->queue_tc_mapping = queue_tc_map;
> +
> + ret = edma_of_read_u32_to_s8_array(node,
> + "ti,edma-queue-tc-map",
> + (s8 *)queue_tc_map,
> + sz/sizeof(u32));
> + if (ret < 0)
> + return ret;
> +
> + prop = of_find_property(node, "ti,edma-queue-priority-map", &sz);
> + if (!prop)
> + return -EINVAL;
> +
> + queue_priority_map = devm_kzalloc(dev,
> + sz/sizeof(s8) + 2*sizeof(s8),
> + GFP_KERNEL);
> + if (!rsv_slots)
> + return -ENOMEM;
> + pdata->queue_priority_mapping = queue_priority_map;
> +
> + ret = edma_of_read_u32_to_s8_array(node,
> + "ti,edma-queue-tc-map",
> + (s8 *)queue_priority_map,
> + sz/sizeof(u32));
> + if (ret < 0)
> + return ret;
> +
> + ret = of_property_read_u32(node, "ti,edma-default-queue", &value);
> + if (ret < 0)
> + return ret;
> + pdata->default_queue = value;
> +
> + return ret;
> +}
> +
> +static struct of_dma_filter_info edma_filter_info = {
> + .filter_fn = edma_filter_fn,
> +};
>
> static int __init edma_probe(struct platform_device *pdev)
> {
> struct edma_soc_info **info = pdev->dev.platform_data;
> - const s8 (*queue_priority_mapping)[2];
> - const s8 (*queue_tc_mapping)[2];
> + s8 (*queue_priority_mapping)[2];
> + s8 (*queue_tc_mapping)[2];
> int i, j, off, ln, found = 0;
> int status = -1;
> - const s16 (*rsv_chans)[2];
> - const s16 (*rsv_slots)[2];
> + s16 (*rsv_chans)[2];
> + s16 (*rsv_slots)[2];

What is the significance of the number "2" in all above members?

> int irq[EDMA_MAX_CC] = {0, 0};
> int err_irq[EDMA_MAX_CC] = {0, 0};
> struct resource *r[EDMA_MAX_CC] = {NULL};
> + struct resource res[EDMA_MAX_CC];
> resource_size_t len[EDMA_MAX_CC];
> char res_name[10];
> char irq_name[10];
> + struct device_node *node = pdev->dev.of_node;
> + struct device *dev = &pdev->dev;
> + struct edma_soc_info *pdata;
> +
> + if (node) {
> + int ret;
> + pdata = devm_kzalloc(dev,
> + sizeof(struct edma_soc_info),
> + GFP_KERNEL);
> + edma_of_parse_dt(dev, node, pdata);
> + info = &pdata;
> + dma_cap_set(DMA_SLAVE, edma_filter_info.dma_cap);
> + of_dma_controller_register(dev->of_node,
> + of_dma_simple_xlate,
> + &edma_filter_info);
> + pm_runtime_enable(dev);
> + ret = pm_runtime_get_sync(dev);
> + if (IS_ERR_VALUE(ret)) {
> + dev_err(dev, "pm_runtime_get_sync() failed\n");
> + return ret;
> + }
> + }
>
> if (!info)
> return -ENODEV;
>
> for (j = 0; j < EDMA_MAX_CC; j++) {
> - sprintf(res_name, "edma_cc%d", j);
> - r[j] = platform_get_resource_byname(pdev, IORESOURCE_MEM,
> + if (node) {
> + int err;
> + err = of_address_to_resource(node, 0, &res[j]);
> + if (err) {
> + dev_err(dev,
> + "unable to find 'reg' property\n");
> + return -EIO;
> + }
> + r[j] = &res[j];
> +
> + } else {
> + sprintf(res_name, "edma_cc%d", j);
> + r[j] = platform_get_resource_byname(pdev,
> + IORESOURCE_MEM,
> res_name);
> + }
> if (!r[j] || !info[j]) {
> if (found)
> break;
> @@ -1465,8 +1678,12 @@ static int __init edma_probe(struct platform_device *pdev)
> }
> }
>
> - sprintf(irq_name, "edma%d", j);
> - irq[j] = platform_get_irq_byname(pdev, irq_name);
> + if (node)
> + irq[j] = irq_of_parse_and_map(node, 0);
> + else {
> + sprintf(irq_name, "edma%d", j);
> + irq[j] = platform_get_irq_byname(pdev, irq_name);
> + }
> edma_cc[j]->irq_res_start = irq[j];
> status = request_irq(irq[j], dma_irq_handler, 0, "edma",
> &pdev->dev);
> @@ -1476,8 +1693,12 @@ static int __init edma_probe(struct platform_device *pdev)
> goto fail;
> }
>
> - sprintf(irq_name, "edma%d_err", j);
> - err_irq[j] = platform_get_irq_byname(pdev, irq_name);
> + if (node)
> + err_irq[j] = irq_of_parse_and_map(node, 2);
> + else {
> + sprintf(irq_name, "edma%d_err", j);
> + err_irq[j] = platform_get_irq_byname(pdev, irq_name);
> + }
> edma_cc[j]->irq_res_end = err_irq[j];
> status = request_irq(err_irq[j], dma_ccerr_handler, 0,
> "edma_error", &pdev->dev);
> @@ -1538,9 +1759,17 @@ fail1:
> return status;
> }
>
> +static const struct of_device_id edma_of_ids[] = {
> + { .compatible = "ti,edma3", },
> + {}
> +};
>
> static struct platform_driver edma_driver = {
> - .driver.name = "edma",
> + .driver = {
> + .name = "edma",
> + .of_match_table = edma_of_ids,

Won't this fail/warn when CONFIG_OF not selected/enabled?

> + },
> + .probe = edma_probe,
> };
>
> static int __init edma_init(void)
> @@ -1548,4 +1777,3 @@ static int __init edma_init(void)
> return platform_driver_probe(&edma_driver, edma_probe);
> }
> arch_initcall(edma_init);
> -

Stray change I believe.

> diff --git a/arch/arm/include/asm/mach/edma.h b/arch/arm/include/asm/mach/edma.h
> index 7e84c90..ce5f6f8 100644
> --- a/arch/arm/include/asm/mach/edma.h
> +++ b/arch/arm/include/asm/mach/edma.h
> @@ -237,8 +237,8 @@ void edma_resume(unsigned channel);
>
> struct edma_rsv_info {
>
> - const s16 (*rsv_chans)[2];
> - const s16 (*rsv_slots)[2];
> + s16 (*rsv_chans)[2];
> + s16 (*rsv_slots)[2];
> };
>
> /* platform_data for EDMA driver */
> @@ -260,8 +260,8 @@ struct edma_soc_info {
> /* Resource reservation for other cores */
> struct edma_rsv_info *rsv;
>
> - const s8 (*queue_tc_mapping)[2];
> - const s8 (*queue_priority_mapping)[2];
> + s8 (*queue_tc_mapping)[2];
> + s8 (*queue_priority_mapping)[2];
> };
>
> #endif
> --
> 1.7.9.5
>
> _______________________________________________
> Davinci-linux-open-source mailing list
> Davinci-linux-open-source@xxxxxxxxxxxxxxxxxxxx
> http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
>

Regards,
Gururaja
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Next message: Wei Ni: "[PATCH v2 2/5] ARM: dt: t20 seaboard: turn on the power for wlan"
Previous message: Hebbar, Gururaja: "RE: [RFC PATCH 05/13] dma: Add TI EDMA device tree binding"
In reply to: Matt Porter: "[RFC PATCH 03/13] ARM: edma: add DT and runtime PM support for AM335x"
Next in thread: Matt Porter: "[RFC PATCH 10/13] spi: omap2-mcspi: dma_request_slave_channel() support for DT platforms"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]