[PATCH] AMCC Crypto4xx Device Driver v2]
James Hsiao
jhsiao at amcc.com
Wed Oct 29 13:18:20 EST 2008
Hi Josh,
Yes, I did miss couple of Kim's comments.
Also, I did not include the message when I submit the V2 patch to Kim.
That patch have format problem so nobody except Kim recieved it.
Please see inline.
Thanks
James
On Tue, 2008-10-28 at 20:51 -0400, Josh Boyer wrote:
> On Tue, 28 Oct 2008 16:41:16 -0700
> James Hsiao <jhsiao at amcc.com> wrote:
>
> > Hi Josh,
> >
> > I am reposting this patch. Thanks Kim Phillips for pointing out format
> > of my patch.
> >
> > Again this patch was already reviewed by Kim Phillips on linux-crypyo.
>
> Kim did a really good review and you only fixed a handful of easy
> things.
>
> > Kim suggest us submit to linuxppc-dev for review.
>
> Yeah, that's fine. But generally you reply to all the
> questions/comments from the original review. I see several unanswered
> comments that still apply to this version. Things like:
>
> - The device_type wasn't removed in the DTS change
Ok, I will remove it in V3 patch.
> - The question on ABLKCIPHER kconfig was ignored
Is ABLKCIPHER a sub set of BLKCIPHER? So, if BLKCIPHER is selected then
if ABLKCIPHER is present, it will use ABLKCIPHER otherwise using
BLKCIPHER algorithm? Correct?
> - Just returning -ENOMEM instead of using a goto for simple error cases
Yes, I miss that one.
> - Marking functions static
We have more than one file, that is why some of the function are not
static.
> - Global lsec_core variable which doesn't allow for more than one
> device
We only support single incidence of device.
> - Complete lack of locking code, how do you enforce mutually exclusive
> access to the device?
The crypto engine have couple bits 'command ready' and 'packet done',
which servers as semaphore here. So, software don't need extra locking.
>
> I'll do a full review tomorrow because I see really odd things in here
> in addition to the above, but I'd like to know why those comments from
> Kim's review weren't answered.
>
> josh
>
More information about the Linuxppc-dev
mailing list