diff mbox series

dt-bindings: remoteproc: pru: Add Interrupt property

Message ID 20230807110836.2612730-1-danishanwar@ti.com
State Superseded
Headers show
Series dt-bindings: remoteproc: pru: Add Interrupt property | expand

Commit Message

MD Danish Anwar Aug. 7, 2023, 11:08 a.m. UTC
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(+)

Comments

Anwar, Md Danish Aug. 8, 2023, 9:44 a.m. UTC | #1
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
>>
Krzysztof Kozlowski Aug. 8, 2023, 10:18 a.m. UTC | #2
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
Conor Dooley Aug. 8, 2023, 10:48 a.m. UTC | #3
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.
Anwar, Md Danish Aug. 8, 2023, 10:57 a.m. UTC | #4
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
Anwar, Md Danish Aug. 8, 2023, 11 a.m. UTC | #5
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.
Conor Dooley Aug. 8, 2023, 11:28 a.m. UTC | #6
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.
Anwar, Md Danish Aug. 8, 2023, 12:52 p.m. UTC | #7
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.
Krzysztof Kozlowski Aug. 8, 2023, 2:53 p.m. UTC | #8
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
Anwar, Md Danish Aug. 11, 2023, 11:18 a.m. UTC | #9
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.
Conor Dooley Aug. 11, 2023, 3:21 p.m. UTC | #10
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.
Anwar, Md Danish Aug. 14, 2023, 5:13 a.m. UTC | #11
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
Conor Dooley Aug. 14, 2023, 6:59 a.m. UTC | #12
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
diff mbox series

Patch

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 {