lkml.org 
[lkml]   [2015]   [Nov]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    SubjectRe: [PATCH v2 1/4] Crypto: Crypto driver support aes/des/des3 for rk3288
    Date
    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);
    -------------------

    > + 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

    [...]

    > + 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.


    Thanks
    Heiko


    \
     
     \ /
      Last update: 2015-11-08 00:41    [W:3.112 / U:0.276 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site