Hi Vladimir > -----Original Message----- > From: linux-crypto-owner@xxxxxxxxxxxxxxx [mailto:linux-crypto- > owner@xxxxxxxxxxxxxxx] On Behalf Of Vladimir Zapolskiy > Sent: 11 November 2014 15:12 > To: James Hartley; grant.likely@xxxxxxxxxx; robh+dt@xxxxxxxxxx; > akpm@xxxxxxxxxxxxxxxxxxxx > Cc: herbert@xxxxxxxxxxxxxxxxxxx; davem@xxxxxxxxxxxxx; > gregkh@xxxxxxxxxxxxxxxxxxx; joe@xxxxxxxxxxx; > mchehab@xxxxxxxxxxxxxxx; crope@xxxxxx; jg1.han@xxxxxxxxxxx; linux- > crypto@xxxxxxxxxxxxxxx; 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 > > 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. 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 majordomo@xxxxxxxxxxxxxxx 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 ��.n��������+%������w��{.n�����{���{ay�ʇڙ���f���h������_�(�階�ݢj"��������G����?���&��