Hi Vladimir
> -----Original Message-----
> From: [email protected] [mailto:linux-crypto-
> [email protected]] On Behalf Of Vladimir Zapolskiy
> Sent: 11 November 2014 15:12
> To: James Hartley; [email protected]; [email protected];
> [email protected]
> Cc: [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected]; [email protected]; linux-
> [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; 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:[email protected]]
> >> Sent: 10 November 2014 15:10
> >> To: James Hartley; [email protected];
> [email protected];
> >> [email protected]; [email protected];
> >> [email protected]; [email protected];
> >> [email protected]; [email protected]; [email protected];
> >> [email protected]; linux- [email protected]
> >> Cc: [email protected]; [email protected];
> >> [email protected]; [email protected];
> >> [email protected]; [email protected]; 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 <[email protected]>
> >>> ---
> >>
>
> [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 [email protected] 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
