| Message ID | 20230807110836.2612730-1-danishanwar@ti.com |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | dt-bindings: remoteproc: pru: Add Interrupt property | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success | |
| robh/patch-applied | success | |
| robh/dtbs-check | warning | build log |
| robh/dt-meta-schema | success |
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 >
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 >>
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
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.
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
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.
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.
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.
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
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.
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.
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
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 --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 {
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(+)