RE: [PATCH 1/2] crypto: Add Imagination Technologies hw hash accelerator

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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����?���&��





[Index of Archives]     [Kernel]     [Gnu Classpath]     [Gnu Crypto]     [DM Crypt]     [Netfilter]     [Bugtraq]

  Powered by Linux