| 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: <37622ee3-95f0-2e05-3897-61664736a286@ti.com>
Date: Thu, 9 Jan 2020 10:53:57 +0200
From: Tero Kristo <t-kristo@...com>
To: Suman Anna <s-anna@...com>, <bjorn.andersson@...aro.org>,
<ohad@...ery.com>, <linux-remoteproc@...r.kernel.org>
CC: <linux-kernel@...r.kernel.org>, <mathieu.poirier@...aro.org>,
<linux-omap@...r.kernel.org>, Tony Lindgren <tony@...mide.com>
Subject: Re: [PATCHv4 02/14] remoteproc/omap: Add device tree support
On 08/01/2020 19:55, Suman Anna wrote:
> Hi Tero,
>
> On 1/2/20 7:18 AM, Tero Kristo wrote:
>> From: Suman Anna <s-anna@...com>
>>
>> OMAP4+ SoCs support device tree boot only. The OMAP remoteproc
>> driver is enhanced to support remoteproc devices created through
>> Device Tree, support for legacy platform devices has been
>> deprecated. The current DT support handles the IPU and DSP
>> processor subsystems on OMAP4 and OMAP5 SoCs.
>>
>> The OMAP remoteproc driver relies on the ti-sysc, reset, and
>> syscon layers for performing clock, reset and boot vector
>> management (DSP remoteprocs only) of the devices, but some of
>> these are limited only to the machine-specific layers
>> in arch/arm. The dependency against control module API for boot
>> vector management of the DSP remoteprocs has now been removed
>> with added logic to parse the boot register from the DT node
>> and program it appropriately directly within the driver.
>>
>> The OMAP remoteproc driver expects the firmware names to be
>> provided via device tree entries (firmware-name.) These are used
>> to load the proper firmware during boot of the remote processor.
>>
>> Cc: Tony Lindgren <tony@...mide.com>
>> Signed-off-by: Suman Anna <s-anna@...com>
>> [t-kristo@...com: converted to use ti-sysc framework]
>> Signed-off-by: Tero Kristo <t-kristo@...com>
>> ---
>> v4:
>> - error handling improvements
>
> Thanks for taking care of these comments.
>
>> - dropped has_bootreg from platform data (instead parsed from DT)
>
> So, I originally used this to ensure that the DT node is not missing the
> bootreg property on the required devices (since it is not present on
> all). I guess this is no longer needed since the idea is that it would
> be caught during the schema check, please confirm.
Yes, schema will complain if you don't have bootreg on the required nodes.
-Tero
>
> regards
> Suman
>
>>
>> drivers/remoteproc/omap_remoteproc.c | 177 ++++++++++++++++++++++++---
>> 1 file changed, 160 insertions(+), 17 deletions(-)
>>
>> diff --git a/drivers/remoteproc/omap_remoteproc.c b/drivers/remoteproc/omap_remoteproc.c
>> index 6398194075aa..fb4902e4dacf 100644
>> --- a/drivers/remoteproc/omap_remoteproc.c
>> +++ b/drivers/remoteproc/omap_remoteproc.c
>> @@ -2,7 +2,7 @@
>> /*
>> * OMAP Remote Processor driver
>> *
>> - * Copyright (C) 2011 Texas Instruments, Inc.
>> + * Copyright (C) 2011-2020 Texas Instruments Incorporated - http://www.ti.com/
>> * Copyright (C) 2011 Google, Inc.
>> *
>> * Ohad Ben-Cohen <ohad@...ery.com>
>> @@ -16,27 +16,51 @@
>> #include <linux/kernel.h>
>> #include <linux/module.h>
>> #include <linux/err.h>
>> +#include <linux/of_device.h>
>> #include <linux/platform_device.h>
>> #include <linux/dma-mapping.h>
>> #include <linux/remoteproc.h>
>> #include <linux/mailbox_client.h>
>> #include <linux/omap-mailbox.h>
>> -
>> -#include <linux/platform_data/remoteproc-omap.h>
>> +#include <linux/regmap.h>
>> +#include <linux/mfd/syscon.h>
>> +#include <linux/reset.h>
>>
>> #include "omap_remoteproc.h"
>> #include "remoteproc_internal.h"
>>
>> +/**
>> + * struct omap_rproc_boot_data - boot data structure for the DSP omap rprocs
>> + * @syscon: regmap handle for the system control configuration module
>> + * @boot_reg: boot register offset within the @syscon regmap
>> + */
>> +struct omap_rproc_boot_data {
>> + struct regmap *syscon;
>> + unsigned int boot_reg;
>> +};
>> +
>> /**
>> * struct omap_rproc - omap remote processor state
>> * @mbox: mailbox channel handle
>> * @client: mailbox client to request the mailbox channel
>> + * @boot_data: boot data structure for setting processor boot address
>> * @rproc: rproc handle
>> + * @reset: reset handle
>> */
>> struct omap_rproc {
>> struct mbox_chan *mbox;
>> struct mbox_client client;
>> + struct omap_rproc_boot_data *boot_data;
>> struct rproc *rproc;
>> + struct reset_control *reset;
>> +};
>> +
>> +/**
>> + * struct omap_rproc_dev_data - device data for the omap remote processor
>> + * @device_name: device name of the remote processor
>> + */
>> +struct omap_rproc_dev_data {
>> + const char *device_name;
>> };
>>
>> /**
>> @@ -92,6 +116,21 @@ static void omap_rproc_kick(struct rproc *rproc, int vqid)
>> ret);
>> }
>>
>> +/**
>> + * omap_rproc_write_dsp_boot_addr - set boot address for a DSP remote processor
>> + * @rproc: handle of a remote processor
>> + *
>> + * Set boot address for a supported DSP remote processor.
>> + */
>> +static void omap_rproc_write_dsp_boot_addr(struct rproc *rproc)
>> +{
>> + struct omap_rproc *oproc = rproc->priv;
>> + struct omap_rproc_boot_data *bdata = oproc->boot_data;
>> + u32 offset = bdata->boot_reg;
>> +
>> + regmap_write(bdata->syscon, offset, rproc->bootaddr);
>> +}
>> +
>> /*
>> * Power up the remote processor.
>> *
>> @@ -103,13 +142,11 @@ static int omap_rproc_start(struct rproc *rproc)
>> {
>> struct omap_rproc *oproc = rproc->priv;
>> struct device *dev = rproc->dev.parent;
>> - struct platform_device *pdev = to_platform_device(dev);
>> - struct omap_rproc_pdata *pdata = pdev->dev.platform_data;
>> int ret;
>> struct mbox_client *client = &oproc->client;
>>
>> - if (pdata->set_bootaddr)
>> - pdata->set_bootaddr(rproc->bootaddr);
>> + if (oproc->boot_data)
>> + omap_rproc_write_dsp_boot_addr(rproc);
>>
>> client->dev = dev;
>> client->tx_done = NULL;
>> @@ -117,7 +154,7 @@ static int omap_rproc_start(struct rproc *rproc)
>> client->tx_block = false;
>> client->knows_txdone = false;
>>
>> - oproc->mbox = omap_mbox_request_channel(client, pdata->mbox_name);
>> + oproc->mbox = mbox_request_channel(client, 0);
>> if (IS_ERR(oproc->mbox)) {
>> ret = -EBUSY;
>> dev_err(dev, "mbox_request_channel failed: %ld\n",
>> @@ -138,9 +175,9 @@ static int omap_rproc_start(struct rproc *rproc)
>> goto put_mbox;
>> }
>>
>> - ret = pdata->device_enable(pdev);
>> + ret = reset_control_deassert(oproc->reset);
>> if (ret) {
>> - dev_err(dev, "omap_device_enable failed: %d\n", ret);
>> + dev_err(dev, "reset control deassert failed: %d\n", ret);
>> goto put_mbox;
>> }
>>
>> @@ -154,13 +191,10 @@ static int omap_rproc_start(struct rproc *rproc)
>> /* power off the remote processor */
>> static int omap_rproc_stop(struct rproc *rproc)
>> {
>> - struct device *dev = rproc->dev.parent;
>> - struct platform_device *pdev = to_platform_device(dev);
>> - struct omap_rproc_pdata *pdata = pdev->dev.platform_data;
>> struct omap_rproc *oproc = rproc->priv;
>> int ret;
>>
>> - ret = pdata->device_shutdown(pdev);
>> + ret = reset_control_assert(oproc->reset);
>> if (ret)
>> return ret;
>>
>> @@ -175,12 +209,115 @@ static const struct rproc_ops omap_rproc_ops = {
>> .kick = omap_rproc_kick,
>> };
>>
>> +static const struct omap_rproc_dev_data omap4_dsp_dev_data = {
>> + .device_name = "dsp",
>> +};
>> +
>> +static const struct omap_rproc_dev_data omap4_ipu_dev_data = {
>> + .device_name = "ipu",
>> +};
>> +
>> +static const struct omap_rproc_dev_data omap5_dsp_dev_data = {
>> + .device_name = "dsp",
>> +};
>> +
>> +static const struct omap_rproc_dev_data omap5_ipu_dev_data = {
>> + .device_name = "ipu",
>> +};
>> +
>> +static const struct of_device_id omap_rproc_of_match[] = {
>> + {
>> + .compatible = "ti,omap4-dsp",
>> + .data = &omap4_dsp_dev_data,
>> + },
>> + {
>> + .compatible = "ti,omap4-ipu",
>> + .data = &omap4_ipu_dev_data,
>> + },
>> + {
>> + .compatible = "ti,omap5-dsp",
>> + .data = &omap5_dsp_dev_data,
>> + },
>> + {
>> + .compatible = "ti,omap5-ipu",
>> + .data = &omap5_ipu_dev_data,
>> + },
>> + {
>> + /* end */
>> + },
>> +};
>> +MODULE_DEVICE_TABLE(of, omap_rproc_of_match);
>> +
>> +static const char *omap_rproc_get_firmware(struct platform_device *pdev)
>> +{
>> + const char *fw_name;
>> + int ret;
>> +
>> + ret = of_property_read_string(pdev->dev.of_node, "firmware-name",
>> + &fw_name);
>> + if (ret)
>> + return ERR_PTR(ret);
>> +
>> + return fw_name;
>> +}
>> +
>> +static int omap_rproc_get_boot_data(struct platform_device *pdev,
>> + struct rproc *rproc)
>> +{
>> + struct device_node *np = pdev->dev.of_node;
>> + struct omap_rproc *oproc = rproc->priv;
>> + const struct omap_rproc_dev_data *data;
>> + int ret;
>> +
>> + data = of_device_get_match_data(&pdev->dev);
>> + if (!data)
>> + return -ENODEV;
>> +
>> + if (!of_property_read_bool(np, "ti,bootreg"))
>> + return 0;
>> +
>> + oproc->boot_data = devm_kzalloc(&pdev->dev, sizeof(*oproc->boot_data),
>> + GFP_KERNEL);
>> + if (!oproc->boot_data)
>> + return -ENOMEM;
>> +
>> + oproc->boot_data->syscon =
>> + syscon_regmap_lookup_by_phandle(np, "ti,bootreg");
>> + if (IS_ERR(oproc->boot_data->syscon)) {
>> + ret = PTR_ERR(oproc->boot_data->syscon);
>> + return ret;
>> + }
>> +
>> + if (of_property_read_u32_index(np, "ti,bootreg", 1,
>> + &oproc->boot_data->boot_reg)) {
>> + dev_err(&pdev->dev, "couldn't get the boot register\n");
>> + return -EINVAL;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> static int omap_rproc_probe(struct platform_device *pdev)
>> {
>> - struct omap_rproc_pdata *pdata = pdev->dev.platform_data;
>> + struct device_node *np = pdev->dev.of_node;
>> struct omap_rproc *oproc;
>> struct rproc *rproc;
>> + const char *firmware;
>> int ret;
>> + struct reset_control *reset;
>> +
>> + if (!np) {
>> + dev_err(&pdev->dev, "only DT-based devices are supported\n");
>> + return -ENODEV;
>> + }
>> +
>> + reset = devm_reset_control_array_get_exclusive(&pdev->dev);
>> + if (IS_ERR(reset))
>> + return PTR_ERR(reset);
>> +
>> + firmware = omap_rproc_get_firmware(pdev);
>> + if (IS_ERR(firmware))
>> + return PTR_ERR(firmware);
>>
>> ret = dma_set_coherent_mask(&pdev->dev, DMA_BIT_MASK(32));
>> if (ret) {
>> @@ -188,16 +325,21 @@ static int omap_rproc_probe(struct platform_device *pdev)
>> return ret;
>> }
>>
>> - rproc = rproc_alloc(&pdev->dev, pdata->name, &omap_rproc_ops,
>> - pdata->firmware, sizeof(*oproc));
>> + rproc = rproc_alloc(&pdev->dev, dev_name(&pdev->dev), &omap_rproc_ops,
>> + firmware, sizeof(*oproc));
>> if (!rproc)
>> return -ENOMEM;
>>
>> oproc = rproc->priv;
>> oproc->rproc = rproc;
>> + oproc->reset = reset;
>> /* All existing OMAP IPU and DSP processors have an MMU */
>> rproc->has_iommu = true;
>>
>> + ret = omap_rproc_get_boot_data(pdev, rproc);
>> + if (ret)
>> + goto free_rproc;
>> +
>> platform_set_drvdata(pdev, rproc);
>>
>> ret = rproc_add(rproc);
>> @@ -226,6 +368,7 @@ static struct platform_driver omap_rproc_driver = {
>> .remove = omap_rproc_remove,
>> .driver = {
>> .name = "omap-rproc",
>> + .of_match_table = omap_rproc_of_match,
>> },
>> };
>>
>>
>
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
Powered by blists - more mailing lists