* [PATCH 3/3] arm64: dts: renesas: r8a7795: add ccree binding
@ 2018-05-17 10:16 ` Geert Uytterhoeven
0 siblings, 0 replies; 86+ messages in thread
From: Geert Uytterhoeven @ 2018-05-17 10:16 UTC (permalink / raw)
To: linux-arm-kernel
Hi Gilad,
On Thu, May 17, 2018 at 10:01 AM, Gilad Ben-Yossef <gilad@benyossef.com> wrote:
> On Wed, May 16, 2018 at 10:43 AM, Simon Horman <horms@verge.net.au> wrote:
>> On Tue, May 15, 2018 at 04:50:44PM +0200, Geert Uytterhoeven wrote:
>>> On Tue, May 15, 2018 at 2:29 PM, Gilad Ben-Yossef <gilad@benyossef.com> wrote:
>>> > Add bindings for CryptoCell instance in the SoC.
>>> >
>>> > Signed-off-by: Gilad Ben-Yossef <gilad@benyossef.com>
>>>
>>> Thanks for your patch!
>>>
>>> > --- a/arch/arm64/boot/dts/renesas/r8a7795.dtsi
>>> > +++ b/arch/arm64/boot/dts/renesas/r8a7795.dtsi
>>> > @@ -528,6 +528,14 @@
>>> > status = "disabled";
>>> > };
>>> >
>>> > + arm_cc630p: crypto at e6601000 {
>>> > + compatible = "arm,cryptocell-630p-ree";
>>> > + interrupts = <GIC_SPI 71 IRQ_TYPE_LEVEL_HIGH>;
>>> > + #interrupt-cells = <2>;
>>>
>>> I believe the #interrupt-cells property is not needed.
>>>
>>> > + reg = <0x0 0xe6601000 0 0x1000>;
>>> > + clocks = <&cpg CPG_MOD 229>;
Missing "power-domains = <&sysc R8A7795_PD_ALWAYS_ON>;", as
the Secure Engine is part of the CPG/MSSR clock domain (see below [*]).
>>> > + };
>>>
>>> The rest looks good, but I cannot verify the register block.
>>>
>>> > +
>>> > i2c3: i2c at e66d0000 {
>>> > #address-cells = <1>;
>>> > #size-cells = <0>;
>>
>> Thanks, I have applied this after dropping the #interrupt-cells property.
>
> Thanks you!
>
> Alas, it will not work without the clk patch (the previous one in the
> series) so they need to be
> taken or dropped together.
Indeed. From a quick glance, it looks like drivers/crypto/ccree/cc_driver.c
does not distinguish between the absence of the clock property, and an
actual error in getting the clock, and never considers any error a failure
(incl. -PROBE_DEFER).
As of_clk_get() returns -ENOENT for both a missing clock property and a
missing clock, you should use (devm_)clk_get() instead, and distinguish
between NULL (no clock property) and IS_ERR() (actual failure -> abort).
Hence in the absence of the clock patch, the driver accesses the crypto
engine while its module clock is turned off, leading to:
ccree e6601000.crypto: Invalid CC signature: SIGNATURE=0x00000000
!= expected=0xDCC63000
You must be lucky, though, usually you get an imprecise external abort
later, crashing the whole system ;-)
So I think this patch should be dropped for now.
However, even with your clock patch, the signature checking fails for me,
on both R-Car H3 ES1.0 and ES2.0.
Does this need changes to the ARM Trusted Firmware, to allow Linux to
access the public SCEG module?
[*] More on the subject of clock control:
At least for Renesas SoCs, where the module is part of a clock domain, and
can be controlled automatically by Runtime PM, you could drop the explicit
clock control, and use Runtime PM instead
(pm_runtime_{enable,get_sync,put,disable}()). That would allow the driver
to work on systems with any kind of PM Domains, too.
Depending on the other platforms that include a CryptoCell and their
(non)reliance on PM Domains, you may have to keep the explicit clock
handling, in addition to Runtime PM.
To decrease power consumption, I suggest to move the clock and/or Runtime
PM handling to the routines that actually use the hardware, instead of
powering the module in the probe routine.
Thanks!
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 86+ messages in thread* Re: [PATCH 3/3] arm64: dts: renesas: r8a7795: add ccree binding
@ 2018-05-17 10:16 ` Geert Uytterhoeven
0 siblings, 0 replies; 86+ messages in thread
From: Geert Uytterhoeven @ 2018-05-17 10:16 UTC (permalink / raw)
To: Gilad Ben-Yossef
Cc: Simon Horman, Magnus Damm, Rob Herring, Mark Rutland,
Catalin Marinas, Will Deacon, Geert Uytterhoeven,
Michael Turquette, Stephen Boyd, Herbert Xu, David S. Miller,
Ofir Drang, Linux-Renesas,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Linux ARM, Linux Kernel Mailing List, linux-clk
Hi Gilad,
On Thu, May 17, 2018 at 10:01 AM, Gilad Ben-Yossef <gilad@benyossef.com> wrote:
> On Wed, May 16, 2018 at 10:43 AM, Simon Horman <horms@verge.net.au> wrote:
>> On Tue, May 15, 2018 at 04:50:44PM +0200, Geert Uytterhoeven wrote:
>>> On Tue, May 15, 2018 at 2:29 PM, Gilad Ben-Yossef <gilad@benyossef.com> wrote:
>>> > Add bindings for CryptoCell instance in the SoC.
>>> >
>>> > Signed-off-by: Gilad Ben-Yossef <gilad@benyossef.com>
>>>
>>> Thanks for your patch!
>>>
>>> > --- a/arch/arm64/boot/dts/renesas/r8a7795.dtsi
>>> > +++ b/arch/arm64/boot/dts/renesas/r8a7795.dtsi
>>> > @@ -528,6 +528,14 @@
>>> > status = "disabled";
>>> > };
>>> >
>>> > + arm_cc630p: crypto@e6601000 {
>>> > + compatible = "arm,cryptocell-630p-ree";
>>> > + interrupts = <GIC_SPI 71 IRQ_TYPE_LEVEL_HIGH>;
>>> > + #interrupt-cells = <2>;
>>>
>>> I believe the #interrupt-cells property is not needed.
>>>
>>> > + reg = <0x0 0xe6601000 0 0x1000>;
>>> > + clocks = <&cpg CPG_MOD 229>;
Missing "power-domains = <&sysc R8A7795_PD_ALWAYS_ON>;", as
the Secure Engine is part of the CPG/MSSR clock domain (see below [*]).
>>> > + };
>>>
>>> The rest looks good, but I cannot verify the register block.
>>>
>>> > +
>>> > i2c3: i2c@e66d0000 {
>>> > #address-cells = <1>;
>>> > #size-cells = <0>;
>>
>> Thanks, I have applied this after dropping the #interrupt-cells property.
>
> Thanks you!
>
> Alas, it will not work without the clk patch (the previous one in the
> series) so they need to be
> taken or dropped together.
Indeed. From a quick glance, it looks like drivers/crypto/ccree/cc_driver.c
does not distinguish between the absence of the clock property, and an
actual error in getting the clock, and never considers any error a failure
(incl. -PROBE_DEFER).
As of_clk_get() returns -ENOENT for both a missing clock property and a
missing clock, you should use (devm_)clk_get() instead, and distinguish
between NULL (no clock property) and IS_ERR() (actual failure -> abort).
Hence in the absence of the clock patch, the driver accesses the crypto
engine while its module clock is turned off, leading to:
ccree e6601000.crypto: Invalid CC signature: SIGNATURE=0x00000000
!= expected=0xDCC63000
You must be lucky, though, usually you get an imprecise external abort
later, crashing the whole system ;-)
So I think this patch should be dropped for now.
However, even with your clock patch, the signature checking fails for me,
on both R-Car H3 ES1.0 and ES2.0.
Does this need changes to the ARM Trusted Firmware, to allow Linux to
access the public SCEG module?
[*] More on the subject of clock control:
At least for Renesas SoCs, where the module is part of a clock domain, and
can be controlled automatically by Runtime PM, you could drop the explicit
clock control, and use Runtime PM instead
(pm_runtime_{enable,get_sync,put,disable}()). That would allow the driver
to work on systems with any kind of PM Domains, too.
Depending on the other platforms that include a CryptoCell and their
(non)reliance on PM Domains, you may have to keep the explicit clock
handling, in addition to Runtime PM.
To decrease power consumption, I suggest to move the clock and/or Runtime
PM handling to the routines that actually use the hardware, instead of
powering the module in the probe routine.
Thanks!
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 86+ messages in thread* Re: [PATCH 3/3] arm64: dts: renesas: r8a7795: add ccree binding
@ 2018-05-17 10:16 ` Geert Uytterhoeven
0 siblings, 0 replies; 86+ messages in thread
From: Geert Uytterhoeven @ 2018-05-17 10:16 UTC (permalink / raw)
To: Gilad Ben-Yossef
Cc: Simon Horman, Magnus Damm, Rob Herring, Mark Rutland,
Catalin Marinas, Will Deacon, Geert Uytterhoeven,
Michael Turquette, Stephen Boyd, Herbert Xu, David S. Miller,
Ofir Drang, Linux-Renesas,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Linux ARM, Linux Kernel Mailing List, linux-clk,
Linux Crypto Mailing List
Hi Gilad,
On Thu, May 17, 2018 at 10:01 AM, Gilad Ben-Yossef <gilad@benyossef.com> wrote:
> On Wed, May 16, 2018 at 10:43 AM, Simon Horman <horms@verge.net.au> wrote:
>> On Tue, May 15, 2018 at 04:50:44PM +0200, Geert Uytterhoeven wrote:
>>> On Tue, May 15, 2018 at 2:29 PM, Gilad Ben-Yossef <gilad@benyossef.com> wrote:
>>> > Add bindings for CryptoCell instance in the SoC.
>>> >
>>> > Signed-off-by: Gilad Ben-Yossef <gilad@benyossef.com>
>>>
>>> Thanks for your patch!
>>>
>>> > --- a/arch/arm64/boot/dts/renesas/r8a7795.dtsi
>>> > +++ b/arch/arm64/boot/dts/renesas/r8a7795.dtsi
>>> > @@ -528,6 +528,14 @@
>>> > status = "disabled";
>>> > };
>>> >
>>> > + arm_cc630p: crypto@e6601000 {
>>> > + compatible = "arm,cryptocell-630p-ree";
>>> > + interrupts = <GIC_SPI 71 IRQ_TYPE_LEVEL_HIGH>;
>>> > + #interrupt-cells = <2>;
>>>
>>> I believe the #interrupt-cells property is not needed.
>>>
>>> > + reg = <0x0 0xe6601000 0 0x1000>;
>>> > + clocks = <&cpg CPG_MOD 229>;
Missing "power-domains = <&sysc R8A7795_PD_ALWAYS_ON>;", as
the Secure Engine is part of the CPG/MSSR clock domain (see below [*]).
>>> > + };
>>>
>>> The rest looks good, but I cannot verify the register block.
>>>
>>> > +
>>> > i2c3: i2c@e66d0000 {
>>> > #address-cells = <1>;
>>> > #size-cells = <0>;
>>
>> Thanks, I have applied this after dropping the #interrupt-cells property.
>
> Thanks you!
>
> Alas, it will not work without the clk patch (the previous one in the
> series) so they need to be
> taken or dropped together.
Indeed. From a quick glance, it looks like drivers/crypto/ccree/cc_driver.c
does not distinguish between the absence of the clock property, and an
actual error in getting the clock, and never considers any error a failure
(incl. -PROBE_DEFER).
As of_clk_get() returns -ENOENT for both a missing clock property and a
missing clock, you should use (devm_)clk_get() instead, and distinguish
between NULL (no clock property) and IS_ERR() (actual failure -> abort).
Hence in the absence of the clock patch, the driver accesses the crypto
engine while its module clock is turned off, leading to:
ccree e6601000.crypto: Invalid CC signature: SIGNATURE=0x00000000
!= expected=0xDCC63000
You must be lucky, though, usually you get an imprecise external abort
later, crashing the whole system ;-)
So I think this patch should be dropped for now.
However, even with your clock patch, the signature checking fails for me,
on both R-Car H3 ES1.0 and ES2.0.
Does this need changes to the ARM Trusted Firmware, to allow Linux to
access the public SCEG module?
[*] More on the subject of clock control:
At least for Renesas SoCs, where the module is part of a clock domain, and
can be controlled automatically by Runtime PM, you could drop the explicit
clock control, and use Runtime PM instead
(pm_runtime_{enable,get_sync,put,disable}()). That would allow the driver
to work on systems with any kind of PM Domains, too.
Depending on the other platforms that include a CryptoCell and their
(non)reliance on PM Domains, you may have to keep the explicit clock
handling, in addition to Runtime PM.
To decrease power consumption, I suggest to move the clock and/or Runtime
PM handling to the routines that actually use the hardware, instead of
powering the module in the probe routine.
Thanks!
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 86+ messages in thread* Re: [PATCH 3/3] arm64: dts: renesas: r8a7795: add ccree binding
2018-05-17 10:16 ` Geert Uytterhoeven
(?)
(?)
@ 2018-05-17 13:09 ` Gilad Ben-Yossef
-1 siblings, 0 replies; 86+ messages in thread
From: Gilad Ben-Yossef @ 2018-05-17 13:09 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Simon Horman, Magnus Damm, Rob Herring, Mark Rutland,
Catalin Marinas, Will Deacon, Geert Uytterhoeven,
Michael Turquette, Stephen Boyd, Herbert Xu, David S. Miller,
Ofir Drang, Linux-Renesas,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Linux ARM, Linux Kernel Mailing List, linux-clk, L
On Thu, May 17, 2018 at 1:16 PM, Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
> Hi Gilad,
>
> On Thu, May 17, 2018 at 10:01 AM, Gilad Ben-Yossef <gilad@benyossef.com> wrote:
>> On Wed, May 16, 2018 at 10:43 AM, Simon Horman <horms@verge.net.au> wrote:
>>> On Tue, May 15, 2018 at 04:50:44PM +0200, Geert Uytterhoeven wrote:
>>>> On Tue, May 15, 2018 at 2:29 PM, Gilad Ben-Yossef <gilad@benyossef.com> wrote:
>>>> > Add bindings for CryptoCell instance in the SoC.
>>>> >
>>>> > Signed-off-by: Gilad Ben-Yossef <gilad@benyossef.com>
>>>>
>>>> Thanks for your patch!
>>>>
>>>> > --- a/arch/arm64/boot/dts/renesas/r8a7795.dtsi
>>>> > +++ b/arch/arm64/boot/dts/renesas/r8a7795.dtsi
>>>> > @@ -528,6 +528,14 @@
>>>> > status = "disabled";
>>>> > };
>>>> >
>>>> > + arm_cc630p: crypto@e6601000 {
>>>> > + compatible = "arm,cryptocell-630p-ree";
>>>> > + interrupts = <GIC_SPI 71 IRQ_TYPE_LEVEL_HIGH>;
>>>> > + #interrupt-cells = <2>;
>>>>
>>>> I believe the #interrupt-cells property is not needed.
>>>>
>>>> > + reg = <0x0 0xe6601000 0 0x1000>;
>>>> > + clocks = <&cpg CPG_MOD 229>;
>
> Missing "power-domains = <&sysc R8A7795_PD_ALWAYS_ON>;", as
> the Secure Engine is part of the CPG/MSSR clock domain (see below [*]).
Thank you. I didn't get this information from Renesas :-)
>
>>>> > + };
>>>>
>>>> The rest looks good, but I cannot verify the register block.
>>>>
>>>> > +
>>>> > i2c3: i2c@e66d0000 {
>>>> > #address-cells = <1>;
>>>> > #size-cells = <0>;
>>>
>>> Thanks, I have applied this after dropping the #interrupt-cells property.
>>
>> Thanks you!
>>
>> Alas, it will not work without the clk patch (the previous one in the
>> series) so they need to be
>> taken or dropped together.
>
> Indeed. From a quick glance, it looks like drivers/crypto/ccree/cc_driver.c
> does not distinguish between the absence of the clock property, and an
> actual error in getting the clock, and never considers any error a failure
> (incl. -PROBE_DEFER).
>
> As of_clk_get() returns -ENOENT for both a missing clock property and a
> missing clock, you should use (devm_)clk_get() instead, and distinguish
> between NULL (no clock property) and IS_ERR() (actual failure -> abort).
>
Thank you, this is very valuable. I will do as you suggested.
> Hence in the absence of the clock patch, the driver accesses the crypto
> engine while its module clock is turned off, leading to:
>
> ccree e6601000.crypto: Invalid CC signature: SIGNATURE=0x00000000
> != expected=0xDCC63000
>
> You must be lucky, though, usually you get an imprecise external abort
> later, crashing the whole system ;-)
>
> So I think this patch should be dropped for now.
>
> However, even with your clock patch, the signature checking fails for me,
> on both R-Car H3 ES1.0 and ES2.0.
> Does this need changes to the ARM Trusted Firmware, to allow Linux to
> access the public SCEG module?
Well, this is actually something different. If you look you will
notice that my patch was part of a 3 part patch series,
the first of which disabled this test.
If you take all the 3 patches, it will work.
To make things more interesting, I have since sending the patch
learned WHY the test does not work, so disabling
it is not needed - to make a long story short, I was reading the wrong
register that just happens to have the right value
in our FPGA based tests systems but does not in the real silicon
implementations.
But you are right - if the clock is not enabled and you are try to
read from the register the system does freeze.
I will send a fixed v2. based on your patch enabling the CR clock.
>
> [*] More on the subject of clock control:
> At least for Renesas SoCs, where the module is part of a clock domain, and
> can be controlled automatically by Runtime PM, you could drop the explicit
> clock control, and use Runtime PM instead
> (pm_runtime_{enable,get_sync,put,disable}()). That would allow the driver
> to work on systems with any kind of PM Domains, too.
> Depending on the other platforms that include a CryptoCell and their
> (non)reliance on PM Domains, you may have to keep the explicit clock
> handling, in addition to Runtime PM.
>
> To decrease power consumption, I suggest to move the clock and/or Runtime
> PM handling to the routines that actually use the hardware, instead of
> powering the module in the probe routine.
>
This is very interesting and I will give it a try.
Thanks again!
Gilad
--
Gilad Ben-Yossef
Chief Coffee Drinker
"If you take a class in large-scale robotics, can you end up in a
situation where the homework eats your dog?"
-- Jean-Baptiste Queru
^ permalink raw reply [flat|nested] 86+ messages in thread* [PATCH 3/3] arm64: dts: renesas: r8a7795: add ccree binding
@ 2018-05-17 13:09 ` Gilad Ben-Yossef
0 siblings, 0 replies; 86+ messages in thread
From: Gilad Ben-Yossef @ 2018-05-17 13:09 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, May 17, 2018 at 1:16 PM, Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
> Hi Gilad,
>
> On Thu, May 17, 2018 at 10:01 AM, Gilad Ben-Yossef <gilad@benyossef.com> wrote:
>> On Wed, May 16, 2018 at 10:43 AM, Simon Horman <horms@verge.net.au> wrote:
>>> On Tue, May 15, 2018 at 04:50:44PM +0200, Geert Uytterhoeven wrote:
>>>> On Tue, May 15, 2018 at 2:29 PM, Gilad Ben-Yossef <gilad@benyossef.com> wrote:
>>>> > Add bindings for CryptoCell instance in the SoC.
>>>> >
>>>> > Signed-off-by: Gilad Ben-Yossef <gilad@benyossef.com>
>>>>
>>>> Thanks for your patch!
>>>>
>>>> > --- a/arch/arm64/boot/dts/renesas/r8a7795.dtsi
>>>> > +++ b/arch/arm64/boot/dts/renesas/r8a7795.dtsi
>>>> > @@ -528,6 +528,14 @@
>>>> > status = "disabled";
>>>> > };
>>>> >
>>>> > + arm_cc630p: crypto at e6601000 {
>>>> > + compatible = "arm,cryptocell-630p-ree";
>>>> > + interrupts = <GIC_SPI 71 IRQ_TYPE_LEVEL_HIGH>;
>>>> > + #interrupt-cells = <2>;
>>>>
>>>> I believe the #interrupt-cells property is not needed.
>>>>
>>>> > + reg = <0x0 0xe6601000 0 0x1000>;
>>>> > + clocks = <&cpg CPG_MOD 229>;
>
> Missing "power-domains = <&sysc R8A7795_PD_ALWAYS_ON>;", as
> the Secure Engine is part of the CPG/MSSR clock domain (see below [*]).
Thank you. I didn't get this information from Renesas :-)
>
>>>> > + };
>>>>
>>>> The rest looks good, but I cannot verify the register block.
>>>>
>>>> > +
>>>> > i2c3: i2c at e66d0000 {
>>>> > #address-cells = <1>;
>>>> > #size-cells = <0>;
>>>
>>> Thanks, I have applied this after dropping the #interrupt-cells property.
>>
>> Thanks you!
>>
>> Alas, it will not work without the clk patch (the previous one in the
>> series) so they need to be
>> taken or dropped together.
>
> Indeed. From a quick glance, it looks like drivers/crypto/ccree/cc_driver.c
> does not distinguish between the absence of the clock property, and an
> actual error in getting the clock, and never considers any error a failure
> (incl. -PROBE_DEFER).
>
> As of_clk_get() returns -ENOENT for both a missing clock property and a
> missing clock, you should use (devm_)clk_get() instead, and distinguish
> between NULL (no clock property) and IS_ERR() (actual failure -> abort).
>
Thank you, this is very valuable. I will do as you suggested.
> Hence in the absence of the clock patch, the driver accesses the crypto
> engine while its module clock is turned off, leading to:
>
> ccree e6601000.crypto: Invalid CC signature: SIGNATURE=0x00000000
> != expected=0xDCC63000
>
> You must be lucky, though, usually you get an imprecise external abort
> later, crashing the whole system ;-)
>
> So I think this patch should be dropped for now.
>
> However, even with your clock patch, the signature checking fails for me,
> on both R-Car H3 ES1.0 and ES2.0.
> Does this need changes to the ARM Trusted Firmware, to allow Linux to
> access the public SCEG module?
Well, this is actually something different. If you look you will
notice that my patch was part of a 3 part patch series,
the first of which disabled this test.
If you take all the 3 patches, it will work.
To make things more interesting, I have since sending the patch
learned WHY the test does not work, so disabling
it is not needed - to make a long story short, I was reading the wrong
register that just happens to have the right value
in our FPGA based tests systems but does not in the real silicon
implementations.
But you are right - if the clock is not enabled and you are try to
read from the register the system does freeze.
I will send a fixed v2. based on your patch enabling the CR clock.
>
> [*] More on the subject of clock control:
> At least for Renesas SoCs, where the module is part of a clock domain, and
> can be controlled automatically by Runtime PM, you could drop the explicit
> clock control, and use Runtime PM instead
> (pm_runtime_{enable,get_sync,put,disable}()). That would allow the driver
> to work on systems with any kind of PM Domains, too.
> Depending on the other platforms that include a CryptoCell and their
> (non)reliance on PM Domains, you may have to keep the explicit clock
> handling, in addition to Runtime PM.
>
> To decrease power consumption, I suggest to move the clock and/or Runtime
> PM handling to the routines that actually use the hardware, instead of
> powering the module in the probe routine.
>
This is very interesting and I will give it a try.
Thanks again!
Gilad
--
Gilad Ben-Yossef
Chief Coffee Drinker
"If you take a class in large-scale robotics, can you end up in a
situation where the homework eats your dog?"
-- Jean-Baptiste Queru
^ permalink raw reply [flat|nested] 86+ messages in thread* Re: [PATCH 3/3] arm64: dts: renesas: r8a7795: add ccree binding
@ 2018-05-17 13:09 ` Gilad Ben-Yossef
0 siblings, 0 replies; 86+ messages in thread
From: Gilad Ben-Yossef @ 2018-05-17 13:09 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Simon Horman, Magnus Damm, Rob Herring, Mark Rutland,
Catalin Marinas, Will Deacon, Geert Uytterhoeven,
Michael Turquette, Stephen Boyd, Herbert Xu, David S. Miller,
Ofir Drang, Linux-Renesas,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Linux ARM, Linux Kernel Mailing List, linux-clk
On Thu, May 17, 2018 at 1:16 PM, Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
> Hi Gilad,
>
> On Thu, May 17, 2018 at 10:01 AM, Gilad Ben-Yossef <gilad@benyossef.com> wrote:
>> On Wed, May 16, 2018 at 10:43 AM, Simon Horman <horms@verge.net.au> wrote:
>>> On Tue, May 15, 2018 at 04:50:44PM +0200, Geert Uytterhoeven wrote:
>>>> On Tue, May 15, 2018 at 2:29 PM, Gilad Ben-Yossef <gilad@benyossef.com> wrote:
>>>> > Add bindings for CryptoCell instance in the SoC.
>>>> >
>>>> > Signed-off-by: Gilad Ben-Yossef <gilad@benyossef.com>
>>>>
>>>> Thanks for your patch!
>>>>
>>>> > --- a/arch/arm64/boot/dts/renesas/r8a7795.dtsi
>>>> > +++ b/arch/arm64/boot/dts/renesas/r8a7795.dtsi
>>>> > @@ -528,6 +528,14 @@
>>>> > status = "disabled";
>>>> > };
>>>> >
>>>> > + arm_cc630p: crypto@e6601000 {
>>>> > + compatible = "arm,cryptocell-630p-ree";
>>>> > + interrupts = <GIC_SPI 71 IRQ_TYPE_LEVEL_HIGH>;
>>>> > + #interrupt-cells = <2>;
>>>>
>>>> I believe the #interrupt-cells property is not needed.
>>>>
>>>> > + reg = <0x0 0xe6601000 0 0x1000>;
>>>> > + clocks = <&cpg CPG_MOD 229>;
>
> Missing "power-domains = <&sysc R8A7795_PD_ALWAYS_ON>;", as
> the Secure Engine is part of the CPG/MSSR clock domain (see below [*]).
Thank you. I didn't get this information from Renesas :-)
>
>>>> > + };
>>>>
>>>> The rest looks good, but I cannot verify the register block.
>>>>
>>>> > +
>>>> > i2c3: i2c@e66d0000 {
>>>> > #address-cells = <1>;
>>>> > #size-cells = <0>;
>>>
>>> Thanks, I have applied this after dropping the #interrupt-cells property.
>>
>> Thanks you!
>>
>> Alas, it will not work without the clk patch (the previous one in the
>> series) so they need to be
>> taken or dropped together.
>
> Indeed. From a quick glance, it looks like drivers/crypto/ccree/cc_driver.c
> does not distinguish between the absence of the clock property, and an
> actual error in getting the clock, and never considers any error a failure
> (incl. -PROBE_DEFER).
>
> As of_clk_get() returns -ENOENT for both a missing clock property and a
> missing clock, you should use (devm_)clk_get() instead, and distinguish
> between NULL (no clock property) and IS_ERR() (actual failure -> abort).
>
Thank you, this is very valuable. I will do as you suggested.
> Hence in the absence of the clock patch, the driver accesses the crypto
> engine while its module clock is turned off, leading to:
>
> ccree e6601000.crypto: Invalid CC signature: SIGNATURE=0x00000000
> != expected=0xDCC63000
>
> You must be lucky, though, usually you get an imprecise external abort
> later, crashing the whole system ;-)
>
> So I think this patch should be dropped for now.
>
> However, even with your clock patch, the signature checking fails for me,
> on both R-Car H3 ES1.0 and ES2.0.
> Does this need changes to the ARM Trusted Firmware, to allow Linux to
> access the public SCEG module?
Well, this is actually something different. If you look you will
notice that my patch was part of a 3 part patch series,
the first of which disabled this test.
If you take all the 3 patches, it will work.
To make things more interesting, I have since sending the patch
learned WHY the test does not work, so disabling
it is not needed - to make a long story short, I was reading the wrong
register that just happens to have the right value
in our FPGA based tests systems but does not in the real silicon
implementations.
But you are right - if the clock is not enabled and you are try to
read from the register the system does freeze.
I will send a fixed v2. based on your patch enabling the CR clock.
>
> [*] More on the subject of clock control:
> At least for Renesas SoCs, where the module is part of a clock domain, and
> can be controlled automatically by Runtime PM, you could drop the explicit
> clock control, and use Runtime PM instead
> (pm_runtime_{enable,get_sync,put,disable}()). That would allow the driver
> to work on systems with any kind of PM Domains, too.
> Depending on the other platforms that include a CryptoCell and their
> (non)reliance on PM Domains, you may have to keep the explicit clock
> handling, in addition to Runtime PM.
>
> To decrease power consumption, I suggest to move the clock and/or Runtime
> PM handling to the routines that actually use the hardware, instead of
> powering the module in the probe routine.
>
This is very interesting and I will give it a try.
Thanks again!
Gilad
--
Gilad Ben-Yossef
Chief Coffee Drinker
"If you take a class in large-scale robotics, can you end up in a
situation where the homework eats your dog?"
-- Jean-Baptiste Queru
^ permalink raw reply [flat|nested] 86+ messages in thread* Re: [PATCH 3/3] arm64: dts: renesas: r8a7795: add ccree binding
@ 2018-05-17 13:09 ` Gilad Ben-Yossef
0 siblings, 0 replies; 86+ messages in thread
From: Gilad Ben-Yossef @ 2018-05-17 13:09 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Simon Horman, Magnus Damm, Rob Herring, Mark Rutland,
Catalin Marinas, Will Deacon, Geert Uytterhoeven,
Michael Turquette, Stephen Boyd, Herbert Xu, David S. Miller,
Ofir Drang, Linux-Renesas,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Linux ARM, Linux Kernel Mailing List, linux-clk,
Linux Crypto Mailing List
On Thu, May 17, 2018 at 1:16 PM, Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
> Hi Gilad,
>
> On Thu, May 17, 2018 at 10:01 AM, Gilad Ben-Yossef <gilad@benyossef.com> wrote:
>> On Wed, May 16, 2018 at 10:43 AM, Simon Horman <horms@verge.net.au> wrote:
>>> On Tue, May 15, 2018 at 04:50:44PM +0200, Geert Uytterhoeven wrote:
>>>> On Tue, May 15, 2018 at 2:29 PM, Gilad Ben-Yossef <gilad@benyossef.com> wrote:
>>>> > Add bindings for CryptoCell instance in the SoC.
>>>> >
>>>> > Signed-off-by: Gilad Ben-Yossef <gilad@benyossef.com>
>>>>
>>>> Thanks for your patch!
>>>>
>>>> > --- a/arch/arm64/boot/dts/renesas/r8a7795.dtsi
>>>> > +++ b/arch/arm64/boot/dts/renesas/r8a7795.dtsi
>>>> > @@ -528,6 +528,14 @@
>>>> > status = "disabled";
>>>> > };
>>>> >
>>>> > + arm_cc630p: crypto@e6601000 {
>>>> > + compatible = "arm,cryptocell-630p-ree";
>>>> > + interrupts = <GIC_SPI 71 IRQ_TYPE_LEVEL_HIGH>;
>>>> > + #interrupt-cells = <2>;
>>>>
>>>> I believe the #interrupt-cells property is not needed.
>>>>
>>>> > + reg = <0x0 0xe6601000 0 0x1000>;
>>>> > + clocks = <&cpg CPG_MOD 229>;
>
> Missing "power-domains = <&sysc R8A7795_PD_ALWAYS_ON>;", as
> the Secure Engine is part of the CPG/MSSR clock domain (see below [*]).
Thank you. I didn't get this information from Renesas :-)
>
>>>> > + };
>>>>
>>>> The rest looks good, but I cannot verify the register block.
>>>>
>>>> > +
>>>> > i2c3: i2c@e66d0000 {
>>>> > #address-cells = <1>;
>>>> > #size-cells = <0>;
>>>
>>> Thanks, I have applied this after dropping the #interrupt-cells property.
>>
>> Thanks you!
>>
>> Alas, it will not work without the clk patch (the previous one in the
>> series) so they need to be
>> taken or dropped together.
>
> Indeed. From a quick glance, it looks like drivers/crypto/ccree/cc_driver.c
> does not distinguish between the absence of the clock property, and an
> actual error in getting the clock, and never considers any error a failure
> (incl. -PROBE_DEFER).
>
> As of_clk_get() returns -ENOENT for both a missing clock property and a
> missing clock, you should use (devm_)clk_get() instead, and distinguish
> between NULL (no clock property) and IS_ERR() (actual failure -> abort).
>
Thank you, this is very valuable. I will do as you suggested.
> Hence in the absence of the clock patch, the driver accesses the crypto
> engine while its module clock is turned off, leading to:
>
> ccree e6601000.crypto: Invalid CC signature: SIGNATURE=0x00000000
> != expected=0xDCC63000
>
> You must be lucky, though, usually you get an imprecise external abort
> later, crashing the whole system ;-)
>
> So I think this patch should be dropped for now.
>
> However, even with your clock patch, the signature checking fails for me,
> on both R-Car H3 ES1.0 and ES2.0.
> Does this need changes to the ARM Trusted Firmware, to allow Linux to
> access the public SCEG module?
Well, this is actually something different. If you look you will
notice that my patch was part of a 3 part patch series,
the first of which disabled this test.
If you take all the 3 patches, it will work.
To make things more interesting, I have since sending the patch
learned WHY the test does not work, so disabling
it is not needed - to make a long story short, I was reading the wrong
register that just happens to have the right value
in our FPGA based tests systems but does not in the real silicon
implementations.
But you are right - if the clock is not enabled and you are try to
read from the register the system does freeze.
I will send a fixed v2. based on your patch enabling the CR clock.
>
> [*] More on the subject of clock control:
> At least for Renesas SoCs, where the module is part of a clock domain, and
> can be controlled automatically by Runtime PM, you could drop the explicit
> clock control, and use Runtime PM instead
> (pm_runtime_{enable,get_sync,put,disable}()). That would allow the driver
> to work on systems with any kind of PM Domains, too.
> Depending on the other platforms that include a CryptoCell and their
> (non)reliance on PM Domains, you may have to keep the explicit clock
> handling, in addition to Runtime PM.
>
> To decrease power consumption, I suggest to move the clock and/or Runtime
> PM handling to the routines that actually use the hardware, instead of
> powering the module in the probe routine.
>
This is very interesting and I will give it a try.
Thanks again!
Gilad
--
Gilad Ben-Yossef
Chief Coffee Drinker
"If you take a class in large-scale robotics, can you end up in a
situation where the homework eats your dog?"
-- Jean-Baptiste Queru
^ permalink raw reply [flat|nested] 86+ messages in thread* Re: [PATCH 3/3] arm64: dts: renesas: r8a7795: add ccree binding
2018-05-17 13:09 ` Gilad Ben-Yossef
(?)
(?)
@ 2018-05-17 13:35 ` Geert Uytterhoeven
-1 siblings, 0 replies; 86+ messages in thread
From: Geert Uytterhoeven @ 2018-05-17 13:35 UTC (permalink / raw)
To: Gilad Ben-Yossef
Cc: Simon Horman, Magnus Damm, Rob Herring, Mark Rutland,
Catalin Marinas, Will Deacon, Geert Uytterhoeven,
Michael Turquette, Stephen Boyd, Herbert Xu, David S. Miller,
Ofir Drang, Linux-Renesas,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Linux ARM, Linux Kernel Mailing List, linux-clk, L
Hi Gilad,
On Thu, May 17, 2018 at 3:09 PM, Gilad Ben-Yossef <gilad@benyossef.com> wrote:
> On Thu, May 17, 2018 at 1:16 PM, Geert Uytterhoeven
> <geert@linux-m68k.org> wrote:
>> However, even with your clock patch, the signature checking fails for me,
>> on both R-Car H3 ES1.0 and ES2.0.
>> Does this need changes to the ARM Trusted Firmware, to allow Linux to
>> access the public SCEG module?
>
> Well, this is actually something different. If you look you will
> notice that my patch was part of a 3 part patch series,
> the first of which disabled this test.
Sorry, I had completely forgotten about the first patch from the series.
With that applied, it continues:
ccree e6601000.crypto: ARM CryptoCell 630P Driver: HW version
0x00000000, Driver version 4.0
ccree e6601000.crypto: Cache params previous: 0x00000777
ccree e6601000.crypto: Cache params current: 0x00000000
(expect: 0x00000000)
alg: No test for cts1(cbc(aes)) (cts1-cbc-aes-ccree)
alg: No test for authenc(xcbc(aes),cbc(aes))
(authenc-xcbc-aes-cbc-aes-ccree)
alg: No test for authenc(xcbc(aes),rfc3686(ctr(aes)))
(authenc-xcbc-aes-rfc3686-ctr-aes-ccree)
ccree e6601000.crypto: ARM ccree device initialized
Is HW version 0x00000000 expected?
> If you take all the 3 patches, it will work.
is there an easy way to test proper operation?
I enabled CONFIG_CRYPT_TEST, and did insmod tcrypt.ko, but I mostly see
"Failed to load transform" messages.
Thanks!
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 86+ messages in thread* [PATCH 3/3] arm64: dts: renesas: r8a7795: add ccree binding
@ 2018-05-17 13:35 ` Geert Uytterhoeven
0 siblings, 0 replies; 86+ messages in thread
From: Geert Uytterhoeven @ 2018-05-17 13:35 UTC (permalink / raw)
To: linux-arm-kernel
Hi Gilad,
On Thu, May 17, 2018 at 3:09 PM, Gilad Ben-Yossef <gilad@benyossef.com> wrote:
> On Thu, May 17, 2018 at 1:16 PM, Geert Uytterhoeven
> <geert@linux-m68k.org> wrote:
>> However, even with your clock patch, the signature checking fails for me,
>> on both R-Car H3 ES1.0 and ES2.0.
>> Does this need changes to the ARM Trusted Firmware, to allow Linux to
>> access the public SCEG module?
>
> Well, this is actually something different. If you look you will
> notice that my patch was part of a 3 part patch series,
> the first of which disabled this test.
Sorry, I had completely forgotten about the first patch from the series.
With that applied, it continues:
ccree e6601000.crypto: ARM CryptoCell 630P Driver: HW version
0x00000000, Driver version 4.0
ccree e6601000.crypto: Cache params previous: 0x00000777
ccree e6601000.crypto: Cache params current: 0x00000000
(expect: 0x00000000)
alg: No test for cts1(cbc(aes)) (cts1-cbc-aes-ccree)
alg: No test for authenc(xcbc(aes),cbc(aes))
(authenc-xcbc-aes-cbc-aes-ccree)
alg: No test for authenc(xcbc(aes),rfc3686(ctr(aes)))
(authenc-xcbc-aes-rfc3686-ctr-aes-ccree)
ccree e6601000.crypto: ARM ccree device initialized
Is HW version 0x00000000 expected?
> If you take all the 3 patches, it will work.
is there an easy way to test proper operation?
I enabled CONFIG_CRYPT_TEST, and did insmod tcrypt.ko, but I mostly see
"Failed to load transform" messages.
Thanks!
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 86+ messages in thread* Re: [PATCH 3/3] arm64: dts: renesas: r8a7795: add ccree binding
@ 2018-05-17 13:35 ` Geert Uytterhoeven
0 siblings, 0 replies; 86+ messages in thread
From: Geert Uytterhoeven @ 2018-05-17 13:35 UTC (permalink / raw)
To: Gilad Ben-Yossef
Cc: Simon Horman, Magnus Damm, Rob Herring, Mark Rutland,
Catalin Marinas, Will Deacon, Geert Uytterhoeven,
Michael Turquette, Stephen Boyd, Herbert Xu, David S. Miller,
Ofir Drang, Linux-Renesas,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Linux ARM, Linux Kernel Mailing List, linux-clk
Hi Gilad,
On Thu, May 17, 2018 at 3:09 PM, Gilad Ben-Yossef <gilad@benyossef.com> wrote:
> On Thu, May 17, 2018 at 1:16 PM, Geert Uytterhoeven
> <geert@linux-m68k.org> wrote:
>> However, even with your clock patch, the signature checking fails for me,
>> on both R-Car H3 ES1.0 and ES2.0.
>> Does this need changes to the ARM Trusted Firmware, to allow Linux to
>> access the public SCEG module?
>
> Well, this is actually something different. If you look you will
> notice that my patch was part of a 3 part patch series,
> the first of which disabled this test.
Sorry, I had completely forgotten about the first patch from the series.
With that applied, it continues:
ccree e6601000.crypto: ARM CryptoCell 630P Driver: HW version
0x00000000, Driver version 4.0
ccree e6601000.crypto: Cache params previous: 0x00000777
ccree e6601000.crypto: Cache params current: 0x00000000
(expect: 0x00000000)
alg: No test for cts1(cbc(aes)) (cts1-cbc-aes-ccree)
alg: No test for authenc(xcbc(aes),cbc(aes))
(authenc-xcbc-aes-cbc-aes-ccree)
alg: No test for authenc(xcbc(aes),rfc3686(ctr(aes)))
(authenc-xcbc-aes-rfc3686-ctr-aes-ccree)
ccree e6601000.crypto: ARM ccree device initialized
Is HW version 0x00000000 expected?
> If you take all the 3 patches, it will work.
is there an easy way to test proper operation?
I enabled CONFIG_CRYPT_TEST, and did insmod tcrypt.ko, but I mostly see
"Failed to load transform" messages.
Thanks!
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 86+ messages in thread* Re: [PATCH 3/3] arm64: dts: renesas: r8a7795: add ccree binding
@ 2018-05-17 13:35 ` Geert Uytterhoeven
0 siblings, 0 replies; 86+ messages in thread
From: Geert Uytterhoeven @ 2018-05-17 13:35 UTC (permalink / raw)
To: Gilad Ben-Yossef
Cc: Simon Horman, Magnus Damm, Rob Herring, Mark Rutland,
Catalin Marinas, Will Deacon, Geert Uytterhoeven,
Michael Turquette, Stephen Boyd, Herbert Xu, David S. Miller,
Ofir Drang, Linux-Renesas,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Linux ARM, Linux Kernel Mailing List, linux-clk,
Linux Crypto Mailing List
Hi Gilad,
On Thu, May 17, 2018 at 3:09 PM, Gilad Ben-Yossef <gilad@benyossef.com> wrote:
> On Thu, May 17, 2018 at 1:16 PM, Geert Uytterhoeven
> <geert@linux-m68k.org> wrote:
>> However, even with your clock patch, the signature checking fails for me,
>> on both R-Car H3 ES1.0 and ES2.0.
>> Does this need changes to the ARM Trusted Firmware, to allow Linux to
>> access the public SCEG module?
>
> Well, this is actually something different. If you look you will
> notice that my patch was part of a 3 part patch series,
> the first of which disabled this test.
Sorry, I had completely forgotten about the first patch from the series.
With that applied, it continues:
ccree e6601000.crypto: ARM CryptoCell 630P Driver: HW version
0x00000000, Driver version 4.0
ccree e6601000.crypto: Cache params previous: 0x00000777
ccree e6601000.crypto: Cache params current: 0x00000000
(expect: 0x00000000)
alg: No test for cts1(cbc(aes)) (cts1-cbc-aes-ccree)
alg: No test for authenc(xcbc(aes),cbc(aes))
(authenc-xcbc-aes-cbc-aes-ccree)
alg: No test for authenc(xcbc(aes),rfc3686(ctr(aes)))
(authenc-xcbc-aes-rfc3686-ctr-aes-ccree)
ccree e6601000.crypto: ARM ccree device initialized
Is HW version 0x00000000 expected?
> If you take all the 3 patches, it will work.
is there an easy way to test proper operation?
I enabled CONFIG_CRYPT_TEST, and did insmod tcrypt.ko, but I mostly see
"Failed to load transform" messages.
Thanks!
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 86+ messages in thread* Re: [PATCH 3/3] arm64: dts: renesas: r8a7795: add ccree binding
2018-05-17 13:35 ` Geert Uytterhoeven
(?)
(?)
@ 2018-05-17 13:41 ` Gilad Ben-Yossef
-1 siblings, 0 replies; 86+ messages in thread
From: Gilad Ben-Yossef @ 2018-05-17 13:41 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Simon Horman, Magnus Damm, Rob Herring, Mark Rutland,
Catalin Marinas, Will Deacon, Geert Uytterhoeven,
Michael Turquette, Stephen Boyd, Herbert Xu, David S. Miller,
Ofir Drang, Linux-Renesas,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Linux ARM, Linux Kernel Mailing List, linux-clk, L
On Thu, May 17, 2018 at 4:35 PM, Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
> Hi Gilad,
>
> On Thu, May 17, 2018 at 3:09 PM, Gilad Ben-Yossef <gilad@benyossef.com> wrote:
>> On Thu, May 17, 2018 at 1:16 PM, Geert Uytterhoeven
>> <geert@linux-m68k.org> wrote:
>>> However, even with your clock patch, the signature checking fails for me,
>>> on both R-Car H3 ES1.0 and ES2.0.
>>> Does this need changes to the ARM Trusted Firmware, to allow Linux to
>>> access the public SCEG module?
>>
>> Well, this is actually something different. If you look you will
>> notice that my patch was part of a 3 part patch series,
>> the first of which disabled this test.
>
> Sorry, I had completely forgotten about the first patch from the series.
> With that applied, it continues:
>
> ccree e6601000.crypto: ARM CryptoCell 630P Driver: HW version
> 0x00000000, Driver version 4.0
> ccree e6601000.crypto: Cache params previous: 0x00000777
> ccree e6601000.crypto: Cache params current: 0x00000000
> (expect: 0x00000000)
> alg: No test for cts1(cbc(aes)) (cts1-cbc-aes-ccree)
> alg: No test for authenc(xcbc(aes),cbc(aes))
> (authenc-xcbc-aes-cbc-aes-ccree)
> alg: No test for authenc(xcbc(aes),rfc3686(ctr(aes)))
> (authenc-xcbc-aes-rfc3686-ctr-aes-ccree)
> ccree e6601000.crypto: ARM ccree device initialized
>
> Is HW version 0x00000000 expected?
It's related to the problem with reading the wrong register I've
mentioned before.
>
>> If you take all the 3 patches, it will work.
>
> is there an easy way to test proper operation?
The lines of the form " alg: No test for cts1(cbc(aes))
(cts1-cbc-aes-ccree)" indicates
you have the Crypto API testmgr enable (or rather not disabled would
be more accurate) so every
cryptographic algorithm except those specified in these messages was
tested with test
vectors from crypto/testmgr.c upon registration. If you don't seen
failure warnings, it works.
You can also check /proc/crypto for all the algorithm with ccree
listed as their driver and check
that their test passed.
> I enabled CONFIG_CRYPT_TEST, and did insmod tcrypt.ko, but I mostly see
> "Failed to load transform" messages.
>
tcrypt.ko is a rather crude developer tool. It has hard coded lists of
test for different algorithms that does
not take into account if some crypto algs are enagled in the build or
not. It's more of a stress test.
Gilad
--
Gilad Ben-Yossef
Chief Coffee Drinker
"If you take a class in large-scale robotics, can you end up in a
situation where the homework eats your dog?"
-- Jean-Baptiste Queru
^ permalink raw reply [flat|nested] 86+ messages in thread
* [PATCH 3/3] arm64: dts: renesas: r8a7795: add ccree binding
@ 2018-05-17 13:41 ` Gilad Ben-Yossef
0 siblings, 0 replies; 86+ messages in thread
From: Gilad Ben-Yossef @ 2018-05-17 13:41 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, May 17, 2018 at 4:35 PM, Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
> Hi Gilad,
>
> On Thu, May 17, 2018 at 3:09 PM, Gilad Ben-Yossef <gilad@benyossef.com> wrote:
>> On Thu, May 17, 2018 at 1:16 PM, Geert Uytterhoeven
>> <geert@linux-m68k.org> wrote:
>>> However, even with your clock patch, the signature checking fails for me,
>>> on both R-Car H3 ES1.0 and ES2.0.
>>> Does this need changes to the ARM Trusted Firmware, to allow Linux to
>>> access the public SCEG module?
>>
>> Well, this is actually something different. If you look you will
>> notice that my patch was part of a 3 part patch series,
>> the first of which disabled this test.
>
> Sorry, I had completely forgotten about the first patch from the series.
> With that applied, it continues:
>
> ccree e6601000.crypto: ARM CryptoCell 630P Driver: HW version
> 0x00000000, Driver version 4.0
> ccree e6601000.crypto: Cache params previous: 0x00000777
> ccree e6601000.crypto: Cache params current: 0x00000000
> (expect: 0x00000000)
> alg: No test for cts1(cbc(aes)) (cts1-cbc-aes-ccree)
> alg: No test for authenc(xcbc(aes),cbc(aes))
> (authenc-xcbc-aes-cbc-aes-ccree)
> alg: No test for authenc(xcbc(aes),rfc3686(ctr(aes)))
> (authenc-xcbc-aes-rfc3686-ctr-aes-ccree)
> ccree e6601000.crypto: ARM ccree device initialized
>
> Is HW version 0x00000000 expected?
It's related to the problem with reading the wrong register I've
mentioned before.
>
>> If you take all the 3 patches, it will work.
>
> is there an easy way to test proper operation?
The lines of the form " alg: No test for cts1(cbc(aes))
(cts1-cbc-aes-ccree)" indicates
you have the Crypto API testmgr enable (or rather not disabled would
be more accurate) so every
cryptographic algorithm except those specified in these messages was
tested with test
vectors from crypto/testmgr.c upon registration. If you don't seen
failure warnings, it works.
You can also check /proc/crypto for all the algorithm with ccree
listed as their driver and check
that their test passed.
> I enabled CONFIG_CRYPT_TEST, and did insmod tcrypt.ko, but I mostly see
> "Failed to load transform" messages.
>
tcrypt.ko is a rather crude developer tool. It has hard coded lists of
test for different algorithms that does
not take into account if some crypto algs are enagled in the build or
not. It's more of a stress test.
Gilad
--
Gilad Ben-Yossef
Chief Coffee Drinker
"If you take a class in large-scale robotics, can you end up in a
situation where the homework eats your dog?"
-- Jean-Baptiste Queru
^ permalink raw reply [flat|nested] 86+ messages in thread
* Re: [PATCH 3/3] arm64: dts: renesas: r8a7795: add ccree binding
@ 2018-05-17 13:41 ` Gilad Ben-Yossef
0 siblings, 0 replies; 86+ messages in thread
From: Gilad Ben-Yossef @ 2018-05-17 13:41 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Simon Horman, Magnus Damm, Rob Herring, Mark Rutland,
Catalin Marinas, Will Deacon, Geert Uytterhoeven,
Michael Turquette, Stephen Boyd, Herbert Xu, David S. Miller,
Ofir Drang, Linux-Renesas,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Linux ARM, Linux Kernel Mailing List, linux-clk
On Thu, May 17, 2018 at 4:35 PM, Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
> Hi Gilad,
>
> On Thu, May 17, 2018 at 3:09 PM, Gilad Ben-Yossef <gilad@benyossef.com> wrote:
>> On Thu, May 17, 2018 at 1:16 PM, Geert Uytterhoeven
>> <geert@linux-m68k.org> wrote:
>>> However, even with your clock patch, the signature checking fails for me,
>>> on both R-Car H3 ES1.0 and ES2.0.
>>> Does this need changes to the ARM Trusted Firmware, to allow Linux to
>>> access the public SCEG module?
>>
>> Well, this is actually something different. If you look you will
>> notice that my patch was part of a 3 part patch series,
>> the first of which disabled this test.
>
> Sorry, I had completely forgotten about the first patch from the series.
> With that applied, it continues:
>
> ccree e6601000.crypto: ARM CryptoCell 630P Driver: HW version
> 0x00000000, Driver version 4.0
> ccree e6601000.crypto: Cache params previous: 0x00000777
> ccree e6601000.crypto: Cache params current: 0x00000000
> (expect: 0x00000000)
> alg: No test for cts1(cbc(aes)) (cts1-cbc-aes-ccree)
> alg: No test for authenc(xcbc(aes),cbc(aes))
> (authenc-xcbc-aes-cbc-aes-ccree)
> alg: No test for authenc(xcbc(aes),rfc3686(ctr(aes)))
> (authenc-xcbc-aes-rfc3686-ctr-aes-ccree)
> ccree e6601000.crypto: ARM ccree device initialized
>
> Is HW version 0x00000000 expected?
It's related to the problem with reading the wrong register I've
mentioned before.
>
>> If you take all the 3 patches, it will work.
>
> is there an easy way to test proper operation?
The lines of the form " alg: No test for cts1(cbc(aes))
(cts1-cbc-aes-ccree)" indicates
you have the Crypto API testmgr enable (or rather not disabled would
be more accurate) so every
cryptographic algorithm except those specified in these messages was
tested with test
vectors from crypto/testmgr.c upon registration. If you don't seen
failure warnings, it works.
You can also check /proc/crypto for all the algorithm with ccree
listed as their driver and check
that their test passed.
> I enabled CONFIG_CRYPT_TEST, and did insmod tcrypt.ko, but I mostly see
> "Failed to load transform" messages.
>
tcrypt.ko is a rather crude developer tool. It has hard coded lists of
test for different algorithms that does
not take into account if some crypto algs are enagled in the build or
not. It's more of a stress test.
Gilad
--
Gilad Ben-Yossef
Chief Coffee Drinker
"If you take a class in large-scale robotics, can you end up in a
situation where the homework eats your dog?"
-- Jean-Baptiste Queru
^ permalink raw reply [flat|nested] 86+ messages in thread
* Re: [PATCH 3/3] arm64: dts: renesas: r8a7795: add ccree binding
@ 2018-05-17 13:41 ` Gilad Ben-Yossef
0 siblings, 0 replies; 86+ messages in thread
From: Gilad Ben-Yossef @ 2018-05-17 13:41 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Simon Horman, Magnus Damm, Rob Herring, Mark Rutland,
Catalin Marinas, Will Deacon, Geert Uytterhoeven,
Michael Turquette, Stephen Boyd, Herbert Xu, David S. Miller,
Ofir Drang, Linux-Renesas,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Linux ARM, Linux Kernel Mailing List, linux-clk,
Linux Crypto Mailing List
On Thu, May 17, 2018 at 4:35 PM, Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
> Hi Gilad,
>
> On Thu, May 17, 2018 at 3:09 PM, Gilad Ben-Yossef <gilad@benyossef.com> wrote:
>> On Thu, May 17, 2018 at 1:16 PM, Geert Uytterhoeven
>> <geert@linux-m68k.org> wrote:
>>> However, even with your clock patch, the signature checking fails for me,
>>> on both R-Car H3 ES1.0 and ES2.0.
>>> Does this need changes to the ARM Trusted Firmware, to allow Linux to
>>> access the public SCEG module?
>>
>> Well, this is actually something different. If you look you will
>> notice that my patch was part of a 3 part patch series,
>> the first of which disabled this test.
>
> Sorry, I had completely forgotten about the first patch from the series.
> With that applied, it continues:
>
> ccree e6601000.crypto: ARM CryptoCell 630P Driver: HW version
> 0x00000000, Driver version 4.0
> ccree e6601000.crypto: Cache params previous: 0x00000777
> ccree e6601000.crypto: Cache params current: 0x00000000
> (expect: 0x00000000)
> alg: No test for cts1(cbc(aes)) (cts1-cbc-aes-ccree)
> alg: No test for authenc(xcbc(aes),cbc(aes))
> (authenc-xcbc-aes-cbc-aes-ccree)
> alg: No test for authenc(xcbc(aes),rfc3686(ctr(aes)))
> (authenc-xcbc-aes-rfc3686-ctr-aes-ccree)
> ccree e6601000.crypto: ARM ccree device initialized
>
> Is HW version 0x00000000 expected?
It's related to the problem with reading the wrong register I've
mentioned before.
>
>> If you take all the 3 patches, it will work.
>
> is there an easy way to test proper operation?
The lines of the form " alg: No test for cts1(cbc(aes))
(cts1-cbc-aes-ccree)" indicates
you have the Crypto API testmgr enable (or rather not disabled would
be more accurate) so every
cryptographic algorithm except those specified in these messages was
tested with test
vectors from crypto/testmgr.c upon registration. If you don't seen
failure warnings, it works.
You can also check /proc/crypto for all the algorithm with ccree
listed as their driver and check
that their test passed.
> I enabled CONFIG_CRYPT_TEST, and did insmod tcrypt.ko, but I mostly see
> "Failed to load transform" messages.
>
tcrypt.ko is a rather crude developer tool. It has hard coded lists of
test for different algorithms that does
not take into account if some crypto algs are enagled in the build or
not. It's more of a stress test.
Gilad
--
Gilad Ben-Yossef
Chief Coffee Drinker
"If you take a class in large-scale robotics, can you end up in a
situation where the homework eats your dog?"
-- Jean-Baptiste Queru
^ permalink raw reply [flat|nested] 86+ messages in thread
* Re: [PATCH 3/3] arm64: dts: renesas: r8a7795: add ccree binding
2018-05-17 13:41 ` Gilad Ben-Yossef
(?)
(?)
@ 2018-05-17 13:49 ` Geert Uytterhoeven
-1 siblings, 0 replies; 86+ messages in thread
From: Geert Uytterhoeven @ 2018-05-17 13:49 UTC (permalink / raw)
To: Gilad Ben-Yossef
Cc: Simon Horman, Magnus Damm, Rob Herring, Mark Rutland,
Catalin Marinas, Will Deacon, Geert Uytterhoeven,
Michael Turquette, Stephen Boyd, Herbert Xu, David S. Miller,
Ofir Drang, Linux-Renesas,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Linux ARM, Linux Kernel Mailing List, linux-clk, L
Hi Gilad,
On Thu, May 17, 2018 at 3:41 PM, Gilad Ben-Yossef <gilad@benyossef.com> wrote:
> On Thu, May 17, 2018 at 4:35 PM, Geert Uytterhoeven
> <geert@linux-m68k.org> wrote:
>> On Thu, May 17, 2018 at 3:09 PM, Gilad Ben-Yossef <gilad@benyossef.com> wrote:
>>> On Thu, May 17, 2018 at 1:16 PM, Geert Uytterhoeven
>>> <geert@linux-m68k.org> wrote:
>>>> However, even with your clock patch, the signature checking fails for me,
>>>> on both R-Car H3 ES1.0 and ES2.0.
>>>> Does this need changes to the ARM Trusted Firmware, to allow Linux to
>>>> access the public SCEG module?
>>>
>>> Well, this is actually something different. If you look you will
>>> notice that my patch was part of a 3 part patch series,
>>> the first of which disabled this test.
>>
>> Sorry, I had completely forgotten about the first patch from the series.
>> With that applied, it continues:
>>
>> ccree e6601000.crypto: ARM CryptoCell 630P Driver: HW version
>> 0x00000000, Driver version 4.0
>> ccree e6601000.crypto: Cache params previous: 0x00000777
>> ccree e6601000.crypto: Cache params current: 0x00000000
>> (expect: 0x00000000)
>> alg: No test for cts1(cbc(aes)) (cts1-cbc-aes-ccree)
>> alg: No test for authenc(xcbc(aes),cbc(aes))
>> (authenc-xcbc-aes-cbc-aes-ccree)
>> alg: No test for authenc(xcbc(aes),rfc3686(ctr(aes)))
>> (authenc-xcbc-aes-rfc3686-ctr-aes-ccree)
>> ccree e6601000.crypto: ARM ccree device initialized
>>
>> Is HW version 0x00000000 expected?
>
> It's related to the problem with reading the wrong register I've
> mentioned before.
OK.
>>> If you take all the 3 patches, it will work.
>>
>> is there an easy way to test proper operation?
>
> The lines of the form " alg: No test for cts1(cbc(aes))
> (cts1-cbc-aes-ccree)" indicates
> you have the Crypto API testmgr enable (or rather not disabled would
> be more accurate) so every
> cryptographic algorithm except those specified in these messages was
> tested with test
> vectors from crypto/testmgr.c upon registration. If you don't seen
> failure warnings, it works.
OK.
> You can also check /proc/crypto for all the algorithm with ccree
> listed as their driver and check
> that their test passed.
OK, in that case everything works as expected, on both R-Car H3 ES1.0 and
ES2.0.
Thanks!
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 86+ messages in thread* [PATCH 3/3] arm64: dts: renesas: r8a7795: add ccree binding
@ 2018-05-17 13:49 ` Geert Uytterhoeven
0 siblings, 0 replies; 86+ messages in thread
From: Geert Uytterhoeven @ 2018-05-17 13:49 UTC (permalink / raw)
To: linux-arm-kernel
Hi Gilad,
On Thu, May 17, 2018 at 3:41 PM, Gilad Ben-Yossef <gilad@benyossef.com> wrote:
> On Thu, May 17, 2018 at 4:35 PM, Geert Uytterhoeven
> <geert@linux-m68k.org> wrote:
>> On Thu, May 17, 2018 at 3:09 PM, Gilad Ben-Yossef <gilad@benyossef.com> wrote:
>>> On Thu, May 17, 2018 at 1:16 PM, Geert Uytterhoeven
>>> <geert@linux-m68k.org> wrote:
>>>> However, even with your clock patch, the signature checking fails for me,
>>>> on both R-Car H3 ES1.0 and ES2.0.
>>>> Does this need changes to the ARM Trusted Firmware, to allow Linux to
>>>> access the public SCEG module?
>>>
>>> Well, this is actually something different. If you look you will
>>> notice that my patch was part of a 3 part patch series,
>>> the first of which disabled this test.
>>
>> Sorry, I had completely forgotten about the first patch from the series.
>> With that applied, it continues:
>>
>> ccree e6601000.crypto: ARM CryptoCell 630P Driver: HW version
>> 0x00000000, Driver version 4.0
>> ccree e6601000.crypto: Cache params previous: 0x00000777
>> ccree e6601000.crypto: Cache params current: 0x00000000
>> (expect: 0x00000000)
>> alg: No test for cts1(cbc(aes)) (cts1-cbc-aes-ccree)
>> alg: No test for authenc(xcbc(aes),cbc(aes))
>> (authenc-xcbc-aes-cbc-aes-ccree)
>> alg: No test for authenc(xcbc(aes),rfc3686(ctr(aes)))
>> (authenc-xcbc-aes-rfc3686-ctr-aes-ccree)
>> ccree e6601000.crypto: ARM ccree device initialized
>>
>> Is HW version 0x00000000 expected?
>
> It's related to the problem with reading the wrong register I've
> mentioned before.
OK.
>>> If you take all the 3 patches, it will work.
>>
>> is there an easy way to test proper operation?
>
> The lines of the form " alg: No test for cts1(cbc(aes))
> (cts1-cbc-aes-ccree)" indicates
> you have the Crypto API testmgr enable (or rather not disabled would
> be more accurate) so every
> cryptographic algorithm except those specified in these messages was
> tested with test
> vectors from crypto/testmgr.c upon registration. If you don't seen
> failure warnings, it works.
OK.
> You can also check /proc/crypto for all the algorithm with ccree
> listed as their driver and check
> that their test passed.
OK, in that case everything works as expected, on both R-Car H3 ES1.0 and
ES2.0.
Thanks!
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 86+ messages in thread* Re: [PATCH 3/3] arm64: dts: renesas: r8a7795: add ccree binding
@ 2018-05-17 13:49 ` Geert Uytterhoeven
0 siblings, 0 replies; 86+ messages in thread
From: Geert Uytterhoeven @ 2018-05-17 13:49 UTC (permalink / raw)
To: Gilad Ben-Yossef
Cc: Simon Horman, Magnus Damm, Rob Herring, Mark Rutland,
Catalin Marinas, Will Deacon, Geert Uytterhoeven,
Michael Turquette, Stephen Boyd, Herbert Xu, David S. Miller,
Ofir Drang, Linux-Renesas,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Linux ARM, Linux Kernel Mailing List, linux-clk
Hi Gilad,
On Thu, May 17, 2018 at 3:41 PM, Gilad Ben-Yossef <gilad@benyossef.com> wrote:
> On Thu, May 17, 2018 at 4:35 PM, Geert Uytterhoeven
> <geert@linux-m68k.org> wrote:
>> On Thu, May 17, 2018 at 3:09 PM, Gilad Ben-Yossef <gilad@benyossef.com> wrote:
>>> On Thu, May 17, 2018 at 1:16 PM, Geert Uytterhoeven
>>> <geert@linux-m68k.org> wrote:
>>>> However, even with your clock patch, the signature checking fails for me,
>>>> on both R-Car H3 ES1.0 and ES2.0.
>>>> Does this need changes to the ARM Trusted Firmware, to allow Linux to
>>>> access the public SCEG module?
>>>
>>> Well, this is actually something different. If you look you will
>>> notice that my patch was part of a 3 part patch series,
>>> the first of which disabled this test.
>>
>> Sorry, I had completely forgotten about the first patch from the series.
>> With that applied, it continues:
>>
>> ccree e6601000.crypto: ARM CryptoCell 630P Driver: HW version
>> 0x00000000, Driver version 4.0
>> ccree e6601000.crypto: Cache params previous: 0x00000777
>> ccree e6601000.crypto: Cache params current: 0x00000000
>> (expect: 0x00000000)
>> alg: No test for cts1(cbc(aes)) (cts1-cbc-aes-ccree)
>> alg: No test for authenc(xcbc(aes),cbc(aes))
>> (authenc-xcbc-aes-cbc-aes-ccree)
>> alg: No test for authenc(xcbc(aes),rfc3686(ctr(aes)))
>> (authenc-xcbc-aes-rfc3686-ctr-aes-ccree)
>> ccree e6601000.crypto: ARM ccree device initialized
>>
>> Is HW version 0x00000000 expected?
>
> It's related to the problem with reading the wrong register I've
> mentioned before.
OK.
>>> If you take all the 3 patches, it will work.
>>
>> is there an easy way to test proper operation?
>
> The lines of the form " alg: No test for cts1(cbc(aes))
> (cts1-cbc-aes-ccree)" indicates
> you have the Crypto API testmgr enable (or rather not disabled would
> be more accurate) so every
> cryptographic algorithm except those specified in these messages was
> tested with test
> vectors from crypto/testmgr.c upon registration. If you don't seen
> failure warnings, it works.
OK.
> You can also check /proc/crypto for all the algorithm with ccree
> listed as their driver and check
> that their test passed.
OK, in that case everything works as expected, on both R-Car H3 ES1.0 and
ES2.0.
Thanks!
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 86+ messages in thread* Re: [PATCH 3/3] arm64: dts: renesas: r8a7795: add ccree binding
@ 2018-05-17 13:49 ` Geert Uytterhoeven
0 siblings, 0 replies; 86+ messages in thread
From: Geert Uytterhoeven @ 2018-05-17 13:49 UTC (permalink / raw)
To: Gilad Ben-Yossef
Cc: Simon Horman, Magnus Damm, Rob Herring, Mark Rutland,
Catalin Marinas, Will Deacon, Geert Uytterhoeven,
Michael Turquette, Stephen Boyd, Herbert Xu, David S. Miller,
Ofir Drang, Linux-Renesas,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Linux ARM, Linux Kernel Mailing List, linux-clk,
Linux Crypto Mailing List
Hi Gilad,
On Thu, May 17, 2018 at 3:41 PM, Gilad Ben-Yossef <gilad@benyossef.com> wrote:
> On Thu, May 17, 2018 at 4:35 PM, Geert Uytterhoeven
> <geert@linux-m68k.org> wrote:
>> On Thu, May 17, 2018 at 3:09 PM, Gilad Ben-Yossef <gilad@benyossef.com> wrote:
>>> On Thu, May 17, 2018 at 1:16 PM, Geert Uytterhoeven
>>> <geert@linux-m68k.org> wrote:
>>>> However, even with your clock patch, the signature checking fails for me,
>>>> on both R-Car H3 ES1.0 and ES2.0.
>>>> Does this need changes to the ARM Trusted Firmware, to allow Linux to
>>>> access the public SCEG module?
>>>
>>> Well, this is actually something different. If you look you will
>>> notice that my patch was part of a 3 part patch series,
>>> the first of which disabled this test.
>>
>> Sorry, I had completely forgotten about the first patch from the series.
>> With that applied, it continues:
>>
>> ccree e6601000.crypto: ARM CryptoCell 630P Driver: HW version
>> 0x00000000, Driver version 4.0
>> ccree e6601000.crypto: Cache params previous: 0x00000777
>> ccree e6601000.crypto: Cache params current: 0x00000000
>> (expect: 0x00000000)
>> alg: No test for cts1(cbc(aes)) (cts1-cbc-aes-ccree)
>> alg: No test for authenc(xcbc(aes),cbc(aes))
>> (authenc-xcbc-aes-cbc-aes-ccree)
>> alg: No test for authenc(xcbc(aes),rfc3686(ctr(aes)))
>> (authenc-xcbc-aes-rfc3686-ctr-aes-ccree)
>> ccree e6601000.crypto: ARM ccree device initialized
>>
>> Is HW version 0x00000000 expected?
>
> It's related to the problem with reading the wrong register I've
> mentioned before.
OK.
>>> If you take all the 3 patches, it will work.
>>
>> is there an easy way to test proper operation?
>
> The lines of the form " alg: No test for cts1(cbc(aes))
> (cts1-cbc-aes-ccree)" indicates
> you have the Crypto API testmgr enable (or rather not disabled would
> be more accurate) so every
> cryptographic algorithm except those specified in these messages was
> tested with test
> vectors from crypto/testmgr.c upon registration. If you don't seen
> failure warnings, it works.
OK.
> You can also check /proc/crypto for all the algorithm with ccree
> listed as their driver and check
> that their test passed.
OK, in that case everything works as expected, on both R-Car H3 ES1.0 and
ES2.0.
Thanks!
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 86+ messages in thread
* Re: [PATCH 3/3] arm64: dts: renesas: r8a7795: add ccree binding
2018-05-17 10:16 ` Geert Uytterhoeven
(?)
(?)
@ 2018-05-17 13:17 ` Geert Uytterhoeven
-1 siblings, 0 replies; 86+ messages in thread
From: Geert Uytterhoeven @ 2018-05-17 13:17 UTC (permalink / raw)
To: Gilad Ben-Yossef
Cc: Simon Horman, Magnus Damm, Rob Herring, Mark Rutland,
Catalin Marinas, Will Deacon, Geert Uytterhoeven,
Michael Turquette, Stephen Boyd, Herbert Xu, David S. Miller,
Ofir Drang, Linux-Renesas,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Linux ARM, Linux Kernel Mailing List, linux-clk, L
On Thu, May 17, 2018 at 12:16 PM, Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
> On Thu, May 17, 2018 at 10:01 AM, Gilad Ben-Yossef <gilad@benyossef.com> wrote:
>> On Wed, May 16, 2018 at 10:43 AM, Simon Horman <horms@verge.net.au> wrote:
>>> On Tue, May 15, 2018 at 04:50:44PM +0200, Geert Uytterhoeven wrote:
>>>> On Tue, May 15, 2018 at 2:29 PM, Gilad Ben-Yossef <gilad@benyossef.com> wrote:
>>>> > Add bindings for CryptoCell instance in the SoC.
>>>> >
>>>> > Signed-off-by: Gilad Ben-Yossef <gilad@benyossef.com>
>>>>
>>>> Thanks for your patch!
>>>>
>>>> > --- a/arch/arm64/boot/dts/renesas/r8a7795.dtsi
>>>> > +++ b/arch/arm64/boot/dts/renesas/r8a7795.dtsi
>>>> > @@ -528,6 +528,14 @@
>>>> > status = "disabled";
>>>> > };
>>>> >
>>>> > + arm_cc630p: crypto@e6601000 {
>>>> > + compatible = "arm,cryptocell-630p-ree";
>>>> > + interrupts = <GIC_SPI 71 IRQ_TYPE_LEVEL_HIGH>;
>>>> > + #interrupt-cells = <2>;
>>>>
>>>> I believe the #interrupt-cells property is not needed.
>>>>
>>>> > + reg = <0x0 0xe6601000 0 0x1000>;
>>>> > + clocks = <&cpg CPG_MOD 229>;
>
> Missing "power-domains = <&sysc R8A7795_PD_ALWAYS_ON>;", as
> the Secure Engine is part of the CPG/MSSR clock domain (see below [*]).
And missing "resets = <&cpg 229>;", as the module is tied to the CPG/MSSR
reset controller.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 86+ messages in thread* [PATCH 3/3] arm64: dts: renesas: r8a7795: add ccree binding
@ 2018-05-17 13:17 ` Geert Uytterhoeven
0 siblings, 0 replies; 86+ messages in thread
From: Geert Uytterhoeven @ 2018-05-17 13:17 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, May 17, 2018 at 12:16 PM, Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
> On Thu, May 17, 2018 at 10:01 AM, Gilad Ben-Yossef <gilad@benyossef.com> wrote:
>> On Wed, May 16, 2018 at 10:43 AM, Simon Horman <horms@verge.net.au> wrote:
>>> On Tue, May 15, 2018 at 04:50:44PM +0200, Geert Uytterhoeven wrote:
>>>> On Tue, May 15, 2018 at 2:29 PM, Gilad Ben-Yossef <gilad@benyossef.com> wrote:
>>>> > Add bindings for CryptoCell instance in the SoC.
>>>> >
>>>> > Signed-off-by: Gilad Ben-Yossef <gilad@benyossef.com>
>>>>
>>>> Thanks for your patch!
>>>>
>>>> > --- a/arch/arm64/boot/dts/renesas/r8a7795.dtsi
>>>> > +++ b/arch/arm64/boot/dts/renesas/r8a7795.dtsi
>>>> > @@ -528,6 +528,14 @@
>>>> > status = "disabled";
>>>> > };
>>>> >
>>>> > + arm_cc630p: crypto at e6601000 {
>>>> > + compatible = "arm,cryptocell-630p-ree";
>>>> > + interrupts = <GIC_SPI 71 IRQ_TYPE_LEVEL_HIGH>;
>>>> > + #interrupt-cells = <2>;
>>>>
>>>> I believe the #interrupt-cells property is not needed.
>>>>
>>>> > + reg = <0x0 0xe6601000 0 0x1000>;
>>>> > + clocks = <&cpg CPG_MOD 229>;
>
> Missing "power-domains = <&sysc R8A7795_PD_ALWAYS_ON>;", as
> the Secure Engine is part of the CPG/MSSR clock domain (see below [*]).
And missing "resets = <&cpg 229>;", as the module is tied to the CPG/MSSR
reset controller.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 86+ messages in thread* Re: [PATCH 3/3] arm64: dts: renesas: r8a7795: add ccree binding
@ 2018-05-17 13:17 ` Geert Uytterhoeven
0 siblings, 0 replies; 86+ messages in thread
From: Geert Uytterhoeven @ 2018-05-17 13:17 UTC (permalink / raw)
To: Gilad Ben-Yossef
Cc: Simon Horman, Magnus Damm, Rob Herring, Mark Rutland,
Catalin Marinas, Will Deacon, Geert Uytterhoeven,
Michael Turquette, Stephen Boyd, Herbert Xu, David S. Miller,
Ofir Drang, Linux-Renesas,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Linux ARM, Linux Kernel Mailing List, linux-clk
On Thu, May 17, 2018 at 12:16 PM, Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
> On Thu, May 17, 2018 at 10:01 AM, Gilad Ben-Yossef <gilad@benyossef.com> wrote:
>> On Wed, May 16, 2018 at 10:43 AM, Simon Horman <horms@verge.net.au> wrote:
>>> On Tue, May 15, 2018 at 04:50:44PM +0200, Geert Uytterhoeven wrote:
>>>> On Tue, May 15, 2018 at 2:29 PM, Gilad Ben-Yossef <gilad@benyossef.com> wrote:
>>>> > Add bindings for CryptoCell instance in the SoC.
>>>> >
>>>> > Signed-off-by: Gilad Ben-Yossef <gilad@benyossef.com>
>>>>
>>>> Thanks for your patch!
>>>>
>>>> > --- a/arch/arm64/boot/dts/renesas/r8a7795.dtsi
>>>> > +++ b/arch/arm64/boot/dts/renesas/r8a7795.dtsi
>>>> > @@ -528,6 +528,14 @@
>>>> > status = "disabled";
>>>> > };
>>>> >
>>>> > + arm_cc630p: crypto@e6601000 {
>>>> > + compatible = "arm,cryptocell-630p-ree";
>>>> > + interrupts = <GIC_SPI 71 IRQ_TYPE_LEVEL_HIGH>;
>>>> > + #interrupt-cells = <2>;
>>>>
>>>> I believe the #interrupt-cells property is not needed.
>>>>
>>>> > + reg = <0x0 0xe6601000 0 0x1000>;
>>>> > + clocks = <&cpg CPG_MOD 229>;
>
> Missing "power-domains = <&sysc R8A7795_PD_ALWAYS_ON>;", as
> the Secure Engine is part of the CPG/MSSR clock domain (see below [*]).
And missing "resets = <&cpg 229>;", as the module is tied to the CPG/MSSR
reset controller.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 86+ messages in thread* Re: [PATCH 3/3] arm64: dts: renesas: r8a7795: add ccree binding
@ 2018-05-17 13:17 ` Geert Uytterhoeven
0 siblings, 0 replies; 86+ messages in thread
From: Geert Uytterhoeven @ 2018-05-17 13:17 UTC (permalink / raw)
To: Gilad Ben-Yossef
Cc: Simon Horman, Magnus Damm, Rob Herring, Mark Rutland,
Catalin Marinas, Will Deacon, Geert Uytterhoeven,
Michael Turquette, Stephen Boyd, Herbert Xu, David S. Miller,
Ofir Drang, Linux-Renesas,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Linux ARM, Linux Kernel Mailing List, linux-clk,
Linux Crypto Mailing List
On Thu, May 17, 2018 at 12:16 PM, Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
> On Thu, May 17, 2018 at 10:01 AM, Gilad Ben-Yossef <gilad@benyossef.com> wrote:
>> On Wed, May 16, 2018 at 10:43 AM, Simon Horman <horms@verge.net.au> wrote:
>>> On Tue, May 15, 2018 at 04:50:44PM +0200, Geert Uytterhoeven wrote:
>>>> On Tue, May 15, 2018 at 2:29 PM, Gilad Ben-Yossef <gilad@benyossef.com> wrote:
>>>> > Add bindings for CryptoCell instance in the SoC.
>>>> >
>>>> > Signed-off-by: Gilad Ben-Yossef <gilad@benyossef.com>
>>>>
>>>> Thanks for your patch!
>>>>
>>>> > --- a/arch/arm64/boot/dts/renesas/r8a7795.dtsi
>>>> > +++ b/arch/arm64/boot/dts/renesas/r8a7795.dtsi
>>>> > @@ -528,6 +528,14 @@
>>>> > status = "disabled";
>>>> > };
>>>> >
>>>> > + arm_cc630p: crypto@e6601000 {
>>>> > + compatible = "arm,cryptocell-630p-ree";
>>>> > + interrupts = <GIC_SPI 71 IRQ_TYPE_LEVEL_HIGH>;
>>>> > + #interrupt-cells = <2>;
>>>>
>>>> I believe the #interrupt-cells property is not needed.
>>>>
>>>> > + reg = <0x0 0xe6601000 0 0x1000>;
>>>> > + clocks = <&cpg CPG_MOD 229>;
>
> Missing "power-domains = <&sysc R8A7795_PD_ALWAYS_ON>;", as
> the Secure Engine is part of the CPG/MSSR clock domain (see below [*]).
And missing "resets = <&cpg 229>;", as the module is tied to the CPG/MSSR
reset controller.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 86+ messages in thread
* Re: [PATCH 3/3] arm64: dts: renesas: r8a7795: add ccree binding
2018-05-17 10:16 ` Geert Uytterhoeven
(?)
(?)
@ 2018-05-21 13:43 ` Gilad Ben-Yossef
-1 siblings, 0 replies; 86+ messages in thread
From: Gilad Ben-Yossef @ 2018-05-21 13:43 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Simon Horman, Magnus Damm, Rob Herring, Mark Rutland,
Catalin Marinas, Will Deacon, Geert Uytterhoeven,
Michael Turquette, Stephen Boyd, Herbert Xu, David S. Miller,
Ofir Drang, Linux-Renesas,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Linux ARM, Linux Kernel Mailing List, linux-clk, L
On Thu, May 17, 2018 at 1:16 PM, Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
>
> Indeed. From a quick glance, it looks like drivers/crypto/ccree/cc_driver.c
> does not distinguish between the absence of the clock property, and an
> actual error in getting the clock, and never considers any error a failure
> (incl. -PROBE_DEFER).
>
> As of_clk_get() returns -ENOENT for both a missing clock property and a
> missing clock, you should use (devm_)clk_get() instead, and distinguish
> between NULL (no clock property) and IS_ERR() (actual failure -> abort).
>
I was trying to do as you suggested but I didn't quite get what is the
dev_id (2nd) parameter to devm_clk_get parameter is supposed to be.
I see what of_clk_get() is doing, so can replicate that but it seems
an over kill.
Any ideas?
Thanks,
Gilad
--
Gilad Ben-Yossef
Chief Coffee Drinker
"If you take a class in large-scale robotics, can you end up in a
situation where the homework eats your dog?"
-- Jean-Baptiste Queru
^ permalink raw reply [flat|nested] 86+ messages in thread
* [PATCH 3/3] arm64: dts: renesas: r8a7795: add ccree binding
@ 2018-05-21 13:43 ` Gilad Ben-Yossef
0 siblings, 0 replies; 86+ messages in thread
From: Gilad Ben-Yossef @ 2018-05-21 13:43 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, May 17, 2018 at 1:16 PM, Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
>
> Indeed. From a quick glance, it looks like drivers/crypto/ccree/cc_driver.c
> does not distinguish between the absence of the clock property, and an
> actual error in getting the clock, and never considers any error a failure
> (incl. -PROBE_DEFER).
>
> As of_clk_get() returns -ENOENT for both a missing clock property and a
> missing clock, you should use (devm_)clk_get() instead, and distinguish
> between NULL (no clock property) and IS_ERR() (actual failure -> abort).
>
I was trying to do as you suggested but I didn't quite get what is the
dev_id (2nd) parameter to devm_clk_get parameter is supposed to be.
I see what of_clk_get() is doing, so can replicate that but it seems
an over kill.
Any ideas?
Thanks,
Gilad
--
Gilad Ben-Yossef
Chief Coffee Drinker
"If you take a class in large-scale robotics, can you end up in a
situation where the homework eats your dog?"
-- Jean-Baptiste Queru
^ permalink raw reply [flat|nested] 86+ messages in thread
* Re: [PATCH 3/3] arm64: dts: renesas: r8a7795: add ccree binding
@ 2018-05-21 13:43 ` Gilad Ben-Yossef
0 siblings, 0 replies; 86+ messages in thread
From: Gilad Ben-Yossef @ 2018-05-21 13:43 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Simon Horman, Magnus Damm, Rob Herring, Mark Rutland,
Catalin Marinas, Will Deacon, Geert Uytterhoeven,
Michael Turquette, Stephen Boyd, Herbert Xu, David S. Miller,
Ofir Drang, Linux-Renesas,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Linux ARM, Linux Kernel Mailing List, linux-clk
On Thu, May 17, 2018 at 1:16 PM, Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
>
> Indeed. From a quick glance, it looks like drivers/crypto/ccree/cc_driver.c
> does not distinguish between the absence of the clock property, and an
> actual error in getting the clock, and never considers any error a failure
> (incl. -PROBE_DEFER).
>
> As of_clk_get() returns -ENOENT for both a missing clock property and a
> missing clock, you should use (devm_)clk_get() instead, and distinguish
> between NULL (no clock property) and IS_ERR() (actual failure -> abort).
>
I was trying to do as you suggested but I didn't quite get what is the
dev_id (2nd) parameter to devm_clk_get parameter is supposed to be.
I see what of_clk_get() is doing, so can replicate that but it seems
an over kill.
Any ideas?
Thanks,
Gilad
--
Gilad Ben-Yossef
Chief Coffee Drinker
"If you take a class in large-scale robotics, can you end up in a
situation where the homework eats your dog?"
-- Jean-Baptiste Queru
^ permalink raw reply [flat|nested] 86+ messages in thread
* Re: [PATCH 3/3] arm64: dts: renesas: r8a7795: add ccree binding
@ 2018-05-21 13:43 ` Gilad Ben-Yossef
0 siblings, 0 replies; 86+ messages in thread
From: Gilad Ben-Yossef @ 2018-05-21 13:43 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Simon Horman, Magnus Damm, Rob Herring, Mark Rutland,
Catalin Marinas, Will Deacon, Geert Uytterhoeven,
Michael Turquette, Stephen Boyd, Herbert Xu, David S. Miller,
Ofir Drang, Linux-Renesas,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Linux ARM, Linux Kernel Mailing List, linux-clk,
Linux Crypto Mailing List
On Thu, May 17, 2018 at 1:16 PM, Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
>
> Indeed. From a quick glance, it looks like drivers/crypto/ccree/cc_driver.c
> does not distinguish between the absence of the clock property, and an
> actual error in getting the clock, and never considers any error a failure
> (incl. -PROBE_DEFER).
>
> As of_clk_get() returns -ENOENT for both a missing clock property and a
> missing clock, you should use (devm_)clk_get() instead, and distinguish
> between NULL (no clock property) and IS_ERR() (actual failure -> abort).
>
I was trying to do as you suggested but I didn't quite get what is the
dev_id (2nd) parameter to devm_clk_get parameter is supposed to be.
I see what of_clk_get() is doing, so can replicate that but it seems
an over kill.
Any ideas?
Thanks,
Gilad
--
Gilad Ben-Yossef
Chief Coffee Drinker
"If you take a class in large-scale robotics, can you end up in a
situation where the homework eats your dog?"
-- Jean-Baptiste Queru
^ permalink raw reply [flat|nested] 86+ messages in thread
* Re: [PATCH 3/3] arm64: dts: renesas: r8a7795: add ccree binding
2018-05-21 13:43 ` Gilad Ben-Yossef
(?)
(?)
@ 2018-05-22 7:48 ` Geert Uytterhoeven
-1 siblings, 0 replies; 86+ messages in thread
From: Geert Uytterhoeven @ 2018-05-22 7:48 UTC (permalink / raw)
To: Gilad Ben-Yossef
Cc: Simon Horman, Magnus Damm, Rob Herring, Mark Rutland,
Catalin Marinas, Will Deacon, Geert Uytterhoeven,
Michael Turquette, Stephen Boyd, Herbert Xu, David S. Miller,
Ofir Drang, Linux-Renesas,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Linux ARM, Linux Kernel Mailing List, linux-clk, L
Hi Gilad,
On Mon, May 21, 2018 at 3:43 PM, Gilad Ben-Yossef <gilad@benyossef.com> wrote:
> On Thu, May 17, 2018 at 1:16 PM, Geert Uytterhoeven
> <geert@linux-m68k.org> wrote:
>> Indeed. From a quick glance, it looks like drivers/crypto/ccree/cc_driver.c
>> does not distinguish between the absence of the clock property, and an
>> actual error in getting the clock, and never considers any error a failure
>> (incl. -PROBE_DEFER).
>>
>> As of_clk_get() returns -ENOENT for both a missing clock property and a
>> missing clock, you should use (devm_)clk_get() instead, and distinguish
>> between NULL (no clock property) and IS_ERR() (actual failure -> abort).
>
> I was trying to do as you suggested but I didn't quite get what is the
> dev_id (2nd) parameter to devm_clk_get parameter is supposed to be.
It's the (optional) name of the clock, helpful in case there is more than one.
In your case, NULL is fine.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 86+ messages in thread* [PATCH 3/3] arm64: dts: renesas: r8a7795: add ccree binding
@ 2018-05-22 7:48 ` Geert Uytterhoeven
0 siblings, 0 replies; 86+ messages in thread
From: Geert Uytterhoeven @ 2018-05-22 7:48 UTC (permalink / raw)
To: linux-arm-kernel
Hi Gilad,
On Mon, May 21, 2018 at 3:43 PM, Gilad Ben-Yossef <gilad@benyossef.com> wrote:
> On Thu, May 17, 2018 at 1:16 PM, Geert Uytterhoeven
> <geert@linux-m68k.org> wrote:
>> Indeed. From a quick glance, it looks like drivers/crypto/ccree/cc_driver.c
>> does not distinguish between the absence of the clock property, and an
>> actual error in getting the clock, and never considers any error a failure
>> (incl. -PROBE_DEFER).
>>
>> As of_clk_get() returns -ENOENT for both a missing clock property and a
>> missing clock, you should use (devm_)clk_get() instead, and distinguish
>> between NULL (no clock property) and IS_ERR() (actual failure -> abort).
>
> I was trying to do as you suggested but I didn't quite get what is the
> dev_id (2nd) parameter to devm_clk_get parameter is supposed to be.
It's the (optional) name of the clock, helpful in case there is more than one.
In your case, NULL is fine.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 86+ messages in thread* Re: [PATCH 3/3] arm64: dts: renesas: r8a7795: add ccree binding
@ 2018-05-22 7:48 ` Geert Uytterhoeven
0 siblings, 0 replies; 86+ messages in thread
From: Geert Uytterhoeven @ 2018-05-22 7:48 UTC (permalink / raw)
To: Gilad Ben-Yossef
Cc: Simon Horman, Magnus Damm, Rob Herring, Mark Rutland,
Catalin Marinas, Will Deacon, Geert Uytterhoeven,
Michael Turquette, Stephen Boyd, Herbert Xu, David S. Miller,
Ofir Drang, Linux-Renesas,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Linux ARM, Linux Kernel Mailing List, linux-clk
Hi Gilad,
On Mon, May 21, 2018 at 3:43 PM, Gilad Ben-Yossef <gilad@benyossef.com> wrote:
> On Thu, May 17, 2018 at 1:16 PM, Geert Uytterhoeven
> <geert@linux-m68k.org> wrote:
>> Indeed. From a quick glance, it looks like drivers/crypto/ccree/cc_driver.c
>> does not distinguish between the absence of the clock property, and an
>> actual error in getting the clock, and never considers any error a failure
>> (incl. -PROBE_DEFER).
>>
>> As of_clk_get() returns -ENOENT for both a missing clock property and a
>> missing clock, you should use (devm_)clk_get() instead, and distinguish
>> between NULL (no clock property) and IS_ERR() (actual failure -> abort).
>
> I was trying to do as you suggested but I didn't quite get what is the
> dev_id (2nd) parameter to devm_clk_get parameter is supposed to be.
It's the (optional) name of the clock, helpful in case there is more than one.
In your case, NULL is fine.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 86+ messages in thread* Re: [PATCH 3/3] arm64: dts: renesas: r8a7795: add ccree binding
@ 2018-05-22 7:48 ` Geert Uytterhoeven
0 siblings, 0 replies; 86+ messages in thread
From: Geert Uytterhoeven @ 2018-05-22 7:48 UTC (permalink / raw)
To: Gilad Ben-Yossef
Cc: Simon Horman, Magnus Damm, Rob Herring, Mark Rutland,
Catalin Marinas, Will Deacon, Geert Uytterhoeven,
Michael Turquette, Stephen Boyd, Herbert Xu, David S. Miller,
Ofir Drang, Linux-Renesas,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Linux ARM, Linux Kernel Mailing List, linux-clk,
Linux Crypto Mailing List
Hi Gilad,
On Mon, May 21, 2018 at 3:43 PM, Gilad Ben-Yossef <gilad@benyossef.com> wrote:
> On Thu, May 17, 2018 at 1:16 PM, Geert Uytterhoeven
> <geert@linux-m68k.org> wrote:
>> Indeed. From a quick glance, it looks like drivers/crypto/ccree/cc_driver.c
>> does not distinguish between the absence of the clock property, and an
>> actual error in getting the clock, and never considers any error a failure
>> (incl. -PROBE_DEFER).
>>
>> As of_clk_get() returns -ENOENT for both a missing clock property and a
>> missing clock, you should use (devm_)clk_get() instead, and distinguish
>> between NULL (no clock property) and IS_ERR() (actual failure -> abort).
>
> I was trying to do as you suggested but I didn't quite get what is the
> dev_id (2nd) parameter to devm_clk_get parameter is supposed to be.
It's the (optional) name of the clock, helpful in case there is more than one.
In your case, NULL is fine.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 86+ messages in thread* Re: [PATCH 3/3] arm64: dts: renesas: r8a7795: add ccree binding
2018-05-22 7:48 ` Geert Uytterhoeven
(?)
(?)
@ 2018-05-24 13:20 ` Gilad Ben-Yossef
-1 siblings, 0 replies; 86+ messages in thread
From: Gilad Ben-Yossef @ 2018-05-24 13:20 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Simon Horman, Magnus Damm, Rob Herring, Mark Rutland,
Catalin Marinas, Will Deacon, Geert Uytterhoeven,
Michael Turquette, Stephen Boyd, Herbert Xu, David S. Miller,
Ofir Drang, Linux-Renesas,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Linux ARM, Linux Kernel Mailing List, linux-clk, L
On Tue, May 22, 2018 at 10:48 AM, Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
> Hi Gilad,
>
> On Mon, May 21, 2018 at 3:43 PM, Gilad Ben-Yossef <gilad@benyossef.com> wrote:
>> On Thu, May 17, 2018 at 1:16 PM, Geert Uytterhoeven
>> <geert@linux-m68k.org> wrote:
>>> Indeed. From a quick glance, it looks like drivers/crypto/ccree/cc_driver.c
>>> does not distinguish between the absence of the clock property, and an
>>> actual error in getting the clock, and never considers any error a failure
>>> (incl. -PROBE_DEFER).
>>>
>>> As of_clk_get() returns -ENOENT for both a missing clock property and a
>>> missing clock, you should use (devm_)clk_get() instead, and distinguish
>>> between NULL (no clock property) and IS_ERR() (actual failure -> abort).
>>
>> I was trying to do as you suggested but I didn't quite get what is the
>> dev_id (2nd) parameter to devm_clk_get parameter is supposed to be.
>
> It's the (optional) name of the clock, helpful in case there is more than one.
> In your case, NULL is fine.
>
I have assumed as much and tried it, it did not work and so I assumed
I am missing something and asked you.
It turns out I was missing the fact I was using the wrong device tree
file... :-(
So thanks, it works now :-)
Having said that, while using devm)clk_get() is a better approach, it
does not seems to distinguish
between no "clocks" and a failure to clock information - it returns
ENOENT in both cases as well.
Thanks,
Gilad
--
Gilad Ben-Yossef
Chief Coffee Drinker
"If you take a class in large-scale robotics, can you end up in a
situation where the homework eats your dog?"
-- Jean-Baptiste Queru
^ permalink raw reply [flat|nested] 86+ messages in thread
* [PATCH 3/3] arm64: dts: renesas: r8a7795: add ccree binding
@ 2018-05-24 13:20 ` Gilad Ben-Yossef
0 siblings, 0 replies; 86+ messages in thread
From: Gilad Ben-Yossef @ 2018-05-24 13:20 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, May 22, 2018 at 10:48 AM, Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
> Hi Gilad,
>
> On Mon, May 21, 2018 at 3:43 PM, Gilad Ben-Yossef <gilad@benyossef.com> wrote:
>> On Thu, May 17, 2018 at 1:16 PM, Geert Uytterhoeven
>> <geert@linux-m68k.org> wrote:
>>> Indeed. From a quick glance, it looks like drivers/crypto/ccree/cc_driver.c
>>> does not distinguish between the absence of the clock property, and an
>>> actual error in getting the clock, and never considers any error a failure
>>> (incl. -PROBE_DEFER).
>>>
>>> As of_clk_get() returns -ENOENT for both a missing clock property and a
>>> missing clock, you should use (devm_)clk_get() instead, and distinguish
>>> between NULL (no clock property) and IS_ERR() (actual failure -> abort).
>>
>> I was trying to do as you suggested but I didn't quite get what is the
>> dev_id (2nd) parameter to devm_clk_get parameter is supposed to be.
>
> It's the (optional) name of the clock, helpful in case there is more than one.
> In your case, NULL is fine.
>
I have assumed as much and tried it, it did not work and so I assumed
I am missing something and asked you.
It turns out I was missing the fact I was using the wrong device tree
file... :-(
So thanks, it works now :-)
Having said that, while using devm)clk_get() is a better approach, it
does not seems to distinguish
between no "clocks" and a failure to clock information - it returns
ENOENT in both cases as well.
Thanks,
Gilad
--
Gilad Ben-Yossef
Chief Coffee Drinker
"If you take a class in large-scale robotics, can you end up in a
situation where the homework eats your dog?"
-- Jean-Baptiste Queru
^ permalink raw reply [flat|nested] 86+ messages in thread
* Re: [PATCH 3/3] arm64: dts: renesas: r8a7795: add ccree binding
@ 2018-05-24 13:20 ` Gilad Ben-Yossef
0 siblings, 0 replies; 86+ messages in thread
From: Gilad Ben-Yossef @ 2018-05-24 13:20 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Simon Horman, Magnus Damm, Rob Herring, Mark Rutland,
Catalin Marinas, Will Deacon, Geert Uytterhoeven,
Michael Turquette, Stephen Boyd, Herbert Xu, David S. Miller,
Ofir Drang, Linux-Renesas,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Linux ARM, Linux Kernel Mailing List, linux-clk
On Tue, May 22, 2018 at 10:48 AM, Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
> Hi Gilad,
>
> On Mon, May 21, 2018 at 3:43 PM, Gilad Ben-Yossef <gilad@benyossef.com> wrote:
>> On Thu, May 17, 2018 at 1:16 PM, Geert Uytterhoeven
>> <geert@linux-m68k.org> wrote:
>>> Indeed. From a quick glance, it looks like drivers/crypto/ccree/cc_driver.c
>>> does not distinguish between the absence of the clock property, and an
>>> actual error in getting the clock, and never considers any error a failure
>>> (incl. -PROBE_DEFER).
>>>
>>> As of_clk_get() returns -ENOENT for both a missing clock property and a
>>> missing clock, you should use (devm_)clk_get() instead, and distinguish
>>> between NULL (no clock property) and IS_ERR() (actual failure -> abort).
>>
>> I was trying to do as you suggested but I didn't quite get what is the
>> dev_id (2nd) parameter to devm_clk_get parameter is supposed to be.
>
> It's the (optional) name of the clock, helpful in case there is more than one.
> In your case, NULL is fine.
>
I have assumed as much and tried it, it did not work and so I assumed
I am missing something and asked you.
It turns out I was missing the fact I was using the wrong device tree
file... :-(
So thanks, it works now :-)
Having said that, while using devm)clk_get() is a better approach, it
does not seems to distinguish
between no "clocks" and a failure to clock information - it returns
ENOENT in both cases as well.
Thanks,
Gilad
--
Gilad Ben-Yossef
Chief Coffee Drinker
"If you take a class in large-scale robotics, can you end up in a
situation where the homework eats your dog?"
-- Jean-Baptiste Queru
^ permalink raw reply [flat|nested] 86+ messages in thread
* Re: [PATCH 3/3] arm64: dts: renesas: r8a7795: add ccree binding
@ 2018-05-24 13:20 ` Gilad Ben-Yossef
0 siblings, 0 replies; 86+ messages in thread
From: Gilad Ben-Yossef @ 2018-05-24 13:20 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Simon Horman, Magnus Damm, Rob Herring, Mark Rutland,
Catalin Marinas, Will Deacon, Geert Uytterhoeven,
Michael Turquette, Stephen Boyd, Herbert Xu, David S. Miller,
Ofir Drang, Linux-Renesas,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Linux ARM, Linux Kernel Mailing List, linux-clk,
Linux Crypto Mailing List
On Tue, May 22, 2018 at 10:48 AM, Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
> Hi Gilad,
>
> On Mon, May 21, 2018 at 3:43 PM, Gilad Ben-Yossef <gilad@benyossef.com> wrote:
>> On Thu, May 17, 2018 at 1:16 PM, Geert Uytterhoeven
>> <geert@linux-m68k.org> wrote:
>>> Indeed. From a quick glance, it looks like drivers/crypto/ccree/cc_driver.c
>>> does not distinguish between the absence of the clock property, and an
>>> actual error in getting the clock, and never considers any error a failure
>>> (incl. -PROBE_DEFER).
>>>
>>> As of_clk_get() returns -ENOENT for both a missing clock property and a
>>> missing clock, you should use (devm_)clk_get() instead, and distinguish
>>> between NULL (no clock property) and IS_ERR() (actual failure -> abort).
>>
>> I was trying to do as you suggested but I didn't quite get what is the
>> dev_id (2nd) parameter to devm_clk_get parameter is supposed to be.
>
> It's the (optional) name of the clock, helpful in case there is more than one.
> In your case, NULL is fine.
>
I have assumed as much and tried it, it did not work and so I assumed
I am missing something and asked you.
It turns out I was missing the fact I was using the wrong device tree
file... :-(
So thanks, it works now :-)
Having said that, while using devm)clk_get() is a better approach, it
does not seems to distinguish
between no "clocks" and a failure to clock information - it returns
ENOENT in both cases as well.
Thanks,
Gilad
--
Gilad Ben-Yossef
Chief Coffee Drinker
"If you take a class in large-scale robotics, can you end up in a
situation where the homework eats your dog?"
-- Jean-Baptiste Queru
^ permalink raw reply [flat|nested] 86+ messages in thread
* Re: [PATCH 3/3] arm64: dts: renesas: r8a7795: add ccree binding
2018-05-24 13:20 ` Gilad Ben-Yossef
(?)
(?)
@ 2018-05-24 13:44 ` Geert Uytterhoeven
-1 siblings, 0 replies; 86+ messages in thread
From: Geert Uytterhoeven @ 2018-05-24 13:44 UTC (permalink / raw)
To: Gilad Ben-Yossef
Cc: Simon Horman, Magnus Damm, Rob Herring, Mark Rutland,
Catalin Marinas, Will Deacon, Geert Uytterhoeven,
Michael Turquette, Stephen Boyd, Herbert Xu, David S. Miller,
Ofir Drang, Linux-Renesas,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Linux ARM, Linux Kernel Mailing List, linux-clk, L
Hi Gilad,
On Thu, May 24, 2018 at 3:20 PM, Gilad Ben-Yossef <gilad@benyossef.com> wrote:
> On Tue, May 22, 2018 at 10:48 AM, Geert Uytterhoeven
> <geert@linux-m68k.org> wrote:
>> On Mon, May 21, 2018 at 3:43 PM, Gilad Ben-Yossef <gilad@benyossef.com> wrote:
>>> On Thu, May 17, 2018 at 1:16 PM, Geert Uytterhoeven
>>> <geert@linux-m68k.org> wrote:
>>>> Indeed. From a quick glance, it looks like drivers/crypto/ccree/cc_driver.c
>>>> does not distinguish between the absence of the clock property, and an
>>>> actual error in getting the clock, and never considers any error a failure
>>>> (incl. -PROBE_DEFER).
>>>>
>>>> As of_clk_get() returns -ENOENT for both a missing clock property and a
>>>> missing clock, you should use (devm_)clk_get() instead, and distinguish
>>>> between NULL (no clock property) and IS_ERR() (actual failure -> abort).
>>>
>>> I was trying to do as you suggested but I didn't quite get what is the
>>> dev_id (2nd) parameter to devm_clk_get parameter is supposed to be.
>>
>> It's the (optional) name of the clock, helpful in case there is more than one.
>> In your case, NULL is fine.
>
> I have assumed as much and tried it, it did not work and so I assumed
> I am missing something and asked you.
> It turns out I was missing the fact I was using the wrong device tree
> file... :-(
>
> So thanks, it works now :-)
Glad to hear that!
> Having said that, while using devm)clk_get() is a better approach, it
> does not seems to distinguish
> between no "clocks" and a failure to clock information - it returns
> ENOENT in both cases as well.
Oh right, I guess I'm too used to not even getting that far due to the PM
Domain code failing to obtain the clock...
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 86+ messages in thread* [PATCH 3/3] arm64: dts: renesas: r8a7795: add ccree binding
@ 2018-05-24 13:44 ` Geert Uytterhoeven
0 siblings, 0 replies; 86+ messages in thread
From: Geert Uytterhoeven @ 2018-05-24 13:44 UTC (permalink / raw)
To: linux-arm-kernel
Hi Gilad,
On Thu, May 24, 2018 at 3:20 PM, Gilad Ben-Yossef <gilad@benyossef.com> wrote:
> On Tue, May 22, 2018 at 10:48 AM, Geert Uytterhoeven
> <geert@linux-m68k.org> wrote:
>> On Mon, May 21, 2018 at 3:43 PM, Gilad Ben-Yossef <gilad@benyossef.com> wrote:
>>> On Thu, May 17, 2018 at 1:16 PM, Geert Uytterhoeven
>>> <geert@linux-m68k.org> wrote:
>>>> Indeed. From a quick glance, it looks like drivers/crypto/ccree/cc_driver.c
>>>> does not distinguish between the absence of the clock property, and an
>>>> actual error in getting the clock, and never considers any error a failure
>>>> (incl. -PROBE_DEFER).
>>>>
>>>> As of_clk_get() returns -ENOENT for both a missing clock property and a
>>>> missing clock, you should use (devm_)clk_get() instead, and distinguish
>>>> between NULL (no clock property) and IS_ERR() (actual failure -> abort).
>>>
>>> I was trying to do as you suggested but I didn't quite get what is the
>>> dev_id (2nd) parameter to devm_clk_get parameter is supposed to be.
>>
>> It's the (optional) name of the clock, helpful in case there is more than one.
>> In your case, NULL is fine.
>
> I have assumed as much and tried it, it did not work and so I assumed
> I am missing something and asked you.
> It turns out I was missing the fact I was using the wrong device tree
> file... :-(
>
> So thanks, it works now :-)
Glad to hear that!
> Having said that, while using devm)clk_get() is a better approach, it
> does not seems to distinguish
> between no "clocks" and a failure to clock information - it returns
> ENOENT in both cases as well.
Oh right, I guess I'm too used to not even getting that far due to the PM
Domain code failing to obtain the clock...
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 86+ messages in thread* Re: [PATCH 3/3] arm64: dts: renesas: r8a7795: add ccree binding
@ 2018-05-24 13:44 ` Geert Uytterhoeven
0 siblings, 0 replies; 86+ messages in thread
From: Geert Uytterhoeven @ 2018-05-24 13:44 UTC (permalink / raw)
To: Gilad Ben-Yossef
Cc: Simon Horman, Magnus Damm, Rob Herring, Mark Rutland,
Catalin Marinas, Will Deacon, Geert Uytterhoeven,
Michael Turquette, Stephen Boyd, Herbert Xu, David S. Miller,
Ofir Drang, Linux-Renesas,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Linux ARM, Linux Kernel Mailing List, linux-clk
Hi Gilad,
On Thu, May 24, 2018 at 3:20 PM, Gilad Ben-Yossef <gilad@benyossef.com> wrote:
> On Tue, May 22, 2018 at 10:48 AM, Geert Uytterhoeven
> <geert@linux-m68k.org> wrote:
>> On Mon, May 21, 2018 at 3:43 PM, Gilad Ben-Yossef <gilad@benyossef.com> wrote:
>>> On Thu, May 17, 2018 at 1:16 PM, Geert Uytterhoeven
>>> <geert@linux-m68k.org> wrote:
>>>> Indeed. From a quick glance, it looks like drivers/crypto/ccree/cc_driver.c
>>>> does not distinguish between the absence of the clock property, and an
>>>> actual error in getting the clock, and never considers any error a failure
>>>> (incl. -PROBE_DEFER).
>>>>
>>>> As of_clk_get() returns -ENOENT for both a missing clock property and a
>>>> missing clock, you should use (devm_)clk_get() instead, and distinguish
>>>> between NULL (no clock property) and IS_ERR() (actual failure -> abort).
>>>
>>> I was trying to do as you suggested but I didn't quite get what is the
>>> dev_id (2nd) parameter to devm_clk_get parameter is supposed to be.
>>
>> It's the (optional) name of the clock, helpful in case there is more than one.
>> In your case, NULL is fine.
>
> I have assumed as much and tried it, it did not work and so I assumed
> I am missing something and asked you.
> It turns out I was missing the fact I was using the wrong device tree
> file... :-(
>
> So thanks, it works now :-)
Glad to hear that!
> Having said that, while using devm)clk_get() is a better approach, it
> does not seems to distinguish
> between no "clocks" and a failure to clock information - it returns
> ENOENT in both cases as well.
Oh right, I guess I'm too used to not even getting that far due to the PM
Domain code failing to obtain the clock...
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 86+ messages in thread* Re: [PATCH 3/3] arm64: dts: renesas: r8a7795: add ccree binding
@ 2018-05-24 13:44 ` Geert Uytterhoeven
0 siblings, 0 replies; 86+ messages in thread
From: Geert Uytterhoeven @ 2018-05-24 13:44 UTC (permalink / raw)
To: Gilad Ben-Yossef
Cc: Simon Horman, Magnus Damm, Rob Herring, Mark Rutland,
Catalin Marinas, Will Deacon, Geert Uytterhoeven,
Michael Turquette, Stephen Boyd, Herbert Xu, David S. Miller,
Ofir Drang, Linux-Renesas,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Linux ARM, Linux Kernel Mailing List, linux-clk,
Linux Crypto Mailing List
Hi Gilad,
On Thu, May 24, 2018 at 3:20 PM, Gilad Ben-Yossef <gilad@benyossef.com> wrote:
> On Tue, May 22, 2018 at 10:48 AM, Geert Uytterhoeven
> <geert@linux-m68k.org> wrote:
>> On Mon, May 21, 2018 at 3:43 PM, Gilad Ben-Yossef <gilad@benyossef.com> wrote:
>>> On Thu, May 17, 2018 at 1:16 PM, Geert Uytterhoeven
>>> <geert@linux-m68k.org> wrote:
>>>> Indeed. From a quick glance, it looks like drivers/crypto/ccree/cc_driver.c
>>>> does not distinguish between the absence of the clock property, and an
>>>> actual error in getting the clock, and never considers any error a failure
>>>> (incl. -PROBE_DEFER).
>>>>
>>>> As of_clk_get() returns -ENOENT for both a missing clock property and a
>>>> missing clock, you should use (devm_)clk_get() instead, and distinguish
>>>> between NULL (no clock property) and IS_ERR() (actual failure -> abort).
>>>
>>> I was trying to do as you suggested but I didn't quite get what is the
>>> dev_id (2nd) parameter to devm_clk_get parameter is supposed to be.
>>
>> It's the (optional) name of the clock, helpful in case there is more than one.
>> In your case, NULL is fine.
>
> I have assumed as much and tried it, it did not work and so I assumed
> I am missing something and asked you.
> It turns out I was missing the fact I was using the wrong device tree
> file... :-(
>
> So thanks, it works now :-)
Glad to hear that!
> Having said that, while using devm)clk_get() is a better approach, it
> does not seems to distinguish
> between no "clocks" and a failure to clock information - it returns
> ENOENT in both cases as well.
Oh right, I guess I'm too used to not even getting that far due to the PM
Domain code failing to obtain the clock...
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 86+ messages in thread