On 12/20/19 3:36 AM, Tero Kristo wrote: > On 20/12/2019 04:08, Suman Anna wrote: >> Hi Tero, Mathieu, >> >> On 12/19/19 5:54 AM, Tero Kristo wrote: >>> On 18/12/2019 01:01, Mathieu Poirier wrote: >>>> Hi Tero, >>>> >>>> On Fri, Dec 13, 2019 at 02:55:24PM +0200, Tero Kristo wrote: >>>>> From: Suman Anna <s-anna@xxxxxx> >>>>> >>>>> 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@xxxxxxxxxxx> >>>>> Signed-off-by: Suman Anna <s-anna@xxxxxx> >>>>> [t-kristo@xxxxxx: converted to use ti-sysc framework] >>>>> Signed-off-by: Tero Kristo <t-kristo@xxxxxx> >>>>> --- >>>>> drivers/remoteproc/omap_remoteproc.c | 191 >>>>> +++++++++++++++++++++++---- >>>>> 1 file changed, 168 insertions(+), 23 deletions(-) >>>>> >>>>> diff --git a/drivers/remoteproc/omap_remoteproc.c >>>>> b/drivers/remoteproc/omap_remoteproc.c >>>>> index 6398194075aa..558634624590 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-2019 Texas Instruments Incorporated - >>>>> http://www.ti.com/ >>>>> * Copyright (C) 2011 Google, Inc. >>>>> * >>>>> * Ohad Ben-Cohen <ohad@xxxxxxxxxx> >>>>> @@ -16,27 +16,53 @@ >>>>> #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 >>>>> + * @has_bootreg: true if this remote processor has boot register >>>>> + */ >>>>> +struct omap_rproc_dev_data { >>>>> + const char *device_name; >>>>> + bool has_bootreg; >>>>> }; >>>>> /** >>>>> @@ -92,6 +118,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 +144,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 +156,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,11 +177,7 @@ static int omap_rproc_start(struct rproc *rproc) >>>>> goto put_mbox; >>>>> } >>>>> - ret = pdata->device_enable(pdev); >>>>> - if (ret) { >>>>> - dev_err(dev, "omap_device_enable failed: %d\n", ret); >>>>> - goto put_mbox; >>>>> - } >>>>> + reset_control_deassert(oproc->reset); >>>>> return 0; >>>>> @@ -154,15 +189,9 @@ 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); >>>>> - if (ret) >>>>> - return ret; >>>>> + reset_control_assert(oproc->reset); >> >> Any reasons for dropping the checks for the return status and wherever >> you replaced the pdata callbacks with the desired reset API? > > Ok, let me try to add the checks back. > >> >>>>> mbox_free_channel(oproc->mbox); >>>>> @@ -175,12 +204,122 @@ 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", >>>>> + .has_bootreg = true, >>>>> +}; >>>>> + >>>>> +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", >>>>> + .has_bootreg = true, >>>>> +}; >>>>> + >>>>> +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 (!data->has_bootreg) >>>>> + return 0; >>>>> + >>>>> + oproc->boot_data = devm_kzalloc(&pdev->dev, >>>>> sizeof(*oproc->boot_data), >>>>> + GFP_KERNEL); >>>>> + if (!oproc->boot_data) >>>>> + return -ENOMEM; >>>>> + >>>>> + if (!of_property_read_bool(np, "ti,bootreg")) { >>>>> + dev_err(&pdev->dev, "ti,bootreg property is missing\n"); >>>>> + return -EINVAL; >>>>> + } >>>>> + >>>>> + 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_optional_exclusive(&pdev->dev); >>>>> + if (IS_ERR(reset)) >>>>> + return PTR_ERR(reset); >>>> >>>> Definition of a reset is listed as "required" in the bindings but here >>>> it is >>>> optional. If this is really what you want then adding a comment to >>>> exlain your >>>> choice is probably a good idea. >>> >>> Right, I think I updated the binding to require this but forgot to >>> update the driver for this part. Will fix this. >>> >>> -Tero >>> >>>> >>>>> + >>>>> + 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 +327,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 +370,7 @@ static struct platform_driver omap_rproc_driver >>>>> = { >>>>> .remove = omap_rproc_remove, >>>>> .driver = { >>>>> .name = "omap-rproc", >>>>> + .of_match_table = omap_rproc_of_match, >>>> >>>> .of_match_table = of_match_ptr(omap_rproc_of_match), >> >> I had dropped this sometime back intentionally as all our platforms are >> DT-only. > > Hmm, dropped what? Dropped the of_match_ptr. regards Suman > > -Tero > >> >> regards >> Suman >> >>>> >>>> Thanks, >>>> Mathieu >>>> >>>>> }, >>>>> }; >>>>> -- >>>>> 2.17.1 >>>>> >>>>> -- >>> >>> -- >> > > -- > Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. > Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
