diff mbox series

[v3,1/5] dt-bindings: remoteproc: Add PRU consumer bindings

Message ID 20220418104118.12878-2-p-mohan@ti.com (mailing list archive)
State Superseded
Headers show
Series Introduce PRU remoteproc consumer API | expand

Commit Message

Puranjay Mohan April 18, 2022, 10:41 a.m. UTC
From: Suman Anna <s-anna@ti.com>

Add a YAML binding document for PRU consumers. The binding includes
all the common properties that can be used by different PRU consumer
or application nodes and supported by the PRU remoteproc driver.
These are used to configure the PRU hardware for specific user
applications.

The application nodes themselves should define their own bindings.

Co-developed-by: Tero Kristo <t-kristo@ti.com>
Signed-off-by: Tero Kristo <t-kristo@ti.com>
Signed-off-by: Suman Anna <s-anna@ti.com>
Co-developed-by: Grzegorz Jaszczyk <grzegorz.jaszczyk@linaro.org>
Signed-off-by: Grzegorz Jaszczyk <grzegorz.jaszczyk@linaro.org>
Signed-off-by: Puranjay Mohan <p-mohan@ti.com>
---
 .../bindings/remoteproc/ti,pru-consumer.yaml  | 70 +++++++++++++++++++
 1 file changed, 70 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/remoteproc/ti,pru-consumer.yaml

Comments

Rob Herring (Arm) May 2, 2022, 7:56 p.m. UTC | #1
On Mon, Apr 18, 2022 at 04:11:14PM +0530, Puranjay Mohan wrote:
> From: Suman Anna <s-anna@ti.com>
> 
> Add a YAML binding document for PRU consumers. The binding includes
> all the common properties that can be used by different PRU consumer
> or application nodes and supported by the PRU remoteproc driver.
> These are used to configure the PRU hardware for specific user
> applications.
> 
> The application nodes themselves should define their own bindings.
> 
> Co-developed-by: Tero Kristo <t-kristo@ti.com>
> Signed-off-by: Tero Kristo <t-kristo@ti.com>
> Signed-off-by: Suman Anna <s-anna@ti.com>
> Co-developed-by: Grzegorz Jaszczyk <grzegorz.jaszczyk@linaro.org>
> Signed-off-by: Grzegorz Jaszczyk <grzegorz.jaszczyk@linaro.org>
> Signed-off-by: Puranjay Mohan <p-mohan@ti.com>
> ---
>  .../bindings/remoteproc/ti,pru-consumer.yaml  | 70 +++++++++++++++++++
>  1 file changed, 70 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/remoteproc/ti,pru-consumer.yaml
> 
> diff --git a/Documentation/devicetree/bindings/remoteproc/ti,pru-consumer.yaml b/Documentation/devicetree/bindings/remoteproc/ti,pru-consumer.yaml
> new file mode 100644
> index 000000000000..5b1f1cb2f098
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/remoteproc/ti,pru-consumer.yaml
> @@ -0,0 +1,70 @@
> +# SPDX-License-Identifier: (GPL-2.0-only or BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/remoteproc/ti,pru-consumer.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Common TI PRU Consumer Binding
> +
> +maintainers:
> +  - Suman Anna <s-anna@ti.com>
> +
> +description: |
> +  A PRU application/consumer/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 define
> +  some properties needed for hardware/firmware configuration. The below
> +  properties are a list of common properties supported by the PRU remoteproc
> +  infrastructure.
> +
> +  The application nodes shall define their own bindings like regular platform
> +  devices, so below are in addition to each node's bindings.
> +
> +properties:
> +  ti,prus:
> +    $ref: /schemas/types.yaml#/definitions/phandle-array
> +    description: phandles to the PRU, RTU or Tx_PRU nodes used
> +    minItems: 1
> +    maxItems: 6
> +    items:
> +      maxItems: 1
> +
> +  firmware-name:
> +    $ref: /schemas/types.yaml#/definitions/string-array
> +    description: |
> +      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 'ti,prus' property

So should be the name number of entries?:

minItems: 1
maxItems: 6

> +
> +  ti,pruss-gp-mux-sel:
> +    $ref: /schemas/types.yaml#/definitions/uint32-array

minItems: 1

> +    maxItems: 6
> +    items:
> +      enum: [0, 1, 2, 3, 4]
> +    description: |
> +      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. Values
> +      should correspond to the PRU cores listed in the 'ti,prus' property. The
> +      GP_MUX_SEL setting is a per-slice setting (one setting for PRU0, RTU0,
> +      and Tx_PRU0 on K3 SoCs). Use the same value for all cores within the
> +      same slice in the associative array. If the array size is smaller than
> +      the size of 'ti,prus' property, the default out-of-reset value (0) for the
> +      PRU core is used.
> +
> +required:
> +  - ti,prus
> +
> +dependencies:
> +  firmware-name: [ 'ti,prus' ]
> +  ti,pruss-gp-mux-sel: [ 'ti,prus' ]

This doesn't make sense because 'ti,prus' is already required. Should 
all 3 properties always be required?

> +
> +additionalProperties: true
> +
> +examples:
> +  - |
> +    /* PRU application node example */
> +    pru-app {
> +        ti,prus = <&pru0>, <&pru1>;
> +        firmware-name = "pruss-app-fw0", "pruss-app-fw1";
> +        ti,pruss-gp-mux-sel = <2>, <1>;

This example never validates, but okay I guess.

> +    };
> -- 
> 2.17.1
> 
>
Puranjay Mohan June 2, 2022, 5:28 a.m. UTC | #2
Hi Rob,

On 03/05/22 01:26, Rob Herring wrote:
> On Mon, Apr 18, 2022 at 04:11:14PM +0530, Puranjay Mohan wrote:
>> From: Suman Anna <s-anna@ti.com>
>>
>> Add a YAML binding document for PRU consumers. The binding includes
>> all the common properties that can be used by different PRU consumer
>> or application nodes and supported by the PRU remoteproc driver.
>> These are used to configure the PRU hardware for specific user
>> applications.
>>
>> The application nodes themselves should define their own bindings.
>>
>> Co-developed-by: Tero Kristo <t-kristo@ti.com>
>> Signed-off-by: Tero Kristo <t-kristo@ti.com>
>> Signed-off-by: Suman Anna <s-anna@ti.com>
>> Co-developed-by: Grzegorz Jaszczyk <grzegorz.jaszczyk@linaro.org>
>> Signed-off-by: Grzegorz Jaszczyk <grzegorz.jaszczyk@linaro.org>
>> Signed-off-by: Puranjay Mohan <p-mohan@ti.com>
>> ---
>>  .../bindings/remoteproc/ti,pru-consumer.yaml  | 70 +++++++++++++++++++
>>  1 file changed, 70 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/remoteproc/ti,pru-consumer.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/remoteproc/ti,pru-consumer.yaml b/Documentation/devicetree/bindings/remoteproc/ti,pru-consumer.yaml
>> new file mode 100644
>> index 000000000000..5b1f1cb2f098
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/remoteproc/ti,pru-consumer.yaml
>> @@ -0,0 +1,70 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only or BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/remoteproc/ti,pru-consumer.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Common TI PRU Consumer Binding
>> +
>> +maintainers:
>> +  - Suman Anna <s-anna@ti.com>
>> +
>> +description: |
>> +  A PRU application/consumer/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 define
>> +  some properties needed for hardware/firmware configuration. The below
>> +  properties are a list of common properties supported by the PRU remoteproc
>> +  infrastructure.
>> +
>> +  The application nodes shall define their own bindings like regular platform
>> +  devices, so below are in addition to each node's bindings.
>> +
>> +properties:
>> +  ti,prus:
>> +    $ref: /schemas/types.yaml#/definitions/phandle-array
>> +    description: phandles to the PRU, RTU or Tx_PRU nodes used
>> +    minItems: 1
>> +    maxItems: 6
>> +    items:
>> +      maxItems: 1
>> +
>> +  firmware-name:
>> +    $ref: /schemas/types.yaml#/definitions/string-array
>> +    description: |
>> +      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 'ti,prus' property
> 
> So should be the name number of entries?:
> 
> minItems: 1
> maxItems: 6

