[PATCH] dt-bindings: remoteproc: pru: Add Interrupt property

[PATCH] dt-bindings: remoteproc: pru: Add Interrupt property

[MD Danish Anwar]

Add interrupts and interrupt-names protperties for PRU and RTU cores.

Signed-off-by: MD Danish Anwar <danishanwar@ti.com>
---
 .../bindings/remoteproc/ti,pru-rproc.yaml     | 22 +++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/Documentation/devicetree/bindings/remoteproc/ti,pru-rproc.yaml b/Documentation/devicetree/bindings/remoteproc/ti,pru-rproc.yaml
index cd55d80137f7..6970316943bb 100644
--- a/Documentation/devicetree/bindings/remoteproc/ti,pru-rproc.yaml
+++ b/Documentation/devicetree/bindings/remoteproc/ti,pru-rproc.yaml
@@ -66,6 +66,16 @@ properties:
       Should contain the name of the default firmware image
       file located on the firmware search path.
 
+  interrupts:
+    maxItems: 1
+    description:
+      Interrupt specifiers enable the virtio/rpmsg communication between MPU
+      and the PRU/RTU cores.
+
+  interrupt-names:
+    items:
+      - const: vring
+
 if:
   properties:
     compatible:
@@ -171,6 +181,9 @@ examples:
               <0x22400 0x100>;
         reg-names = "iram", "control", "debug";
         firmware-name = "am65x-pru0_0-fw";
+        interrupt-parent = <&icssg0_intc>;
+        interrupts = <16 2 2>;
+        interrupt-names = "vring";
       };
 
       rtu0_0: rtu@4000 {
@@ -180,6 +193,9 @@ examples:
               <0x23400 0x100>;
         reg-names = "iram", "control", "debug";
         firmware-name = "am65x-rtu0_0-fw";
+        interrupt-parent = <&icssg0_intc>;
+        interrupts = <20 4 4>;
+        interrupt-names = "vring";
       };
 
       tx_pru0_0: txpru@a000 {
@@ -198,6 +214,9 @@ examples:
               <0x24400 0x100>;
         reg-names = "iram", "control", "debug";
         firmware-name = "am65x-pru0_1-fw";
+        interrupt-parent = <&icssg0_intc>;
+        interrupts = <18 3 3>;
+        interrupt-names = "vring";
       };
 
       rtu0_1: rtu@6000 {
@@ -207,6 +226,9 @@ examples:
               <0x23c00 0x100>;
         reg-names = "iram", "control", "debug";
         firmware-name = "am65x-rtu0_1-fw";
+        interrupt-parent = <&icssg0_intc>;
+        interrupts = <22 5 5>;
+        interrupt-names = "vring";
       };
 
       tx_pru0_1: txpru@c000 {
-- 
2.34.1

Re: [PATCH] dt-bindings: remoteproc: pru: Add Interrupt property

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 2564 bytes --]

On Mon, Aug 07, 2023 at 04:38:36PM +0530, MD Danish Anwar wrote:
> Add interrupts and interrupt-names protperties for PRU and RTU cores.
> 
> Signed-off-by: MD Danish Anwar <danishanwar@ti.com>
> ---
>  .../bindings/remoteproc/ti,pru-rproc.yaml     | 22 +++++++++++++++++++
>  1 file changed, 22 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/remoteproc/ti,pru-rproc.yaml b/Documentation/devicetree/bindings/remoteproc/ti,pru-rproc.yaml
> index cd55d80137f7..6970316943bb 100644
> --- a/Documentation/devicetree/bindings/remoteproc/ti,pru-rproc.yaml
> +++ b/Documentation/devicetree/bindings/remoteproc/ti,pru-rproc.yaml
> @@ -66,6 +66,16 @@ properties:
>        Should contain the name of the default firmware image
>        file located on the firmware search path.
>  
> +  interrupts:
> +    maxItems: 1
> +    description:
> +      Interrupt specifiers enable the virtio/rpmsg communication between MPU
> +      and the PRU/RTU cores.
> +
> +  interrupt-names:
> +    items:
> +      - const: vring
> +
>  if:
>    properties:
>      compatible:
> @@ -171,6 +181,9 @@ examples:
>                <0x22400 0x100>;
>          reg-names = "iram", "control", "debug";
>          firmware-name = "am65x-pru0_0-fw";
> +        interrupt-parent = <&icssg0_intc>;
> +        interrupts = <16 2 2>;
> +        interrupt-names = "vring";
>        };

