| Message ID | 6bd99d4a7e50904b57bb3ad050725fbb418874b7.1597852360.git.cristian.ciocaltea@gmail.com |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | Add Actions Semi Owl family sirq support | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success | |
| robh/dt-meta-schema | success |
On Wed, Aug 19, 2020 at 07:37:56PM +0300, Cristian Ciocaltea wrote: > Actions Semi Owl SoCs SIRQ interrupt controller is found in S500, S700 > and S900 SoCs and provides support for handling up to 3 external > interrupt lines. > > Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@gmail.com> > --- > Changes in v5: > - Updated controller description statements both in the commit message > and the binding doc > > .../actions,owl-sirq.yaml | 68 +++++++++++++++++++ > 1 file changed, 68 insertions(+) > create mode 100644 Documentation/devicetree/bindings/interrupt-controller/actions,owl-sirq.yaml > > diff --git a/Documentation/devicetree/bindings/interrupt-controller/actions,owl-sirq.yaml b/Documentation/devicetree/bindings/interrupt-controller/actions,owl-sirq.yaml > new file mode 100644 > index 000000000000..cf9b7a514e4e > --- /dev/null > +++ b/Documentation/devicetree/bindings/interrupt-controller/actions,owl-sirq.yaml > @@ -0,0 +1,68 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/interrupt-controller/actions,owl-sirq.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Actions Semi Owl SoCs SIRQ interrupt controller > + > +maintainers: > + - Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> > + - Cristian Ciocaltea <cristian.ciocaltea@gmail.com> > + > +description: | > + This interrupt controller is found in the Actions Semi Owl SoCs (S500, S700 > + and S900) and provides support for handling up to 3 external interrupt lines. > + > +properties: > + compatible: > + oneOf: > + - items: > + - enum: > + - actions,s500-sirq > + - actions,s700-sirq > + - actions,s900-sirq > + - const: actions,owl-sirq > + - const: actions,owl-sirq This should be dropped. You should always have the SoC specific compatible. > + > + reg: > + maxItems: 1 > + > + interrupt-controller: true > + > + '#interrupt-cells': > + const: 2 > + description: > + The first cell is the input IRQ number, between 0 and 2, while the second > + cell is the trigger type as defined in interrupt.txt in this directory. > + > + 'actions,ext-interrupts': > + description: | > + Contains the GIC SPI IRQ numbers mapped to the external interrupt > + lines. They shall be specified sequentially from output 0 to 2. > + $ref: /schemas/types.yaml#/definitions/uint32-array > + minItems: 3 > + maxItems: 3 Can't you use 'interrupts' here? > + > +required: > + - compatible > + - reg > + - interrupt-controller > + - '#interrupt-cells' > + - 'actions,ext-interrupts' > + > +additionalProperties: false > + > +examples: > + - | > + sirq: interrupt-controller@b01b0200 { > + compatible = "actions,s500-sirq", "actions,owl-sirq"; > + reg = <0xb01b0200 0x4>; > + interrupt-controller; > + #interrupt-cells = <2>; > + actions,ext-interrupts = <13>, /* SIRQ0 */ > + <14>, /* SIRQ1 */ > + <15>; /* SIRQ2 */ > + }; > + > +... > -- > 2.28.0 >
Hi Rob, Thanks for the review! On Tue, Aug 25, 2020 at 04:09:13PM -0600, Rob Herring wrote: > On Wed, Aug 19, 2020 at 07:37:56PM +0300, Cristian Ciocaltea wrote: > > Actions Semi Owl SoCs SIRQ interrupt controller is found in S500, S700 > > and S900 SoCs and provides support for handling up to 3 external > > interrupt lines. > > > > Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@gmail.com> > > --- > > Changes in v5: > > - Updated controller description statements both in the commit message > > and the binding doc > > > > .../actions,owl-sirq.yaml | 68 +++++++++++++++++++ > > 1 file changed, 68 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/interrupt-controller/actions,owl-sirq.yaml > > > > diff --git a/Documentation/devicetree/bindings/interrupt-controller/actions,owl-sirq.yaml b/Documentation/devicetree/bindings/interrupt-controller/actions,owl-sirq.yaml > > new file mode 100644 > > index 000000000000..cf9b7a514e4e > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/interrupt-controller/actions,owl-sirq.yaml > > @@ -0,0 +1,68 @@ > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > +%YAML 1.2 > > +--- > > +$id: http://devicetree.org/schemas/interrupt-controller/actions,owl-sirq.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: Actions Semi Owl SoCs SIRQ interrupt controller > > + > > +maintainers: > > + - Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> > > + - Cristian Ciocaltea <cristian.ciocaltea@gmail.com> > > + > > +description: | > > + This interrupt controller is found in the Actions Semi Owl SoCs (S500, S700 > > + and S900) and provides support for handling up to 3 external interrupt lines. > > + > > +properties: > > + compatible: > > + oneOf: > > + - items: > > + - enum: > > + - actions,s500-sirq > > + - actions,s700-sirq > > + - actions,s900-sirq > > + - const: actions,owl-sirq > > + - const: actions,owl-sirq > > This should be dropped. You should always have the SoC specific > compatible. Sure, I will get rid of the 'owl-sirq' compatible. > > + > > + reg: > > + maxItems: 1 > > + > > + interrupt-controller: true > > + > > + '#interrupt-cells': > > + const: 2 > > + description: > > + The first cell is the input IRQ number, between 0 and 2, while the second > > + cell is the trigger type as defined in interrupt.txt in this directory. > > + > > + 'actions,ext-interrupts': > > + description: | > > + Contains the GIC SPI IRQ numbers mapped to the external interrupt > > + lines. They shall be specified sequentially from output 0 to 2. > > + $ref: /schemas/types.yaml#/definitions/uint32-array > > + minItems: 3 > > + maxItems: 3 > > Can't you use 'interrupts' here? This was actually my initial idea, but it might confuse the users since this is not following the parent controller IRQ specs, i.e. the trigger type is set internally by the SIRQ driver, it's not taken from DT. Please see the DTS sample bellow where both devices are on the same level and have GIC as interrupt parent. The 'interrupts' property in the sirq node looks incomplete now. That is why I decided to use a custom name for it, although I'm not sure it's the most relevant one, I am open to any other suggestion. i2c0: i2c@b0170000 { [...] interrupts = <GIC_SPI 25 IRQ_TYPE_LEVEL_HIGH>; [...] }; sirq: interrupt-controller@b01b0200 { [...] interrupt-controller; #interrupt-cells = <2>; interrupts = <13>, /* SIRQ0 */ <14>, /* SIRQ1 */ <15>; /* SIRQ2 */ }; Regards, Cristi > > + > > +required: > > + - compatible > > + - reg > > + - interrupt-controller > > + - '#interrupt-cells' > > + - 'actions,ext-interrupts' > > + > > +additionalProperties: false > > + > > +examples: > > + - | > > + sirq: interrupt-controller@b01b0200 { > > + compatible = "actions,s500-sirq", "actions,owl-sirq"; > > + reg = <0xb01b0200 0x4>; > > + interrupt-controller; > > + #interrupt-cells = <2>; > > + actions,ext-interrupts = <13>, /* SIRQ0 */ > > + <14>, /* SIRQ1 */ > > + <15>; /* SIRQ2 */ > > + }; > > + > > +... > > -- > > 2.28.0 > >
On Wed, Aug 26, 2020 at 3:42 PM Cristian Ciocaltea <cristian.ciocaltea@gmail.com> wrote: > > Hi Rob, > > Thanks for the review! > > On Tue, Aug 25, 2020 at 04:09:13PM -0600, Rob Herring wrote: > > On Wed, Aug 19, 2020 at 07:37:56PM +0300, Cristian Ciocaltea wrote: > > > Actions Semi Owl SoCs SIRQ interrupt controller is found in S500, S700 > > > and S900 SoCs and provides support for handling up to 3 external > > > interrupt lines. > > > > > > Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@gmail.com> > > > --- > > > Changes in v5: > > > - Updated controller description statements both in the commit message > > > and the binding doc > > > > > > .../actions,owl-sirq.yaml | 68 +++++++++++++++++++ > > > 1 file changed, 68 insertions(+) > > > create mode 100644 Documentation/devicetree/bindings/interrupt-controller/actions,owl-sirq.yaml > > > > > > diff --git a/Documentation/devicetree/bindings/interrupt-controller/actions,owl-sirq.yaml b/Documentation/devicetree/bindings/interrupt-controller/actions,owl-sirq.yaml > > > new file mode 100644 > > > index 000000000000..cf9b7a514e4e > > > --- /dev/null > > > +++ b/Documentation/devicetree/bindings/interrupt-controller/actions,owl-sirq.yaml > > > @@ -0,0 +1,68 @@ > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > > +%YAML 1.2 > > > +--- > > > +$id: http://devicetree.org/schemas/interrupt-controller/actions,owl-sirq.yaml# > > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > > + > > > +title: Actions Semi Owl SoCs SIRQ interrupt controller > > > + > > > +maintainers: > > > + - Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> > > > + - Cristian Ciocaltea <cristian.ciocaltea@gmail.com> > > > + > > > +description: | > > > + This interrupt controller is found in the Actions Semi Owl SoCs (S500, S700 > > > + and S900) and provides support for handling up to 3 external interrupt lines. > > > + > > > +properties: > > > + compatible: > > > + oneOf: > > > + - items: > > > + - enum: > > > + - actions,s500-sirq > > > + - actions,s700-sirq > > > + - actions,s900-sirq > > > + - const: actions,owl-sirq > > > + - const: actions,owl-sirq > > > > This should be dropped. You should always have the SoC specific > > compatible. > > Sure, I will get rid of the 'owl-sirq' compatible. > > > > + > > > + reg: > > > + maxItems: 1 > > > + > > > + interrupt-controller: true > > > + > > > + '#interrupt-cells': > > > + const: 2 > > > + description: > > > + The first cell is the input IRQ number, between 0 and 2, while the second > > > + cell is the trigger type as defined in interrupt.txt in this directory. > > > + > > > + 'actions,ext-interrupts': > > > + description: | > > > + Contains the GIC SPI IRQ numbers mapped to the external interrupt > > > + lines. They shall be specified sequentially from output 0 to 2. > > > + $ref: /schemas/types.yaml#/definitions/uint32-array > > > + minItems: 3 > > > + maxItems: 3 > > > > Can't you use 'interrupts' here? > > This was actually my initial idea, but it might confuse the users since > this is not following the parent controller IRQ specs, i.e. the trigger > type is set internally by the SIRQ driver, it's not taken from DT. Then what's the 2nd cell for? > Please see the DTS sample bellow where both devices are on the same > level and have GIC as interrupt parent. The 'interrupts' property > in the sirq node looks incomplete now. That is why I decided to use > a custom name for it, although I'm not sure it's the most relevant one, > I am open to any other suggestion. > > i2c0: i2c@b0170000 { > [...] > interrupts = <GIC_SPI 25 IRQ_TYPE_LEVEL_HIGH>; > [...] > }; > > sirq: interrupt-controller@b01b0200 { > [...] > interrupt-controller; > #interrupt-cells = <2>; > interrupts = <13>, /* SIRQ0 */ > <14>, /* SIRQ1 */ > <15>; /* SIRQ2 */ This isn't valid if the GIC is the parent as you have to have 3 cells for each interrupt. Ultimately the GIC trigger type has to be something. Is it fixed or passed thru? If the latter, just use 0 (IRQ_TYPE_NONE) if the GIC trigger mode is not fixed. Having some sort of translation of the trigger is pretty common. Rob
On Wed, Aug 26, 2020 at 04:48:38PM -0600, Rob Herring wrote: > On Wed, Aug 26, 2020 at 3:42 PM Cristian Ciocaltea > <cristian.ciocaltea@gmail.com> wrote: > > > > Hi Rob, > > > > Thanks for the review! > > > > On Tue, Aug 25, 2020 at 04:09:13PM -0600, Rob Herring wrote: > > > On Wed, Aug 19, 2020 at 07:37:56PM +0300, Cristian Ciocaltea wrote: > > > > Actions Semi Owl SoCs SIRQ interrupt controller is found in S500, S700 > > > > and S900 SoCs and provides support for handling up to 3 external > > > > interrupt lines. > > > > > > > > Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@gmail.com> > > > > --- > > > > Changes in v5: > > > > - Updated controller description statements both in the commit message > > > > and the binding doc > > > > > > > > .../actions,owl-sirq.yaml | 68 +++++++++++++++++++ > > > > 1 file changed, 68 insertions(+) > > > > create mode 100644 Documentation/devicetree/bindings/interrupt-controller/actions,owl-sirq.yaml > > > > > > > > diff --git a/Documentation/devicetree/bindings/interrupt-controller/actions,owl-sirq.yaml b/Documentation/devicetree/bindings/interrupt-controller/actions,owl-sirq.yaml > > > > new file mode 100644 > > > > index 000000000000..cf9b7a514e4e > > > > --- /dev/null > > > > +++ b/Documentation/devicetree/bindings/interrupt-controller/actions,owl-sirq.yaml > > > > @@ -0,0 +1,68 @@ > > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > > > +%YAML 1.2 > > > > +--- > > > > +$id: http://devicetree.org/schemas/interrupt-controller/actions,owl-sirq.yaml# > > > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > > > + > > > > +title: Actions Semi Owl SoCs SIRQ interrupt controller > > > > + > > > > +maintainers: > > > > + - Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> > > > > + - Cristian Ciocaltea <cristian.ciocaltea@gmail.com> > > > > + > > > > +description: | > > > > + This interrupt controller is found in the Actions Semi Owl SoCs (S500, S700 > > > > + and S900) and provides support for handling up to 3 external interrupt lines. > > > > + > > > > +properties: > > > > + compatible: > > > > + oneOf: > > > > + - items: > > > > + - enum: > > > > + - actions,s500-sirq > > > > + - actions,s700-sirq > > > > + - actions,s900-sirq > > > > + - const: actions,owl-sirq > > > > + - const: actions,owl-sirq > > > > > > This should be dropped. You should always have the SoC specific > > > compatible. > > > > Sure, I will get rid of the 'owl-sirq' compatible. > > > > > > + > > > > + reg: > > > > + maxItems: 1 > > > > + > > > > + interrupt-controller: true > > > > + > > > > + '#interrupt-cells': > > > > + const: 2 > > > > + description: > > > > + The first cell is the input IRQ number, between 0 and 2, while the second > > > > + cell is the trigger type as defined in interrupt.txt in this directory. > > > > + > > > > + 'actions,ext-interrupts': > > > > + description: | > > > > + Contains the GIC SPI IRQ numbers mapped to the external interrupt > > > > + lines. They shall be specified sequentially from output 0 to 2. > > > > + $ref: /schemas/types.yaml#/definitions/uint32-array > > > > + minItems: 3 > > > > + maxItems: 3 > > > > > > Can't you use 'interrupts' here? > > > > This was actually my initial idea, but it might confuse the users since > > this is not following the parent controller IRQ specs, i.e. the trigger > > type is set internally by the SIRQ driver, it's not taken from DT. > > Then what's the 2nd cell for? I should have added also a child device sample to make it more clear how this is supposed to work: &i2c0 { atc260x: pmic@65 { [...] interrupt-parent = <&sirq>; interrupts = <2 IRQ_TYPE_LEVEL_HIGH>; }; }; The PMIC above uses the SIRQ2 pin of the SIRQ controller to trigger interrupts, while the controller is responsible for proper translation before sending to GIC, i.e. converting falling edge to rising edge signal and active low to active high signal. > > Please see the DTS sample bellow where both devices are on the same > > level and have GIC as interrupt parent. The 'interrupts' property > > in the sirq node looks incomplete now. That is why I decided to use > > a custom name for it, although I'm not sure it's the most relevant one, > > I am open to any other suggestion. > > > > i2c0: i2c@b0170000 { > > [...] > > interrupts = <GIC_SPI 25 IRQ_TYPE_LEVEL_HIGH>; > > [...] > > }; > > > > sirq: interrupt-controller@b01b0200 { > > [...] > > interrupt-controller; > > #interrupt-cells = <2>; > > interrupts = <13>, /* SIRQ0 */ > > <14>, /* SIRQ1 */ > > <15>; /* SIRQ2 */ > > This isn't valid if the GIC is the parent as you have to have 3 cells > for each interrupt. Right, that's the reason of replacing 'interrupts' with 'actions,ext-interrupts'. > Ultimately the GIC trigger type has to be > something. Is it fixed or passed thru? If the latter, just use 0 > (IRQ_TYPE_NONE) if the GIC trigger mode is not fixed. Having some sort > of translation of the trigger is pretty common. Yes, as explained above, the SIRQ controller performs indeed the translation of the incoming signal. So if I understand correctly, your suggestion would be to use the following inside the sirq node: interrupts = <GIC_SPI 13 IRQ_TYPE_NONE>, /* SIRQ0 */ [...] > Rob Thanks, Cristi
On 2020-08-27 11:06, Cristian Ciocaltea wrote: > On Wed, Aug 26, 2020 at 04:48:38PM -0600, Rob Herring wrote: >> On Wed, Aug 26, 2020 at 3:42 PM Cristian Ciocaltea >> <cristian.ciocaltea@gmail.com> wrote: [...] >> Ultimately the GIC trigger type has to be >> something. Is it fixed or passed thru? If the latter, just use 0 >> (IRQ_TYPE_NONE) if the GIC trigger mode is not fixed. Having some sort >> of translation of the trigger is pretty common. > > Yes, as explained above, the SIRQ controller performs indeed the > translation of the incoming signal. So if I understand correctly, your > suggestion would be to use the following inside the sirq node: > > interrupts = <GIC_SPI 13 IRQ_TYPE_NONE>, /* SIRQ0 */ > [...] Please don't. If you are describing a GIC interrupt, use a trigger that actually exists. Given that you have a 1:1 mapping between input and output, just encode the output trigger that matches the input. M.
Hi Marc, On Thu, Aug 27, 2020 at 11:35:06AM +0100, Marc Zyngier wrote: > On 2020-08-27 11:06, Cristian Ciocaltea wrote: > > On Wed, Aug 26, 2020 at 04:48:38PM -0600, Rob Herring wrote: > > > On Wed, Aug 26, 2020 at 3:42 PM Cristian Ciocaltea > > > <cristian.ciocaltea@gmail.com> wrote: > > [...] > > > > Ultimately the GIC trigger type has to be > > > something. Is it fixed or passed thru? If the latter, just use 0 > > > (IRQ_TYPE_NONE) if the GIC trigger mode is not fixed. Having some sort > > > of translation of the trigger is pretty common. > > > > Yes, as explained above, the SIRQ controller performs indeed the > > translation of the incoming signal. So if I understand correctly, your > > suggestion would be to use the following inside the sirq node: > > > > interrupts = <GIC_SPI 13 IRQ_TYPE_NONE>, /* SIRQ0 */ > > [...] > > Please don't. If you are describing a GIC interrupt, use a > trigger that actually exists. Given that you have a 1:1 > mapping between input and output, just encode the output > trigger that matches the input. Understood, the only remark here is that internally, the driver will not use this information and instead will continue to rely on the input to properly set the trigger type for the output. The question is if the driver should also emit a warning (or error?) when the trigger type supplied via DT doesn't match the expected value. If yes, we should also clarify what the user is supposed to provide in the controller node: the trigger type before the conversion (the input) or the one after the conversion (the output). > M. > -- > Jazz is not dead. It just smells funny... Thanks, Cristi
Cristian, On 2020-08-27 16:24, Cristian Ciocaltea wrote: > Hi Marc, > > On Thu, Aug 27, 2020 at 11:35:06AM +0100, Marc Zyngier wrote: >> On 2020-08-27 11:06, Cristian Ciocaltea wrote: >> > On Wed, Aug 26, 2020 at 04:48:38PM -0600, Rob Herring wrote: >> > > On Wed, Aug 26, 2020 at 3:42 PM Cristian Ciocaltea >> > > <cristian.ciocaltea@gmail.com> wrote: >> >> [...] >> >> > > Ultimately the GIC trigger type has to be >> > > something. Is it fixed or passed thru? If the latter, just use 0 >> > > (IRQ_TYPE_NONE) if the GIC trigger mode is not fixed. Having some sort >> > > of translation of the trigger is pretty common. >> > >> > Yes, as explained above, the SIRQ controller performs indeed the >> > translation of the incoming signal. So if I understand correctly, your >> > suggestion would be to use the following inside the sirq node: >> > >> > interrupts = <GIC_SPI 13 IRQ_TYPE_NONE>, /* SIRQ0 */ >> > [...] >> >> Please don't. If you are describing a GIC interrupt, use a >> trigger that actually exists. Given that you have a 1:1 >> mapping between input and output, just encode the output >> trigger that matches the input. > > Understood, the only remark here is that internally, the driver will > not use this information and instead will continue to rely on the input > to properly set the trigger type for the output. It's fine. The binding has to be consistent on its own, but doesn't dictate the way the driver does thing. > The question is if the driver should also emit a warning (or error?) > when the trigger type supplied via DT doesn't match the expected value. Rob will tell you that the kernel isn't a validation tool for broken DTs. Shout if you want, but you are allowed to simply ignore the output trigger for example > If yes, we should also clarify what the user is supposed to provide in > the controller node: the trigger type before the conversion (the input) > or the one after the conversion (the output). The output of a SIRQ should be compatible with the GIC input it is attached to. You can have: device (LEVEL_LOW) -> SIRQ (LEVEL_HIGH) -> GIC but you can't have: device (LEVEL_LOW) -> SIRQ (EDGE_RISING) -> GIC because that's not an acceptable transformation for the SIRQ, nor can you have: device (EDGE_FALLING) -> SIRQ (EDGE_FALLING) -> GIC because EDGE_FALLING isn't a valid input for the GIC. In both of the invalid cases, you would be free to apply which ever transformation actually makes sense, and shout at the user if you want to help them debugging their turf. The later part is definitely optional. Hope this helps, M.
On Thu, Aug 27, 2020 at 04:42:04PM +0100, Marc Zyngier wrote: > Cristian, > > On 2020-08-27 16:24, Cristian Ciocaltea wrote: > > Hi Marc, > > > > On Thu, Aug 27, 2020 at 11:35:06AM +0100, Marc Zyngier wrote: > > > On 2020-08-27 11:06, Cristian Ciocaltea wrote: > > > > On Wed, Aug 26, 2020 at 04:48:38PM -0600, Rob Herring wrote: > > > > > On Wed, Aug 26, 2020 at 3:42 PM Cristian Ciocaltea > > > > > <cristian.ciocaltea@gmail.com> wrote: > > > > > > [...] > > > > > > > > Ultimately the GIC trigger type has to be > > > > > something. Is it fixed or passed thru? If the latter, just use 0 > > > > > (IRQ_TYPE_NONE) if the GIC trigger mode is not fixed. Having some sort > > > > > of translation of the trigger is pretty common. > > > > > > > > Yes, as explained above, the SIRQ controller performs indeed the > > > > translation of the incoming signal. So if I understand correctly, your > > > > suggestion would be to use the following inside the sirq node: > > > > > > > > interrupts = <GIC_SPI 13 IRQ_TYPE_NONE>, /* SIRQ0 */ > > > > [...] > > > > > > Please don't. If you are describing a GIC interrupt, use a > > > trigger that actually exists. Given that you have a 1:1 > > > mapping between input and output, just encode the output > > > trigger that matches the input. > > > > Understood, the only remark here is that internally, the driver will > > not use this information and instead will continue to rely on the input > > to properly set the trigger type for the output. > > It's fine. The binding has to be consistent on its own, but > doesn't dictate the way the driver does thing. > > > The question is if the driver should also emit a warning (or error?) > > when the trigger type supplied via DT doesn't match the expected value. > > Rob will tell you that the kernel isn't a validation tool for broken > DTs. Shout if you want, but you are allowed to simply ignore the > output trigger for example > > > If yes, we should also clarify what the user is supposed to provide in > > the controller node: the trigger type before the conversion (the input) > > or the one after the conversion (the output). > > The output of a SIRQ should be compatible with the GIC input it is > attached to. You can have: > > device (LEVEL_LOW) -> SIRQ (LEVEL_HIGH) -> GIC > > but you can't have: > > device (LEVEL_LOW) -> SIRQ (EDGE_RISING) -> GIC > > because that's not an acceptable transformation for the SIRQ, > nor can you have: > > device (EDGE_FALLING) -> SIRQ (EDGE_FALLING) -> GIC > > because EDGE_FALLING isn't a valid input for the GIC. > > In both of the invalid cases, you would be free to apply > which ever transformation actually makes sense, and shout > at the user if you want to help them debugging their turf. > The later part is definitely optional. > > Hope this helps, This certainly helps a lot, now I have a clear understanding of what is to be done next. > M. > -- > Jazz is not dead. It just smells funny... Many thanks for the detailed explanations, Cristi
diff --git a/Documentation/devicetree/bindings/interrupt-controller/actions,owl-sirq.yaml b/Documentation/devicetree/bindings/interrupt-controller/actions,owl-sirq.yaml new file mode 100644 index 000000000000..cf9b7a514e4e --- /dev/null +++ b/Documentation/devicetree/bindings/interrupt-controller/actions,owl-sirq.yaml @@ -0,0 +1,68 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/interrupt-controller/actions,owl-sirq.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Actions Semi Owl SoCs SIRQ interrupt controller + +maintainers: + - Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> + - Cristian Ciocaltea <cristian.ciocaltea@gmail.com> + +description: | + This interrupt controller is found in the Actions Semi Owl SoCs (S500, S700 + and S900) and provides support for handling up to 3 external interrupt lines. + +properties: + compatible: + oneOf: + - items: + - enum: + - actions,s500-sirq + - actions,s700-sirq + - actions,s900-sirq + - const: actions,owl-sirq + - const: actions,owl-sirq + + reg: + maxItems: 1 + + interrupt-controller: true + + '#interrupt-cells': + const: 2 + description: + The first cell is the input IRQ number, between 0 and 2, while the second + cell is the trigger type as defined in interrupt.txt in this directory. + + 'actions,ext-interrupts': + description: | + Contains the GIC SPI IRQ numbers mapped to the external interrupt + lines. They shall be specified sequentially from output 0 to 2. + $ref: /schemas/types.yaml#/definitions/uint32-array + minItems: 3 + maxItems: 3 + +required: + - compatible + - reg + - interrupt-controller + - '#interrupt-cells' + - 'actions,ext-interrupts' + +additionalProperties: false + +examples: + - | + sirq: interrupt-controller@b01b0200 { + compatible = "actions,s500-sirq", "actions,owl-sirq"; + reg = <0xb01b0200 0x4>; + interrupt-controller; + #interrupt-cells = <2>; + actions,ext-interrupts = <13>, /* SIRQ0 */ + <14>, /* SIRQ1 */ + <15>; /* SIRQ2 */ + }; + +...
Actions Semi Owl SoCs SIRQ interrupt controller is found in S500, S700 and S900 SoCs and provides support for handling up to 3 external interrupt lines. Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@gmail.com> --- Changes in v5: - Updated controller description statements both in the commit message and the binding doc .../actions,owl-sirq.yaml | 68 +++++++++++++++++++ 1 file changed, 68 insertions(+) create mode 100644 Documentation/devicetree/bindings/interrupt-controller/actions,owl-sirq.yaml