| Message ID | 20190222122144.19024-4-k.konieczny@partner.samsung.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | add AES support for Exynos5433 | expand |
On Fri, 22 Feb 2019 at 13:22, Kamil Konieczny <k.konieczny@partner.samsung.com> wrote: > > Add AES crypto HW acceleration for Exynos5433, with the help of SlimSSS IP. > > Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org> > Signed-off-by: Kamil Konieczny <k.konieczny@partner.samsung.com> > --- > 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 <krzk@kernel.org> wrote: > > On Fri, 22 Feb 2019 at 13:22, Kamil Konieczny > <k.konieczny@partner.samsung.com> wrote: > > > > Add AES crypto HW acceleration for Exynos5433, with the help of SlimSSS IP. > > > > Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org> > > Signed-off-by: Kamil Konieczny <k.konieczny@partner.samsung.com> > > --- > > 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, 1 Mar 2019 at 11:07, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote: > > On Fri, 1 Mar 2019 at 10:56, Krzysztof Kozlowski <krzk@kernel.org> wrote: > > > > On Fri, 22 Feb 2019 at 13:22, Kamil Konieczny > > <k.konieczny@partner.samsung.com> wrote: > > > > > > Add AES crypto HW acceleration for Exynos5433, with the help of SlimSSS IP. > > > > > > Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org> > > > Signed-off-by: Kamil Konieczny <k.konieczny@partner.samsung.com> > > > --- > > > 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, 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 <krzk@kernel.org> > Signed-off-by: Kamil Konieczny <k.konieczny@partner.samsung.com> > --- > 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 <linux@roeck-us.net> 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 <krzk@kernel.org> > > Signed-off-by: Kamil Konieczny <k.konieczny@partner.samsung.com> > > --- > > 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 <linux@roeck-us.net> 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 <krzk@kernel.org> >>> Signed-off-by: Kamil Konieczny <k.konieczny@partner.samsung.com> >>> --- >>> 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
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;