These examples would probably be more helpful if they used the
appropriate defines, no?

>  
>        rtu0_0: rtu@4000 {
> @@ -180,6 +193,9 @@ examples:
>                <0x23400 0x100>;
>          reg-names = "iram", "control", "debug";
>          firmware-name = "am65x-rtu0_0-fw";
> +        interrupt-parent = <&icssg0_intc>;
> +        interrupts = <20 4 4>;
> +        interrupt-names = "vring";
>        };
>  
>        tx_pru0_0: txpru@a000 {
> @@ -198,6 +214,9 @@ examples:
>                <0x24400 0x100>;
>          reg-names = "iram", "control", "debug";
>          firmware-name = "am65x-pru0_1-fw";
> +        interrupt-parent = <&icssg0_intc>;
> +        interrupts = <18 3 3>;
> +        interrupt-names = "vring";
>        };
>  
>        rtu0_1: rtu@6000 {
> @@ -207,6 +226,9 @@ examples:
>                <0x23c00 0x100>;
>          reg-names = "iram", "control", "debug";
>          firmware-name = "am65x-rtu0_1-fw";
> +        interrupt-parent = <&icssg0_intc>;
> +        interrupts = <22 5 5>;
> +        interrupt-names = "vring";
>        };
>  
>        tx_pru0_1: txpru@c000 {
> -- 
> 2.34.1
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH] dt-bindings: remoteproc: pru: Add Interrupt property

[Md Danish Anwar]

Hi Conor,

On 07/08/23 8:09 pm, Conor Dooley wrote:
> On Mon, Aug 07, 2023 at 04:38:36PM +0530, MD Danish Anwar wrote:
>> Add interrupts and interrupt-names protperties for PRU and RTU cores.
>>
>> Signed-off-by: MD Danish Anwar <danishanwar@ti.com>
>> ---
>>  .../bindings/remoteproc/ti,pru-rproc.yaml     | 22 +++++++++++++++++++
>>  1 file changed, 22 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/remoteproc/ti,pru-rproc.yaml b/Documentation/devicetree/bindings/remoteproc/ti,pru-rproc.yaml
>> index cd55d80137f7..6970316943bb 100644
>> --- a/Documentation/devicetree/bindings/remoteproc/ti,pru-rproc.yaml
>> +++ b/Documentation/devicetree/bindings/remoteproc/ti,pru-rproc.yaml
>> @@ -66,6 +66,16 @@ properties:
>>        Should contain the name of the default firmware image
>>        file located on the firmware search path.
>>  
>> +  interrupts:
>> +    maxItems: 1
>> +    description:
>> +      Interrupt specifiers enable the virtio/rpmsg communication between MPU
>> +      and the PRU/RTU cores.
>> +
>> +  interrupt-names:
>> +    items:
>> +      - const: vring
>> +
>>  if:
>>    properties:
>>      compatible:
>> @@ -171,6 +181,9 @@ examples:
>>                <0x22400 0x100>;
>>          reg-names = "iram", "control", "debug";
>>          firmware-name = "am65x-pru0_0-fw";
>> +        interrupt-parent = <&icssg0_intc>;
>> +        interrupts = <16 2 2>;
>> +        interrupt-names = "vring";
>>        };
> 
> These examples would probably be more helpful if they used the
> appropriate defines, no?
> 

PRUSS Interrupt controller doesn't have any appropriate defines. This doesn't
use GIC so defines from arm-gic.h can not be used here. These are specific to
PRUSS INTC.

I think these example are OK. Please let me know if this is OK to you.