will add in v4

> 
>> +
>> +  ti,pruss-gp-mux-sel:
>> +    $ref: /schemas/types.yaml#/definitions/uint32-array
> 
> minItems: 1

will add in v4

> 
>> +    maxItems: 6
>> +    items:
>> +      enum: [0, 1, 2, 3, 4]
>> +    description: |
>> +      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. Values
>> +      should correspond to the PRU cores listed in the 'ti,prus' property. The
>> +      GP_MUX_SEL setting is a per-slice setting (one setting for PRU0, RTU0,
>> +      and Tx_PRU0 on K3 SoCs). Use the same value for all cores within the
>> +      same slice in the associative array. If the array size is smaller than
>> +      the size of 'ti,prus' property, the default out-of-reset value (0) for the
>> +      PRU core is used.
>> +
>> +required:
>> +  - ti,prus
>> +
>> +dependencies:
>> +  firmware-name: [ 'ti,prus' ]
>> +  ti,pruss-gp-mux-sel: [ 'ti,prus' ]
> 
> This doesn't make sense because 'ti,prus' is already required. Should 
> all 3 properties always be required?

All three of these are always required, so, I will remove the
"dependencies:" tag and add all three of them to "required:" in v4
Will it be the correct way to do it?

> 
>> +
>> +additionalProperties: true
>> +
>> +examples:
>> +  - |
>> +    /* PRU application node example */
>> +    pru-app {
>> +        ti,prus = <&pru0>, <&pru1>;
>> +        firmware-name = "pruss-app-fw0", "pruss-app-fw1";
>> +        ti,pruss-gp-mux-sel = <2>, <1>;
> 
> This example never validates, but okay I guess.
> 
>> +    };
>> -- 
>> 2.17.1
>>
>>

Thanks,
Puranjay Mohan
Roger Quadros June 3, 2022, 8:11 a.m. UTC | #3
Hi Puranjay,

