Hi James, On 11.11.2014 16:59, James Hartley wrote: > Hi Vladimir, thanks for the review! > >> -----Original Message----- >> From: Vladimir Zapolskiy [mailto:vladimir_zapolskiy@xxxxxxxxxx] >> Sent: 10 November 2014 15:10 >> To: James Hartley; herbert@xxxxxxxxxxxxxxxxxxx; davem@xxxxxxxxxxxxx; >> grant.likely@xxxxxxxxxx; robh+dt@xxxxxxxxxx; akpm@xxxxxxxxxxxxxxxxxxxx; >> gregkh@xxxxxxxxxxxxxxxxxxx; joe@xxxxxxxxxxx; >> mchehab@xxxxxxxxxxxxxxx; crope@xxxxxx; jg1.han@xxxxxxxxxxx; linux- >> crypto@xxxxxxxxxxxxxxx >> Cc: devicetree@xxxxxxxxxxxxxxx; pawel.moll@xxxxxxx; >> mark.rutland@xxxxxxx; ijc+devicetree@xxxxxxxxxxxxxx; >> galak@xxxxxxxxxxxxxx; abrestic@xxxxxxxxxxxx; Ezequiel Garcia >> Subject: Re: [PATCH 1/2] crypto: Add Imagination Technologies hw hash >> accelerator >> >> Hello James, >> >> On 10.11.2014 14:10, James Hartley wrote: >>> This adds support for the Imagination Technologies hash accelerator >>> that provides hardware acceleration for >>> SHA1 SHA224 SHA256 and MD5 Hashes. >>> >>> Signed-off-by: James Hartley <james.hartley@xxxxxxxxxx> >>> --- >> [snip] >>> + >>> + return 0; >>> + >>> +err_algs: >>> + spin_lock(&img_hash.lock); >>> + list_del(&hdev->list); >>> + spin_unlock(&img_hash.lock); >>> + dma_release_channel(hdev->dma_lch); >>> +err_dma: >>> + iounmap(hdev->io_base); >> >> Mixing of devm_* resource initialization and commodity resource release >> leads to double decrement of clock usage count reference. > > Ok, changed to devm_iounmap > just one small comment, please double check, but most probably you don't need to call devm_iounmap() explicitly on error path. [snip] > >> >>> + >>> +static int img_hash_remove(struct platform_device *pdev) { >>> + static struct img_hash_dev *hdev; >>> + >>> + hdev = platform_get_drvdata(pdev); >>> + if (!hdev) >>> + return -ENODEV; >>> + spin_lock(&img_hash.lock); >>> + list_del(&hdev->list); >>> + spin_unlock(&img_hash.lock); >>> + >>> + img_unregister_algs(hdev); >>> + >>> + tasklet_kill(&hdev->done_task); >>> + tasklet_kill(&hdev->dma_task); >>> + img_hash_dma_cleanup(hdev); >>> + >>> + iounmap(hdev->io_base); >> >> Same as above, devres iounmap() is good enough. > > Done > Same as above, I suppose you can simply remove iounmap() call without adding explicit devm_iounmap(). -- With best wishes, Vladimir -- 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