>>  
>>        rtu0_0: rtu@4000 {
>> @@ -180,6 +193,9 @@ examples:
>>                <0x23400 0x100>;
>>          reg-names = "iram", "control", "debug";
>>          firmware-name = "am65x-rtu0_0-fw";
>> +        interrupt-parent = <&icssg0_intc>;
>> +        interrupts = <20 4 4>;
>> +        interrupt-names = "vring";
>>        };
>>  
>>        tx_pru0_0: txpru@a000 {
>> @@ -198,6 +214,9 @@ examples:
>>                <0x24400 0x100>;
>>          reg-names = "iram", "control", "debug";
>>          firmware-name = "am65x-pru0_1-fw";
>> +        interrupt-parent = <&icssg0_intc>;
>> +        interrupts = <18 3 3>;
>> +        interrupt-names = "vring";
>>        };
>>  
>>        rtu0_1: rtu@6000 {
>> @@ -207,6 +226,9 @@ examples:
>>                <0x23c00 0x100>;
>>          reg-names = "iram", "control", "debug";
>>          firmware-name = "am65x-rtu0_1-fw";
>> +        interrupt-parent = <&icssg0_intc>;
>> +        interrupts = <22 5 5>;
>> +        interrupt-names = "vring";
>>        };
>>  
>>        tx_pru0_1: txpru@c000 {
>> -- 
>> 2.34.1
>>

-- 
Thanks and Regards,
Danish.

Re: [PATCH] dt-bindings: remoteproc: pru: Add Interrupt property

[Krzysztof Kozlowski]

On 08/08/2023 11:44, Md Danish Anwar wrote:
>>>    properties:
>>>      compatible:
>>> @@ -171,6 +181,9 @@ examples:
>>>                <0x22400 0x100>;
>>>          reg-names = "iram", "control", "debug";
>>>          firmware-name = "am65x-pru0_0-fw";
>>> +        interrupt-parent = <&icssg0_intc>;
>>> +        interrupts = <16 2 2>;
>>> +        interrupt-names = "vring";
>>>        };
>>
>> These examples would probably be more helpful if they used the
>> appropriate defines, no?
>>
> 
> PRUSS Interrupt controller doesn't have any appropriate defines. This doesn't
> use GIC so defines from arm-gic.h can not be used here. These are specific to
> PRUSS INTC.
> 
> I think these example are OK. Please let me know if this is OK to you.

But isn't "2" type of the interrupt?

Best regards,
Krzysztof

Re: [PATCH] dt-bindings: remoteproc: pru: Add Interrupt property

[Md Danish Anwar]

On 08/08/23 3:48 pm, Krzysztof Kozlowski wrote:
> On 08/08/2023 11:44, Md Danish Anwar wrote:
>>>>    properties:
>>>>      compatible:
>>>> @@ -171,6 +181,9 @@ examples:
>>>>                <0x22400 0x100>;
>>>>          reg-names = "iram", "control", "debug";
>>>>          firmware-name = "am65x-pru0_0-fw";
>>>> +        interrupt-parent = <&icssg0_intc>;
>>>> +        interrupts = <16 2 2>;
>>>> +        interrupt-names = "vring";
>>>>        };
>>>
>>> These examples would probably be more helpful if they used the
>>> appropriate defines, no?
>>>
>>
>> PRUSS Interrupt controller doesn't have any appropriate defines. This doesn't
>> use GIC so defines from arm-gic.h can not be used here. These are specific to
>> PRUSS INTC.
>>
>> I think these example are OK. Please let me know if this is OK to you.
> 
> But isn't "2" type of the interrupt?
> 
> Best regards,
> Krzysztof
> 

As per the description of interrupts property in ti,pruss-intc.yaml [1]

Cell 1 is PRU System event number, cell 2 is PRU channel and cell 3 is PRU
host_event (target). None of them is type of interrupt. So that's why they all
are hardcoded. I don't think we can use IRQ_TYPE macros here.

[1]
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/Documentation/devicetree/bindings/interrupt-controller/ti,pruss-intc.yaml#:~:text=const%3A%203-,description%3A%20%7C,-Client%20users%20shall

-- 
Thanks and Regards,
Danish.

Re: [PATCH] dt-bindings: remoteproc: pru: Add Interrupt property

[Krzysztof Kozlowski]

On 08/08/2023 12:57, Md Danish Anwar wrote:
> On 08/08/23 3:48 pm, Krzysztof Kozlowski wrote:
>> On 08/08/2023 11:44, Md Danish Anwar wrote:
>>>>>    properties:
>>>>>      compatible:
>>>>> @@ -171,6 +181,9 @@ examples:
>>>>>                <0x22400 0x100>;
>>>>>          reg-names = "iram", "control", "debug";
>>>>>          firmware-name = "am65x-pru0_0-fw";
>>>>> +        interrupt-parent = <&icssg0_intc>;
>>>>> +        interrupts = <16 2 2>;
>>>>> +        interrupt-names = "vring";
>>>>>        };
>>>>
>>>> These examples would probably be more helpful if they used the
>>>> appropriate defines, no?
>>>>
>>>
>>> PRUSS Interrupt controller doesn't have any appropriate defines. This doesn't
>>> use GIC so defines from arm-gic.h can not be used here. These are specific to
>>> PRUSS INTC.
>>>
>>> I think these example are OK. Please let me know if this is OK to you.
>>
>> But isn't "2" type of the interrupt?
>>
>> Best regards,
>> Krzysztof
>>
> 
> As per the description of interrupts property in ti,pruss-intc.yaml [1]
> 
> Cell 1 is PRU System event number, cell 2 is PRU channel and cell 3 is PRU
> host_event (target). None of them is type of interrupt. So that's why they all
> are hardcoded. I don't think we can use IRQ_TYPE macros here.