On 02/06/2022 08:28, Puranjay Mohan wrote:
> Hi Rob,
> 
> On 03/05/22 01:26, Rob Herring wrote:
>> On Mon, Apr 18, 2022 at 04:11:14PM +0530, Puranjay Mohan wrote:
>>> From: Suman Anna <s-anna@ti.com>
>>>
>>> Add a YAML binding document for PRU consumers. The binding includes
>>> all the common properties that can be used by different PRU consumer
>>> or application nodes and supported by the PRU remoteproc driver.
>>> These are used to configure the PRU hardware for specific user
>>> applications.
>>>
>>> The application nodes themselves should define their own bindings.
>>>
>>> Co-developed-by: Tero Kristo <t-kristo@ti.com>
>>> Signed-off-by: Tero Kristo <t-kristo@ti.com>
>>> Signed-off-by: Suman Anna <s-anna@ti.com>
>>> Co-developed-by: Grzegorz Jaszczyk <grzegorz.jaszczyk@linaro.org>
>>> Signed-off-by: Grzegorz Jaszczyk <grzegorz.jaszczyk@linaro.org>
>>> Signed-off-by: Puranjay Mohan <p-mohan@ti.com>
>>> ---
>>>  .../bindings/remoteproc/ti,pru-consumer.yaml  | 70 +++++++++++++++++++
>>>  1 file changed, 70 insertions(+)
>>>  create mode 100644 Documentation/devicetree/bindings/remoteproc/ti,pru-consumer.yaml
>>>
>>> diff --git a/Documentation/devicetree/bindings/remoteproc/ti,pru-consumer.yaml b/Documentation/devicetree/bindings/remoteproc/ti,pru-consumer.yaml
>>> new file mode 100644
>>> index 000000000000..5b1f1cb2f098
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/remoteproc/ti,pru-consumer.yaml
>>> @@ -0,0 +1,70 @@
>>> +# SPDX-License-Identifier: (GPL-2.0-only or BSD-2-Clause)
>>> +%YAML 1.2
>>> +---
>>> +$id: http://devicetree.org/schemas/remoteproc/ti,pru-consumer.yaml#
>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>> +
>>> +title: Common TI PRU Consumer Binding
>>> +
>>> +maintainers:
>>> +  - Suman Anna <s-anna@ti.com>
>>> +
>>> +description: |
>>> +  A PRU application/consumer/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 define
>>> +  some properties needed for hardware/firmware configuration. The below
>>> +  properties are a list of common properties supported by the PRU remoteproc
>>> +  infrastructure.
>>> +
>>> +  The application nodes shall define their own bindings like regular platform
>>> +  devices, so below are in addition to each node's bindings.
>>> +
>>> +properties:
>>> +  ti,prus:
>>> +    $ref: /schemas/types.yaml#/definitions/phandle-array
>>> +    description: phandles to the PRU, RTU or Tx_PRU nodes used
>>> +    minItems: 1
>>> +    maxItems: 6
>>> +    items:
>>> +      maxItems: 1
>>> +
>>> +  firmware-name:
>>> +    $ref: /schemas/types.yaml#/definitions/string-array
>>> +    description: |
>>> +      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 'ti,prus' property
>>
>> So should be the name number of entries?:
>>
>> minItems: 1
>> maxItems: 6
> 
> will add in v4
> 
>>
>>> +
>>> +  ti,pruss-gp-mux-sel:
>>> +    $ref: /schemas/types.yaml#/definitions/uint32-array
>>
>> minItems: 1
> 
> will add in v4
> 
>>
>>> +    maxItems: 6
>>> +    items:
>>> +      enum: [0, 1, 2, 3, 4]
>>> +    description: |
>>> +      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. Values
>>> +      should correspond to the PRU cores listed in the 'ti,prus' property. The
>>> +      GP_MUX_SEL setting is a per-slice setting (one setting for PRU0, RTU0,
>>> +      and Tx_PRU0 on K3 SoCs). Use the same value for all cores within the
>>> +      same slice in the associative array. If the array size is smaller than
>>> +      the size of 'ti,prus' property, the default out-of-reset value (0) for the
>>> +      PRU core is used.
>>> +
>>> +required:
>>> +  - ti,prus
>>> +
>>> +dependencies:
>>> +  firmware-name: [ 'ti,prus' ]
>>> +  ti,pruss-gp-mux-sel: [ 'ti,prus' ]
>>
>> This doesn't make sense because 'ti,prus' is already required. Should 
>> all 3 properties always be required?
> 
> All three of these are always required, so, I will remove the

Are you sure? It should not be required and remoteproc driver should use
default name if not provided in DT.
In patch 5 see what is being done in pru_rproc_get().
It doesn't error out if firmware-name is not provided.

Same for ti,pruss-gp-mux-sel. Did you miss the patch that adds support for this
in this series?

> "dependencies:" tag and add all three of them to "required:" in v4
> Will it be the correct way to do it?
> 
>>
>>> +
>>> +additionalProperties: true
>>> +
>>> +examples:
>>> +  - |
>>> +    /* PRU application node example */
>>> +    pru-app {
>>> +        ti,prus = <&pru0>, <&pru1>;
>>> +        firmware-name = "pruss-app-fw0", "pruss-app-fw1";
>>> +        ti,pruss-gp-mux-sel = <2>, <1>;
>>
>> This example never validates, but okay I guess.
>>
>>> +    };
>>> -- 
>>> 2.17.1
>>>
>>>
> 
> Thanks,
> Puranjay Mohan

cheers,
-roger
Puranjay Mohan June 3, 2022, 10:14 a.m. UTC | #4
Hi Roger,

