| Message ID | 1432301642-11470-15-git-send-email-boris.brezillon@free-electrons.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Herbert Xu |
| Headers | show |
Hi Boris, On 22/05/2015 15:34, Boris Brezillon wrote: > Enable the crypto IP on armada-xp-gp. > > Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com> > --- > arch/arm/boot/dts/armada-xp-gp.dts | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/arch/arm/boot/dts/armada-xp-gp.dts b/arch/arm/boot/dts/armada-xp-gp.dts > index 565227e..8a739f4 100644 > --- a/arch/arm/boot/dts/armada-xp-gp.dts > +++ b/arch/arm/boot/dts/armada-xp-gp.dts > @@ -94,7 +94,9 @@ > soc { > ranges = <MBUS_ID(0xf0, 0x01) 0 0 0xf1000000 0x100000 > MBUS_ID(0x01, 0x1d) 0 0 0xfff00000 0x100000 > - MBUS_ID(0x01, 0x2f) 0 0 0xf0000000 0x1000000>; > + MBUS_ID(0x01, 0x2f) 0 0 0xf0000000 0x1000000 > + MBUS_ID(0x09, 0x09) 0 0 0xf1100000 0x10000 > + MBUS_ID(0x09, 0x05) 0 0 0xf1110000 0x10000>; As the crypto engine really depend on the SoC itself and not of the board, what about updating the dts of the other board using an Armada XP? Thanks, Gregory > > devbus-bootcs { > status = "okay"; >
On Mon, 25 May 2015 17:10:37 +0200 Gregory CLEMENT <gregory.clement@free-electrons.com> wrote: > Hi Boris, > > On 22/05/2015 15:34, Boris Brezillon wrote: > > Enable the crypto IP on armada-xp-gp. > > > > Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com> > > --- > > arch/arm/boot/dts/armada-xp-gp.dts | 4 +++- > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > diff --git a/arch/arm/boot/dts/armada-xp-gp.dts b/arch/arm/boot/dts/armada-xp-gp.dts > > index 565227e..8a739f4 100644 > > --- a/arch/arm/boot/dts/armada-xp-gp.dts > > +++ b/arch/arm/boot/dts/armada-xp-gp.dts > > @@ -94,7 +94,9 @@ > > soc { > > ranges = <MBUS_ID(0xf0, 0x01) 0 0 0xf1000000 0x100000 > > MBUS_ID(0x01, 0x1d) 0 0 0xfff00000 0x100000 > > - MBUS_ID(0x01, 0x2f) 0 0 0xf0000000 0x1000000>; > > + MBUS_ID(0x01, 0x2f) 0 0 0xf0000000 0x1000000 > > + MBUS_ID(0x09, 0x09) 0 0 0xf1100000 0x10000 > > + MBUS_ID(0x09, 0x05) 0 0 0xf1110000 0x10000>; > > As the crypto engine really depend on the SoC itself and not of the board, > what about updating the dts of the other board using an Armada XP? But that means introducing changes I haven't tested. Are you okay with that ?
On Tue, 26 May 2015 10:59:36 +0200, Boris Brezillon <boris.brezillon@free-electrons.com> wrote: <snip> >> As the crypto engine really depend on the SoC itself and not of the >> board, >> what about updating the dts of the other board using an Armada XP? > > But that means introducing changes I haven't tested. Are you okay with > that ? Well, worst case send it as a separate patch as RFC - I'm sure people will test and report back. Cheers, Imre -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, May 26, 2015 at 11:22:45AM +0200, Imre Kaloz wrote: > On Tue, 26 May 2015 10:59:36 +0200, Boris Brezillon > <boris.brezillon@free-electrons.com> wrote: > > <snip> > > >>As the crypto engine really depend on the SoC itself and not of > >>the board, > >>what about updating the dts of the other board using an Armada XP? > > > >But that means introducing changes I haven't tested. Are you okay with > >that ? > > Well, worst case send it as a separate patch as RFC - I'm sure > people will test and report back. Yes, there are a few of us with diverse hardware that can perform tests. It is best if you provide a git tree with all the patches applied, and instructions how to test. Andrew -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 26/05/2015 10:59, Boris Brezillon wrote: > On Mon, 25 May 2015 17:10:37 +0200 > Gregory CLEMENT <gregory.clement@free-electrons.com> wrote: > >> Hi Boris, >> >> On 22/05/2015 15:34, Boris Brezillon wrote: >>> Enable the crypto IP on armada-xp-gp. >>> >>> Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com> >>> --- >>> arch/arm/boot/dts/armada-xp-gp.dts | 4 +++- >>> 1 file changed, 3 insertions(+), 1 deletion(-) >>> >>> diff --git a/arch/arm/boot/dts/armada-xp-gp.dts b/arch/arm/boot/dts/armada-xp-gp.dts >>> index 565227e..8a739f4 100644 >>> --- a/arch/arm/boot/dts/armada-xp-gp.dts >>> +++ b/arch/arm/boot/dts/armada-xp-gp.dts >>> @@ -94,7 +94,9 @@ >>> soc { >>> ranges = <MBUS_ID(0xf0, 0x01) 0 0 0xf1000000 0x100000 >>> MBUS_ID(0x01, 0x1d) 0 0 0xfff00000 0x100000 >>> - MBUS_ID(0x01, 0x2f) 0 0 0xf0000000 0x1000000>; >>> + MBUS_ID(0x01, 0x2f) 0 0 0xf0000000 0x1000000 >>> + MBUS_ID(0x09, 0x09) 0 0 0xf1100000 0x10000 >>> + MBUS_ID(0x09, 0x05) 0 0 0xf1110000 0x10000>; >> >> As the crypto engine really depend on the SoC itself and not of the board, >> what about updating the dts of the other board using an Armada XP? > > But that means introducing changes I haven't tested. Are you okay with > that ? Maybe I missed something but as the crypto is fully integrated in the SoC, if for a given SoC it works on a board it would work on all the boards using the same SoC. The board specific part seems about setting memory address on the mbus. By the way could you add a comment in front of the new line ? so next time someone will copy and past one of the dts file, he will understand the signification of these two lines. But is it really depending of the board itself? I see that the first lines are the same on all the dts, I just remember that there was a reason why we could not put it in the dtsi. My point here, is as the configuration is the same on all the boards, adding the crypto on all the board should work without any issue. Thanks, Gregory > >
Dear Gregory CLEMENT, On Wed, 27 May 2015 12:20:49 +0200, Gregory CLEMENT wrote: > But is it really depending of the board itself? > I see that the first lines are the same on all the dts, I just remember that > there was a reason why we could not put it in the dtsi. Yes, because the DT language doesn't have a += operator, basically. Some of the MBus ranges are inherently board-specific: when you have a NOR flash, you need a specific MBus range for it. And such a MBus range is board-specific. Since it's not possible to do: ranges = <SoC level ranges> in .dtsi, and: ranges += <board level ranges> in .dts, we simply decided to always put: ranges = <SoC level and board level ranges> in the .dts. It does create some duplication, but that's the best we could do with the existing DT infrastructure. Best regards, Thomas
Hi Thomas, Boris, On 27/05/2015 13:23, Thomas Petazzoni wrote: > Dear Gregory CLEMENT, > > On Wed, 27 May 2015 12:20:49 +0200, Gregory CLEMENT wrote: > >> But is it really depending of the board itself? >> I see that the first lines are the same on all the dts, I just remember that >> there was a reason why we could not put it in the dtsi. > > Yes, because the DT language doesn't have a += operator, basically. > > Some of the MBus ranges are inherently board-specific: when you have a > NOR flash, you need a specific MBus range for it. And such a MBus range > is board-specific. > > Since it's not possible to do: > > ranges = <SoC level ranges> > > in .dtsi, and: > > ranges += <board level ranges> > > in .dts, we simply decided to always put: > > ranges = <SoC level and board level ranges> > > in the .dts. > > It does create some duplication, but that's the best we could do with > the existing DT infrastructure. Thanks for the remainder. So I think we should duplicate the crypto related part in all the dts file which use an Armada XP SoC. And we don't have to test it again as soon as it was tested on an Armada XP board (and it is the case with the Armada XP one). Gregory > > Best regards, > > Thomas >
Dear Gregory CLEMENT, On Wed, 27 May 2015 13:33:37 +0200, Gregory CLEMENT wrote: > So I think we should duplicate the crypto related part in all the dts > file which use an Armada XP SoC. And we don't have to test it again > as soon as it was tested on an Armada XP board (and it is the case > with the Armada XP one). Yes, I also believe that the board-level changes are trivial enough that if it's been tested to work on a given board using the Armada XP, we can replicate to all Armada XP .dts files and have good confidence that a review is sufficient to say it will work on the other boards. Of course, we need at least one tested board for each SoC, however. Best regards, Thomas
diff --git a/arch/arm/boot/dts/armada-xp-gp.dts b/arch/arm/boot/dts/armada-xp-gp.dts index 565227e..8a739f4 100644 --- a/arch/arm/boot/dts/armada-xp-gp.dts +++ b/arch/arm/boot/dts/armada-xp-gp.dts @@ -94,7 +94,9 @@ soc { ranges = <MBUS_ID(0xf0, 0x01) 0 0 0xf1000000 0x100000 MBUS_ID(0x01, 0x1d) 0 0 0xfff00000 0x100000 - MBUS_ID(0x01, 0x2f) 0 0 0xf0000000 0x1000000>; + MBUS_ID(0x01, 0x2f) 0 0 0xf0000000 0x1000000 + MBUS_ID(0x09, 0x09) 0 0 0xf1100000 0x10000 + MBUS_ID(0x09, 0x05) 0 0 0xf1110000 0x10000>; devbus-bootcs { status = "okay";
Enable the crypto IP on armada-xp-gp. Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com> --- arch/arm/boot/dts/armada-xp-gp.dts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)