OK, thanks for clarifying this.

Best regards,
Krzysztof

Re: [PATCH] dt-bindings: remoteproc: pru: Add Interrupt property

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 2146 bytes --]

On Tue, Aug 08, 2023 at 03:14:31PM +0530, Md Danish Anwar wrote:
> Hi Conor,
> 
> On 07/08/23 8:09 pm, Conor Dooley wrote:
> > On Mon, Aug 07, 2023 at 04:38:36PM +0530, MD Danish Anwar wrote:
> >> Add interrupts and interrupt-names protperties for PRU and RTU cores.
> >>
> >> Signed-off-by: MD Danish Anwar <danishanwar@ti.com>
> >> ---
> >>  .../bindings/remoteproc/ti,pru-rproc.yaml     | 22 +++++++++++++++++++
> >>  1 file changed, 22 insertions(+)
> >>
> >> diff --git a/Documentation/devicetree/bindings/remoteproc/ti,pru-rproc.yaml b/Documentation/devicetree/bindings/remoteproc/ti,pru-rproc.yaml
> >> index cd55d80137f7..6970316943bb 100644
> >> --- a/Documentation/devicetree/bindings/remoteproc/ti,pru-rproc.yaml
> >> +++ b/Documentation/devicetree/bindings/remoteproc/ti,pru-rproc.yaml
> >> @@ -66,6 +66,16 @@ properties:
> >>        Should contain the name of the default firmware image
> >>        file located on the firmware search path.
> >>  
> >> +  interrupts:
> >> +    maxItems: 1
> >> +    description:
> >> +      Interrupt specifiers enable the virtio/rpmsg communication between MPU
> >> +      and the PRU/RTU cores.
> >> +
> >> +  interrupt-names:
> >> +    items:
> >> +      - const: vring
> >> +
> >>  if:
> >>    properties:
> >>      compatible:
> >> @@ -171,6 +181,9 @@ examples:
> >>                <0x22400 0x100>;
> >>          reg-names = "iram", "control", "debug";
> >>          firmware-name = "am65x-pru0_0-fw";
> >> +        interrupt-parent = <&icssg0_intc>;
> >> +        interrupts = <16 2 2>;
> >> +        interrupt-names = "vring";
> >>        };
> > 
> > These examples would probably be more helpful if they used the
> > appropriate defines, no?
> > 
> 
> PRUSS Interrupt controller doesn't have any appropriate defines. This doesn't
> use GIC so defines from arm-gic.h can not be used here. These are specific to
> PRUSS INTC.

I was deliberately vague in case the gic stuff applied too, but my main
question was about the standard defines used for interrupt types.

> I think these example are OK. Please let me know if this is OK to you.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH] dt-bindings: remoteproc: pru: Add Interrupt property

[Md Danish Anwar]

On 08/08/23 4:18 pm, Conor Dooley wrote:
> On Tue, Aug 08, 2023 at 03:14:31PM +0530, Md Danish Anwar wrote:
>> Hi Conor,
>>
>> On 07/08/23 8:09 pm, Conor Dooley wrote:
>>> On Mon, Aug 07, 2023 at 04:38:36PM +0530, MD Danish Anwar wrote:
>>>> Add interrupts and interrupt-names protperties for PRU and RTU cores.
>>>>
>>>> Signed-off-by: MD Danish Anwar <danishanwar@ti.com>
>>>> ---
>>>>  .../bindings/remoteproc/ti,pru-rproc.yaml     | 22 +++++++++++++++++++
>>>>  1 file changed, 22 insertions(+)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/remoteproc/ti,pru-rproc.yaml b/Documentation/devicetree/bindings/remoteproc/ti,pru-rproc.yaml
>>>> index cd55d80137f7..6970316943bb 100644
>>>> --- a/Documentation/devicetree/bindings/remoteproc/ti,pru-rproc.yaml
>>>> +++ b/Documentation/devicetree/bindings/remoteproc/ti,pru-rproc.yaml
>>>> @@ -66,6 +66,16 @@ properties:
>>>>        Should contain the name of the default firmware image
>>>>        file located on the firmware search path.
>>>>  
>>>> +  interrupts:
>>>> +    maxItems: 1
>>>> +    description:
>>>> +      Interrupt specifiers enable the virtio/rpmsg communication between MPU
>>>> +      and the PRU/RTU cores.
>>>> +
>>>> +  interrupt-names:
>>>> +    items:
>>>> +      - const: vring
>>>> +
>>>>  if:
>>>>    properties:
>>>>      compatible:
>>>> @@ -171,6 +181,9 @@ examples:
>>>>                <0x22400 0x100>;
>>>>          reg-names = "iram", "control", "debug";
>>>>          firmware-name = "am65x-pru0_0-fw";
>>>> +        interrupt-parent = <&icssg0_intc>;
>>>> +        interrupts = <16 2 2>;
>>>> +        interrupt-names = "vring";
>>>>        };
>>>
>>> These examples would probably be more helpful if they used the
>>> appropriate defines, no?
>>>
>>
>> PRUSS Interrupt controller doesn't have any appropriate defines. This doesn't
>> use GIC so defines from arm-gic.h can not be used here. These are specific to
>> PRUSS INTC.
> 
> I was deliberately vague in case the gic stuff applied too, but my main
> question was about the standard defines used for interrupt types.
> 

