From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-8.4 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH, MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 1BFAAC32771 for ; Thu, 9 Jan 2020 08:54:09 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id D08332067D for ; Thu, 9 Jan 2020 08:54:08 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=ti.com header.i=@ti.com header.b="Vhqweda6" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728947AbgAIIyH (ORCPT ); Thu, 9 Jan 2020 03:54:07 -0500 Received: from lelv0143.ext.ti.com ([198.47.23.248]:47672 "EHLO lelv0143.ext.ti.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1728782AbgAIIyH (ORCPT ); Thu, 9 Jan 2020 03:54:07 -0500 Received: from lelv0265.itg.ti.com ([10.180.67.224]) by lelv0143.ext.ti.com (8.15.2/8.15.2) with ESMTP id 0098s2bu109333; Thu, 9 Jan 2020 02:54:02 -0600 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ti.com; s=ti-com-17Q1; t=1578560042; bh=UtTnldwt1URYkVixdFv9rhGPFWBg52Xc2qL7CUFPZlU=; h=Subject:To:CC:References:From:Date:In-Reply-To; b=Vhqweda6xGLhJagbkZX/b9rlL/NNySSJ26VVAzD1JgWjSmEbk3yxJSwPZQW3BFACf ifCm7PJJUcN0jS3QGYDjGdSH1bQAEjGajR2TAoMzWxC3jI7LT76e2pbs5Kl3KMLOrd BckKWAp7sWHuMZnkYqqOc1/Fr4ui3adgodT84dcU= Received: from DFLE109.ent.ti.com (dfle109.ent.ti.com [10.64.6.30]) by lelv0265.itg.ti.com (8.15.2/8.15.2) with ESMTPS id 0098s2Pr079552 (version=TLSv1.2 cipher=AES256-GCM-SHA384 bits=256 verify=FAIL); Thu, 9 Jan 2020 02:54:02 -0600 Received: from DFLE112.ent.ti.com (10.64.6.33) by DFLE109.ent.ti.com (10.64.6.30) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256_P256) id 15.1.1847.3; Thu, 9 Jan 2020 02:54:01 -0600 Received: from fllv0040.itg.ti.com (10.64.41.20) by DFLE112.ent.ti.com (10.64.6.33) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256_P256) id 15.1.1847.3 via Frontend Transport; Thu, 9 Jan 2020 02:54:01 -0600 Received: from [127.0.0.1] (ileax41-snat.itg.ti.com [10.172.224.153]) by fllv0040.itg.ti.com (8.15.2/8.15.2) with ESMTP id 0098rwX2114038; Thu, 9 Jan 2020 02:53:59 -0600 Subject: Re: [PATCHv4 02/14] remoteproc/omap: Add device tree support To: Suman Anna , , , CC: , , , Tony Lindgren References: <20200102131845.12992-1-t-kristo@ti.com> <20200102131845.12992-3-t-kristo@ti.com> <1f9f69df-6b92-7d29-7769-4d10811dfffe@ti.com> From: Tero Kristo Message-ID: <37622ee3-95f0-2e05-3897-61664736a286@ti.com> Date: Thu, 9 Jan 2020 10:53:57 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.2.2 MIME-Version: 1.0 In-Reply-To: <1f9f69df-6b92-7d29-7769-4d10811dfffe@ti.com> Content-Type: text/plain; charset="utf-8"; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit X-EXCLAIMER-MD-CONFIG: e1e8a2fd-e40a-4ac6-ac9b-f7e9cc9ee180 Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 08/01/2020 19:55, Suman Anna wrote: > Hi Tero, > > On 1/2/20 7:18 AM, Tero Kristo wrote: >> From: Suman Anna >> >> 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 >> Signed-off-by: Suman Anna >> [t-kristo@ti.com: converted to use ti-sysc framework] >> Signed-off-by: Tero Kristo >> --- >> 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 >> @@ -16,27 +16,51 @@ >> #include >> #include >> #include >> +#include >> #include >> #include >> #include >> #include >> #include >> - >> -#include >> +#include >> +#include >> +#include >> >> #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