From: Cristian Stoica <cristian.stoica@freescale.com>
To: Kim Phillips <kim.phillips@freescale.com>
Cc: <herbert@gondor.apana.org.au>, <davem@davemloft.net>,
<linux-crypto@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
<horia.geanta@freescale.com>, <marex@denx.de>
Subject: Re: [PATCH] crypto: caam: fix error reporting
Date: Tue, 4 Nov 2014 10:57:57 +0200 [thread overview]
Message-ID: <54589515.2010903@freescale.com> (raw)
In-Reply-To: <20141103134716.775acd39d6334c6f8aeca151@freescale.com>
Hi Kim,
>> Actually, our static code analyzer did not see this one.
>
> ok, so the patch technically isn't fixing anything broken, then.
Are you saying the code isn't broken _because_ a static tool analyser
did not see anything wrong here?
> the new code just added a new condition, which doesn't invalidate
> the comment. And simply removing the comment as opposed to amending
> it is a bit overkill.
You are partially right, but will the staggering lack of comments in the
caam driver be fixed by duplicating a cascade of three ifs into english?
>> It is indeed simpler but does it consider also the missing error codes
>> at index 1 and 5? Just checking for an upper bound is not enough.
>
> no, the existing code already handles that. Note that newer
> documentation fills the 1 and 5 slots, too.
If you have the new error codes please send them to me for an update.
>> On the other hand, if the error field is only three bits wide instead of
>> four as stated by the documentation, a better fix means using a three
>> bit mask instead of reporting an invalid error code.
>
> true, but then we'd introduce a direct discrepancy with the
> documentation, and thus h/w.
You basically ask me to agree that if there are no _documented_ error
codes between 0x8 and 0xf then I should trust that they will never come
up on a 4 bit field.
Do you want me to drop the patch and pretend there is nothing to see?
Cristian S.
next prev parent reply other threads:[~2014-11-04 8:58 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-10-31 16:57 [PATCH] crypto: caam: fix error reporting Cristian Stoica
2014-10-31 18:22 ` Kim Phillips
2014-11-01 11:43 ` Marek Vasut
2014-11-03 9:18 ` Cristian Stoica
2014-11-03 19:47 ` Kim Phillips
2014-11-04 8:57 ` Cristian Stoica [this message]
2014-11-04 16:57 ` Kim Phillips
2014-11-05 9:21 ` [PATCH v2] " Cristian Stoica
2014-11-05 16:43 ` Kim Phillips
2014-11-06 8:01 ` Cristian Stoica
2014-11-06 15:17 ` Herbert Xu
2014-11-05 9:27 ` [PATCH] " Cristian Stoica
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=54589515.2010903@freescale.com \
--to=cristian.stoica@freescale.com \
--cc=davem@davemloft.net \
--cc=herbert@gondor.apana.org.au \
--cc=horia.geanta@freescale.com \
--cc=kim.phillips@freescale.com \
--cc=linux-crypto@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=marex@denx.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.