| Message ID | 1432301642-11470-16-git-send-email-boris.brezillon@free-electrons.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Herbert Xu |
| Headers | show |
Hi Boris, Arnaud, On 22/05/2015 15:34, Boris Brezillon wrote: > From: Arnaud Ebalard <arno@natisbad.org> > > Add crypto related nodes in armada-370.dtsi. > > Signed-off-by: Arnaud Ebalard <arno@natisbad.org> > Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com> > --- > arch/arm/boot/dts/armada-370.dtsi | 20 ++++++++++++++++++++ > 1 file changed, 20 insertions(+) > > diff --git a/arch/arm/boot/dts/armada-370.dtsi b/arch/arm/boot/dts/armada-370.dtsi > index 00b50db5..1255318 100644 > --- a/arch/arm/boot/dts/armada-370.dtsi > +++ b/arch/arm/boot/dts/armada-370.dtsi > @@ -307,6 +307,26 @@ > dmacap,memset; > }; > }; > + > + crypto@90000 { > + compatible = "marvell,armada-370-crypto"; > + reg = <0x90000 0x10000>; > + reg-names = "regs"; > + interrupts = <48>; There is no clocks property. After a quick look on the datasheet, indeed I didn't find any clock which match the CESA. In this case you should update the binding documentation by adding that the clock is optional for armada-370-crypto. But for this patch: Acked-by: Gregory CLEMENT <gregory.clement@free-electrons.com> And as for the other one I will wait for that the driver will be merged before applying it. Thanks, Gregory > + marvell,crypto-srams = <&crypto_sram>; > + marvell,crypto-sram-size = <0x800>; > + status = "okay"; > + }; > + }; > + > + crypto_sram: sa-sram { > + compatible = "mmio-sram"; > + reg = <MBUS_ID(0x09, 0x01) 0 0x800>; > + reg-names = "sram"; > + #address-cells = <1>; > + #size-cells = <1>; > + ranges = <0 MBUS_ID(0x09, 0x01) 0 0x800>; > + status = "okay"; > }; > }; > }; >
Hi Gregory, On Mon, 25 May 2015 17:33:24 +0200 Gregory CLEMENT <gregory.clement@free-electrons.com> wrote: > Hi Boris, Arnaud, > > > > On 22/05/2015 15:34, Boris Brezillon wrote: > > From: Arnaud Ebalard <arno@natisbad.org> > > > > Add crypto related nodes in armada-370.dtsi. > > > > Signed-off-by: Arnaud Ebalard <arno@natisbad.org> > > Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com> > > --- > > arch/arm/boot/dts/armada-370.dtsi | 20 ++++++++++++++++++++ > > 1 file changed, 20 insertions(+) > > > > diff --git a/arch/arm/boot/dts/armada-370.dtsi b/arch/arm/boot/dts/armada-370.dtsi > > index 00b50db5..1255318 100644 > > --- a/arch/arm/boot/dts/armada-370.dtsi > > +++ b/arch/arm/boot/dts/armada-370.dtsi > > @@ -307,6 +307,26 @@ > > dmacap,memset; > > }; > > }; > > + > > + crypto@90000 { > > + compatible = "marvell,armada-370-crypto"; > > + reg = <0x90000 0x10000>; > > + reg-names = "regs"; > > + interrupts = <48>; > > There is no clocks property. After a quick look on the datasheet, indeed I didn't > find any clock which match the CESA. In this case you should update the binding > documentation by adding that the clock is optional for armada-370-crypto. Hm, actually the clock is defined in Marvell's LSP, but not described in the datasheet. Which one should I trust :-) ? Anyway, if this clock appears to be present on armada-370, we'll have to modify the clock driver to account for this before referencing the clock in the crypto node. Best Regards, Boris
Dear Boris Brezillon, On Tue, 26 May 2015 11:03:45 +0200, Boris Brezillon wrote: > > There is no clocks property. After a quick look on the datasheet, indeed I didn't > > find any clock which match the CESA. In this case you should update the binding > > documentation by adding that the clock is optional for armada-370-crypto. > > Hm, actually the clock is defined in Marvell's LSP, but not described > in the datasheet. Which one should I trust :-) ? I'd say: trust the reality. Declare the clock in the clock driver, but don't take a reference to it from your crypto driver. The kernel will disable it at the end of the boot. Check if crypto still works or not, and you'll get your answer :-) Thomas
On Tue, 26 May 2015 11:10:41 +0200 Thomas Petazzoni <thomas.petazzoni@free-electrons.com> wrote: > Dear Boris Brezillon, > > On Tue, 26 May 2015 11:03:45 +0200, Boris Brezillon wrote: > > > > There is no clocks property. After a quick look on the datasheet, indeed I didn't > > > find any clock which match the CESA. In this case you should update the binding > > > documentation by adding that the clock is optional for armada-370-crypto. > > > > Hm, actually the clock is defined in Marvell's LSP, but not described > > in the datasheet. Which one should I trust :-) ? > > I'd say: trust the reality. Declare the clock in the clock driver, but > don't take a reference to it from your crypto driver. The kernel will > disable it at the end of the boot. Check if crypto still works or not, > and you'll get your answer :-) Okay, you got me: I was to lazy to test that :-). Anyway, I just did it, and apparently this clock is mandatory (the platform hangs when launching crypto requests if the clock is not referenced by the crypto device). I'll send a patch adding this clock to the armada-370 clock driver and I'll fix the dtsi accordingly. Thanks, Boris > > Thomas
diff --git a/arch/arm/boot/dts/armada-370.dtsi b/arch/arm/boot/dts/armada-370.dtsi index 00b50db5..1255318 100644 --- a/arch/arm/boot/dts/armada-370.dtsi +++ b/arch/arm/boot/dts/armada-370.dtsi @@ -307,6 +307,26 @@ dmacap,memset; }; }; + + crypto@90000 { + compatible = "marvell,armada-370-crypto"; + reg = <0x90000 0x10000>; + reg-names = "regs"; + interrupts = <48>; + marvell,crypto-srams = <&crypto_sram>; + marvell,crypto-sram-size = <0x800>; + status = "okay"; + }; + }; + + crypto_sram: sa-sram { + compatible = "mmio-sram"; + reg = <MBUS_ID(0x09, 0x01) 0 0x800>; + reg-names = "sram"; + #address-cells = <1>; + #size-cells = <1>; + ranges = <0 MBUS_ID(0x09, 0x01) 0 0x800>; + status = "okay"; }; }; };