There are no standard defines for these interrupt types. However I can create a
new .h file defining all the three interrupt cells and their values for both
PRU and RTU cores if you think that is required. Otherwise we can go with
hardcoded values.

Please let me know what you think should be done here.

>> I think these example are OK. Please let me know if this is OK to you.

-- 
Thanks and Regards,
Danish.

Re: [PATCH] dt-bindings: remoteproc: pru: Add Interrupt property

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 2740 bytes --]

On Tue, Aug 08, 2023 at 04:30:32PM +0530, Md Danish Anwar wrote:
> On 08/08/23 4:18 pm, Conor Dooley wrote:
> > On Tue, Aug 08, 2023 at 03:14:31PM +0530, Md Danish Anwar wrote:
> >> On 07/08/23 8:09 pm, Conor Dooley wrote:
> >>> On Mon, Aug 07, 2023 at 04:38:36PM +0530, MD Danish Anwar wrote:
> >>>> Add interrupts and interrupt-names protperties for PRU and RTU cores.
> >>>>
> >>>> Signed-off-by: MD Danish Anwar <danishanwar@ti.com>
> >>>> ---
> >>>>  .../bindings/remoteproc/ti,pru-rproc.yaml     | 22 +++++++++++++++++++
> >>>>  1 file changed, 22 insertions(+)
> >>>>
> >>>> diff --git a/Documentation/devicetree/bindings/remoteproc/ti,pru-rproc.yaml b/Documentation/devicetree/bindings/remoteproc/ti,pru-rproc.yaml
> >>>> index cd55d80137f7..6970316943bb 100644
> >>>> --- a/Documentation/devicetree/bindings/remoteproc/ti,pru-rproc.yaml
> >>>> +++ b/Documentation/devicetree/bindings/remoteproc/ti,pru-rproc.yaml
> >>>> @@ -66,6 +66,16 @@ properties:
> >>>>        Should contain the name of the default firmware image
> >>>>        file located on the firmware search path.
> >>>>  
> >>>> +  interrupts:
> >>>> +    maxItems: 1
> >>>> +    description:
> >>>> +      Interrupt specifiers enable the virtio/rpmsg communication between MPU
> >>>> +      and the PRU/RTU cores.
> >>>> +
> >>>> +  interrupt-names:
> >>>> +    items:
> >>>> +      - const: vring
> >>>> +
> >>>>  if:
> >>>>    properties:
> >>>>      compatible:
> >>>> @@ -171,6 +181,9 @@ examples:
> >>>>                <0x22400 0x100>;
> >>>>          reg-names = "iram", "control", "debug";
> >>>>          firmware-name = "am65x-pru0_0-fw";
> >>>> +        interrupt-parent = <&icssg0_intc>;
> >>>> +        interrupts = <16 2 2>;
> >>>> +        interrupt-names = "vring";
> >>>>        };
> >>>
> >>> These examples would probably be more helpful if they used the
> >>> appropriate defines, no?
> >>>
> >>
> >> PRUSS Interrupt controller doesn't have any appropriate defines. This doesn't
> >> use GIC so defines from arm-gic.h can not be used here. These are specific to
> >> PRUSS INTC.
> > 
> > I was deliberately vague in case the gic stuff applied too, but my main
> > question was about the standard defines used for interrupt types.
> > 
> 
> There are no standard defines for these interrupt types. However I can create a
> new .h file defining all the three interrupt cells and their values for both
> PRU and RTU cores if you think that is required. Otherwise we can go with
> hardcoded values.
> 
> Please let me know what you think should be done here.

