| Message ID | 29530953.Mr1LQyjF1I@phil (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Heiko, Thanks for your serious comments always. On 2015?11?08? 07:19, Heiko Stuebner wrote: > Hi Zain, > > looks like my comment on v1 came later than your v2 submission, > so here it is again :-) > > Am Freitag, 6. November 2015, 09:17:21 schrieb Zain Wang: >> The names registered are: >> ecb(aes) cbc(aes) ecb(des) cbc(des) ecb(des3_ede) cbc(des3_ede) >> You can alloc tags above in your case. >> >> And other algorithms and platforms will be added later on. >> >> Signed-off-by: Zain Wang <zain.wang@rock-chips.com> >> --- >> > [...] > >> diff --git a/drivers/crypto/rockchip/rk3288_crypto.c b/drivers/crypto/rockchip/rk3288_crypto.c >> new file mode 100644 >> index 0000000..c2a419b >> --- /dev/null >> +++ b/drivers/crypto/rockchip/rk3288_crypto.c > [...] > >> +static int rk_crypto_probe(struct platform_device *pdev) >> +{ >> + int err = 0; >> + struct resource *res; >> + struct device *dev = &pdev->dev; >> + struct crypto_info_t *crypto_info; >> + > rk3288 chromebooks use the crypto-engine to validate the boot images and > seem to leave it in a half-on state. This results in an irq pending > during probe and thus a null-pointer dereference in the irq-handler, as > it runs before the crypto-device is fully initialized. > > resetting the crypto block, successfull fixed that issue, so I did the > following change: > > ------------------- > diff --git a/arch/arm/boot/dts/rk3288.dtsi b/arch/arm/boot/dts/rk3288.dtsi > index 121b6d5..e978fb2 100644 > --- a/arch/arm/boot/dts/rk3288.dtsi > +++ b/arch/arm/boot/dts/rk3288.dtsi > @@ -182,6 +182,8 @@ > "hclk", > "sclk", > "apb_pclk"; > + resets = <&cru SRST_CRYPTO>; > + reset-names = "crypto"; > status = "okay"; > }; > > diff --git a/drivers/crypto/rockchip/rk3288_crypto.c b/drivers/crypto/rockchip/rk3288_crypto.c > index 02830f2..2245d3d 100644 > --- a/drivers/crypto/rockchip/rk3288_crypto.c > +++ b/drivers/crypto/rockchip/rk3288_crypto.c > @@ -18,6 +18,7 @@ > #include <linux/of.h> > #include <linux/clk.h> > #include <linux/crypto.h> > +#include <linux/reset.h> > > struct crypto_info_t *crypto_p; > > @@ -266,6 +267,15 @@ static int rk_crypto_probe(struct platform_device *pdev) > struct resource *res; > struct device *dev = &pdev->dev; > struct crypto_info_t *crypto_info; > + struct reset_control *rst; > + > + /* reset the block to remove any pending actions */ > + rst = devm_reset_control_get(dev, "crypto"); > + if (!IS_ERR(rst)) { > + reset_control_assert(rst); > + usleep_range(10, 20); > + reset_control_deassert(rst); > + } > > crypto_info = devm_kzalloc(&pdev->dev, > sizeof(*crypto_info), GFP_KERNEL); > ------------------- > Ok! done! >> + crypto_info = devm_kzalloc(&pdev->dev, >> + sizeof(*crypto_info), GFP_KERNEL); >> + if (!crypto_info) >> + return -ENOMEM; >> + >> + spin_lock_init(&crypto_info->lock); >> + >> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); >> + crypto_info->reg = devm_ioremap_resource(&pdev->dev, res); >> + if (IS_ERR(crypto_info->reg)) { >> + err = PTR_ERR(crypto_info->reg); >> + goto err_ioremap; >> + } >> + >> + crypto_info->aclk = devm_clk_get(&pdev->dev, "aclk"); >> + if (IS_ERR(crypto_info->aclk)) { >> + err = PTR_ERR(crypto_info->aclk); >> + goto err_ioremap; >> + } >> + >> + crypto_info->hclk = devm_clk_get(&pdev->dev, "hclk"); >> + if (IS_ERR(crypto_info->hclk)) { >> + err = PTR_ERR(crypto_info->hclk); >> + goto err_ioremap; >> + } >> + >> + crypto_info->sclk = devm_clk_get(&pdev->dev, "sclk"); >> + if (IS_ERR(crypto_info->sclk)) { >> + err = PTR_ERR(crypto_info->sclk); >> + goto err_ioremap; >> + } >> + >> + crypto_info->dmaclk = devm_clk_get(&pdev->dev, "apb_pclk"); >> + if (IS_ERR(crypto_info->dmaclk)) { >> + err = PTR_ERR(crypto_info->dmaclk); >> + goto err_ioremap; >> + } >> + >> + crypto_info->irq = platform_get_irq(pdev, 0); >> + if (crypto_info->irq < 0) { >> + dev_warn(crypto_info->dev, >> + "control Interrupt is not available.\n"); >> + err = crypto_info->irq; >> + goto err_ioremap; >> + } >> + >> + err = devm_request_irq(&pdev->dev, crypto_info->irq, crypto_irq_handle, >> + IRQF_SHARED, "rk-crypto", pdev); >> + >> + if (err) { >> + dev_err(crypto_info->dev, "irq request failed.\n"); >> + goto err_ioremap; >> + } >> + >> + crypto_info->dev = &pdev->dev; >> + platform_set_drvdata(pdev, crypto_info); >> + crypto_p = crypto_info; >> + >> + tasklet_init(&crypto_info->crypto_tasklet, >> + rk_crypto_tasklet_cb, (unsigned long)crypto_info); >> + crypto_init_queue(&crypto_info->queue, 50); >> + >> + crypto_info->enable_clk = rk_crypto_enable_clk; >> + crypto_info->disable_clk = rk_crypto_disable_clk; >> + crypto_info->load_data = rk_load_data; >> + crypto_info->unload_data = rk_unload_data; >> + >> + err = rk_crypto_register(); >> + if (err) { >> + dev_err(dev, "err in register alg"); >> + goto err_reg_alg; >> + } >> + >> + return 0; >> + >> +err_reg_alg: >> + free_irq(crypto_info->irq, crypto_info); >> +err_ioremap: >> + crypto_p = NULL; >> + >> + return err; >> +} >> + > [...] > >> diff --git a/drivers/crypto/rockchip/rk3288_crypto.h b/drivers/crypto/rockchip/rk3288_crypto.h >> new file mode 100644 >> index 0000000..cf4cd18 >> --- /dev/null >> +++ b/drivers/crypto/rockchip/rk3288_crypto.h >> @@ -0,0 +1,222 @@ > [...] > >> +struct crypto_info_t { > this is highly rockchip specific, so should probably be named > rk_crypto_info > or similar instead of a generic sounding crypto_info_t Ok! Done! > > [...] > >> + struct device *dev; >> + struct clk *aclk; >> + struct clk *hclk; >> + struct clk *sclk; >> + struct clk *dmaclk; >> + void __iomem *reg; >> + int irq; >> + struct crypto_queue queue; >> + struct tasklet_struct crypto_tasklet; >> + struct ablkcipher_request *ablk_req; >> + /* device lock */ >> + spinlock_t lock; >> + >> + /* the public variable */ >> + struct scatterlist *sg_src; >> + struct scatterlist *sg_dst; >> + struct scatterlist sg_tmp; >> + struct scatterlist *first; >> + unsigned int left_bytes; >> + void *addr_vir; >> + int aligned; >> + int align_size; >> + size_t nents; >> + unsigned int total; >> + unsigned int count; >> + u32 mode; >> + dma_addr_t addr_in; >> + dma_addr_t addr_out; >> + int (*start)(struct crypto_info_t *dev); >> + int (*update)(struct crypto_info_t *dev); >> + void (*complete)(struct crypto_info_t *dev, int err); >> + int (*enable_clk)(struct crypto_info_t *dev); >> + void (*disable_clk)(struct crypto_info_t *dev); >> + int (*load_data)(struct crypto_info_t *dev, >> + struct scatterlist *sg_src, >> + struct scatterlist *sg_dst); >> + void (*unload_data)(struct crypto_info_t *dev); >> +}; >> + >> +/* the private variable of cipher */ >> +struct rk_cipher_ctx { >> + struct crypto_info_t *dev; >> + unsigned int keylen; >> +}; >> + >> +extern struct crypto_info_t *crypto_p; >> + >> +extern struct crypto_alg rk_ecb_aes_alg; >> +extern struct crypto_alg rk_cbc_aes_alg; >> +extern struct crypto_alg rk_ecb_des_alg; >> +extern struct crypto_alg rk_cbc_des_alg; >> +extern struct crypto_alg rk_ecb_des3_ede_alg; >> +extern struct crypto_alg rk_cbc_des3_ede_alg; >> + >> +#endif >> diff --git a/drivers/crypto/rockchip/rk3288_crypto_ablkcipher.c b/drivers/crypto/rockchip/rk3288_crypto_ablkcipher.c >> new file mode 100644 >> index 0000000..28b49c9 >> --- /dev/null >> +++ b/drivers/crypto/rockchip/rk3288_crypto_ablkcipher.c >> @@ -0,0 +1,511 @@ > [...] > >> +static int rk_ablk_cra_init(struct crypto_tfm *tfm) >> +{ >> + struct rk_cipher_ctx *ctx = crypto_tfm_ctx(tfm); >> + >> + ctx->dev = crypto_p; > please no static pointers for devices. > > For example, sunxi_ss does the following to transport the core device-data > into the init function: > > struct sun4i_tfm_ctx *op = crypto_tfm_ctx(tfm); > struct crypto_alg *alg = tfm->__crt_alg; > struct sun4i_ss_alg_template *algt; > > algt = container_of(alg, struct sun4i_ss_alg_template, alg.crypto); > op->ss = algt->ss; > > so you could probably do something similar > > Same of course for the other crypto_p users. Ok! Done! > > > Thanks > Heiko > > > Thanks Zain
diff --git a/arch/arm/boot/dts/rk3288.dtsi b/arch/arm/boot/dts/rk3288.dtsi index 121b6d5..e978fb2 100644 --- a/arch/arm/boot/dts/rk3288.dtsi +++ b/arch/arm/boot/dts/rk3288.dtsi @@ -182,6 +182,8 @@ "hclk", "sclk", "apb_pclk"; + resets = <&cru SRST_CRYPTO>; + reset-names = "crypto"; status = "okay"; }; diff --git a/drivers/crypto/rockchip/rk3288_crypto.c b/drivers/crypto/rockchip/rk3288_crypto.c index 02830f2..2245d3d 100644 --- a/drivers/crypto/rockchip/rk3288_crypto.c +++ b/drivers/crypto/rockchip/rk3288_crypto.c @@ -18,6 +18,7 @@ #include <linux/of.h> #include <linux/clk.h> #include <linux/crypto.h> +#include <linux/reset.h> struct crypto_info_t *crypto_p; @@ -266,6 +267,15 @@ static int rk_crypto_probe(struct platform_device *pdev) struct resource *res; struct device *dev = &pdev->dev; struct crypto_info_t *crypto_info; + struct reset_control *rst; + + /* reset the block to remove any pending actions */ + rst = devm_reset_control_get(dev, "crypto"); + if (!IS_ERR(rst)) { + reset_control_assert(rst); + usleep_range(10, 20); + reset_control_deassert(rst); + } crypto_info = devm_kzalloc(&pdev->dev, sizeof(*crypto_info), GFP_KERNEL);