Hi Vladimir > -----Original Message----- > From: linux-crypto-ow...@vger.kernel.org [mailto:linux-crypto- > ow...@vger.kernel.org] On Behalf Of Vladimir Zapolskiy > Sent: 11 November 2014 15:12 > To: James Hartley; grant.lik...@linaro.org; robh...@kernel.org; > a...@linux-foundation.org > Cc: herb...@gondor.apana.org.au; da...@davemloft.net; > gre...@linuxfoundation.org; j...@perches.com; > mche...@osg.samsung.com; cr...@iki.fi; jg1....@samsung.com; linux- > cry...@vger.kernel.org; devicet...@vger.kernel.org; > pawel.m...@arm.com; mark.rutl...@arm.com; > ijc+devicet...@hellion.org.uk; ga...@codeaurora.org; > abres...@chromium.org; Ezequiel Garcia > Subject: Re: [PATCH 1/2] crypto: Add Imagination Technologies hw hash > accelerator > > Hi James, > > On 11.11.2014 16:59, James Hartley wrote: > > Hi Vladimir, thanks for the review! > > > >> -----Original Message----- > >> From: Vladimir Zapolskiy [mailto:vladimir_zapols...@mentor.com] > >> Sent: 10 November 2014 15:10 > >> To: James Hartley; herb...@gondor.apana.org.au; > da...@davemloft.net; > >> grant.lik...@linaro.org; robh...@kernel.org; > >> a...@linux-foundation.org; gre...@linuxfoundation.org; > >> j...@perches.com; mche...@osg.samsung.com; cr...@iki.fi; > >> jg1....@samsung.com; linux- cry...@vger.kernel.org > >> Cc: devicet...@vger.kernel.org; pawel.m...@arm.com; > >> mark.rutl...@arm.com; ijc+devicet...@hellion.org.uk; > >> ga...@codeaurora.org; abres...@chromium.org; 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.hart...@imgtec.com> > >>> --- > >> > > [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.
Yes you are right, I will remove them > > [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(). As above. > > -- > With best wishes, > Vladimir > -- > To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the > body of a message to majord...@vger.kernel.org More majordomo info at > http://vger.kernel.org/majordomo-info.html Thanks again for the quick review. I'll post a V2 patchset when I've figured out what I can do about the error handling you mentioned previously. Thanks James