On 03/06/22 13:41, Roger Quadros wrote:
> Hi Puranjay,
> 
> On 02/06/2022 08:28, Puranjay Mohan wrote:
>> Hi Rob,
>>
>> On 03/05/22 01:26, Rob Herring wrote:
>>> On Mon, Apr 18, 2022 at 04:11:14PM +0530, Puranjay Mohan wrote:
>>>> From: Suman Anna <s-anna@ti.com>
>>>>
>>>> Add a YAML binding document for PRU consumers. The binding includes
>>>> all the common properties that can be used by different PRU consumer
>>>> or application nodes and supported by the PRU remoteproc driver.
>>>> These are used to configure the PRU hardware for specific user
>>>> applications.
>>>>
>>>> The application nodes themselves should define their own bindings.
>>>>
>>>> Co-developed-by: Tero Kristo <t-kristo@ti.com>
>>>> Signed-off-by: Tero Kristo <t-kristo@ti.com>
>>>> Signed-off-by: Suman Anna <s-anna@ti.com>
>>>> Co-developed-by: Grzegorz Jaszczyk <grzegorz.jaszczyk@linaro.org>
>>>> Signed-off-by: Grzegorz Jaszczyk <grzegorz.jaszczyk@linaro.org>
>>>> Signed-off-by: Puranjay Mohan <p-mohan@ti.com>
>>>> ---
>>>>  .../bindings/remoteproc/ti,pru-consumer.yaml  | 70 +++++++++++++++++++
>>>>  1 file changed, 70 insertions(+)
>>>>  create mode 100644 Documentation/devicetree/bindings/remoteproc/ti,pru-consumer.yaml
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/remoteproc/ti,pru-consumer.yaml b/Documentation/devicetree/bindings/remoteproc/ti,pru-consumer.yaml
>>>> new file mode 100644
>>>> index 000000000000..5b1f1cb2f098
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/remoteproc/ti,pru-consumer.yaml
>>>> @@ -0,0 +1,70 @@
>>>> +# SPDX-License-Identifier: (GPL-2.0-only or BSD-2-Clause)
>>>> +%YAML 1.2
>>>> +---
>>>> +$id: http://devicetree.org/schemas/remoteproc/ti,pru-consumer.yaml#
>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>>> +
>>>> +title: Common TI PRU Consumer Binding
>>>> +
>>>> +maintainers:
>>>> +  - Suman Anna <s-anna@ti.com>
>>>> +
>>>> +description: |
>>>> +  A PRU application/consumer/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 define
>>>> +  some properties needed for hardware/firmware configuration. The below
>>>> +  properties are a list of common properties supported by the PRU remoteproc
>>>> +  infrastructure.
>>>> +
>>>> +  The application nodes shall define their own bindings like regular platform
>>>> +  devices, so below are in addition to each node's bindings.
>>>> +
>>>> +properties:
>>>> +  ti,prus:
>>>> +    $ref: /schemas/types.yaml#/definitions/phandle-array
>>>> +    description: phandles to the PRU, RTU or Tx_PRU nodes used
>>>> +    minItems: 1
>>>> +    maxItems: 6
>>>> +    items:
>>>> +      maxItems: 1
>>>> +
>>>> +  firmware-name:
>>>> +    $ref: /schemas/types.yaml#/definitions/string-array
>>>> +    description: |
>>>> +      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 'ti,prus' property
>>>
>>> So should be the name number of entries?:
>>>
>>> minItems: 1
>>> maxItems: 6
>>
>> will add in v4
>>
>>>
>>>> +
>>>> +  ti,pruss-gp-mux-sel:
>>>> +    $ref: /schemas/types.yaml#/definitions/uint32-array
>>>
>>> minItems: 1
>>
>> will add in v4
>>
>>>
>>>> +    maxItems: 6
>>>> +    items:
>>>> +      enum: [0, 1, 2, 3, 4]
>>>> +    description: |
>>>> +      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. Values
>>>> +      should correspond to the PRU cores listed in the 'ti,prus' property. The
>>>> +      GP_MUX_SEL setting is a per-slice setting (one setting for PRU0, RTU0,
>>>> +      and Tx_PRU0 on K3 SoCs). Use the same value for all cores within the
>>>> +      same slice in the associative array. If the array size is smaller than
>>>> +      the size of 'ti,prus' property, the default out-of-reset value (0) for the
>>>> +      PRU core is used.
>>>> +
>>>> +required:
>>>> +  - ti,prus
>>>> +
>>>> +dependencies:
>>>> +  firmware-name: [ 'ti,prus' ]
>>>> +  ti,pruss-gp-mux-sel: [ 'ti,prus' ]
>>>
>>> This doesn't make sense because 'ti,prus' is already required. Should 
>>> all 3 properties always be required?
>>
>> All three of these are always required, so, I will remove the
> 
> Are you sure? It should not be required and remoteproc driver should use
> default name if not provided in DT.
> In patch 5 see what is being done in pru_rproc_get().
> It doesn't error out if firmware-name is not provided.

