[PATCH v3 03/14] crypto: aspeed: Add AST2600 HACE support

[Simon Glass]

Hi ChiaWei,

On Fri, 23 Jul 2021 at 00:30, ChiaWei Wang <chiawei_wang at aspeedtech.com> wrote:
>
> Hi Simon,
>
> > -----Original Message-----
> > From: Simon Glass <sjg at chromium.org>
> > Sent: Wednesday, July 21, 2021 2:33 AM
> >
> > Hi Chia-Wei,
> >
> > On Tue, 20 Jul 2021 at 00:38, Chia-Wei Wang
> > <chiawei_wang at aspeedtech.com> wrote:
> > >
> > > From: Joel Stanley <joel at jms.id.au>
> > >
> > > Hash and Crypto Engine (HACE) is designed to accelerate the throughput
> > > of hash data digest, and symmetric-key encryption.
> > >
> > > Signed-off-by: Joel Stanley <joel at jms.id.au>
> > > Signed-off-by: Chia-Wei Wang <chiawei_wang at aspeedtech.com>
> > > ---
> > >  drivers/crypto/Kconfig              |   2 +
> > >  drivers/crypto/Makefile             |   1 +
> > >  drivers/crypto/aspeed/Kconfig       |  12 ++
> > >  drivers/crypto/aspeed/Makefile      |   1 +
> > >  drivers/crypto/aspeed/aspeed_hace.c | 308
> > > ++++++++++++++++++++++++++++
> > >  5 files changed, 324 insertions(+)
> > >  create mode 100644 drivers/crypto/aspeed/Kconfig  create mode 100644
> > > drivers/crypto/aspeed/Makefile  create mode 100644
> > > drivers/crypto/aspeed/aspeed_hace.c
> >
> > This hash interface is wonky.
> >
> > There should be a hash uclass and this driver should be in that uclass. The
> > existing hw_... functions should be dropped.
> >
> > Please let me know if you need further guidance, but basically, we need a
> > proper uclass and driver.
>
> I am thinking to take the UCLASS_MOD_EXP as an example to create UCLASS_HASH.
> This UCLASS_HASH should provide .ops callback like UCLASS_MOD_EXP do to support both SW and HW implemented HSAH.
> Is this a right direction to start with?

Yes I think so. You can have API like those in struct hash_algo and
hopefully remove that struct.

Then there is a software driver to write. Then people can add hardware
drivers as needed.

The hash uclass needs provide a way to call the first driver it finds.
We can either make it add a software driver or have it call the
software functions directly if there is no (hardware) driver. I'm not
sure which is best...

- having a s/w driver requires binding it at runtime
- not having a s/w driver makes the uclass a bit odd

>
> In addition, adding UCLASS_HASH may involve lots of common code modification.
> Should we make it part of the current patch series? Or a new patch series after this one?

I would prefer a series before this one since it needs to be done
before this. Or patches in the same series if you prefer.

The problem has been created already, unfortunately, so this is a clean-up.

Regards,
Simon