It'd be good to reference to the documentation for the cells, I don't
think adding a header is necessary here.

Thanks,
Conor.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH] dt-bindings: remoteproc: pru: Add Interrupt property

[Md Danish Anwar]

On 08/08/23 4:58 pm, Conor Dooley wrote:
> On Tue, Aug 08, 2023 at 04:30:32PM +0530, Md Danish Anwar wrote:
>> On 08/08/23 4:18 pm, Conor Dooley wrote:
>>> On Tue, Aug 08, 2023 at 03:14:31PM +0530, Md Danish Anwar wrote:
>>>> On 07/08/23 8:09 pm, Conor Dooley wrote:
>>>>> On Mon, Aug 07, 2023 at 04:38:36PM +0530, MD Danish Anwar wrote:
>>>>>> Add interrupts and interrupt-names protperties for PRU and RTU cores.
>>>>>>
>>>>>> Signed-off-by: MD Danish Anwar <danishanwar@ti.com>
>>>>>> ---
>>>>>>  .../bindings/remoteproc/ti,pru-rproc.yaml     | 22 +++++++++++++++++++
>>>>>>  1 file changed, 22 insertions(+)
>>>>>>
>>>>>> diff --git a/Documentation/devicetree/bindings/remoteproc/ti,pru-rproc.yaml b/Documentation/devicetree/bindings/remoteproc/ti,pru-rproc.yaml
>>>>>> index cd55d80137f7..6970316943bb 100644
>>>>>> --- a/Documentation/devicetree/bindings/remoteproc/ti,pru-rproc.yaml
>>>>>> +++ b/Documentation/devicetree/bindings/remoteproc/ti,pru-rproc.yaml
>>>>>> @@ -66,6 +66,16 @@ properties:
>>>>>>        Should contain the name of the default firmware image
>>>>>>        file located on the firmware search path.
>>>>>>  
>>>>>> +  interrupts:
>>>>>> +    maxItems: 1
>>>>>> +    description:
>>>>>> +      Interrupt specifiers enable the virtio/rpmsg communication between MPU
>>>>>> +      and the PRU/RTU cores.
>>>>>> +
>>>>>> +  interrupt-names:
>>>>>> +    items:
>>>>>> +      - const: vring
>>>>>> +
>>>>>>  if:
>>>>>>    properties:
>>>>>>      compatible:
>>>>>> @@ -171,6 +181,9 @@ examples:
>>>>>>                <0x22400 0x100>;
>>>>>>          reg-names = "iram", "control", "debug";
>>>>>>          firmware-name = "am65x-pru0_0-fw";
>>>>>> +        interrupt-parent = <&icssg0_intc>;
>>>>>> +        interrupts = <16 2 2>;
>>>>>> +        interrupt-names = "vring";
>>>>>>        };
>>>>>
>>>>> These examples would probably be more helpful if they used the
>>>>> appropriate defines, no?
>>>>>
>>>>
>>>> PRUSS Interrupt controller doesn't have any appropriate defines. This doesn't
>>>> use GIC so defines from arm-gic.h can not be used here. These are specific to
>>>> PRUSS INTC.
>>>
>>> I was deliberately vague in case the gic stuff applied too, but my main
>>> question was about the standard defines used for interrupt types.
>>>
>>
>> There are no standard defines for these interrupt types. However I can create a
>> new .h file defining all the three interrupt cells and their values for both
>> PRU and RTU cores if you think that is required. Otherwise we can go with
>> hardcoded values.
>>
>> Please let me know what you think should be done here.
> 
> It'd be good to reference to the documentation for the cells, I don't
> think adding a header is necessary here.

Sure. Then I would keep this as it is. the interrupt cell values will remain as
it is. No change required here then. Please let me know if any other change is
required in this patch.

> 
> Thanks,
> Conor.

-- 
Thanks and Regards,
Danish.

Re: [PATCH] dt-bindings: remoteproc: pru: Add Interrupt property

[Md Danish Anwar]

Hi Conor,

