Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751301AbdFETK5 (ORCPT ); Mon, 5 Jun 2017 15:10:57 -0400 Received: from mail.kernel.org ([198.145.29.99]:45356 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751077AbdFETKz (ORCPT ); Mon, 5 Jun 2017 15:10:55 -0400 DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 8D66C23A17 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=robh@kernel.org MIME-Version: 1.0 In-Reply-To: <39a18ffb-cd33-29a7-c3e9-f8e2e6d3432b@ti.com> References: <20170526165317.23164-1-s-anna@ti.com> <20170526165317.23164-2-s-anna@ti.com> <20170531191231.5qt2lfv73p572gmd@rob-hp-laptop> <884f5585-eac2-57b2-a2be-879bf23f2e00@ti.com> <20170605172648.6xonnkvbwmd3cehg@rob-hp-laptop> <39a18ffb-cd33-29a7-c3e9-f8e2e6d3432b@ti.com> From: Rob Herring Date: Mon, 5 Jun 2017 14:10:33 -0500 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH 1/3] Documentation: DT: add Keystone DSP remoteproc binding To: Suman Anna Cc: Bjorn Andersson , Ohad Ben-Cohen , Santosh Shilimkar , Mark Rutland , "open list:REMOTE PROCESSOR (REMOTEPROC) SUBSYSTEM" , "linux-arm-kernel@lists.infradead.org" , "devicetree@vger.kernel.org" , "linux-kernel@vger.kernel.org" , "Andrew F. Davis" , Sam Nelson Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 5714 Lines: 118 On Mon, Jun 5, 2017 at 1:21 PM, Suman Anna wrote: > Hi Rob, > >>> >>> On 05/31/2017 02:12 PM, Rob Herring wrote: >>>> On Fri, May 26, 2017 at 11:53:15AM -0500, Suman Anna wrote: >>>>> Add the device tree bindings document for the Texas Instrument's >>>>> Keystone 2 DSP remoteproc devices. >>>>> >>>>> Signed-off-by: Suman Anna >>>>> Signed-off-by: Sam Nelson >>>>> --- >>>>> .../bindings/remoteproc/ti,keystone-rproc.txt | 132 +++++++++++++++++++++ >>>>> 1 file changed, 132 insertions(+) >>>>> create mode 100644 Documentation/devicetree/bindings/remoteproc/ti,keystone-rproc.txt >>>>> >>>>> diff --git a/Documentation/devicetree/bindings/remoteproc/ti,keystone-rproc.txt b/Documentation/devicetree/bindings/remoteproc/ti,keystone-rproc.txt >>>>> new file mode 100644 >>>>> index 000000000000..f1ba88edd00d >>>>> --- /dev/null >>>>> +++ b/Documentation/devicetree/bindings/remoteproc/ti,keystone-rproc.txt >>>>> @@ -0,0 +1,132 @@ >>>>> +TI Keystone DSP devices >>>>> +======================= >>>>> + >>>>> +Binding status: Unstable - Subject to changes for using multiple memory regions >>>> >>>> I don't really see what would be unstable here. memory-region is easily >>>> extended to multiple entries. >>> >>> OK will drop this line. >>> >>>> >>>>> + >>>>> +The TI Keystone 2 family of SoCs usually have one or more (upto 8) TI DSP Core >>>>> +sub-systems that are used to offload some of the processor-intensive tasks or >>>>> +algorithms, for achieving various system level goals. >>>>> + >>>>> +These processor sub-systems usually contain additional sub-modules like L1 >>>>> +and/or L2 caches/SRAMs, an Interrupt Controller, an external memory controller, >>>>> +a dedicated local power/sleep controller etc. The DSP processor core in >>>>> +Keystone 2 SoCs is usually a TMS320C66x CorePac processor. >>>>> + >>>>> +DSP Device Node: >>>>> +================ >>>>> +Each DSP Core sub-system is represented as a single DT node. Each node has a >>>>> +number of required or optional properties that enable the OS running on the >>>>> +host processor (ARM CorePac) to perform the device management of the remote >>>>> +processor and to communicate with the remote processor. >>>>> + >>>>> +Required properties: >>>>> +-------------------- >>>>> +The following are the mandatory properties: >>>>> + >>>>> +- compatible: Should be one of the following, >>>>> + "ti,k2hk-dsp" for DSPs on Keystone 2 66AK2H/K SoCs >>>>> + "ti,k2l-dsp" for DSPs on Keystone 2 66AK2L SoCs >>>>> + "ti,k2e-dsp" for DSPs on Keystone 2 66AK2E SoCs >>>>> + >>>>> +- reg: Should contain an entry for each value in 'reg-names'. >>>>> + Each entry should have the memory region's start address >>>>> + and the size of the region, the representation matching >>>>> + the parent node's '#address-cells' and '#size-cells' values. >>>>> + >>>>> +- reg-names: Should contain strings with the following names, each >>>>> + representing a specific internal memory region, and >>>>> + should be defined in this order, >>>>> + "l2sram", "l1pram", "l1dram" >>>>> + >>>>> +- label: Should contain a string identifying the DSP instance >>>>> + within the SoC. Should be the string "dsp" followed by >>>>> + the instance number. Eg: "dsp0", "dsp1",..."dsp7" etc >>>> >>>> Why does a user need to know or care? >>> >>> This is used to distinguish the exact DSP instance from others since the >>> DT node name is a generic "dsp". One of the uses is to be able to >>> construct a firmware name within the driver using this label. >> >> firmware-name doesn't work for you? > > Has the stance changed w.r.t coding up the firmware-name in DT now? My > understanding was that this has to be coded up in the driver from a > previous related discussion on this [1]. I am more than happy to move > the firmware name into DT for all remoteprocs if that is an accepted > solution now. There's not a hard and fast rule. For FPGAs at least it made sense to use firmware-name. >> It was obvious that you are using this to number instances. Why do you >> care which instance is which? For example, I want dsp0 because it has X >> or runs at XXMHz, etc. If each instance is different (i.e. not a pool of >> DSPs), then you should describe the difference some way. > > Well, I am not exactly using the label to number the instance, it is a > possibility outside of using it to encode the firmware name. The > partitioning is really upto the application/SoC s/w system integration > aspects. Most of the times, you are not running identical software on > these DSPs or other remote processors. For example, you might have one > DSP running an OpenCL stack, another DSP dedicated for audio > post-processing etc. > > That said, I do have a need for numbering the instances. This is needed > usually for constructing a destination address to encode the processor > when using a common Inter-Processor Communication API across all > processors (Linux or otherwise). Pre-DT, I have used the platform device > id for this on Linux behaving as the master processor. For DT, my > preferred solution for that is an alias. If an alias is not acceptable, > then I still have to provide an equivalent functionality through a > driver-specific scheme. I would prefer aliases over labels here. It may make sense to just have a property for the desitination address (or some offset). Or perhaps a list of IPC targets and the index to that list is the instance. Rob