| Message ID | 20181220152815.3613164-1-arnd@arndb.de (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Herbert Xu |
| Headers | show |
| Series | crypto: caam/qi2 - add a CRYPTO_DEV_FSL_CAAM dependency | expand |
On 12/20/2018 5:28 PM, Arnd Bergmann wrote: > My previous bugfix was incomplete, we still have a broken kernel > with CRYPTO_DEV_FSL_CAAM=m and CRYPTO_DEV_FSL_DPAA2_CAAM=y: > > drivers/crypto/caam/caamalg_desc.o: In function `cnstr_shdsc_aead_null_encap': > caamalg_desc.c:(.text+0x14): undefined reference to `caam_little_end' > drivers/crypto/caam/caamalg_desc.o: In function `cnstr_shdsc_aead_null_encap': > caamalg_desc.c:(.text+0x310): undefined reference to `caam_imx' > caamalg_desc.c:(.text+0x4a8): undefined reference to `caam_little_end' > drivers/crypto/caam/caamalg_desc.o: In function `cnstr_shdsc_aead_null_encap': > caamalg_desc.c:(.text+0x664): undefined reference to `caam_imx' > > Everything is fine for the other combinations: if both are loadable > modules, or both are built-in, the flags work as expected, also if > only one of the two is enabled. > > Add a dependency to enforce using one of the working configurations. > Overall, I'm still not happy with that dependency, but for now > it documents what the code requires. > Thanks Arnd. This indeed solves the issue, but as you mentioned won't allow for CRYPTO_DEV_FSL_DPAA2_CAAM=y when CRYPTO_DEV_FSL_CAAM=m. I think a better solution is the following: -- >8 -- Subject: [PATCH] crypto: caam - move shared symbols in a common location There are several issues with symbols shared b/w: -caam/jr and caam/qi drivers on one hand -caam/qi2 driver on the other hand Commit 52813ab24959 ("crypto: caam/qi2 - avoid double export") fixed some of them, however compilation still fails for CRYPTO_DEV_FSL_CAAM=m and CRYPTO_DEV_FSL_DPAA2_CAAM=y. Another issue is related to dependency cycles reported by depmod when CRYPTO_DEV_FSL_CAAM=n and CRYPTO_DEV_FSL_DPAA2_CAAM=m, as mentioned in 82c7b351be3f ("Revert "arm64: defconfig: Enable FSL_MC_BUS and FSL_MC_DPIO"") To fix all these, move the symbols shared by these drivers in a common location. The only existing possibility is error.c file (note that naming doesn't help and should eventually change). Fixes: 52813ab24959 ("crypto: caam/qi2 - avoid double export") Reported-by: Arnd Bergmann <arnd@arndb.de> Signed-off-by: Horia Geantă <horia.geanta@nxp.com> --- drivers/crypto/caam/caamalg_qi2.c | 7 ------- drivers/crypto/caam/ctrl.c | 4 ---- drivers/crypto/caam/error.c | 6 ++++++ 3 files changed, 6 insertions(+), 11 deletions(-) diff --git a/drivers/crypto/caam/caamalg_qi2.c b/drivers/crypto/caam/caamalg_qi2.c index 425d5d974613..cc59814afd29 100644 --- a/drivers/crypto/caam/caamalg_qi2.c +++ b/drivers/crypto/caam/caamalg_qi2.c @@ -25,13 +25,6 @@ #define CAAM_MAX_KEY_SIZE (AES_MAX_KEY_SIZE + CTR_RFC3686_NONCE_SIZE + \ SHA512_DIGEST_SIZE * 2) -#if !IS_ENABLED(CONFIG_CRYPTO_DEV_FSL_CAAM) -bool caam_little_end; -EXPORT_SYMBOL(caam_little_end); -bool caam_imx; -EXPORT_SYMBOL(caam_imx); -#endif - /* * This is a a cache of buffers, from which the users of CAAM QI driver * can allocate short buffers. It's speedier than doing kmalloc on the hotpath. diff --git a/drivers/crypto/caam/ctrl.c b/drivers/crypto/caam/ctrl.c index 16bbc72f041a..14fb09223156 100644 --- a/drivers/crypto/caam/ctrl.c +++ b/drivers/crypto/caam/ctrl.c @@ -18,12 +18,8 @@ #include "desc_constr.h" #include "ctrl.h" -bool caam_little_end; -EXPORT_SYMBOL(caam_little_end); bool caam_dpaa2; EXPORT_SYMBOL(caam_dpaa2); -bool caam_imx; -EXPORT_SYMBOL(caam_imx); #ifdef CONFIG_CAAM_QI #include "qi.h" diff --git a/drivers/crypto/caam/error.c b/drivers/crypto/caam/error.c index 7e8d690f2827..21a70fd32f5d 100644 --- a/drivers/crypto/caam/error.c +++ b/drivers/crypto/caam/error.c @@ -50,6 +50,12 @@ void caam_dump_sg(const char *level, const char *prefix_str, int prefix_type, #endif /* DEBUG */ EXPORT_SYMBOL(caam_dump_sg); +bool caam_little_end; +EXPORT_SYMBOL(caam_little_end); + +bool caam_imx; +EXPORT_SYMBOL(caam_imx); + static const struct { u8 value; const char *error_text;
On Thu, Dec 20, 2018 at 09:27:16PM +0000, Horia Geanta wrote: > > This indeed solves the issue, but as you mentioned won't allow for > CRYPTO_DEV_FSL_DPAA2_CAAM=y when CRYPTO_DEV_FSL_CAAM=m. > > I think a better solution is the following: > > -- >8 -- > Subject: [PATCH] crypto: caam - move shared symbols in a common location Hi Horia: Please resend this patch with a new Subject line as patchwork no longer tracks patches sent in reply to existing patches. Thanks,
diff --git a/drivers/crypto/caam/Kconfig b/drivers/crypto/caam/Kconfig index c4b1cade55c1..4dff65e518e7 100644 --- a/drivers/crypto/caam/Kconfig +++ b/drivers/crypto/caam/Kconfig @@ -158,6 +158,7 @@ config CRYPTO_DEV_FSL_DPAA2_CAAM tristate "QorIQ DPAA2 CAAM (DPSECI) driver" depends on FSL_MC_DPIO depends on NETDEVICES + depends on CRYPTO_DEV_FSL_CAAM || !CRYPTO_DEV_FSL_CAAM select CRYPTO_DEV_FSL_CAAM_COMMON select CRYPTO_BLKCIPHER select CRYPTO_AUTHENC
My previous bugfix was incomplete, we still have a broken kernel with CRYPTO_DEV_FSL_CAAM=m and CRYPTO_DEV_FSL_DPAA2_CAAM=y: drivers/crypto/caam/caamalg_desc.o: In function `cnstr_shdsc_aead_null_encap': caamalg_desc.c:(.text+0x14): undefined reference to `caam_little_end' drivers/crypto/caam/caamalg_desc.o: In function `cnstr_shdsc_aead_null_encap': caamalg_desc.c:(.text+0x310): undefined reference to `caam_imx' caamalg_desc.c:(.text+0x4a8): undefined reference to `caam_little_end' drivers/crypto/caam/caamalg_desc.o: In function `cnstr_shdsc_aead_null_encap': caamalg_desc.c:(.text+0x664): undefined reference to `caam_imx' Everything is fine for the other combinations: if both are loadable modules, or both are built-in, the flags work as expected, also if only one of the two is enabled. Add a dependency to enforce using one of the working configurations. Overall, I'm still not happy with that dependency, but for now it documents what the code requires. Fixes: 52813ab24959 ("crypto: caam/qi2 - avoid double export") Signed-off-by: Arnd Bergmann <arnd@arndb.de> --- drivers/crypto/caam/Kconfig | 1 + 1 file changed, 1 insertion(+)