On 08/08/23 4:58 pm, Conor Dooley wrote:
> On Tue, Aug 08, 2023 at 04:30:32PM +0530, Md Danish Anwar wrote:
>> On 08/08/23 4:18 pm, Conor Dooley wrote:
>>> On Tue, Aug 08, 2023 at 03:14:31PM +0530, Md Danish Anwar wrote:
>>>> On 07/08/23 8:09 pm, Conor Dooley wrote:
>>>>> On Mon, Aug 07, 2023 at 04:38:36PM +0530, MD Danish Anwar wrote:
>>>>>> Add interrupts and interrupt-names protperties for PRU and RTU cores.
>>>>>>
>>>>>> Signed-off-by: MD Danish Anwar <danishanwar@ti.com>
>>>>>> ---
>>>>>>  .../bindings/remoteproc/ti,pru-rproc.yaml     | 22 +++++++++++++++++++
>>>>>>  1 file changed, 22 insertions(+)
>>>>>>
>>>>>> diff --git a/Documentation/devicetree/bindings/remoteproc/ti,pru-rproc.yaml b/Documentation/devicetree/bindings/remoteproc/ti,pru-rproc.yaml
>>>>>> index cd55d80137f7..6970316943bb 100644
>>>>>> --- a/Documentation/devicetree/bindings/remoteproc/ti,pru-rproc.yaml
>>>>>> +++ b/Documentation/devicetree/bindings/remoteproc/ti,pru-rproc.yaml
>>>>>> @@ -66,6 +66,16 @@ properties:
>>>>>>        Should contain the name of the default firmware image
>>>>>>        file located on the firmware search path.
>>>>>>  
>>>>>> +  interrupts:
>>>>>> +    maxItems: 1
>>>>>> +    description:
>>>>>> +      Interrupt specifiers enable the virtio/rpmsg communication between MPU
>>>>>> +      and the PRU/RTU cores.
>>>>>> +
>>>>>> +  interrupt-names:
>>>>>> +    items:
>>>>>> +      - const: vring
>>>>>> +
>>>>>>  if:
>>>>>>    properties:
>>>>>>      compatible:
>>>>>> @@ -171,6 +181,9 @@ examples:
>>>>>>                <0x22400 0x100>;
>>>>>>          reg-names = "iram", "control", "debug";
>>>>>>          firmware-name = "am65x-pru0_0-fw";
>>>>>> +        interrupt-parent = <&icssg0_intc>;
>>>>>> +        interrupts = <16 2 2>;
>>>>>> +        interrupt-names = "vring";
>>>>>>        };
>>>>>
>>>>> These examples would probably be more helpful if they used the
>>>>> appropriate defines, no?
>>>>>
>>>>
>>>> PRUSS Interrupt controller doesn't have any appropriate defines. This doesn't
>>>> use GIC so defines from arm-gic.h can not be used here. These are specific to
>>>> PRUSS INTC.
>>>
>>> I was deliberately vague in case the gic stuff applied too, but my main
>>> question was about the standard defines used for interrupt types.
>>>
>>
>> There are no standard defines for these interrupt types. However I can create a
>> new .h file defining all the three interrupt cells and their values for both
>> PRU and RTU cores if you think that is required. Otherwise we can go with
>> hardcoded values.
>>
>> Please let me know what you think should be done here.
> 
> It'd be good to reference to the documentation for the cells, I don't
> think adding a header is necessary here.
> 

How should I reference to the documentation for the cells?

Should I just add the details of cells in description of interrupt property here.

  interrupts:
    maxItems: 1
    description:
      Interrupt specifiers enable the virtio/rpmsg communication between MPU
      and the PRU/RTU cores. The value of the interrupts should be the PRU
      System event number [cell 1], PRU channel [cell 2] and PRU host_event
      (target) [cell 3].

Please let me know if this looks OK to you.


> Thanks,
> Conor.

-- 
Thanks and Regards,
Danish.

Re: [PATCH] dt-bindings: remoteproc: pru: Add Interrupt property

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 1168 bytes --]

On Fri, Aug 11, 2023 at 04:48:28PM +0530, Md Danish Anwar wrote:

> >> There are no standard defines for these interrupt types. However I can create a
> >> new .h file defining all the three interrupt cells and their values for both
> >> PRU and RTU cores if you think that is required. Otherwise we can go with
> >> hardcoded values.
> >>
> >> Please let me know what you think should be done here.
> > 
> > It'd be good to reference to the documentation for the cells, I don't
> > think adding a header is necessary here.
> > 
> 
> How should I reference to the documentation for the cells?
> 
> Should I just add the details of cells in description of interrupt property here.
> 
>   interrupts:
>     maxItems: 1
>     description:
>       Interrupt specifiers enable the virtio/rpmsg communication between MPU
>       and the PRU/RTU cores. The value of the interrupts should be the PRU
>       System event number [cell 1], PRU channel [cell 2] and PRU host_event
>       (target) [cell 3].
> 
> Please let me know if this looks OK to you.

