| Message ID | 20170424075407.19730-2-antoine.tenart@free-electrons.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Herbert Xu |
| Headers | show |
On 24/04/17 08:54, Antoine Tenart wrote: > The Inside Secure Safexcel cryptographic engine is found on some Marvell > SoCs (7k/8k). Document the bindings used by its driver. > > Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com> > --- > .../bindings/crypto/inside-secure-safexcel.txt | 27 ++++++++++++++++++++++ > 1 file changed, 27 insertions(+) > create mode 100644 Documentation/devicetree/bindings/crypto/inside-secure-safexcel.txt > > diff --git a/Documentation/devicetree/bindings/crypto/inside-secure-safexcel.txt b/Documentation/devicetree/bindings/crypto/inside-secure-safexcel.txt > new file mode 100644 > index 000000000000..ff56b9384fcc > --- /dev/null > +++ b/Documentation/devicetree/bindings/crypto/inside-secure-safexcel.txt > @@ -0,0 +1,27 @@ > +Inside Secure SafeXcel cryptographic engine > + > +Required properties: > +- compatible: Should be "inside-secure,safexcel-eip197". > +- reg: Base physical address of the engine and length of memory mapped region. > +- interrupts: Interrupt numbers for the rings and engine. > +- interrupt-names: Should be "ring0", "ring1", "ring2", "ring3", "eip", "mem". > + > +Optional properties: > +- clocks: Reference to the crypto engine clock. > + > +Example: > + > + crypto: crypto@800000 { > + compatible = "inside-secure,safexcel-eip197"; > + reg = <0x800000 0x200000>; > + interrupts = <GIC_SPI 34 (IRQ_TYPE_EDGE_RISING | IRQ_TYPE_LEVEL_HIGH)>, I'm puzzled. How can the interrupt can be both level *and* edge? That doesn't make any sense. Thanks, M.
Hi Marc, On Wed, May 03, 2017 at 05:36:38PM +0100, Marc Zyngier wrote: > On 24/04/17 08:54, Antoine Tenart wrote: > > + > > + crypto: crypto@800000 { > > + compatible = "inside-secure,safexcel-eip197"; > > + reg = <0x800000 0x200000>; > > + interrupts = <GIC_SPI 34 (IRQ_TYPE_EDGE_RISING | IRQ_TYPE_LEVEL_HIGH)>, > > I'm puzzled. How can the interrupt can be both level *and* edge? That > doesn't make any sense. I agree this looks odd. I took it from Russel's ICU mapping: http://lists.infradead.org/pipermail/linux-arm-kernel/2017-February/489040.html The driver does not use this interrupt (yet?). I'm not sure what is this interrupt exact configuration... Do you want me to remove this from the documentation (and device trees)? Thanks, Antoine
Hi Antoine, On 22/05/17 15:30, Antoine Tenart wrote: > Hi Marc, > > On Wed, May 03, 2017 at 05:36:38PM +0100, Marc Zyngier wrote: >> On 24/04/17 08:54, Antoine Tenart wrote: >>> + >>> + crypto: crypto@800000 { >>> + compatible = "inside-secure,safexcel-eip197"; >>> + reg = <0x800000 0x200000>; >>> + interrupts = <GIC_SPI 34 (IRQ_TYPE_EDGE_RISING | IRQ_TYPE_LEVEL_HIGH)>, >> >> I'm puzzled. How can the interrupt can be both level *and* edge? That >> doesn't make any sense. > > I agree this looks odd. I took it from Russel's ICU mapping: > http://lists.infradead.org/pipermail/linux-arm-kernel/2017-February/489040.html This emails says: ICU-irq => GIC-SPI-num Enable Edge/Level ICU-group [...] 24 => 34 En Lv 0 which seems to clearly indicate Level (no Edge involved here). > The driver does not use this interrupt (yet?). I'm not sure what is this > interrupt exact configuration... Surely the TRM would indicate this, wouldn't it? > Do you want me to remove this from the documentation (and device trees)? I'd rather you clarify it, if possible. Thanks, M.
On Mon, May 22, 2017 at 03:48:30PM +0100, Marc Zyngier wrote: > On 22/05/17 15:30, Antoine Tenart wrote: > > On Wed, May 03, 2017 at 05:36:38PM +0100, Marc Zyngier wrote: > >> On 24/04/17 08:54, Antoine Tenart wrote: > >>> + > >>> + crypto: crypto@800000 { > >>> + compatible = "inside-secure,safexcel-eip197"; > >>> + reg = <0x800000 0x200000>; > >>> + interrupts = <GIC_SPI 34 (IRQ_TYPE_EDGE_RISING | IRQ_TYPE_LEVEL_HIGH)>, > >> > >> I'm puzzled. How can the interrupt can be both level *and* edge? That > >> doesn't make any sense. > > > > I agree this looks odd. I took it from Russel's ICU mapping: > > http://lists.infradead.org/pipermail/linux-arm-kernel/2017-February/489040.html > > This emails says: > > ICU-irq => GIC-SPI-num Enable Edge/Level ICU-group > [...] > 24 => 34 En Lv 0 It also says: 87 => 34 En Lv 5, which is the IRQ I'm looking for. Antoine.
On 22/05/17 15:54, Antoine Tenart wrote: > On Mon, May 22, 2017 at 03:48:30PM +0100, Marc Zyngier wrote: >> On 22/05/17 15:30, Antoine Tenart wrote: >>> On Wed, May 03, 2017 at 05:36:38PM +0100, Marc Zyngier wrote: >>>> On 24/04/17 08:54, Antoine Tenart wrote: >>>>> + >>>>> + crypto: crypto@800000 { >>>>> + compatible = "inside-secure,safexcel-eip197"; >>>>> + reg = <0x800000 0x200000>; >>>>> + interrupts = <GIC_SPI 34 (IRQ_TYPE_EDGE_RISING | IRQ_TYPE_LEVEL_HIGH)>, >>>> >>>> I'm puzzled. How can the interrupt can be both level *and* edge? That >>>> doesn't make any sense. >>> >>> I agree this looks odd. I took it from Russel's ICU mapping: >>> http://lists.infradead.org/pipermail/linux-arm-kernel/2017-February/489040.html >> >> This emails says: >> >> ICU-irq => GIC-SPI-num Enable Edge/Level ICU-group >> [...] >> 24 => 34 En Lv 0 > > It also says: 87 => 34 En Lv 5, which is the IRQ I'm looking for. Ah, that one as well. So how is the interrupt routed? Via the ICU, and then to the GIC (with several ICU sources mapped on a single SPI)? If so, the binding should reflect this. Thanks, M.
Hello, On Mon, 22 May 2017 16:02:33 +0100, Marc Zyngier wrote: > > It also says: 87 => 34 En Lv 5, which is the IRQ I'm looking for. > > Ah, that one as well. So how is the interrupt routed? Via the ICU, and > then to the GIC (with several ICU sources mapped on a single SPI)? The crypto block being in the CP part, it has a wired interrupt to the ICU (also in the CP). The ICU then turns this wired interrupt into a memory write transaction to a register called GICP SPI in the AP, which triggers a SPI interrupt in the GIC. In the current mainline kernel, the ICU is configured by the firmware and creates static associations between wired interrupts in the CP and corresponding SPI interrupts. Therefore the Device Tree currently reference such SPI interrupts directly. However, I have a patch series that I plan to submit hopefully in the next days that adds an ICU driver, and changes the Device Tree to refer to the ICU interrupt instead. Therefore, I don't think the binding should reference anything else than the usual info about the interrupts property. Best regards, Thomas
On 22/05/17 20:37, Thomas Petazzoni wrote: > Hello, > > On Mon, 22 May 2017 16:02:33 +0100, Marc Zyngier wrote: > >>> It also says: 87 => 34 En Lv 5, which is the IRQ I'm looking for. >> >> Ah, that one as well. So how is the interrupt routed? Via the ICU, and >> then to the GIC (with several ICU sources mapped on a single SPI)? > > The crypto block being in the CP part, it has a wired interrupt to the > ICU (also in the CP). The ICU then turns this wired interrupt into a > memory write transaction to a register called GICP SPI in the AP, which > triggers a SPI interrupt in the GIC. Is that some kind of Level-triggered MSI, à la GICv3 GICD_SETSPI_NSR? > In the current mainline kernel, the ICU is configured by the firmware > and creates static associations between wired interrupts in the CP and > corresponding SPI interrupts. Therefore the Device Tree currently > reference such SPI interrupts directly. > > However, I have a patch series that I plan to submit hopefully in the > next days that adds an ICU driver, and changes the Device Tree to refer > to the ICU interrupt instead. OK, I'm quite interested to see that, specially if my above hunch is right... > Therefore, I don't think the binding should reference anything else > than the usual info about the interrupts property. That I completely agree with, as long as it doesn't describe anything that is semantically incorrect. Thanks, M.
Hello, On Tue, 23 May 2017 12:13:28 +0100, Marc Zyngier wrote: > > The crypto block being in the CP part, it has a wired interrupt to the > > ICU (also in the CP). The ICU then turns this wired interrupt into a > > memory write transaction to a register called GICP SPI in the AP, which > > triggers a SPI interrupt in the GIC. > > Is that some kind of Level-triggered MSI, à la GICv3 GICD_SETSPI_NSR? It is some kind of MSI, and the registers are called GICP_SETSPI/GICP_CLRSPI, so I would assume it's quite similar to this GICv3 feature. > > However, I have a patch series that I plan to submit hopefully in the > > next days that adds an ICU driver, and changes the Device Tree to refer > > to the ICU interrupt instead. > > OK, I'm quite interested to see that, specially if my above hunch is > right... I'll send the patches soon. I'm sure there will be lots of comments :) Best regards, Thomas
diff --git a/Documentation/devicetree/bindings/crypto/inside-secure-safexcel.txt b/Documentation/devicetree/bindings/crypto/inside-secure-safexcel.txt new file mode 100644 index 000000000000..ff56b9384fcc --- /dev/null +++ b/Documentation/devicetree/bindings/crypto/inside-secure-safexcel.txt @@ -0,0 +1,27 @@ +Inside Secure SafeXcel cryptographic engine + +Required properties: +- compatible: Should be "inside-secure,safexcel-eip197". +- reg: Base physical address of the engine and length of memory mapped region. +- interrupts: Interrupt numbers for the rings and engine. +- interrupt-names: Should be "ring0", "ring1", "ring2", "ring3", "eip", "mem". + +Optional properties: +- clocks: Reference to the crypto engine clock. + +Example: + + crypto: crypto@800000 { + compatible = "inside-secure,safexcel-eip197"; + reg = <0x800000 0x200000>; + interrupts = <GIC_SPI 34 (IRQ_TYPE_EDGE_RISING | IRQ_TYPE_LEVEL_HIGH)>, + <GIC_SPI 54 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 55 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 56 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 57 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 58 IRQ_TYPE_LEVEL_HIGH>; + interrupt-names = "mem", "ring0", "ring1", "ring2", "ring3", + "eip"; + clocks = <&cpm_syscon0 1 26>; + status = "disabled"; + };
The Inside Secure Safexcel cryptographic engine is found on some Marvell SoCs (7k/8k). Document the bindings used by its driver. Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com> --- .../bindings/crypto/inside-secure-safexcel.txt | 27 ++++++++++++++++++++++ 1 file changed, 27 insertions(+) create mode 100644 Documentation/devicetree/bindings/crypto/inside-secure-safexcel.txt