On 29/11/18 18:33, David Lechner wrote: > On 11/29/18 4:07 AM, Roger Quadros wrote: >> On 27/11/18 01:27, David Lechner wrote: >>> On 11/26/18 1:52 AM, Roger Quadros wrote: >>>> From: Tero Kristo <t-kristo@xxxxxx> >>>> >>>> Add documentation for the Texas Instruments PRU application nodes. >>>> These are used to configure specific user applications for PRU instances. >>> >>> Could this be made into a generic remoteproc producer/consumer binding? Or >>> are there really things that are specific to the TI PRU that need to be >>> handled? >> >> The remoteproc handle and firmware name sound generic enough. >> But there are TI PRU specific properties as well which we'll discuss if >> they can be made generic. >> >>> >>>> >>>> Signed-off-by: Tero Kristo <t-kristo@xxxxxx> >>>> [s-anna@xxxxxx: some binding updates] >>>> Signed-off-by: Suman Anna <s-anna@xxxxxx> >>>> Signed-off-by: Roger Quadros <rogerq@xxxxxx> >>>> --- >>>> .../devicetree/bindings/soc/ti/ti,pruss.txt | 43 ++++++++++++++++++++++ >>>> 1 file changed, 43 insertions(+) >>>> >>>> diff --git a/Documentation/devicetree/bindings/soc/ti/ti,pruss.txt b/Documentation/devicetree/bindings/soc/ti/ti,pruss.txt >>>> index 3e5f32f..94c91ee 100644 >>>> --- a/Documentation/devicetree/bindings/soc/ti/ti,pruss.txt >>>> +++ b/Documentation/devicetree/bindings/soc/ti/ti,pruss.txt >>>> @@ -210,6 +210,38 @@ used in TI Davinci SoCs. Please refer to the corresponding binding document, >>>> Documentation/devicetree/bindings/net/davinci-mdio.txt for details. >>>> +Application/User Nodes >>>> +======================= >>> >>> Are these supposed to be stand-alone platform devices? >>> >> >> Yes. The first use case we're going to address is the Ethernet ports on the IDKs. >> (Industrial development Kit) http://www.ti.com/tool/TMDXIDK437X >> >>>> +A PRU application/user node typically uses one or more PRU device nodes to >>>> +implement a PRU application/functionality. Each application/client node would >>>> +need a reference to at least a PRU node, and optionally pass some configuration >>>> +parameters. >>> >>> I thought device tree is not supposed to be used for configuration. >> >> I think we need to word it properly. It is really a hardware/firmware map. >>> >>>> + >>>> +Required Properties: >>>> +-------------------- >>>> +- prus : phandles to the PRU nodes used >>>> + >>>> +Optional Properties: >>>> +-------------------- >>>> +- firmware-name : firmwares for the PRU cores, the default firmware >>>> + for the core from the PRU node will be used if not >>>> + provided. The firmware names should correspond to >>>> + the PRU cores listed in the 'prus' property >>> >>> Perhaps this should be a "compatible" property instead of "firmware-name"? The >>> driver that matches the compatible string can then set the firmware names. >> >> Compatible property is there to choose the application driver. Should have mentioned >> it in Required properties. >> >> It is tricky for the driver to decipher the firmware-name as it needs to support >> the same use case on multiple platforms and the firmware name will be different for each. >> The driver itself is platform agnostic. >> >> So providing the firmware-name in the DT is the easiest and scalable solution. >>> >>>> +- ti,pruss-gp-mux-sel : array of values for the GP_MUX_SEL under PRUSS_GPCFG >>>> + register for a PRU. This selects the internal muxing >>>> + scheme for the PRU instance. If not provided, the >>>> + default out-of-reset value (0) for the PRU core is >>>> + used. Values should correspond to the PRU cores listed >>>> + in the 'prus' property >>> >>> Is this supposed to be a pinmux? So maybe we should be using pinmux bindings? >> >> We already have pinmux binding for the PRU pins. This GP mux setting is an odd duck. >> >> It provides a way for a set of signals inside the ICSS to be connected to the PRU pins >> on the SOC, which are again multiplexed with other SOC pins via the regular pinmux. >> >> Some of the sets are >> >> GPIO mode (0) >> EnDAT mode (1) >> SD mode (3) >> MII2 mode (4) >> >> The application node needs to decide which set it wants to use. >> >>> >>>> +- ti,pru-interrupt-map : PRU interrupt mappings, containing an array of entries >>>> + with each entry consisting of 4 cell-values. First one >>>> + is an index towards the "prus" property to identify the >>>> + PRU core for the interrupt map, second is the PRU >>>> + System Event id, third is the PRU interrupt channel id >>>> + and fourth is the PRU host interrupt id. If provided, >>>> + this map will supercede any other configuration >>>> + provided through firmware >>> >>> Could this mapping just be cells of the interrupt consumer nodes instead of an >>> extra property? As I mentioned in a reply to another patch, unless there is a >>> compelling reason to do otherwise, the channel to host mapping can be required >>> to be 1:1 as recommended in the TRMs, so that cell can be omitted. Also, since >>> the interrupt controller is independent of the PRU cores, the cell specifying the >>> index of the PRU core is not needed in this case. The #interrupt-cells already >>> includes a cell for the system event number, so this just leaves one cell, the >>> host channel, to be added to the #interrupt-cells. >>> >>> So, instead of: >>> >>> ti,pru-interrupt-map = <0 16 2 7 >, <1 19 1 3>; >>> >>> we could have: >>> >>> interrupt-parent = <&pruss_intc>; >>> interrupts = <16 7>, <19 3>; >>> >> >> No, interrupts property will be used to provide the actual sysevent IRQs to the >> application driver. Below is how the ethernet application node looks like. >> >> pruss2_eth { >> compatible = "ti,am57-prueth"; >> prus = <&pru2_0>, <&pru2_1>; >> firmware-name = "ti-pruss/am57xx-pru0-prueth-fw.elf", >> "ti-pruss/am57xx-pru1-prueth-fw.elf"; >> sram = <&ocmcram1>; >> interrupt-parent = <&pruss2_intc>; >> mii-rt = <&pruss2_mii_rt>; >> iep = <&pruss2_iep>; >> >> pruss2_emac0: ethernet-mii0 { >> phy-handle = <&pruss2_eth0_phy>; >> phy-mode = "mii"; >> interrupts = <20>, <22>; >> interrupt-names = "rx", "tx"; >> }; >> >> pruss2_emac1: ethernet-mii1 { >> phy-handle = <&pruss2_eth1_phy>; >> phy-mode = "mii"; >> interrupts = <21>, <23>; >> interrupt-names = "rx", "tx"; >> }; >> }; >> >> You can see that interrupts is providing the RX and TX sysevents. >> >> There needs to be a different way to provide the internal INTC map. >> >> Currently there are 2 ways of providing the INTC map. One is via the >> resource table and other is via DT. >> >>> There are also already alternate interrupt bindings that allow for the case >>> where there is more than one interrupt-parent. > > Thanks for the insights. On the example above there is not a > ti,pru-interrupt-map property. Does this mean that the interrupt > mapping table comes from the firmware/resource-table in this case? > Yes, that's correct. >>> >>>> + >>>> Example: >>>> ======== >>>> 1. /* AM33xx PRU-ICSS */ >>>> @@ -397,3 +429,14 @@ Example: >>>> ... >>>> }; >>>> }; >>>> + >>>> +3: /* PRU application node example */ >>>> + app_node: app_node { >>>> + prus = <&pru1_0>, <&pru1_1>; >>>> + firmware-name = "pruss-app-fw", "pruss-app-fw-2"; >>>> + ti,pruss-gp-mux-sel = <2>, <1>; >>>> + /* setup interrupts for prus: >>>> + prus[0] => pru1_0: ev=16, chnl=2, host-irq=7, >>>> + prus[1] => pru1_1: ev=19, chnl=1, host-irq=3 */ >>>> + ti,pru-interrupt-map = <0 16 2 7 >, <1 19 1 3>; >>>> + } >>>> >>> >> cheers, -roger -- Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
