| Message ID | 20150702224225.GG1712@linux.vnet.ibm.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
On Thu, Jul 02, 2015 at 03:42:26PM -0700, Nishanth Aravamudan wrote: > Based off the CONFIG_SPU_FS_MODULE code, only attempt to load platform > modules if the nx-842 pseries/powernv drivers are built as modules. > > Otherwise, if CONFIG_DEV_NX_COMPRESS=y, > CONFIG_DEV_NX_COMPRESS_PSERIES=y, CONFIG_DEV_NX_POWERNV=y, the following > message is emitted at boot: > > nx_compress: no nx842 driver found. > > even though the drivers successfully loads. > > This is because in the =y case, the module_init() calls get converted to > initcalls and the nx842_init() runs before the platform driver > nx842_pseries_init() or nx842_powernv_init() functions, which are what > normally set the static platform driver. > > Signed-off-by: Nishanth Aravamudan <nacc@linux.vnet.ibm.com> > Cc: Dan Streetman <ddstreet@us.ibm.com> > Cc: Herbert Xu <herbert@gondor.apana.org.au> > Cc: "David S. Miller" <davem@davemloft.net> > Cc: linux-crypto@vger.kernel.org > Cc: linuxppc-dev@lists.ozlabs.org Ugh, I think this whole thing is redundant. The whole point of the crypto API is to allow the coexistence of multiple underlying implementations. Please get rid of nx-842-platform.c completely and move the crypto registration into the individual platform drivers. That is, powernv and pseries should each register their own crypto driver. They can of course share a common set of crypto code which can live in its own module. There should be no need for mucking with module reference counts at all. Thanks,
On 06.07.2015 [16:13:07 +0800], Herbert Xu wrote: > On Thu, Jul 02, 2015 at 03:42:26PM -0700, Nishanth Aravamudan wrote: > > Based off the CONFIG_SPU_FS_MODULE code, only attempt to load platform > > modules if the nx-842 pseries/powernv drivers are built as modules. > > > > Otherwise, if CONFIG_DEV_NX_COMPRESS=y, > > CONFIG_DEV_NX_COMPRESS_PSERIES=y, CONFIG_DEV_NX_POWERNV=y, the following > > message is emitted at boot: > > > > nx_compress: no nx842 driver found. > > > > even though the drivers successfully loads. > > > > This is because in the =y case, the module_init() calls get converted to > > initcalls and the nx842_init() runs before the platform driver > > nx842_pseries_init() or nx842_powernv_init() functions, which are what > > normally set the static platform driver. > > > > Signed-off-by: Nishanth Aravamudan <nacc@linux.vnet.ibm.com> > > Cc: Dan Streetman <ddstreet@us.ibm.com> > > Cc: Herbert Xu <herbert@gondor.apana.org.au> > > Cc: "David S. Miller" <davem@davemloft.net> > > Cc: linux-crypto@vger.kernel.org > > Cc: linuxppc-dev@lists.ozlabs.org > > Ugh, I think this whole thing is redundant. The whole point of > the crypto API is to allow the coexistence of multiple underlying > implementations. Sure, that makes sense -- sorry, I was picking this up while Dan was on vacation. Will provide a better v2. > Please get rid of nx-842-platform.c completely and move the crypto > registration into the individual platform drivers. That is, powernv > and pseries should each register their own crypto driver. They can of > course share a common set of crypto code which can live in its own > module. There should be no need for mucking with module reference > counts at all. Will do, thanks! -Nish
On Mon, Jul 6, 2015 at 1:07 PM, Nishanth Aravamudan <nacc@linux.vnet.ibm.com> wrote: > On 06.07.2015 [16:13:07 +0800], Herbert Xu wrote: >> On Thu, Jul 02, 2015 at 03:42:26PM -0700, Nishanth Aravamudan wrote: >> > Based off the CONFIG_SPU_FS_MODULE code, only attempt to load platform >> > modules if the nx-842 pseries/powernv drivers are built as modules. >> > >> > Otherwise, if CONFIG_DEV_NX_COMPRESS=y, >> > CONFIG_DEV_NX_COMPRESS_PSERIES=y, CONFIG_DEV_NX_POWERNV=y, the following >> > message is emitted at boot: >> > >> > nx_compress: no nx842 driver found. >> > >> > even though the drivers successfully loads. >> > >> > This is because in the =y case, the module_init() calls get converted to >> > initcalls and the nx842_init() runs before the platform driver >> > nx842_pseries_init() or nx842_powernv_init() functions, which are what >> > normally set the static platform driver. >> > >> > Signed-off-by: Nishanth Aravamudan <nacc@linux.vnet.ibm.com> >> > Cc: Dan Streetman <ddstreet@us.ibm.com> >> > Cc: Herbert Xu <herbert@gondor.apana.org.au> >> > Cc: "David S. Miller" <davem@davemloft.net> >> > Cc: linux-crypto@vger.kernel.org >> > Cc: linuxppc-dev@lists.ozlabs.org >> >> Ugh, I think this whole thing is redundant. The whole point of >> the crypto API is to allow the coexistence of multiple underlying >> implementations. > > Sure, that makes sense -- sorry, I was picking this up while Dan was on > vacation. Will provide a better v2. > >> Please get rid of nx-842-platform.c completely and move the crypto >> registration into the individual platform drivers. That is, powernv >> and pseries should each register their own crypto driver. They can of >> course share a common set of crypto code which can live in its own >> module. There should be no need for mucking with module reference >> counts at all. > > Will do, thanks! Yep, I originally did it this way because I didn't realize crypto could register different drivers with the same alg name (but different driver names). I have some patches already to start doing this but they weren't ready enough to send before I left for vacation; I'll finish them up and send them. > > -Nish > > -- > To unsubscribe from this list: send the line "unsubscribe linux-crypto" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/crypto/nx/nx-842-platform.c b/drivers/crypto/nx/nx-842-platform.c index 664f13dd06ed..5363c72593b4 100644 --- a/drivers/crypto/nx/nx-842-platform.c +++ b/drivers/crypto/nx/nx-842-platform.c @@ -53,6 +53,7 @@ void nx842_platform_driver_unset(struct nx842_driver *_driver) } EXPORT_SYMBOL_GPL(nx842_platform_driver_unset); +#if defined(CONFIG_CRYPTO_DEV_NX_COMPRESS_PSERIES_MODULE) || defined(CONFIG_CRYPTO_DEV_NX_COMPRESS_POWERNV_MODULE) bool nx842_platform_driver_get(void) { bool ret = false; @@ -66,7 +67,6 @@ bool nx842_platform_driver_get(void) return ret; } -EXPORT_SYMBOL_GPL(nx842_platform_driver_get); void nx842_platform_driver_put(void) { @@ -77,6 +77,17 @@ void nx842_platform_driver_put(void) spin_unlock(&driver_lock); } +#else +bool nx842_platform_driver_get(void) +{ + return true; +} + +void nx842_platform_driver_put(void) +{ +} +#endif +EXPORT_SYMBOL_GPL(nx842_platform_driver_get); EXPORT_SYMBOL_GPL(nx842_platform_driver_put); MODULE_LICENSE("GPL");
Based off the CONFIG_SPU_FS_MODULE code, only attempt to load platform modules if the nx-842 pseries/powernv drivers are built as modules. Otherwise, if CONFIG_DEV_NX_COMPRESS=y, CONFIG_DEV_NX_COMPRESS_PSERIES=y, CONFIG_DEV_NX_POWERNV=y, the following message is emitted at boot: nx_compress: no nx842 driver found. even though the drivers successfully loads. This is because in the =y case, the module_init() calls get converted to initcalls and the nx842_init() runs before the platform driver nx842_pseries_init() or nx842_powernv_init() functions, which are what normally set the static platform driver. Signed-off-by: Nishanth Aravamudan <nacc@linux.vnet.ibm.com> Cc: Dan Streetman <ddstreet@us.ibm.com> Cc: Herbert Xu <herbert@gondor.apana.org.au> Cc: "David S. Miller" <davem@davemloft.net> Cc: linux-crypto@vger.kernel.org Cc: linuxppc-dev@lists.ozlabs.org --- drivers/crypto/nx/nx-842-platform.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-)