Yes, you are right, I missed this part.
So, only 'ti,prus' is required and 'firmware-name',
'ti,pruss-gp-mux-sel' are optional but are dependent on 'ti,prus'

So, as 'ti,prus' is always required we don't need the dependencies tag
to show that 'firmware-name', 'ti,pruss-gp-mux-sel' are dependent on
'ti,prus'

So, the change that I will be making in v4 is the removal of the
dependencies tag. This seems right?

> 
> Same for ti,pruss-gp-mux-sel. Did you miss the patch that adds support for this
> in this series?

Yes, the patch you are referring to was not a part of v2 so I didn't add
it in v3. I will add it in v4 now as it make more sense to add it with
the series.

> 
>> "dependencies:" tag and add all three of them to "required:" in v4
>> Will it be the correct way to do it?
>>
>>>
>>>> +
>>>> +additionalProperties: true
>>>> +
>>>> +examples:
>>>> +  - |
>>>> +    /* PRU application node example */
>>>> +    pru-app {
>>>> +        ti,prus = <&pru0>, <&pru1>;
>>>> +        firmware-name = "pruss-app-fw0", "pruss-app-fw1";
>>>> +        ti,pruss-gp-mux-sel = <2>, <1>;
>>>
>>> This example never validates, but okay I guess.
>>>
>>>> +    };
>>>> -- 
>>>> 2.17.1
>>>>
>>>>
>>
>> Thanks,
>> Puranjay Mohan
> 
> cheers,
> -roger

