On Thu, Sep 26, 2013 at 8:18 AM, Marek Vasut <marex@xxxxxxx> wrote:
> +config CRYPTO_DEV_MXS_DCP
> + tristate "Support for Freescale MXS DCP"
> + depends on ARCH_MXS
> + select CRYPTO_SHA1
> + select CRYPTO_SHA256
> + select CRYPTO_CBC
> + select CRYPTO_ECB
> + select CRYPTO_AES
> + select CRYPTO_BLKCIPHER
> + select CRYPTO_ALGAPI
> + help
> + The Freescale i.MX23/i.MX28 has SHA1/SHA256 and AES128 CBC/ECB
> + co-processor on the die.
> +
> + To compile this driver as a module, choose M here: the module
> + will be called atmel-sha.
Actually it will be called 'mxs-dcp'.
> + * There can even be only one instance of the MXS DCP due to the
> + * design of Linux Crypto API.
Is this true? Usually we don't want to create a global struct.
> +
> +/* AES 128 ECB and AES 128 CBC */
> +static struct crypto_alg dcp_aes_algs[] = {
> + [0] = {
No need for explicitely add this [0]
> + .cra_name = "ecb(aes)",
> + .cra_driver_name = "ecb-aes-dcp",
> + .cra_priority = 400,
> + .cra_alignmask = 15,
> + .cra_flags = CRYPTO_ALG_TYPE_ABLKCIPHER |
> + CRYPTO_ALG_ASYNC |
> + CRYPTO_ALG_NEED_FALLBACK,
> + .cra_init = mxs_dcp_aes_fallback_init,
> + .cra_exit = mxs_dcp_aes_fallback_exit,
> + .cra_blocksize = AES_BLOCK_SIZE,
> + .cra_ctxsize = sizeof(struct dcp_async_ctx),
> + .cra_type = &crypto_ablkcipher_type,
> + .cra_module = THIS_MODULE,
> + .cra_u = {
> + .ablkcipher = {
> + .min_keysize = AES_MIN_KEY_SIZE,
> + .max_keysize = AES_MAX_KEY_SIZE,
> + .setkey = mxs_dcp_aes_setkey,
> + .encrypt = mxs_dcp_aes_ecb_encrypt,
> + .decrypt = mxs_dcp_aes_ecb_decrypt
> + }
> + }
> + },
> + [1] = {
Same here.
> +static int mxs_dcp_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct dcp *sdcp = NULL;
> + int i, ret;
> +
> + struct resource *iores;
> + int dcp_vmi_irq, dcp_irq;
> +
> + mutex_lock(&global_mutex);
> + if (global_sdcp) {
> + dev_err(dev, "Only one DCP instance allowed!\n");
> + ret = -ENODEV;
> + goto err_mutex;
> + }
> +
> + iores = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + dcp_vmi_irq = platform_get_irq(pdev, 0);
> + dcp_irq = platform_get_irq(pdev, 1);
> + if (!iores || dcp_vmi_irq < 0 || dcp_irq < 0) {
No need to check for !iores here.
You use it inside devm_ioremap_resource, which already does this checking.
> + /*
> + * We do not enable context switching. Give the context buffer a
> + * pointer to an illegal address so if context switching is
> + * inadvertantly enabled, the DCP will return an error instead of
> + * trashing good memory. The DCP DMA cannot access ROM, so any ROM
> + * address will do.
> + */
> + writel(0xffff0000, sdcp->base + MXS_DCP_CONTEXT);
Can you use a define instead of this hardcoded number?
> +static int mxs_dcp_remove(struct platform_device *pdev)
> +{
> + struct dcp *sdcp;
> + int i;
> +
> + sdcp = platform_get_drvdata(pdev);
> +
> + kthread_stop(sdcp->thread[DCP_CHAN_HASH_SHA]);
> + kthread_stop(sdcp->thread[DCP_CHAN_CRYPTO]);
> +
> + platform_set_drvdata(pdev, NULL);
> +
> + dma_free_coherent(sdcp->dev, sizeof(struct dcp_coherent_block),
> + sdcp->coh, sdcp->coh_phys);
> +
> + if (sdcp->caps & MXS_DCP_CAPABILITY1_SHA256)
> + crypto_unregister_ahash(&dcp_sha256_alg);
> +
> + if (sdcp->caps & MXS_DCP_CAPABILITY1_SHA1)
> + crypto_unregister_ahash(&dcp_sha1_alg);
> +
> + if (sdcp->caps & MXS_DCP_CAPABILITY1_AES128) {
> + for (i = ARRAY_SIZE(dcp_aes_algs); i >= 0; i--)
> + crypto_unregister_alg(&dcp_aes_algs[i]);
> + }
> +
> + mutex_lock(&global_mutex);
> + global_sdcp = NULL;
> + mutex_unlock(&global_mutex);
The order of the resources removal does not look correct here.
It should match the order of the error path in probe.
> +
> + return 0;
> +}
> +
> +static const struct of_device_id mxs_dcp_dt_ids[] = {
> + {.compatible = "fsl,mxs-dcp", .data = NULL,},
In the other mxs/imx drivers we use:
.compatible = "fsl,<soc>-dcp"
You also need to provide a devicetree documentation for this binding.
> + { /* sentinel */ }
> +};
> +
> +MODULE_DEVICE_TABLE(of, mxs_dcp_dt_ids);
> +
> +static struct platform_driver mxs_dcp_driver = {
> + .probe = mxs_dcp_probe,
> + .remove = mxs_dcp_remove,
> + .driver = {
> + .name = "mxs-dcp",
> + .owner = THIS_MODULE,
> + .of_match_table = mxs_dcp_dt_ids,
> + },
> +};
> +
> +module_platform_driver(mxs_dcp_driver);
> +
> +MODULE_AUTHOR("Marek Vasut <marex@xxxxxxx>");
> +MODULE_DESCRIPTION("Freescale MXS DCP Driver");
> +MODULE_LICENSE("GPL");
Could also add MODULE_ALIAS.
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
