| Message ID | 20200520124454.10532-5-lokeshvutla@ti.com |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | irqchip: ti,sci-intr/inta: Update the dt bindings to accept different interrupt parents | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success |
On Wed, May 20, 2020 at 06:14:46PM +0530, Lokesh Vutla wrote: > Drop the firmware related dt-bindings and use the hardware specified > interrupt numbers within Interrupt Router. This ensures interrupt router > DT node need not assume any interrupt parent type. I didn't like this binding to begin with, but now you're breaking compatibility. > Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com> > --- > .../interrupt-controller/ti,sci-intr.txt | 31 ++++++++++--------- > 1 file changed, 16 insertions(+), 15 deletions(-) > > diff --git a/Documentation/devicetree/bindings/interrupt-controller/ti,sci-intr.txt b/Documentation/devicetree/bindings/interrupt-controller/ti,sci-intr.txt > index 1a8718f8855d..8b56b2de1c73 100644 > --- a/Documentation/devicetree/bindings/interrupt-controller/ti,sci-intr.txt > +++ b/Documentation/devicetree/bindings/interrupt-controller/ti,sci-intr.txt > @@ -44,15 +44,17 @@ Required Properties: > 4: If intr supports level triggered interrupts. > - interrupt-controller: Identifies the node as an interrupt controller > - #interrupt-cells: Specifies the number of cells needed to encode an > - interrupt source. The value should be 2. > - First cell should contain the TISCI device ID of source > - Second cell should contain the interrupt source offset > - within the device. > + interrupt source. The value should be 1. > + First cell should contain interrupt router input number > + as specified by hardware. > - ti,sci: Phandle to TI-SCI compatible System controller node. > -- ti,sci-dst-id: TISCI device ID of the destination IRQ controller. > -- ti,sci-rm-range-girq: Array of TISCI subtype ids representing the host irqs > - assigned to this interrupt router. Each subtype id > - corresponds to a range of host irqs. > +- ti,sci-dev-id: TISCI device id of interrupt controller. > +- ti,interrupt-ranges: Set of triplets containing ranges that convert > + the INTR output interrupt numbers to parent's > + interrupt number. Each triplet has following entries: > + - First entry specifies the base for intr output irq > + - Second entry specifies the base for parent irqs > + - Third entry specifies the limit Humm, sounds like what interrupt-map does. > > For more details on TISCI IRQ resource management refer: > http://downloads.ti.com/tisci/esd/latest/2_tisci_msgs/rm/rm_irq.html > @@ -62,21 +64,20 @@ Example: > The following example demonstrates both interrupt router node and the consumer > node(main gpio) on the AM654 SoC: > > -main_intr: interrupt-controller0 { > +main_gpio_intr: interrupt-controller0 { > compatible = "ti,sci-intr"; > ti,intr-trigger-type = <1>; > interrupt-controller; > interrupt-parent = <&gic500>; > - #interrupt-cells = <2>; > + #interrupt-cells = <1>; > ti,sci = <&dmsc>; > - ti,sci-dst-id = <56>; > - ti,sci-rm-range-girq = <0x1>; > + ti,sci-dev-id = <131>; > + ti,interrupt-ranges = <0 360 32>; > }; > > main_gpio0: gpio@600000 { > ... > - interrupt-parent = <&main_intr>; > - interrupts = <57 256>, <57 257>, <57 258>, > - <57 259>, <57 260>, <57 261>; > + interrupt-parent = <&main_gpio_intr>; > + interrupts = <192>, <193>, <194>, <195>, <196>, <197>; > ... > }; > -- > 2.17.1 >
Hi Rob, On 29/05/20 3:44 am, Rob Herring wrote: > On Wed, May 20, 2020 at 06:14:46PM +0530, Lokesh Vutla wrote: >> Drop the firmware related dt-bindings and use the hardware specified >> interrupt numbers within Interrupt Router. This ensures interrupt router >> DT node need not assume any interrupt parent type. > > I didn't like this binding to begin with, but now you're breaking > compatibility. Yes, I do agree that this change is breaking backward compatibility. But IMHO, this does cleanup of firmware specific properties from DT. Since this is not deployed out yet in the wild market, I took the leverage of breaking backward compatibility. Before accepting these changes from firmware team, I did discuss[0] with Marc on this topic. > >> Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com> >> --- >> .../interrupt-controller/ti,sci-intr.txt | 31 ++++++++++--------- >> 1 file changed, 16 insertions(+), 15 deletions(-) >> >> diff --git a/Documentation/devicetree/bindings/interrupt-controller/ti,sci-intr.txt b/Documentation/devicetree/bindings/interrupt-controller/ti,sci-intr.txt >> index 1a8718f8855d..8b56b2de1c73 100644 >> --- a/Documentation/devicetree/bindings/interrupt-controller/ti,sci-intr.txt >> +++ b/Documentation/devicetree/bindings/interrupt-controller/ti,sci-intr.txt >> @@ -44,15 +44,17 @@ Required Properties: >> 4: If intr supports level triggered interrupts. >> - interrupt-controller: Identifies the node as an interrupt controller >> - #interrupt-cells: Specifies the number of cells needed to encode an >> - interrupt source. The value should be 2. >> - First cell should contain the TISCI device ID of source >> - Second cell should contain the interrupt source offset >> - within the device. >> + interrupt source. The value should be 1. >> + First cell should contain interrupt router input number >> + as specified by hardware. >> - ti,sci: Phandle to TI-SCI compatible System controller node. >> -- ti,sci-dst-id: TISCI device ID of the destination IRQ controller. >> -- ti,sci-rm-range-girq: Array of TISCI subtype ids representing the host irqs >> - assigned to this interrupt router. Each subtype id >> - corresponds to a range of host irqs. >> +- ti,sci-dev-id: TISCI device id of interrupt controller. >> +- ti,interrupt-ranges: Set of triplets containing ranges that convert >> + the INTR output interrupt numbers to parent's >> + interrupt number. Each triplet has following entries: >> + - First entry specifies the base for intr output irq >> + - Second entry specifies the base for parent irqs >> + - Third entry specifies the limit > > Humm, sounds like what interrupt-map does. Okay, Ill look at it. [0] https://lore.kernel.org/linux-arm-kernel/2331f04eacee3b06cc152fc609225233@www.loen.fr/ Thanks and regards, Lokesh
On 2020-05-29 11:14, Lokesh Vutla wrote: > Hi Rob, > > On 29/05/20 3:44 am, Rob Herring wrote: >> On Wed, May 20, 2020 at 06:14:46PM +0530, Lokesh Vutla wrote: >>> Drop the firmware related dt-bindings and use the hardware specified >>> interrupt numbers within Interrupt Router. This ensures interrupt >>> router >>> DT node need not assume any interrupt parent type. >> >> I didn't like this binding to begin with, but now you're breaking >> compatibility. > > Yes, I do agree that this change is breaking backward compatibility. > But IMHO, > this does cleanup of firmware specific properties from DT. Since this > is not > deployed out yet in the wild market, I took the leverage of breaking > backward > compatibility. Before accepting these changes from firmware team, I did > discuss[0] with Marc on this topic. And I assume that should anyone complain about the kernel being broken because they have an old firmware, you'll be OK with the patches being reverted, right? Thanks, M.
Hi Rob, [..snip..] >> >> diff --git a/Documentation/devicetree/bindings/interrupt-controller/ti,sci-intr.txt b/Documentation/devicetree/bindings/interrupt-controller/ti,sci-intr.txt >> index 1a8718f8855d..8b56b2de1c73 100644 >> --- a/Documentation/devicetree/bindings/interrupt-controller/ti,sci-intr.txt >> +++ b/Documentation/devicetree/bindings/interrupt-controller/ti,sci-intr.txt >> @@ -44,15 +44,17 @@ Required Properties: >> 4: If intr supports level triggered interrupts. >> - interrupt-controller: Identifies the node as an interrupt controller >> - #interrupt-cells: Specifies the number of cells needed to encode an >> - interrupt source. The value should be 2. >> - First cell should contain the TISCI device ID of source >> - Second cell should contain the interrupt source offset >> - within the device. >> + interrupt source. The value should be 1. >> + First cell should contain interrupt router input number >> + as specified by hardware. >> - ti,sci: Phandle to TI-SCI compatible System controller node. >> -- ti,sci-dst-id: TISCI device ID of the destination IRQ controller. >> -- ti,sci-rm-range-girq: Array of TISCI subtype ids representing the host irqs >> - assigned to this interrupt router. Each subtype id >> - corresponds to a range of host irqs. >> +- ti,sci-dev-id: TISCI device id of interrupt controller. >> +- ti,interrupt-ranges: Set of triplets containing ranges that convert >> + the INTR output interrupt numbers to parent's >> + interrupt number. Each triplet has following entries: >> + - First entry specifies the base for intr output irq >> + - Second entry specifies the base for parent irqs >> + - Third entry specifies the limit > > Humm, sounds like what interrupt-map does. IIUC, for every irq translation there should be an entry in interrupt-map property. These Controllers has interrupts ranging from 32, 256 and more. It might be odd to have 256 entries in the interrupt map no? Also there are multiple interrupt controllers which need such translations. Thanks and regards, Lokesh
Hi Marc, On 29/05/20 3:48 pm, Marc Zyngier wrote: > On 2020-05-29 11:14, Lokesh Vutla wrote: >> Hi Rob, >> >> On 29/05/20 3:44 am, Rob Herring wrote: >>> On Wed, May 20, 2020 at 06:14:46PM +0530, Lokesh Vutla wrote: >>>> Drop the firmware related dt-bindings and use the hardware specified >>>> interrupt numbers within Interrupt Router. This ensures interrupt router >>>> DT node need not assume any interrupt parent type. >>> >>> I didn't like this binding to begin with, but now you're breaking >>> compatibility. >> >> Yes, I do agree that this change is breaking backward compatibility. But IMHO, >> this does cleanup of firmware specific properties from DT. Since this is not >> deployed out yet in the wild market, I took the leverage of breaking backward >> compatibility. Before accepting these changes from firmware team, I did >> discuss[0] with Marc on this topic. > > And I assume that should anyone complain about the kernel being broken > because they have an old firmware, you'll be OK with the patches being > reverted, right? I am assuming there is no one to complain as there is no product available yet with upstream kernel. Internally everyone is aware of the changes. So, yes I would agree with you to revert the changes if someone really needs it. :) Thanks and regards, Lokesh
Hi Marc, On 01/06/20 5:06 pm, Lokesh Vutla wrote: > Hi Marc, > > On 29/05/20 3:48 pm, Marc Zyngier wrote: >> On 2020-05-29 11:14, Lokesh Vutla wrote: >>> Hi Rob, >>> >>> On 29/05/20 3:44 am, Rob Herring wrote: >>>> On Wed, May 20, 2020 at 06:14:46PM +0530, Lokesh Vutla wrote: >>>>> Drop the firmware related dt-bindings and use the hardware specified >>>>> interrupt numbers within Interrupt Router. This ensures interrupt router >>>>> DT node need not assume any interrupt parent type. >>>> >>>> I didn't like this binding to begin with, but now you're breaking >>>> compatibility. >>> >>> Yes, I do agree that this change is breaking backward compatibility. But IMHO, >>> this does cleanup of firmware specific properties from DT. Since this is not >>> deployed out yet in the wild market, I took the leverage of breaking backward >>> compatibility. Before accepting these changes from firmware team, I did >>> discuss[0] with Marc on this topic. >> >> And I assume that should anyone complain about the kernel being broken >> because they have an old firmware, you'll be OK with the patches being >> reverted, right? > > I am assuming there is no one to complain as there is no product available yet > with upstream kernel. Internally everyone is aware of the changes. So, yes I > would agree with you to revert the changes if someone really needs it. :) Any chance you can shower your blessings on this series :) Thanks and regards, Lokesh > > Thanks and regards, > Lokesh > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel >
On 2020-06-15 09:03, Lokesh Vutla wrote: > Hi Marc, > > On 01/06/20 5:06 pm, Lokesh Vutla wrote: >> Hi Marc, >> >> On 29/05/20 3:48 pm, Marc Zyngier wrote: >>> On 2020-05-29 11:14, Lokesh Vutla wrote: >>>> Hi Rob, >>>> >>>> On 29/05/20 3:44 am, Rob Herring wrote: >>>>> On Wed, May 20, 2020 at 06:14:46PM +0530, Lokesh Vutla wrote: >>>>>> Drop the firmware related dt-bindings and use the hardware >>>>>> specified >>>>>> interrupt numbers within Interrupt Router. This ensures interrupt >>>>>> router >>>>>> DT node need not assume any interrupt parent type. >>>>> >>>>> I didn't like this binding to begin with, but now you're breaking >>>>> compatibility. >>>> >>>> Yes, I do agree that this change is breaking backward compatibility. >>>> But IMHO, >>>> this does cleanup of firmware specific properties from DT. Since >>>> this is not >>>> deployed out yet in the wild market, I took the leverage of breaking >>>> backward >>>> compatibility. Before accepting these changes from firmware team, I >>>> did >>>> discuss[0] with Marc on this topic. >>> >>> And I assume that should anyone complain about the kernel being >>> broken >>> because they have an old firmware, you'll be OK with the patches >>> being >>> reverted, right? >> >> I am assuming there is no one to complain as there is no product >> available yet >> with upstream kernel. Internally everyone is aware of the changes. So, >> yes I >> would agree with you to revert the changes if someone really needs it. >> :) > > Any chance you can shower your blessings on this series :) I have purposely ignored it just before and during the merge window. It is now firmly in my review queue. M.
Hi Marc, On 15/06/20 2:04 pm, Marc Zyngier wrote: > On 2020-06-15 09:03, Lokesh Vutla wrote: >> Hi Marc, >> >> On 01/06/20 5:06 pm, Lokesh Vutla wrote: >>> Hi Marc, >>> >>> On 29/05/20 3:48 pm, Marc Zyngier wrote: >>>> On 2020-05-29 11:14, Lokesh Vutla wrote: >>>>> Hi Rob, >>>>> >>>>> On 29/05/20 3:44 am, Rob Herring wrote: >>>>>> On Wed, May 20, 2020 at 06:14:46PM +0530, Lokesh Vutla wrote: >>>>>>> Drop the firmware related dt-bindings and use the hardware specified >>>>>>> interrupt numbers within Interrupt Router. This ensures interrupt router >>>>>>> DT node need not assume any interrupt parent type. >>>>>> >>>>>> I didn't like this binding to begin with, but now you're breaking >>>>>> compatibility. >>>>> >>>>> Yes, I do agree that this change is breaking backward compatibility. But IMHO, >>>>> this does cleanup of firmware specific properties from DT. Since this is not >>>>> deployed out yet in the wild market, I took the leverage of breaking backward >>>>> compatibility. Before accepting these changes from firmware team, I did >>>>> discuss[0] with Marc on this topic. >>>> >>>> And I assume that should anyone complain about the kernel being broken >>>> because they have an old firmware, you'll be OK with the patches being >>>> reverted, right? >>> >>> I am assuming there is no one to complain as there is no product available yet >>> with upstream kernel. Internally everyone is aware of the changes. So, yes I >>> would agree with you to revert the changes if someone really needs it. :) >> >> Any chance you can shower your blessings on this series :) > > I have purposely ignored it just before and during the merge window. It is now > firmly in my review queue. rc4 is tagged now. Do you want me to rebase, split the series and repost it? Thanks and reagrds, Lokesh > > M.
diff --git a/Documentation/devicetree/bindings/interrupt-controller/ti,sci-intr.txt b/Documentation/devicetree/bindings/interrupt-controller/ti,sci-intr.txt index 1a8718f8855d..8b56b2de1c73 100644 --- a/Documentation/devicetree/bindings/interrupt-controller/ti,sci-intr.txt +++ b/Documentation/devicetree/bindings/interrupt-controller/ti,sci-intr.txt @@ -44,15 +44,17 @@ Required Properties: 4: If intr supports level triggered interrupts. - interrupt-controller: Identifies the node as an interrupt controller - #interrupt-cells: Specifies the number of cells needed to encode an - interrupt source. The value should be 2. - First cell should contain the TISCI device ID of source - Second cell should contain the interrupt source offset - within the device. + interrupt source. The value should be 1. + First cell should contain interrupt router input number + as specified by hardware. - ti,sci: Phandle to TI-SCI compatible System controller node. -- ti,sci-dst-id: TISCI device ID of the destination IRQ controller. -- ti,sci-rm-range-girq: Array of TISCI subtype ids representing the host irqs - assigned to this interrupt router. Each subtype id - corresponds to a range of host irqs. +- ti,sci-dev-id: TISCI device id of interrupt controller. +- ti,interrupt-ranges: Set of triplets containing ranges that convert + the INTR output interrupt numbers to parent's + interrupt number. Each triplet has following entries: + - First entry specifies the base for intr output irq + - Second entry specifies the base for parent irqs + - Third entry specifies the limit For more details on TISCI IRQ resource management refer: http://downloads.ti.com/tisci/esd/latest/2_tisci_msgs/rm/rm_irq.html @@ -62,21 +64,20 @@ Example: The following example demonstrates both interrupt router node and the consumer node(main gpio) on the AM654 SoC: -main_intr: interrupt-controller0 { +main_gpio_intr: interrupt-controller0 { compatible = "ti,sci-intr"; ti,intr-trigger-type = <1>; interrupt-controller; interrupt-parent = <&gic500>; - #interrupt-cells = <2>; + #interrupt-cells = <1>; ti,sci = <&dmsc>; - ti,sci-dst-id = <56>; - ti,sci-rm-range-girq = <0x1>; + ti,sci-dev-id = <131>; + ti,interrupt-ranges = <0 360 32>; }; main_gpio0: gpio@600000 { ... - interrupt-parent = <&main_intr>; - interrupts = <57 256>, <57 257>, <57 258>, - <57 259>, <57 260>, <57 261>; + interrupt-parent = <&main_gpio_intr>; + interrupts = <192>, <193>, <194>, <195>, <196>, <197>; ... };
Drop the firmware related dt-bindings and use the hardware specified interrupt numbers within Interrupt Router. This ensures interrupt router DT node need not assume any interrupt parent type. Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com> --- .../interrupt-controller/ti,sci-intr.txt | 31 ++++++++++--------- 1 file changed, 16 insertions(+), 15 deletions(-)