Thanks,
Puranjay Mohan
Roger Quadros June 3, 2022, 10:46 a.m. UTC | #5
On 03/06/2022 13:14, Puranjay Mohan wrote:
> Hi Roger,
> 
> On 03/06/22 13:41, Roger Quadros wrote:
>> Hi Puranjay,
>>
>> On 02/06/2022 08:28, Puranjay Mohan wrote:
>>> Hi Rob,
>>>
>>> On 03/05/22 01:26, Rob Herring wrote:
>>>> On Mon, Apr 18, 2022 at 04:11:14PM +0530, Puranjay Mohan wrote:
>>>>> From: Suman Anna <s-anna@ti.com>
>>>>>
>>>>> Add a YAML binding document for PRU consumers. The binding includes
>>>>> all the common properties that can be used by different PRU consumer
>>>>> or application nodes and supported by the PRU remoteproc driver.
>>>>> These are used to configure the PRU hardware for specific user
>>>>> applications.
>>>>>
>>>>> The application nodes themselves should define their own bindings.
>>>>>
>>>>> Co-developed-by: Tero Kristo <t-kristo@ti.com>
>>>>> Signed-off-by: Tero Kristo <t-kristo@ti.com>
>>>>> Signed-off-by: Suman Anna <s-anna@ti.com>
>>>>> Co-developed-by: Grzegorz Jaszczyk <grzegorz.jaszczyk@linaro.org>
>>>>> Signed-off-by: Grzegorz Jaszczyk <grzegorz.jaszczyk@linaro.org>
>>>>> Signed-off-by: Puranjay Mohan <p-mohan@ti.com>
>>>>> ---
>>>>>  .../bindings/remoteproc/ti,pru-consumer.yaml  | 70 +++++++++++++++++++
>>>>>  1 file changed, 70 insertions(+)
>>>>>  create mode 100644 Documentation/devicetree/bindings/remoteproc/ti,pru-consumer.yaml
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/remoteproc/ti,pru-consumer.yaml b/Documentation/devicetree/bindings/remoteproc/ti,pru-consumer.yaml
>>>>> new file mode 100644
>>>>> index 000000000000..5b1f1cb2f098
>>>>> --- /dev/null
>>>>> +++ b/Documentation/devicetree/bindings/remoteproc/ti,pru-consumer.yaml
>>>>> @@ -0,0 +1,70 @@
>>>>> +# SPDX-License-Identifier: (GPL-2.0-only or BSD-2-Clause)
>>>>> +%YAML 1.2
>>>>> +---
>>>>> +$id: http://devicetree.org/schemas/remoteproc/ti,pru-consumer.yaml#
>>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>>>> +
>>>>> +title: Common TI PRU Consumer Binding
>>>>> +
>>>>> +maintainers:
>>>>> +  - Suman Anna <s-anna@ti.com>
>>>>> +
>>>>> +description: |
>>>>> +  A PRU application/consumer/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 define
>>>>> +  some properties needed for hardware/firmware configuration. The below
>>>>> +  properties are a list of common properties supported by the PRU remoteproc
>>>>> +  infrastructure.
>>>>> +
>>>>> +  The application nodes shall define their own bindings like regular platform
>>>>> +  devices, so below are in addition to each node's bindings.
>>>>> +
>>>>> +properties:
>>>>> +  ti,prus:
>>>>> +    $ref: /schemas/types.yaml#/definitions/phandle-array
>>>>> +    description: phandles to the PRU, RTU or Tx_PRU nodes used
>>>>> +    minItems: 1
>>>>> +    maxItems: 6
>>>>> +    items:
>>>>> +      maxItems: 1
>>>>> +
>>>>> +  firmware-name:
>>>>> +    $ref: /schemas/types.yaml#/definitions/string-array
>>>>> +    description: |
>>>>> +      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 'ti,prus' property
>>>>
>>>> So should be the name number of entries?:
>>>>
>>>> minItems: 1
>>>> maxItems: 6
>>>
>>> will add in v4
>>>
>>>>
>>>>> +
>>>>> +  ti,pruss-gp-mux-sel:
>>>>> +    $ref: /schemas/types.yaml#/definitions/uint32-array
>>>>
>>>> minItems: 1
>>>
>>> will add in v4
>>>
>>>>
>>>>> +    maxItems: 6
>>>>> +    items:
>>>>> +      enum: [0, 1, 2, 3, 4]
>>>>> +    description: |
>>>>> +      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. Values
>>>>> +      should correspond to the PRU cores listed in the 'ti,prus' property. The
>>>>> +      GP_MUX_SEL setting is a per-slice setting (one setting for PRU0, RTU0,
>>>>> +      and Tx_PRU0 on K3 SoCs). Use the same value for all cores within the
>>>>> +      same slice in the associative array. If the array size is smaller than
>>>>> +      the size of 'ti,prus' property, the default out-of-reset value (0) for the
>>>>> +      PRU core is used.
>>>>> +
>>>>> +required:
>>>>> +  - ti,prus
>>>>> +
>>>>> +dependencies:
>>>>> +  firmware-name: [ 'ti,prus' ]
>>>>> +  ti,pruss-gp-mux-sel: [ 'ti,prus' ]
>>>>
>>>> This doesn't make sense because 'ti,prus' is already required. Should 
>>>> all 3 properties always be required?
>>>
>>> All three of these are always required, so, I will remove the
>>
>> Are you sure? It should not be required and remoteproc driver should use
>> default name if not provided in DT.
>> In patch 5 see what is being done in pru_rproc_get().
>> It doesn't error out if firmware-name is not provided.
> 
> Yes, you are right, I missed this part.
> So, only 'ti,prus' is required and 'firmware-name',
> 'ti,pruss-gp-mux-sel' are optional but are dependent on 'ti,prus'
> 
> So, as 'ti,prus' is always required we don't need the dependencies tag
> to show that 'firmware-name', 'ti,pruss-gp-mux-sel' are dependent on
> 'ti,prus'
> 
> So, the change that I will be making in v4 is the removal of the
> dependencies tag. This seems right?

