[PATCH next v7 04/12] crypto: aspeed: Add AST2600 HACE support

[Johnny Huang]

> On 2021/11/16, 2:33 PM, "Joel Stanley" <joel at jms.id.au> wrote:
>
> On Thu, 21 Oct 2021 at 02:13, ChiaWei Wang <chiawei_wang at aspeedtech.com> wrote:
> >
> > > From: Joel Stanley <joel at jms.id.au>
> > > Sent: Thursday, October 21, 2021 8:08 AM
> > >
> > > On Wed, 20 Oct 2021 at 02:50, Chia-Wei Wang
> > > <chiawei_wang at aspeedtech.com> wrote:
> > > >
> > > > From: Johnny Huang <johnny_huang at aspeedtech.com>
> > > >
> > > > Hash and Crypto Engine (HACE) is designed to accelerate the throughput
> > > > of hash data digest, and symmetric-key encryption.
> > > >
> > > > Signed-off-by: Johnny Huang <johnny_huang at aspeedtech.com>
> > > > Signed-off-by: Chia-Wei Wang <chiawei_wang at aspeedtech.com>
> > >
> > > I see you've re-written the driver from the version I submitted. It's got a bit
> > > more code now, but there's no explanation as to why you needed to add this
> > > code.
> > >
> > > From what I can guess, you needed to pat the watchdog while waiting for hash
> > > operations to complete. Why not pat the watchdog in
> > > aspeed_hace_wait_completion instead of chunking up the operation?
> >
> > The AST2600 HACE has HW issue using one shot mode.
> > The patch you previously provided is exactly the workaround for that. (Thanks!)
> > https://lore.kernel.org/openbmc/20A7316B-7A1B-41B3-B5D4-EF9CEA98F279@linux.ibm.com/
> >
> > To avoid upstreaming a workaround solution, Johnny confirmed that HACE has no issue using accumulate mode.
> > Thus the driver is re-written.
>
> Is there a problem in having to perform the reset? As I said, we have
> been using this driver in our systems for some time now and it
> performs well.
>
> I have a strong preference for keeping the driver as it was first implemented.

AST2600 HACE and RSA engine are sharing the same reset, the reset workaround may 
cause some issue in RSA driver.

> >
> > >
> > > Secondly, your incremental API is not using the scatter gather mode any more.
> > > Is there a reason for this? We have been using it successfully in the system for
> > > some time.
> >
> > This is merely for the simplicity of driver implementation.
> > As MMU is not enabled in U-Boot. We thought that SG mode may not be required.
>
> I think it is prefered, as it eliminates the need to memcpy the data around.