Add slimSSS node to DT and crypto AES support for Exynos5433. Tested on
Exynos5433 board with crypto run-time self tests and with tcrypt with
command insmod tcrypt.ko mode=500 sec=1
Changes since v3:
- moved bindings documentation out from samsung-sss.txt into separate file
samsung-slimsss.txt
Changes since v2:
- address Corentine Labbe note: add "const" to char* in declaration of
struct samsung_aes_variant
Changes since v1:
- address Krzysztof Kozlowski review: add missing comma in struct
definition, add goto for error code path, correct indentation,
in documentation add that Exynos5433 has both slimSSS and SSS IPs.
Kamil Konieczny (3):
arm64: dts: exynos: add SlimSSS for Exynos5433
dt-bindings: crypto: document Exynos5433 SlimSSS
crypto: s5p: add AES support for Exynos5433
.../bindings/crypto/samsung-slimsss.txt | 19 +++++++
arch/arm64/boot/dts/exynos/exynos5433.dtsi | 9 ++++
drivers/crypto/s5p-sss.c | 50 +++++++++++++++++--
3 files changed, 74 insertions(+), 4 deletions(-)
create mode 100644 Documentation/devicetree/bindings/crypto/samsung-slimsss.txt
--
2.20.1
Document DT bindings for crypto Samsung Exynos5433 SlimSSS (Slim Security
SubSystem) IP.
Signed-off-by: Kamil Konieczny <[email protected]>
---
.../bindings/crypto/samsung-slimsss.txt | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
create mode 100644 Documentation/devicetree/bindings/crypto/samsung-slimsss.txt
diff --git a/Documentation/devicetree/bindings/crypto/samsung-slimsss.txt b/Documentation/devicetree/bindings/crypto/samsung-slimsss.txt
new file mode 100644
index 000000000000..7ec9a5a7727a
--- /dev/null
+++ b/Documentation/devicetree/bindings/crypto/samsung-slimsss.txt
@@ -0,0 +1,19 @@
+Samsung SoC SlimSSS (Slim Security SubSystem) module
+
+The SlimSSS module in Exynos5433 SoC supports the following:
+-- Feeder (FeedCtrl)
+-- Advanced Encryption Standard (AES) with ECB,CBC,CTR,XTS and (CBC/XTS)/CTS
+-- SHA-1/SHA-256 and (SHA-1/SHA-256)/HMAC
+
+Required properties:
+
+- compatible : Should contain entry for slimSSS version:
+ - "samsung,exynos5433-slim-sss" for Exynos5433 SoC.
+- reg : Offset and length of the register set for the module
+- interrupts : interrupt specifiers of SlimSSS module interrupts (one feed
+ control interrupt).
+
+- clocks : list of clock phandle and specifier pairs for all clocks listed in
+ clock-names property.
+- clock-names : list of device clock input names; should contain "pclk" and
+ "aclk" for slim-sss in Exynos5433.
--
2.20.1
Add DT node for SlimSSS (aka Slim SecuritySubSystem) in Exynos5433 SoC.
The users can use compatibility "samsung,exynos5433-slim-sss".
Signed-off-by: Kamil Konieczny <[email protected]>
---
arch/arm64/boot/dts/exynos/exynos5433.dtsi | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/arch/arm64/boot/dts/exynos/exynos5433.dtsi b/arch/arm64/boot/dts/exynos/exynos5433.dtsi
index e7cd3b67d818..9e5fcb6c7776 100644
--- a/arch/arm64/boot/dts/exynos/exynos5433.dtsi
+++ b/arch/arm64/boot/dts/exynos/exynos5433.dtsi
@@ -559,6 +559,15 @@
<&cmu_top CLK_DIV_ACLK_IMEM_200>;
};
+ slim_sss: slim-sss@11140000 {
+ compatible = "samsung,exynos5433-slim-sss";
+ reg = <0x11140000 0x1000>;
+ interrupts = <GIC_SPI 312 IRQ_TYPE_LEVEL_HIGH>;
+ clock-names = "aclk", "pclk";
+ clocks = <&cmu_imem CLK_ACLK_SLIMSSS>,
+ <&cmu_imem CLK_PCLK_SLIMSSS>;
+ };
+
pd_gscl: power-domain@105c4000 {
compatible = "samsung,exynos5433-pd";
reg = <0x105c4000 0x20>;
--
2.20.1
Add AES crypto HW acceleration for Exynos5433, with the help of SlimSSS IP.
Reviewed-by: Krzysztof Kozlowski <[email protected]>
Signed-off-by: Kamil Konieczny <[email protected]>
---
drivers/crypto/s5p-sss.c | 50 ++++++++++++++++++++++++++++++++++++----
1 file changed, 46 insertions(+), 4 deletions(-)
diff --git a/drivers/crypto/s5p-sss.c b/drivers/crypto/s5p-sss.c
index 0064be0e3941..3f45cc5cb94a 100644
--- a/drivers/crypto/s5p-sss.c
+++ b/drivers/crypto/s5p-sss.c
@@ -232,6 +232,7 @@
* struct samsung_aes_variant - platform specific SSS driver data
* @aes_offset: AES register offset from SSS module's base.
* @hash_offset: HASH register offset from SSS module's base.
+ * @clk_names: names of clocks needed to run SSS IP
*
* Specifies platform specific configuration of SSS module.
* Note: A structure for driver specific platform data is used for future
@@ -240,6 +241,7 @@
struct samsung_aes_variant {
unsigned int aes_offset;
unsigned int hash_offset;
+ const char *clk_names[];
};
struct s5p_aes_reqctx {
@@ -296,6 +298,7 @@ struct s5p_aes_ctx {
struct s5p_aes_dev {
struct device *dev;
struct clk *clk;
+ struct clk *pclk;
void __iomem *ioaddr;
void __iomem *aes_ioaddr;
int irq_fc;
@@ -384,11 +387,19 @@ struct s5p_hash_ctx {
static const struct samsung_aes_variant s5p_aes_data = {
.aes_offset = 0x4000,
.hash_offset = 0x6000,
+ .clk_names = { "secss", },
};
static const struct samsung_aes_variant exynos_aes_data = {
.aes_offset = 0x200,
.hash_offset = 0x400,
+ .clk_names = { "secss", },
+};
+
+static const struct samsung_aes_variant exynos5433_slim_aes_data = {
+ .aes_offset = 0x400,
+ .hash_offset = 0x800,
+ .clk_names = { "pclk", "aclk", },
};
static const struct of_device_id s5p_sss_dt_match[] = {
@@ -400,6 +411,10 @@ static const struct of_device_id s5p_sss_dt_match[] = {
.compatible = "samsung,exynos4210-secss",
.data = &exynos_aes_data,
},
+ {
+ .compatible = "samsung,exynos5433-slim-sss",
+ .data = &exynos5433_slim_aes_data,
+ },
{ },
};
MODULE_DEVICE_TABLE(of, s5p_sss_dt_match);
@@ -2208,18 +2223,39 @@ static int s5p_aes_probe(struct platform_device *pdev)
return PTR_ERR(pdata->ioaddr);
}
- pdata->clk = devm_clk_get(dev, "secss");
+ pdata->clk = devm_clk_get(dev, variant->clk_names[0]);
if (IS_ERR(pdata->clk)) {
- dev_err(dev, "failed to find secss clock source\n");
+ dev_err(dev, "failed to find secss clock %s\n",
+ variant->clk_names[0]);
return -ENOENT;
}
err = clk_prepare_enable(pdata->clk);
if (err < 0) {
- dev_err(dev, "Enabling SSS clk failed, err %d\n", err);
+ dev_err(dev, "Enabling clock %s failed, err %d\n",
+ variant->clk_names[0], err);
return err;
}
+ if (variant->clk_names[1]) {
+ pdata->pclk = devm_clk_get(dev, variant->clk_names[1]);
+ if (IS_ERR(pdata->pclk)) {
+ dev_err(dev, "failed to find clock %s\n",
+ variant->clk_names[1]);
+ err = -ENOENT;
+ goto err_clk;
+ }
+
+ err = clk_prepare_enable(pdata->pclk);
+ if (err < 0) {
+ dev_err(dev, "Enabling clock %s failed, err %d\n",
+ variant->clk_names[0], err);
+ goto err_clk;
+ }
+ } else {
+ pdata->pclk = NULL;
+ }
+
spin_lock_init(&pdata->lock);
spin_lock_init(&pdata->hash_lock);
@@ -2295,8 +2331,11 @@ static int s5p_aes_probe(struct platform_device *pdev)
tasklet_kill(&pdata->tasklet);
err_irq:
- clk_disable_unprepare(pdata->clk);
+ if (pdata->pclk)
+ clk_disable_unprepare(pdata->pclk);
+err_clk:
+ clk_disable_unprepare(pdata->clk);
s5p_dev = NULL;
return err;
@@ -2323,6 +2362,9 @@ static int s5p_aes_remove(struct platform_device *pdev)
pdata->use_hash = false;
}
+ if (pdata->pclk)
+ clk_disable_unprepare(pdata->pclk);
+
clk_disable_unprepare(pdata->clk);
s5p_dev = NULL;
--
2.20.1
On Fri, Feb 22, 2019 at 01:21:41PM +0100, Kamil Konieczny wrote:
> Add slimSSS node to DT and crypto AES support for Exynos5433. Tested on
> Exynos5433 board with crypto run-time self tests and with tcrypt with
> command insmod tcrypt.ko mode=500 sec=1
>
> Changes since v3:
> - moved bindings documentation out from samsung-sss.txt into separate file
> samsung-slimsss.txt
>
> Changes since v2:
> - address Corentine Labbe note: add "const" to char* in declaration of
> struct samsung_aes_variant
>
> Changes since v1:
> - address Krzysztof Kozlowski review: add missing comma in struct
> definition, add goto for error code path, correct indentation,
> in documentation add that Exynos5433 has both slimSSS and SSS IPs.
>
> Kamil Konieczny (3):
> arm64: dts: exynos: add SlimSSS for Exynos5433
> dt-bindings: crypto: document Exynos5433 SlimSSS
> crypto: s5p: add AES support for Exynos5433
>
> .../bindings/crypto/samsung-slimsss.txt | 19 +++++++
> arch/arm64/boot/dts/exynos/exynos5433.dtsi | 9 ++++
> drivers/crypto/s5p-sss.c | 50 +++++++++++++++++--
> 3 files changed, 74 insertions(+), 4 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/crypto/samsung-slimsss.txt
Patches 2-3 applied. Thanks.
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
On Fri, 1 Mar 2019 at 11:07, Ard Biesheuvel <[email protected]> wrote:
>
> On Fri, 1 Mar 2019 at 10:56, Krzysztof Kozlowski <[email protected]> wrote:
> >
> > On Fri, 22 Feb 2019 at 13:22, Kamil Konieczny
> > <[email protected]> wrote:
> > >
> > > Add AES crypto HW acceleration for Exynos5433, with the help of SlimSSS IP.
> > >
> > > Reviewed-by: Krzysztof Kozlowski <[email protected]>
> > > Signed-off-by: Kamil Konieczny <[email protected]>
> > > ---
> > > drivers/crypto/s5p-sss.c | 50 ++++++++++++++++++++++++++++++++++++----
> > > 1 file changed, 46 insertions(+), 4 deletions(-)
> > > diff --git a/drivers/crypto/s5p-sss.c b/drivers/crypto/s5p-sss.c
> > > index 0064be0e3941..3f45cc5cb94a 100644
> > > --- a/drivers/crypto/s5p-sss.c
> > > +++ b/drivers/crypto/s5p-sss.c
> > > @@ -232,6 +232,7 @@
> > > * struct samsung_aes_variant - platform specific SSS driver data
> > > * @aes_offset: AES register offset from SSS module's base.
> > > * @hash_offset: HASH register offset from SSS module's base.
> > > + * @clk_names: names of clocks needed to run SSS IP
> > > *
> > > * Specifies platform specific configuration of SSS module.
> > > * Note: A structure for driver specific platform data is used for future
> > > @@ -240,6 +241,7 @@
> > > struct samsung_aes_variant {
> > > unsigned int aes_offset;
> > > unsigned int hash_offset;
> > > + const char *clk_names[];
> > > };
> > >
> > > struct s5p_aes_reqctx {
> > > @@ -296,6 +298,7 @@ struct s5p_aes_ctx {
> > > struct s5p_aes_dev {
> > > struct device *dev;
> > > struct clk *clk;
> > > + struct clk *pclk;
> > > void __iomem *ioaddr;
> > > void __iomem *aes_ioaddr;
> > > int irq_fc;
> > > @@ -384,11 +387,19 @@ struct s5p_hash_ctx {
> > > static const struct samsung_aes_variant s5p_aes_data = {
> > > .aes_offset = 0x4000,
> > > .hash_offset = 0x6000,
> > > + .clk_names = { "secss", },
> > > };
> > >
> > > static const struct samsung_aes_variant exynos_aes_data = {
> > > .aes_offset = 0x200,
> > > .hash_offset = 0x400,
> > > + .clk_names = { "secss", },
> > > +};
> > > +
> > > +static const struct samsung_aes_variant exynos5433_slim_aes_data = {
> > > + .aes_offset = 0x400,
> > > + .hash_offset = 0x800,
> > > + .clk_names = { "pclk", "aclk", },
> > > };
> > >
> > > static const struct of_device_id s5p_sss_dt_match[] = {
> > > @@ -400,6 +411,10 @@ static const struct of_device_id s5p_sss_dt_match[] = {
> > > .compatible = "samsung,exynos4210-secss",
> > > .data = &exynos_aes_data,
> > > },
> > > + {
> > > + .compatible = "samsung,exynos5433-slim-sss",
> > > + .data = &exynos5433_slim_aes_data,
> > > + },
> > > { },
> > > };
> > > MODULE_DEVICE_TABLE(of, s5p_sss_dt_match);
> > > @@ -2208,18 +2223,39 @@ static int s5p_aes_probe(struct platform_device *pdev)
> > > return PTR_ERR(pdata->ioaddr);
> > > }
> > >
> > > - pdata->clk = devm_clk_get(dev, "secss");
> > > + pdata->clk = devm_clk_get(dev, variant->clk_names[0]);
> > > if (IS_ERR(pdata->clk)) {
> > > - dev_err(dev, "failed to find secss clock source\n");
> > > + dev_err(dev, "failed to find secss clock %s\n",
> > > + variant->clk_names[0]);
> > > return -ENOENT;
> > > }
> > >
> > > err = clk_prepare_enable(pdata->clk);
> > > if (err < 0) {
> > > - dev_err(dev, "Enabling SSS clk failed, err %d\n", err);
> > > + dev_err(dev, "Enabling clock %s failed, err %d\n",
> > > + variant->clk_names[0], err);
> > > return err;
> > > }
> > >
> > > + if (variant->clk_names[1]) {
> > > + pdata->pclk = devm_clk_get(dev, variant->clk_names[1]);
> >
> > I see on today's linux next:
> > [ 6.391256] Unable to handle kernel paging request at virtual address 00004000
> > on all my Exynos boards (U3, XU, XU3, HC1).
> > I did not do a bisect yet but it might be connected with this commit.
> >
> > The call trace is:
> > (strcmp) from [<c0774014>] (of_property_match_string+0x58/0xd0)
> > (of_property_match_string) from [<c04d3d2c>] (__of_clk_get_by_name+0x5c/0x130)
> > (__of_clk_get_by_name) from [<c04d3fa0>] (clk_get+0x34/0x70)
> > (clk_get) from [<c04d3680>] (devm_clk_get+0x38/0x74)
> > (devm_clk_get) from [<c075c248>] (s5p_aes_probe+0xd4/0x4a0)
> > (s5p_aes_probe) from [<c059dbc4>] (platform_drv_probe+0x6c/0xa4)
> > (platform_drv_probe) from [<c059b4c8>] (really_probe+0x280/0x414)
> > (really_probe) from [<c059b810>] (driver_probe_device+0x78/0x1c0)
> > (driver_probe_device) from [<c059bbb8>] (device_driver_attach+0x58/0x60)
> > (device_driver_attach) from [<c059bcbc>] (__driver_attach+0xfc/0x160)
> > (__driver_attach) from [<c059928c>] (bus_for_each_dev+0x68/0xb4)
> > (bus_for_each_dev) from [<c059a5c4>] (bus_add_driver+0x158/0x214)
> > (bus_add_driver) from [<c059cb94>] (driver_register+0x78/0x110)
> > (driver_register) from [<c01031f8>] (do_one_initcall+0x8c/0x404)
> >
> > Full log:
> > https://krzk.eu/#/builders/21/builds/1153/steps/12/logs/serial0
> >
> > The s5p_aes_probe address points here.
> >
>
> It seems that dereferencing clk_names[1] of struct samsung_aes_variant
> exynos_aes_data
>
> > > + .clk_names = { "secss", },
>
> returns the first member of
>
> > > static const struct samsung_aes_variant s5p_aes_data = {
> > > .aes_offset = 0x4000,
>
> since the array has only one entry in this case, and so the second,
> non-existing entry overlaps with the next data item in the object
> file.
>
> So the fix would be to use
>
> .clk_names = { "secss", NULL },
>
> instead (or define the array to have 2 members)
Ah yes, the size of the array is not fixed during definition. Probably
having explicit size would be less error-prone in the future (if
someone adds third clock).
Best regards,
Krzysztof
On Fri, 22 Feb 2019 at 13:22, Kamil Konieczny
<[email protected]> wrote:
>
> Add AES crypto HW acceleration for Exynos5433, with the help of SlimSSS IP.
>
> Reviewed-by: Krzysztof Kozlowski <[email protected]>
> Signed-off-by: Kamil Konieczny <[email protected]>
> ---
> drivers/crypto/s5p-sss.c | 50 ++++++++++++++++++++++++++++++++++++----
> 1 file changed, 46 insertions(+), 4 deletions(-)
> diff --git a/drivers/crypto/s5p-sss.c b/drivers/crypto/s5p-sss.c
> index 0064be0e3941..3f45cc5cb94a 100644
> --- a/drivers/crypto/s5p-sss.c
> +++ b/drivers/crypto/s5p-sss.c
> @@ -232,6 +232,7 @@
> * struct samsung_aes_variant - platform specific SSS driver data
> * @aes_offset: AES register offset from SSS module's base.
> * @hash_offset: HASH register offset from SSS module's base.
> + * @clk_names: names of clocks needed to run SSS IP
> *
> * Specifies platform specific configuration of SSS module.
> * Note: A structure for driver specific platform data is used for future
> @@ -240,6 +241,7 @@
> struct samsung_aes_variant {
> unsigned int aes_offset;
> unsigned int hash_offset;
> + const char *clk_names[];
> };
>
> struct s5p_aes_reqctx {
> @@ -296,6 +298,7 @@ struct s5p_aes_ctx {
> struct s5p_aes_dev {
> struct device *dev;
> struct clk *clk;
> + struct clk *pclk;
> void __iomem *ioaddr;
> void __iomem *aes_ioaddr;
> int irq_fc;
> @@ -384,11 +387,19 @@ struct s5p_hash_ctx {
> static const struct samsung_aes_variant s5p_aes_data = {
> .aes_offset = 0x4000,
> .hash_offset = 0x6000,
> + .clk_names = { "secss", },
> };
>
> static const struct samsung_aes_variant exynos_aes_data = {
> .aes_offset = 0x200,
> .hash_offset = 0x400,
> + .clk_names = { "secss", },
> +};
> +
> +static const struct samsung_aes_variant exynos5433_slim_aes_data = {
> + .aes_offset = 0x400,
> + .hash_offset = 0x800,
> + .clk_names = { "pclk", "aclk", },
> };
>
> static const struct of_device_id s5p_sss_dt_match[] = {
> @@ -400,6 +411,10 @@ static const struct of_device_id s5p_sss_dt_match[] = {
> .compatible = "samsung,exynos4210-secss",
> .data = &exynos_aes_data,
> },
> + {
> + .compatible = "samsung,exynos5433-slim-sss",
> + .data = &exynos5433_slim_aes_data,
> + },
> { },
> };
> MODULE_DEVICE_TABLE(of, s5p_sss_dt_match);
> @@ -2208,18 +2223,39 @@ static int s5p_aes_probe(struct platform_device *pdev)
> return PTR_ERR(pdata->ioaddr);
> }
>
> - pdata->clk = devm_clk_get(dev, "secss");
> + pdata->clk = devm_clk_get(dev, variant->clk_names[0]);
> if (IS_ERR(pdata->clk)) {
> - dev_err(dev, "failed to find secss clock source\n");
> + dev_err(dev, "failed to find secss clock %s\n",
> + variant->clk_names[0]);
> return -ENOENT;
> }
>
> err = clk_prepare_enable(pdata->clk);
> if (err < 0) {
> - dev_err(dev, "Enabling SSS clk failed, err %d\n", err);
> + dev_err(dev, "Enabling clock %s failed, err %d\n",
> + variant->clk_names[0], err);
> return err;
> }
>
> + if (variant->clk_names[1]) {
> + pdata->pclk = devm_clk_get(dev, variant->clk_names[1]);
I see on today's linux next:
[ 6.391256] Unable to handle kernel paging request at virtual address 00004000
on all my Exynos boards (U3, XU, XU3, HC1).
I did not do a bisect yet but it might be connected with this commit.
The call trace is:
(strcmp) from [<c0774014>] (of_property_match_string+0x58/0xd0)
(of_property_match_string) from [<c04d3d2c>] (__of_clk_get_by_name+0x5c/0x130)
(__of_clk_get_by_name) from [<c04d3fa0>] (clk_get+0x34/0x70)
(clk_get) from [<c04d3680>] (devm_clk_get+0x38/0x74)
(devm_clk_get) from [<c075c248>] (s5p_aes_probe+0xd4/0x4a0)
(s5p_aes_probe) from [<c059dbc4>] (platform_drv_probe+0x6c/0xa4)
(platform_drv_probe) from [<c059b4c8>] (really_probe+0x280/0x414)
(really_probe) from [<c059b810>] (driver_probe_device+0x78/0x1c0)
(driver_probe_device) from [<c059bbb8>] (device_driver_attach+0x58/0x60)
(device_driver_attach) from [<c059bcbc>] (__driver_attach+0xfc/0x160)
(__driver_attach) from [<c059928c>] (bus_for_each_dev+0x68/0xb4)
(bus_for_each_dev) from [<c059a5c4>] (bus_add_driver+0x158/0x214)
(bus_add_driver) from [<c059cb94>] (driver_register+0x78/0x110)
(driver_register) from [<c01031f8>] (do_one_initcall+0x8c/0x404)
Full log:
https://krzk.eu/#/builders/21/builds/1153/steps/12/logs/serial0
The s5p_aes_probe address points here.
Best regards,
Krzysztof
>
> + if (IS_ERR(pdata->pclk)) {
> + dev_err(dev, "failed to find clock %s\n",
> + variant->clk_names[1]);
> + err = -ENOENT;
> + goto err_clk;
> + }
> +
> + err = clk_prepare_enable(pdata->pclk);
> + if (err < 0) {
> + dev_err(dev, "Enabling clock %s failed, err %d\n",
> + variant->clk_names[0], err);
> + goto err_clk;
> + }
> + } else {
> + pdata->pclk = NULL;
> + }
> +
> spin_lock_init(&pdata->lock);
> spin_lock_init(&pdata->hash_lock);
>
> @@ -2295,8 +2331,11 @@ static int s5p_aes_probe(struct platform_device *pdev)
> tasklet_kill(&pdata->tasklet);
>
> err_irq:
> - clk_disable_unprepare(pdata->clk);
> + if (pdata->pclk)
> + clk_disable_unprepare(pdata->pclk);
>
> +err_clk:
> + clk_disable_unprepare(pdata->clk);
> s5p_dev = NULL;
>
> return err;
> @@ -2323,6 +2362,9 @@ static int s5p_aes_remove(struct platform_device *pdev)
> pdata->use_hash = false;
> }
>
> + if (pdata->pclk)
> + clk_disable_unprepare(pdata->pclk);
> +
> clk_disable_unprepare(pdata->clk);
> s5p_dev = NULL;
>
> --
> 2.20.1
>
On Fri, 1 Mar 2019 at 10:56, Krzysztof Kozlowski <[email protected]> wrote:
>
> On Fri, 22 Feb 2019 at 13:22, Kamil Konieczny
> <[email protected]> wrote:
> >
> > Add AES crypto HW acceleration for Exynos5433, with the help of SlimSSS IP.
> >
> > Reviewed-by: Krzysztof Kozlowski <[email protected]>
> > Signed-off-by: Kamil Konieczny <[email protected]>
> > ---
> > drivers/crypto/s5p-sss.c | 50 ++++++++++++++++++++++++++++++++++++----
> > 1 file changed, 46 insertions(+), 4 deletions(-)
> > diff --git a/drivers/crypto/s5p-sss.c b/drivers/crypto/s5p-sss.c
> > index 0064be0e3941..3f45cc5cb94a 100644
> > --- a/drivers/crypto/s5p-sss.c
> > +++ b/drivers/crypto/s5p-sss.c
> > @@ -232,6 +232,7 @@
> > * struct samsung_aes_variant - platform specific SSS driver data
> > * @aes_offset: AES register offset from SSS module's base.
> > * @hash_offset: HASH register offset from SSS module's base.
> > + * @clk_names: names of clocks needed to run SSS IP
> > *
> > * Specifies platform specific configuration of SSS module.
> > * Note: A structure for driver specific platform data is used for future
> > @@ -240,6 +241,7 @@
> > struct samsung_aes_variant {
> > unsigned int aes_offset;
> > unsigned int hash_offset;
> > + const char *clk_names[];
> > };
> >
> > struct s5p_aes_reqctx {
> > @@ -296,6 +298,7 @@ struct s5p_aes_ctx {
> > struct s5p_aes_dev {
> > struct device *dev;
> > struct clk *clk;
> > + struct clk *pclk;
> > void __iomem *ioaddr;
> > void __iomem *aes_ioaddr;
> > int irq_fc;
> > @@ -384,11 +387,19 @@ struct s5p_hash_ctx {
> > static const struct samsung_aes_variant s5p_aes_data = {
> > .aes_offset = 0x4000,
> > .hash_offset = 0x6000,
> > + .clk_names = { "secss", },
> > };
> >
> > static const struct samsung_aes_variant exynos_aes_data = {
> > .aes_offset = 0x200,
> > .hash_offset = 0x400,
> > + .clk_names = { "secss", },
> > +};
> > +
> > +static const struct samsung_aes_variant exynos5433_slim_aes_data = {
> > + .aes_offset = 0x400,
> > + .hash_offset = 0x800,
> > + .clk_names = { "pclk", "aclk", },
> > };
> >
> > static const struct of_device_id s5p_sss_dt_match[] = {
> > @@ -400,6 +411,10 @@ static const struct of_device_id s5p_sss_dt_match[] = {
> > .compatible = "samsung,exynos4210-secss",
> > .data = &exynos_aes_data,
> > },
> > + {
> > + .compatible = "samsung,exynos5433-slim-sss",
> > + .data = &exynos5433_slim_aes_data,
> > + },
> > { },
> > };
> > MODULE_DEVICE_TABLE(of, s5p_sss_dt_match);
> > @@ -2208,18 +2223,39 @@ static int s5p_aes_probe(struct platform_device *pdev)
> > return PTR_ERR(pdata->ioaddr);
> > }
> >
> > - pdata->clk = devm_clk_get(dev, "secss");
> > + pdata->clk = devm_clk_get(dev, variant->clk_names[0]);
> > if (IS_ERR(pdata->clk)) {
> > - dev_err(dev, "failed to find secss clock source\n");
> > + dev_err(dev, "failed to find secss clock %s\n",
> > + variant->clk_names[0]);
> > return -ENOENT;
> > }
> >
> > err = clk_prepare_enable(pdata->clk);
> > if (err < 0) {
> > - dev_err(dev, "Enabling SSS clk failed, err %d\n", err);
> > + dev_err(dev, "Enabling clock %s failed, err %d\n",
> > + variant->clk_names[0], err);
> > return err;
> > }
> >
> > + if (variant->clk_names[1]) {
> > + pdata->pclk = devm_clk_get(dev, variant->clk_names[1]);
>
> I see on today's linux next:
> [ 6.391256] Unable to handle kernel paging request at virtual address 00004000
> on all my Exynos boards (U3, XU, XU3, HC1).
> I did not do a bisect yet but it might be connected with this commit.
>
> The call trace is:
> (strcmp) from [<c0774014>] (of_property_match_string+0x58/0xd0)
> (of_property_match_string) from [<c04d3d2c>] (__of_clk_get_by_name+0x5c/0x130)
> (__of_clk_get_by_name) from [<c04d3fa0>] (clk_get+0x34/0x70)
> (clk_get) from [<c04d3680>] (devm_clk_get+0x38/0x74)
> (devm_clk_get) from [<c075c248>] (s5p_aes_probe+0xd4/0x4a0)
> (s5p_aes_probe) from [<c059dbc4>] (platform_drv_probe+0x6c/0xa4)
> (platform_drv_probe) from [<c059b4c8>] (really_probe+0x280/0x414)
> (really_probe) from [<c059b810>] (driver_probe_device+0x78/0x1c0)
> (driver_probe_device) from [<c059bbb8>] (device_driver_attach+0x58/0x60)
> (device_driver_attach) from [<c059bcbc>] (__driver_attach+0xfc/0x160)
> (__driver_attach) from [<c059928c>] (bus_for_each_dev+0x68/0xb4)
> (bus_for_each_dev) from [<c059a5c4>] (bus_add_driver+0x158/0x214)
> (bus_add_driver) from [<c059cb94>] (driver_register+0x78/0x110)
> (driver_register) from [<c01031f8>] (do_one_initcall+0x8c/0x404)
>
> Full log:
> https://krzk.eu/#/builders/21/builds/1153/steps/12/logs/serial0
>
> The s5p_aes_probe address points here.
>
It seems that dereferencing clk_names[1] of struct samsung_aes_variant
exynos_aes_data
> > + .clk_names = { "secss", },
returns the first member of
> > static const struct samsung_aes_variant s5p_aes_data = {
> > .aes_offset = 0x4000,
since the array has only one entry in this case, and so the second,
non-existing entry overlaps with the next data item in the object
file.
So the fix would be to use
.clk_names = { "secss", NULL },
instead (or define the array to have 2 members)
>
> >
> > + if (IS_ERR(pdata->pclk)) {
> > + dev_err(dev, "failed to find clock %s\n",
> > + variant->clk_names[1]);
> > + err = -ENOENT;
> > + goto err_clk;
> > + }
> > +
> > + err = clk_prepare_enable(pdata->pclk);
> > + if (err < 0) {
> > + dev_err(dev, "Enabling clock %s failed, err %d\n",
> > + variant->clk_names[0], err);
> > + goto err_clk;
> > + }
> > + } else {
> > + pdata->pclk = NULL;
> > + }
> > +
> > spin_lock_init(&pdata->lock);
> > spin_lock_init(&pdata->hash_lock);
> >
> > @@ -2295,8 +2331,11 @@ static int s5p_aes_probe(struct platform_device *pdev)
> > tasklet_kill(&pdata->tasklet);
> >
> > err_irq:
> > - clk_disable_unprepare(pdata->clk);
> > + if (pdata->pclk)
> > + clk_disable_unprepare(pdata->pclk);
> >
> > +err_clk:
> > + clk_disable_unprepare(pdata->clk);
> > s5p_dev = NULL;
> >
> > return err;
> > @@ -2323,6 +2362,9 @@ static int s5p_aes_remove(struct platform_device *pdev)
> > pdata->use_hash = false;
> > }
> >
> > + if (pdata->pclk)
> > + clk_disable_unprepare(pdata->pclk);
> > +
> > clk_disable_unprepare(pdata->clk);
> > s5p_dev = NULL;
> >
> > --
> > 2.20.1
> >
On Fri, Feb 22, 2019 at 01:21:44PM +0100, Kamil Konieczny wrote:
> Add AES crypto HW acceleration for Exynos5433, with the help of SlimSSS IP.
>
> Reviewed-by: Krzysztof Kozlowski <[email protected]>
> Signed-off-by: Kamil Konieczny <[email protected]>
> ---
> drivers/crypto/s5p-sss.c | 50 ++++++++++++++++++++++++++++++++++++----
> 1 file changed, 46 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/crypto/s5p-sss.c b/drivers/crypto/s5p-sss.c
> index 0064be0e3941..3f45cc5cb94a 100644
> --- a/drivers/crypto/s5p-sss.c
> +++ b/drivers/crypto/s5p-sss.c
> @@ -232,6 +232,7 @@
> * struct samsung_aes_variant - platform specific SSS driver data
> * @aes_offset: AES register offset from SSS module's base.
> * @hash_offset: HASH register offset from SSS module's base.
> + * @clk_names: names of clocks needed to run SSS IP
> *
> * Specifies platform specific configuration of SSS module.
> * Note: A structure for driver specific platform data is used for future
> @@ -240,6 +241,7 @@
> struct samsung_aes_variant {
> unsigned int aes_offset;
> unsigned int hash_offset;
> + const char *clk_names[];
This array does not have a fixed size.
> };
>
> struct s5p_aes_reqctx {
> @@ -296,6 +298,7 @@ struct s5p_aes_ctx {
> struct s5p_aes_dev {
> struct device *dev;
> struct clk *clk;
> + struct clk *pclk;
> void __iomem *ioaddr;
> void __iomem *aes_ioaddr;
> int irq_fc;
> @@ -384,11 +387,19 @@ struct s5p_hash_ctx {
> static const struct samsung_aes_variant s5p_aes_data = {
> .aes_offset = 0x4000,
> .hash_offset = 0x6000,
> + .clk_names = { "secss", },
In this instantiation, there is no [1] element.
> };
>
> static const struct samsung_aes_variant exynos_aes_data = {
> .aes_offset = 0x200,
> .hash_offset = 0x400,
> + .clk_names = { "secss", },
and neither is here.
> +};
> +
> +static const struct samsung_aes_variant exynos5433_slim_aes_data = {
> + .aes_offset = 0x400,
> + .hash_offset = 0x800,
> + .clk_names = { "pclk", "aclk", },
> };
>
> static const struct of_device_id s5p_sss_dt_match[] = {
> @@ -400,6 +411,10 @@ static const struct of_device_id s5p_sss_dt_match[] = {
> .compatible = "samsung,exynos4210-secss",
> .data = &exynos_aes_data,
> },
> + {
> + .compatible = "samsung,exynos5433-slim-sss",
> + .data = &exynos5433_slim_aes_data,
> + },
> { },
> };
> MODULE_DEVICE_TABLE(of, s5p_sss_dt_match);
> @@ -2208,18 +2223,39 @@ static int s5p_aes_probe(struct platform_device *pdev)
> return PTR_ERR(pdata->ioaddr);
> }
>
> - pdata->clk = devm_clk_get(dev, "secss");
> + pdata->clk = devm_clk_get(dev, variant->clk_names[0]);
> if (IS_ERR(pdata->clk)) {
> - dev_err(dev, "failed to find secss clock source\n");
> + dev_err(dev, "failed to find secss clock %s\n",
> + variant->clk_names[0]);
> return -ENOENT;
> }
>
> err = clk_prepare_enable(pdata->clk);
> if (err < 0) {
> - dev_err(dev, "Enabling SSS clk failed, err %d\n", err);
> + dev_err(dev, "Enabling clock %s failed, err %d\n",
> + variant->clk_names[0], err);
> return err;
> }
>
> + if (variant->clk_names[1]) {
This results in
[ 31.721963] Unhandled fault: page domain fault (0x01b) at 0x00004000
[ 31.722417] pgd = (ptrval)
[ 31.722611] [00004000] *pgd=00000000
[ 31.723519] Internal error: : 1b [#1] PREEMPT SMP ARM
...
[ 31.753264] [<c0be9978>] (strcmp) from [<c08cad50>] (of_property_match_string+0x58/0xd0)
[ 31.754012] [<c08cad50>] (of_property_match_string) from [<c061eaac>] (of_clk_get_hw+0x5c/0x114)
[ 31.754731] [<c061eaac>] (of_clk_get_hw) from [<c061656c>] (clk_get+0x34/0x74)
[ 31.755397] [<c061656c>] (clk_get) from [<c0615dd8>] (devm_clk_get+0x3c/0x6c)
[ 31.756018] [<c0615dd8>] (devm_clk_get) from [<c08b3298>] (s5p_aes_probe+0xcc/0x508)
[ 31.756892] [<c08b3298>] (s5p_aes_probe) from [<c06e6740>] (platform_drv_probe+0x48/0x98)
[ 31.757571] [<c06e6740>] (platform_drv_probe) from [<c06e40a4>] (really_probe+0x2bc/0x408)
[ 31.758245] [<c06e40a4>] (really_probe) from [<c06e43a4>] (driver_probe_device+0x78/0x1cc)
[ 31.758913] [<c06e43a4>] (driver_probe_device) from [<c06e4764>] (device_driver_attach+0x58/0x60)
[ 31.759578] [<c06e4764>] (device_driver_attach) from [<c06e483c>] (__driver_attach+0xd0/0x168)
[ 31.760010] [<c06e483c>] (__driver_attach) from [<c06e1e38>] (bus_for_each_dev+0x74/0xb4)
[ 31.760354] [<c06e1e38>] (bus_for_each_dev) from [<c06e3174>] (bus_add_driver+0x174/0x210)
[ 31.760697] [<c06e3174>] (bus_add_driver) from [<c06e572c>] (driver_register+0x74/0x108)
[ 31.761039] [<c06e572c>] (driver_register) from [<c010325c>] (do_one_initcall+0x90/0x428)
[ 31.761390] [<c010325c>] (do_one_initcall) from [<c1001318>] (kernel_init_freeable+0x42c/0x4cc)
[ 31.761752] [<c1001318>] (kernel_init_freeable) from [<c0bf3ae8>] (kernel_init+0x8/0x118)
[ 31.762093] [<c0bf3ae8>] (kernel_init) from [<c01010b4>] (ret_from_fork+0x14/0x20)
when booting one of the "old" platforms where variant->clk_names[1]
points beyond the end of the array.
Guenter
> + pdata->pclk = devm_clk_get(dev, variant->clk_names[1]);
> + if (IS_ERR(pdata->pclk)) {
> + dev_err(dev, "failed to find clock %s\n",
> + variant->clk_names[1]);
> + err = -ENOENT;
> + goto err_clk;
> + }
> +
> + err = clk_prepare_enable(pdata->pclk);
> + if (err < 0) {
> + dev_err(dev, "Enabling clock %s failed, err %d\n",
> + variant->clk_names[0], err);
> + goto err_clk;
> + }
> + } else {
> + pdata->pclk = NULL;
> + }
> +
> spin_lock_init(&pdata->lock);
> spin_lock_init(&pdata->hash_lock);
>
> @@ -2295,8 +2331,11 @@ static int s5p_aes_probe(struct platform_device *pdev)
> tasklet_kill(&pdata->tasklet);
>
> err_irq:
> - clk_disable_unprepare(pdata->clk);
> + if (pdata->pclk)
> + clk_disable_unprepare(pdata->pclk);
>
> +err_clk:
> + clk_disable_unprepare(pdata->clk);
> s5p_dev = NULL;
>
> return err;
> @@ -2323,6 +2362,9 @@ static int s5p_aes_remove(struct platform_device *pdev)
> pdata->use_hash = false;
> }
>
> + if (pdata->pclk)
> + clk_disable_unprepare(pdata->pclk);
> +
> clk_disable_unprepare(pdata->clk);
> s5p_dev = NULL;
>
On Tue, 5 Mar 2019 at 18:13, Guenter Roeck <[email protected]> wrote:
>
> On Fri, Feb 22, 2019 at 01:21:44PM +0100, Kamil Konieczny wrote:
> > Add AES crypto HW acceleration for Exynos5433, with the help of SlimSSS IP.
> >
> > Reviewed-by: Krzysztof Kozlowski <[email protected]>
> > Signed-off-by: Kamil Konieczny <[email protected]>
> > ---
> > drivers/crypto/s5p-sss.c | 50 ++++++++++++++++++++++++++++++++++++----
> > 1 file changed, 46 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/crypto/s5p-sss.c b/drivers/crypto/s5p-sss.c
> > index 0064be0e3941..3f45cc5cb94a 100644
> > --- a/drivers/crypto/s5p-sss.c
> > +++ b/drivers/crypto/s5p-sss.c
> > @@ -232,6 +232,7 @@
> > * struct samsung_aes_variant - platform specific SSS driver data
> > * @aes_offset: AES register offset from SSS module's base.
> > * @hash_offset: HASH register offset from SSS module's base.
> > + * @clk_names: names of clocks needed to run SSS IP
> > *
> > * Specifies platform specific configuration of SSS module.
> > * Note: A structure for driver specific platform data is used for future
> > @@ -240,6 +241,7 @@
> > struct samsung_aes_variant {
> > unsigned int aes_offset;
> > unsigned int hash_offset;
> > + const char *clk_names[];
>
> This array does not have a fixed size.
Yes, correct. Kamil already sent a fix:
https://patchwork.kernel.org/patch/10835389/
Best regards,
Krzysztof
On 3/5/19 11:51 AM, Krzysztof Kozlowski wrote:
> On Tue, 5 Mar 2019 at 18:13, Guenter Roeck <[email protected]> wrote:
>>
>> On Fri, Feb 22, 2019 at 01:21:44PM +0100, Kamil Konieczny wrote:
>>> Add AES crypto HW acceleration for Exynos5433, with the help of SlimSSS IP.
>>>
>>> Reviewed-by: Krzysztof Kozlowski <[email protected]>
>>> Signed-off-by: Kamil Konieczny <[email protected]>
>>> ---
>>> drivers/crypto/s5p-sss.c | 50 ++++++++++++++++++++++++++++++++++++----
>>> 1 file changed, 46 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/crypto/s5p-sss.c b/drivers/crypto/s5p-sss.c
>>> index 0064be0e3941..3f45cc5cb94a 100644
>>> --- a/drivers/crypto/s5p-sss.c
>>> +++ b/drivers/crypto/s5p-sss.c
>>> @@ -232,6 +232,7 @@
>>> * struct samsung_aes_variant - platform specific SSS driver data
>>> * @aes_offset: AES register offset from SSS module's base.
>>> * @hash_offset: HASH register offset from SSS module's base.
>>> + * @clk_names: names of clocks needed to run SSS IP
>>> *
>>> * Specifies platform specific configuration of SSS module.
>>> * Note: A structure for driver specific platform data is used for future
>>> @@ -240,6 +241,7 @@
>>> struct samsung_aes_variant {
>>> unsigned int aes_offset;
>>> unsigned int hash_offset;
>>> + const char *clk_names[];
>>
>> This array does not have a fixed size.
>
> Yes, correct. Kamil already sent a fix:
> https://patchwork.kernel.org/patch/10835389/
>
Excellent. Note that the offending patch is now in mainline.
Guenter
On Fri, Feb 22, 2019 at 01:21:42PM +0100, Kamil Konieczny wrote:
> Add DT node for SlimSSS (aka Slim SecuritySubSystem) in Exynos5433 SoC.
> The users can use compatibility "samsung,exynos5433-slim-sss".
>
> Signed-off-by: Kamil Konieczny <[email protected]>
> ---
> arch/arm64/boot/dts/exynos/exynos5433.dtsi | 9 +++++++++
Thanks, applied.
Best regards,
Krzysztof