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

Reply via email to