Look good to me.
> 
>>
>> Same for ti,pruss-gp-mux-sel. Did you miss the patch that adds support for this
>> in this series?
> 
> Yes, the patch you are referring to was not a part of v2 so I didn't add
> it in v3. I will add it in v4 now as it make more sense to add it with
> the series.

Alright. Thanks!

> 
>>
>>> "dependencies:" tag and add all three of them to "required:" in v4
>>> Will it be the correct way to do it?
>>>
>>>>
>>>>> +
>>>>> +additionalProperties: true
>>>>> +
>>>>> +examples:
>>>>> +  - |
>>>>> +    /* PRU application node example */
>>>>> +    pru-app {
>>>>> +        ti,prus = <&pru0>, <&pru1>;
>>>>> +        firmware-name = "pruss-app-fw0", "pruss-app-fw1";
>>>>> +        ti,pruss-gp-mux-sel = <2>, <1>;
>>>>
>>>> This example never validates, but okay I guess.
>>>>
>>>>> +    };
>>>>> -- 
>>>>> 2.17.1
>>>>>
>>>>>
>>>
>>> Thanks,
>>> Puranjay Mohan
>>
>> cheers,
>> -roger
> 
> Thanks,
> Puranjay Mohan

cheers,
-roger
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/remoteproc/ti,pru-consumer.yaml b/Documentation/devicetree/bindings/remoteproc/ti,pru-consumer.yaml
new file mode 100644
index 000000000000..5b1f1cb2f098
--- /dev/null
+++ b/Documentation/devicetree/bindings/remoteproc/ti,pru-consumer.yaml
@@ -0,0 +1,70 @@ 
+# SPDX-License-Identifier: (GPL-2.0-only or BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/remoteproc/ti,pru-consumer.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Common TI PRU Consumer Binding
+
+maintainers:
+  - Suman Anna <s-anna@ti.com>
+
+description: |
+  A PRU application/consumer/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 define
+  some properties needed for hardware/firmware configuration. The below
+  properties are a list of common properties supported by the PRU remoteproc
+  infrastructure.
+
+  The application nodes shall define their own bindings like regular platform
+  devices, so below are in addition to each node's bindings.
+
+properties:
+  ti,prus:
+    $ref: /schemas/types.yaml#/definitions/phandle-array
+    description: phandles to the PRU, RTU or Tx_PRU nodes used
+    minItems: 1
+    maxItems: 6
+    items:
+      maxItems: 1
+
+  firmware-name:
+    $ref: /schemas/types.yaml#/definitions/string-array
+    description: |
+      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 'ti,prus' property
+
+  ti,pruss-gp-mux-sel:
+    $ref: /schemas/types.yaml#/definitions/uint32-array
+    maxItems: 6
+    items:
+      enum: [0, 1, 2, 3, 4]
+    description: |
+      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. Values
+      should correspond to the PRU cores listed in the 'ti,prus' property. The
+      GP_MUX_SEL setting is a per-slice setting (one setting for PRU0, RTU0,
+      and Tx_PRU0 on K3 SoCs). Use the same value for all cores within the
+      same slice in the associative array. If the array size is smaller than
+      the size of 'ti,prus' property, the default out-of-reset value (0) for the
+      PRU core is used.
+
+required:
+  - ti,prus
+
+dependencies:
+  firmware-name: [ 'ti,prus' ]
+  ti,pruss-gp-mux-sel: [ 'ti,prus' ]
+
+additionalProperties: true
+
+examples:
+  - |
+    /* PRU application node example */
+    pru-app {
+        ti,prus = <&pru0>, <&pru1>;
+        firmware-name = "pruss-app-fw0", "pruss-app-fw1";
+        ti,pruss-gp-mux-sel = <2>, <1>;
+    };