I was thinking there'd be an binding for the interrupt controller that
you could mentioned.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH] dt-bindings: remoteproc: pru: Add Interrupt property

[Md Danish Anwar]

On 11/08/23 8:51 pm, Conor Dooley wrote:
> On Fri, Aug 11, 2023 at 04:48:28PM +0530, Md Danish Anwar wrote:
> 
>>>> There are no standard defines for these interrupt types. However I can create a
>>>> new .h file defining all the three interrupt cells and their values for both
>>>> PRU and RTU cores if you think that is required. Otherwise we can go with
>>>> hardcoded values.
>>>>
>>>> Please let me know what you think should be done here.
>>>
>>> It'd be good to reference to the documentation for the cells, I don't
>>> think adding a header is necessary here.
>>>
>>
>> How should I reference to the documentation for the cells?
>>
>> Should I just add the details of cells in description of interrupt property here.
>>
>>   interrupts:
>>     maxItems: 1
>>     description:
>>       Interrupt specifiers enable the virtio/rpmsg communication between MPU
>>       and the PRU/RTU cores. The value of the interrupts should be the PRU
>>       System event number [cell 1], PRU channel [cell 2] and PRU host_event
>>       (target) [cell 3].
>>
>> Please let me know if this looks OK to you.
> 
> I was thinking there'd be an binding for the interrupt controller that
> you could mentioned.

There is a binding for interrupt-controller [1] that I can mention. I tried using

- $ref: /schemas/interrupt-controller/ti,pruss-intc.yaml#

But it was throwing dt binding errors so I didn't add the ref.

I will mention this file name in the description of the property like below,

     description:
       Interrupt specifiers enable the virtio/rpmsg communication between MPU
       and the PRU/RTU cores. For the values of the interrupt cells please
       refer to interrupt-controller/ti,pruss-intc.yaml schema.

Please let me know if this looks OK to you.

[1]
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/interrupt-controller/ti,pruss-intc.yaml

-- 
Thanks and Regards,
Danish.

Re: [PATCH] dt-bindings: remoteproc: pru: Add Interrupt property

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 2060 bytes --]

On Mon, Aug 14, 2023 at 10:43:58AM +0530, Md Danish Anwar wrote:
> On 11/08/23 8:51 pm, Conor Dooley wrote:
> > On Fri, Aug 11, 2023 at 04:48:28PM +0530, Md Danish Anwar wrote:
> > 
> >>>> There are no standard defines for these interrupt types. However I can create a
> >>>> new .h file defining all the three interrupt cells and their values for both
> >>>> PRU and RTU cores if you think that is required. Otherwise we can go with
> >>>> hardcoded values.
> >>>>
> >>>> Please let me know what you think should be done here.
> >>>
> >>> It'd be good to reference to the documentation for the cells, I don't
> >>> think adding a header is necessary here.
> >>>
> >>
> >> How should I reference to the documentation for the cells?
> >>
> >> Should I just add the details of cells in description of interrupt property here.
> >>
> >>   interrupts:
> >>     maxItems: 1
> >>     description:
> >>       Interrupt specifiers enable the virtio/rpmsg communication between MPU
> >>       and the PRU/RTU cores. The value of the interrupts should be the PRU
> >>       System event number [cell 1], PRU channel [cell 2] and PRU host_event
> >>       (target) [cell 3].
> >>
> >> Please let me know if this looks OK to you.
> > 
> > I was thinking there'd be an binding for the interrupt controller that
> > you could mentioned.
> 
> There is a binding for interrupt-controller [1] that I can mention. I tried using
> 
> - $ref: /schemas/interrupt-controller/ti,pruss-intc.yaml#
> 
> But it was throwing dt binding errors so I didn't add the ref.

Yeah, you're not a pruss-itc so that makes sense.

> 
> I will mention this file name in the description of the property like below,
> 
>      description:
>        Interrupt specifiers enable the virtio/rpmsg communication between MPU
>        and the PRU/RTU cores. For the values of the interrupt cells please
>        refer to interrupt-controller/ti,pruss-intc.yaml schema.
> 
> Please let me know if this looks OK to you.

This is what I would've expected, yea

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]