[2/3] ARM: dts: sun8i: a33: add the new SecuritySystem compatible
diff mbox series

Message ID 20191114144812.22747-3-clabbe.montjoie@gmail.com
State New
Headers show
Series
  • crypto: sun4i-ss: fix SHA1 on A33 SecuritySystem
Related show

Commit Message

Corentin Labbe Nov. 14, 2019, 2:48 p.m. UTC
Add the new A33 SecuritySystem compatible to the crypto node.

Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
---
 arch/arm/boot/dts/sun8i-a33.dtsi | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Maxime Ripard Nov. 18, 2019, 11:11 a.m. UTC | #1
Hi,

On Thu, Nov 14, 2019 at 03:48:11PM +0100, Corentin Labbe wrote:
> Add the new A33 SecuritySystem compatible to the crypto node.
>
> Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
> ---
>  arch/arm/boot/dts/sun8i-a33.dtsi | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/boot/dts/sun8i-a33.dtsi b/arch/arm/boot/dts/sun8i-a33.dtsi
> index 1532a0e59af4..5680fa1de102 100644
> --- a/arch/arm/boot/dts/sun8i-a33.dtsi
> +++ b/arch/arm/boot/dts/sun8i-a33.dtsi
> @@ -215,7 +215,8 @@
>  		};
>
>  		crypto: crypto-engine@1c15000 {
> -			compatible = "allwinner,sun4i-a10-crypto";
> +			compatible = "allwinner,sun8i-a33-crypto",
> +				     "allwinner,sun4i-a10-crypto";

If some algorithms aren't working properly, we can't really fall back
to it, we should just use the a33 compatible.

Maxime
Corentin Labbe Nov. 19, 2019, 7:39 a.m. UTC | #2
On Mon, Nov 18, 2019 at 12:11:43PM +0100, Maxime Ripard wrote:
> Hi,
> 
> On Thu, Nov 14, 2019 at 03:48:11PM +0100, Corentin Labbe wrote:
> > Add the new A33 SecuritySystem compatible to the crypto node.
> >
> > Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
> > ---
> >  arch/arm/boot/dts/sun8i-a33.dtsi | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/arm/boot/dts/sun8i-a33.dtsi b/arch/arm/boot/dts/sun8i-a33.dtsi
> > index 1532a0e59af4..5680fa1de102 100644
> > --- a/arch/arm/boot/dts/sun8i-a33.dtsi
> > +++ b/arch/arm/boot/dts/sun8i-a33.dtsi
> > @@ -215,7 +215,8 @@
> >  		};
> >
> >  		crypto: crypto-engine@1c15000 {
> > -			compatible = "allwinner,sun4i-a10-crypto";
> > +			compatible = "allwinner,sun8i-a33-crypto",
> > +				     "allwinner,sun4i-a10-crypto";
> 
> If some algorithms aren't working properly, we can't really fall back
> to it, we should just use the a33 compatible.
> 

Since crypto selftest detect the problem, the fallback could be used and SS will just be in degraded mode (no sha1).
But since nobody reported this problem since 4 years (when SS was added in a33 dts), the absence of sha1 is clearly not an issue.

Regards
Maxime Ripard Nov. 20, 2019, 1:45 p.m. UTC | #3
Hi,

On Tue, Nov 19, 2019 at 08:39:24AM +0100, Corentin Labbe wrote:
> On Mon, Nov 18, 2019 at 12:11:43PM +0100, Maxime Ripard wrote:
> > Hi,
> >
> > On Thu, Nov 14, 2019 at 03:48:11PM +0100, Corentin Labbe wrote:
> > > Add the new A33 SecuritySystem compatible to the crypto node.
> > >
> > > Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
> > > ---
> > >  arch/arm/boot/dts/sun8i-a33.dtsi | 3 ++-
> > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/arch/arm/boot/dts/sun8i-a33.dtsi b/arch/arm/boot/dts/sun8i-a33.dtsi
> > > index 1532a0e59af4..5680fa1de102 100644
> > > --- a/arch/arm/boot/dts/sun8i-a33.dtsi
> > > +++ b/arch/arm/boot/dts/sun8i-a33.dtsi
> > > @@ -215,7 +215,8 @@
> > >  		};
> > >
> > >  		crypto: crypto-engine@1c15000 {
> > > -			compatible = "allwinner,sun4i-a10-crypto";
> > > +			compatible = "allwinner,sun8i-a33-crypto",
> > > +				     "allwinner,sun4i-a10-crypto";
> >
> > If some algorithms aren't working properly, we can't really fall back
> > to it, we should just use the a33 compatible.
>
> Since crypto selftest detect the problem, the fallback could be used
> and SS will just be in degraded mode (no sha1).
>
> But since nobody reported this problem since 4 years (when SS was
> added in a33 dts), the absence of sha1 is clearly not an issue.

It's not really the point though. There's a bug, it's something that
was overlooked and it's unfortunate. The bug is still there though,
and the only option to fix it properly is to simply fix, not claim
that it's somewhat ok to keep it around since no one really uses it
anyway.

Maxime

Patch
diff mbox series

diff --git a/arch/arm/boot/dts/sun8i-a33.dtsi b/arch/arm/boot/dts/sun8i-a33.dtsi
index 1532a0e59af4..5680fa1de102 100644
--- a/arch/arm/boot/dts/sun8i-a33.dtsi
+++ b/arch/arm/boot/dts/sun8i-a33.dtsi
@@ -215,7 +215,8 @@ 
 		};
 
 		crypto: crypto-engine@1c15000 {
-			compatible = "allwinner,sun4i-a10-crypto";
+			compatible = "allwinner,sun8i-a33-crypto",
+				     "allwinner,sun4i-a10-crypto";
 			reg = <0x01c15000 0x1000>;
 			interrupts = <GIC_SPI 80 IRQ_TYPE_LEVEL_HIGH>;
 			clocks = <&ccu CLK_BUS_SS>, <&ccu CLK_SS>;