On Fri, 6 Mar 2015 17:42:26 +0100
Christophe Leroy <christophe.leroy@xxxxxx> wrote:
> This patch adds talitos1.c and talitos1.h with all specificities needed
> to handle the SEC1 security engine found in MPC885 and MPC8272.
>
> The SEC1 has several differences with its younger brother SEC2:
> * Several bits in registers have different locations
> * Many bits are missing
> * Some bits are in addition
> * SEC1 doesn't support scatter/gather
> * SEC1 has a different descriptor structure
>
> We add a helper function for clearing the desc field in the descriptor as that field needs to be cleared on SEC1 and doesn't exist on SEC2.
>
> Signed-off-by: Christophe Leroy <christophe.leroy@xxxxxx>
Hi Christophe,
Thanks for rebasing on cryptodev-2.6, now I can easily see the
differences for review.
> +++ b/drivers/crypto/Kconfig
> @@ -211,7 +211,8 @@ config CRYPTO_DEV_TALITOS
> select CRYPTO_ALGAPI
> select CRYPTO_AUTHENC
> select HW_RANDOM
> - select CRYPTO_DEV_TALITOS2
> + select CRYPTO_DEV_TALITOS1 if PPC_8xx || PPC_82xx
> + select CRYPTO_DEV_TALITOS2 if !CRYPTO_DEV_TALITOS1
This will set CONFIG_CRYPTO_DEV_TALITOS1=y on a kernel with both
PPC_82xx and PPC_83xx set, and will therefore break talitos when run
on an 83xx part.
The driver needs to work on both v1 and v2/3 SECs dynamically,
and behave accordingly depending on what version h/w is described in
the device tree ("fsl,secX").
So, barring making a completely new driver, a couple of points:
- A h/w version 1 vs 2 compatible priv variable can be allocated for
parts of the driver that need quicker access than calling
of_device_is_compatible(). Other parts that do better as complete
function rewrites (the scatter-gather mapping fns?) can be
abstracted away using pointers to the v1 vs. v2+ base functions.
The pointers would live in the device's priv struct, and be assigned
at module initialization time.
- instead of rewriting the structs talitos_ptr, talitos_desc, either
use the v2-named member as-is, or make unions.
E.g., instead of having a new struct talitos_ptr, either use the v2
h/w version as-is and shift the length into place, or, add a v1 len
via a union, and make to_talitos_ptr a pointer to a function, with
the v1 version of the function updating only the v1 len.
For struct talitos_desc, how about:
struct talitos_desc {
__be32 hdr;
struct talitos_ptr[8];
}
and have the desc->ptr[X] assigning code add 1 via a new macro iff
on a SEC2 compatible (the hdr_lo code would have to be hardcoded to
refer to ptr[0] instead of hdr_lo directly, and next_desc would be
ptr[7]).
talitos_edesc shouldn't need the extra ptr_{in,out}, rather allocate
the extra space when they're needed, and refer to them via aliases
to defines that cast and use link_tbl[0] as a base reference.
For IO register accesses, either a lookup table can be used, or,
since they're not frequent, define v1-specific defines in addition
to the v2 ones, and reference using the compatible variable.
Other than that, please make sure to use checkpatch.pl --strict,
sparse -D__CHECK_ENDIAN__, and that building as a module doesn't
break.
Thanks,
Kim
--
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
